diff --git a/agent/consul/acl_endpoint.go b/agent/consul/acl_endpoint.go index 6ebbf89f0c..798b864a3b 100644 --- a/agent/consul/acl_endpoint.go +++ b/agent/consul/acl_endpoint.go @@ -482,9 +482,15 @@ func (a *ACL) TokenDelete(args *structs.ACLTokenDeleteRequest, reply *string) er return err } - if !a.srv.InACLDatacenter() && !token.Local { - args.Datacenter = a.srv.config.ACLDatacenter - return a.srv.forwardDC("ACL.TokenDelete", a.srv.config.ACLDatacenter, args, reply) + if token != nil { + if args.Token == token.SecretID { + return fmt.Errorf("Deletion of the request's authorization token is not permitted") + } + + if !a.srv.InACLDatacenter() && !token.Local { + args.Datacenter = a.srv.config.ACLDatacenter + return a.srv.forwardDC("ACL.TokenDelete", a.srv.config.ACLDatacenter, args, reply) + } } req := &structs.ACLTokenBatchDeleteRequest{ diff --git a/agent/consul/acl_endpoint_test.go b/agent/consul/acl_endpoint_test.go index 8185bad7d5..f6664183a7 100644 --- a/agent/consul/acl_endpoint_test.go +++ b/agent/consul/acl_endpoint_test.go @@ -873,10 +873,31 @@ func TestACLEndpoint_TokenDelete(t *testing.T) { testrpc.WaitForLeader(t, s1.RPC, "dc1") + dir2, s2 := testServerWithConfig(t, func(c *Config) { + c.ACLDatacenter = "dc1" + c.ACLsEnabled = true + c.Datacenter = "dc2" + // token replication is required to test deleting non-local tokens in secondary dc + c.ACLTokenReplication = true + }) + defer os.RemoveAll(dir2) + defer s2.Shutdown() + codec2 := rpcClient(t, s2) + defer codec2.Close() + + s2.tokens.UpdateACLReplicationToken("root") + + testrpc.WaitForLeader(t, s1.RPC, "dc1") + testrpc.WaitForLeader(t, s2.RPC, "dc2") + + // Try to join + joinWAN(t, s2, s1) + existingToken, err := upsertTestToken(codec, "root", "dc1") assert.NoError(err) acl := ACL{srv: s1} + acl2 := ACL{srv: s2} // deletes a token { @@ -897,6 +918,32 @@ func TestACLEndpoint_TokenDelete(t *testing.T) { assert.NoError(err) } + // can't delete itself + { + readReq := structs.ACLTokenGetRequest{ + Datacenter: "dc1", + TokenID: "root", + TokenIDType: structs.ACLTokenSecret, + QueryOptions: structs.QueryOptions{Token: "root"}, + } + + var out structs.ACLTokenResponse + + err := msgpackrpc.CallWithCodec(codec, "ACL.TokenRead", &readReq, &out) + + assert.NoError(err) + + req := structs.ACLTokenDeleteRequest{ + Datacenter: "dc1", + TokenID: out.Token.AccessorID, + WriteRequest: structs.WriteRequest{Token: "root"}, + } + + var resp string + err = acl.TokenDelete(&req, &resp) + assert.EqualError(err, "Deletion of the request's authorization token is not permitted") + } + // errors when token doesn't exist { fakeID, err := uuid.GenerateUUID() @@ -918,6 +965,30 @@ func TestACLEndpoint_TokenDelete(t *testing.T) { assert.Nil(tokenResp.Token) assert.NoError(err) } + + // don't segfault when attempting to delete non existant token in secondary dc + { + fakeID, err := uuid.GenerateUUID() + assert.NoError(err) + + req := structs.ACLTokenDeleteRequest{ + Datacenter: "dc2", + TokenID: fakeID, + WriteRequest: structs.WriteRequest{Token: "root"}, + } + + var resp string + + waitForNewACLs(t, s2) + + err = acl2.TokenDelete(&req, &resp) + assert.NoError(err) + + // token should be nil + tokenResp, err := retrieveTestToken(codec2, "root", "dc1", existingToken.AccessorID) + assert.Nil(tokenResp.Token) + assert.NoError(err) + } } func TestACLEndpoint_TokenDelete_anon(t *testing.T) { t.Parallel() diff --git a/agent/consul/helper_test.go b/agent/consul/helper_test.go index d19f2cfaeb..a488ee0ae1 100644 --- a/agent/consul/helper_test.go +++ b/agent/consul/helper_test.go @@ -9,6 +9,7 @@ import ( "github.com/hashicorp/consul/testutil/retry" "github.com/hashicorp/raft" "github.com/hashicorp/serf/serf" + "github.com/stretchr/testify/require" ) func waitForLeader(servers ...*Server) error { @@ -152,6 +153,14 @@ func joinWAN(t *testing.T, member, leader *Server) { } } +func waitForNewACLs(t *testing.T, server *Server) { + retry.Run(t, func(r *retry.R) { + require.False(r, server.UseLegacyACLs(), "Server cannot use new ACLs") + }) + + require.False(t, server.UseLegacyACLs(), "Server cannot use new ACLs") +} + func seeEachOther(a, b []serf.Member, addra, addrb string) bool { return serfMembersContains(a, addrb) && serfMembersContains(b, addra) }