TokenSet draft #1317
TokenSet draft #1317
Conversation
| constructor(sdk: OktaAuthCoreInterface) { | ||
| this.sdk = sdk; | ||
|
|
||
| this.map = new Map(); |
There was a problem hiding this comment.
I would move the map to a higher level (see below)
| } | ||
| } | ||
|
|
||
| useTokenSet(key) { |
There was a problem hiding this comment.
I think this method should be moved up a level and allow for "querying" of multiple criteria, like scopes, acr_values
authClient.with({name: 'token_set_key'})
// or
authClient.with({scopes: ['offline_access']})
// or
authClient.with({acr: 'acr_value'}).with will return a TokenSet that matches the criteria that was "queried" for
There was a problem hiding this comment.
We will need to make sure each query end up with only one token set, so maybe just use one type of identifier. For internal api (myaccount), we can find the best practice to pick the key, otherwise it leaves to devs to figure out the key.
|
|
||
| constructor(...args: any[]) { | ||
| super(...args); | ||
|
|
||
| // AuthStateManager | ||
| this.authStateManager = new AuthStateManager<M, S, O>(this); | ||
|
|
||
| // ServiceManager | ||
| this.serviceManager = new ServiceManager<M, S, O>(this, this.options.services); | ||
| this.tokenSet = new TokenSet(this); |
There was a problem hiding this comment.
Something like:
this.map = new Map();
const defaultTokenSet = new TokenSet(this);
this.map.set(DEFAULT_TOKEN_SET_KEY, defaultTokenSet);There was a problem hiding this comment.
so authjs itself becomes the manager of different token sets, and TokenSet becomes a logic group for TokenManager and ServiceManager?
There was a problem hiding this comment.
personally, I think we should avoid put too much management logic in the top level instance, maybe rename TokenSet as TokenSetManager?
|
|
||
| this.map = new Map(); | ||
| const options = sdk.options; | ||
| const tokenManager = new TokenManager(this.sdk, options.tokenManager); |
There was a problem hiding this comment.
As I look through the code some more, maybe we want to have the TokenSet contain an instance of the oidc interface rather than purely a TokenManager instance
https://github.com/okta/okta-auth-js/blob/master/lib/oidc/mixin/index.ts
this contains the methods like isAuthenticated and getIdToken etc we will want to expose on the TokenSet class
There was a problem hiding this comment.
I think it comes back to the question, should we manage one or multiple authState, ideally isAuthenticated and getIdToken should return the same result as the in memory authState, and those wrapper method does not add too much value.
For this change, I think we can focus on the following goals, and add extra public methods at later stage if needed:
- add public interface/s to support multiple token sets
- keep existing methods functional by default
No description provided.