From 4daa1585b0d6839ae8fd926b08558e240ee58dbb Mon Sep 17 00:00:00 2001 From: Matt Keeler Date: Tue, 30 Apr 2019 11:45:36 -0400 Subject: [PATCH] ACL Token ID Initialization (#5307) --- agent/acl_endpoint.go | 38 ++- agent/acl_endpoint_test.go | 253 +++++++++++++- agent/consul/acl_endpoint.go | 132 ++++--- agent/consul/acl_endpoint_test.go | 323 +++++++++++++++++- agent/consul/acl_replication_test.go | 1 + agent/structs/acl.go | 5 +- api/acl.go | 13 +- .../acl/policy/create/policy_create_test.go | 10 +- command/acl/token/create/token_create.go | 10 +- command/acl/token/create/token_create_test.go | 45 ++- command/acl/token/update/token_update.go | 2 +- website/source/api/acl/tokens.html.md | 8 + .../docs/commands/acl/acl-token.html.md.erb | 8 + 13 files changed, 766 insertions(+), 82 deletions(-) diff --git a/agent/acl_endpoint.go b/agent/acl_endpoint.go index 12c6b313a2..db456ff715 100644 --- a/agent/acl_endpoint.go +++ b/agent/acl_endpoint.go @@ -266,7 +266,7 @@ func (s *HTTPServer) ACLPolicyCreate(resp http.ResponseWriter, req *http.Request return nil, nil } - return s.ACLPolicyWrite(resp, req, "") + return s.aclPolicyWriteInternal(resp, req, "", true) } // fixTimeAndHashFields is used to help in decoding the ExpirationTTL, ExpirationTime, CreateTime, and Hash @@ -317,6 +317,10 @@ func fixTimeAndHashFields(raw interface{}) error { } func (s *HTTPServer) ACLPolicyWrite(resp http.ResponseWriter, req *http.Request, policyID string) (interface{}, error) { + return s.aclPolicyWriteInternal(resp, req, policyID, false) +} + +func (s *HTTPServer) aclPolicyWriteInternal(resp http.ResponseWriter, req *http.Request, policyID string, create bool) (interface{}, error) { args := structs.ACLPolicySetRequest{ Datacenter: s.agent.config.Datacenter, } @@ -328,10 +332,16 @@ func (s *HTTPServer) ACLPolicyWrite(resp http.ResponseWriter, req *http.Request, args.Policy.Syntax = acl.SyntaxCurrent - if args.Policy.ID != "" && args.Policy.ID != policyID { - return nil, BadRequestError{Reason: "Policy ID in URL and payload do not match"} - } else if args.Policy.ID == "" { - args.Policy.ID = policyID + if create { + if args.Policy.ID != "" { + return nil, BadRequestError{Reason: "Cannot specify the ID when creating a new policy"} + } + } else { + if args.Policy.ID != "" && args.Policy.ID != policyID { + return nil, BadRequestError{Reason: "Policy ID in URL and payload do not match"} + } else if args.Policy.ID == "" { + args.Policy.ID = policyID + } } var out structs.ACLPolicy @@ -458,7 +468,7 @@ func (s *HTTPServer) ACLTokenCreate(resp http.ResponseWriter, req *http.Request) return nil, nil } - return s.ACLTokenSet(resp, req, "") + return s.aclTokenSetInternal(resp, req, "", true) } func (s *HTTPServer) ACLTokenGet(resp http.ResponseWriter, req *http.Request, tokenID string) (interface{}, error) { @@ -490,8 +500,13 @@ func (s *HTTPServer) ACLTokenGet(resp http.ResponseWriter, req *http.Request, to } func (s *HTTPServer) ACLTokenSet(resp http.ResponseWriter, req *http.Request, tokenID string) (interface{}, error) { + return s.aclTokenSetInternal(resp, req, tokenID, false) +} + +func (s *HTTPServer) aclTokenSetInternal(resp http.ResponseWriter, req *http.Request, tokenID string, create bool) (interface{}, error) { args := structs.ACLTokenSetRequest{ Datacenter: s.agent.config.Datacenter, + Create: create, } s.parseToken(req, &args.Token) @@ -499,10 +514,12 @@ func (s *HTTPServer) ACLTokenSet(resp http.ResponseWriter, req *http.Request, to return nil, BadRequestError{Reason: fmt.Sprintf("Token decoding failed: %v", err)} } - if args.ACLToken.AccessorID != "" && args.ACLToken.AccessorID != tokenID { - return nil, BadRequestError{Reason: "Token Accessor ID in URL and payload do not match"} - } else if args.ACLToken.AccessorID == "" { - args.ACLToken.AccessorID = tokenID + if !create { + if args.ACLToken.AccessorID != "" && args.ACLToken.AccessorID != tokenID { + return nil, BadRequestError{Reason: "Token Accessor ID in URL and payload do not match"} + } else if args.ACLToken.AccessorID == "" { + args.ACLToken.AccessorID = tokenID + } } var out structs.ACLToken @@ -534,6 +551,7 @@ func (s *HTTPServer) ACLTokenClone(resp http.ResponseWriter, req *http.Request, args := structs.ACLTokenSetRequest{ Datacenter: s.agent.config.Datacenter, + Create: true, } if err := decodeBody(req, &args.ACLToken, fixTimeAndHashFields); err != nil && err.Error() != "EOF" { diff --git a/agent/acl_endpoint_test.go b/agent/acl_endpoint_test.go index ab92c1457a..d38d52ed97 100644 --- a/agent/acl_endpoint_test.go +++ b/agent/acl_endpoint_test.go @@ -143,7 +143,12 @@ func TestACL_HTTP(t *testing.T) { // we can intelligently order these tests so we can still // test everything with less actual operations and do // so in a manner that is less prone to being flaky - // 3. While this test will be large it should + // + // This could be accomplished with just blocks of code but I find + // the go test output nicer to pinpoint the error if they are grouped. + // + // NOTE: None of the subtests should be parallelized in order for + // any of it to work properly. t.Run("Policy", func(t *testing.T) { t.Run("Create", func(t *testing.T) { policyInput := &structs.ACLPolicy{ @@ -822,6 +827,252 @@ func TestACL_HTTP(t *testing.T) { require.Len(t, token.Policies, 1) require.Equal(t, structs.ACLPolicyGlobalManagementID, token.Policies[0].ID) }) + t.Run("Create with Accessor", func(t *testing.T) { + tokenInput := &structs.ACLToken{ + AccessorID: "56e8e6a3-708b-4a2f-8ab3-b973cce39108", + Description: "test", + Policies: []structs.ACLTokenPolicyLink{ + structs.ACLTokenPolicyLink{ + ID: idMap["policy-test"], + Name: policyMap[idMap["policy-test"]].Name, + }, + structs.ACLTokenPolicyLink{ + ID: idMap["policy-read-all-nodes"], + Name: policyMap[idMap["policy-read-all-nodes"]].Name, + }, + }, + } + + req, _ := http.NewRequest("PUT", "/v1/acl/token?token=root", jsonBody(tokenInput)) + resp := httptest.NewRecorder() + obj, err := a.srv.ACLTokenCreate(resp, req) + require.NoError(t, err) + + token, ok := obj.(*structs.ACLToken) + require.True(t, ok) + + // 36 = length of the string form of uuids + require.Equal(t, tokenInput.AccessorID, token.AccessorID) + require.Len(t, token.SecretID, 36) + require.Equal(t, tokenInput.Description, token.Description) + require.Equal(t, tokenInput.Policies, token.Policies) + require.True(t, token.CreateIndex > 0) + require.Equal(t, token.CreateIndex, token.ModifyIndex) + require.NotNil(t, token.Hash) + require.NotEqual(t, token.Hash, []byte{}) + + idMap["token-test"] = token.AccessorID + tokenMap[token.AccessorID] = token + }) + + t.Run("Create with Secret", func(t *testing.T) { + tokenInput := &structs.ACLToken{ + SecretID: "4e3efd15-d06c-442e-a7cc-1744f55c8dea", + Description: "test", + Policies: []structs.ACLTokenPolicyLink{ + structs.ACLTokenPolicyLink{ + ID: idMap["policy-test"], + Name: policyMap[idMap["policy-test"]].Name, + }, + structs.ACLTokenPolicyLink{ + ID: idMap["policy-read-all-nodes"], + Name: policyMap[idMap["policy-read-all-nodes"]].Name, + }, + }, + } + + req, _ := http.NewRequest("PUT", "/v1/acl/token?token=root", jsonBody(tokenInput)) + resp := httptest.NewRecorder() + obj, err := a.srv.ACLTokenCreate(resp, req) + require.NoError(t, err) + + token, ok := obj.(*structs.ACLToken) + require.True(t, ok) + + // 36 = length of the string form of uuids + require.Equal(t, tokenInput.SecretID, token.SecretID) + require.Len(t, token.AccessorID, 36) + require.Equal(t, tokenInput.Description, token.Description) + require.Equal(t, tokenInput.Policies, token.Policies) + require.True(t, token.CreateIndex > 0) + require.Equal(t, token.CreateIndex, token.ModifyIndex) + require.NotNil(t, token.Hash) + require.NotEqual(t, token.Hash, []byte{}) + + idMap["token-test"] = token.AccessorID + tokenMap[token.AccessorID] = token + }) + + t.Run("Create with Accessor and Secret", func(t *testing.T) { + tokenInput := &structs.ACLToken{ + AccessorID: "dee863fa-e548-4c61-a96f-9aa07999249f", + SecretID: "10126ffa-b28f-4137-b9a9-e89ab1e97c5b", + Description: "test", + Policies: []structs.ACLTokenPolicyLink{ + structs.ACLTokenPolicyLink{ + ID: idMap["policy-test"], + Name: policyMap[idMap["policy-test"]].Name, + }, + structs.ACLTokenPolicyLink{ + ID: idMap["policy-read-all-nodes"], + Name: policyMap[idMap["policy-read-all-nodes"]].Name, + }, + }, + } + + req, _ := http.NewRequest("PUT", "/v1/acl/token?token=root", jsonBody(tokenInput)) + resp := httptest.NewRecorder() + obj, err := a.srv.ACLTokenCreate(resp, req) + require.NoError(t, err) + + token, ok := obj.(*structs.ACLToken) + require.True(t, ok) + + // 36 = length of the string form of uuids + require.Equal(t, tokenInput.SecretID, token.SecretID) + require.Equal(t, tokenInput.AccessorID, token.AccessorID) + require.Equal(t, tokenInput.Description, token.Description) + require.Equal(t, tokenInput.Policies, token.Policies) + require.True(t, token.CreateIndex > 0) + require.Equal(t, token.CreateIndex, token.ModifyIndex) + require.NotNil(t, token.Hash) + require.NotEqual(t, token.Hash, []byte{}) + + idMap["token-test"] = token.AccessorID + tokenMap[token.AccessorID] = token + }) + + t.Run("Create with Accessor Dup", func(t *testing.T) { + tokenInput := &structs.ACLToken{ + AccessorID: "dee863fa-e548-4c61-a96f-9aa07999249f", + Description: "test", + Policies: []structs.ACLTokenPolicyLink{ + structs.ACLTokenPolicyLink{ + ID: idMap["policy-test"], + Name: policyMap[idMap["policy-test"]].Name, + }, + structs.ACLTokenPolicyLink{ + ID: idMap["policy-read-all-nodes"], + Name: policyMap[idMap["policy-read-all-nodes"]].Name, + }, + }, + } + + req, _ := http.NewRequest("PUT", "/v1/acl/token?token=root", jsonBody(tokenInput)) + resp := httptest.NewRecorder() + _, err := a.srv.ACLTokenCreate(resp, req) + require.Error(t, err) + }) + + t.Run("Create with Secret as Accessor Dup", func(t *testing.T) { + tokenInput := &structs.ACLToken{ + SecretID: "dee863fa-e548-4c61-a96f-9aa07999249f", + Description: "test", + Policies: []structs.ACLTokenPolicyLink{ + structs.ACLTokenPolicyLink{ + ID: idMap["policy-test"], + Name: policyMap[idMap["policy-test"]].Name, + }, + structs.ACLTokenPolicyLink{ + ID: idMap["policy-read-all-nodes"], + Name: policyMap[idMap["policy-read-all-nodes"]].Name, + }, + }, + } + + req, _ := http.NewRequest("PUT", "/v1/acl/token?token=root", jsonBody(tokenInput)) + resp := httptest.NewRecorder() + _, err := a.srv.ACLTokenCreate(resp, req) + require.Error(t, err) + }) + + t.Run("Create with Secret Dup", func(t *testing.T) { + tokenInput := &structs.ACLToken{ + SecretID: "10126ffa-b28f-4137-b9a9-e89ab1e97c5b", + Description: "test", + Policies: []structs.ACLTokenPolicyLink{ + structs.ACLTokenPolicyLink{ + ID: idMap["policy-test"], + Name: policyMap[idMap["policy-test"]].Name, + }, + structs.ACLTokenPolicyLink{ + ID: idMap["policy-read-all-nodes"], + Name: policyMap[idMap["policy-read-all-nodes"]].Name, + }, + }, + } + + req, _ := http.NewRequest("PUT", "/v1/acl/token?token=root", jsonBody(tokenInput)) + resp := httptest.NewRecorder() + _, err := a.srv.ACLTokenCreate(resp, req) + require.Error(t, err) + }) + + t.Run("Create with Accessor as Secret Dup", func(t *testing.T) { + tokenInput := &structs.ACLToken{ + AccessorID: "10126ffa-b28f-4137-b9a9-e89ab1e97c5b", + Description: "test", + Policies: []structs.ACLTokenPolicyLink{ + structs.ACLTokenPolicyLink{ + ID: idMap["policy-test"], + Name: policyMap[idMap["policy-test"]].Name, + }, + structs.ACLTokenPolicyLink{ + ID: idMap["policy-read-all-nodes"], + Name: policyMap[idMap["policy-read-all-nodes"]].Name, + }, + }, + } + + req, _ := http.NewRequest("PUT", "/v1/acl/token?token=root", jsonBody(tokenInput)) + resp := httptest.NewRecorder() + _, err := a.srv.ACLTokenCreate(resp, req) + require.Error(t, err) + }) + + t.Run("Create with Reserved Accessor", func(t *testing.T) { + tokenInput := &structs.ACLToken{ + AccessorID: "00000000-0000-0000-0000-00000000005b", + Description: "test", + Policies: []structs.ACLTokenPolicyLink{ + structs.ACLTokenPolicyLink{ + ID: idMap["policy-test"], + Name: policyMap[idMap["policy-test"]].Name, + }, + structs.ACLTokenPolicyLink{ + ID: idMap["policy-read-all-nodes"], + Name: policyMap[idMap["policy-read-all-nodes"]].Name, + }, + }, + } + + req, _ := http.NewRequest("PUT", "/v1/acl/token?token=root", jsonBody(tokenInput)) + resp := httptest.NewRecorder() + _, err := a.srv.ACLTokenCreate(resp, req) + require.Error(t, err) + }) + + t.Run("Create with Reserved Secret", func(t *testing.T) { + tokenInput := &structs.ACLToken{ + SecretID: "00000000-0000-0000-0000-00000000005b", + Description: "test", + Policies: []structs.ACLTokenPolicyLink{ + structs.ACLTokenPolicyLink{ + ID: idMap["policy-test"], + Name: policyMap[idMap["policy-test"]].Name, + }, + structs.ACLTokenPolicyLink{ + ID: idMap["policy-read-all-nodes"], + Name: policyMap[idMap["policy-read-all-nodes"]].Name, + }, + }, + } + + req, _ := http.NewRequest("PUT", "/v1/acl/token?token=root", jsonBody(tokenInput)) + resp := httptest.NewRecorder() + _, err := a.srv.ACLTokenCreate(resp, req) + require.Error(t, err) + }) }) } diff --git a/agent/consul/acl_endpoint.go b/agent/consul/acl_endpoint.go index cc3b5da2e7..0825d57350 100644 --- a/agent/consul/acl_endpoint.go +++ b/agent/consul/acl_endpoint.go @@ -345,20 +345,62 @@ func (a *ACL) tokenSetInternal(args *structs.ACLTokenSetRequest, reply *structs. state := a.srv.fsm.State() - if token.AccessorID == "" { - // Token Create - var err error + var accessorMatch *structs.ACLToken + var secretMatch *structs.ACLToken + var err error - // Generate the AccessorID - token.AccessorID, err = lib.GenerateUUID(a.srv.checkTokenUUID) + if token.AccessorID != "" { + _, accessorMatch, err = state.ACLTokenGetByAccessor(nil, token.AccessorID) if err != nil { - return err + return fmt.Errorf("Failed acl token lookup by accessor: %v", err) + } + } + if token.SecretID != "" { + _, secretMatch, err = state.ACLTokenGetBySecret(nil, token.SecretID) + if err != nil { + return fmt.Errorf("Failed acl token lookup by secret: %v", err) + } + } + + if token.AccessorID == "" || args.Create { + // Token Create + + // Generate the AccessorID if not specified + if token.AccessorID == "" { + token.AccessorID, err = lib.GenerateUUID(a.srv.checkTokenUUID) + if err != nil { + return err + } + } else if _, err := uuid.ParseUUID(token.AccessorID); err != nil { + return fmt.Errorf("Invalid Token: AccessorID is not a valid UUID") + } else if accessorMatch != nil { + return fmt.Errorf("Invalid Token: AccessorID is already in use") + } else if _, match, err := state.ACLTokenGetBySecret(nil, token.AccessorID); err != nil || match != nil { + if err != nil { + return fmt.Errorf("Failed to lookup the acl token: %v", err) + } + return fmt.Errorf("Invalid Token: AccessorID is already in use") + } else if structs.ACLIDReserved(token.AccessorID) { + return fmt.Errorf("Invalid Token: UUIDs with the prefix %q are reserved", structs.ACLReservedPrefix) } - // Generate the SecretID - not supporting non-UUID secrets - token.SecretID, err = lib.GenerateUUID(a.srv.checkTokenUUID) - if err != nil { - return err + // Generate the AccessorID if not specified + if token.SecretID == "" { + token.SecretID, err = lib.GenerateUUID(a.srv.checkTokenUUID) + if err != nil { + return err + } + } else if _, err := uuid.ParseUUID(token.SecretID); err != nil { + return fmt.Errorf("Invalid Token: SecretID is not a valid UUID") + } else if secretMatch != nil { + return fmt.Errorf("Invalid Token: SecretID is already in use") + } else if _, match, err := state.ACLTokenGetByAccessor(nil, token.SecretID); err != nil || match != nil { + if err != nil { + return fmt.Errorf("Failed to lookup the acl token: %v", err) + } + return fmt.Errorf("Invalid Token: SecretID is already in use") + } else if structs.ACLIDReserved(token.SecretID) { + return fmt.Errorf("Invalid Token: UUIDs with the prefix %q are reserved", structs.ACLReservedPrefix) } token.CreateTime = time.Now() @@ -422,27 +464,23 @@ func (a *ACL) tokenSetInternal(args *structs.ACLTokenSetRequest, reply *structs. } // Verify the token exists - _, existing, err := state.ACLTokenGetByAccessor(nil, token.AccessorID) - if err != nil { - return fmt.Errorf("Failed to lookup the acl token %q: %v", token.AccessorID, err) - } - if existing == nil || existing.IsExpired(time.Now()) { + if accessorMatch == nil || accessorMatch.IsExpired(time.Now()) { return fmt.Errorf("Cannot find token %q", token.AccessorID) } if token.SecretID == "" { - token.SecretID = existing.SecretID - } else if existing.SecretID != token.SecretID { + token.SecretID = accessorMatch.SecretID + } else if accessorMatch.SecretID != token.SecretID { return fmt.Errorf("Changing a tokens SecretID is not permitted") } // Cannot toggle the "Global" mode - if token.Local != existing.Local { + if token.Local != accessorMatch.Local { return fmt.Errorf("cannot toggle local mode of %s", token.AccessorID) } if token.AuthMethod == "" { - token.AuthMethod = existing.AuthMethod - } else if token.AuthMethod != existing.AuthMethod { + token.AuthMethod = accessorMatch.AuthMethod + } else if token.AuthMethod != accessorMatch.AuthMethod { return fmt.Errorf("Cannot change AuthMethod of %s", token.AccessorID) } @@ -451,14 +489,14 @@ func (a *ACL) tokenSetInternal(args *structs.ACLTokenSetRequest, reply *structs. } if !token.HasExpirationTime() { - token.ExpirationTime = existing.ExpirationTime - } else if !existing.HasExpirationTime() { + token.ExpirationTime = accessorMatch.ExpirationTime + } else if !accessorMatch.HasExpirationTime() { return fmt.Errorf("Cannot change expiration time of %s", token.AccessorID) - } else if !token.ExpirationTime.Equal(*existing.ExpirationTime) { + } else if !token.ExpirationTime.Equal(*accessorMatch.ExpirationTime) { return fmt.Errorf("Cannot change expiration time of %s", token.AccessorID) } - token.CreateTime = existing.CreateTime + token.CreateTime = accessorMatch.CreateTime } policyIDs := make(map[string]struct{}) @@ -871,40 +909,46 @@ func (a *ACL) PolicySet(args *structs.ACLPolicySetRequest, reply *structs.ACLPol return fmt.Errorf("Invalid Policy: invalid Name. Only alphanumeric characters, '-' and '_' are allowed") } + var idMatch *structs.ACLPolicy + var nameMatch *structs.ACLPolicy + var err error + + if policy.ID != "" { + if _, err := uuid.ParseUUID(policy.ID); err != nil { + return fmt.Errorf("Policy ID invalid UUID") + } + + _, idMatch, err = state.ACLPolicyGetByID(nil, policy.ID) + if err != nil { + return fmt.Errorf("acl policy lookup by id failed: %v", err) + } + } + _, nameMatch, err = state.ACLPolicyGetByName(nil, policy.Name) + if err != nil { + return fmt.Errorf("acl policy lookup by name failed: %v", err) + } + if policy.ID == "" { // with no policy ID one will be generated var err error - policy.ID, err = lib.GenerateUUID(a.srv.checkPolicyUUID) if err != nil { return err } // validate the name is unique - if _, existing, err := state.ACLPolicyGetByName(nil, policy.Name); err != nil { - return fmt.Errorf("acl policy lookup by name failed: %v", err) - } else if existing != nil { + if nameMatch != nil { return fmt.Errorf("Invalid Policy: A Policy with Name %q already exists", policy.Name) } } else { - if _, err := uuid.ParseUUID(policy.ID); err != nil { - return fmt.Errorf("Policy ID invalid UUID") - } - // Verify the policy exists - _, existing, err := state.ACLPolicyGetByID(nil, policy.ID) - if err != nil { - return fmt.Errorf("acl policy lookup failed: %v", err) - } else if existing == nil { + if idMatch == nil { return fmt.Errorf("cannot find policy %s", policy.ID) } - if existing.Name != policy.Name { - if _, nameMatch, err := state.ACLPolicyGetByName(nil, policy.Name); err != nil { - return fmt.Errorf("acl policy lookup by name failed: %v", err) - } else if nameMatch != nil { - return fmt.Errorf("Invalid Policy: A policy with name %q already exists", policy.Name) - } + // Verify that the name isn't changing or that the name is not already used + if idMatch.Name != policy.Name && nameMatch != nil { + return fmt.Errorf("Invalid Policy: A policy with name %q already exists", policy.Name) } if policy.ID == structs.ACLPolicyGlobalManagementID { @@ -912,14 +956,14 @@ func (a *ACL) PolicySet(args *structs.ACLPolicySetRequest, reply *structs.ACLPol return fmt.Errorf("Changing the Datacenters of the builtin global-management policy is not permitted") } - if policy.Rules != existing.Rules { + if policy.Rules != idMatch.Rules { return fmt.Errorf("Changing the Rules for the builtin global-management policy is not permitted") } } } // validate the rules - _, err := acl.NewPolicyFromSource("", 0, policy.Rules, policy.Syntax, a.srv.sentinel) + _, err = acl.NewPolicyFromSource("", 0, policy.Rules, policy.Syntax, a.srv.sentinel) if err != nil { return err } diff --git a/agent/consul/acl_endpoint_test.go b/agent/consul/acl_endpoint_test.go index bfdecafea9..ff6f0ddf99 100644 --- a/agent/consul/acl_endpoint_test.go +++ b/agent/consul/acl_endpoint_test.go @@ -1496,6 +1496,276 @@ func TestACLEndpoint_TokenSet(t *testing.T) { }) } +func TestACLEndpoint_TokenSet_CustomID(t *testing.T) { + t.Parallel() + + dir1, s1 := testServerWithConfig(t, func(c *Config) { + c.ACLDatacenter = "dc1" + c.ACLsEnabled = true + c.ACLMasterToken = "root" + }) + defer os.RemoveAll(dir1) + defer s1.Shutdown() + codec := rpcClient(t, s1) + defer codec.Close() + + testrpc.WaitForLeader(t, s1.RPC, "dc1") + + acl := ACL{srv: s1} + + // No Create Arg + t.Run("no create arg", func(t *testing.T) { + req := structs.ACLTokenSetRequest{ + Datacenter: "dc1", + ACLToken: structs.ACLToken{ + AccessorID: "5d62a983-bcab-4e0c-9bcd-5dabebe3e273", + SecretID: "10a8ad77-2bdf-4939-a9d7-1b7de79d6beb", + Description: "foobar", + Policies: nil, + Local: false, + }, + WriteRequest: structs.WriteRequest{Token: "root"}, + } + + resp := structs.ACLToken{} + + err := acl.TokenSet(&req, &resp) + require.Error(t, err) + }) + + // Use the Create Arg + t.Run("create arg", func(t *testing.T) { + req := structs.ACLTokenSetRequest{ + Datacenter: "dc1", + ACLToken: structs.ACLToken{ + AccessorID: "5d62a983-bcab-4e0c-9bcd-5dabebe3e273", + SecretID: "10a8ad77-2bdf-4939-a9d7-1b7de79d6beb", + Description: "foobar", + Policies: nil, + Local: false, + }, + Create: true, + WriteRequest: structs.WriteRequest{Token: "root"}, + } + + resp := structs.ACLToken{} + + err := acl.TokenSet(&req, &resp) + require.NoError(t, err) + + // Get the token directly to validate that it exists + tokenResp, err := retrieveTestToken(codec, "root", "dc1", resp.AccessorID) + require.NoError(t, err) + token := tokenResp.Token + + require.Equal(t, req.ACLToken.AccessorID, token.AccessorID) + require.Equal(t, req.ACLToken.SecretID, token.SecretID) + require.Equal(t, token.Description, "foobar") + }) + + // Reserved AccessorID + t.Run("reserved AccessorID", func(t *testing.T) { + req := structs.ACLTokenSetRequest{ + Datacenter: "dc1", + ACLToken: structs.ACLToken{ + AccessorID: "00000000-0000-0000-0000-000000000073", + Description: "foobar", + Policies: nil, + Local: false, + }, + Create: true, + WriteRequest: structs.WriteRequest{Token: "root"}, + } + + resp := structs.ACLToken{} + + err := acl.TokenSet(&req, &resp) + require.Error(t, err) + }) + + // Reserved SecretID + t.Run("reserved SecretID", func(t *testing.T) { + req := structs.ACLTokenSetRequest{ + Datacenter: "dc1", + ACLToken: structs.ACLToken{ + SecretID: "00000000-0000-0000-0000-000000000073", + Description: "foobar", + Policies: nil, + Local: false, + }, + Create: true, + WriteRequest: structs.WriteRequest{Token: "root"}, + } + + resp := structs.ACLToken{} + + err := acl.TokenSet(&req, &resp) + require.Error(t, err) + }) + + // Accessor is dup + t.Run("accessor Dup", func(t *testing.T) { + req := structs.ACLTokenSetRequest{ + Datacenter: "dc1", + ACLToken: structs.ACLToken{ + AccessorID: "5d62a983-bcab-4e0c-9bcd-5dabebe3e273", + Description: "foobar", + Policies: nil, + Local: false, + }, + Create: true, + WriteRequest: structs.WriteRequest{Token: "root"}, + } + + resp := structs.ACLToken{} + + err := acl.TokenSet(&req, &resp) + require.Error(t, err) + }) + + // Accessor is dup of secret + t.Run("accessor dup of secret", func(t *testing.T) { + req := structs.ACLTokenSetRequest{ + Datacenter: "dc1", + ACLToken: structs.ACLToken{ + AccessorID: "10a8ad77-2bdf-4939-a9d7-1b7de79d6beb", + Description: "foobar", + Policies: nil, + Local: false, + }, + Create: true, + WriteRequest: structs.WriteRequest{Token: "root"}, + } + + resp := structs.ACLToken{} + + err := acl.TokenSet(&req, &resp) + require.Error(t, err) + }) + + // Secret is dup of Accessor + t.Run("secret dup of accessor", func(t *testing.T) { + req := structs.ACLTokenSetRequest{ + Datacenter: "dc1", + ACLToken: structs.ACLToken{ + SecretID: "5d62a983-bcab-4e0c-9bcd-5dabebe3e273", + Description: "foobar", + Policies: nil, + Local: false, + }, + Create: true, + WriteRequest: structs.WriteRequest{Token: "root"}, + } + + resp := structs.ACLToken{} + + err := acl.TokenSet(&req, &resp) + require.Error(t, err) + }) + + // Secret is dup + t.Run("secret dup", func(t *testing.T) { + req := structs.ACLTokenSetRequest{ + Datacenter: "dc1", + ACLToken: structs.ACLToken{ + SecretID: "10a8ad77-2bdf-4939-a9d7-1b7de79d6beb", + Description: "foobar", + Policies: nil, + Local: false, + }, + Create: true, + WriteRequest: structs.WriteRequest{Token: "root"}, + } + + resp := structs.ACLToken{} + + err := acl.TokenSet(&req, &resp) + require.Error(t, err) + }) + + // Update Accessor attempt + t.Run("update accessor", func(t *testing.T) { + req := structs.ACLTokenSetRequest{ + Datacenter: "dc1", + ACLToken: structs.ACLToken{ + AccessorID: "75a0d6a9-6882-4f7a-a053-906db1d55a73", + SecretID: "10a8ad77-2bdf-4939-a9d7-1b7de79d6beb", + Description: "foobar", + Policies: nil, + Local: false, + }, + WriteRequest: structs.WriteRequest{Token: "root"}, + } + + resp := structs.ACLToken{} + + err := acl.TokenSet(&req, &resp) + require.Error(t, err) + }) + + // Update Accessor attempt - with Create + t.Run("update accessor create", func(t *testing.T) { + req := structs.ACLTokenSetRequest{ + Datacenter: "dc1", + ACLToken: structs.ACLToken{ + AccessorID: "75a0d6a9-6882-4f7a-a053-906db1d55a73", + SecretID: "10a8ad77-2bdf-4939-a9d7-1b7de79d6beb", + Description: "foobar", + Policies: nil, + Local: false, + }, + Create: true, + WriteRequest: structs.WriteRequest{Token: "root"}, + } + + resp := structs.ACLToken{} + + err := acl.TokenSet(&req, &resp) + require.Error(t, err) + }) + + // Update Secret attempt + t.Run("update secret", func(t *testing.T) { + req := structs.ACLTokenSetRequest{ + Datacenter: "dc1", + ACLToken: structs.ACLToken{ + AccessorID: "5d62a983-bcab-4e0c-9bcd-5dabebe3e273", + SecretID: "f551f807-b3a7-4483-9ade-97230c974bf3", + Description: "foobar", + Policies: nil, + Local: false, + }, + WriteRequest: structs.WriteRequest{Token: "root"}, + } + + resp := structs.ACLToken{} + + err := acl.TokenSet(&req, &resp) + require.Error(t, err) + }) + + // Update Secret attempt - with Create + t.Run("update secret create", func(t *testing.T) { + req := structs.ACLTokenSetRequest{ + Datacenter: "dc1", + ACLToken: structs.ACLToken{ + AccessorID: "5d62a983-bcab-4e0c-9bcd-5dabebe3e273", + SecretID: "f551f807-b3a7-4483-9ade-97230c974bf3", + Description: "foobar", + Policies: nil, + Local: false, + }, + Create: true, + WriteRequest: structs.WriteRequest{Token: "root"}, + } + + resp := structs.ACLToken{} + + err := acl.TokenSet(&req, &resp) + require.Error(t, err) + }) +} + func TestACLEndpoint_TokenSet_anon(t *testing.T) { t.Parallel() @@ -1536,6 +1806,7 @@ func TestACLEndpoint_TokenSet_anon(t *testing.T) { tokenResp, err := retrieveTestToken(codec, "root", "dc1", structs.ACLTokenAnonymousID) require.Equal(t, len(tokenResp.Token.Policies), 1) require.Equal(t, tokenResp.Token.Policies[0].ID, policy.ID) + } func TestACLEndpoint_TokenDelete(t *testing.T) { @@ -1792,7 +2063,7 @@ func TestACLEndpoint_TokenList(t *testing.T) { require.NoError(t, err) t3, err := upsertTestToken(codec, "root", "dc1", func(token *structs.ACLToken) { - token.ExpirationTTL = 11 * time.Millisecond + token.ExpirationTTL = 20 * time.Millisecond }) require.NoError(t, err) @@ -2031,6 +2302,22 @@ func TestACLEndpoint_PolicySet(t *testing.T) { policyID = policy.ID }) + t.Run("Name Dup", func(t *testing.T) { + req := structs.ACLPolicySetRequest{ + Datacenter: "dc1", + Policy: structs.ACLPolicy{ + Description: "foobar", + Name: "baz", + Rules: "service \"\" { policy = \"read\" }", + }, + WriteRequest: structs.WriteRequest{Token: "root"}, + } + resp := structs.ACLPolicy{} + + err := acl.PolicySet(&req, &resp) + require.Error(t, err) + }) + t.Run("Update it", func(t *testing.T) { req := structs.ACLPolicySetRequest{ Datacenter: "dc1", @@ -2060,6 +2347,40 @@ func TestACLEndpoint_PolicySet(t *testing.T) { }) } +func TestACLEndpoint_PolicySet_CustomID(t *testing.T) { + t.Parallel() + + dir1, s1 := testServerWithConfig(t, func(c *Config) { + c.ACLDatacenter = "dc1" + c.ACLsEnabled = true + c.ACLMasterToken = "root" + }) + defer os.RemoveAll(dir1) + defer s1.Shutdown() + codec := rpcClient(t, s1) + defer codec.Close() + + testrpc.WaitForLeader(t, s1.RPC, "dc1") + + acl := ACL{srv: s1} + + // Attempt to create policy with ID + req := structs.ACLPolicySetRequest{ + Datacenter: "dc1", + Policy: structs.ACLPolicy{ + ID: "7ee166a5-b4b7-453c-bdc0-bca8ce50823e", + Description: "foobar", + Name: "baz", + Rules: "service \"\" { policy = \"read\" }", + }, + WriteRequest: structs.WriteRequest{Token: "root"}, + } + resp := structs.ACLPolicy{} + + err := acl.PolicySet(&req, &resp) + require.Error(t, err) +} + func TestACLEndpoint_PolicySet_globalManagement(t *testing.T) { t.Parallel() diff --git a/agent/consul/acl_replication_test.go b/agent/consul/acl_replication_test.go index e8a6a7d693..44be6e0b45 100644 --- a/agent/consul/acl_replication_test.go +++ b/agent/consul/acl_replication_test.go @@ -661,6 +661,7 @@ func TestACLReplication_TokensRedacted(t *testing.T) { } err := s2.RPC("ACL.TokenRead", &req, &tokenResp) require.NoError(r, err) + require.NotNil(r, tokenResp.Token) require.Equal(r, "root", tokenResp.Token.SecretID) var status structs.ACLReplicationStatus diff --git a/agent/structs/acl.go b/agent/structs/acl.go index 0fc63a12cd..96719fcd14 100644 --- a/agent/structs/acl.go +++ b/agent/structs/acl.go @@ -86,6 +86,8 @@ session_prefix "" { // user. ACLTokenAnonymousID = "00000000-0000-0000-0000-000000000002" + ACLReservedPrefix = "00000000-0000-0000-0000-0000000000" + // aclPolicyTemplateServiceIdentity is the template used for synthesizing // policies for service identities. aclPolicyTemplateServiceIdentity = ` @@ -104,7 +106,7 @@ node_prefix "" { ) func ACLIDReserved(id string) bool { - return strings.HasPrefix(id, "00000000-0000-0000-0000-0000000000") + return strings.HasPrefix(id, ACLReservedPrefix) } const ( @@ -1002,6 +1004,7 @@ type ACLReplicationStatus struct { // at the RPC layer type ACLTokenSetRequest struct { ACLToken ACLToken // Token to manipulate - I really dislike this name but "Token" is taken in the WriteRequest + Create bool // Used to explicitly mark this request as a creation Datacenter string // The datacenter to perform the request within WriteRequest } diff --git a/api/acl.go b/api/acl.go index 3327f667c3..124409ff2b 100644 --- a/api/acl.go +++ b/api/acl.go @@ -396,17 +396,9 @@ func (a *ACL) Replication(q *QueryOptions) (*ACLReplicationStatus, *QueryMeta, e return entries, qm, nil } -// TokenCreate creates a new ACL token. It requires that the AccessorID and SecretID fields -// of the ACLToken structure to be empty as these will be filled in by Consul. +// TokenCreate creates a new ACL token. If either the AccessorID or SecretID fields +// of the ACLToken structure are empty they will be filled in by Consul. func (a *ACL) TokenCreate(token *ACLToken, q *WriteOptions) (*ACLToken, *WriteMeta, error) { - if token.AccessorID != "" { - return nil, nil, fmt.Errorf("Cannot specify an AccessorID in Token Creation") - } - - if token.SecretID != "" { - return nil, nil, fmt.Errorf("Cannot specify a SecretID in Token Creation") - } - r := a.c.newRequest("PUT", "/v1/acl/token") r.setWriteOptions(q) r.obj = token @@ -567,7 +559,6 @@ func (a *ACL) PolicyCreate(policy *ACLPolicy, q *WriteOptions) (*ACLPolicy, *Wri if policy.ID != "" { return nil, nil, fmt.Errorf("Cannot specify an ID in Policy Creation") } - r := a.c.newRequest("PUT", "/v1/acl/policy") r.setWriteOptions(q) r.obj = policy diff --git a/command/acl/policy/create/policy_create_test.go b/command/acl/policy/create/policy_create_test.go index a9f917491c..59e51949de 100644 --- a/command/acl/policy/create/policy_create_test.go +++ b/command/acl/policy/create/policy_create_test.go @@ -11,7 +11,7 @@ import ( "github.com/hashicorp/consul/sdk/testutil" "github.com/hashicorp/consul/testrpc" "github.com/mitchellh/cli" - "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestPolicyCreateCommand_noTabs(t *testing.T) { @@ -24,7 +24,7 @@ func TestPolicyCreateCommand_noTabs(t *testing.T) { func TestPolicyCreateCommand(t *testing.T) { t.Parallel() - assert := assert.New(t) + require := require.New(t) testDir := testutil.TempDir(t, "acl") defer os.RemoveAll(testDir) @@ -48,7 +48,7 @@ func TestPolicyCreateCommand(t *testing.T) { rules := []byte("service \"\" { policy = \"write\" }") err := ioutil.WriteFile(testDir+"/rules.hcl", rules, 0644) - assert.NoError(err) + require.NoError(err) args := []string{ "-http-addr=" + a.HTTPAddr(), @@ -58,6 +58,6 @@ func TestPolicyCreateCommand(t *testing.T) { } code := cmd.Run(args) - assert.Equal(code, 0) - assert.Empty(ui.ErrorWriter.String()) + require.Equal(code, 0) + require.Empty(ui.ErrorWriter.String()) } diff --git a/command/acl/token/create/token_create.go b/command/acl/token/create/token_create.go index 5d55563919..f8c90d4228 100644 --- a/command/acl/token/create/token_create.go +++ b/command/acl/token/create/token_create.go @@ -23,19 +23,25 @@ type cmd struct { http *flags.HTTPFlags help string + accessor string + secret string policyIDs []string policyNames []string + description string roleIDs []string roleNames []string serviceIdents []string expirationTTL time.Duration - description string local bool showMeta bool } func (c *cmd) init() { c.flags = flag.NewFlagSet("", flag.ContinueOnError) + c.flags.StringVar(&c.accessor, "accessor", "", "Create the token with this Accessor ID. "+ + "It must be a UUID. If not specified one will be auto-generated") + c.flags.StringVar(&c.secret, "secret", "", "Create the token with this Secret ID. "+ + "It must be a UUID. If not specified one will be auto-generated") c.flags.BoolVar(&c.showMeta, "meta", false, "Indicates that token metadata such "+ "as the content hash and raft indices should be shown for each entry") c.flags.BoolVar(&c.local, "local", false, "Create this as a datacenter local token") @@ -80,6 +86,8 @@ func (c *cmd) Run(args []string) int { newToken := &api.ACLToken{ Description: c.description, Local: c.local, + AccessorID: c.accessor, + SecretID: c.secret, } if c.expirationTTL > 0 { newToken.ExpirationTTL = c.expirationTTL diff --git a/command/acl/token/create/token_create_test.go b/command/acl/token/create/token_create_test.go index c092180cc3..d2efc4d8b1 100644 --- a/command/acl/token/create/token_create_test.go +++ b/command/acl/token/create/token_create_test.go @@ -11,7 +11,7 @@ import ( "github.com/hashicorp/consul/sdk/testutil" "github.com/hashicorp/consul/testrpc" "github.com/mitchellh/cli" - "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestTokenCreateCommand_noTabs(t *testing.T) { @@ -24,7 +24,7 @@ func TestTokenCreateCommand_noTabs(t *testing.T) { func TestTokenCreateCommand(t *testing.T) { t.Parallel() - assert := assert.New(t) + require := require.New(t) testDir := testutil.TempDir(t, "acl") defer os.RemoveAll(testDir) @@ -53,7 +53,7 @@ func TestTokenCreateCommand(t *testing.T) { &api.ACLPolicy{Name: "test-policy"}, &api.WriteOptions{Token: "root"}, ) - assert.NoError(err) + require.NoError(err) // create with policy by name { @@ -65,8 +65,8 @@ func TestTokenCreateCommand(t *testing.T) { } code := cmd.Run(args) - assert.Equal(code, 0) - assert.Empty(ui.ErrorWriter.String()) + require.Equal(code, 0) + require.Empty(ui.ErrorWriter.String()) } // create with policy by id @@ -79,7 +79,38 @@ func TestTokenCreateCommand(t *testing.T) { } code := cmd.Run(args) - assert.Equal(code, 0) - assert.Empty(ui.ErrorWriter.String()) + require.Empty(ui.ErrorWriter.String()) + require.Equal(code, 0) + } + + // create with accessor and secret + { + args := []string{ + "-http-addr=" + a.HTTPAddr(), + "-token=root", + "-policy-id=" + policy.ID, + "-description=test token", + "-accessor=3d852bb8-5153-4388-a3ca-8ca78661889f", + "-secret=3a69a8d8-c4d4-485d-9b19-b5b61648ea0c", + } + + code := cmd.Run(args) + require.Empty(ui.ErrorWriter.String()) + require.Equal(code, 0) + + conf := api.DefaultConfig() + conf.Address = a.HTTPAddr() + conf.Token = "root" + + // going to use the API client to grab the token - we could potentially try to grab the values + // out of the command output but this seems easier. + client, err := api.NewClient(conf) + require.NoError(err) + require.NotNil(client) + + token, _, err := client.ACL().TokenRead("3d852bb8-5153-4388-a3ca-8ca78661889f", nil) + require.NoError(err) + require.Equal("3d852bb8-5153-4388-a3ca-8ca78661889f", token.AccessorID) + require.Equal("3a69a8d8-c4d4-485d-9b19-b5b61648ea0c", token.SecretID) } } diff --git a/command/acl/token/update/token_update.go b/command/acl/token/update/token_update.go index 72663f8193..4f814aa674 100644 --- a/command/acl/token/update/token_update.go +++ b/command/acl/token/update/token_update.go @@ -46,7 +46,7 @@ func (c *cmd) init() { "with the existing roles") c.flags.BoolVar(&c.mergeServiceIdents, "merge-service-identities", false, "Merge the new service identities "+ "with the existing service identities") - c.flags.StringVar(&c.tokenID, "id", "", "The Accessor ID of the token to read. "+ + c.flags.StringVar(&c.tokenID, "id", "", "The Accessor ID of the token to update. "+ "It may be specified as a unique ID prefix but will error if the prefix "+ "matches multiple token Accessor IDs") c.flags.StringVar(&c.description, "description", "", "A description of the token") diff --git a/website/source/api/acl/tokens.html.md b/website/source/api/acl/tokens.html.md index 1d13a34571..644ce8c701 100644 --- a/website/source/api/acl/tokens.html.md +++ b/website/source/api/acl/tokens.html.md @@ -35,6 +35,14 @@ The table below shows this endpoint's support for ### Parameters +- `AccessorID` `(string: "")` - Specifies a UUID to use as the token's Accessor ID. + If not specified a UUID will be generated for this field. Added in v1.5.0. + +- `SecretID` `(string: "")` - Specifies a UUID to use as the token's Secret ID. + If not specified a UUID will be generated for this field. Added in v1.5.0. + **Note**: The SecretID is used to authorize operations against Consul and should + be generated from an appropriate cryptographic source. + - `Description` `(string: "")` - Free form human readable description of the token. - `Policies` `(array)` - The list of policies that should diff --git a/website/source/docs/commands/acl/acl-token.html.md.erb b/website/source/docs/commands/acl/acl-token.html.md.erb index ac30448092..e7e3147c00 100644 --- a/website/source/docs/commands/acl/acl-token.html.md.erb +++ b/website/source/docs/commands/acl/acl-token.html.md.erb @@ -60,6 +60,9 @@ may use a unique prefix of the UUID as a shortcut for specifying the entire UUID * [Common Subcommand Options](#common-subcommand-options) +* `-accessor=` - Create the token with this Accessor ID. It must be a UUID. If not + specified one will be auto-generated + * `-description=` - A description of the token. * `-local` - Create this as a datacenter local token. @@ -71,6 +74,11 @@ may use a unique prefix of the UUID as a shortcut for specifying the entire UUID * `-meta` - Indicates that token metadata such as the content hash and raft indices should be shown for each entry. +* `-secret=` - Create the token with this Secret ID. It must be a UUID. If not + specified one will be auto-generated. + **Note**: The SecretID is used to authorize operations against Consul and should + be generated from an appropriate cryptographic source. + ### Examples Create a new token: