feat: add custom token exchange support #928
Conversation
yogeshchoudhary147
force-pushed
the
feat/custom-token-exchange
branch
2 times, most recently
from
56fc5c6 to
c34a6bf
Compare
gyaneshgouraw-okta
force-pushed
the
feat/custom-token-exchange
branch
from
7f32394 to
3d98928
Compare
| throw tokenError(error); | ||
| } finally { | ||
| dispatch({ | ||
| type: 'GET_ACCESS_TOKEN_COMPLETE', |
There was a problem hiding this comment.
- This block will execute even if the tokenExchange fails. Do we want to send a
GET_ACCESS_TOKEN_COMPLETEdispatch in the case of error ? - Do we want to dispatch a
TOKEN_EXCHANGE_COMPLETEor a similar event here? Just a suggestion, open to all perspectives on this.
There was a problem hiding this comment.
Acknowledged. @gyaneshgouraw-okta, looping you in to address this comment regarding the implementation design.
There was a problem hiding this comment.
@tusharpandey13 , @gyaneshgouraw-okta and I discussed this. We are inclined to stick with GET_ACCESS_TOKEN_COMPLETE for now.
The reducer is currently designed around the outcome (obtaining a token) rather than the source (Silent Auth vs. CTE). Since the impact on the state is identical, adding a new action type would force us to duplicate the reconciliation logic (or alias the case) without changing the actual behavior. We feel it follows DRY principles better to keep a single source of truth for "token received" events.
There was a problem hiding this comment.
This block will execute even if the tokenExchange fails. Do we want to send a GET_ACCESS_TOKEN_COMPLETE dispatch in the case of error ?
Is this expected behaviour?
There was a problem hiding this comment.
Yes, it is. We are following the exact same pattern as getAccessTokenSilently here to ensure the CTE flow behaves consistently with the rest of the SDK, regardless of success or failure.
There was a problem hiding this comment.
Since this is to enable backwards compat, this behaviour should be documented (CTE + normal token flows)
There was a problem hiding this comment.
Just to clarify, do you mean adding an inline code comment explaining why we reuse GET_ACCESS_TOKEN_COMPLETE, or are you asking for an update in the public docs?
There was a problem hiding this comment.
Addressed in the latest commit. I documented the backward compatibility behavior both in the source code (inline) and in the EXAMPLES.md guide.
EXAMPLES.md
Outdated
| const tokenResponse = await exchangeToken({ | ||
| subject_token: externalToken, | ||
| subject_token_type: 'urn:your-company:legacy-system-token', | ||
| authorizationParams: { |
There was a problem hiding this comment.
I don't think exchangeToken accepts authorizationParams. options is of type CustomTokenExchangeOptions and it's coming from auth0-spa-js here: https://auth0.github.io/auth0-spa-js/types/CustomTokenExchangeOptions.html
Can you confirm this ?
There was a problem hiding this comment.
Good catch. You're right, exchangeToken expects those at the root level, not nested. I'll push a fix now.
There was a problem hiding this comment.
Updated. Thanks again for spotting that!
**Added** - feat: add custom token exchange support [\#928](#928) ([gyaneshgouraw-okta](https://github.com/gyaneshgouraw-okta)) --------- Co-authored-by: Frederik Prijck <frederik.prijck@auth0.com>
📝 Summary
This PR adds support for the
exchangeTokenmethod from auth0-spa-js to auth0-react, enabling token exchange flows based on RFC 8693 This allows apps to exchange external or legacy tokens for Auth0 tokens directly in React.🔧 Changes
exchangeTokento theAuth0ContextInterfacewith full TypeScript and JSDoc support.exchangeTokenusing auseCallbackwith proper error handling.useAuth0docs to include the new method.EXAMPLES.md.🧪 Testing
💥 Impact