From 97fa1183bf5de817a87dea61b629fac95ee22cb4 Mon Sep 17 00:00:00 2001 From: Mark Sagi-Kazar Date: Tue, 27 Sep 2022 15:31:01 +0200 Subject: [PATCH 1/3] feat: add WeakStringList type to support lists in aud claim Signed-off-by: Mark Sagi-Kazar --- registry/auth/token/types.go | 55 ++++++++++++++++++++ registry/auth/token/types_test.go | 85 +++++++++++++++++++++++++++++++ 2 files changed, 140 insertions(+) create mode 100644 registry/auth/token/types.go create mode 100644 registry/auth/token/types_test.go diff --git a/registry/auth/token/types.go b/registry/auth/token/types.go new file mode 100644 index 00000000..4559d6da --- /dev/null +++ b/registry/auth/token/types.go @@ -0,0 +1,55 @@ +package token + +import ( + "encoding/json" + "reflect" +) + +// WeakStringList is a slice of strings that can be deserialized from either a single string value or a list of strings. +type WeakStringList []string + +func (s *WeakStringList) UnmarshalJSON(data []byte) (err error) { + var value interface{} + + if err = json.Unmarshal(data, &value); err != nil { + return err + } + + switch v := value.(type) { + case string: + *s = []string{v} + + case []string: + *s = v + + case []interface{}: + var ss []string + + for _, vv := range v { + vs, ok := vv.(string) + if !ok { + return &json.UnsupportedTypeError{ + Type: reflect.TypeOf(vv), + } + } + + ss = append(ss, vs) + } + + *s = ss + + case nil: + return nil + + default: + return &json.UnsupportedTypeError{ + Type: reflect.TypeOf(v), + } + } + + return +} + +func (s WeakStringList) MarshalJSON() (b []byte, err error) { + return json.Marshal([]string(s)) +} diff --git a/registry/auth/token/types_test.go b/registry/auth/token/types_test.go new file mode 100644 index 00000000..283d86a8 --- /dev/null +++ b/registry/auth/token/types_test.go @@ -0,0 +1,85 @@ +package token + +import ( + "encoding/json" + "testing" +) + +func TestWeakStringList_Unmarshal(t *testing.T) { + t.Run("OK", func(t *testing.T) { + testCases := []struct { + value string + expected WeakStringList + }{ + { + value: `"audience"`, + expected: WeakStringList{"audience"}, + }, + { + value: `["audience1", "audience2"]`, + expected: WeakStringList{"audience1", "audience2"}, + }, + { + value: `null`, + expected: nil, + }, + } + + for _, testCase := range testCases { + testCase := testCase + + t.Run("", func(t *testing.T) { + var actual WeakStringList + + err := json.Unmarshal([]byte(testCase.value), &actual) + if err != nil { + t.Fatal(err) + } + + assertStringListEqual(t, testCase.expected, actual) + }) + } + }) + + t.Run("Error", func(t *testing.T) { + var actual WeakStringList + + err := json.Unmarshal([]byte("1234"), &actual) + if err == nil { + t.Fatal("expected unmarshal to fail") + } + }) +} + +func TestWeakStringList_Marshal(t *testing.T) { + value := WeakStringList{"audience"} + + expected := `["audience"]` + + actual, err := json.Marshal(value) + if err != nil { + t.Fatal(err) + } + + if expected != string(actual) { + t.Errorf("expected marshaled list to be %v, got %v", expected, actual) + } +} + +func assertStringListEqual(t *testing.T, expected []string, actual []string) { + t.Helper() + + if len(expected) != len(actual) { + t.Errorf("length mismatch: expected %d long slice, got %d", len(expected), len(actual)) + + return + } + + for i, v := range expected { + if v != actual[i] { + t.Errorf("expected %d. item to be %q, got %q", i, v, actual[i]) + } + + return + } +} From 3472f7a8e3b9a645240c5edc9067ef3e5d50a0e1 Mon Sep 17 00:00:00 2001 From: Mark Sagi-Kazar Date: Tue, 27 Sep 2022 15:34:26 +0200 Subject: [PATCH 2/3] feat: accept lists in the token audience claim Signed-off-by: Mark Sagi-Kazar --- contrib/token-server/token.go | 2 +- registry/auth/token/token.go | 34 ++++++++++++++++--------------- registry/auth/token/token_test.go | 10 ++++----- registry/auth/token/util.go | 11 ++++++++++ 4 files changed, 35 insertions(+), 22 deletions(-) diff --git a/contrib/token-server/token.go b/contrib/token-server/token.go index a66c767d..dc0956d4 100644 --- a/contrib/token-server/token.go +++ b/contrib/token-server/token.go @@ -180,7 +180,7 @@ func (issuer *TokenIssuer) CreateJWT(subject string, audience string, grantedAcc claimSet := token.ClaimSet{ Issuer: issuer.Issuer, Subject: subject, - Audience: audience, + Audience: []string{audience}, Expiration: now.Add(exp).Unix(), NotBefore: now.Unix(), IssuedAt: now.Unix(), diff --git a/registry/auth/token/token.go b/registry/auth/token/token.go index 2f86120a..4c182d95 100644 --- a/registry/auth/token/token.go +++ b/registry/auth/token/token.go @@ -42,13 +42,13 @@ type ResourceActions struct { // ClaimSet describes the main section of a JSON Web Token. type ClaimSet struct { // Public claims - Issuer string `json:"iss"` - Subject string `json:"sub"` - Audience string `json:"aud"` - Expiration int64 `json:"exp"` - NotBefore int64 `json:"nbf"` - IssuedAt int64 `json:"iat"` - JWTID string `json:"jti"` + Issuer string `json:"iss"` + Subject string `json:"sub"` + Audience WeakStringList `json:"aud"` + Expiration int64 `json:"exp"` + NotBefore int64 `json:"nbf"` + IssuedAt int64 `json:"iat"` + JWTID string `json:"jti"` // Private claims Access []*ResourceActions `json:"access"` @@ -141,8 +141,8 @@ func (t *Token) Verify(verifyOpts VerifyOptions) error { } // Verify that the Audience claim is allowed. - if !contains(verifyOpts.AcceptedAudiences, t.Claims.Audience) { - log.Infof("token intended for another audience: %q", t.Claims.Audience) + if !containsAny(verifyOpts.AcceptedAudiences, t.Claims.Audience) { + log.Infof("token intended for another audience: %v", t.Claims.Audience) return ErrInvalidToken } @@ -185,13 +185,15 @@ func (t *Token) Verify(verifyOpts VerifyOptions) error { // VerifySigningKey attempts to get the key which was used to sign this token. // The token header should contain either of these 3 fields: -// `x5c` - The x509 certificate chain for the signing key. Needs to be -// verified. -// `jwk` - The JSON Web Key representation of the signing key. -// May contain its own `x5c` field which needs to be verified. -// `kid` - The unique identifier for the key. This library interprets it -// as a libtrust fingerprint. The key itself can be looked up in -// the trustedKeys field of the given verify options. +// +// `x5c` - The x509 certificate chain for the signing key. Needs to be +// verified. +// `jwk` - The JSON Web Key representation of the signing key. +// May contain its own `x5c` field which needs to be verified. +// `kid` - The unique identifier for the key. This library interprets it +// as a libtrust fingerprint. The key itself can be looked up in +// the trustedKeys field of the given verify options. +// // Each of these methods are tried in that order of preference until the // signing key is found or an error is returned. func (t *Token) VerifySigningKey(verifyOpts VerifyOptions) (signingKey libtrust.PublicKey, err error) { diff --git a/registry/auth/token/token_test.go b/registry/auth/token/token_test.go index 77250d4b..0bdcd363 100644 --- a/registry/auth/token/token_test.go +++ b/registry/auth/token/token_test.go @@ -117,7 +117,7 @@ func makeTestToken(issuer, audience string, access []*ResourceActions, rootKey l claimSet := &ClaimSet{ Issuer: issuer, Subject: "foo", - Audience: audience, + Audience: []string{audience}, Expiration: exp.Unix(), NotBefore: now.Unix(), IssuedAt: now.Unix(), @@ -307,10 +307,10 @@ func writeTempRootCerts(rootKeys []libtrust.PrivateKey) (filename string, err er // TestAccessController tests complete integration of the token auth package. // It starts by mocking the options for a token auth accessController which // it creates. It then tries a few mock requests: -// - don't supply a token; should error with challenge -// - supply an invalid token; should error with challenge -// - supply a token with insufficient access; should error with challenge -// - supply a valid token; should not error +// - don't supply a token; should error with challenge +// - supply an invalid token; should error with challenge +// - supply a token with insufficient access; should error with challenge +// - supply a valid token; should not error func TestAccessController(t *testing.T) { // Make 2 keys; only the first is to be a trusted root key. rootKeys, err := makeRootKeys(2) diff --git a/registry/auth/token/util.go b/registry/auth/token/util.go index d7f95be4..a219df86 100644 --- a/registry/auth/token/util.go +++ b/registry/auth/token/util.go @@ -56,3 +56,14 @@ func contains(ss []string, q string) bool { return false } + +// containsAny returns true if any of q is found in ss. +func containsAny(ss []string, q []string) bool { + for _, s := range ss { + if contains(q, s) { + return true + } + } + + return false +} From d6ea77ae6517bd7443e9d1c88c8ac05d56603197 Mon Sep 17 00:00:00 2001 From: Mark Sagi-Kazar Date: Fri, 21 Oct 2022 11:11:50 +0200 Subject: [PATCH 3/3] refactor: rename WeakStringList to AudienceList Signed-off-by: Mark Sagi-Kazar --- registry/auth/token/token.go | 14 +++++++------- registry/auth/token/types.go | 8 ++++---- registry/auth/token/types_test.go | 16 ++++++++-------- 3 files changed, 19 insertions(+), 19 deletions(-) diff --git a/registry/auth/token/token.go b/registry/auth/token/token.go index 4c182d95..ec224754 100644 --- a/registry/auth/token/token.go +++ b/registry/auth/token/token.go @@ -42,13 +42,13 @@ type ResourceActions struct { // ClaimSet describes the main section of a JSON Web Token. type ClaimSet struct { // Public claims - Issuer string `json:"iss"` - Subject string `json:"sub"` - Audience WeakStringList `json:"aud"` - Expiration int64 `json:"exp"` - NotBefore int64 `json:"nbf"` - IssuedAt int64 `json:"iat"` - JWTID string `json:"jti"` + Issuer string `json:"iss"` + Subject string `json:"sub"` + Audience AudienceList `json:"aud"` + Expiration int64 `json:"exp"` + NotBefore int64 `json:"nbf"` + IssuedAt int64 `json:"iat"` + JWTID string `json:"jti"` // Private claims Access []*ResourceActions `json:"access"` diff --git a/registry/auth/token/types.go b/registry/auth/token/types.go index 4559d6da..2aa5c9ba 100644 --- a/registry/auth/token/types.go +++ b/registry/auth/token/types.go @@ -5,10 +5,10 @@ import ( "reflect" ) -// WeakStringList is a slice of strings that can be deserialized from either a single string value or a list of strings. -type WeakStringList []string +// AudienceList is a slice of strings that can be deserialized from either a single string value or a list of strings. +type AudienceList []string -func (s *WeakStringList) UnmarshalJSON(data []byte) (err error) { +func (s *AudienceList) UnmarshalJSON(data []byte) (err error) { var value interface{} if err = json.Unmarshal(data, &value); err != nil { @@ -50,6 +50,6 @@ func (s *WeakStringList) UnmarshalJSON(data []byte) (err error) { return } -func (s WeakStringList) MarshalJSON() (b []byte, err error) { +func (s AudienceList) MarshalJSON() (b []byte, err error) { return json.Marshal([]string(s)) } diff --git a/registry/auth/token/types_test.go b/registry/auth/token/types_test.go index 283d86a8..5e547761 100644 --- a/registry/auth/token/types_test.go +++ b/registry/auth/token/types_test.go @@ -5,19 +5,19 @@ import ( "testing" ) -func TestWeakStringList_Unmarshal(t *testing.T) { +func TestAudienceList_Unmarshal(t *testing.T) { t.Run("OK", func(t *testing.T) { testCases := []struct { value string - expected WeakStringList + expected AudienceList }{ { value: `"audience"`, - expected: WeakStringList{"audience"}, + expected: AudienceList{"audience"}, }, { value: `["audience1", "audience2"]`, - expected: WeakStringList{"audience1", "audience2"}, + expected: AudienceList{"audience1", "audience2"}, }, { value: `null`, @@ -29,7 +29,7 @@ func TestWeakStringList_Unmarshal(t *testing.T) { testCase := testCase t.Run("", func(t *testing.T) { - var actual WeakStringList + var actual AudienceList err := json.Unmarshal([]byte(testCase.value), &actual) if err != nil { @@ -42,7 +42,7 @@ func TestWeakStringList_Unmarshal(t *testing.T) { }) t.Run("Error", func(t *testing.T) { - var actual WeakStringList + var actual AudienceList err := json.Unmarshal([]byte("1234"), &actual) if err == nil { @@ -51,8 +51,8 @@ func TestWeakStringList_Unmarshal(t *testing.T) { }) } -func TestWeakStringList_Marshal(t *testing.T) { - value := WeakStringList{"audience"} +func TestAudienceList_Marshal(t *testing.T) { + value := AudienceList{"audience"} expected := `["audience"]`