feat(userInfo): implement persisting user info to support limited tokens #24729
Conversation
Changed Packages
|
| @@ -190,7 +189,7 @@ export class TokenFactory implements TokenIssuer { | |||
| alg: key.alg, | |||
| kid: key.kid, | |||
| }, | |||
| payload: { sub, ent, iat, exp }, | |||
There was a problem hiding this comment.
Did we want to remove ent from all tokens now that the user info service is flushed out or just from limited ones?
There was a problem hiding this comment.
The goal is to remove it from all tokens. But we may want to keep it in there still for some time, while the userInfo usage ramps up. As long as plugins exist that look at the claim directly (via an old IdentityService for example), the data may need to stay. It's fine to make the deprecation of that field a separate effort from implementing the endpoint fully.
| /** | ||
| * @param {import('knex').Knex} knex | ||
| */ | ||
| exports.up = async function up(knex) { |
There was a problem hiding this comment.
note that there's a migrations.test.ts that you may want to add this to; follow the pattern of updating to the version before this one, then going the extra step and populating with like a row of data, and then downgrading again
There was a problem hiding this comment.
oh cool wasn't aware of that test file - updated!
| .comment('User entity reference'); | ||
|
|
||
| table | ||
| .text('ownership_entity_refs', 'longtext') |
There was a problem hiding this comment.
An open question here (maybe @Rugvip) - should we make this one just user_info (with all json stringified info) and not storing all possible pieces of data one by one? Not sure that the service really will care about the detailed contents.
For context, we were discussing before about what to do with custom claims - why not store custom claims here too? Then where to put those?
There was a problem hiding this comment.
Yep, I think we will want to store custom claims here too and make this an opaque user info blob.
Signed-off-by: Phil Kuang <pkuang@factset.com>
|
Any rough estimation on when this PR will be merged and part of release as it is affecting all our users (such as leaders, tech leads etc.) who are part of many ad groups? |
Hey, I just made a Pull Request!
Part of BEP-0003
Fixes #24420
✔️ Checklist
Signed-off-byline in the message. (more info)