From 645e480bf224c0580073d91758a60cc65fe61c1f Mon Sep 17 00:00:00 2001 From: RemcoBuddelmeijer Date: Tue, 31 May 2022 23:08:39 +0200 Subject: [PATCH] Bind fields 'ttl' and 'num_uses' to role's configuration. Rather than implicitly overriding, error when the ttl is lower than and the num uses higher than the role's configuration. #14390 --- builtin/credential/approle/path_role.go | 9 +++ builtin/credential/approle/path_role_test.go | 79 ++++++++++++++++++-- 2 files changed, 83 insertions(+), 5 deletions(-) diff --git a/builtin/credential/approle/path_role.go b/builtin/credential/approle/path_role.go index 1dd83ec5e5d9..d6b130f439d8 100644 --- a/builtin/credential/approle/path_role.go +++ b/builtin/credential/approle/path_role.go @@ -2382,6 +2382,11 @@ func (b *backend) handleRoleSecretIDCommon(ctx context.Context, req *logical.Req if numUses < 0 { return logical.ErrorResponse("num_uses cannot be negative"), nil } + + // If the role's secret_id_num_uses is lower than the specified num_uses, throw an error rather than implicitly overriding + if numUses > role.SecretIDNumUses { + return logical.ErrorResponse("num_uses cannot be higher than the role's secret_id_num_uses"), nil + } } else { numUses = role.SecretIDNumUses } @@ -2390,6 +2395,10 @@ func (b *backend) handleRoleSecretIDCommon(ctx context.Context, req *logical.Req // Check whether or not ttl is defined, otherwise fallback to secret_id_ttl if ttlRaw, ok := data.GetOk("ttl"); ok { ttl = time.Second * time.Duration(ttlRaw.(int)) + // If the ttl is less than the role's secret_id_ttl, throw an error rather than implicitly overriding + if ttl < role.SecretIDTTL { + return logical.ErrorResponse("ttl cannot be shorter than the role's secret_id_ttl"), nil + } } else { ttl = role.SecretIDTTL } diff --git a/builtin/credential/approle/path_role_test.go b/builtin/credential/approle/path_role_test.go index d82a4bb1d052..3e5ddb05bb3b 100644 --- a/builtin/credential/approle/path_role_test.go +++ b/builtin/credential/approle/path_role_test.go @@ -1141,7 +1141,6 @@ func TestAppRole_RoleSecretID(t *testing.T) { "secret_id": "abcd123", } roleSecretIDReq.Data = roleCustomSecretIDData - roleSecretIDReq.Operation = logical.UpdateOperation resp, err = b.HandleRequest(context.Background(), roleSecretIDReq) if err != nil || (resp != nil && resp.IsError()) { t.Fatalf("err:%v resp:%#v", err, resp) @@ -1152,15 +1151,15 @@ func TestAppRole_RoleSecretID(t *testing.T) { } } -func TestAppRole_RoleSecretIDWithFields(t *testing.T) { +func TestAppRole_RoleSecretIDWithValidFields(t *testing.T) { var resp *logical.Response var err error b, storage := createBackendWithStorage(t) roleData := map[string]interface{}{ "policies": "p,q,r,s", - "secret_id_num_uses": 0, - "secret_id_ttl": 0, + "secret_id_num_uses": 5, + "secret_id_ttl": 5, "token_ttl": 400, "token_max_ttl": 500, } @@ -1206,7 +1205,6 @@ func TestAppRole_RoleSecretIDWithFields(t *testing.T) { roleCustomSecretIDData["secret_id"] = "abcd123" roleSecretIDReq.Data = roleCustomSecretIDData - roleSecretIDReq.Operation = logical.UpdateOperation resp, err = b.HandleRequest(context.Background(), roleSecretIDReq) if err != nil || (resp != nil && resp.IsError()) { t.Fatalf("err:%v resp:%#v", err, resp) @@ -1223,6 +1221,77 @@ func TestAppRole_RoleSecretIDWithFields(t *testing.T) { } } +func TestAppRole_ErrorsRoleSecretIDWithInvalidFields(t *testing.T) { + var resp *logical.Response + var err error + b, storage := createBackendWithStorage(t) + + roleData := map[string]interface{}{ + "policies": "p,q,r,s", + "secret_id_num_uses": 1, + "secret_id_ttl": 1, + "token_ttl": 400, + "token_max_ttl": 500, + } + roleReq := &logical.Request{ + Operation: logical.CreateOperation, + Path: "role/role1", + Storage: storage, + Data: roleData, + } + + resp, err = b.HandleRequest(context.Background(), roleReq) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("err:%v resp:%#v", err, resp) + } + + type testCase struct { + name string + payload map[string]interface{} + expected string + } + + testCases := []testCase{ + { + name: "invalid 'ttl' field", + payload: map[string]interface{}{"secret_id": "abcd123", "ttl": 0}, + expected: "ttl cannot be shorter than the role's secret_id_ttl", + }, + { + name: "invalid 'num_uses' field", + payload: map[string]interface{}{"secret_id": "abcd123", "num_uses": 2}, + expected: "num_uses cannot be higher than the role's secret_id_num_uses", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + roleSecretIDReq := &logical.Request{ + Operation: logical.UpdateOperation, + Path: "role/role1/secret-id", + Storage: storage, + } + roleSecretIDReq.Data = tc.payload + resp, err = b.HandleRequest(context.Background(), roleSecretIDReq) + if err != nil || (resp != nil && !resp.IsError()) { + t.Fatalf("err:%v resp:%#v", err, resp) + } + if resp.Data["error"].(string) != tc.expected { + t.Fatalf("expected: %q, got: %q", tc.expected, resp.Data["error"].(string)) + } + + roleSecretIDReq.Path = "role/role1/custom-secret-id" + resp, err = b.HandleRequest(context.Background(), roleSecretIDReq) + if err != nil || (resp != nil && !resp.IsError()) { + t.Fatalf("err:%v resp:%#v", err, resp) + } + if resp.Data["error"].(string) != tc.expected { + t.Fatalf("expected: %q, got: %q", tc.expected, resp.Data["error"].(string)) + } + }) + } +} + func TestAppRole_RoleCRUD(t *testing.T) { var resp *logical.Response var err error