go-sdk icon indicating copy to clipboard operation
go-sdk copied to clipboard

Proposal: Implement Standard JWT fields

Open RyoKusnadi opened this issue 4 months ago • 7 comments

Is your feature request related to a problem? Please describe. In Auth TODO indicates missing standard JWT fields, and i think we can implement JWT Claims using field that stated in Here

Describe the solution you'd like Add new fields to the existing TokenInfo struct using pointer types for optional values maybe something like this

type TokenInfo struct {
    // Existing fields (unchanged)
    Scopes     []string
    Expiration time.Time
    Extra      map[string]any
    
    // New standard JWT fields
    Subject   *string    `json:"sub,omitempty"`   // Subject (user identifier)
    Issuer    *string    `json:"iss,omitempty"`   // Issuer (who issued the token)
    Audience  []string   `json:"aud,omitempty"`   // Audience (intended recipients)
    IssuedAt  *time.Time `json:"iat,omitempty"`   // Issued at time
    NotBefore *time.Time `json:"nbf,omitempty"`   // Not valid before time
    JWTID     *string    `json:"jti,omitempty"`   // JWT ID (unique identifier)
}

Still thinking the access pattern, might be something like below

tokenInfo := auth.TokenInfoFromContext(ctx)
if tokenInfo != nil {
    // Safe access with nil checks
    if tokenInfo.Subject != nil {
        userID := *tokenInfo.Subject
    }
    
    // Audience is a slice, can be checked for length
    if len(tokenInfo.Audience) > 0 {
        primaryAudience := tokenInfo.Audience[0]
    }
    
    // Time fields with nil checks
    if tokenInfo.IssuedAt != nil {
        issuedTime := *tokenInfo.IssuedAt
    }
}

Describe alternatives you've considered

  1. Using the existing Extra map Store JWT claims in TokenInfo.Extra as map[string]any Pros: No struct changes needed Cons: No type safety, requires type assertions, error-prone
  2. Create a separate JWTTokenInfo struct New struct that embeds TokenInfo with JWT fields Pros: Clean separation, no existing struct changes Cons: Breaking change, requires new context functions, fragments the API
  3. Add JWT claims as methods instead of fields Functions like GetSubject(), GetIssuer() that parse from Extra Pros: Backward compatible, encapsulated logic Cons: Performance overhead, still requires Extra map usage NB: Will taking https://github.com/golang-jwt/jwt/blob/main/claims.go#L13 as reference during development

Additional context NA

RyoKusnadi avatar Sep 22 '25 15:09 RyoKusnadi

I'm happy to provide a full PR for this!

RyoKusnadi avatar Sep 22 '25 15:09 RyoKusnadi

hi @jba / @findleyr wondering is my raised issue not clear? please let me know what should i add. sorry this is my first tike raise issue haha

RyoKusnadi avatar Sep 24 '25 17:09 RyoKusnadi

It's clear, we're still evaluating it. I know there's a TODO for this, but I'm not 100% sure we want to add all those fields. I will look more carefully at this soon.

jba avatar Sep 24 '25 18:09 jba

I've confirmed that these are all registered claims (https://datatracker.ietf.org/doc/html/rfc7519#section-4.1). None of them need to be pointers: the empty string or zero time is enough to indicate missing. (They should still have the omitempty tag.)

If there is no objection from @findleyr or @samthanawalla, you can proceed with a PR.

jba avatar Sep 24 '25 18:09 jba

got it, thanks @jba !

please let me know @findleyr / @samthanawalla if i'm okay to proceed.

RyoKusnadi avatar Sep 25 '25 01:09 RyoKusnadi

Fine by me!

samthanawalla avatar Sep 25 '25 14:09 samthanawalla

I’m having second thoughts about this. First of all, I’m not sure we want a TokenInfo to be serializable as a JWT. The two should be separate. That suggests having a separate struct. But the package that we use internally, github.com/golang-jwt/jwt/v5, already has that struct (RegisteredClaims).
However, we don’t want to make that package’s API part of our own.

So let’s say we write our own struct with these registered claims, and make it a field of TokenInfo. Now what happens when someone wants to use a JWT with additional claims? The package I mentioned above handles this by making Claims an interface. If the TokenInfo field is an interface, a user can provide their own implementation with additional claims. If we go this way, we are basically replicating that JWT package.

So I’m not sure what the right thing to do here is. Maybe Extra should have been an any instead of a map, so you could put whatever you want in it. but we can’t change it now.

I think it’s best to wait-and-see what the community comes up with. You can always work around this by setting keys in Extra.

jba avatar Oct 02 '25 18:10 jba