From 4b254b98776659a2a167cf05dd6b1844886ab624 Mon Sep 17 00:00:00 2001
From: Beardo Moore <beardo.moore@temporal.io>
Date: Wed, 5 Mar 2025 14:02:44 -0700
Subject: [PATCH] feat: allow account role to be set to none

---
 app/role.go      |  9 +++++++-
 app/user.go      | 16 +++++++++++---
 app/user_test.go | 55 ++++++++++++++++++++++++++++++++++++++++++++++--
 3 files changed, 74 insertions(+), 6 deletions(-)

diff --git a/app/role.go b/app/role.go
index 60ca1e98..720d6b18 100644
--- a/app/role.go
+++ b/app/role.go
@@ -15,6 +15,10 @@ var (
 	namespaceActionGroups = getNamespaceActionGroups()
 )
 
+const (
+	accountActionGroupNone = "none"
+)
+
 func getAccountActionGroups() []string {
 	var rv []string
 	for n, v := range auth.AccountActionGroup_value {
@@ -63,7 +67,10 @@ func getNamespaceRolesBatch(ctx context.Context, client authservice.AuthServiceC
 	return res.Roles, nil
 }
 
-func getAccountRole(ctx context.Context, client authservice.AuthServiceClient, actionGroup string) (*auth.Role, error) {
+func getAccountRole(ctx context.Context, client authservice.AuthServiceClient, actionGroup string, allowNone bool) (*auth.Role, error) {
+	if allowNone && strings.ToLower(strings.TrimSpace(actionGroup)) == accountActionGroupNone {
+		return nil, nil
+	}
 	ag, err := toAccountActionGroup(actionGroup)
 	if err != nil {
 		return nil, err
diff --git a/app/user.go b/app/user.go
index 3f98e520..56d63456 100644
--- a/app/user.go
+++ b/app/user.go
@@ -146,7 +146,7 @@ func (c *UserClient) inviteUsers(
 	var roleIDs []string
 
 	// first get the required account role
-	role, err := getAccountRole(c.ctx, c.client, accountRole)
+	role, err := getAccountRole(c.ctx, c.client, accountRole, false)
 	if err != nil {
 		return err
 	}
@@ -257,11 +257,21 @@ func (c *UserClient) setAccountRole(
 		return err
 	}
 	var newRoleIDs []string
-	accountRoleToSet, err := getAccountRole(c.ctx, c.client, accountRole)
+	accountRoleToSet, err := getAccountRole(c.ctx, c.client, accountRole, true)
 	if err != nil {
 		return err
 	}
-	if accountRoleToSet.Spec.AccountRole.ActionGroup == auth.ACCOUNT_ACTION_GROUP_ADMIN {
+	if accountRoleToSet == nil {
+		// Setting account role to none.
+		for _, r := range userRoles {
+			// remove any account roles
+			if r.Type == auth.ROLE_TYPE_PREDEFINED && r.Spec.AccountRole != nil {
+				continue
+			} else {
+				newRoleIDs = append(newRoleIDs, r.Id)
+			}
+		}
+	} else if accountRoleToSet.Spec.AccountRole.ActionGroup == auth.ACCOUNT_ACTION_GROUP_ADMIN {
 		// set the user account admin role
 		y, err := ConfirmPrompt(ctx, "Setting admin role on user. All existing namespace permissions will be replaced, please confirm")
 		if err != nil {
diff --git a/app/user_test.go b/app/user_test.go
index a4b1ef59..7bb9210a 100644
--- a/app/user_test.go
+++ b/app/user_test.go
@@ -3,6 +3,9 @@ package app
 import (
 	"context"
 	"errors"
+	"reflect"
+	"testing"
+
 	"github.com/golang/mock/gomock"
 	"github.com/stretchr/testify/suite"
 	"github.com/temporalio/tcld/protogen/api/auth/v1"
@@ -10,8 +13,6 @@ import (
 	"github.com/temporalio/tcld/protogen/api/request/v1"
 	authservicemock "github.com/temporalio/tcld/protogen/apimock/authservice/v1"
 	"github.com/urfave/cli/v2"
-	"reflect"
-	"testing"
 )
 
 func TestUser(t *testing.T) {
@@ -397,6 +398,56 @@ func (s *UserTestSuite) TestSetAccountRoleAdmin() {
 	s.NoError(s.RunCmd("user", "set-account-role", "--user-email", "test@example.com", "--account-role", "Admin"))
 }
 
+func (s *UserTestSuite) TestSetAccountRoleNone() {
+	s.mockAuthService.EXPECT().GetUser(gomock.Any(), gomock.Any()).Return(&authservice.GetUserResponse{
+		User: &auth.User{
+			Id: "test-user-id",
+			Spec: &auth.UserSpec{
+				Email: "test@example.com",
+			},
+		},
+	}, nil).Times(1)
+	s.mockAuthService.EXPECT().GetRoles(gomock.Any(), gomock.Any()).Return(&authservice.GetRolesResponse{
+		Roles: []*auth.Role{
+			{
+				Id:   "test-account-developer-role",
+				Type: auth.ROLE_TYPE_PREDEFINED,
+				Spec: &auth.RoleSpec{
+					AccountRole: &auth.AccountRoleSpec{
+						ActionGroup: auth.ACCOUNT_ACTION_GROUP_DEVELOPER,
+					},
+				},
+			},
+			{
+				Id:   "test-namespace-admin-role",
+				Type: auth.ROLE_TYPE_PREDEFINED,
+				Spec: &auth.RoleSpec{
+					NamespaceRoles: []*auth.NamespaceRoleSpec{
+						{
+							Namespace:   "ns1",
+							ActionGroup: auth.NAMESPACE_ACTION_GROUP_ADMIN,
+						},
+					},
+				},
+			},
+		},
+	}, nil).Times(1)
+	s.mockAuthService.EXPECT().UpdateUser(gomock.Any(), gomock.All(&updateUserRequestMatcher{
+		request: &authservice.UpdateUserRequest{
+			UserId: "test-user-id",
+			Spec: &auth.UserSpec{
+				Email: "test@example.com",
+				Roles: []string{"test-namespace-admin-role"},
+			},
+		},
+	})).Return(&authservice.UpdateUserResponse{
+		RequestStatus: &request.RequestStatus{
+			State: request.STATE_FULFILLED,
+		},
+	}, nil)
+	s.NoError(s.RunCmd("user", "set-account-role", "--user-email", "test@example.com", "--account-role", "none"))
+}
+
 func (s *UserTestSuite) TestSetNamespacePermissions() {
 	s.mockAuthService.EXPECT().GetUser(gomock.Any(), gomock.Any()).Return(&authservice.GetUserResponse{
 		User: &auth.User{