okta / okta-auth-js

The official js wrapper around Okta's auth API
Other
447 stars 260 forks source link

ExtraParams are not included during token renewal #1524

Closed MelvinVermeer closed 3 weeks ago

MelvinVermeer commented 2 months ago

Describe the bug

When signing in using the following snippet, I would expect the same extraParams object to be used when renewing the token.

await oktaAuth.signInWithRedirect({
  extraParams: {
    organizationId: 'some uuid',
  },
})
// the following DID NOT include the extra params in the authorize url.
await oktaAuth.tokenManager.renew('accessToken')

Reproduction Steps?

  1. Sign in with some extraParams.
  2. Call oktaAuth.tokenManager.renew('accessToken')

Observe that in the first call (sign in) the extraParams are in the authorize url, while in the second call (renewal) they aren't.

SDK Versions

System: OS: macOS 14.4.1 CPU: (12) arm64 Apple M3 Pro Memory: 63.22 MB / 18.00 GB Shell: 5.9 - /bin/zsh Binaries: Node: 18.19.0 - ~/.nvm/versions/node/v18.19.0/bin/node Yarn: 1.22.22 - /opt/homebrew/bin/yarn npm: 10.2.3 - ~/.nvm/versions/node/v18.19.0/bin/npm Browsers: Chrome: 126.0.6478.127 Firefox: 127.0.2 Safari: 17.4.1 npmPackages: @okta/okta-auth-js: 7.7.0 => 7.7.0 @okta/okta-react: 6.9.0 => 6.9.0

Additional Information?

Proposed solution #1525

Store the extraParams during the sign in process, and reuse when renewing the token. A working version of this is implemented in PR #1525.

Alternative solution

Another way this could work is implementing a second, optional parameter for the renew function with the type RenewTokensParams.

await oktaAuth.tokenManager.renew('accessToken', { extraParams: { organizationId: 'some uuid'  })

I chose not to implement this solution because it would eventually require changing getOrRenewAccessToken(). In my opinion having the behavior implicitly is just convenient.

jaredperreault-okta commented 2 months ago

@MelvinVermeer I assume you're not using refresh tokens? (aka including the offline_access scope)

MelvinVermeer commented 2 months ago

We are not using refresh tokens, that's right.

ljvanschie commented 2 months ago

Slightly related.. when you do work with refresh tokens, the extraParams are not passed in the token request when calling

await oktaAuth.token.renewTokens({ extraParams: { organizationId: 'test' } });

As you can see here, options.extraParams is not included when building the query string: https://github.com/okta/okta-auth-js/blob/master/lib/oidc/endpoints/token.ts#L134

It would be very useful if both flows would work, and that the extraParams you initially pass to signInWithRedirect, would subsequently be used in the token auto renewal (as @MelvinVermeer suggested).

jaredperreault-okta commented 2 months ago

@MelvinVermeer @ljvanschie Do you mind expanding on your use case a bit? Since these appended params won't be consumed by the Authorization Server, I am assuming they are for tracking purposes. If so, perhaps providing a custom http client that appends a header would be sufficient (docs)

ljvanschie commented 2 months ago

Ah yes of course. Our account model is based on organizations. A user can be part of multiple organizations, but claims are dependent of the organization that the token is requested for.

To do this, a token inline hook is configured in the Okta access policy, which is a piece of custom code that reads the organizationId from extraParams and fetches the permissions and settings (from a database) for that user-organization combination. It then applies these to the token so that the JWT will contain the specific claims.

Users in our web app can select the organization they work for from a menu. Then, we signInWithRedirect for that organization and this all works fine. But when we auto-renew the tokens, the extraParams are lost.

MelvinVermeer commented 2 months ago

For anyone that comes across this issue, and is in need for a quick workaround.

This is the patch (similar to the mentioned PR) that solves the issue for our team in the mean time.

diff --git a/node_modules/@okta/okta-auth-js/esm/browser/oidc/TokenManager.js b/node_modules/@okta/okta-auth-js/esm/browser/oidc/TokenManager.js
index 79931e7..fca7212 100644
--- a/node_modules/@okta/okta-auth-js/esm/browser/oidc/TokenManager.js
+++ b/node_modules/@okta/okta-auth-js/esm/browser/oidc/TokenManager.js
@@ -330,7 +330,8 @@ class TokenManager {
             return Promise.reject(err);
         }
         this.clearExpireEventTimeout(key);
-        const renewPromise = this.state.renewPromise = this.sdk.token.renewTokens()
+        const extraParams = this.sdk.storageManager.getSharedTansactionStorage().getItem('extraParams');
+        const renewPromise = this.state.renewPromise = this.sdk.token.renewTokens({ extraParams })
             .then(tokens => {
             this.setTokens(tokens);
             if (!token && key === 'accessToken') {
diff --git a/node_modules/@okta/okta-auth-js/esm/browser/oidc/getWithRedirect.js b/node_modules/@okta/okta-auth-js/esm/browser/oidc/getWithRedirect.js
index 449f92d..aa9dcb4 100644
--- a/node_modules/@okta/okta-auth-js/esm/browser/oidc/getWithRedirect.js
+++ b/node_modules/@okta/okta-auth-js/esm/browser/oidc/getWithRedirect.js
@@ -26,6 +26,7 @@ async function getWithRedirect(sdk, options) {
     const tokenParams = await prepareTokenParams(sdk, options);
     const meta = createOAuthMeta(sdk, tokenParams);
     const requestUrl = meta.urls.authorizeUrl + buildAuthorizeParams(tokenParams);
+    sdk.storageManager.getSharedTansactionStorage().setItem('extraParams', tokenParams.extraParams);
     sdk.transactionManager.save(meta);
     if (sdk.options.setLocation) {
         sdk.options.setLocation(requestUrl);
jaredperreault-okta commented 1 month ago

Internal Ref: OKTA-750886

jaredperreault-okta commented 1 month ago

@MelvinVermeer @kev1nboer @ljvanschie

Are you able to try this branch (https://github.com/okta/okta-auth-js/pull/1529) and confirm it works for your use case?

ljvanschie commented 1 month ago

Thank you for the quick action, token renewal works as expected, it keeps using the same extraParams! 🙌

jaredperreault-okta commented 1 month ago

Thanks for the confirmation. I'll need to add some tests before this can be merged. Should be released shortly

ljvanschie commented 1 month ago

Sounds great. Oh there is one more thing I need to test as well. I checked whether the extraParams remain in the refresh token after token renewal, but I also need to confirm whether it actually reaches the token inline hook. Will let you know tomorrow.

MelvinVermeer commented 1 month ago

@jaredperreault-okta 👍 works for my case. Thanks

ljvanschie commented 1 month ago

I tested it again and unfortunately I don't see the extraParams being appended to the /token url when using refresh tokens. Our current inline hook implementation tries to fetch the extra params from the url (oktaRequest.Data.Context.Request.Url), which works fine for the 'normal' sign in flow.

When I inspect the auto-renewal token requests that okta-auth-js performs on the background, I also don't see the extraParams in the formdata. So I'm trying to find out how to access those in the inline hook.

jaredperreault-okta commented 1 month ago

@ljvanschie I misunderstood the token integration; the params are meant to be appended as query params, I added them to the post body. I just pushed an update, mind confirming one more time?

ljvanschie commented 1 month ago

Thanks a lot, it now works good! 🎉

ljvanschie commented 1 month ago

When do you expect this to be released? Is there anything we can do to help?

jaredperreault-okta commented 3 weeks ago

This was resolved by #1529 and just released to npm in 7.7.1