fix(OIDC): typo + make PKCE and nonce mandatory as per specs#491
Merged
Conversation
commit: |
PKCE and nonce mandatory as per latest spec revisionsPKCE and nonce mandatory as per specs
atinux
approved these changes
Jan 26, 2026
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.
Followup for #444
Excluding what seems a small typo, I've wanted to jump in and help with the OIDC, and thus OAuth, specs.
First off made sure
PKCEis always used. As per spec this is not a replacement for client secrets, but instead a mitigation for man-in-the-middle and replay attacks. It was indeed originally developed for public clients in 2015 via RFC7636 but over the time it got adopted as best practice also for confidential clients, now required as per OAuth 2.1 drafts.Also added the
nonceclaim generation and check. Since the current implementation uses OIDC as a SSO provider it is best to follow the OIDC Core 1.0 spec and make sure that the issued tokens are actually bound to that specific client request. Its goal is to also mitigate on replay attacks, in fact this was the original implementation of such prevention that made into OIDC in 2014, but never in OAuth since the PKCE spec was already in draft (which did brought a number of advantages IMHO). This theoretically could be made as an opt-out feature, but since we are using OIDC as a SSO identity provider it would also mean that such IdP should conform to the best practices of the last decade or so.P.S.: To check for
nonceI limited myself to only decode theid_tokenwhich means we are not actually checking its signature, this is for two reasons:access_tokenis compromised and we have no way to know or mitigate this.