Add controller-led worker auth flow (#2413)

This adds an action to the API that allows pulling out an activation
token that can be given to a worker in its config (directly or via env
or file). The worker can pass this along with its normal request in
place of its normal nonce and the server will automatically accept it.

Co-authored-by: Irena Rindos <irenarindos@users.noreply.github.com>

internal/daemon/controller/handlers/scopes/scope_service_test.go

@@ -97,6 +97,7 @@ var globalAuthorizedCollectionActions = map[string]*structpb.ListValue{
 	},
 	"workers": {
 		Values: []*structpb.Value{
+			structpb.NewStringValue("create:controller-led"),
 			structpb.NewStringValue("create:worker-led"),
 			structpb.NewStringValue("list"),
 		},

internal/daemon/controller/handlers/targets/target_service_test.go

@@ -1,10 +1,13 @@
 package targets
 
 import (
+	"context"
+	"crypto/rand"
 	"fmt"
 	"testing"
 
 	"github.com/hashicorp/boundary/internal/db"
+	"github.com/hashicorp/boundary/internal/kms"
 	"github.com/hashicorp/boundary/internal/server"
 	"github.com/hashicorp/boundary/internal/types/scope"
 	pb "github.com/hashicorp/boundary/sdk/pbs/controller/api/resources/targets"
@@ -35,6 +38,8 @@ func TestWorkerList_Addresses(t *testing.T) {
 func TestWorkerList_Filter(t *testing.T) {
 	conn, _ := db.TestSetup(t, "postgres")
 	wrapper := db.TestWrapper(t)
+	kmsCache := kms.TestKms(t, conn, wrapper)
+	require.NoError(t, kmsCache.CreateKeys(context.Background(), scope.Global.String(), kms.WithRandomReader(rand.Reader)))
 	var workers []*server.Worker
 	for i := 0; i < 5; i++ {
 		switch {

internal/daemon/controller/handlers/workers/worker_service.go

@@ -19,6 +19,7 @@ import (
 	"github.com/hashicorp/boundary/internal/types/action"
 	"github.com/hashicorp/boundary/internal/types/resource"
 	"github.com/hashicorp/boundary/internal/types/scope"
+	"github.com/hashicorp/boundary/internal/util"
 	pb "github.com/hashicorp/boundary/sdk/pbs/controller/api/resources/workers"
 	"github.com/hashicorp/go-secure-stdlib/strutil"
 	"github.com/hashicorp/nodeenrollment/types"
@@ -52,6 +53,7 @@ var (
 	// CollectionActions contains the set of actions that can be performed on
 	// this collection
 	CollectionActions = action.ActionSet{
+		action.CreateControllerLed,
 		action.CreateWorkerLed,
 		action.List,
 	}
@@ -199,18 +201,20 @@ func (s Service) GetWorker(ctx context.Context, req *pbs.GetWorkerRequest) (*pbs
 	return &pbs.GetWorkerResponse{Item: item}, nil
 }
 
-// CreateWorker implements the interface pbs.WorkerServiceServer.
+// CreateWorkerLed implements the interface pbs.WorkerServiceServer and handles
+// a request to create a new worker and consume a worker-generated authorization
+// request
 func (s Service) CreateWorkerLed(ctx context.Context, req *pbs.CreateWorkerLedRequest) (*pbs.CreateWorkerLedResponse, error) {
 	const op = "workers.(Service).CreateWorkerLed"
 
-	if err := validateCreateRequest(req); err != nil {
+	act := action.CreateWorkerLed
+	item := req.GetItem()
+
+	if err := validateCreateRequest(item, act); err != nil {
 		return nil, err
 	}
-	authResults := s.authResult(ctx, req.GetItem().GetScopeId(), action.CreateWorkerLed)
-	if authResults.Error != nil {
-		return nil, authResults.Error
-	}
-	reqBytes, err := base58.FastBase58Decoding(req.GetItem().WorkerGeneratedAuthToken.GetValue())
+
+	reqBytes, err := base58.FastBase58Decoding(item.WorkerGeneratedAuthToken.GetValue())
 	if err != nil {
 		return nil, fmt.Errorf("%s: error decoding node_credentials_token: %w", op, err)
 	}
@@ -219,7 +223,43 @@ func (s Service) CreateWorkerLed(ctx context.Context, req *pbs.CreateWorkerLedRe
 	if err := proto.Unmarshal(reqBytes, creds); err != nil {
 		return nil, fmt.Errorf("%s: error unmarshaling node_credentials_token: %w", op, err)
 	}
-	created, err := s.createInRepo(ctx, req.GetItem(), creds)
+
+	out, err := s.createCommon(ctx, item, act, server.WithFetchNodeCredentialsRequest(creds))
+	if err != nil {
+		return nil, errors.Wrap(ctx, err, op)
+	}
+
+	return &pbs.CreateWorkerLedResponse{Item: out}, nil
+}
+
+// CreateControllerLed implements the interface pbs.WorkerServiceServer and handles
+// a request to create a new worker, generating and returning an activation
+// token
+func (s Service) CreateControllerLed(ctx context.Context, req *pbs.CreateControllerLedRequest) (*pbs.CreateControllerLedResponse, error) {
+	const op = "workers.(Service).CreateControllerLed"
+	act := action.CreateControllerLed
+
+	if err := validateCreateRequest(req.GetItem(), act); err != nil {
+		return nil, err
+	}
+
+	out, err := s.createCommon(ctx, req.GetItem(), act, server.WithCreateControllerLedActivationToken(true))
+	if err != nil {
+		return nil, errors.Wrap(ctx, err, op)
+	}
+
+	return &pbs.CreateControllerLedResponse{Item: out}, nil
+}
+
+func (s Service) createCommon(ctx context.Context, in *pb.Worker, act action.Type, opt ...server.Option) (*pb.Worker, error) {
+	const op = "workers.(Service).createCommon"
+
+	authResults := s.authResult(ctx, in.GetScopeId(), act)
+	if authResults.Error != nil {
+		return nil, authResults.Error
+	}
+
+	created, err := s.createInRepo(ctx, in, opt...)
 	if err != nil {
 		return nil, fmt.Errorf("%s: error creating worker: %w", op, err)
 	}
@@ -243,7 +283,7 @@ func (s Service) CreateWorkerLed(ctx context.Context, req *pbs.CreateWorkerLedRe
 		return nil, err
 	}
 
-	return &pbs.CreateWorkerLedResponse{Item: item}, nil
+	return item, nil
 }
 
 // DeleteWorker implements the interface pbs.WorkerServiceServer.
@@ -438,7 +478,7 @@ func (s Service) getFromRepo(ctx context.Context, id string) (*server.Worker, er
 	return w, nil
 }
 
-func (s Service) createInRepo(ctx context.Context, worker *pb.Worker, creds *types.FetchNodeCredentialsRequest) (*server.Worker, error) {
+func (s Service) createInRepo(ctx context.Context, worker *pb.Worker, opt ...server.Option) (*server.Worker, error) {
 	const op = "workers.(Service).createInRepo"
 	repo, err := s.repoFn()
 	if err != nil {
@@ -449,7 +489,7 @@ func (s Service) createInRepo(ctx context.Context, worker *pb.Worker, creds *typ
 		server.WithName(worker.GetName().GetValue()),
 		server.WithDescription(worker.GetDescription().GetValue()),
 	)
-	retWorker, err := repo.CreateWorker(ctx, newWorker, server.WithFetchNodeCredentialsRequest(creds))
+	retWorker, err := repo.CreateWorker(ctx, newWorker, opt...)
 	if err != nil {
 		return nil, errors.Wrap(ctx, err, op, errors.WithMsg("unable to create worker"))
 	}
@@ -593,7 +633,7 @@ func (s Service) authResult(ctx context.Context, id string, a action.Type) auth.
 	var parentId string
 	opts := []auth.Option{auth.WithType(resource.Worker), auth.WithAction(a)}
 	switch a {
-	case action.List, action.CreateWorkerLed:
+	case action.List, action.CreateWorkerLed, action.CreateControllerLed:
 		parentId = id
 	default:
 		w, err := repo.LookupWorker(ctx, id)
@@ -670,6 +710,9 @@ func toProto(ctx context.Context, in *server.Worker, opt ...handlers.Option) (*p
 	if outputFields.Has(globals.ActiveConnectionCountField) {
 		out.ActiveConnectionCount = &wrapperspb.UInt32Value{Value: in.ActiveConnectionCount()}
 	}
+	if outputFields.Has(globals.ControllerGeneratedActivationToken) && in.ControllerGeneratedActivationToken != "" {
+		out.ControllerGeneratedActivationToken = &wrapperspb.StringValue{Value: in.ControllerGeneratedActivationToken}
+	}
 	if outputFields.Has(globals.ConfigTagsField) && len(in.GetConfigTags()) > 0 {
 		var err error
 		out.ConfigTags, err = tagsToMapProto(in.GetConfigTags())
@@ -769,45 +812,57 @@ func validateUpdateRequest(req *pbs.UpdateWorkerRequest) error {
 	}, server.WorkerPrefix)
 }
 
-func validateCreateRequest(req *pbs.CreateWorkerLedRequest) error {
-	return handlers.ValidateCreateRequest(req.GetItem(), func() map[string]string {
+func validateCreateRequest(item *pb.Worker, act action.Type) error {
+	if util.IsNil(item) {
+		return handlers.InvalidArgumentErrorf("Request item is nil", nil)
+	}
+	switch act {
+	case action.CreateWorkerLed:
+	case action.CreateControllerLed:
+	default:
+		// This shouldn't happen because we shouldn't be routed to one of the
+		// handlers if it's the wrong action, but check anyways.
+		return handlers.InvalidArgumentErrorf("Invalid action", nil)
+	}
+	return handlers.ValidateCreateRequest(item, func() map[string]string {
 		const (
 			mustBeGlobalMsg  = "Must be 'global'"
 			cannotBeEmptyMsg = "Cannot be empty."
 			readOnlyFieldMsg = "This is a read only field."
 		)
 		badFields := map[string]string{}
-		if scope.Global.String() != req.GetItem().GetScopeId() {
+		if scope.Global.String() != item.GetScopeId() {
 			badFields[globals.ScopeIdField] = mustBeGlobalMsg
 		}
-		// FIXME: in the future, we won't require this token since we'll support
-		// the server led flow where a token is returned when a worker is created.
-		if req.GetItem().WorkerGeneratedAuthToken == nil {
+		switch {
+		case act == action.CreateWorkerLed && item.WorkerGeneratedAuthToken == nil:
 			badFields[globals.WorkerGeneratedAuthTokenField] = cannotBeEmptyMsg
+		case act == action.CreateControllerLed && item.WorkerGeneratedAuthToken != nil:
+			badFields[globals.WorkerGeneratedAuthTokenField] = "Worker-generated auth tokens are not used with the controller-led creation flow."
 		}
-		if req.GetItem().Address != "" {
+		if item.Address != "" {
 			badFields[globals.CanonicalAddressField] = readOnlyFieldMsg
 		}
-		if req.GetItem().CanonicalTags != nil {
+		if item.CanonicalTags != nil {
 			badFields[globals.CanonicalTagsField] = readOnlyFieldMsg
 		}
-		if req.GetItem().ConfigTags != nil {
+		if item.ConfigTags != nil {
 			badFields[globals.ConfigTagsField] = readOnlyFieldMsg
 		}
-		if req.GetItem().LastStatusTime != nil {
+		if item.LastStatusTime != nil {
 			badFields[globals.LastStatusTimeField] = readOnlyFieldMsg
 		}
-		if req.GetItem().AuthorizedActions != nil {
+		if item.AuthorizedActions != nil {
 			badFields[globals.AuthorizedActionsField] = readOnlyFieldMsg
 		}
-		nameString := req.GetItem().GetName().String()
+		nameString := item.GetName().String()
 		if !strutil.Printable(nameString) {
 			badFields[globals.NameField] = "Name contains non-printable characters."
 		}
 		if strings.ToLower(nameString) != nameString {
 			badFields[globals.NameField] = "Name must be all lowercase."
 		}
-		descriptionString := req.GetItem().GetDescription().String()
+		descriptionString := item.GetDescription().String()
 		if !strutil.Printable(descriptionString) {
 			badFields[globals.DescriptionField] = "Description contains non-printable characters."
 		}

internal/daemon/controller/handlers/workers/worker_service_test.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"fmt"
 	"sort"
+	"strings"
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
@@ -22,6 +23,7 @@ import (
 	"github.com/hashicorp/boundary/sdk/pbs/controller/api/resources/workers"
 	pb "github.com/hashicorp/boundary/sdk/pbs/controller/api/resources/workers"
 	"github.com/hashicorp/go-secure-stdlib/strutil"
+	"github.com/hashicorp/nodeenrollment"
 	"github.com/hashicorp/nodeenrollment/rotation"
 	"github.com/hashicorp/nodeenrollment/storage/file"
 	"github.com/hashicorp/nodeenrollment/types"
@@ -1316,6 +1318,350 @@ func TestCreateWorkerLed(t *testing.T) {
 	}
 }
 
+func TestCreateControllerLed(t *testing.T) {
+	t.Parallel()
+	conn, _ := db.TestSetup(t, "postgres")
+	testRootWrapper := db.TestWrapper(t)
+	iamRepo := iam.TestRepo(t, conn, testRootWrapper)
+	iamRepoFn := func() (*iam.Repository, error) {
+		return iamRepo, nil
+	}
+	rw := db.New(conn)
+	testKms := kms.TestKms(t, conn, testRootWrapper)
+	repoFn := func() (*server.Repository, error) {
+		return server.NewRepository(rw, rw, testKms)
+	}
+	testCtx := context.Background()
+
+	testSrv, err := NewService(testCtx, repoFn, iamRepoFn)
+	require.NoError(t, err, "Error when getting new worker service.")
+
+	// Get an initial set of authorized node credentials
+	rootStorage, err := server.NewRepositoryStorage(testCtx, rw, rw, testKms)
+	require.NoError(t, err)
+	_, err = rotation.RotateRootCertificates(testCtx, rootStorage)
+	require.NoError(t, err)
+
+	fetchReqFn := func() string {
+		// This happens on the worker
+		fileStorage, err := file.New(testCtx)
+		require.NoError(t, err)
+		defer fileStorage.Cleanup()
+
+		nodeCreds, err := types.NewNodeCredentials(testCtx, fileStorage)
+		require.NoError(t, err)
+		// Create request using worker id
+		fetchReq, err := nodeCreds.CreateFetchNodeCredentialsRequest(testCtx)
+		require.NoError(t, err)
+
+		fetchEncoded, err := proto.Marshal(fetchReq)
+		require.NoError(t, err)
+
+		return base58.Encode(fetchEncoded)
+	}
+
+	tests := []struct {
+		name            string
+		scopeId         string
+		service         Service
+		req             *pbs.CreateControllerLedRequest
+		res             *pbs.CreateControllerLedResponse
+		wantErr         bool
+		wantErrIs       error
+		wantErrContains string
+	}{
+		{
+			name:    "invalid-scope",
+			service: testSrv,
+			scopeId: scope.Global.String(),
+			req: &pbs.CreateControllerLedRequest{
+				Item: &workers.Worker{
+					ScopeId: "invalid-scope",
+				},
+			},
+			wantErr:         true,
+			wantErrIs:       handlers.ApiErrorWithCode(codes.InvalidArgument),
+			wantErrContains: "Must be 'global'",
+		},
+		{
+			name:    "supplied-node-auth-request",
+			service: testSrv,
+			scopeId: scope.Global.String(),
+			req: &pbs.CreateControllerLedRequest{
+				Item: &workers.Worker{
+					ScopeId:                  scope.Global.String(),
+					WorkerGeneratedAuthToken: &wrapperspb.StringValue{Value: fetchReqFn()},
+				},
+			},
+			wantErr:         true,
+			wantErrIs:       handlers.ApiErrorWithCode(codes.InvalidArgument),
+			wantErrContains: globals.WorkerGeneratedAuthTokenField,
+		},
+		{
+			name:    "invalid-id",
+			service: testSrv,
+			scopeId: scope.Global.String(),
+			req: &pbs.CreateControllerLedRequest{
+				Item: &workers.Worker{
+					ScopeId: scope.Global.String(),
+					Id:      "invalid-id",
+				},
+			},
+			wantErr:         true,
+			wantErrIs:       handlers.ApiErrorWithCode(codes.InvalidArgument),
+			wantErrContains: globals.IdField,
+		},
+		{
+			name:    "invalid-address",
+			service: testSrv,
+			scopeId: scope.Global.String(),
+			req: &pbs.CreateControllerLedRequest{
+				Item: &workers.Worker{
+					ScopeId: scope.Global.String(),
+					Address: "invalid-address",
+				},
+			},
+			wantErr:         true,
+			wantErrIs:       handlers.ApiErrorWithCode(codes.InvalidArgument),
+			wantErrContains: globals.AddressField,
+		},
+		{
+			name:    "invalid-config-tags",
+			service: testSrv,
+			scopeId: scope.Global.String(),
+			req: &pbs.CreateControllerLedRequest{
+				Item: &workers.Worker{
+					ScopeId: scope.Global.String(),
+					ConfigTags: map[string]*structpb.ListValue{
+						"invalid": {Values: []*structpb.Value{
+							structpb.NewStringValue("invalid-tags"),
+						}},
+					},
+				},
+			},
+			wantErr:         true,
+			wantErrIs:       handlers.ApiErrorWithCode(codes.InvalidArgument),
+			wantErrContains: globals.ConfigTagsField,
+		},
+		{
+			name:    "invalid-canonical-tags",
+			service: testSrv,
+			scopeId: scope.Global.String(),
+			req: &pbs.CreateControllerLedRequest{
+				Item: &workers.Worker{
+					ScopeId: scope.Global.String(),
+					CanonicalTags: map[string]*structpb.ListValue{
+						"invalid": {Values: []*structpb.Value{
+							structpb.NewStringValue("invalid-canonical-tags"),
+						}},
+					},
+				},
+			},
+			wantErr:         true,
+			wantErrIs:       handlers.ApiErrorWithCode(codes.InvalidArgument),
+			wantErrContains: globals.CanonicalTagsField,
+		},
+		{
+			name:    "invalid-last-status-time",
+			service: testSrv,
+			scopeId: scope.Global.String(),
+			req: &pbs.CreateControllerLedRequest{
+				Item: &workers.Worker{
+					ScopeId:        scope.Global.String(),
+					LastStatusTime: timestamppb.Now(),
+				},
+			},
+			wantErr:         true,
+			wantErrIs:       handlers.ApiErrorWithCode(codes.InvalidArgument),
+			wantErrContains: globals.LastStatusTimeField,
+		},
+		{
+			name:    "invalid-authorized-actions",
+			service: testSrv,
+			scopeId: scope.Global.String(),
+			req: &pbs.CreateControllerLedRequest{
+				Item: &workers.Worker{
+					ScopeId:           scope.Global.String(),
+					AuthorizedActions: []string{"invalid-authorized-actions"},
+				},
+			},
+			wantErr:         true,
+			wantErrIs:       handlers.ApiErrorWithCode(codes.InvalidArgument),
+			wantErrContains: globals.AuthorizedActionsField,
+		},
+		{
+			name:    "invalid-create-time",
+			service: testSrv,
+			scopeId: scope.Global.String(),
+			req: &pbs.CreateControllerLedRequest{
+				Item: &workers.Worker{
+					ScopeId:     scope.Global.String(),
+					CreatedTime: timestamppb.Now(),
+				},
+			},
+			wantErr:         true,
+			wantErrIs:       handlers.ApiErrorWithCode(codes.InvalidArgument),
+			wantErrContains: globals.CreatedTimeField,
+		},
+		{
+			name:    "invalid-update-time",
+			service: testSrv,
+			scopeId: scope.Global.String(),
+			req: &pbs.CreateControllerLedRequest{
+				Item: &workers.Worker{
+					ScopeId:     scope.Global.String(),
+					UpdatedTime: timestamppb.Now(),
+				},
+			},
+			wantErr:         true,
+			wantErrIs:       handlers.ApiErrorWithCode(codes.InvalidArgument),
+			wantErrContains: globals.UpdatedTimeField,
+		},
+		{
+			name:    "invalid-version",
+			service: testSrv,
+			scopeId: scope.Global.String(),
+			req: &pbs.CreateControllerLedRequest{
+				Item: &workers.Worker{
+					ScopeId: scope.Global.String(),
+					Version: 1,
+				},
+			},
+			wantErr:         true,
+			wantErrIs:       handlers.ApiErrorWithCode(codes.InvalidArgument),
+			wantErrContains: globals.VersionField,
+		},
+		{
+			name:    "invalid-auth",
+			service: testSrv,
+			scopeId: "splat-auth-scope",
+			req: &pbs.CreateControllerLedRequest{
+				Item: &workers.Worker{
+					ScopeId: scope.Global.String(),
+				},
+			},
+			wantErr:         true,
+			wantErrContains: "non-existent scope \"splat-auth-scope\"",
+		},
+		{
+			name: "create-error",
+			service: func() Service {
+				repoFn := func() (*server.Repository, error) {
+					return server.NewRepository(rw, &db.Db{}, testKms)
+				}
+				testSrv, err := NewService(testCtx, repoFn, iamRepoFn)
+				require.NoError(t, err, "Error when getting new worker service.")
+				return testSrv
+			}(),
+			scopeId: scope.Global.String(),
+			req: &pbs.CreateControllerLedRequest{
+				Item: &workers.Worker{
+					ScopeId:     scope.Global.String(),
+					Name:        &wrapperspb.StringValue{Value: "success"},
+					Description: &wrapperspb.StringValue{Value: "success-description"},
+				},
+			},
+			wantErr:         true,
+			wantErrContains: "error creating worker",
+		},
+		{
+			name: "bad-repo-function-in-create",
+			service: func() Service {
+				// var cnt gives us a way for the repoFn to fail the 2nd time
+				// it's called so we can get past the AuthRequest check in
+				// CreateWorker(...) and test the createInRepo(...) failure
+				// path if the repoFn returns and err.
+				cnt := 0
+				repoFn := func() (*server.Repository, error) {
+					cnt = cnt + 1
+					switch {
+					case cnt > 1:
+						return nil, errors.New(testCtx, errors.Internal, "bad-repo-function", "error creating repo")
+					default:
+						return server.NewRepository(rw, rw, testKms)
+					}
+				}
+				testSrv, err := NewService(testCtx, repoFn, iamRepoFn)
+				require.NoError(t, err, "Error when getting new worker service.")
+				return testSrv
+			}(),
+			scopeId: scope.Global.String(),
+			req: &pbs.CreateControllerLedRequest{
+				Item: &workers.Worker{
+					ScopeId:     scope.Global.String(),
+					Name:        &wrapperspb.StringValue{Value: "success"},
+					Description: &wrapperspb.StringValue{Value: "success-description"},
+				},
+			},
+			wantErr:         true,
+			wantErrContains: "error creating worker: bad-repo-function: error creating repo",
+		},
+		{
+			name:    "success",
+			service: testSrv,
+			scopeId: scope.Global.String(),
+			req: &pbs.CreateControllerLedRequest{
+				Item: &workers.Worker{
+					ScopeId:     scope.Global.String(),
+					Name:        &wrapperspb.StringValue{Value: "success"},
+					Description: &wrapperspb.StringValue{Value: "success-description"},
+				},
+			},
+			res: &pbs.CreateControllerLedResponse{
+				Item: &workers.Worker{
+					ScopeId:               scope.Global.String(),
+					Name:                  &wrapperspb.StringValue{Value: "success"},
+					Description:           &wrapperspb.StringValue{Value: "success-description"},
+					ActiveConnectionCount: &wrapperspb.UInt32Value{Value: 0},
+					Version:               1,
+					Type:                  PkiWorkerType,
+				},
+			},
+		},
+	}
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			assert, require := assert.New(t), require.New(t)
+			got, err := tc.service.CreateControllerLed(auth.DisabledAuthTestContext(iamRepoFn, tc.scopeId), tc.req)
+			if tc.wantErr {
+				require.Error(err)
+				assert.Nil(got)
+				if tc.wantErrIs != nil {
+					assert.ErrorIs(err, tc.wantErrIs)
+				}
+				if tc.wantErrContains != "" {
+					assert.Contains(err.Error(), tc.wantErrContains)
+				}
+				return
+			}
+			require.NoError(err)
+			require.NotNil(got)
+			{
+				// we only need to check that these "read-only" fields were set
+				// outbound. Other portions of the domain is responsible for
+				// testing that these values are correct and there's no reason
+				// to repeat those tests here.
+				assert.NotEmpty(got.GetItem().GetId())
+				assert.NotEmpty(got.GetItem().CreatedTime)
+				assert.NotEmpty(got.GetItem().UpdatedTime)
+				assert.NotEmpty(got.GetItem().Scope)
+				assert.NotEmpty(got.GetItem().AuthorizedActions)
+				assert.NotEmpty(got.GetItem().ControllerGeneratedActivationToken)
+				assert.True(strings.HasPrefix(got.GetItem().ControllerGeneratedActivationToken.GetValue(), nodeenrollment.ServerLedActivationTokenPrefix))
+				{
+					tc.res.Item.Id = got.GetItem().GetId()
+					tc.res.Item.CreatedTime = got.GetItem().GetCreatedTime()
+					tc.res.Item.UpdatedTime = got.GetItem().GetUpdatedTime()
+					tc.res.Item.Scope = got.GetItem().Scope
+					tc.res.Item.AuthorizedActions = got.GetItem().GetAuthorizedActions()
+					tc.res.Item.ControllerGeneratedActivationToken = got.GetItem().GetControllerGeneratedActivationToken()
+				}
+			}
+			assert.Equal(tc.res, got)
+		})
+	}
+}
+
 func TestService_AddWorkerTags(t *testing.T) {
 	t.Parallel()
 	assert, require := assert.New(t), require.New(t)
@@ -1662,8 +2008,9 @@ func TestService_RemoveWorkerTags(t *testing.T) {
 			name: "remove-valid-tag",
 			req: func() *pbs.RemoveWorkerTagsRequest {
 				worker := server.TestKmsWorker(t, conn, wrapper)
-				s.addTagsInRepo(auth.DisabledAuthTestContext(iamRepoFn, proj.GetPublicId()), worker.PublicId,
+				_, err := s.addTagsInRepo(auth.DisabledAuthTestContext(iamRepoFn, proj.GetPublicId()), worker.PublicId,
 					worker.Version, map[string]*structpb.ListValue{"key": {Values: []*structpb.Value{structpb.NewStringValue("value")}}})
+				require.NoError(err)
 				return &pbs.RemoveWorkerTagsRequest{
 					Id:      worker.PublicId,
 					Version: worker.Version + 1,
@@ -1676,7 +2023,7 @@ func TestService_RemoveWorkerTags(t *testing.T) {
 			name: "remove-many-valid-tags",
 			req: func() *pbs.RemoveWorkerTagsRequest {
 				worker := server.TestKmsWorker(t, conn, wrapper)
-				s.addTagsInRepo(auth.DisabledAuthTestContext(iamRepoFn, proj.GetPublicId()), worker.PublicId,
+				_, err := s.addTagsInRepo(auth.DisabledAuthTestContext(iamRepoFn, proj.GetPublicId()), worker.PublicId,
 					worker.Version, map[string]*structpb.ListValue{
 						"key":  {Values: []*structpb.Value{structpb.NewStringValue("value"), structpb.NewStringValue("value1")}},
 						"key2": {Values: []*structpb.Value{structpb.NewStringValue("value2")}},
@@ -1685,6 +2032,7 @@ func TestService_RemoveWorkerTags(t *testing.T) {
 						"key5": {Values: []*structpb.Value{structpb.NewStringValue("value5")}},
 						"key6": {Values: []*structpb.Value{structpb.NewStringValue("value")}},
 					})
+				require.NoError(err)
 				return &pbs.RemoveWorkerTagsRequest{
 					Id:      worker.PublicId,
 					Version: worker.Version + 1,
@@ -1738,7 +2086,7 @@ func TestService_RemoveWorkerTags(t *testing.T) {
 			name: "remove-nonexistent-tags",
 			req: func() *pbs.RemoveWorkerTagsRequest {
 				worker := server.TestKmsWorker(t, conn, wrapper)
-				s.addTagsInRepo(auth.DisabledAuthTestContext(iamRepoFn, proj.GetPublicId()), worker.PublicId,
+				_, err := s.addTagsInRepo(auth.DisabledAuthTestContext(iamRepoFn, proj.GetPublicId()), worker.PublicId,
 					worker.Version, map[string]*structpb.ListValue{
 						"key":  {Values: []*structpb.Value{structpb.NewStringValue("value")}},
 						"key2": {Values: []*structpb.Value{structpb.NewStringValue("value2")}},
@@ -1747,6 +2095,7 @@ func TestService_RemoveWorkerTags(t *testing.T) {
 						"key5": {Values: []*structpb.Value{structpb.NewStringValue("value5")}},
 						"key6": {Values: []*structpb.Value{structpb.NewStringValue("value")}},
 					})
+				require.NoError(err)
 				return &pbs.RemoveWorkerTagsRequest{
 					Id:      worker.PublicId,
 					Version: worker.Version + 1,