duo_api_nodejs icon indicating copy to clipboard operation
duo_api_nodejs copied to clipboard

Proposal of evolution for the library

Open guenoledc opened this issue 5 years ago • 5 comments

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)

guenoledc avatar Jan 10 '20 21:01 guenoledc

@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 )

CalebAlbers avatar Jul 10 '20 18:07 CalebAlbers

@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.

AaronAtDuo avatar Jul 15 '20 14:07 AaronAtDuo

@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 avatar Sep 20 '20 16:09 driverjb

@AaronAtDuo. Ready to go: #31

driverjb avatar Sep 21 '20 23:09 driverjb

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

driverjb avatar Aug 12 '22 00:08 driverjb