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

'aud' claim is not necessarily a string #114

Closed
pedromss opened this issue Oct 22, 2021 · 3 comments
Closed

'aud' claim is not necessarily a string #114

pedromss opened this issue Oct 22, 2021 · 3 comments

Comments

@pedromss
Copy link

Hi. Was wiring things up in a service and noticed that jwt.StandardClaims assumes aud is a string.

According to the RFC https://datatracker.ietf.org/doc/html/rfc7519#section-4.1.3 this field can sometimes be a string. The way its written leads me to believe a []string is actually more common, but that's interpretation.

The quick way I found around this was to use jwt.MapClaims and serialize + deserialize this into a custom struct. Could this be improved by transforming aud into a interface{} and having a method on jwt.StandardClaims like:

type St struct {
	Aud interface{} `json:"aud"`
}

func (sc St) Audience() ([]string, error) {
  if v, ok := sc.Aud.(string); ok {
  	return []string{v}, nil
  } else if v, ok := sc.Aud.([]interface{}); ok {
  	var results []string
	  for _, s := range v {
	  	if asString, isString := s.(string); isString {
			results = append(results, asString)
		} else {
			return nil, e.New("found an aud that is not a string")
		}
	  }
  	return results, nil
  }

  return nil, e.New("unparsable aud claim")
}

Simple tests to verify it works at some level at least:

func TestBla(t *testing.T) {
	var singleAud St
	err := json.Unmarshal([]byte(`{ "aud": "bla" }`), &singleAud)
	require.NoError(t, err)
	audience, err := singleAud.Audience()
	require.NoError(t, err)
	require.ElementsMatch(t, audience, []string{"bla"})

	var multiAud St
	err = json.Unmarshal([]byte(`{ "aud": [ "bla", "ble" ] }`), &multiAud)
	require.NoError(t, err)
	audience, err = multiAud.Audience()
	require.NoError(t, err)
	require.ElementsMatch(t, audience, []string{"bla", "ble"})
}

If aud is the only parameter that can be either a string or a []string adding a new type might be a more pleasant experience:

type StandardClaimsWithSingleAud struct {
  Aud string ...
}

type StandardClaimsWithMulitAud struct {
  Aud []string ...
}

Is there already a way to deal with this in a typed fashion instead of dealing with the untyped jwt.MapClaims?

@mfridman
Copy link
Member

mfridman commented Oct 22, 2021

Hi, we put a depreciation notice on StandardClaims and for backwards compatibility cannot break the existing API.

The good news is we also introduced a new type: RegisteredClaims (PR #15) which does have a custom ClaimStrings for audience.

jwt/claims.go

Lines 24 to 45 in c0ffb89

type RegisteredClaims struct {
// the `iss` (Issuer) claim. See https://datatracker.ietf.org/doc/html/rfc7519#section-4.1.1
Issuer string `json:"iss,omitempty"`
// the `sub` (Subject) claim. See https://datatracker.ietf.org/doc/html/rfc7519#section-4.1.2
Subject string `json:"sub,omitempty"`
// the `aud` (Audience) claim. See https://datatracker.ietf.org/doc/html/rfc7519#section-4.1.3
Audience ClaimStrings `json:"aud,omitempty"`
// the `exp` (Expiration Time) claim. See https://datatracker.ietf.org/doc/html/rfc7519#section-4.1.4
ExpiresAt *NumericDate `json:"exp,omitempty"`
// the `nbf` (Not Before) claim. See https://datatracker.ietf.org/doc/html/rfc7519#section-4.1.5
NotBefore *NumericDate `json:"nbf,omitempty"`
// the `iat` (Issued At) claim. See https://datatracker.ietf.org/doc/html/rfc7519#section-4.1.6
IssuedAt *NumericDate `json:"iat,omitempty"`
// the `jti` (JWT ID) claim. See https://datatracker.ietf.org/doc/html/rfc7519#section-4.1.7
ID string `json:"jti,omitempty"`
}

@mfridman
Copy link
Member

One other thing to point out, the RFC is vague, unfortuantely.

In the special case when the JWT has one audience, the "aud" value MAY be a single case-sensitive string containing a StringOrURI value.

So we've added an option MarshalSingleStringAsArray(true by default) to allow you to modify the marshal behaviour. But I don't think you'll need to use it, as a list of strings is most common, at least from what I've observed.

jwt/types.go

Lines 116 to 119 in 80625fb

// This handles a special case in the JWT RFC. If the string array, e.g. used by the "aud" field,
// only contains one element, it MAY be serialized as a single string. This may or may not be
// desired based on the ecosystem of other JWT library used, so we make it configurable by the
// variable MarshalSingleStringAsArray.

@pedromss
Copy link
Author

Thanks @mfridman, that makes sense. We'll give RegisteredClaims a go when we update to 4.1. For now I guess we'll stick with the untyped MapClaims.

Closing this as I have nothing to add.

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

No branches or pull requests

2 participants