diff --git a/management/server/account.go b/management/server/account.go index 25b03466869..c8d4a1943c2 100644 --- a/management/server/account.go +++ b/management/server/account.go @@ -211,6 +211,7 @@ type UserInfo struct { Status string `json:"-"` IsServiceUser bool `json:"is_service_user"` IsBlocked bool `json:"is_blocked"` + NonDeletable bool `json:"non_deletable"` LastLogin time.Time `json:"last_login"` Issued string `json:"issued"` IntegrationReference IntegrationReference `json:"-"` diff --git a/management/server/http/users_handler.go b/management/server/http/users_handler.go index 0441e8cc054..e2bf77de65b 100644 --- a/management/server/http/users_handler.go +++ b/management/server/http/users_handler.go @@ -197,10 +197,14 @@ func (h *UsersHandler) GetAllUsers(w http.ResponseWriter, r *http.Request) { users := make([]*api.User, 0) for _, r := range data { + if r.NonDeletable { + continue + } if serviceUser == "" { users = append(users, toUserResponse(r, claims.UserId)) continue } + includeServiceUser, err := strconv.ParseBool(serviceUser) log.Debugf("Should include service user: %v", includeServiceUser) if err != nil { diff --git a/management/server/http/users_handler_test.go b/management/server/http/users_handler_test.go index b4d449be385..ff886ca9fda 100644 --- a/management/server/http/users_handler_test.go +++ b/management/server/http/users_handler_test.go @@ -20,8 +20,9 @@ import ( ) const ( - serviceUserID = "serviceUserID" - regularUserID = "regularUserID" + serviceUserID = "serviceUserID" + nonDeletableServiceUserID = "nonDeletableServiceUserID" + regularUserID = "regularUserID" ) var usersTestAccount = &server.Account{ @@ -49,6 +50,13 @@ var usersTestAccount = &server.Account{ AutoGroups: []string{"group_1"}, Issued: server.UserIssuedAPI, }, + nonDeletableServiceUserID: { + Id: serviceUserID, + Role: "admin", + IsServiceUser: true, + NonDeletable: true, + Issued: server.UserIssuedIntegration, + }, }, } @@ -67,6 +75,7 @@ func initUsersTestData() *UsersHandler { Name: "", Email: "", IsServiceUser: v.IsServiceUser, + NonDeletable: v.NonDeletable, Issued: v.Issued, }) } diff --git a/management/server/user.go b/management/server/user.go index fd92bf35fd1..e2581770350 100644 --- a/management/server/user.go +++ b/management/server/user.go @@ -69,6 +69,8 @@ type User struct { AccountID string `json:"-" gorm:"index"` Role UserRole IsServiceUser bool + // NonDeletable indicates whether the service user can be deleted + NonDeletable bool // ServiceUserName is only set if IsServiceUser is true ServiceUserName string // AutoGroups is a list of Group IDs to auto-assign to peers registered by this user @@ -158,6 +160,7 @@ func (u *User) Copy() *User { Role: u.Role, AutoGroups: autoGroups, IsServiceUser: u.IsServiceUser, + NonDeletable: u.NonDeletable, ServiceUserName: u.ServiceUserName, PATs: pats, Blocked: u.Blocked, @@ -168,11 +171,12 @@ func (u *User) Copy() *User { } // NewUser creates a new user -func NewUser(id string, role UserRole, isServiceUser bool, serviceUserName string, autoGroups []string, issued string) *User { +func NewUser(id string, role UserRole, isServiceUser bool, nonDeletable bool, serviceUserName string, autoGroups []string, issued string) *User { return &User{ Id: id, Role: role, IsServiceUser: isServiceUser, + NonDeletable: nonDeletable, ServiceUserName: serviceUserName, AutoGroups: autoGroups, Issued: issued, @@ -181,16 +185,16 @@ func NewUser(id string, role UserRole, isServiceUser bool, serviceUserName strin // NewRegularUser creates a new user with role UserRoleUser func NewRegularUser(id string) *User { - return NewUser(id, UserRoleUser, false, "", []string{}, UserIssuedAPI) + return NewUser(id, UserRoleUser, false, false, "", []string{}, UserIssuedAPI) } // NewAdminUser creates a new user with role UserRoleAdmin func NewAdminUser(id string) *User { - return NewUser(id, UserRoleAdmin, false, "", []string{}, UserIssuedAPI) + return NewUser(id, UserRoleAdmin, false, false, "", []string{}, UserIssuedAPI) } // createServiceUser creates a new service user under the given account. -func (am *DefaultAccountManager) createServiceUser(accountID string, initiatorUserID string, role UserRole, serviceUserName string, autoGroups []string) (*UserInfo, error) { +func (am *DefaultAccountManager) createServiceUser(accountID string, initiatorUserID string, role UserRole, serviceUserName string, nonDeletable bool, autoGroups []string) (*UserInfo, error) { unlock := am.Store.AcquireAccountLock(accountID) defer unlock() @@ -208,7 +212,7 @@ func (am *DefaultAccountManager) createServiceUser(accountID string, initiatorUs } newUserID := uuid.New().String() - newUser := NewUser(newUserID, role, true, serviceUserName, autoGroups, UserIssuedAPI) + newUser := NewUser(newUserID, role, true, nonDeletable, serviceUserName, autoGroups, UserIssuedAPI) log.Debugf("New User: %v", newUser) account.Users[newUserID] = newUser @@ -236,7 +240,7 @@ func (am *DefaultAccountManager) createServiceUser(accountID string, initiatorUs // CreateUser creates a new user under the given account. Effectively this is a user invite. func (am *DefaultAccountManager) CreateUser(accountID, userID string, user *UserInfo) (*UserInfo, error) { if user.IsServiceUser { - return am.createServiceUser(accountID, userID, StrRoleToUserRole(user.Role), user.Name, user.AutoGroups) + return am.createServiceUser(accountID, userID, StrRoleToUserRole(user.Role), user.Name, user.NonDeletable, user.AutoGroups) } return am.inviteNewUser(accountID, userID, user) } @@ -420,6 +424,10 @@ func (am *DefaultAccountManager) DeleteUser(accountID, initiatorUserID string, t // handle service user first and exit, no need to fetch extra data from IDP, etc if targetUser.IsServiceUser { + if targetUser.NonDeletable { + return status.Errorf(status.PermissionDenied, "service user is marked as non-deletable") + } + am.deleteServiceUser(account, initiatorUserID, targetUser) return am.Store.SaveAccount(account) } @@ -955,6 +963,7 @@ func (am *DefaultAccountManager) GetUsersFromAccount(accountID, userID string) ( AutoGroups: localUser.AutoGroups, Status: string(UserStatusActive), IsServiceUser: localUser.IsServiceUser, + NonDeletable: localUser.NonDeletable, } } userInfos = append(userInfos, info) diff --git a/management/server/user_test.go b/management/server/user_test.go index 818a267d4ae..e628981b0b1 100644 --- a/management/server/user_test.go +++ b/management/server/user_test.go @@ -332,7 +332,7 @@ func TestUser_CreateServiceUser(t *testing.T) { eventStore: &activity.InMemoryEventStore{}, } - user, err := am.createServiceUser(mockAccountID, mockUserID, mockRole, mockServiceUserName, []string{"group1", "group2"}) + user, err := am.createServiceUser(mockAccountID, mockUserID, mockRole, mockServiceUserName, false, []string{"group1", "group2"}) if err != nil { t.Fatalf("Error when creating service user: %s", err) } @@ -413,31 +413,62 @@ func TestUser_CreateUser_RegularUser(t *testing.T) { } func TestUser_DeleteUser_ServiceUser(t *testing.T) { - store := newStore(t) - account := newAccountWithId(mockAccountID, mockUserID, "") - account.Users[mockServiceUserID] = &User{ - Id: mockServiceUserID, - IsServiceUser: true, - ServiceUserName: mockServiceUserName, - } - - err := store.SaveAccount(account) - if err != nil { - t.Fatalf("Error when saving account: %s", err) - } - - am := DefaultAccountManager{ - Store: store, - eventStore: &activity.InMemoryEventStore{}, + tests := []struct { + name string + serviceUser *User + assertErrFunc assert.ErrorAssertionFunc + assertErrMessage string + }{ + { + name: "Can delete service user", + serviceUser: &User{ + Id: mockServiceUserID, + IsServiceUser: true, + ServiceUserName: mockServiceUserName, + }, + assertErrFunc: assert.NoError, + }, + { + name: "Cannot delete non-deletable service user", + serviceUser: &User{ + Id: mockServiceUserID, + IsServiceUser: true, + ServiceUserName: mockServiceUserName, + NonDeletable: true, + }, + assertErrFunc: assert.Error, + assertErrMessage: "service user is marked as non-deletable", + }, } - err = am.DeleteUser(mockAccountID, mockUserID, mockServiceUserID) - if err != nil { - t.Fatalf("Error when deleting user: %s", err) + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + store := newStore(t) + account := newAccountWithId(mockAccountID, mockUserID, "") + account.Users[mockServiceUserID] = tt.serviceUser + + err := store.SaveAccount(account) + if err != nil { + t.Fatalf("Error when saving account: %s", err) + } + + am := DefaultAccountManager{ + Store: store, + eventStore: &activity.InMemoryEventStore{}, + } + + err = am.DeleteUser(mockAccountID, mockUserID, mockServiceUserID) + tt.assertErrFunc(t, err, tt.assertErrMessage) + + if err != nil { + assert.Equal(t, 2, len(store.Accounts[mockAccountID].Users)) + assert.NotNil(t, store.Accounts[mockAccountID].Users[mockServiceUserID]) + } else { + assert.Equal(t, 1, len(store.Accounts[mockAccountID].Users)) + assert.Nil(t, store.Accounts[mockAccountID].Users[mockServiceUserID]) + } + }) } - - assert.Equal(t, 1, len(store.Accounts[mockAccountID].Users)) - assert.Nil(t, store.Accounts[mockAccountID].Users[mockServiceUserID]) } func TestUser_DeleteUser_SelfDelete(t *testing.T) {