intuit / oauth-jsclient

Intuit's NodeJS OAuth client provides a set of methods to make it easier to work with OAuth2.0 and Open ID
https://developer.intuit.com/
Apache License 2.0
125 stars 159 forks source link

Unable to use token when using constructor OAuthClient #7

Closed rickygarg closed 5 years ago

rickygarg commented 5 years ago

OAuthClient needs to be session specific. Hence, I've to initialize the OAuthClient again from its JSON saved in the session. However, that fails because the wrong param seems to be passed to the Token constructor:

function OAuthClient(config) {

    this.token = new Token(config);

}

Proposed solution: Change to:

function OAuthClient(config) {

    this.token = new Token(config.token);

}
abisalehalliprasan commented 5 years ago

Hi @rickygarg :

The client offers helper methods named getToken() and setToken() using the oauthClient instance as shown here

You can set the JSON saved in the session to the token using the above method. Let me know if this helps.

rickygarg commented 5 years ago

Thanks, assuming that this is a workaround until a future commit?

The setToken() doesn't work that smooth because it doesn't seem to set the realmId. (Likewise, getToken also skips on 'id_token' - is that intentional?)

abisalehalliprasan commented 5 years ago

@rickygarg : realmId is passed to the redirectUri as a query parameter when you use OAuth2.0 Scopes and you would need to use id_token when using OpenID scope. ( This is the limitation of passing it directly to the client )

However, based on the valid use case you can optionally pass realmId and id_token using the setToken() here

rickygarg commented 5 years ago

@abisalehalliprasan thanks for including realm_id. However, in the createToken function:

The effect of this

this.getToken().realmId = (params['realmId'] ? params['realmId'] : '');

is now overriden a few lines afterwards by

this.token.setToken(json);

Also is there any reason that the following suggested change doesn't make sense:

function OAuthClient(config) {

    this.token = new Token(config.token);

}
abisalehalliprasan commented 5 years ago

@rickygarg : Can you check the changes I have added. Also, can you create a PR for this, if you have a working version?

rickygarg commented 5 years ago

The last commit with version 1.1.1 resolves this. Thanks.

walexnelson commented 5 years ago

The client offers helper methods named getToken() and setToken() using the oauthClient instance as shown here

You can set the JSON saved in the session to the token using the above method. Let me know if this helps.

I may have just misunderstood this comment and the example in the README, but I just spent a good chunk of time trying to figure out why client.isAccessTokenValid() was returning true even after I knew the access token had expired. I was setting the token via client.getToken().setToken(json).

Turns out client.getToken().setToken() converts seconds into a timestamp - so if you're providing the token that was returned from createToken or refresh like I was (which has already converted the seconds into a timestamp) then be sure you pass it into the config and NOT through the setToken method.

A note might want to be made about this since when would we be fetching a token without this library, and any token returned by this library already has 3600 seconds converted into a timestamp.

abisalehalliprasan commented 5 years ago

@walexnelson : That's a good point to mention. I will revise the README.md and update it. Thank you for your time and patience.