Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proposal of evolution for the library #25

Open
guenoledc opened this issue Jan 10, 2020 · 5 comments
Open

Proposal of evolution for the library #25

guenoledc opened this issue Jan 10, 2020 · 5 comments

Comments

@guenoledc
Copy link

Please accept this modest contribution to make your library support Promise format.

This is what I add after the lib import to be able to use it in async/await

const duo_api = require('@duosecurity/duo_api')

duo_api.Client.prototype.asyncApiCall = async function(method, path, params) {
    return new Promise( (resolve, reject)=>{
        try {
            this.jsonApiCall(method, path, params, resolve)
        } catch (error) {
            resolve({
                stat: 'ERROR',
                message: error.message ? error.message : error
            })
        }
    })
}

duo_api.Client.prototype.get = async function (path, params) {
    return this.asyncApiCall('GET', path, params)
}
duo_api.Client.prototype.post = async function (path, params) {
    return this.asyncApiCall('POST', path, params)
}

That can then be used nicely as follow

    const request = new duo_api.Client(integrationKey, secretKey, duoHost);

    const auth = await request.post('/auth/v2/auth', {
        user_id: user_id,
        factor: 'push',
        async: true,
        device: 'auto',
        type: 'Test'
    })
    console.log('AUTH:', auth)


    const status = await request.get('/auth/v2/auth_status', {txid: auth.response.txid})
    console.log('STATUS:', auth)
@CalebAlbers
Copy link

@vbscott / @yizshi or anyone else on the Duo team, would you consider adding this? It would make Duo's package much more idiomatic with how NodeJS is used today.

It appears that @driverjb made a PR implementing this already (#22 )

@AaronAtDuo
Copy link
Contributor

@CalebAlbers Thanks for pinging us on this. Let me admit right away that I am hopelessly behind the times on Node JS development, so please forgive any weird questions I might ask until I can find some internal experts to get their thoughts.

We do want to make our clients useful and easy for as many developers as possible. So in that light, could this change cause problems for anyone? For instance, is the Promise format only available after a certain version? Are there commonly used frameworks that don't support it? Would this be a backwards compatible change that users can simply drop in without updating their application code? etc. etc.

I would love to get with the times, but as long as it doesn't cause problems for anyone that's been using the client as-is.

@driverjb
Copy link
Contributor

@AaronAtDuo if you check out my old PR (#22) you will see I have been using this in production and it's compatible with your existing code. Promises are standard in all of the currently supported versions of NodeJS. I can make a new PR and clean it up a bit since my newer production code we use is even further updated. I think there were some auto-formatting issues in my last one. I designed the update so that all of the existing calls are left intact and the new calls just use a promise version of the existing calls.

@driverjb
Copy link
Contributor

driverjb commented Sep 21, 2020

@AaronAtDuo. Ready to go: #31

@driverjb
Copy link
Contributor

For anyone interested I have started my own package for this API. It’s available here: https://www.npmjs.com/package/duo-admin-api

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants