Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add non-deletable service user #1311

Merged
merged 8 commits into from
Nov 15, 2023
1 change: 1 addition & 0 deletions management/server/account.go
Original file line number Diff line number Diff line change
Expand Up @@ -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:"-"`
Expand Down
6 changes: 4 additions & 2 deletions management/server/http/users_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,9 @@ func (h *UsersHandler) GetAllUsers(w http.ResponseWriter, r *http.Request) {
users := make([]*api.User, 0)
for _, r := range data {
bcmmbaga marked this conversation as resolved.
Show resolved Hide resolved
if serviceUser == "" {
users = append(users, toUserResponse(r, claims.UserId))
if !r.NonDeletable {
users = append(users, toUserResponse(r, claims.UserId))
}
continue
}
includeServiceUser, err := strconv.ParseBool(serviceUser)
Expand All @@ -207,7 +209,7 @@ func (h *UsersHandler) GetAllUsers(w http.ResponseWriter, r *http.Request) {
util.WriteError(status.Errorf(status.InvalidArgument, "invalid service_user query parameter"), w)
return
}
if includeServiceUser == r.IsServiceUser {
if includeServiceUser == r.IsServiceUser && !r.NonDeletable {
users = append(users, toUserResponse(r, claims.UserId))
}
}
Expand Down
13 changes: 11 additions & 2 deletions management/server/http/users_handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,9 @@ import (
)

const (
serviceUserID = "serviceUserID"
regularUserID = "regularUserID"
serviceUserID = "serviceUserID"
nonDeletableServiceUserID = "nonDeletableServiceUserID"
regularUserID = "regularUserID"
)

var usersTestAccount = &server.Account{
Expand Down Expand Up @@ -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,
},
},
}

Expand All @@ -67,6 +75,7 @@ func initUsersTestData() *UsersHandler {
Name: "",
Email: "",
IsServiceUser: v.IsServiceUser,
NonDeletable: v.NonDeletable,
Issued: v.Issued,
})
}
Expand Down
21 changes: 15 additions & 6 deletions management/server/user.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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()

Expand All @@ -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

Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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)
Expand Down
77 changes: 54 additions & 23 deletions management/server/user_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down Expand Up @@ -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) {
Expand Down
Loading