http: update legacy ACL endpoints to return an error

Also move a test for the ACLReplicationStatus endpoint into the correct file.

agent/acl_endpoint_legacy.go

@@ -3,201 +3,11 @@ package agent
 import (
 	"fmt"
 	"net/http"
-	"strings"
-
-	"github.com/hashicorp/consul/acl"
-	"github.com/hashicorp/consul/agent/structs"
 )
 
-type aclCreateResponse struct {
+func (s *HTTPHandlers) ACLLegacy(resp http.ResponseWriter, req *http.Request) (interface{}, error) {
-	ID string
+	resp.WriteHeader(http.StatusGone)
-}
+	msg := "Endpoint %v for the legacy ACL system was removed in Consul 1.11."
-
+	fmt.Fprintf(resp, msg, req.URL.Path)
-func (s *HTTPHandlers) ACLDestroy(resp http.ResponseWriter, req *http.Request) (interface{}, error) {
+	return nil, nil
-	if s.checkACLDisabled(resp, req) {
-		return nil, nil
-	}
-
-	args := structs.ACLRequest{
-		Datacenter: s.agent.config.PrimaryDatacenter,
-		Op:         structs.ACLDelete,
-	}
-	s.parseToken(req, &args.Token)
-
-	// Pull out the acl id
-	args.ACL.ID = strings.TrimPrefix(req.URL.Path, "/v1/acl/destroy/")
-	if args.ACL.ID == "" {
-		resp.WriteHeader(http.StatusBadRequest)
-		fmt.Fprint(resp, "Missing ACL")
-		return nil, nil
-	}
-
-	var out string
-	if err := s.agent.RPC("ACL.Apply", &args, &out); err != nil {
-		return nil, err
-	}
-	return true, nil
-}
-
-func (s *HTTPHandlers) ACLCreate(resp http.ResponseWriter, req *http.Request) (interface{}, error) {
-	if s.checkACLDisabled(resp, req) {
-		return nil, nil
-	}
-	return s.aclSet(resp, req, false)
-}
-
-func (s *HTTPHandlers) ACLUpdate(resp http.ResponseWriter, req *http.Request) (interface{}, error) {
-	if s.checkACLDisabled(resp, req) {
-		return nil, nil
-	}
-	return s.aclSet(resp, req, true)
-}
-
-func (s *HTTPHandlers) aclSet(resp http.ResponseWriter, req *http.Request, update bool) (interface{}, error) {
-	args := structs.ACLRequest{
-		Datacenter: s.agent.config.PrimaryDatacenter,
-		Op:         structs.ACLSet,
-		ACL: structs.ACL{
-			Type: structs.ACLTokenTypeClient,
-		},
-	}
-	s.parseToken(req, &args.Token)
-
-	// Handle optional request body
-	if req.ContentLength > 0 {
-		if err := decodeBody(req.Body, &args.ACL); err != nil {
-			resp.WriteHeader(http.StatusBadRequest)
-			fmt.Fprintf(resp, "Request decode failed: %v", err)
-			return nil, nil
-		}
-	}
-
-	// Ensure there is an ID set for update. ID is optional for
-	// create, as one will be generated if not provided.
-	if update && args.ACL.ID == "" {
-		resp.WriteHeader(http.StatusBadRequest)
-		fmt.Fprint(resp, "ACL ID must be set")
-		return nil, nil
-	}
-
-	// Create the acl, get the ID
-	var out string
-	if err := s.agent.RPC("ACL.Apply", &args, &out); err != nil {
-		return nil, err
-	}
-
-	// Format the response as a JSON object
-	return aclCreateResponse{out}, nil
-}
-
-func (s *HTTPHandlers) ACLClone(resp http.ResponseWriter, req *http.Request) (interface{}, error) {
-	if s.checkACLDisabled(resp, req) {
-		return nil, nil
-	}
-
-	args := structs.ACLSpecificRequest{
-		Datacenter: s.agent.config.PrimaryDatacenter,
-	}
-	var dc string
-	if done := s.parse(resp, req, &dc, &args.QueryOptions); done {
-		return nil, nil
-	}
-
-	// Pull out the acl id
-	args.ACL = strings.TrimPrefix(req.URL.Path, "/v1/acl/clone/")
-	if args.ACL == "" {
-		resp.WriteHeader(http.StatusBadRequest)
-		fmt.Fprint(resp, "Missing ACL")
-		return nil, nil
-	}
-
-	var out structs.IndexedACLs
-	defer setMeta(resp, &out.QueryMeta)
-	if err := s.agent.RPC("ACL.Get", &args, &out); err != nil {
-		return nil, err
-	}
-
-	// Bail if the ACL is not found, this could be a 404 or a 403, so
-	// always just return a 403.
-	if len(out.ACLs) == 0 {
-		return nil, acl.ErrPermissionDenied
-	}
-
-	// Create a new ACL
-	createArgs := structs.ACLRequest{
-		Datacenter: args.Datacenter,
-		Op:         structs.ACLSet,
-		ACL:        *out.ACLs[0],
-	}
-	createArgs.ACL.ID = ""
-	createArgs.Token = args.Token
-
-	// Create the acl, get the ID
-	var outID string
-	if err := s.agent.RPC("ACL.Apply", &createArgs, &outID); err != nil {
-		return nil, err
-	}
-
-	// Format the response as a JSON object
-	return aclCreateResponse{outID}, nil
-}
-
-func (s *HTTPHandlers) ACLGet(resp http.ResponseWriter, req *http.Request) (interface{}, error) {
-	if s.checkACLDisabled(resp, req) {
-		return nil, nil
-	}
-
-	args := structs.ACLSpecificRequest{
-		Datacenter: s.agent.config.PrimaryDatacenter,
-	}
-	var dc string
-	if done := s.parse(resp, req, &dc, &args.QueryOptions); done {
-		return nil, nil
-	}
-
-	// Pull out the acl id
-	args.ACL = strings.TrimPrefix(req.URL.Path, "/v1/acl/info/")
-	if args.ACL == "" {
-		resp.WriteHeader(http.StatusBadRequest)
-		fmt.Fprint(resp, "Missing ACL")
-		return nil, nil
-	}
-
-	var out structs.IndexedACLs
-	defer setMeta(resp, &out.QueryMeta)
-	if err := s.agent.RPC("ACL.Get", &args, &out); err != nil {
-		return nil, err
-	}
-
-	// Use empty list instead of nil
-	if out.ACLs == nil {
-		out.ACLs = make(structs.ACLs, 0)
-	}
-	return out.ACLs, nil
-}
-
-func (s *HTTPHandlers) ACLList(resp http.ResponseWriter, req *http.Request) (interface{}, error) {
-	if s.checkACLDisabled(resp, req) {
-		return nil, nil
-	}
-
-	args := structs.DCSpecificRequest{
-		Datacenter: s.agent.config.PrimaryDatacenter,
-	}
-	var dc string
-	if done := s.parse(resp, req, &dc, &args.QueryOptions); done {
-		return nil, nil
-	}
-
-	var out structs.IndexedACLs
-	defer setMeta(resp, &out.QueryMeta)
-	if err := s.agent.RPC("ACL.List", &args, &out); err != nil {
-		return nil, err
-	}
-
-	// Use empty list instead of nil
-	if out.ACLs == nil {
-		out.ACLs = make(structs.ACLs, 0)
-	}
-	return out.ACLs, nil
 }

agent/acl_endpoint_legacy_test.go

@@ -1,20 +1,14 @@
 package agent
 
 import (
-	"bytes"
-	"encoding/json"
-	"fmt"
 	"net/http"
 	"net/http/httptest"
-	"strings"
 	"testing"
 
-	"github.com/hashicorp/consul/acl"
+	"github.com/stretchr/testify/require"
-	"github.com/hashicorp/consul/agent/structs"
-	"github.com/hashicorp/consul/testrpc"
 )
 
-func TestACL_Legacy_Disabled_Response(t *testing.T) {
+func TestHTTPHandlers_ACLLegacy(t *testing.T) {
 	if testing.Short() {
 		t.Skip("too slow for testing.Short")
 	}
@@ -23,306 +17,33 @@ func TestACL_Legacy_Disabled_Response(t *testing.T) {
 	a := NewTestAgent(t, "")
 	defer a.Shutdown()
 
-	tests := []func(resp http.ResponseWriter, req *http.Request) (interface{}, error){
+	type testCase struct {
-		a.srv.ACLDestroy,
+		method string
-		a.srv.ACLCreate,
+		path   string
-		a.srv.ACLUpdate,
-		a.srv.ACLClone,
-		a.srv.ACLGet,
-		a.srv.ACLList,
 	}
-	testrpc.WaitForLeader(t, a.RPC, "dc1")
+
-	for i, tt := range tests {
+	run := func(t *testing.T, tc testCase) {
-		t.Run(fmt.Sprintf("%d", i), func(t *testing.T) {
+		req, err := http.NewRequest(tc.method, tc.path, nil)
-			req, _ := http.NewRequest("PUT", "/should/not/care", nil)
+		require.NoError(t, err)
-			resp := httptest.NewRecorder()
+		resp := httptest.NewRecorder()
-			obj, err := tt(resp, req)
+
-			if err != nil {
+		a.srv.h.ServeHTTP(resp, req)
-				t.Fatalf("err: %v", err)
+		require.Equal(t, resp.Code, http.StatusGone)
-			}
+		require.Contains(t, resp.Body.String(), "the legacy ACL system was removed")
-			if obj != nil {
+	}
-				t.Fatalf("bad: %#v", obj)
+
-			}
+	var testCases = []testCase{
-			if got, want := resp.Code, http.StatusUnauthorized; got != want {
+		{method: http.MethodPut, path: "/v1/acl/create"},
-				t.Fatalf("got %d want %d", got, want)
+		{method: http.MethodPut, path: "/v1/acl/update"},
-			}
+		{method: http.MethodPut, path: "/v1/acl/destroy/ID"},
-			if !strings.Contains(resp.Body.String(), "ACL support disabled") {
+		{method: http.MethodGet, path: "/v1/acl/info/ID"},
-				t.Fatalf("bad: %#v", resp)
+		{method: http.MethodPut, path: "/v1/acl/clone/ID"},
-			}
+		{method: http.MethodGet, path: "/v1/acl/list"},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.method+tc.path, func(t *testing.T) {
+			run(t, tc)
 		})
 	}
 }
-
-func makeTestACL(t *testing.T, srv *HTTPHandlers) string {
-	body := bytes.NewBuffer(nil)
-	enc := json.NewEncoder(body)
-	raw := map[string]interface{}{
-		"Name":  "User Token",
-		"Type":  "client",
-		"Rules": "",
-	}
-	enc.Encode(raw)
-
-	req, _ := http.NewRequest("PUT", "/v1/acl/create?token=root", body)
-	resp := httptest.NewRecorder()
-	obj, err := srv.ACLCreate(resp, req)
-	if err != nil {
-		t.Fatalf("err: %v", err)
-	}
-	aclResp := obj.(aclCreateResponse)
-	return aclResp.ID
-}
-
-func TestACL_Legacy_Update(t *testing.T) {
-	if testing.Short() {
-		t.Skip("too slow for testing.Short")
-	}
-
-	t.Parallel()
-	a := NewTestAgent(t, TestACLConfig())
-	defer a.Shutdown()
-
-	testrpc.WaitForLeader(t, a.RPC, "dc1")
-	id := makeTestACL(t, a.srv)
-
-	body := bytes.NewBuffer(nil)
-	enc := json.NewEncoder(body)
-	raw := map[string]interface{}{
-		"ID":    id,
-		"Name":  "User Token 2",
-		"Type":  "client",
-		"Rules": "",
-	}
-	enc.Encode(raw)
-
-	req, _ := http.NewRequest("PUT", "/v1/acl/update?token=root", body)
-	resp := httptest.NewRecorder()
-	obj, err := a.srv.ACLUpdate(resp, req)
-	if err != nil {
-		t.Fatalf("err: %v", err)
-	}
-	aclResp := obj.(aclCreateResponse)
-	if aclResp.ID != id {
-		t.Fatalf("bad: %v", aclResp)
-	}
-}
-
-func TestACL_Legacy_UpdateUpsert(t *testing.T) {
-	if testing.Short() {
-		t.Skip("too slow for testing.Short")
-	}
-
-	t.Parallel()
-	a := NewTestAgent(t, TestACLConfig())
-	defer a.Shutdown()
-
-	body := bytes.NewBuffer(nil)
-	enc := json.NewEncoder(body)
-	raw := map[string]interface{}{
-		"ID":    "my-old-id",
-		"Name":  "User Token 2",
-		"Type":  "client",
-		"Rules": "",
-	}
-	enc.Encode(raw)
-
-	req, _ := http.NewRequest("PUT", "/v1/acl/update?token=root", body)
-	resp := httptest.NewRecorder()
-
-	testrpc.WaitForLeader(t, a.RPC, "dc1")
-	obj, err := a.srv.ACLUpdate(resp, req)
-	if err != nil {
-		t.Fatalf("err: %v", err)
-	}
-	aclResp := obj.(aclCreateResponse)
-	if aclResp.ID != "my-old-id" {
-		t.Fatalf("bad: %v", aclResp)
-	}
-}
-
-func TestACL_Legacy_Destroy(t *testing.T) {
-	if testing.Short() {
-		t.Skip("too slow for testing.Short")
-	}
-
-	t.Parallel()
-	a := NewTestAgent(t, TestACLConfig())
-	defer a.Shutdown()
-
-	testrpc.WaitForLeader(t, a.RPC, "dc1")
-	id := makeTestACL(t, a.srv)
-	req, _ := http.NewRequest("PUT", "/v1/acl/destroy/"+id+"?token=root", nil)
-	resp := httptest.NewRecorder()
-	obj, err := a.srv.ACLDestroy(resp, req)
-	if err != nil {
-		t.Fatalf("err: %v", err)
-	}
-	if resp, ok := obj.(bool); !ok || !resp {
-		t.Fatalf("should work")
-	}
-
-	req, _ = http.NewRequest("GET", "/v1/acl/info/"+id, nil)
-	resp = httptest.NewRecorder()
-	obj, err = a.srv.ACLGet(resp, req)
-	if err != nil {
-		t.Fatalf("err: %v", err)
-	}
-	respObj, ok := obj.(structs.ACLs)
-	if !ok {
-		t.Fatalf("should work")
-	}
-	if len(respObj) != 0 {
-		t.Fatalf("bad: %v", respObj)
-	}
-}
-
-func TestACL_Legacy_Clone(t *testing.T) {
-	if testing.Short() {
-		t.Skip("too slow for testing.Short")
-	}
-
-	t.Parallel()
-	a := NewTestAgent(t, TestACLConfig())
-	defer a.Shutdown()
-
-	testrpc.WaitForLeader(t, a.RPC, "dc1")
-	id := makeTestACL(t, a.srv)
-
-	req, _ := http.NewRequest("PUT", "/v1/acl/clone/"+id, nil)
-	resp := httptest.NewRecorder()
-	_, err := a.srv.ACLClone(resp, req)
-	if !acl.IsErrPermissionDenied(err) {
-		t.Fatalf("err: %v", err)
-	}
-
-	req, _ = http.NewRequest("PUT", "/v1/acl/clone/"+id+"?token=root", nil)
-	resp = httptest.NewRecorder()
-	obj, err := a.srv.ACLClone(resp, req)
-	if err != nil {
-		t.Fatalf("err: %v", err)
-	}
-	aclResp, ok := obj.(aclCreateResponse)
-	if !ok {
-		t.Fatalf("should work: %#v %#v", obj, resp)
-	}
-	if aclResp.ID == id {
-		t.Fatalf("bad id")
-	}
-
-	req, _ = http.NewRequest("GET", "/v1/acl/info/"+aclResp.ID, nil)
-	resp = httptest.NewRecorder()
-	obj, err = a.srv.ACLGet(resp, req)
-	if err != nil {
-		t.Fatalf("err: %v", err)
-	}
-	respObj, ok := obj.(structs.ACLs)
-	if !ok {
-		t.Fatalf("should work")
-	}
-	if len(respObj) != 1 {
-		t.Fatalf("bad: %v", respObj)
-	}
-}
-
-func TestACL_Legacy_Get(t *testing.T) {
-	if testing.Short() {
-		t.Skip("too slow for testing.Short")
-	}
-
-	t.Parallel()
-	t.Run("wrong id", func(t *testing.T) {
-		a := NewTestAgent(t, TestACLConfig())
-		defer a.Shutdown()
-
-		req, _ := http.NewRequest("GET", "/v1/acl/info/nope", nil)
-		resp := httptest.NewRecorder()
-		testrpc.WaitForLeader(t, a.RPC, "dc1")
-		obj, err := a.srv.ACLGet(resp, req)
-		if err != nil {
-			t.Fatalf("err: %v", err)
-		}
-		respObj, ok := obj.(structs.ACLs)
-		if !ok {
-			t.Fatalf("should work")
-		}
-		if respObj == nil || len(respObj) != 0 {
-			t.Fatalf("bad: %v", respObj)
-		}
-	})
-
-	t.Run("right id", func(t *testing.T) {
-		a := NewTestAgent(t, TestACLConfig())
-		defer a.Shutdown()
-
-		testrpc.WaitForLeader(t, a.RPC, "dc1")
-		id := makeTestACL(t, a.srv)
-
-		req, _ := http.NewRequest("GET", "/v1/acl/info/"+id, nil)
-		resp := httptest.NewRecorder()
-		obj, err := a.srv.ACLGet(resp, req)
-		if err != nil {
-			t.Fatalf("err: %v", err)
-		}
-		respObj, ok := obj.(structs.ACLs)
-		if !ok {
-			t.Fatalf("should work")
-		}
-		if len(respObj) != 1 {
-			t.Fatalf("bad: %v", respObj)
-		}
-	})
-}
-
-func TestACL_Legacy_List(t *testing.T) {
-	if testing.Short() {
-		t.Skip("too slow for testing.Short")
-	}
-
-	t.Parallel()
-	a := NewTestAgent(t, TestACLConfig())
-	defer a.Shutdown()
-
-	testrpc.WaitForLeader(t, a.RPC, "dc1")
-	for i := 0; i < 10; i++ {
-		makeTestACL(t, a.srv)
-	}
-
-	req, _ := http.NewRequest("GET", "/v1/acl/list?token=root", nil)
-	resp := httptest.NewRecorder()
-	obj, err := a.srv.ACLList(resp, req)
-	if err != nil {
-		t.Fatalf("err: %v", err)
-	}
-	respObj, ok := obj.(structs.ACLs)
-	if !ok {
-		t.Fatalf("should work")
-	}
-
-	// 10  + master
-	// anonymous token is a new token and wont show up in this list
-	if len(respObj) != 11 {
-		t.Fatalf("bad: %v", respObj)
-	}
-}
-
-func TestACLReplicationStatus(t *testing.T) {
-	if testing.Short() {
-		t.Skip("too slow for testing.Short")
-	}
-
-	t.Parallel()
-	a := NewTestAgent(t, TestACLConfig())
-	defer a.Shutdown()
-
-	req, _ := http.NewRequest("GET", "/v1/acl/replication", nil)
-	resp := httptest.NewRecorder()
-	testrpc.WaitForLeader(t, a.RPC, "dc1")
-	obj, err := a.srv.ACLReplicationStatus(resp, req)
-	if err != nil {
-		t.Fatalf("err: %v", err)
-	}
-	_, ok := obj.(structs.ACLReplicationStatus)
-	if !ok {
-		t.Fatalf("should work")
-	}
-}

agent/acl_endpoint_test.go

@@ -10,6 +10,10 @@ import (
 	"testing"
 	"time"
 
+	"github.com/hashicorp/go-uuid"
+	"github.com/stretchr/testify/require"
+	"gopkg.in/square/go-jose.v2/jwt"
+
 	"github.com/hashicorp/consul/acl"
 	"github.com/hashicorp/consul/agent/consul/authmethod/testauth"
 	"github.com/hashicorp/consul/agent/structs"
@@ -17,9 +21,6 @@ import (
 	"github.com/hashicorp/consul/sdk/freeport"
 	"github.com/hashicorp/consul/sdk/testutil"
 	"github.com/hashicorp/consul/testrpc"
-	"github.com/hashicorp/go-uuid"
-	"github.com/stretchr/testify/require"
-	"gopkg.in/square/go-jose.v2/jwt"
 )
 
 // NOTE: The tests contained herein are designed to test the HTTP API
@@ -2320,3 +2321,25 @@ func startSSOTestServer(t *testing.T) *oidcauthtest.Server {
 		func() { freeport.Return(ports) },
 	))
 }
+
+func TestHTTPHandlers_ACLReplicationStatus(t *testing.T) {
+	if testing.Short() {
+		t.Skip("too slow for testing.Short")
+	}
+
+	t.Parallel()
+	a := NewTestAgent(t, TestACLConfig())
+	defer a.Shutdown()
+
+	req, _ := http.NewRequest("GET", "/v1/acl/replication", nil)
+	resp := httptest.NewRecorder()
+	testrpc.WaitForLeader(t, a.RPC, "dc1")
+	obj, err := a.srv.ACLReplicationStatus(resp, req)
+	if err != nil {
+		t.Fatalf("err: %v", err)
+	}
+	_, ok := obj.(structs.ACLReplicationStatus)
+	if !ok {
+		t.Fatalf("should work")
+	}
+}

agent/http_register.go

@@ -120,10 +120,10 @@ func init() {
 	registerEndpoint("/v1/txn", []string{"PUT"}, (*HTTPHandlers).Txn)
 
 	// Deprecated ACL endpoints, they do nothing but return an error
-	registerEndpoint("/v1/acl/create", []string{"PUT"}, (*HTTPHandlers).ACLCreate)
+	registerEndpoint("/v1/acl/create", []string{"PUT"}, (*HTTPHandlers).ACLLegacy)
-	registerEndpoint("/v1/acl/update", []string{"PUT"}, (*HTTPHandlers).ACLUpdate)
+	registerEndpoint("/v1/acl/update", []string{"PUT"}, (*HTTPHandlers).ACLLegacy)
-	registerEndpoint("/v1/acl/destroy/", []string{"PUT"}, (*HTTPHandlers).ACLDestroy)
+	registerEndpoint("/v1/acl/destroy/", []string{"PUT"}, (*HTTPHandlers).ACLLegacy)
-	registerEndpoint("/v1/acl/info/", []string{"GET"}, (*HTTPHandlers).ACLGet)
+	registerEndpoint("/v1/acl/info/", []string{"GET"}, (*HTTPHandlers).ACLLegacy)
-	registerEndpoint("/v1/acl/clone/", []string{"PUT"}, (*HTTPHandlers).ACLClone)
+	registerEndpoint("/v1/acl/clone/", []string{"PUT"}, (*HTTPHandlers).ACLLegacy)
-	registerEndpoint("/v1/acl/list", []string{"GET"}, (*HTTPHandlers).ACLList)
+	registerEndpoint("/v1/acl/list", []string{"GET"}, (*HTTPHandlers).ACLLegacy)
 }