WithCustomClaims in Verify token options should accept a type instead of pointer

[vijathanga]

Currently WithCustomClaims function in token_option.go accepts a pointer to a struct variable. So we have to define and pass the struct pointer when initializing the WithSessionV2 middleware. The same struct pointer is used for unmarshalling the custom claims for all incoming requests. This can cause consistency issues when multiple requests are processed at the same time.

This can be solved by either

1. Accept a Custom claim struct type in WithCustomClaims function and dynamically create, populate the struct and set the value in context
2. Provide a function hook for dealing with custom claims

[georgepsarakis]

Hello @vijathanga ! Sorry that you encountered this issue and thanks for reporting. Your observation is correct:

This can cause consistency issues when multiple requests are processed at the same time.

The WithCustomClaims option was designed for direct calls to VerifyToken since the custom claims are assigned directly to the referenced struct instance which would be constructed before each VerifyToken call:

clerk-sdk-go/clerk/clerk.go

Line 51 in 2f041fe

VerifyToken(token string, opts ...VerifyTokenOption) (*SessionClaims, error)

Accept a Custom claim struct type in WithCustomClaims function

Do you perhaps mean a constructor function for new objects that will be populated with the custom claims? Something like:

WithSessionV2(client, WithSessionClaimsFunc(func() interface{} {
  return &MyCustomClaims{}
})

populate the struct and set the value in context

Would exposing the custom claims as a field in the SessionClaims struct that's already available in the context work instead?

clerk-sdk-go/clerk/middleware_v2.go

Line 38 in 2f041fe

func SessionFromContext(ctx context.Context) (*SessionClaims, bool) {

So to sum up the approach assuming a custom struct MyCustomClaims:

client, err := clerk.NewClient(apiKey)
// Configure the middleware
clerk.WithSessionV2(client, WithSessionClaimsFunc(func() interface{} {
  return &MyCustomClaims{}
})

// In the request
sessionClaims, ok := clerk.SessionFromContext(req.Context())
if !ok {
  // unauthenticated
}
customClaims, ok := sessionClaims.Custom.(*MyCustomClaims)
if ok {
  // use customClaims
}

Mostly asking if the final snippet would address your use case, this change will require some additional design considerations from our side!

[vijathanga]

Hi @georgepsarakis, I appreciate the quick response.

The final snippet looks good, It should satisfy our needs. Kindly keep me updated on the changes. Thanks!

[georgepsarakis]

Thanks for the quick follow-up @vijathanga , I've opened a ticket internally so that we can look into this.

[gkats]

@vijathanga Hello, sorry this took so long. We were preparing a new version for the Go SDK.

Here's an attempt to solve the concurrent access issue by providing a constructor for the custom claims.
#253

Feel free to provide feedback and follow along with the discussion.

[gkats]

Closing this since it is fixed in v2.