From fa293362beb0e0dbaeb516c9874889d6e5689a47 Mon Sep 17 00:00:00 2001 From: "Chris S. Kim" Date: Tue, 26 Oct 2021 15:20:57 -0400 Subject: [PATCH] agent: Ensure partition is considered in agent endpoints (#11427) --- agent/acl_endpoint_legacy_test.go | 2 +- agent/agent.go | 2 +- agent/agent_endpoint.go | 58 +++++++++++++++++++++++-------- agent/agent_endpoint_test.go | 28 +++++++-------- agent/consul/acl_client.go | 4 +++ agent/consul/acl_server.go | 4 +++ 6 files changed, 68 insertions(+), 30 deletions(-) diff --git a/agent/acl_endpoint_legacy_test.go b/agent/acl_endpoint_legacy_test.go index 3dc0f76f0a..8c7fc251c5 100644 --- a/agent/acl_endpoint_legacy_test.go +++ b/agent/acl_endpoint_legacy_test.go @@ -28,7 +28,7 @@ func TestHTTPHandlers_ACLLegacy(t *testing.T) { resp := httptest.NewRecorder() a.srv.h.ServeHTTP(resp, req) - require.Equal(t, resp.Code, http.StatusGone) + require.Equal(t, http.StatusGone, resp.Code) require.Contains(t, resp.Body.String(), "the legacy ACL system was removed") } diff --git a/agent/agent.go b/agent/agent.go index e5385abbce..5464994b20 100644 --- a/agent/agent.go +++ b/agent/agent.go @@ -139,7 +139,7 @@ type delegate interface { // ResolveTokenAndDefaultMeta returns an acl.Authorizer which authorizes // actions based on the permissions granted to the token. // If either entMeta or authzContext are non-nil they will be populated with the - // default namespace from the token. + // default partition and namespace from the token. ResolveTokenAndDefaultMeta(token string, entMeta *structs.EnterpriseMeta, authzContext *acl.AuthorizerContext) (acl.Authorizer, error) RPC(method string, args interface{}, reply interface{}) error diff --git a/agent/agent_endpoint.go b/agent/agent_endpoint.go index a22a3ea9d2..c79a0af1b1 100644 --- a/agent/agent_endpoint.go +++ b/agent/agent_endpoint.go @@ -55,7 +55,11 @@ func (s *HTTPHandlers) AgentSelf(resp http.ResponseWriter, req *http.Request) (i if err != nil { return nil, err } - if authz.AgentRead(s.agent.config.NodeName, nil) != acl.Allow { + + // Authorize using the agent's own enterprise meta, not the token. + var authzContext acl.AuthorizerContext + s.agent.agentEnterpriseMeta().FillAuthzContext(&authzContext) + if authz.AgentRead(s.agent.config.NodeName, &authzContext) != acl.Allow { return nil, acl.ErrPermissionDenied } @@ -141,7 +145,11 @@ func (s *HTTPHandlers) AgentMetrics(resp http.ResponseWriter, req *http.Request) if err != nil { return nil, err } - if authz.AgentRead(s.agent.config.NodeName, nil) != acl.Allow { + + // Authorize using the agent's own enterprise meta, not the token. + var authzContext acl.AuthorizerContext + s.agent.agentEnterpriseMeta().FillAuthzContext(&authzContext) + if authz.AgentRead(s.agent.config.NodeName, &authzContext) != acl.Allow { return nil, acl.ErrPermissionDenied } if enablePrometheusOutput(req) { @@ -169,10 +177,14 @@ func (s *HTTPHandlers) AgentMetricsStream(resp http.ResponseWriter, req *http.Re var token string s.parseToken(req, &token) authz, err := s.agent.delegate.ResolveTokenAndDefaultMeta(token, nil, nil) - switch { - case err != nil: + if err != nil { return nil, err - case authz.AgentRead(s.agent.config.NodeName, nil) != acl.Allow: + } + + // Authorize using the agent's own enterprise meta, not the token. + var authzContext acl.AuthorizerContext + s.agent.agentEnterpriseMeta().FillAuthzContext(&authzContext) + if authz.AgentRead(s.agent.config.NodeName, &authzContext) != acl.Allow { return nil, acl.ErrPermissionDenied } @@ -221,7 +233,11 @@ func (s *HTTPHandlers) AgentReload(resp http.ResponseWriter, req *http.Request) if err != nil { return nil, err } - if authz.AgentWrite(s.agent.config.NodeName, nil) != acl.Allow { + + // Authorize using the agent's own enterprise meta, not the token. + var authzContext acl.AuthorizerContext + s.agent.agentEnterpriseMeta().FillAuthzContext(&authzContext) + if authz.AgentWrite(s.agent.config.NodeName, &authzContext) != acl.Allow { return nil, acl.ErrPermissionDenied } @@ -528,7 +544,11 @@ func (s *HTTPHandlers) AgentJoin(resp http.ResponseWriter, req *http.Request) (i if err != nil { return nil, err } - if authz.AgentWrite(s.agent.config.NodeName, nil) != acl.Allow { + + // Authorize using the agent's own enterprise meta, not the token. + var authzContext acl.AuthorizerContext + s.agent.agentEnterpriseMeta().FillAuthzContext(&authzContext) + if authz.AgentWrite(s.agent.config.NodeName, &authzContext) != acl.Allow { return nil, acl.ErrPermissionDenied } @@ -561,7 +581,11 @@ func (s *HTTPHandlers) AgentLeave(resp http.ResponseWriter, req *http.Request) ( if err != nil { return nil, err } - if authz.AgentWrite(s.agent.config.NodeName, nil) != acl.Allow { + + // Authorize using the agent's own enterprise meta, not the token. + var authzContext acl.AuthorizerContext + s.agent.agentEnterpriseMeta().FillAuthzContext(&authzContext) + if authz.AgentWrite(s.agent.config.NodeName, &authzContext) != acl.Allow { return nil, acl.ErrPermissionDenied } @@ -584,7 +608,7 @@ func (s *HTTPHandlers) AgentForceLeave(resp http.ResponseWriter, req *http.Reque return nil, acl.ErrPermissionDenied } - //Check the value of the prune query + // Check the value of the prune query _, prune := req.URL.Query()["prune"] addr := strings.TrimPrefix(req.URL.Path, "/v1/agent/force-leave/") @@ -1272,7 +1296,11 @@ func (s *HTTPHandlers) AgentMonitor(resp http.ResponseWriter, req *http.Request) if err != nil { return nil, err } - if authz.AgentRead(s.agent.config.NodeName, nil) != acl.Allow { + + // Authorize using the agent's own enterprise meta, not the token. + var authzContext acl.AuthorizerContext + s.agent.agentEnterpriseMeta().FillAuthzContext(&authzContext) + if authz.AgentRead(s.agent.config.NodeName, &authzContext) != acl.Allow { return nil, acl.ErrPermissionDenied } @@ -1351,7 +1379,11 @@ func (s *HTTPHandlers) AgentToken(resp http.ResponseWriter, req *http.Request) ( if err != nil { return nil, err } - if authz.AgentWrite(s.agent.config.NodeName, nil) != acl.Allow { + + // Authorize using the agent's own enterprise meta, not the token. + var authzContext acl.AuthorizerContext + s.agent.agentEnterpriseMeta().FillAuthzContext(&authzContext) + if authz.AgentWrite(s.agent.config.NodeName, &authzContext) != acl.Allow { return nil, acl.ErrPermissionDenied } @@ -1359,9 +1391,7 @@ func (s *HTTPHandlers) AgentToken(resp http.ResponseWriter, req *http.Request) ( // fields to this later if needed. var args api.AgentToken if err := decodeBody(req.Body, &args); err != nil { - resp.WriteHeader(http.StatusBadRequest) - fmt.Fprintf(resp, "Request decode failed: %v", err) - return nil, nil + return nil, BadRequestError{Reason: fmt.Sprintf("Request decode failed: %v", err)} } // Figure out the target token. diff --git a/agent/agent_endpoint_test.go b/agent/agent_endpoint_test.go index 6450f7acda..2c2ca91238 100644 --- a/agent/agent_endpoint_test.go +++ b/agent/agent_endpoint_test.go @@ -4685,7 +4685,7 @@ func TestAgent_NodeMaintenance_BadRequest(t *testing.T) { testrpc.WaitForTestAgent(t, a.RPC, "dc1") // Fails when no enable flag provided - req, _ := http.NewRequest("PUT", "/v1/agent/self/maintenance", nil) + req, _ := http.NewRequest("PUT", "/v1/agent/maintenance", nil) resp := httptest.NewRecorder() if _, err := a.srv.AgentNodeMaintenance(resp, req); err != nil { t.Fatalf("err: %s", err) @@ -4706,7 +4706,7 @@ func TestAgent_NodeMaintenance_Enable(t *testing.T) { testrpc.WaitForTestAgent(t, a.RPC, "dc1") // Force the node into maintenance mode - req, _ := http.NewRequest("PUT", "/v1/agent/self/maintenance?enable=true&reason=broken&token=mytoken", nil) + req, _ := http.NewRequest("PUT", "/v1/agent/maintenance?enable=true&reason=broken&token=mytoken", nil) resp := httptest.NewRecorder() if _, err := a.srv.AgentNodeMaintenance(resp, req); err != nil { t.Fatalf("err: %s", err) @@ -4746,7 +4746,7 @@ func TestAgent_NodeMaintenance_Disable(t *testing.T) { a.EnableNodeMaintenance("", "") // Leave maintenance mode - req, _ := http.NewRequest("PUT", "/v1/agent/self/maintenance?enable=false", nil) + req, _ := http.NewRequest("PUT", "/v1/agent/maintenance?enable=false", nil) resp := httptest.NewRecorder() if _, err := a.srv.AgentNodeMaintenance(resp, req); err != nil { t.Fatalf("err: %s", err) @@ -4772,14 +4772,14 @@ func TestAgent_NodeMaintenance_ACLDeny(t *testing.T) { testrpc.WaitForLeader(t, a.RPC, "dc1") t.Run("no token", func(t *testing.T) { - req, _ := http.NewRequest("PUT", "/v1/agent/self/maintenance?enable=true&reason=broken", nil) + req, _ := http.NewRequest("PUT", "/v1/agent/maintenance?enable=true&reason=broken", nil) if _, err := a.srv.AgentNodeMaintenance(nil, req); !acl.IsErrPermissionDenied(err) { t.Fatalf("err: %v", err) } }) t.Run("root token", func(t *testing.T) { - req, _ := http.NewRequest("PUT", "/v1/agent/self/maintenance?enable=true&reason=broken&token=root", nil) + req, _ := http.NewRequest("PUT", "/v1/agent/maintenance?enable=true&reason=broken&token=root", nil) if _, err := a.srv.AgentNodeMaintenance(nil, req); err != nil { t.Fatalf("err: %v", err) } @@ -5227,21 +5227,21 @@ func TestAgent_Token(t *testing.T) { init tokens raw tokens effective tokens - expectedErr error + expectedErr string }{ { name: "bad token name", method: "PUT", url: "nope?token=root", body: body("X"), - expectedErr: NotFoundError{Reason: `Token "nope" is unknown`}, + expectedErr: `Token "nope" is unknown`, }, { - name: "bad JSON", - method: "PUT", - url: "acl_token?token=root", - body: badJSON(), - code: http.StatusBadRequest, + name: "bad JSON", + method: "PUT", + url: "acl_token?token=root", + body: badJSON(), + expectedErr: `Bad request: Request decode failed: json: cannot unmarshal bool into Go value of type api.AgentToken`, }, { name: "set user legacy", @@ -5398,8 +5398,8 @@ func TestAgent_Token(t *testing.T) { req, _ := http.NewRequest(tt.method, url, tt.body) _, err := a.srv.AgentToken(resp, req) - if tt.expectedErr != nil { - require.Equal(t, tt.expectedErr, err) + if tt.expectedErr != "" { + require.EqualError(t, err, tt.expectedErr) return } require.NoError(t, err) diff --git a/agent/consul/acl_client.go b/agent/consul/acl_client.go index 5ace9ce616..36f3f19a79 100644 --- a/agent/consul/acl_client.go +++ b/agent/consul/acl_client.go @@ -58,6 +58,10 @@ func (c *Client) ResolveTokenAndDefaultMeta(token string, entMeta *structs.Enter return nil, err } + if entMeta == nil { + entMeta = &structs.EnterpriseMeta{} + } + // Default the EnterpriseMeta based on the Tokens meta or actual defaults // in the case of unknown identity if identity != nil { diff --git a/agent/consul/acl_server.go b/agent/consul/acl_server.go index 33372bae8f..21969ca49b 100644 --- a/agent/consul/acl_server.go +++ b/agent/consul/acl_server.go @@ -178,6 +178,10 @@ func (s *Server) ResolveTokenAndDefaultMeta(token string, entMeta *structs.Enter return nil, err } + if entMeta == nil { + entMeta = &structs.EnterpriseMeta{} + } + // Default the EnterpriseMeta based on the Tokens meta or actual defaults // in the case of unknown identity if identity != nil {