From c4b32ced0e4893dde2d8b577ae89e4fad3e02c46 Mon Sep 17 00:00:00 2001 From: Jim Kalafut Date: Fri, 10 Aug 2018 09:00:52 -0700 Subject: [PATCH] Fix DB role statement update (#5058) The backwards compatibility logic was preventing updates to role statements from taking effect. This change removes persistence of deprecated statement fields. --- builtin/logical/database/backend_test.go | 83 ++++++++++++++++++++++-- builtin/logical/database/path_roles.go | 6 ++ 2 files changed, 82 insertions(+), 7 deletions(-) diff --git a/builtin/logical/database/backend_test.go b/builtin/logical/database/backend_test.go index 69dcb0f554..60038eeb1a 100644 --- a/builtin/logical/database/backend_test.go +++ b/builtin/logical/database/backend_test.go @@ -900,13 +900,11 @@ func TestBackend_roleCrud(t *testing.T) { } } - // Test role modification + // Test role modification of TTL { data = map[string]interface{}{ - "name": "plugin-role-test", - "rollback_statements": testRole, - "renew_statements": defaultRevocationSQL, - "max_ttl": "7m", + "name": "plugin-role-test", + "max_ttl": "7m", } req = &logical.Request{ Operation: logical.UpdateOperation, @@ -945,9 +943,7 @@ func TestBackend_roleCrud(t *testing.T) { expected := dbplugin.Statements{ Creation: []string{strings.TrimSpace(testRole)}, - Rollback: []string{strings.TrimSpace(testRole)}, Revocation: []string{strings.TrimSpace(defaultRevocationSQL)}, - Renewal: []string{strings.TrimSpace(defaultRevocationSQL)}, } actual := dbplugin.Statements{ @@ -973,6 +969,79 @@ func TestBackend_roleCrud(t *testing.T) { } + // Test role modification of statements + { + data = map[string]interface{}{ + "name": "plugin-role-test", + "creation_statements": []string{testRole, testRole}, + "revocation_statements": []string{defaultRevocationSQL, defaultRevocationSQL}, + "rollback_statements": testRole, + "renew_statements": defaultRevocationSQL, + } + req = &logical.Request{ + Operation: logical.UpdateOperation, + Path: "roles/plugin-role-test", + Storage: config.StorageView, + Data: data, + } + resp, err = b.HandleRequest(context.Background(), req) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("err:%v resp:%#v\n", err, resp) + } + + exists, err := b.pathRoleExistenceCheck()(context.Background(), req, &framework.FieldData{ + Raw: data, + Schema: pathRoles(b).Fields, + }) + if err != nil { + t.Fatal(err) + } + if !exists { + t.Fatal("expected exists") + } + + // Read the role + data = map[string]interface{}{} + req = &logical.Request{ + Operation: logical.ReadOperation, + Path: "roles/plugin-role-test", + Storage: config.StorageView, + Data: data, + } + resp, err = b.HandleRequest(context.Background(), req) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("err:%s resp:%#v\n", err, resp) + } + + expected := dbplugin.Statements{ + Creation: []string{strings.TrimSpace(testRole), strings.TrimSpace(testRole)}, + Rollback: []string{strings.TrimSpace(testRole)}, + Revocation: []string{strings.TrimSpace(defaultRevocationSQL), strings.TrimSpace(defaultRevocationSQL)}, + Renewal: []string{strings.TrimSpace(defaultRevocationSQL)}, + } + + actual := dbplugin.Statements{ + Creation: resp.Data["creation_statements"].([]string), + Revocation: resp.Data["revocation_statements"].([]string), + Rollback: resp.Data["rollback_statements"].([]string), + Renewal: resp.Data["renew_statements"].([]string), + } + + if !reflect.DeepEqual(expected, actual) { + t.Fatalf("Statements did not match, expected %#v, got %#v", expected, actual) + } + + if diff := deep.Equal(resp.Data["db_name"], "plugin-test"); diff != nil { + t.Fatal(diff) + } + if diff := deep.Equal(resp.Data["default_ttl"], float64(300)); diff != nil { + t.Fatal(diff) + } + if diff := deep.Equal(resp.Data["max_ttl"], float64(420)); diff != nil { + t.Fatal(diff) + } + } + // Delete the role data = map[string]interface{}{} req = &logical.Request{ diff --git a/builtin/logical/database/path_roles.go b/builtin/logical/database/path_roles.go index 22a00df520..ef1475cebf 100644 --- a/builtin/logical/database/path_roles.go +++ b/builtin/logical/database/path_roles.go @@ -210,6 +210,12 @@ func (b *databaseBackend) pathRoleCreateUpdate() framework.OperationFunc { } else if req.Operation == logical.CreateOperation { role.Statements.Renewal = data.Get("renew_statements").([]string) } + + // Do not persist deprecated statements that are populated on role read + role.Statements.CreationStatements = "" + role.Statements.RevocationStatements = "" + role.Statements.RenewStatements = "" + role.Statements.RollbackStatements = "" } // Store it