acl: ensure auth method configs are correctly and fully hashed. (#19677)

This commit is contained in:
James Rasell
2024-01-09 14:03:26 +00:00
committed by GitHub
parent f3bc9c7c41
commit a3a03dff78
3 changed files with 246 additions and 13 deletions

3
.changelog/19677.txt Normal file
View File

@@ -0,0 +1,3 @@
```release-note:bug
acl: Fixed auth method hashing which meant changing some fields would be silently ignored
```

View File

@@ -779,21 +779,35 @@ func (a *ACLAuthMethod) SetHash() []byte {
_, _ = hash.Write([]byte(strconv.FormatBool(a.Default)))
if a.Config != nil {
_, _ = hash.Write([]byte(a.Config.JWKSURL))
_, _ = hash.Write([]byte(a.Config.JWKSCACert))
_, _ = hash.Write([]byte(a.Config.OIDCDiscoveryURL))
_, _ = hash.Write([]byte(a.Config.OIDCClientID))
_, _ = hash.Write([]byte(a.Config.OIDCClientSecret))
_, _ = hash.Write([]byte(a.Config.ExpirationLeeway.String()))
_, _ = hash.Write([]byte(a.Config.NotBeforeLeeway.String()))
_, _ = hash.Write([]byte(a.Config.ClockSkewLeeway.String()))
for _, ba := range a.Config.BoundAudiences {
_, _ = hash.Write([]byte(ba))
}
for _, bi := range a.Config.BoundIssuer {
_, _ = hash.Write([]byte(bi))
}
for _, uri := range a.Config.AllowedRedirectURIs {
_, _ = hash.Write([]byte(uri))
}
for _, pem := range a.Config.DiscoveryCaPem {
_, _ = hash.Write([]byte(pem))
}
for _, scope := range a.Config.OIDCScopes {
_, _ = hash.Write([]byte(scope))
}
for _, sa := range a.Config.SigningAlgs {
_, _ = hash.Write([]byte(sa))
}
for _, key := range a.Config.JWTValidationPubKeys {
_, _ = hash.Write([]byte(key))
}
for k, v := range a.Config.ClaimMappings {
_, _ = hash.Write([]byte(k))
_, _ = hash.Write([]byte(v))

View File

@@ -851,21 +851,237 @@ func Test_ACLAuthMethodGetRequest(t *testing.T) {
func TestACLAuthMethodSetHash(t *testing.T) {
ci.Parallel(t)
am := &ACLAuthMethod{
Name: "foo",
Type: "bad type",
testCases := []struct {
name string
fistACLAuthMethod *ACLAuthMethod
secondACLAuthMethod *ACLAuthMethod
expectedEqual bool
}{
{
name: "type_change",
fistACLAuthMethod: &ACLAuthMethod{
Name: "foo",
Type: "bad type",
},
secondACLAuthMethod: &ACLAuthMethod{
Name: "foo",
Type: "good type",
},
expectedEqual: false,
},
{
name: "oidc_scopes_change",
fistACLAuthMethod: &ACLAuthMethod{
Name: "foo",
Type: "type",
Config: &ACLAuthMethodConfig{
OIDCScopes: []string{"groups"},
},
},
secondACLAuthMethod: &ACLAuthMethod{
Name: "foo",
Type: "type",
Config: &ACLAuthMethodConfig{
OIDCScopes: []string{"groups", "roles"},
},
},
expectedEqual: false,
},
{
name: "jwksurl_change",
fistACLAuthMethod: &ACLAuthMethod{
Name: "foo",
Type: "type",
Config: &ACLAuthMethodConfig{
JWKSURL: "some-url",
},
},
secondACLAuthMethod: &ACLAuthMethod{
Name: "foo",
Type: "type",
Config: &ACLAuthMethodConfig{
JWKSURL: "some-other-url",
},
},
expectedEqual: false,
},
{
name: "jwkscacert_change",
fistACLAuthMethod: &ACLAuthMethod{
Name: "foo",
Type: "type",
Config: &ACLAuthMethodConfig{
JWKSCACert: "some-ca-cert",
},
},
secondACLAuthMethod: &ACLAuthMethod{
Name: "foo",
Type: "type",
Config: &ACLAuthMethodConfig{
JWKSCACert: "some-other-ca-cert",
},
},
expectedEqual: false,
},
{
name: "expiration_leeway_change",
fistACLAuthMethod: &ACLAuthMethod{
Name: "foo",
Type: "type",
Config: &ACLAuthMethodConfig{
ExpirationLeeway: 1 * time.Second,
},
},
secondACLAuthMethod: &ACLAuthMethod{
Name: "foo",
Type: "type",
Config: &ACLAuthMethodConfig{
ExpirationLeeway: 60 * time.Second,
},
},
expectedEqual: false,
},
{
name: "not_before_leeway_change",
fistACLAuthMethod: &ACLAuthMethod{
Name: "foo",
Type: "type",
Config: &ACLAuthMethodConfig{
NotBeforeLeeway: 1 * time.Second,
},
},
secondACLAuthMethod: &ACLAuthMethod{
Name: "foo",
Type: "type",
Config: &ACLAuthMethodConfig{
NotBeforeLeeway: 60 * time.Second,
},
},
expectedEqual: false,
},
{
name: "clock_skew_leeway_change",
fistACLAuthMethod: &ACLAuthMethod{
Name: "foo",
Type: "type",
Config: &ACLAuthMethodConfig{
ClockSkewLeeway: 1 * time.Second,
},
},
secondACLAuthMethod: &ACLAuthMethod{
Name: "foo",
Type: "type",
Config: &ACLAuthMethodConfig{
ClockSkewLeeway: 60 * time.Second,
},
},
expectedEqual: false,
},
{
name: "bound_issuer_change",
fistACLAuthMethod: &ACLAuthMethod{
Name: "foo",
Type: "type",
Config: &ACLAuthMethodConfig{
BoundIssuer: []string{"issuer"},
},
},
secondACLAuthMethod: &ACLAuthMethod{
Name: "foo",
Type: "type",
Config: &ACLAuthMethodConfig{
BoundIssuer: []string{"issuer", "other-issuer"},
},
},
expectedEqual: false,
},
{
name: "jtw_validation_pub_keys_change",
fistACLAuthMethod: &ACLAuthMethod{
Name: "foo",
Type: "type",
Config: &ACLAuthMethodConfig{
JWTValidationPubKeys: []string{"pubkey"},
},
},
secondACLAuthMethod: &ACLAuthMethod{
Name: "foo",
Type: "type",
Config: &ACLAuthMethodConfig{
JWTValidationPubKeys: []string{"pubkey", "other-pubkey"},
},
},
expectedEqual: false,
},
{
name: "full_unchanged",
fistACLAuthMethod: &ACLAuthMethod{
Name: "foo",
Type: "type",
Config: &ACLAuthMethodConfig{
JWTValidationPubKeys: []string{"some-jwt-validation-pubkey"},
JWKSURL: "some-jks-url",
OIDCDiscoveryURL: "some-oidc-discovery-url",
OIDCClientID: "some-oidc-client-id",
OIDCClientSecret: "some-oidc-client-secret",
BoundAudiences: []string{"audience1", "audience2"},
AllowedRedirectURIs: []string{"foo", "bar"},
DiscoveryCaPem: []string{"foo"},
SigningAlgs: []string{"bar"},
OIDCScopes: []string{"some-oidc-scope"},
BoundIssuer: []string{"some-bound-issuer"},
JWKSCACert: "some-jks-ca-cert",
ExpirationLeeway: 5 * time.Second,
NotBeforeLeeway: 5 * time.Second,
ClockSkewLeeway: 5 * time.Second,
ClaimMappings: map[string]string{"foo": "bar"},
ListClaimMappings: map[string]string{"foo": "bar"},
},
},
secondACLAuthMethod: &ACLAuthMethod{
Name: "foo",
Type: "type",
Config: &ACLAuthMethodConfig{
JWTValidationPubKeys: []string{"some-jwt-validation-pubkey"},
JWKSURL: "some-jks-url",
OIDCDiscoveryURL: "some-oidc-discovery-url",
OIDCClientID: "some-oidc-client-id",
OIDCClientSecret: "some-oidc-client-secret",
BoundAudiences: []string{"audience1", "audience2"},
AllowedRedirectURIs: []string{"foo", "bar"},
DiscoveryCaPem: []string{"foo"},
SigningAlgs: []string{"bar"},
OIDCScopes: []string{"some-oidc-scope"},
BoundIssuer: []string{"some-bound-issuer"},
JWKSCACert: "some-jks-ca-cert",
ExpirationLeeway: 5 * time.Second,
NotBeforeLeeway: 5 * time.Second,
ClockSkewLeeway: 5 * time.Second,
ClaimMappings: map[string]string{"foo": "bar"},
ListClaimMappings: map[string]string{"foo": "bar"},
},
},
expectedEqual: true,
},
}
out1 := am.SetHash()
must.NotNil(t, out1)
must.NotNil(t, am.Hash)
must.Eq(t, out1, am.Hash)
am.Type = "good type"
out2 := am.SetHash()
must.NotNil(t, out2)
must.NotNil(t, am.Hash)
must.Eq(t, out2, am.Hash)
must.NotEq(t, out1, out2)
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
must.NotNil(t, tc.fistACLAuthMethod.SetHash())
must.NotNil(t, tc.fistACLAuthMethod.Hash)
hash2 := tc.secondACLAuthMethod.SetHash()
must.NotNil(t, hash2)
must.NotNil(t, tc.secondACLAuthMethod.Hash)
must.Eq(t, hash2, tc.secondACLAuthMethod.Hash)
if tc.expectedEqual {
must.Eq(t, tc.fistACLAuthMethod.Hash, tc.secondACLAuthMethod.Hash)
} else {
must.NotEq(t, tc.fistACLAuthMethod.Hash, tc.secondACLAuthMethod.Hash)
}
})
}
}
func TestACLAuthMethod_Stub(t *testing.T) {