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

feat: JWT parsing with custom claims #253

Merged
merged 1 commit into from
Feb 16, 2024
Merged

Conversation

gkats
Copy link
Member

@gkats gkats commented Feb 16, 2024

Replaced the CustomClaims parameter with a CustomClaimsConstructor function when verifying a session JWT. The option is also available in the HTTP middleware.

The constructor function will be called when the JWT is parsed, producing a new struct instance instead of writing on a single instance.

The custom claims will be made available in the SessionClaims.Custom field.

@gkats gkats requested a review from a team as a code owner February 16, 2024 12:21
@gkats gkats requested a review from georgepsarakis February 16, 2024 12:21
@gkats gkats force-pushed the core-1490-custom-claims-func branch 2 times, most recently from b11bc36 to be62a04 Compare February 16, 2024 12:30
// // custom claims are available in the SessionClaims.Custom field.
// sessionClaims, ok := clerk.SessionClaimsFromContext(r.Context())
// customClaims, ok := sessionClaims.Custom.(*MyCustomClaims)
func CustomClaimsConstructor(constructor func() any) AuthorizationOption {
Copy link
Contributor

Choose a reason for hiding this comment

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

❓ Would it make sense to pass the request context in the constructor?

Suggested change
func CustomClaimsConstructor(constructor func() any) AuthorizationOption {
func CustomClaimsConstructor(constructor func(ctx context.Context) any) AuthorizationOption {

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, but since this goes down to jwt.Verify, the context might not make sense there.

We can have different params for middleware and jwt.Verify though. I'll revise this.

Copy link
Member Author

Choose a reason for hiding this comment

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

I was wrong, jwt.Verify does accept a context. 😄

Updated.

Copy link
Contributor

Choose a reason for hiding this comment

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

Mostly noted this because it's going to be tough to change afterwards.

Copy link
Member Author

Choose a reason for hiding this comment

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

And you were 100% right to do so 🙏 😄

jwt/jwt.go Show resolved Hide resolved
jwt/jwt.go Show resolved Hide resolved
Replaced the CustomClaims parameter with a CustomClaimsConstructor
function when verifying a session JWT. The option is also available in
the HTTP middleware.
The constructor function will be called when the JWT is parsed,
producing a new struct instance instead of writing on a single instance.
The custom claims will be made available in the SessionClaims.Custom
field.
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