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

Strip namespace prefix from custom claims #4061

Draft
wants to merge 4 commits into
base: 1.x
Choose a base branch
from

Conversation

gguillemas
Copy link
Contributor

@gguillemas gguillemas commented May 17, 2024

Thank you for submitting this pull request! We really appreciate you spending the time to work on these changes.

What is the motivation?

To behave similarly with custom JWT claims as with default JWT claims expected by SurrealDB. With default claims, names are aliased to support namespace prefixes. With custom claims, namespace prefixes will be stripped so that the canonical claim names can be directly accessible using the token parameter.

What does this change do?

Strips namespace prefixes from custom claims before adding it to the token object unless the claim already exists to prevent overwriting a public claim with a private one. Keeps the original claims accessible through the $token parameter in order to preserve backward compatibility in 1.X. This will not be the case in 2.X.

What is your testing strategy?

Add a test to ensure that namespaced custom claims are correctly extracted.

Is this related to any issues?

Does this change need documentation?

No.

Have you read the Contributing Guidelines?

@gguillemas gguillemas added the topic:security This is related to security label May 17, 2024
@gguillemas gguillemas marked this pull request as ready for review May 21, 2024 08:00
@gguillemas gguillemas requested a review from a team as a code owner May 21, 2024 08:00
@gguillemas
Copy link
Contributor Author

Let's hold off on this PR for a bit more. It addresses the issue, but I would like to implement a proper solution. Any small fixes like this one will either cause private claims (i.e. custom claims) to overwrite public ones or cause private claims to not be processed if there is already a public one with the same name, which is what namespacing is intended to prevent.

@gguillemas gguillemas marked this pull request as draft May 21, 2024 17:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic:security This is related to security
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants