From cc833a030ea17b1d617d217e3431b8f65d7b0118 Mon Sep 17 00:00:00 2001 From: Jeff Mitchell Date: Wed, 9 Mar 2016 11:59:54 -0500 Subject: [PATCH] Address final feedback --- vault/token_store.go | 80 +++++++++++++++++++++++++++++++-------- vault/token_store_test.go | 70 ++++++++++++++++++++++++++++------ 2 files changed, 123 insertions(+), 27 deletions(-) diff --git a/vault/token_store.go b/vault/token_store.go index ba76050226..67f44f7193 100644 --- a/vault/token_store.go +++ b/vault/token_store.go @@ -140,10 +140,13 @@ func NewTokenStore(c *Core, config *logical.BackendConfig) (*TokenStore, error) Callbacks: map[logical.Operation]framework.OperationFunc{ logical.ReadOperation: t.tokenStoreRoleRead, - logical.UpdateOperation: t.tokenStoreRoleCreate, + logical.CreateOperation: t.tokenStoreRoleCreateUpdate, + logical.UpdateOperation: t.tokenStoreRoleCreateUpdate, logical.DeleteOperation: t.tokenStoreRoleDelete, }, + ExistenceCheck: t.tokenStoreRoleExistenceCheck, + HelpSynopsis: tokenPathRolesHelp, HelpDescription: tokenPathRolesHelp, }, @@ -1164,31 +1167,78 @@ func (ts *TokenStore) tokenStoreRoleRead( return resp, nil } -func (ts *TokenStore) tokenStoreRoleCreate( +func (ts *TokenStore) tokenStoreRoleExistenceCheck(req *logical.Request, data *framework.FieldData) (bool, error) { + name := data.Get("role_name").(string) + if name == "" { + return false, fmt.Errorf("role name cannot be empty") + } + role, err := ts.tokenStoreRole(name) + if err != nil { + return false, err + } + + return role != nil, nil +} + +func (ts *TokenStore) tokenStoreRoleCreateUpdate( req *logical.Request, data *framework.FieldData) (*logical.Response, error) { name := data.Get("role_name").(string) if name == "" { return logical.ErrorResponse("role name cannot be empty"), nil } + entry, err := ts.tokenStoreRole(name) + if err != nil { + return nil, err + } - pathSuffix := data.Get("path_suffix").(string) - if pathSuffix != "" { - matched := pathSuffixSanitize.MatchString(pathSuffix) - if !matched { - return logical.ErrorResponse(fmt.Sprintf("given role path suffix contains invalid characters; must match %s", pathSuffixSanitize.String())), nil + // Due to the existence check, entry will only be nil if it's a create + // operation, so just create a new one + if entry == nil { + entry = &tsRoleEntry{ + Name: name, } } - entry := &tsRoleEntry{ - Name: name, - Orphan: data.Get("orphan").(bool), - Period: time.Second * time.Duration(data.Get("period").(int)), - PathSuffix: pathSuffix, + // In this series of blocks, if we do not find a user-provided value and + // it's a creation operation, we call data.Get to get the appropriate + // default + + orphanInt, ok := data.GetOk("orphan") + if ok { + entry.Orphan = orphanInt.(bool) + } else if req.Operation == logical.CreateOperation { + entry.Orphan = data.Get("orphan").(bool) } - allowedPolicies := data.Get("allowed_policies").(string) - if allowedPolicies != "" { - entry.AllowedPolicies = strings.Split(allowedPolicies, ",") + periodInt, ok := data.GetOk("period") + if ok { + entry.Period = time.Second * time.Duration(periodInt.(int)) + } else if req.Operation == logical.CreateOperation { + entry.Period = time.Second * time.Duration(data.Get("period").(int)) + } + + pathSuffixInt, ok := data.GetOk("path_suffix") + if ok { + pathSuffix := pathSuffixInt.(string) + if pathSuffix != "" { + matched := pathSuffixSanitize.MatchString(pathSuffix) + if !matched { + return logical.ErrorResponse(fmt.Sprintf("given role path suffix contains invalid characters; must match %s", pathSuffixSanitize.String())), nil + } + entry.PathSuffix = pathSuffix + } + } else if req.Operation == logical.CreateOperation { + entry.PathSuffix = data.Get("path_suffix").(string) + } + + allowedPoliciesInt, ok := data.GetOk("allowed_policies") + if ok { + allowedPolicies := allowedPoliciesInt.(string) + if allowedPolicies != "" { + entry.AllowedPolicies = strings.Split(allowedPolicies, ",") + } + } else if req.Operation == logical.CreateOperation { + entry.AllowedPolicies = strings.Split(data.Get("allowed_policies").(string), ",") } // Store it diff --git a/vault/token_store_test.go b/vault/token_store_test.go index b9f1f056e6..b006c44f49 100644 --- a/vault/token_store_test.go +++ b/vault/token_store_test.go @@ -1113,12 +1113,12 @@ func TestTokenStore_HandleRequest_RenewSelf(t *testing.T) { } func TestTokenStore_RoleCRUD(t *testing.T) { - _, ts, _, root := TestCoreWithTokenStore(t) + core, _, _, root := TestCoreWithTokenStore(t) - req := logical.TestRequest(t, logical.ReadOperation, "roles/test") + req := logical.TestRequest(t, logical.ReadOperation, "auth/token/roles/test") req.ClientToken = root - resp, err := ts.HandleRequest(req) + resp, err := core.HandleRequest(req) if err != nil { t.Fatalf("err: %v %v", err, resp) } @@ -1126,7 +1126,8 @@ func TestTokenStore_RoleCRUD(t *testing.T) { t.Fatalf("should not see a role") } - req.Operation = logical.UpdateOperation + // First test creation + req.Operation = logical.CreateOperation req.Data = map[string]interface{}{ "orphan": true, "period": "72h", @@ -1134,7 +1135,7 @@ func TestTokenStore_RoleCRUD(t *testing.T) { "path_suffix": "happenin", } - resp, err = ts.HandleRequest(req) + resp, err = core.HandleRequest(req) if err != nil { t.Fatalf("err: %v %v", err, resp) } @@ -1145,7 +1146,7 @@ func TestTokenStore_RoleCRUD(t *testing.T) { req.Operation = logical.ReadOperation req.Data = map[string]interface{}{} - resp, err = ts.HandleRequest(req) + resp, err = core.HandleRequest(req) if err != nil { t.Fatalf("err: %v %v", err, resp) } @@ -1171,10 +1172,55 @@ func TestTokenStore_RoleCRUD(t *testing.T) { t.Fatalf("expected:\n%v\nactual:\n%v\n", expected, actual) } - req.Operation = logical.ListOperation - req.Path = "roles" + // Now test updating; this should be set to an UpdateOperation + // automatically due to the existence check + req.Operation = logical.CreateOperation + req.Data = map[string]interface{}{ + "period": "79h", + "allowed_policies": "test3", + "path_suffix": "happenin", + } + + resp, err = core.HandleRequest(req) + if err != nil { + t.Fatalf("err: %v %v", err, resp) + } + if resp != nil { + t.Fatalf("expected a nil response") + } + + req.Operation = logical.ReadOperation req.Data = map[string]interface{}{} - resp, err = ts.HandleRequest(req) + + resp, err = core.HandleRequest(req) + if err != nil { + t.Fatalf("err: %v %v", err, resp) + } + if resp == nil { + t.Fatalf("got a nil response") + } + + err = mapstructure.WeakDecode(resp.Data, &actual) + if err != nil { + t.Fatalf("error decoding role json: %v", err) + } + + expected = tsRoleEntry{ + Name: "test", + Orphan: true, + Period: 79 * time.Hour, + AllowedPolicies: []string{"test3"}, + PathSuffix: "happenin", + } + + if !reflect.DeepEqual(expected, actual) { + t.Fatalf("expected:\n%v\nactual:\n%v\n", expected, actual) + } + + req.Operation = logical.ListOperation + req.Path = "auth/token/roles" + req.Data = map[string]interface{}{} + resp, err = core.HandleRequest(req) if err != nil { t.Fatalf("err: %v %v", err, resp) } @@ -1197,8 +1243,8 @@ func TestTokenStore_RoleCRUD(t *testing.T) { } req.Operation = logical.DeleteOperation - req.Path = "roles/test" - resp, err = ts.HandleRequest(req) + req.Path = "auth/token/roles/test" + resp, err = core.HandleRequest(req) if err != nil { t.Fatalf("err: %v %v", err, resp) } @@ -1207,7 +1253,7 @@ func TestTokenStore_RoleCRUD(t *testing.T) { } req.Operation = logical.ReadOperation - resp, err = ts.HandleRequest(req) + resp, err = core.HandleRequest(req) if err != nil { t.Fatalf("err: %v %v", err, resp) }