-
Notifications
You must be signed in to change notification settings - Fork 265
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
getWithRedirect clientId parameter not stored in the session cookie #102
Comments
ClientId is not stored in the session cookie before getWithRedirect() returns. This means that when the clientId is passed as a parameter to getWithRedirect() it is not available for use after the redirect (during the parseFromUrl method). This commit fixes that problem. Resolves: Github Issue okta#102
Anyone able to merge this PR? Or if not why hasn't it been merged? I'd like to get this code in. |
Hey @Daniel-Houston - not sure I'm quite following why you'd need to pass this through the Regarding token validation - best practice is to assert against a known set of claims ( Are you using the It might be helpful if you could give a little bit more information about your flow. Thanks! |
Hey @jmelberg-okta, thanks for the reply! My flow is a little bit wonky, and I think that's what led me to find this problem. I have created a UI 'shell' that loads different single-page apps depending on your url path base. e.g. If I have app1 and app2, and my url is www.example.com, when I navigate to www.example.com, the shell loads and takes me to www.example.com/app1. Then, when I click a top-level menu button it routes me to www.example.com/app2 and loads a different SPA. I know this is a little weird given the notion of a 'Single-Page-App' but our website has some special needs. That being said, I configured both app1 and app2 in Okta as separate applications, and instead of passing in the clientID in the AuthClient constructor, I'm passing in the clientID for my different apps in the getWithRedirect() method, in order to allow me to use one AuthClient for multiple client applications. When I do that, I get an error: 'The jwt, iss, and aud arguments are all required' which I traced to lib/oauthUtil.js#validateClaims line 106.
That clued me in to find that the clientID is being passed to that function from lib/token.js#verifyToken as the audience parameter.
By following the chain up (verifyToken -> handleOAuthResponse -> parseFromUrl), I found that if the clientID isn't given as part of the AuthClient configuration, then it will be undefined when assigned in handleOAuthResponse,
eventually causing the error in validateClaims (referenced above). My solution for this was to include the clientID in the cookie to account for the case that it was passed in as a parameter to one of the get methods (getWithRedirect, etc). If you don't like this approach, I would recommend changing your documentation to say that clientID is required as part of the client configuration here. Does that make more sense? |
Thanks @Daniel-Houston! That helps provide a bit more context in why you're seeing this issue. It seems like we're expecting I'll try and get to #103 this week or next! |
Resolved with |
I am using the getWithRedirect() method to authenticate with okta. One of the parameters I am passing into the method is the clientId:
While doing this I noticed that the clientId is not stored in the session cookie, which means it will not be available for use in the parseFromUrl() function after the redirect, resulting in a token claim validation error. I have created a pull request which resolves this issue.
The text was updated successfully, but these errors were encountered: