remove legacy tokens

.changelog/15947.txt

@@ -0,0 +1,3 @@
+```release-note:deprecation
+acl: remove all acl token migration functionality and references to legacy acl tokens.
+```

acl/errors.go

@@ -96,9 +96,7 @@ func (e PermissionDeniedError) Error() string {
 		return message.String()
 	}
 
-	if e.Accessor == "" {
-		message.WriteString(": provided token")
-	} else if e.Accessor == AnonymousTokenID {
+	if e.Accessor == AnonymousTokenID {
 		message.WriteString(": anonymous token")
 	} else {
 		fmt.Fprintf(&message, ": token with AccessorID '%s'", e.Accessor)

acl/errors_test.go

@@ -29,11 +29,11 @@ func TestPermissionDeniedError(t *testing.T) {
 		},
 		{
 			err:      PermissionDeniedByACL(&auth1, nil, ResourceService, AccessRead, "foobar"),
-			expected: "Permission denied: provided token lacks permission 'service:read' on \"foobar\"",
+			expected: "Permission denied: token with AccessorID '' lacks permission 'service:read' on \"foobar\"",
 		},
 		{
 			err:      PermissionDeniedByACLUnnamed(&auth1, nil, ResourceService, AccessRead),
-			expected: "Permission denied: provided token lacks permission 'service:read'",
+			expected: "Permission denied: token with AccessorID '' lacks permission 'service:read'",
 		},
 	}
 

agent/acl_endpoint.go

@@ -428,10 +428,8 @@ func (s *HTTPHandlers) aclTokenSetInternal(req *http.Request, tokenID string, cr
 	}
 
 	if !create {
-		if args.ACLToken.AccessorID != "" && args.ACLToken.AccessorID != tokenID {
+		if args.ACLToken.AccessorID != tokenID {
 			return nil, HTTPError{StatusCode: http.StatusBadRequest, Reason: "Token Accessor ID in URL and payload do not match"}
-		} else if args.ACLToken.AccessorID == "" {
-			args.ACLToken.AccessorID = tokenID
 		}
 	}
 

agent/acl_endpoint_test.go

@@ -837,6 +837,7 @@ func TestACL_HTTP(t *testing.T) {
 
 			// Accessor and Secret will be filled in
 			tokenInput := &structs.ACLToken{
+				AccessorID:  tokenMap[idMap["token-cloned"]].AccessorID,
 				Description: "Better description for this cloned token",
 				Policies: []structs.ACLTokenPolicyLink{
 					{

agent/acl_test.go

@@ -182,8 +182,6 @@ func authzFromPolicy(policy *acl.Policy, cfg *acl.Config) (acl.Authorizer, error
 
 type testToken struct {
 	token structs.ACLToken
-	// yes the rules can exist on the token itself but that is legacy behavior
-	// that I would prefer these tests not rely on
 	rules string
 }
 

agent/consul/acl_endpoint.go

@@ -470,10 +470,6 @@ func (a *ACL) TokenClone(args *structs.ACLTokenSetRequest, reply *structs.ACLTok
 		return fmt.Errorf("Cannot clone a token created from an auth method")
 	}
 
-	if token.Rules != "" {
-		return fmt.Errorf("Cannot clone a legacy ACL with this endpoint")
-	}
-
 	clone := &structs.ACLToken{
 		Policies:          token.Policies,
 		Roles:             token.Roles,

agent/consul/acl_endpoint_test.go

@@ -483,7 +483,6 @@ func TestACLEndpoint_TokenClone(t *testing.T) {
 		require.Equal(t, t1.Roles, t2.Roles)
 		require.Equal(t, t1.ServiceIdentities, t2.ServiceIdentities)
 		require.Equal(t, t1.NodeIdentities, t2.NodeIdentities)
-		require.Equal(t, t1.Rules, t2.Rules)
 		require.Equal(t, t1.Local, t2.Local)
 		require.NotEqual(t, t1.AccessorID, t2.AccessorID)
 		require.NotEqual(t, t1.SecretID, t2.SecretID)

agent/consul/auth/token_writer.go

@@ -306,14 +306,6 @@ func (w *TokenWriter) write(token, existing *structs.ACLToken, fromLogin bool) (
 	}
 	token.NodeIdentities = nodeIdentities
 
-	if token.Rules != "" {
-		return nil, errors.New("Rules cannot be specified for this token")
-	}
-
-	if token.Type != "" {
-		return nil, errors.New("Type cannot be specified for this token")
-	}
-
 	if err := w.enterpriseValidation(token, existing); err != nil {
 		return nil, err
 	}

agent/consul/auth/token_writer_test.go

@@ -98,14 +98,6 @@ func TestTokenWriter_Create_Validation(t *testing.T) {
 			fromLogin:     false,
 			errorContains: "AuthMethod field is disallowed outside of login",
 		},
-		"Rules set": {
-			token:         structs.ACLToken{Rules: "some rules"},
-			errorContains: "Rules cannot be specified for this token",
-		},
-		"Type set": {
-			token:         structs.ACLToken{Type: "some-type"},
-			errorContains: "Type cannot be specified for this token",
-		},
 	}
 	for desc, tc := range testCases {
 		t.Run(desc, func(t *testing.T) {
@@ -498,14 +490,6 @@ func TestTokenWriter_Update_Validation(t *testing.T) {
 			token:         structs.ACLToken{AccessorID: token.AccessorID, ExpirationTime: timePointer(token.ExpirationTime.Add(1 * time.Minute))},
 			errorContains: "Cannot change expiration time",
 		},
-		"Rules set": {
-			token:         structs.ACLToken{AccessorID: token.AccessorID, Rules: "some rules"},
-			errorContains: "Rules cannot be specified for this token",
-		},
-		"Type set": {
-			token:         structs.ACLToken{AccessorID: token.AccessorID, Type: "some-type"},
-			errorContains: "Type cannot be specified for this token",
-		},
 	}
 	for desc, tc := range testCases {
 		t.Run(desc, func(t *testing.T) {

agent/consul/fsm/snapshot_oss.go

@@ -20,8 +20,6 @@ func init() {
 	registerRestorer(structs.KVSRequestType, restoreKV)
 	registerRestorer(structs.TombstoneRequestType, restoreTombstone)
 	registerRestorer(structs.SessionRequestType, restoreSession)
-	registerRestorer(structs.DeprecatedACLRequestType, restoreACL) // TODO(ACL-Legacy-Compat) - remove in phase 2
-	registerRestorer(structs.ACLBootstrapRequestType, restoreACLBootstrap)
 	registerRestorer(structs.CoordinateBatchUpdateType, restoreCoordinates)
 	registerRestorer(structs.PreparedQueryRequestType, restorePreparedQuery)
 	registerRestorer(structs.AutopilotRequestType, restoreAutopilot)
@@ -660,73 +658,6 @@ func restoreSession(header *SnapshotHeader, restore *state.Restore, decoder *cod
 	return nil
 }
 
-// TODO(ACL-Legacy-Compat) - remove in phase 2
-func restoreACL(_ *SnapshotHeader, restore *state.Restore, decoder *codec.Decoder) error {
-	var req LegacyACL
-	if err := decoder.Decode(&req); err != nil {
-		return err
-	}
-
-	if err := restore.ACLToken(req.Convert()); err != nil {
-		return err
-	}
-	return nil
-}
-
-// TODO(ACL-Legacy-Compat) - remove in phase 2
-type LegacyACL struct {
-	ID    string
-	Name  string
-	Type  string
-	Rules string
-
-	structs.RaftIndex
-}
-
-// TODO(ACL-Legacy-Compat): remove in phase 2, used by snapshot restore
-func (a LegacyACL) Convert() *structs.ACLToken {
-	correctedRules := structs.SanitizeLegacyACLTokenRules(a.Rules)
-	if correctedRules != "" {
-		a.Rules = correctedRules
-	}
-
-	token := &structs.ACLToken{
-		AccessorID:        "",
-		SecretID:          a.ID,
-		Description:       a.Name,
-		Policies:          nil,
-		ServiceIdentities: nil,
-		NodeIdentities:    nil,
-		Type:              a.Type,
-		Rules:             a.Rules,
-		Local:             false,
-		RaftIndex:         a.RaftIndex,
-	}
-
-	token.SetHash(true)
-	return token
-}
-
-// TODO(ACL-Legacy-Compat) - remove in phase 2
-func restoreACLBootstrap(_ *SnapshotHeader, restore *state.Restore, decoder *codec.Decoder) error {
-	type ACLBootstrap struct {
-		// AllowBootstrap will only be true if no existing management tokens
-		// have been found.
-		AllowBootstrap bool
-
-		structs.RaftIndex
-	}
-
-	var req ACLBootstrap
-	if err := decoder.Decode(&req); err != nil {
-		return err
-	}
-
-	// With V2 ACLs whether bootstrapping has been performed is stored in the index table like nomad
-	// so this "restores" into that index table.
-	return restore.IndexRestore(&state.IndexEntry{Key: "acl-token-bootstrap", Value: req.ModifyIndex})
-}
-
 func restoreCoordinates(header *SnapshotHeader, restore *state.Restore, decoder *codec.Decoder) error {
 	var req structs.Coordinates
 	if err := decoder.Decode(&req); err != nil {
@@ -819,14 +750,6 @@ func restoreToken(header *SnapshotHeader, restore *state.Restore, decoder *codec
 		return err
 	}
 
-	// DEPRECATED (ACL-Legacy-Compat)
-	if req.Rules != "" {
-		// When we restore a snapshot we may have to correct old HCL in legacy
-		// tokens to prevent the in-memory representation from using an older
-		// syntax.
-		structs.SanitizeLegacyACLToken(&req)
-	}
-
 	// only set if unset - mitigates a bug where converted legacy tokens could end up without a hash
 	req.SetHash(false)
 

agent/consul/fsm/snapshot_test.go

@@ -2,7 +2,6 @@ package fsm
 
 import (
 	"bytes"
-	"fmt"
 	"net"
 	"testing"
 	"time"
@@ -12,7 +11,6 @@ import (
 
 	"github.com/hashicorp/consul-net-rpc/go-msgpack/codec"
 
-	"github.com/hashicorp/consul/acl"
 	"github.com/hashicorp/consul/agent/connect"
 	"github.com/hashicorp/consul/agent/consul/state"
 	"github.com/hashicorp/consul/agent/structs"
@@ -87,7 +85,6 @@ func TestFSM_SnapshotRestore_OSS(t *testing.T) {
 		Name:        "global-management",
 		Description: "Builtin Policy that grants unlimited access",
 		Rules:       structs.ACLPolicyGlobalManagement,
-		Syntax:      acl.SyntaxCurrent,
 	}
 	policy.SetHash(true)
 	require.NoError(t, fsm.state.ACLPolicySet(1, policy))
@@ -116,7 +113,6 @@ func TestFSM_SnapshotRestore_OSS(t *testing.T) {
 		},
 		CreateTime: time.Now(),
 		Local:      false,
-		Type:       "management",
 	}
 	require.NoError(t, fsm.state.ACLBootstrap(10, 0, token))
 
@@ -537,21 +533,6 @@ func TestFSM_SnapshotRestore_OSS(t *testing.T) {
 	// be persisted but that we still need to be able to restore
 	encoder := codec.NewEncoder(sink, structs.MsgpackHandle)
 
-	// Persist a legacy ACL token - this is not done in newer code
-	// but we want to ensure that restoring legacy tokens works as
-	// expected so we must inject one here manually
-	_, err = sink.Write([]byte{byte(structs.DeprecatedACLRequestType)})
-	require.NoError(t, err)
-
-	acl := LegacyACL{
-		ID:        "1057354f-69ef-4487-94ab-aead3c755445",
-		Name:      "test-legacy",
-		Type:      "client",
-		Rules:     `operator = "read"`,
-		RaftIndex: structs.RaftIndex{CreateIndex: 1, ModifyIndex: 2},
-	}
-	require.NoError(t, encoder.Encode(&acl))
-
 	// Persist a ACLToken without a Hash - the state store will
 	// now tack these on but we want to ensure we can restore
 	// tokens without a hash and have the hash be set.
@@ -561,8 +542,13 @@ func TestFSM_SnapshotRestore_OSS(t *testing.T) {
 		Description: "Test No Hash",
 		CreateTime:  time.Now(),
 		Local:       false,
-		Rules:       `operator = "read"`,
-		RaftIndex:   structs.RaftIndex{CreateIndex: 1, ModifyIndex: 2},
+		Policies: []structs.ACLTokenPolicyLink{
+			{
+				Name: "global-management",
+				ID:   structs.ACLPolicyGlobalManagementID,
+			},
+		},
+		RaftIndex: structs.RaftIndex{CreateIndex: 1, ModifyIndex: 2},
 	}
 
 	_, err = sink.Write([]byte{byte(structs.ACLTokenSetRequestType)})
@@ -673,16 +659,6 @@ func TestFSM_SnapshotRestore_OSS(t *testing.T) {
 	// adds the Hash to our local var.
 	require.Equal(t, token, rtoken)
 
-	// Verify legacy ACL is restored
-	_, rtoken, err = fsm2.state.ACLTokenGetBySecret(nil, acl.ID, nil)
-	require.NoError(t, err)
-	require.NotNil(t, rtoken)
-	require.NotEmpty(t, rtoken.Hash)
-
-	restoredACL, err := convertACLTokenToLegacy(rtoken)
-	require.NoError(t, err)
-	require.Equal(t, &acl, restoredACL)
-
 	// Verify ACLToken without hash computes the Hash during restoration
 	_, rtoken, err = fsm2.state.ACLTokenGetByAccessor(nil, token2.AccessorID, nil)
 	require.NoError(t, err)
@@ -884,23 +860,6 @@ func TestFSM_SnapshotRestore_OSS(t *testing.T) {
 	}
 }
 
-// convertACLTokenToLegacy attempts to convert an ACLToken into an legacy ACL.
-// TODO(ACL-Legacy-Compat): remove in phase 2, used by snapshot restore
-func convertACLTokenToLegacy(tok *structs.ACLToken) (*LegacyACL, error) {
-	if tok.Type == "" {
-		return nil, fmt.Errorf("Cannot convert ACLToken into compat token")
-	}
-
-	compat := &LegacyACL{
-		ID:        tok.SecretID,
-		Name:      tok.Description,
-		Type:      tok.Type,
-		Rules:     tok.Rules,
-		RaftIndex: tok.RaftIndex,
-	}
-	return compat, nil
-}
-
 func TestFSM_BadRestore_OSS(t *testing.T) {
 	t.Parallel()
 	// Create an FSM with some state.