Fix case handling in policyutil.EquivalentPolicies() (#16484)

The previous logic would consider not normalize casing before comparing
the policy names which meant that a token associated to a policy with
an uppercase could not be renewed for the following auth methods:

  - AppID
  - Cert
  - GitHub
  - LDAP
  - Okta
  - Radius
  - Userpass

Co-authored-by: Violet Hynes <violet.hynes@hashicorp.com>
This commit is contained in:
Rémi Lapeyre
2024-05-31 15:58:03 +02:00
committed by GitHub
parent 5672937149
commit f8eb0154d4
4 changed files with 64 additions and 44 deletions

View File

@@ -843,7 +843,7 @@ func TestBackend_NonCAExpiry(t *testing.T) {
time.Sleep(5 * time.Second) time.Sleep(5 * time.Second)
// Login attempt after certificate expiry should fail // Login attempt after certificate expiry should fail
resp, err = b.HandleRequest(context.Background(), loginReq) _, err = b.HandleRequest(context.Background(), loginReq)
if err == nil { if err == nil {
t.Fatalf("expected error due to expired certificate") t.Fatalf("expected error due to expired certificate")
} }
@@ -2178,12 +2178,13 @@ func Test_Renew(t *testing.T) {
Raw: map[string]interface{}{ Raw: map[string]interface{}{
"name": "test", "name": "test",
"certificate": ca, "certificate": ca,
"policies": "foo,bar", // Uppercase B should not cause an issue during renewal
"token_policies": "foo,Bar",
}, },
Schema: pathCerts(b).Fields, Schema: pathCerts(b).Fields,
} }
resp, err := b.pathCertWrite(context.Background(), req, fd) _, err = b.pathCertWrite(context.Background(), req, fd)
if err != nil { if err != nil {
t.Fatal(err) t.Fatal(err)
} }
@@ -2192,7 +2193,7 @@ func Test_Renew(t *testing.T) {
Raw: map[string]interface{}{}, Raw: map[string]interface{}{},
Schema: pathLogin(b).Fields, Schema: pathLogin(b).Fields,
} }
resp, err = b.pathLogin(context.Background(), req, empty_login_fd) resp, err := b.pathLogin(context.Background(), req, empty_login_fd)
if err != nil { if err != nil {
t.Fatal(err) t.Fatal(err)
} }
@@ -2219,20 +2220,20 @@ func Test_Renew(t *testing.T) {
} }
// Change the policies -- this should fail // Change the policies -- this should fail
fd.Raw["policies"] = "zip,zap" fd.Raw["token_policies"] = "zip,zap"
resp, err = b.pathCertWrite(context.Background(), req, fd) _, err = b.pathCertWrite(context.Background(), req, fd)
if err != nil { if err != nil {
t.Fatal(err) t.Fatal(err)
} }
resp, err = b.pathLoginRenew(context.Background(), req, empty_login_fd) _, err = b.pathLoginRenew(context.Background(), req, empty_login_fd)
if err == nil { if err == nil {
t.Fatal("expected error") t.Fatal("expected error")
} }
// Put the policies back, this should be okay // Put the policies back, this should be okay
fd.Raw["policies"] = "bar,foo" fd.Raw["token_policies"] = "bar,foo"
resp, err = b.pathCertWrite(context.Background(), req, fd) _, err = b.pathCertWrite(context.Background(), req, fd)
if err != nil { if err != nil {
t.Fatal(err) t.Fatal(err)
} }
@@ -2251,7 +2252,7 @@ func Test_Renew(t *testing.T) {
// Add period value to cert entry // Add period value to cert entry
period := 350 * time.Second period := 350 * time.Second
fd.Raw["period"] = period.String() fd.Raw["period"] = period.String()
resp, err = b.pathCertWrite(context.Background(), req, fd) _, err = b.pathCertWrite(context.Background(), req, fd)
if err != nil { if err != nil {
t.Fatal(err) t.Fatal(err)
} }
@@ -2272,7 +2273,7 @@ func Test_Renew(t *testing.T) {
} }
// Delete CA, make sure we can't renew // Delete CA, make sure we can't renew
resp, err = b.pathCertDelete(context.Background(), req, fd) _, err = b.pathCertDelete(context.Background(), req, fd)
if err != nil { if err != nil {
t.Fatal(err) t.Fatal(err)
} }

3
changelog/16484.txt Normal file
View File

@@ -0,0 +1,3 @@
```release-note:bug
auth/appid, auth/cert, auth/github, auth/ldap, auth/okta, auth/radius, auth/userpass: fixed an issue with policy name normalization that would prevent a token associated with a policy containing an uppercase character to be renewed.
```

View File

@@ -79,33 +79,25 @@ func SanitizePolicies(policies []string, addDefault bool) []string {
// EquivalentPolicies checks whether the given policy sets are equivalent, as in, // EquivalentPolicies checks whether the given policy sets are equivalent, as in,
// they contain the same values. The benefit of this method is that it leaves // they contain the same values. The benefit of this method is that it leaves
// the "default" policy out of its comparisons as it may be added later by core // the "default" policy out of its comparisons as it may be added later by core
// after a set of policies has been saved by a backend. // after a set of policies has been saved by a backend and performs policy name
// normalization.
func EquivalentPolicies(a, b []string) bool { func EquivalentPolicies(a, b []string) bool {
switch {
case a == nil && b == nil:
return true
case a == nil && len(b) == 1 && b[0] == "default":
return true
case b == nil && len(a) == 1 && a[0] == "default":
return true
case a == nil || b == nil:
return false
}
// First we'll build maps to ensure unique values and filter default // First we'll build maps to ensure unique values and filter default
mapA := map[string]bool{} mapA := map[string]struct{}{}
mapB := map[string]bool{} mapB := map[string]struct{}{}
for _, keyA := range a { for _, keyA := range a {
keyA := strings.ToLower(keyA)
if keyA == "default" { if keyA == "default" {
continue continue
} }
mapA[keyA] = true mapA[keyA] = struct{}{}
} }
for _, keyB := range b { for _, keyB := range b {
keyB := strings.ToLower(keyB)
if keyB == "default" { if keyB == "default" {
continue continue
} }
mapB[keyB] = true mapB[keyB] = struct{}{}
} }
// Now we'll build our checking slices // Now we'll build our checking slices

View File

@@ -56,24 +56,48 @@ func TestParsePolicies(t *testing.T) {
} }
func TestEquivalentPolicies(t *testing.T) { func TestEquivalentPolicies(t *testing.T) {
a := []string{"foo", "bar"} testCases := map[string]struct {
var b []string A []string
if EquivalentPolicies(a, b) { B []string
t.Fatal("bad") Expected bool
}{
"nil": {
A: nil,
B: nil,
Expected: true,
},
"empty": {
A: []string{"foo", "bar"},
B: []string{},
Expected: false,
},
"missing": {
A: []string{"foo", "bar"},
B: []string{"foo"},
Expected: false,
},
"equal": {
A: []string{"bar", "foo"},
B: []string{"bar", "foo"},
Expected: true,
},
"default": {
A: []string{"bar", "foo"},
B: []string{"foo", "default", "bar"},
Expected: true,
},
"case-insensitive": {
A: []string{"test"},
B: []string{"Test"},
Expected: true,
},
} }
b = []string{"foo"} for name, tc := range testCases {
if EquivalentPolicies(a, b) { t.Run(name, func(t *testing.T) {
t.Fatal("bad") if EquivalentPolicies(tc.A, tc.B) != tc.Expected {
} t.Fatal("bad")
}
b = []string{"bar", "foo"} })
if !EquivalentPolicies(a, b) {
t.Fatal("bad")
}
b = []string{"foo", "default", "bar"}
if !EquivalentPolicies(a, b) {
t.Fatal("bad")
} }
} }