diff --git a/client/internal/dns/server.go b/client/internal/dns/server.go index 9bb9a76a9..f984f02ec 100644 --- a/client/internal/dns/server.go +++ b/client/internal/dns/server.go @@ -488,7 +488,18 @@ func (s *DefaultServer) addHostRootZone() { handler := newUpstreamResolver(s.ctx) handler.upstreamServers = make([]string, len(s.hostsDnsList)) for n, ua := range s.hostsDnsList { - handler.upstreamServers[n] = fmt.Sprintf("%s:53", ua) + a, err := netip.ParseAddr(ua) + if err != nil { + log.Errorf("invalid upstream IP address: %s, error: %s", ua, err) + continue + } + + ipString := ua + if !a.Is4() { + ipString = fmt.Sprintf("[%s]", ua) + } + + handler.upstreamServers[n] = fmt.Sprintf("%s:53", ipString) } handler.deactivate = func() {} handler.reactivate = func() {} diff --git a/management/server/account.go b/management/server/account.go index 2ac00410c..14fd241a1 100644 --- a/management/server/account.go +++ b/management/server/account.go @@ -905,7 +905,7 @@ func (am *DefaultAccountManager) UpdateAccountSettings(accountID, userID string, return nil, err } - if !user.IsAdmin() { + if !user.HasAdminPower() { return nil, status.Errorf(status.PermissionDenied, "user is not allowed to update account") } @@ -1047,7 +1047,7 @@ func (am *DefaultAccountManager) DeleteAccount(accountID, userID string) error { return err } - if !user.IsAdmin() { + if !user.HasAdminPower() { return status.Errorf(status.PermissionDenied, "user is not allowed to delete account") } @@ -1731,7 +1731,7 @@ func newAccountWithId(accountID, userID, domain string) *Account { routes := make(map[string]*route.Route) setupKeys := map[string]*SetupKey{} nameServersGroups := make(map[string]*nbdns.NameServerGroup) - users[userID] = NewAdminUser(userID) + users[userID] = NewOwnerUser(userID) dnsSettings := DNSSettings{ DisabledManagementGroups: make([]string, 0), } diff --git a/management/server/account_test.go b/management/server/account_test.go index cea60d5de..5e204984e 100644 --- a/management/server/account_test.go +++ b/management/server/account_test.go @@ -462,7 +462,7 @@ func TestDefaultAccountManager_GetAccountFromToken(t *testing.T) { inputInitUserParams: defaultInitAccount, testingFunc: require.NotEqual, expectedMSG: "account IDs shouldn't match", - expectedUserRole: UserRoleAdmin, + expectedUserRole: UserRoleOwner, expectedDomainCategory: "", expectedDomain: publicDomain, expectedPrimaryDomainStatus: false, @@ -484,7 +484,7 @@ func TestDefaultAccountManager_GetAccountFromToken(t *testing.T) { inputInitUserParams: initUnknown, testingFunc: require.NotEqual, expectedMSG: "account IDs shouldn't match", - expectedUserRole: UserRoleAdmin, + expectedUserRole: UserRoleOwner, expectedDomain: unknownDomain, expectedDomainCategory: "", expectedPrimaryDomainStatus: false, @@ -502,7 +502,7 @@ func TestDefaultAccountManager_GetAccountFromToken(t *testing.T) { inputInitUserParams: defaultInitAccount, testingFunc: require.NotEqual, expectedMSG: "account IDs shouldn't match", - expectedUserRole: UserRoleAdmin, + expectedUserRole: UserRoleOwner, expectedDomain: privateDomain, expectedDomainCategory: PrivateCategory, expectedPrimaryDomainStatus: true, @@ -543,7 +543,7 @@ func TestDefaultAccountManager_GetAccountFromToken(t *testing.T) { inputInitUserParams: defaultInitAccount, testingFunc: require.Equal, expectedMSG: "account IDs should match", - expectedUserRole: UserRoleAdmin, + expectedUserRole: UserRoleOwner, expectedDomain: defaultInitAccount.Domain, expectedDomainCategory: PrivateCategory, expectedPrimaryDomainStatus: true, @@ -562,7 +562,7 @@ func TestDefaultAccountManager_GetAccountFromToken(t *testing.T) { inputInitUserParams: defaultInitAccount, testingFunc: require.Equal, expectedMSG: "account IDs should match", - expectedUserRole: UserRoleAdmin, + expectedUserRole: UserRoleOwner, expectedDomain: defaultInitAccount.Domain, expectedDomainCategory: PrivateCategory, expectedPrimaryDomainStatus: true, @@ -580,7 +580,7 @@ func TestDefaultAccountManager_GetAccountFromToken(t *testing.T) { inputInitUserParams: defaultInitAccount, testingFunc: require.NotEqual, expectedMSG: "account IDs shouldn't match", - expectedUserRole: UserRoleAdmin, + expectedUserRole: UserRoleOwner, expectedDomain: "", expectedDomainCategory: "", expectedPrimaryDomainStatus: false, @@ -1339,7 +1339,7 @@ func TestGetUsersFromAccount(t *testing.T) { t.Fatal(err) } - users := map[string]*User{"1": {Id: "1", Role: "admin"}, "2": {Id: "2", Role: "user"}, "3": {Id: "3", Role: "user"}} + users := map[string]*User{"1": {Id: "1", Role: UserRoleOwner}, "2": {Id: "2", Role: "user"}, "3": {Id: "3", Role: "user"}} accountId := "test_account_id" account, err := createAccount(manager, accountId, users["1"].Id, "") diff --git a/management/server/activity/codes.go b/management/server/activity/codes.go index bcd788b6f..54a27e4cc 100644 --- a/management/server/activity/codes.go +++ b/management/server/activity/codes.go @@ -128,6 +128,8 @@ const ( PeerApproved // PeerApprovalRevoked indicates that the peer approval has been revoked PeerApprovalRevoked + // TransferredOwnerRole indicates that the user transferred the owner role of the account + TransferredOwnerRole ) var activityMap = map[Activity]Code{ @@ -190,6 +192,7 @@ var activityMap = map[Activity]Code{ AccountPeerApprovalDisabled: {"Account peer approval disabled", "account.setting.peer.approval.disable"}, PeerApproved: {"Peer approved", "peer.approve"}, PeerApprovalRevoked: {"Peer approval revoked", "peer.approval.revoke"}, + TransferredOwnerRole: {"Transferred owner role", "transferred.owner.role"}, } // StringCode returns a string code of the activity diff --git a/management/server/dns.go b/management/server/dns.go index 236ba1f57..820a5431f 100644 --- a/management/server/dns.go +++ b/management/server/dns.go @@ -48,8 +48,8 @@ func (am *DefaultAccountManager) GetDNSSettings(accountID string, userID string) return nil, err } - if !user.IsAdmin() { - return nil, status.Errorf(status.PermissionDenied, "only admins are allowed to view DNS settings") + if !user.HasAdminPower() { + return nil, status.Errorf(status.PermissionDenied, "only users with admin power are allowed to view DNS settings") } dnsSettings := account.DNSSettings.Copy() return &dnsSettings, nil @@ -70,8 +70,8 @@ func (am *DefaultAccountManager) SaveDNSSettings(accountID string, userID string return err } - if !user.IsAdmin() { - return status.Errorf(status.PermissionDenied, "only admins are allowed to update DNS settings") + if !user.HasAdminPower() { + return status.Errorf(status.PermissionDenied, "only users with admin power are allowed to update DNS settings") } if dnsSettingsToSave == nil { diff --git a/management/server/group.go b/management/server/group.go index 07f61425e..cf7320c29 100644 --- a/management/server/group.go +++ b/management/server/group.go @@ -170,7 +170,7 @@ func (am *DefaultAccountManager) DeleteGroup(accountId, userId, groupID string) return status.Errorf(status.NotFound, "user not found") } if executingUser.Role != UserRoleAdmin || !executingUser.IsServiceUser { - return status.Errorf(status.PermissionDenied, "only admins service user can delete integration group") + return status.Errorf(status.PermissionDenied, "only service users with admin power can delete integration group") } } diff --git a/management/server/group_test.go b/management/server/group_test.go index 97a1b75f5..e2051a656 100644 --- a/management/server/group_test.go +++ b/management/server/group_test.go @@ -57,7 +57,7 @@ func TestDefaultAccountManager_DeleteGroup(t *testing.T) { { "integration", "grp-for-integration", - "only admins service user can delete integration group", + "only service users with admin power can delete integration group", }, } diff --git a/management/server/http/accounts_handler.go b/management/server/http/accounts_handler.go index fd535c9af..c2751abd4 100644 --- a/management/server/http/accounts_handler.go +++ b/management/server/http/accounts_handler.go @@ -41,7 +41,7 @@ func (h *AccountsHandler) GetAllAccounts(w http.ResponseWriter, r *http.Request) return } - if !user.IsAdmin() { + if !user.HasAdminPower() { util.WriteError(status.Errorf(status.PermissionDenied, "the user has no permission to access account data"), w) return } diff --git a/management/server/http/middleware/access_control.go b/management/server/http/middleware/access_control.go index 31b5a2a9d..2f9374fc1 100644 --- a/management/server/http/middleware/access_control.go +++ b/management/server/http/middleware/access_control.go @@ -53,7 +53,7 @@ func (a *AccessControl) Handler(h http.Handler) http.Handler { return } - if !user.IsAdmin() { + if !user.HasAdminPower() { switch r.Method { case http.MethodDelete, http.MethodPost, http.MethodPatch, http.MethodPut: @@ -63,7 +63,7 @@ func (a *AccessControl) Handler(h http.Handler) http.Handler { return } - util.WriteError(status.Errorf(status.PermissionDenied, "only admin can perform this operation"), w) + util.WriteError(status.Errorf(status.PermissionDenied, "only users with admin power can perform this operation"), w) return } } diff --git a/management/server/idp/mock.go b/management/server/idp/mock.go new file mode 100644 index 000000000..7605466e7 --- /dev/null +++ b/management/server/idp/mock.go @@ -0,0 +1,77 @@ +package idp + +// MockIDP is a mock implementation of the IDP interface +type MockIDP struct { + UpdateUserAppMetadataFunc func(userId string, appMetadata AppMetadata) error + GetUserDataByIDFunc func(userId string, appMetadata AppMetadata) (*UserData, error) + GetAccountFunc func(accountId string) ([]*UserData, error) + GetAllAccountsFunc func() (map[string][]*UserData, error) + CreateUserFunc func(email, name, accountID, invitedByEmail string) (*UserData, error) + GetUserByEmailFunc func(email string) ([]*UserData, error) + InviteUserByIDFunc func(userID string) error + DeleteUserFunc func(userID string) error +} + +// UpdateUserAppMetadata is a mock implementation of the IDP interface UpdateUserAppMetadata method +func (m *MockIDP) UpdateUserAppMetadata(userId string, appMetadata AppMetadata) error { + if m.UpdateUserAppMetadataFunc != nil { + return m.UpdateUserAppMetadataFunc(userId, appMetadata) + } + return nil +} + +// GetUserDataByID is a mock implementation of the IDP interface GetUserDataByID method +func (m *MockIDP) GetUserDataByID(userId string, appMetadata AppMetadata) (*UserData, error) { + if m.GetUserDataByIDFunc != nil { + return m.GetUserDataByIDFunc(userId, appMetadata) + } + return nil, nil +} + +// GetAccount is a mock implementation of the IDP interface GetAccount method +func (m *MockIDP) GetAccount(accountId string) ([]*UserData, error) { + if m.GetAccountFunc != nil { + return m.GetAccountFunc(accountId) + } + return nil, nil +} + +// GetAllAccounts is a mock implementation of the IDP interface GetAllAccounts method +func (m *MockIDP) GetAllAccounts() (map[string][]*UserData, error) { + if m.GetAllAccountsFunc != nil { + return m.GetAllAccountsFunc() + } + return nil, nil +} + +// CreateUser is a mock implementation of the IDP interface CreateUser method +func (m *MockIDP) CreateUser(email, name, accountID, invitedByEmail string) (*UserData, error) { + if m.CreateUserFunc != nil { + return m.CreateUserFunc(email, name, accountID, invitedByEmail) + } + return nil, nil +} + +// GetUserByEmail is a mock implementation of the IDP interface GetUserByEmail method +func (m *MockIDP) GetUserByEmail(email string) ([]*UserData, error) { + if m.GetUserByEmailFunc != nil { + return m.GetUserByEmailFunc(email) + } + return nil, nil +} + +// InviteUserByID is a mock implementation of the IDP interface InviteUserByID method +func (m *MockIDP) InviteUserByID(userID string) error { + if m.InviteUserByIDFunc != nil { + return m.InviteUserByIDFunc(userID) + } + return nil +} + +// DeleteUser is a mock implementation of the IDP interface DeleteUser method +func (m *MockIDP) DeleteUser(userID string) error { + if m.DeleteUserFunc != nil { + return m.DeleteUserFunc(userID) + } + return nil +} diff --git a/management/server/peer.go b/management/server/peer.go index 8fafb7b71..630a01548 100644 --- a/management/server/peer.go +++ b/management/server/peer.go @@ -53,7 +53,7 @@ func (am *DefaultAccountManager) GetPeers(accountID, userID string) ([]*nbpeer.P peers := make([]*nbpeer.Peer, 0) peersMap := make(map[string]*nbpeer.Peer) for _, peer := range account.Peers { - if !user.IsAdmin() && user.Id != peer.UserID { + if !user.HasAdminPower() && user.Id != peer.UserID { // only display peers that belong to the current user if the current user is not an admin continue } @@ -701,7 +701,7 @@ func (am *DefaultAccountManager) GetPeer(accountID, peerID, userID string) (*nbp } // if admin or user owns this peer, return peer - if user.IsAdmin() || peer.UserID == userID { + if user.HasAdminPower() || peer.UserID == userID { return peer, nil } diff --git a/management/server/policy.go b/management/server/policy.go index 14147a652..37718a3e0 100644 --- a/management/server/policy.go +++ b/management/server/policy.go @@ -320,8 +320,8 @@ func (am *DefaultAccountManager) GetPolicy(accountID, policyID, userID string) ( return nil, err } - if !user.IsAdmin() { - return nil, status.Errorf(status.PermissionDenied, "only admins are allowed to view policies") + if !user.HasAdminPower() { + return nil, status.Errorf(status.PermissionDenied, "only users with admin power are allowed to view policies") } for _, policy := range account.Policies { @@ -403,8 +403,8 @@ func (am *DefaultAccountManager) ListPolicies(accountID, userID string) ([]*Poli return nil, err } - if !user.IsAdmin() { - return nil, status.Errorf(status.PermissionDenied, "Only Administrators can view policies") + if !user.HasAdminPower() { + return nil, status.Errorf(status.PermissionDenied, "only users with admin power can view policies") } return account.Policies, nil diff --git a/management/server/route.go b/management/server/route.go index 14b5bef3f..5f7976fe4 100644 --- a/management/server/route.go +++ b/management/server/route.go @@ -27,8 +27,8 @@ func (am *DefaultAccountManager) GetRoute(accountID, routeID, userID string) (*r return nil, err } - if !user.IsAdmin() { - return nil, status.Errorf(status.PermissionDenied, "Only administrators can view Network Routes") + if !user.HasAdminPower() { + return nil, status.Errorf(status.PermissionDenied, "only users with admin power can view Network Routes") } wantedRoute, found := account.Routes[routeID] @@ -296,8 +296,8 @@ func (am *DefaultAccountManager) ListRoutes(accountID, userID string) ([]*route. return nil, err } - if !user.IsAdmin() { - return nil, status.Errorf(status.PermissionDenied, "Only administrators can view Network Routes") + if !user.HasAdminPower() { + return nil, status.Errorf(status.PermissionDenied, "only users with admin power can view Network Routes") } routes := make([]*route.Route, 0, len(account.Routes)) diff --git a/management/server/setupkey.go b/management/server/setupkey.go index d347fb181..b557b07c8 100644 --- a/management/server/setupkey.go +++ b/management/server/setupkey.go @@ -342,7 +342,7 @@ func (am *DefaultAccountManager) ListSetupKeys(accountID, userID string) ([]*Set keys := make([]*SetupKey, 0, len(account.SetupKeys)) for _, key := range account.SetupKeys { var k *SetupKey - if !user.IsAdmin() { + if !user.HasAdminPower() { k = key.HiddenCopy(999) } else { k = key.Copy() @@ -384,7 +384,7 @@ func (am *DefaultAccountManager) GetSetupKey(accountID, userID, keyID string) (* foundKey.UpdatedAt = foundKey.CreatedAt } - if !user.IsAdmin() { + if !user.HasAdminPower() { foundKey = foundKey.HiddenCopy(999) } diff --git a/management/server/user.go b/management/server/user.go index e303893dd..a84765cb1 100644 --- a/management/server/user.go +++ b/management/server/user.go @@ -16,6 +16,7 @@ import ( ) const ( + UserRoleOwner UserRole = "owner" UserRoleAdmin UserRole = "admin" UserRoleUser UserRole = "user" UserRoleUnknown UserRole = "unknown" @@ -31,6 +32,8 @@ const ( // StrRoleToUserRole returns UserRole for a given strRole or UserRoleUnknown if the specified role is unknown func StrRoleToUserRole(strRole string) UserRole { switch strings.ToLower(strRole) { + case "owner": + return UserRoleOwner case "admin": return UserRoleAdmin case "user": @@ -98,9 +101,9 @@ func (u *User) LastDashboardLoginChanged(LastLogin time.Time) bool { return LastLogin.After(u.LastLogin) && !u.LastLogin.IsZero() } -// IsAdmin returns true if the user is an admin, false otherwise -func (u *User) IsAdmin() bool { - return u.Role == UserRoleAdmin +// HasAdminPower returns true if the user has admin or owner roles, false otherwise +func (u *User) HasAdminPower() bool { + return u.Role == UserRoleAdmin || u.Role == UserRoleOwner } // ToUserInfo converts a User object to a UserInfo object. @@ -194,6 +197,11 @@ func NewAdminUser(id string) *User { return NewUser(id, UserRoleAdmin, false, false, "", []string{}, UserIssuedAPI) } +// NewOwnerUser creates a new user with role UserRoleOwner +func NewOwnerUser(id string) *User { + return NewUser(id, UserRoleOwner, 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, nonDeletable bool, autoGroups []string) (*UserInfo, error) { unlock := am.Store.AcquireAccountLock(accountID) @@ -208,8 +216,12 @@ func (am *DefaultAccountManager) createServiceUser(accountID string, initiatorUs if executingUser == nil { return nil, status.Errorf(status.NotFound, "user not found") } - if executingUser.Role != UserRoleAdmin { - return nil, status.Errorf(status.PermissionDenied, "only admins can create service users") + if !executingUser.HasAdminPower() { + return nil, status.Errorf(status.PermissionDenied, "only users with admin power can create service users") + } + + if role == UserRoleOwner { + return nil, status.Errorf(status.InvalidArgument, "can't create a service user with owner role") } newUserID := uuid.New().String() @@ -259,11 +271,15 @@ func (am *DefaultAccountManager) inviteNewUser(accountID, userID string, invite return nil, fmt.Errorf("provided user update is nil") } + invitedRole := StrRoleToUserRole(invite.Role) + switch { case invite.Name == "": return nil, status.Errorf(status.InvalidArgument, "name can't be empty") case invite.Email == "": return nil, status.Errorf(status.InvalidArgument, "email can't be empty") + case invitedRole == UserRoleOwner: + return nil, status.Errorf(status.InvalidArgument, "can't invite a user with owner role") default: } @@ -312,10 +328,9 @@ func (am *DefaultAccountManager) inviteNewUser(accountID, userID string, invite return nil, err } - role := StrRoleToUserRole(invite.Role) newUser := &User{ Id: idpUser.ID, - Role: role, + Role: invitedRole, AutoGroups: invite.AutoGroups, Issued: invite.Issued, IntegrationReference: invite.IntegrationReference, @@ -417,8 +432,8 @@ func (am *DefaultAccountManager) DeleteUser(accountID, initiatorUserID string, t if executingUser == nil { return status.Errorf(status.NotFound, "user not found") } - if executingUser.Role != UserRoleAdmin { - return status.Errorf(status.PermissionDenied, "only admins can delete users") + if !executingUser.HasAdminPower() { + return status.Errorf(status.PermissionDenied, "only users with admin power can delete users") } targetUser := account.Users[targetUserID] @@ -426,9 +441,13 @@ func (am *DefaultAccountManager) DeleteUser(accountID, initiatorUserID string, t return status.Errorf(status.NotFound, "target user not found") } + if targetUser.Role == UserRoleOwner { + return status.Errorf(status.PermissionDenied, "unable to delete a user with owner role") + } + // disable deleting integration user if the initiator is not admin service user if targetUser.Issued == UserIssuedIntegration && !executingUser.IsServiceUser { - return status.Errorf(status.PermissionDenied, "only admin service user can delete this user") + return status.Errorf(status.PermissionDenied, "only integration service user can delete this user") } // handle service user first and exit, no need to fetch extra data from IDP, etc @@ -567,7 +586,7 @@ func (am *DefaultAccountManager) CreatePAT(accountID string, initiatorUserID str return nil, status.Errorf(status.NotFound, "user not found") } - if !(initiatorUserID == targetUserID || (executingUser.IsAdmin() && targetUser.IsServiceUser)) { + if !(initiatorUserID == targetUserID || (executingUser.HasAdminPower() && targetUser.IsServiceUser)) { return nil, status.Errorf(status.PermissionDenied, "no permission to create PAT for this user") } @@ -609,7 +628,7 @@ func (am *DefaultAccountManager) DeletePAT(accountID string, initiatorUserID str return status.Errorf(status.NotFound, "user not found") } - if !(initiatorUserID == targetUserID || (executingUser.IsAdmin() && targetUser.IsServiceUser)) { + if !(initiatorUserID == targetUserID || (executingUser.HasAdminPower() && targetUser.IsServiceUser)) { return status.Errorf(status.PermissionDenied, "no permission to delete PAT for this user") } @@ -659,7 +678,7 @@ func (am *DefaultAccountManager) GetPAT(accountID string, initiatorUserID string return nil, status.Errorf(status.NotFound, "user not found") } - if !(initiatorUserID == targetUserID || (executingUser.IsAdmin() && targetUser.IsServiceUser)) { + if !(initiatorUserID == targetUserID || (executingUser.HasAdminPower() && targetUser.IsServiceUser)) { return nil, status.Errorf(status.PermissionDenied, "no permission to get PAT for this userser") } @@ -691,7 +710,7 @@ func (am *DefaultAccountManager) GetAllPATs(accountID string, initiatorUserID st return nil, status.Errorf(status.NotFound, "user not found") } - if !(initiatorUserID == targetUserID || (executingUser.IsAdmin() && targetUser.IsServiceUser)) { + if !(initiatorUserID == targetUserID || (executingUser.HasAdminPower() && targetUser.IsServiceUser)) { return nil, status.Errorf(status.PermissionDenied, "no permission to get PAT for this user") } @@ -728,8 +747,8 @@ func (am *DefaultAccountManager) SaveOrAddUser(accountID, initiatorUserID string return nil, err } - if !initiatorUser.IsAdmin() || initiatorUser.IsBlocked() { - return nil, status.Errorf(status.PermissionDenied, "only admins are authorized to perform user update operations") + if !initiatorUser.HasAdminPower() || initiatorUser.IsBlocked() { + return nil, status.Errorf(status.PermissionDenied, "only users with admin power are authorized to perform user update operations") } oldUser := account.Users[update.Id] @@ -741,14 +760,38 @@ func (am *DefaultAccountManager) SaveOrAddUser(accountID, initiatorUserID string oldUser = update } - if initiatorUser.IsAdmin() && initiatorUserID == update.Id && oldUser.Blocked != update.Blocked { + if initiatorUser.HasAdminPower() && initiatorUserID == update.Id && oldUser.Blocked != update.Blocked { return nil, status.Errorf(status.PermissionDenied, "admins can't block or unblock themselves") } - if initiatorUser.IsAdmin() && initiatorUserID == update.Id && update.Role != UserRoleAdmin { + if initiatorUser.HasAdminPower() && initiatorUserID == update.Id && update.Role != initiatorUser.Role { return nil, status.Errorf(status.PermissionDenied, "admins can't change their role") } + if initiatorUser.Role == UserRoleAdmin && oldUser.Role == UserRoleOwner && update.Role != oldUser.Role { + return nil, status.Errorf(status.PermissionDenied, "only owners can remove owner role from their user") + } + + if initiatorUser.Role == UserRoleAdmin && oldUser.Role == UserRoleOwner && update.IsBlocked() && !oldUser.IsBlocked() { + return nil, status.Errorf(status.PermissionDenied, "unable to block owner user") + } + + if initiatorUser.Role == UserRoleAdmin && update.Role == UserRoleOwner && update.Role != oldUser.Role { + return nil, status.Errorf(status.PermissionDenied, "only owners can add owner role to other users") + } + + if oldUser.IsServiceUser && update.Role == UserRoleOwner { + return nil, status.Errorf(status.PermissionDenied, "can't update a service user with owner role") + } + + transferedOwnerRole := false + if initiatorUser.Role == UserRoleOwner && initiatorUserID != update.Id && update.Role == UserRoleOwner { + newInitiatorUser := initiatorUser.Copy() + newInitiatorUser.Role = UserRoleAdmin + account.Users[initiatorUserID] = newInitiatorUser + transferedOwnerRole = true + } + // only auto groups, revoked status, and integration reference can be updated for now newUser := oldUser.Copy() newUser.Role = update.Role @@ -807,9 +850,12 @@ func (am *DefaultAccountManager) SaveOrAddUser(accountID, initiatorUserID string } } - // store activity logs - if oldUser.Role != newUser.Role { + switch { + case transferedOwnerRole: + am.StoreEvent(initiatorUserID, oldUser.Id, accountID, activity.TransferredOwnerRole, nil) + case oldUser.Role != newUser.Role: am.StoreEvent(initiatorUserID, oldUser.Id, accountID, activity.UserRoleUpdated, map[string]any{"role": newUser.Role}) + default: } if update.AutoGroups != nil { @@ -883,7 +929,7 @@ func (am *DefaultAccountManager) GetOrCreateAccountByUser(userID, domain string) userObj := account.Users[userID] - if account.Domain != lowerDomain && userObj.Role == UserRoleAdmin { + if account.Domain != lowerDomain && userObj.Role == UserRoleOwner { account.Domain = lowerDomain err = am.Store.SaveAccount(account) if err != nil { @@ -941,7 +987,7 @@ func (am *DefaultAccountManager) GetUsersFromAccount(accountID, userID string) ( // in case of self-hosted, or IDP doesn't return anything, we will return the locally stored userInfo if len(queriedUsers) == 0 { for _, accountUser := range account.Users { - if !user.IsAdmin() && user.Id != accountUser.Id { + if !user.HasAdminPower() && user.Id != accountUser.Id { // if user is not an admin then show only current user and do not show other users continue } @@ -955,7 +1001,7 @@ func (am *DefaultAccountManager) GetUsersFromAccount(accountID, userID string) ( } for _, localUser := range account.Users { - if !user.IsAdmin() && user.Id != localUser.Id { + if !user.HasAdminPower() && user.Id != localUser.Id { // if user is not an admin then show only current user and do not show other users continue } diff --git a/management/server/user_test.go b/management/server/user_test.go index e628981b0..d445ade62 100644 --- a/management/server/user_test.go +++ b/management/server/user_test.go @@ -348,6 +348,11 @@ func TestUser_CreateServiceUser(t *testing.T) { assert.Zero(t, user.Email) assert.True(t, user.IsServiceUser) assert.Equal(t, "active", user.Status) + + _, err = am.createServiceUser(mockAccountID, mockUserID, UserRoleOwner, mockServiceUserName, false, nil) + if err == nil { + t.Fatal("should return error when creating service user with owner role") + } } func TestUser_CreateUser_ServiceUser(t *testing.T) { @@ -412,6 +417,75 @@ func TestUser_CreateUser_RegularUser(t *testing.T) { assert.Errorf(t, err, "Not configured IDP will throw error but right path used") } +func TestUser_InviteNewUser(t *testing.T) { + store := newStore(t) + account := newAccountWithId(mockAccountID, mockUserID, "") + + err := store.SaveAccount(account) + if err != nil { + t.Fatalf("Error when saving account: %s", err) + } + + am := DefaultAccountManager{ + Store: store, + eventStore: &activity.InMemoryEventStore{}, + cacheLoading: map[string]chan struct{}{}, + } + + goCacheClient := gocache.New(CacheExpirationMax, 30*time.Minute) + goCacheStore := cacheStore.NewGoCache(goCacheClient) + am.cacheManager = cache.NewLoadable[[]*idp.UserData](am.loadAccount, cache.New[[]*idp.UserData](goCacheStore)) + + mockData := []*idp.UserData{ + { + Email: "user@test.com", + Name: "user", + ID: mockUserID, + }, + } + + idpMock := idp.MockIDP{ + CreateUserFunc: func(email, name, accountID, invitedByEmail string) (*idp.UserData, error) { + newData := &idp.UserData{ + Email: email, + Name: name, + ID: "id", + } + + mockData = append(mockData, newData) + + return newData, nil + }, + GetAccountFunc: func(accountId string) ([]*idp.UserData, error) { + return mockData, nil + }, + } + + am.idpManager = &idpMock + + // test if new invite with regular role works + _, err = am.inviteNewUser(mockAccountID, mockUserID, &UserInfo{ + Name: mockServiceUserName, + Role: mockRole, + Email: "test@teste.com", + IsServiceUser: false, + AutoGroups: []string{"group1", "group2"}, + }) + + assert.NoErrorf(t, err, "Invite user should not throw error") + + // test if new invite with owner role fails + _, err = am.inviteNewUser(mockAccountID, mockUserID, &UserInfo{ + Name: mockServiceUserName, + Role: string(UserRoleOwner), + Email: "test2@teste.com", + IsServiceUser: false, + AutoGroups: []string{"group1", "group2"}, + }) + + assert.Errorf(t, err, "Invite user with owner role should throw error") +} + func TestUser_DeleteUser_ServiceUser(t *testing.T) { tests := []struct { name string @@ -514,6 +588,14 @@ func TestUser_DeleteUser_regularUser(t *testing.T) { Issued: UserIssuedIntegration, } + targetId = "user5" + account.Users[targetId] = &User{ + Id: targetId, + IsServiceUser: false, + Issued: UserIssuedAPI, + Role: UserRoleOwner, + } + err := store.SaveAccount(account) if err != nil { t.Fatalf("Error when saving account: %s", err) @@ -546,6 +628,12 @@ func TestUser_DeleteUser_regularUser(t *testing.T) { assertErrFunc: assert.Error, assertErrMessage: "only admin service user can delete this user", }, + { + name: "Delete user with owner role should return permission denied ", + userID: "user5", + assertErrFunc: assert.Error, + assertErrMessage: "unable to delete a user with owner role", + }, } for _, testCase := range testCases { @@ -581,7 +669,7 @@ func TestDefaultAccountManager_GetUser(t *testing.T) { } assert.Equal(t, mockUserID, user.Id) - assert.True(t, user.IsAdmin()) + assert.True(t, user.HasAdminPower()) assert.False(t, user.IsBlocked()) } @@ -609,7 +697,7 @@ func TestDefaultAccountManager_ListUsers(t *testing.T) { admins := 0 regular := 0 for _, user := range users { - if user.IsAdmin() { + if user.HasAdminPower() { admins++ continue } @@ -677,10 +765,10 @@ func TestDefaultAccountManager_ExternalCache(t *testing.T) { func TestUser_IsAdmin(t *testing.T) { user := NewAdminUser(mockUserID) - assert.True(t, user.IsAdmin()) + assert.True(t, user.HasAdminPower()) user = NewRegularUser(mockUserID) - assert.False(t, user.IsAdmin()) + assert.False(t, user.HasAdminPower()) } func TestUser_GetUsersFromAccount_ForAdmin(t *testing.T) { @@ -746,26 +834,39 @@ func TestDefaultAccountManager_SaveUser(t *testing.T) { } regularUserID := "regularUser" + serviceUserID := "serviceUser" + adminUserID := "adminUser" + ownerUserID := "ownerUser" tt := []struct { - name string - adminInitiator bool - update *User - expectedErr bool + name string + initiatorID string + update *User + expectedErr bool }{ { - name: "Should_Fail_To_Update_Admin_Role", - expectedErr: true, - adminInitiator: true, + name: "Should_Fail_To_Update_Admin_Role", + expectedErr: true, + initiatorID: adminUserID, update: &User{ - Id: userID, + Id: adminUserID, Role: UserRoleUser, Blocked: false, }, }, { - name: "Should_Fail_When_Admin_Blocks_Themselves", - expectedErr: true, - adminInitiator: true, + name: "Should_Fail_When_Admin_Blocks_Themselves", + expectedErr: true, + initiatorID: adminUserID, + update: &User{ + Id: adminUserID, + Role: UserRoleAdmin, + Blocked: true, + }, + }, + { + name: "Should_Fail_To_Update_Non_Existing_User", + expectedErr: true, + initiatorID: adminUserID, update: &User{ Id: userID, Role: UserRoleAdmin, @@ -773,66 +874,125 @@ func TestDefaultAccountManager_SaveUser(t *testing.T) { }, }, { - name: "Should_Fail_To_Update_Non_Existing_User", - expectedErr: true, - adminInitiator: true, + name: "Should_Fail_To_Update_When_Initiator_Is_Not_An_Admin", + expectedErr: true, + initiatorID: regularUserID, update: &User{ - Id: userID, + Id: adminUserID, Role: UserRoleAdmin, Blocked: true, }, }, { - name: "Should_Fail_To_Update_When_Initiator_Is_Not_An_Admin", - expectedErr: true, - adminInitiator: false, - update: &User{ - Id: userID, - Role: UserRoleAdmin, - Blocked: true, - }, - }, - { - name: "Should_Update_User", - expectedErr: false, - adminInitiator: true, + name: "Should_Update_User", + expectedErr: false, + initiatorID: adminUserID, update: &User{ Id: regularUserID, Role: UserRoleAdmin, Blocked: true, }, }, + { + name: "Should_Transfer_Owner_Role_To_User", + expectedErr: false, + initiatorID: ownerUserID, + update: &User{ + Id: adminUserID, + Role: UserRoleAdmin, + Blocked: false, + }, + }, + { + name: "Should_Fail_To_Transfer_Owner_Role_To_Service_User", + expectedErr: true, + initiatorID: ownerUserID, + update: &User{ + Id: serviceUserID, + Role: UserRoleOwner, + Blocked: false, + }, + }, + { + name: "Should_Fail_To_Update_Owner_User_Role_By_Admin", + expectedErr: true, + initiatorID: adminUserID, + update: &User{ + Id: ownerUserID, + Role: UserRoleAdmin, + Blocked: false, + }, + }, + { + name: "Should_Fail_To_Update_Owner_User_Role_By_User", + expectedErr: true, + initiatorID: regularUserID, + update: &User{ + Id: ownerUserID, + Role: UserRoleAdmin, + Blocked: false, + }, + }, + { + name: "Should_Fail_To_Update_Owner_User_Role_By_Service_User", + expectedErr: true, + initiatorID: serviceUserID, + update: &User{ + Id: ownerUserID, + Role: UserRoleAdmin, + Blocked: false, + }, + }, + { + name: "Should_Fail_To_Update_Owner_Role_By_Admin", + expectedErr: true, + initiatorID: adminUserID, + update: &User{ + Id: regularUserID, + Role: UserRoleOwner, + Blocked: false, + }, + }, + { + name: "Should_Fail_To_Block_Owner_Role_By_Admin", + expectedErr: true, + initiatorID: adminUserID, + update: &User{ + Id: ownerUserID, + Role: UserRoleOwner, + Blocked: true, + }, + }, } for _, tc := range tt { + t.Run(tc.name, func(t *testing.T) { - // create an account and an admin user - account, err := manager.GetOrCreateAccountByUser(userID, "netbird.io") - if err != nil { - t.Fatal(err) - } + // create an account and an admin user + account, err := manager.GetOrCreateAccountByUser(ownerUserID, "netbird.io") + if err != nil { + t.Fatal(err) + } - // create a regular user - account.Users[regularUserID] = NewRegularUser(regularUserID) - err = manager.Store.SaveAccount(account) - if err != nil { - t.Fatal(err) - } + // create other users + account.Users[regularUserID] = NewRegularUser(regularUserID) + account.Users[adminUserID] = NewAdminUser(adminUserID) + account.Users[serviceUserID] = &User{IsServiceUser: true, Id: serviceUserID, Role: UserRoleAdmin, ServiceUserName: "service"} + err = manager.Store.SaveAccount(account) + if err != nil { + t.Fatal(err) + } - initiatorID := userID - if !tc.adminInitiator { - initiatorID = regularUserID - } + updated, err := manager.SaveUser(account.Id, tc.initiatorID, tc.update) + if tc.expectedErr { + require.Errorf(t, err, "expecting SaveUser to throw an error") + } else { + require.NoError(t, err, "expecting SaveUser not to throw an error") + assert.NotNil(t, updated) - updated, err := manager.SaveUser(account.Id, initiatorID, tc.update) - if tc.expectedErr { - require.Errorf(t, err, "expecting SaveUser to throw an error") - } else { - require.NoError(t, err, "expecting SaveUser not to throw an error") - assert.NotNil(t, updated) - - assert.Equal(t, string(tc.update.Role), updated.Role) - assert.Equal(t, tc.update.IsBlocked(), updated.IsBlocked) - } + assert.Equal(t, string(tc.update.Role), updated.Role) + assert.Equal(t, tc.update.IsBlocked(), updated.IsBlocked) + } + }) } }