kataras / jwt

A fast and simple JWT implementation for Go
MIT License
204 stars 19 forks source link

Fix or remove Audience Claim from standard claims #3

Closed geordee closed 3 years ago

geordee commented 3 years ago

According to RFC 7519

In the general case, the "aud" value is an array of case-sensitive strings, each containing a StringOrURI value. In the special case when the JWT has one audience, the "aud" value MAY be a single case-sensitive string containing a StringOrURI value.

Currently in the code it is

Audience []string `json:"aud,omitempty"`

Since it could be an array or a string, and the implementation is application-specific please deserialize in a generic manner or even skip the claim.

kataras commented 3 years ago

Hello @geordee, you are right, there is no need to skip this claim though, you can use your own structure for standard claims too. We have two solutions for that:

1

On the claims.go:

    Audience []string `json:"-"`
    // RawAudience is used for encoding and decoding the Audience field, it can be a string or a []string.
    RawAudience json.RawMessage `json:"aud,omitempty"`

On the claims.go#ApplyClaims:

// ...

    if v := c.Audience; len(v) > 0 {
        dest.Audience = v
        dest.RawAudience, _ = json.Marshal(v) // lint: ignore
    }

On the verify.go#VerifyEncrypted:

        if aud := standardClaims.RawAudience; len(aud) > 0 {
            switch aud[0] {
            case '"': // it's a single string.
                var audString string
                err = json.Unmarshal(aud, &audString)
                if err == nil {
                    standardClaims.Audience = []string{audString}
                }
            case '[': // it's an array of strings.
                err = json.Unmarshal(aud, &standardClaims.Audience)
            }
        }

This fixes the decoding issue you described above. However, on Sign mehods the value will be produced will be always a json array of strings, we can fix it by checking if the Audience == 1 and put the RawAudience to a single string (on the ApplyClaims method) but I think it is cleaner if we just keep it as []string always and let the decisions on the decode side as we did on 1.

2

If you want to explictly produce a single or a []string we must change the Audience type to a custom type or interface{} but it doesn't look right to me... waiting for your opinion.

kataras commented 3 years ago

@geordee I decided to modify the Audience []string to a custom type of Audience Audience which it is still a []string so we have no breaking changes on Sign and Verify methods and how the standard "aud" claims is stored.

The issue is fixed by implemting the json unmarshal interface on that Audience new type (its code is identical to my previous comment).