-
Notifications
You must be signed in to change notification settings - Fork 513
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
feat: create new JWT tokens and invalidate older for multitenancy #1725
feat: create new JWT tokens and invalidate older for multitenancy #1725
Conversation
Signed-off-by: Timo Glastra <timo@animo.id>
Codecov Report
@@ Coverage Diff @@
## main #1725 +/- ##
=======================================
Coverage 95.26% 95.26%
=======================================
Files 528 528
Lines 33109 33118 +9
=======================================
+ Hits 31541 31550 +9
Misses 1568 1568 |
I love the However this deterministic quality is lost due to the inability to retrieve current token. A more optimal solution will be to use the stored |
Thanks for the feedback @MonolithicMonk! How would that exactly work with updating the However if we start rotating e.g. the JWT Secret based on the |
I see your general point regarding So a developer can a write script that retrieves all wallet records where |
And yes this update should occur using the current update endpoint. Although the iat in the record doesn't reflect a token, updating its value should be trigger to generate a new token |
I think this is a good approach. Regarding it being a breaking change - this just means that any existing tokens will be invalid and the tenant will need to request a new token, right? |
No, existing tokens will still be valid until a new token is created (we could easily swap it to invalidate all existing tokens). The breaking change in mostly in behaviour. Previously whenever you called the get token endpoint you would get the same token. Now it will always be a new token, invalidating the old token. |
That sounds fine to me. |
Signed-off-by: Timo Glastra <timo@animo.id>
Ok. added a short note to the docs that creating a new token will invalidate the old token. |
Signed-off-by: Timo Glastra <timo@animo.id>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Based on previous reviews from @andrewwhitehead and @ianco , and conversation with @MonolithicMonk . Most recent commit was a minor doc update.
Thank you all who worked on this feature! I'm impressed how fast you implemented a solution after the topic came up on Discord. |
Tokens will now include an
iat
value which is also stored in the wallet record. If theiat
of the JWT doesn't match the user won't be authorized. This means when a new token is created (using the multitenant apis) the old token becomes invalid.Looking for some input if this is a desired approach. It is a breaking change as previously you would always get the same token, while now you always get a new token revoking the older tokens.