Merged
Conversation
Contributor
|
Looks and tests out great 💪 🥳 |
kjac
approved these changes
Dec 7, 2022
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR changes the OpenId connect implementation to use reference tokens, token entry validation, and the token format to ASP.NET Core Data Protection, instead of JWT.
For a more in-depth read about all of these things you can see:
But long story short, it has the following implications:
Reference Tokens
Enabling reference tokens means that the actual payload for the token isn't sent to the client, it is instead stored in the database, and a crypto-secure identifier is sent to the client, that identifies the stored token, this has the benefit of the token that needs to be sent to the client being smaller. But that does mean that the token could be used for impersonation, should the database be compromised, it's therefore highly recommended to use encryption at rest or ASP.NET Core Data Protection.
Token Entry Validation
Token entry validation means that every time a token is retrieved it will be validated, this does incur a performance penalty in the sense that we'll have to do a database lookup whenever we need to authenticate a token, the main benefit here though is that it allows us to do immediate access token revocation, for instance when disabling a back-office user, or changing permissions.
ASP.NET Core Data Protection
Using ASP.NET Core Data Protection for the token format means that the token itself will be encrypted using symmetric encryption, this alleviates the issue posed by using reference tokens, the other benefit here is that as per the open id dict documentation "ASP.NET Core Data Protection has been designed to achieve high throughput as it's natively used by ASP.NET Core for authentication cookies, antiforgery tokens and session cookies.", which is great additionally the tokens are generally a bit shorter.
The data protection approach does come with a bit of a downside though, mainly that if the authorization and resources servers are not part of the same application, for instance in a load balanced setup, ASP.NET Core Data Protection MUST be configured to use the same application name and share the same key ring. This isn't that big of a deal in this scenario though, since we already require that for load balanced setups.
Testing
The main thing here is to ensure that the auth still works, to do that I followed the same testing steps as #13318