Two-pronged fix for renew policy checking (#4960)

1) In backends, ensure they are now using TokenPolicies
2) Don't reassign auth.Policies until after expmgr registration as we
don't need them at that point

Fixes #4829

builtin/credential/app-id/path_login.go

@@ -143,7 +143,7 @@ func (b *backend) pathLoginRenew(ctx context.Context, req *logical.Request, d *f
 	if err != nil {
 		return nil, err
 	}
-	if !policyutil.EquivalentPolicies(mapPolicies, req.Auth.Policies) {
+	if !policyutil.EquivalentPolicies(mapPolicies, req.Auth.TokenPolicies) {
 		return nil, fmt.Errorf("policies do not match")
 	}
 

builtin/credential/cert/backend_test.go

@@ -1672,6 +1672,7 @@ func Test_Renew(t *testing.T) {
 	req.Auth.Metadata = resp.Auth.Metadata
 	req.Auth.LeaseOptions = resp.Auth.LeaseOptions
 	req.Auth.Policies = resp.Auth.Policies
+	req.Auth.TokenPolicies = req.Auth.Policies
 	req.Auth.Period = resp.Auth.Period
 
 	// Normal renewal

builtin/credential/cert/path_login.go

@@ -159,7 +159,7 @@ func (b *backend) pathLoginRenew(ctx context.Context, req *logical.Request, d *f
 		return nil, nil
 	}
 
-	if !policyutil.EquivalentPolicies(cert.Policies, req.Auth.Policies) {
+	if !policyutil.EquivalentPolicies(cert.Policies, req.Auth.TokenPolicies) {
 		return nil, fmt.Errorf("policies have changed, not renewing")
 	}
 

builtin/credential/github/path_login.go

@@ -121,7 +121,7 @@ func (b *backend) pathLoginRenew(ctx context.Context, req *logical.Request, d *f
 	} else {
 		verifyResp = verifyResponse
 	}
-	if !policyutil.EquivalentPolicies(verifyResp.Policies, req.Auth.Policies) {
+	if !policyutil.EquivalentPolicies(verifyResp.Policies, req.Auth.TokenPolicies) {
 		return nil, fmt.Errorf("policies do not match")
 	}
 

builtin/credential/ldap/path_login.go

@@ -107,7 +107,7 @@ func (b *backend) pathLoginRenew(ctx context.Context, req *logical.Request, d *f
 		return resp, err
 	}
 
-	if !policyutil.EquivalentPolicies(loginPolicies, req.Auth.Policies) {
+	if !policyutil.EquivalentPolicies(loginPolicies, req.Auth.TokenPolicies) {
 		return nil, fmt.Errorf("policies have changed, not renewing")
 	}
 

builtin/credential/okta/path_login.go

@@ -118,7 +118,7 @@ func (b *backend) pathLoginRenew(ctx context.Context, req *logical.Request, d *f
 		return resp, err
 	}
 
-	if !policyutil.EquivalentPolicies(loginPolicies, req.Auth.Policies) {
+	if !policyutil.EquivalentPolicies(loginPolicies, req.Auth.TokenPolicies) {
 		return nil, fmt.Errorf("policies have changed, not renewing")
 	}
 

builtin/credential/radius/path_login.go

@@ -122,7 +122,7 @@ func (b *backend) pathLoginRenew(ctx context.Context, req *logical.Request, d *f
 		return resp, err
 	}
 
-	if !policyutil.EquivalentPolicies(loginPolicies, req.Auth.Policies) {
+	if !policyutil.EquivalentPolicies(loginPolicies, req.Auth.TokenPolicies) {
 		return nil, fmt.Errorf("policies have changed, not renewing")
 	}
 

builtin/credential/userpass/path_login.go

@@ -119,7 +119,7 @@ func (b *backend) pathLoginRenew(ctx context.Context, req *logical.Request, d *f
 		return nil, nil
 	}
 
-	if !policyutil.EquivalentPolicies(user.Policies, req.Auth.Policies) {
+	if !policyutil.EquivalentPolicies(user.Policies, req.Auth.TokenPolicies) {
 		return nil, fmt.Errorf("policies have changed, not renewing")
 	}
 

plugins/database/cassandra/test-fixtures/cassandra.yaml

@@ -572,7 +572,7 @@ ssl_storage_port: 7001
 #
 # Setting listen_address to 0.0.0.0 is always wrong.
 #
-listen_address: 172.17.0.4
+listen_address: 172.17.0.2
 
 # Set listen_address OR listen_interface, not both. Interfaces must correspond
 # to a single address, IP aliasing is not supported.

vault/request_handling.go

@@ -639,15 +639,19 @@ func (c *Core) handleRequest(ctx context.Context, req *logical.Request) (retResp
 		}
 
 		resp.Auth.TokenPolicies = policyutil.SanitizePolicies(resp.Auth.Policies, policyutil.DoNotAddDefaultPolicy)
-		resp.Auth.IdentityPolicies = policyutil.SanitizePolicies(identityPolicies, policyutil.DoNotAddDefaultPolicy)
-		resp.Auth.Policies = policyutil.SanitizePolicies(append(resp.Auth.Policies, identityPolicies...), policyutil.DoNotAddDefaultPolicy)
-
 		if err := c.expiration.RegisterAuth(resp.Auth.CreationPath, resp.Auth); err != nil {
 			c.tokenStore.revokeOrphan(ctx, te.ID)
 			c.logger.Error("failed to register token lease", "request_path", req.Path, "error", err)
 			retErr = multierror.Append(retErr, ErrInternalError)
 			return nil, auth, retErr
 		}
+
+		// We do these later since it's not meaningful for backends/expmgr to
+		// have what is purely a snapshot of current identity policies, and
+		// plugins can be confused if they are checking contents of
+		// Auth.Policies instead of Auth.TokenPolicies
+		resp.Auth.IdentityPolicies = policyutil.SanitizePolicies(identityPolicies, policyutil.DoNotAddDefaultPolicy)
+		resp.Auth.Policies = policyutil.SanitizePolicies(append(resp.Auth.Policies, identityPolicies...), policyutil.DoNotAddDefaultPolicy)
 	}
 
 	if resp != nil &&
@@ -836,13 +840,12 @@ func (c *Core) handleLoginRequest(ctx context.Context, req *logical.Request) (re
 		}
 
 		auth.TokenPolicies = te.Policies
-		auth.IdentityPolicies = policyutil.SanitizePolicies(identityPolicies, policyutil.DoNotAddDefaultPolicy)
-		auth.Policies = policyutil.SanitizePolicies(append(te.Policies, identityPolicies...), policyutil.DoNotAddDefaultPolicy)
+		allPolicies := policyutil.SanitizePolicies(append(te.Policies, identityPolicies...), policyutil.DoNotAddDefaultPolicy)
 
 		// Prevent internal policies from being assigned to tokens. We check
 		// this on auth.Policies including derived ones from Identity before
 		// actually making the token.
-		for _, policy := range auth.Policies {
+		for _, policy := range allPolicies {
 			if policy == "root" {
 				return logical.ErrorResponse("auth methods cannot create root tokens"), nil, logical.ErrInvalidRequest
 			}
@@ -868,6 +871,9 @@ func (c *Core) handleLoginRequest(ctx context.Context, req *logical.Request) (re
 			return nil, auth, ErrInternalError
 		}
 
+		auth.IdentityPolicies = policyutil.SanitizePolicies(identityPolicies, policyutil.DoNotAddDefaultPolicy)
+		auth.Policies = allPolicies
+
 		// Attach the display name, might be used by audit backends
 		req.DisplayName = auth.DisplayName
 	}

vault/token_store_ext_test.go

@@ -141,6 +141,14 @@ func TestTokenStore_IdentityPolicies(t *testing.T) {
 	}
 	ldapClientToken := secret.Auth.ClientToken
 
+	expectedPolicies := []string{
+		"default",
+		"testgroup1-policy",
+	}
+	if !reflect.DeepEqual(expectedPolicies, secret.Auth.Policies) {
+		t.Fatalf("bad: identity policies; expected: %#v\nactual: %#v", expectedPolicies, secret.Auth.Policies)
+	}
+
 	// At this point there shouldn't be any identity policy on the token
 	secret, err = client.Logical().Read("auth/token/lookup/" + ldapClientToken)
 	if err != nil {
@@ -175,7 +183,7 @@ func TestTokenStore_IdentityPolicies(t *testing.T) {
 	}
 	sort.Strings(actualPolicies)
 
-	expectedPolicies := []string{
+	expectedPolicies = []string{
 		"entity_policy_1",
 		"entity_policy_2",
 	}
@@ -283,6 +291,18 @@ func TestTokenStore_IdentityPolicies(t *testing.T) {
 	if !reflect.DeepEqual(expectedPolicies, actualPolicies) {
 		t.Fatalf("bad: identity policies; expected: %#v\nactual: %#v", expectedPolicies, actualPolicies)
 	}
+
+	// Log in and get a new token, then renew it. See issue #4829
+	secret, err = client.Logical().Write("auth/ldap/#/tesla", map[string]interface{}{
+		"password": "password",
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	secret, err = client.Auth().Token().Renew(secret.Auth.ClientToken, 10)
+	if err != nil {
+		t.Fatal(err)
+	}
 }
 
 func TestTokenStore_CIDRBlocks(t *testing.T) {