From 2a2c131a64a2062c6911ba32202641aa66be1805 Mon Sep 17 00:00:00 2001 From: Buck Doyle Date: Fri, 26 Jul 2019 13:07:42 -0500 Subject: [PATCH 1/3] Update policy endpoint to permit anonymous access --- nomad/acl_endpoint.go | 47 ++++++++++++++++++++------------------ nomad/acl_endpoint_test.go | 22 ++++++++++++++++-- 2 files changed, 45 insertions(+), 24 deletions(-) diff --git a/nomad/acl_endpoint.go b/nomad/acl_endpoint.go index 7c382e4dcb98..bb19962fc117 100644 --- a/nomad/acl_endpoint.go +++ b/nomad/acl_endpoint.go @@ -219,32 +219,35 @@ func (a *ACL) GetPolicy(args *structs.ACLPolicySpecificRequest, reply *structs.S return structs.ErrPermissionDenied } - // If it is not a management token determine if it can get this policy - mgt := acl.IsManagement() - if !mgt { - snap, err := a.srv.fsm.State().Snapshot() - if err != nil { - return err - } + // If the policy is the anonymous one, anyone can get it + if args.Name != "anonymous" { + // If it is not a management token determine if it can get this policy + mgt := acl.IsManagement() + if !mgt { + snap, err := a.srv.fsm.State().Snapshot() + if err != nil { + return err + } - token, err := snap.ACLTokenBySecretID(nil, args.AuthToken) - if err != nil { - return err - } - if token == nil { - return structs.ErrTokenNotFound - } + token, err := snap.ACLTokenBySecretID(nil, args.AuthToken) + if err != nil { + return err + } + if token == nil { + return structs.ErrTokenNotFound + } - found := false - for _, p := range token.Policies { - if p == args.Name { - found = true - break + found := false + for _, p := range token.Policies { + if p == args.Name { + found = true + break + } } - } - if !found { - return structs.ErrPermissionDenied + if !found { + return structs.ErrPermissionDenied + } } } diff --git a/nomad/acl_endpoint_test.go b/nomad/acl_endpoint_test.go index 4f55a7e1dbeb..984346a362a7 100644 --- a/nomad/acl_endpoint_test.go +++ b/nomad/acl_endpoint_test.go @@ -28,10 +28,14 @@ func TestACLEndpoint_GetPolicy(t *testing.T) { policy := mock.ACLPolicy() s1.fsm.State().UpsertACLPolicies(1000, []*structs.ACLPolicy{policy}) + anonymousPolicy := mock.ACLPolicy() + anonymousPolicy.Name = "anonymous" + s1.fsm.State().UpsertACLPolicies(1001, []*structs.ACLPolicy{anonymousPolicy}) + // Create a token with one the policy token := mock.ACLToken() token.Policies = []string{policy.Name} - s1.fsm.State().UpsertACLTokens(1001, []*structs.ACLToken{token}) + s1.fsm.State().UpsertACLTokens(1002, []*structs.ACLToken{token}) // Lookup the policy get := &structs.ACLPolicySpecificRequest{ @@ -53,7 +57,7 @@ func TestACLEndpoint_GetPolicy(t *testing.T) { if err := msgpackrpc.CallWithCodec(codec, "ACL.GetPolicy", get, &resp); err != nil { t.Fatalf("err: %v", err) } - assert.Equal(t, uint64(1000), resp.Index) + assert.Equal(t, uint64(1001), resp.Index) assert.Nil(t, resp.Policy) // Lookup the policy with the token @@ -70,6 +74,20 @@ func TestACLEndpoint_GetPolicy(t *testing.T) { } assert.EqualValues(t, 1000, resp2.Index) assert.Equal(t, policy, resp2.Policy) + + // Lookup the anonymous policy with no token + get = &structs.ACLPolicySpecificRequest{ + Name: anonymousPolicy.Name, + QueryOptions: structs.QueryOptions{ + Region: "global", + }, + } + var resp3 structs.SingleACLPolicyResponse + if err := msgpackrpc.CallWithCodec(codec, "ACL.GetPolicy", get, &resp3); err != nil { + t.Fatalf("err: %v", err) + } + assert.EqualValues(t, 1001, resp3.Index) + assert.Equal(t, anonymousPolicy, resp3.Policy) } func TestACLEndpoint_GetPolicy_Blocking(t *testing.T) { From dd704e585fd4b75d9ecc78d6a1a984398000c5d5 Mon Sep 17 00:00:00 2001 From: Buck Doyle Date: Mon, 29 Jul 2019 10:35:07 -0500 Subject: [PATCH 2/3] Update assertion to use better failure-reporting --- nomad/acl_endpoint_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/nomad/acl_endpoint_test.go b/nomad/acl_endpoint_test.go index 984346a362a7..6d2927f86e54 100644 --- a/nomad/acl_endpoint_test.go +++ b/nomad/acl_endpoint_test.go @@ -15,6 +15,7 @@ import ( "github.com/hashicorp/nomad/nomad/structs" "github.com/hashicorp/nomad/testutil" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestACLEndpoint_GetPolicy(t *testing.T) { @@ -84,7 +85,7 @@ func TestACLEndpoint_GetPolicy(t *testing.T) { } var resp3 structs.SingleACLPolicyResponse if err := msgpackrpc.CallWithCodec(codec, "ACL.GetPolicy", get, &resp3); err != nil { - t.Fatalf("err: %v", err) + require.NoError(t, err) } assert.EqualValues(t, 1001, resp3.Index) assert.Equal(t, anonymousPolicy, resp3.Policy) From 8389af6ae64fe321de28456970cb8f40154b0869 Mon Sep 17 00:00:00 2001 From: Buck Doyle Date: Mon, 29 Jul 2019 10:38:07 -0500 Subject: [PATCH 3/3] Combine conditionals --- nomad/acl_endpoint.go | 46 +++++++++++++++++++++---------------------- 1 file changed, 22 insertions(+), 24 deletions(-) diff --git a/nomad/acl_endpoint.go b/nomad/acl_endpoint.go index bb19962fc117..8f3b3a0d6e22 100644 --- a/nomad/acl_endpoint.go +++ b/nomad/acl_endpoint.go @@ -220,34 +220,32 @@ func (a *ACL) GetPolicy(args *structs.ACLPolicySpecificRequest, reply *structs.S } // If the policy is the anonymous one, anyone can get it - if args.Name != "anonymous" { - // If it is not a management token determine if it can get this policy - mgt := acl.IsManagement() - if !mgt { - snap, err := a.srv.fsm.State().Snapshot() - if err != nil { - return err - } + // If it is not a management token determine if it can get this policy + mgt := acl.IsManagement() + if !mgt && args.Name != "anonymous" { + snap, err := a.srv.fsm.State().Snapshot() + if err != nil { + return err + } - token, err := snap.ACLTokenBySecretID(nil, args.AuthToken) - if err != nil { - return err - } - if token == nil { - return structs.ErrTokenNotFound - } + token, err := snap.ACLTokenBySecretID(nil, args.AuthToken) + if err != nil { + return err + } + if token == nil { + return structs.ErrTokenNotFound + } - found := false - for _, p := range token.Policies { - if p == args.Name { - found = true - break - } + found := false + for _, p := range token.Policies { + if p == args.Name { + found = true + break } + } - if !found { - return structs.ErrPermissionDenied - } + if !found { + return structs.ErrPermissionDenied } }