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

Add support for 'scope' claim with multiple scopes #359

Merged

Conversation

tony2001
Copy link
Contributor

@tony2001 tony2001 commented Jul 8, 2024

Claim 'scope' is formally standardized by this RFC:
https://datatracker.ietf.org/doc/html/rfc8693#name-scope-scopes-claim

It's been in use for quite a while though and it's a part of standard token provided by Keycloak, which is a long time industry standard for access management.
Keycloak tokens may and quite often do contain multiple scopes, hence the need for this patch.

@simo5
Copy link
Member

simo5 commented Jul 8, 2024

Code looks good thanks!
Please fix the style issues, you can test them locally using tox

Copy link
Member

@simo5 simo5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style, otherwise LGTM

@tony2001 tony2001 force-pushed the validate_scope_claim_support_multiple_scopes branch from 62c8e8d to f72a818 Compare July 8, 2024 17:10
@tony2001
Copy link
Contributor Author

tony2001 commented Jul 8, 2024

I'm not really familiar with Python, but I think I've fixed the issues.
Could check again pls?

@tony2001 tony2001 requested a review from simo5 July 8, 2024 17:12
Copy link
Member

@simo5 simo5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@simo5
Copy link
Member

simo5 commented Jul 8, 2024

The failing test is codespell being stupid and flagging a function name in code you have not touched, I'll fix it later.

Thanks for the contribution, really appreciated!

@simo5 simo5 merged commit f105494 into latchset:main Jul 8, 2024
13 of 14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants