From cf30d409c97f9fe7a7c1ac3fd8373a46ab1e97a0 Mon Sep 17 00:00:00 2001 From: James Phillips Date: Tue, 28 Nov 2017 13:47:30 -0800 Subject: [PATCH] Moves ACL disabled response logic down into endpoints. This lets us make the registration of endpoints less fancy, on the road to adding a registration mechanism. --- agent/acl_endpoint.go | 35 +++++++++++++++++++++++++++++--- agent/acl_endpoint_test.go | 39 ++++++++++++++++++++++++++++++++++++ agent/agent_endpoint.go | 3 +++ agent/agent_endpoint_test.go | 4 ++++ agent/http.go | 30 +++++++++------------------ 5 files changed, 87 insertions(+), 24 deletions(-) diff --git a/agent/acl_endpoint.go b/agent/acl_endpoint.go index c30f5f55e..9a40a6596 100644 --- a/agent/acl_endpoint.go +++ b/agent/acl_endpoint.go @@ -14,16 +14,24 @@ type aclCreateResponse struct { ID string } -// ACLDisabled handles if ACL datacenter is not configured -func ACLDisabled(resp http.ResponseWriter, req *http.Request) (interface{}, error) { +// checkACLDisabled will return a standard response if ACLs are disabled. This +// returns true if they are disabled and we should not continue. +func (s *HTTPServer) checkACLDisabled(resp http.ResponseWriter, req *http.Request) bool { + if s.agent.config.ACLDatacenter != "" { + return false + } + resp.WriteHeader(http.StatusUnauthorized) fmt.Fprint(resp, "ACL support disabled") - return nil, nil + return true } // ACLBootstrap is used to perform a one-time ACL bootstrap operation on // a cluster to get the first management token. func (s *HTTPServer) ACLBootstrap(resp http.ResponseWriter, req *http.Request) (interface{}, error) { + if s.checkACLDisabled(resp, req) { + return nil, nil + } if req.Method != "PUT" { return nil, MethodNotAllowedError{req.Method, []string{"PUT"}} } @@ -48,6 +56,9 @@ func (s *HTTPServer) ACLBootstrap(resp http.ResponseWriter, req *http.Request) ( } func (s *HTTPServer) ACLDestroy(resp http.ResponseWriter, req *http.Request) (interface{}, error) { + if s.checkACLDisabled(resp, req) { + return nil, nil + } if req.Method != "PUT" { return nil, MethodNotAllowedError{req.Method, []string{"PUT"}} } @@ -74,6 +85,9 @@ func (s *HTTPServer) ACLDestroy(resp http.ResponseWriter, req *http.Request) (in } func (s *HTTPServer) ACLCreate(resp http.ResponseWriter, req *http.Request) (interface{}, error) { + if s.checkACLDisabled(resp, req) { + return nil, nil + } if req.Method != "PUT" { return nil, MethodNotAllowedError{req.Method, []string{"PUT"}} } @@ -81,6 +95,9 @@ func (s *HTTPServer) ACLCreate(resp http.ResponseWriter, req *http.Request) (int } func (s *HTTPServer) ACLUpdate(resp http.ResponseWriter, req *http.Request) (interface{}, error) { + if s.checkACLDisabled(resp, req) { + return nil, nil + } if req.Method != "PUT" { return nil, MethodNotAllowedError{req.Method, []string{"PUT"}} } @@ -129,6 +146,9 @@ func (s *HTTPServer) aclSet(resp http.ResponseWriter, req *http.Request, update } func (s *HTTPServer) ACLClone(resp http.ResponseWriter, req *http.Request) (interface{}, error) { + if s.checkACLDisabled(resp, req) { + return nil, nil + } if req.Method != "PUT" { return nil, MethodNotAllowedError{req.Method, []string{"PUT"}} } @@ -181,6 +201,9 @@ func (s *HTTPServer) ACLClone(resp http.ResponseWriter, req *http.Request) (inte } func (s *HTTPServer) ACLGet(resp http.ResponseWriter, req *http.Request) (interface{}, error) { + if s.checkACLDisabled(resp, req) { + return nil, nil + } if req.Method != "GET" { return nil, MethodNotAllowedError{req.Method, []string{"GET"}} } @@ -215,6 +238,9 @@ func (s *HTTPServer) ACLGet(resp http.ResponseWriter, req *http.Request) (interf } func (s *HTTPServer) ACLList(resp http.ResponseWriter, req *http.Request) (interface{}, error) { + if s.checkACLDisabled(resp, req) { + return nil, nil + } if req.Method != "GET" { return nil, MethodNotAllowedError{req.Method, []string{"GET"}} } @@ -241,6 +267,9 @@ func (s *HTTPServer) ACLList(resp http.ResponseWriter, req *http.Request) (inter } func (s *HTTPServer) ACLReplicationStatus(resp http.ResponseWriter, req *http.Request) (interface{}, error) { + if s.checkACLDisabled(resp, req) { + return nil, nil + } if req.Method != "GET" { return nil, MethodNotAllowedError{req.Method, []string{"GET"}} } diff --git a/agent/acl_endpoint_test.go b/agent/acl_endpoint_test.go index 9aba5154c..aa15c4868 100644 --- a/agent/acl_endpoint_test.go +++ b/agent/acl_endpoint_test.go @@ -3,14 +3,53 @@ package agent import ( "bytes" "encoding/json" + "fmt" "net/http" "net/http/httptest" + "strings" "testing" "github.com/hashicorp/consul/acl" "github.com/hashicorp/consul/agent/structs" ) +func TestACL_Disabled_Response(t *testing.T) { + t.Parallel() + a := NewTestAgent(t.Name(), "") + defer a.Shutdown() + + tests := []func(resp http.ResponseWriter, req *http.Request) (interface{}, error){ + a.srv.ACLBootstrap, + a.srv.ACLDestroy, + a.srv.ACLCreate, + a.srv.ACLUpdate, + a.srv.ACLClone, + a.srv.ACLGet, + a.srv.ACLList, + a.srv.ACLReplicationStatus, + a.srv.AgentToken, // See TestAgent_Token. + } + for i, tt := range tests { + t.Run(fmt.Sprintf("%d", i), func(t *testing.T) { + req, _ := http.NewRequest("PUT", "/should/not/care", nil) + resp := httptest.NewRecorder() + obj, err := tt(resp, req) + if err != nil { + t.Fatalf("err: %v", err) + } + if obj != nil { + t.Fatalf("bad: %#v", obj) + } + if got, want := resp.Code, http.StatusUnauthorized; got != want { + t.Fatalf("got %d want %d", got, want) + } + if !strings.Contains(resp.Body.String(), "ACL support disabled") { + t.Fatalf("bad: %#v", resp) + } + }) + } +} + func makeTestACL(t *testing.T, srv *HTTPServer) string { body := bytes.NewBuffer(nil) enc := json.NewEncoder(body) diff --git a/agent/agent_endpoint.go b/agent/agent_endpoint.go index bf7482723..39b38294d 100644 --- a/agent/agent_endpoint.go +++ b/agent/agent_endpoint.go @@ -809,6 +809,9 @@ func (h *httpLogHandler) HandleLog(log string) { } func (s *HTTPServer) AgentToken(resp http.ResponseWriter, req *http.Request) (interface{}, error) { + if s.checkACLDisabled(resp, req) { + return nil, nil + } if req.Method != "PUT" { return nil, MethodNotAllowedError{req.Method, []string{"PUT"}} } diff --git a/agent/agent_endpoint_test.go b/agent/agent_endpoint_test.go index 3426b3bac..427154f9e 100644 --- a/agent/agent_endpoint_test.go +++ b/agent/agent_endpoint_test.go @@ -1771,6 +1771,10 @@ func TestAgent_Monitor_ACLDeny(t *testing.T) { func TestAgent_Token(t *testing.T) { t.Parallel() + + // The behavior of this handler when ACLs are disabled is vetted over + // in TestACL_Disabled_Response since there's already good infra set + // up over there to test this, and it calls the common function. a := NewTestAgent(t.Name(), TestACLConfig()+` acl_token = "" acl_agent_token = "" diff --git a/agent/http.go b/agent/http.go index a51e2c944..ca3085fd4 100644 --- a/agent/http.go +++ b/agent/http.go @@ -77,27 +77,15 @@ func (s *HTTPServer) handler(enableDebug bool) http.Handler { mux.HandleFunc("/", s.Index) // API V1. - if s.agent.config.ACLDatacenter != "" { - handleFuncMetrics("/v1/acl/bootstrap", s.wrap(s.ACLBootstrap)) - handleFuncMetrics("/v1/acl/create", s.wrap(s.ACLCreate)) - handleFuncMetrics("/v1/acl/update", s.wrap(s.ACLUpdate)) - handleFuncMetrics("/v1/acl/destroy/", s.wrap(s.ACLDestroy)) - handleFuncMetrics("/v1/acl/info/", s.wrap(s.ACLGet)) - handleFuncMetrics("/v1/acl/clone/", s.wrap(s.ACLClone)) - handleFuncMetrics("/v1/acl/list", s.wrap(s.ACLList)) - handleFuncMetrics("/v1/acl/replication", s.wrap(s.ACLReplicationStatus)) - handleFuncMetrics("/v1/agent/token/", s.wrap(s.AgentToken)) - } else { - handleFuncMetrics("/v1/acl/bootstrap", s.wrap(ACLDisabled)) - handleFuncMetrics("/v1/acl/create", s.wrap(ACLDisabled)) - handleFuncMetrics("/v1/acl/update", s.wrap(ACLDisabled)) - handleFuncMetrics("/v1/acl/destroy/", s.wrap(ACLDisabled)) - handleFuncMetrics("/v1/acl/info/", s.wrap(ACLDisabled)) - handleFuncMetrics("/v1/acl/clone/", s.wrap(ACLDisabled)) - handleFuncMetrics("/v1/acl/list", s.wrap(ACLDisabled)) - handleFuncMetrics("/v1/acl/replication", s.wrap(ACLDisabled)) - handleFuncMetrics("/v1/agent/token/", s.wrap(ACLDisabled)) - } + handleFuncMetrics("/v1/acl/bootstrap", s.wrap(s.ACLBootstrap)) + handleFuncMetrics("/v1/acl/create", s.wrap(s.ACLCreate)) + handleFuncMetrics("/v1/acl/update", s.wrap(s.ACLUpdate)) + handleFuncMetrics("/v1/acl/destroy/", s.wrap(s.ACLDestroy)) + handleFuncMetrics("/v1/acl/info/", s.wrap(s.ACLGet)) + handleFuncMetrics("/v1/acl/clone/", s.wrap(s.ACLClone)) + handleFuncMetrics("/v1/acl/list", s.wrap(s.ACLList)) + handleFuncMetrics("/v1/acl/replication", s.wrap(s.ACLReplicationStatus)) + handleFuncMetrics("/v1/agent/token/", s.wrap(s.AgentToken)) handleFuncMetrics("/v1/agent/self", s.wrap(s.AgentSelf)) handleFuncMetrics("/v1/agent/maintenance", s.wrap(s.AgentNodeMaintenance)) handleFuncMetrics("/v1/agent/reload", s.wrap(s.AgentReload))