Skip to content
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

♻️(backend) rename required claims to essential claims as per spec #531

Merged
merged 2 commits into from
Jan 10, 2025

Conversation

sampaccoud
Copy link
Member

@sampaccoud sampaccoud commented Dec 27, 2024

Purpose

It was pointed by @lebaudantoine that the OIDC specification uses the term "essential claims" for what we called "required claims".

Further more, the Mozilla OIDC library that we use, validates claims in a method called "verify_claims".
Let's do it in the same method by overriding it.

Proposal

  • rename "required claims" to "essential claims"
  • make use of the "verify_claims" method to validate that all essential claims are included in user infos
  • ensure that "sub" is always mandatory as per OIDC specification
  • add tests to cover edge cases

@sampaccoud sampaccoud self-assigned this Dec 27, 2024
@sampaccoud sampaccoud requested a review from qbey December 27, 2024 19:56
@sampaccoud sampaccoud added enhancement New feature or request python Pull requests that update Python code backend labels Dec 27, 2024
@sampaccoud sampaccoud force-pushed the rename-required-claims-to-essential branch from 99bf441 to c56ffce Compare December 27, 2024 19:57
@sampaccoud sampaccoud force-pushed the rename-required-claims-to-essential branch from c56ffce to fb22ba4 Compare December 27, 2024 20:06
@sampaccoud sampaccoud force-pushed the rename-required-claims-to-essential branch 2 times, most recently from 6f82122 to e087cf7 Compare January 3, 2025 13:47
@sampaccoud sampaccoud force-pushed the rename-required-claims-to-essential branch from f06ce87 to 0effe82 Compare January 3, 2025 16:47
It was pointed by @lebaudantoine that the OIDC specification uses
the term "essential claims" for what we called required claims.

Further more, the Mozilla OIDC library that we use, validates claims
in a method called "verify_claims". Let's override this method.
We had doubts that the user was correctly updated in the case where
its identity was matched on the email and not on the sub. I added
a test and confirmed that it was working correctly. I still modified
the backend to update the user based on its "id" instead of its "sub"
because it was confusing, but both actually work the same.
@sampaccoud sampaccoud force-pushed the rename-required-claims-to-essential branch from 0effe82 to 2b2daf6 Compare January 10, 2025 18:20
@sampaccoud sampaccoud enabled auto-merge (rebase) January 10, 2025 18:20
@sampaccoud sampaccoud merged commit 945f55f into main Jan 10, 2025
18 checks passed
@sampaccoud sampaccoud deleted the rename-required-claims-to-essential branch January 10, 2025 18:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend enhancement New feature or request python Pull requests that update Python code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants