From 7b64953eedfb271f64fe290a112f3e41cb832fe6 Mon Sep 17 00:00:00 2001 From: Pedro Maia Costa <550684+pnmcosta@users.noreply.github.com> Date: Thu, 1 May 2025 11:24:55 +0100 Subject: [PATCH] [management] user info with role permissions (#3728) --- management/client/rest/users_test.go | 7 +- management/server/account/manager.go | 3 +- management/server/http/api/openapi.yml | 24 +- management/server/http/api/types.gen.go | 15 +- .../http/handlers/users/users_handler.go | 30 +- .../http/handlers/users/users_handler_test.go | 197 +++++++++--- management/server/mock_server/account_mock.go | 7 +- management/server/peer.go | 2 +- management/server/permissions/manager.go | 21 ++ management/server/permissions/manager_mock.go | 15 + .../server/permissions/modules/module.go | 16 + .../server/permissions/roles/network_admin.go | 12 +- management/server/types/user.go | 26 +- management/server/user.go | 60 ++-- management/server/user_test.go | 288 ++++++++---------- management/server/users/user.go | 14 + 16 files changed, 446 insertions(+), 291 deletions(-) create mode 100644 management/server/users/user.go diff --git a/management/client/rest/users_test.go b/management/client/rest/users_test.go index f68c5f083..715eb1661 100644 --- a/management/client/rest/users_test.go +++ b/management/client/rest/users_test.go @@ -30,11 +30,8 @@ var ( Issued: ptr("api"), LastLogin: &time.Time{}, Name: "M. Essam", - Permissions: &api.UserPermissions{ - DashboardView: ptr(api.UserPermissionsDashboardViewFull), - }, - Role: "user", - Status: api.UserStatusActive, + Role: "user", + Status: api.UserStatusActive, } ) diff --git a/management/server/account/manager.go b/management/server/account/manager.go index aed83349f..9bc4f9605 100644 --- a/management/server/account/manager.go +++ b/management/server/account/manager.go @@ -16,6 +16,7 @@ import ( "github.com/netbirdio/netbird/management/server/posture" "github.com/netbirdio/netbird/management/server/store" "github.com/netbirdio/netbird/management/server/types" + "github.com/netbirdio/netbird/management/server/users" "github.com/netbirdio/netbird/route" ) @@ -115,5 +116,5 @@ type Manager interface { CreateAccountByPrivateDomain(ctx context.Context, initiatorId, domain string) (*types.Account, error) UpdateToPrimaryAccount(ctx context.Context, accountId string) (*types.Account, error) GetOwnerInfo(ctx context.Context, accountId string) (*types.UserInfo, error) - GetCurrentUserInfo(ctx context.Context, accountID, userID string) (*types.UserInfo, error) + GetCurrentUserInfo(ctx context.Context, userAuth nbcontext.UserAuth) (*users.UserInfoWithPermissions, error) } diff --git a/management/server/http/api/openapi.yml b/management/server/http/api/openapi.yml index 51ffd65b2..bf40777fc 100644 --- a/management/server/http/api/openapi.yml +++ b/management/server/http/api/openapi.yml @@ -216,11 +216,25 @@ components: UserPermissions: type: object properties: - dashboard_view: - description: User's permission to view the dashboard - type: string - enum: [ "limited", "blocked", "full" ] - example: limited + is_restricted: + type: boolean + description: Indicates whether this User's Peers view is restricted + modules: + type: object + additionalProperties: + type: object + additionalProperties: + type: boolean + propertyNames: + type: string + description: The operation type + propertyNames: + type: string + description: The module name + example: {"networks": { "read": true, "create": false, "update": false, "delete": false}, "peers": { "read": false, "create": false, "update": false, "delete": false} } + required: + - modules + - is_restricted UserRequest: type: object properties: diff --git a/management/server/http/api/types.gen.go b/management/server/http/api/types.gen.go index e01275b99..e108c6884 100644 --- a/management/server/http/api/types.gen.go +++ b/management/server/http/api/types.gen.go @@ -178,13 +178,6 @@ const ( UserStatusInvited UserStatus = "invited" ) -// Defines values for UserPermissionsDashboardView. -const ( - UserPermissionsDashboardViewBlocked UserPermissionsDashboardView = "blocked" - UserPermissionsDashboardViewFull UserPermissionsDashboardView = "full" - UserPermissionsDashboardViewLimited UserPermissionsDashboardView = "limited" -) - // Defines values for GetApiEventsNetworkTrafficParamsType. const ( GetApiEventsNetworkTrafficParamsTypeTYPEDROP GetApiEventsNetworkTrafficParamsType = "TYPE_DROP" @@ -1757,13 +1750,11 @@ type UserCreateRequest struct { // UserPermissions defines model for UserPermissions. type UserPermissions struct { - // DashboardView User's permission to view the dashboard - DashboardView *UserPermissionsDashboardView `json:"dashboard_view,omitempty"` + // IsRestricted Indicates whether this User's Peers view is restricted + IsRestricted bool `json:"is_restricted"` + Modules map[string]map[string]bool `json:"modules"` } -// UserPermissionsDashboardView User's permission to view the dashboard -type UserPermissionsDashboardView string - // UserRequest defines model for UserRequest. type UserRequest struct { // AutoGroups Group IDs to auto-assign to peers registered by this user diff --git a/management/server/http/handlers/users/users_handler.go b/management/server/http/handlers/users/users_handler.go index c69c6b944..ac04b8e35 100644 --- a/management/server/http/handlers/users/users_handler.go +++ b/management/server/http/handlers/users/users_handler.go @@ -13,6 +13,7 @@ import ( "github.com/netbirdio/netbird/management/server/http/util" "github.com/netbirdio/netbird/management/server/status" "github.com/netbirdio/netbird/management/server/types" + "github.com/netbirdio/netbird/management/server/users" nbcontext "github.com/netbirdio/netbird/management/server/context" ) @@ -272,15 +273,33 @@ func (h *handler) getCurrentUser(w http.ResponseWriter, r *http.Request) { return } - accountID, userID := userAuth.AccountId, userAuth.UserId - - user, err := h.accountManager.GetCurrentUserInfo(ctx, accountID, userID) + user, err := h.accountManager.GetCurrentUserInfo(ctx, userAuth) if err != nil { util.WriteError(r.Context(), err, w) return } - util.WriteJSONObject(r.Context(), w, toUserResponse(user, userID)) + util.WriteJSONObject(r.Context(), w, toUserWithPermissionsResponse(user, userAuth.UserId)) +} + +func toUserWithPermissionsResponse(user *users.UserInfoWithPermissions, userID string) *api.User { + response := toUserResponse(user.UserInfo, userID) + + // stringify modules and operations keys + modules := make(map[string]map[string]bool) + for module, operations := range user.Permissions { + modules[string(module)] = make(map[string]bool) + for op, val := range operations { + modules[string(module)][string(op)] = val + } + } + + response.Permissions = &api.UserPermissions{ + IsRestricted: user.Restricted, + Modules: modules, + } + + return response } func toUserResponse(user *types.UserInfo, currenUserID string) *api.User { @@ -316,8 +335,5 @@ func toUserResponse(user *types.UserInfo, currenUserID string) *api.User { IsBlocked: user.IsBlocked, LastLogin: &user.LastLogin, Issued: &user.Issued, - Permissions: &api.UserPermissions{ - DashboardView: (*api.UserPermissionsDashboardView)(&user.Permissions.DashboardView), - }, } } diff --git a/management/server/http/handlers/users/users_handler_test.go b/management/server/http/handlers/users/users_handler_test.go index 604954819..58e33a6d5 100644 --- a/management/server/http/handlers/users/users_handler_test.go +++ b/management/server/http/handlers/users/users_handler_test.go @@ -13,12 +13,16 @@ import ( "github.com/gorilla/mux" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" nbcontext "github.com/netbirdio/netbird/management/server/context" "github.com/netbirdio/netbird/management/server/http/api" "github.com/netbirdio/netbird/management/server/mock_server" + "github.com/netbirdio/netbird/management/server/permissions/modules" + "github.com/netbirdio/netbird/management/server/permissions/roles" "github.com/netbirdio/netbird/management/server/status" "github.com/netbirdio/netbird/management/server/types" + "github.com/netbirdio/netbird/management/server/users" ) const ( @@ -107,7 +111,7 @@ func initUsersTestData() *handler { return nil, status.Errorf(status.NotFound, "user with ID %s does not exists", userID) } - info, err := update.Copy().ToUserInfo(nil, &types.Settings{RegularUsersViewBlocked: false}) + info, err := update.Copy().ToUserInfo(nil) if err != nil { return nil, err } @@ -124,8 +128,8 @@ func initUsersTestData() *handler { return nil }, - GetCurrentUserInfoFunc: func(ctx context.Context, accountID, userID string) (*types.UserInfo, error) { - switch userID { + GetCurrentUserInfoFunc: func(ctx context.Context, userAuth nbcontext.UserAuth) (*users.UserInfoWithPermissions, error) { + switch userAuth.UserId { case "not-found": return nil, status.NewUserNotFoundError("not-found") case "not-of-account": @@ -135,52 +139,68 @@ func initUsersTestData() *handler { case "service-user": return nil, status.NewPermissionDeniedError() case "owner": - return &types.UserInfo{ - ID: "owner", - Name: "", - Role: "owner", - Status: "active", - IsServiceUser: false, - IsBlocked: false, - NonDeletable: false, - Issued: "api", - Permissions: types.UserPermissions{ - DashboardView: "full", + return &users.UserInfoWithPermissions{ + UserInfo: &types.UserInfo{ + ID: "owner", + Name: "", + Role: "owner", + Status: "active", + IsServiceUser: false, + IsBlocked: false, + NonDeletable: false, + Issued: "api", }, + Permissions: mergeRolePermissions(roles.Owner), }, nil case "regular-user": - return &types.UserInfo{ - ID: "regular-user", - Name: "", - Role: "user", - Status: "active", - IsServiceUser: false, - IsBlocked: false, - NonDeletable: false, - Issued: "api", - Permissions: types.UserPermissions{ - DashboardView: "limited", + return &users.UserInfoWithPermissions{ + UserInfo: &types.UserInfo{ + ID: "regular-user", + Name: "", + Role: "user", + Status: "active", + IsServiceUser: false, + IsBlocked: false, + NonDeletable: false, + Issued: "api", }, + Permissions: mergeRolePermissions(roles.User), }, nil case "admin-user": - return &types.UserInfo{ - ID: "admin-user", - Name: "", - Role: "admin", - Status: "active", - IsServiceUser: false, - IsBlocked: false, - NonDeletable: false, - LastLogin: time.Time{}, - Issued: "api", - Permissions: types.UserPermissions{ - DashboardView: "full", + return &users.UserInfoWithPermissions{ + UserInfo: &types.UserInfo{ + ID: "admin-user", + Name: "", + Role: "admin", + Status: "active", + IsServiceUser: false, + IsBlocked: false, + NonDeletable: false, + LastLogin: time.Time{}, + Issued: "api", }, + Permissions: mergeRolePermissions(roles.Admin), + }, nil + case "restricted-user": + return &users.UserInfoWithPermissions{ + UserInfo: &types.UserInfo{ + ID: "restricted-user", + Name: "", + Role: "user", + Status: "active", + IsServiceUser: false, + IsBlocked: false, + NonDeletable: false, + LastLogin: time.Time{}, + Issued: "api", + }, + Permissions: mergeRolePermissions(roles.User), + Restricted: true, }, nil } - return nil, fmt.Errorf("user id %s not handled", userID) + return nil, fmt.Errorf("user id %s not handled", userAuth.UserId) }, }, } @@ -546,6 +566,7 @@ func TestCurrentUser(t *testing.T) { name string expectedStatus int requestAuth nbcontext.UserAuth + expectedResult *api.User }{ { name: "without auth", @@ -575,16 +596,78 @@ func TestCurrentUser(t *testing.T) { name: "owner", requestAuth: nbcontext.UserAuth{UserId: "owner"}, expectedStatus: http.StatusOK, + expectedResult: &api.User{ + Id: "owner", + Role: "owner", + Status: "active", + IsBlocked: false, + IsCurrent: ptr(true), + IsServiceUser: ptr(false), + AutoGroups: []string{}, + Issued: ptr("api"), + LastLogin: ptr(time.Time{}), + Permissions: &api.UserPermissions{ + Modules: stringifyPermissionsKeys(mergeRolePermissions(roles.Owner)), + }, + }, }, { name: "regular user", requestAuth: nbcontext.UserAuth{UserId: "regular-user"}, expectedStatus: http.StatusOK, + expectedResult: &api.User{ + Id: "regular-user", + Role: "user", + Status: "active", + IsBlocked: false, + IsCurrent: ptr(true), + IsServiceUser: ptr(false), + AutoGroups: []string{}, + Issued: ptr("api"), + LastLogin: ptr(time.Time{}), + Permissions: &api.UserPermissions{ + Modules: stringifyPermissionsKeys(mergeRolePermissions(roles.User)), + }, + }, }, { name: "admin user", requestAuth: nbcontext.UserAuth{UserId: "admin-user"}, expectedStatus: http.StatusOK, + expectedResult: &api.User{ + Id: "admin-user", + Role: "admin", + Status: "active", + IsBlocked: false, + IsCurrent: ptr(true), + IsServiceUser: ptr(false), + AutoGroups: []string{}, + Issued: ptr("api"), + LastLogin: ptr(time.Time{}), + Permissions: &api.UserPermissions{ + Modules: stringifyPermissionsKeys(mergeRolePermissions(roles.Admin)), + }, + }, + }, + { + name: "restricted user", + requestAuth: nbcontext.UserAuth{UserId: "restricted-user"}, + expectedStatus: http.StatusOK, + expectedResult: &api.User{ + Id: "restricted-user", + Role: "user", + Status: "active", + IsBlocked: false, + IsCurrent: ptr(true), + IsServiceUser: ptr(false), + AutoGroups: []string{}, + Issued: ptr("api"), + LastLogin: ptr(time.Time{}), + Permissions: &api.UserPermissions{ + IsRestricted: true, + Modules: stringifyPermissionsKeys(mergeRolePermissions(roles.User)), + }, + }, }, } @@ -603,10 +686,42 @@ func TestCurrentUser(t *testing.T) { res := rr.Result() defer res.Body.Close() - if status := rr.Code; status != tc.expectedStatus { - t.Fatalf("handler returned wrong status code: got %v want %v", - status, tc.expectedStatus) + assert.Equal(t, tc.expectedStatus, rr.Code, "handler returned wrong status code") + + if tc.expectedResult != nil { + var result api.User + require.NoError(t, json.NewDecoder(res.Body).Decode(&result)) + assert.EqualValues(t, *tc.expectedResult, result) } }) } } + +func ptr[T any, PT *T](x T) PT { + return &x +} + +func mergeRolePermissions(role roles.RolePermissions) roles.Permissions { + permissions := roles.Permissions{} + + for k := range modules.All { + if rolePermissions, ok := role.Permissions[k]; ok { + permissions[k] = rolePermissions + continue + } + permissions[k] = role.AutoAllowNew + } + + return permissions +} + +func stringifyPermissionsKeys(permissions roles.Permissions) map[string]map[string]bool { + modules := make(map[string]map[string]bool) + for module, operations := range permissions { + modules[string(module)] = make(map[string]bool) + for op, val := range operations { + modules[string(module)][string(op)] = val + } + } + return modules +} diff --git a/management/server/mock_server/account_mock.go b/management/server/mock_server/account_mock.go index 2b57e6888..0dd3f927e 100644 --- a/management/server/mock_server/account_mock.go +++ b/management/server/mock_server/account_mock.go @@ -19,6 +19,7 @@ import ( "github.com/netbirdio/netbird/management/server/posture" "github.com/netbirdio/netbird/management/server/store" "github.com/netbirdio/netbird/management/server/types" + "github.com/netbirdio/netbird/management/server/users" "github.com/netbirdio/netbird/route" ) @@ -115,7 +116,7 @@ type MockAccountManager struct { CreateAccountByPrivateDomainFunc func(ctx context.Context, initiatorId, domain string) (*types.Account, error) UpdateToPrimaryAccountFunc func(ctx context.Context, accountId string) (*types.Account, error) GetOwnerInfoFunc func(ctx context.Context, accountID string) (*types.UserInfo, error) - GetCurrentUserInfoFunc func(ctx context.Context, accountID, userID string) (*types.UserInfo, error) + GetCurrentUserInfoFunc func(ctx context.Context, userAuth nbcontext.UserAuth) (*users.UserInfoWithPermissions, error) GetAccountMetaFunc func(ctx context.Context, accountID, userID string) (*types.AccountMeta, error) } @@ -882,9 +883,9 @@ func (am *MockAccountManager) GetOwnerInfo(ctx context.Context, accountId string return nil, status.Errorf(codes.Unimplemented, "method GetOwnerInfo is not implemented") } -func (am *MockAccountManager) GetCurrentUserInfo(ctx context.Context, accountID, userID string) (*types.UserInfo, error) { +func (am *MockAccountManager) GetCurrentUserInfo(ctx context.Context, userAuth nbcontext.UserAuth) (*users.UserInfoWithPermissions, error) { if am.GetCurrentUserInfoFunc != nil { - return am.GetCurrentUserInfoFunc(ctx, accountID, userID) + return am.GetCurrentUserInfoFunc(ctx, userAuth) } return nil, status.Errorf(codes.Unimplemented, "method GetCurrentUserInfo is not implemented") } diff --git a/management/server/peer.go b/management/server/peer.go index a4210e3f0..9ff80442e 100644 --- a/management/server/peer.go +++ b/management/server/peer.go @@ -59,7 +59,7 @@ func (am *DefaultAccountManager) GetPeers(ctx context.Context, accountID, userID return nil, fmt.Errorf("failed to get account settings: %w", err) } - if settings.RegularUsersViewBlocked { + if user.IsRestrictable() && settings.RegularUsersViewBlocked { return []*nbpeer.Peer{}, nil } diff --git a/management/server/permissions/manager.go b/management/server/permissions/manager.go index 50a44eb0f..ebbce5d4a 100644 --- a/management/server/permissions/manager.go +++ b/management/server/permissions/manager.go @@ -20,6 +20,8 @@ type Manager interface { ValidateUserPermissions(ctx context.Context, accountID, userID string, module modules.Module, operation operations.Operation) (bool, error) ValidateRoleModuleAccess(ctx context.Context, accountID string, role roles.RolePermissions, module modules.Module, operation operations.Operation) bool ValidateAccountAccess(ctx context.Context, accountID string, user *types.User, allowOwnerAndAdmin bool) error + + GetPermissionsByRole(ctx context.Context, role types.UserRole) (roles.Permissions, error) } type managerImpl struct { @@ -96,3 +98,22 @@ func (m *managerImpl) ValidateAccountAccess(ctx context.Context, accountID strin } return nil } + +func (m *managerImpl) GetPermissionsByRole(ctx context.Context, role types.UserRole) (roles.Permissions, error) { + roleMap, ok := roles.RolesMap[role] + if !ok { + return roles.Permissions{}, status.NewUserRoleNotFoundError(string(role)) + } + + permissions := roles.Permissions{} + + for k := range modules.All { + if rolePermissions, ok := roleMap.Permissions[k]; ok { + permissions[k] = rolePermissions + continue + } + permissions[k] = roleMap.AutoAllowNew + } + + return permissions, nil +} diff --git a/management/server/permissions/manager_mock.go b/management/server/permissions/manager_mock.go index 266a24270..fa115d628 100644 --- a/management/server/permissions/manager_mock.go +++ b/management/server/permissions/manager_mock.go @@ -38,6 +38,21 @@ func (m *MockManager) EXPECT() *MockManagerMockRecorder { return m.recorder } +// GetPermissionsByRole mocks base method. +func (m *MockManager) GetPermissionsByRole(ctx context.Context, role types.UserRole) (roles.Permissions, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetPermissionsByRole", ctx, role) + ret0, _ := ret[0].(roles.Permissions) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetPermissionsByRole indicates an expected call of GetPermissionsByRole. +func (mr *MockManagerMockRecorder) GetPermissionsByRole(ctx, role interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetPermissionsByRole", reflect.TypeOf((*MockManager)(nil).GetPermissionsByRole), ctx, role) +} + // ValidateAccountAccess mocks base method. func (m *MockManager) ValidateAccountAccess(ctx context.Context, accountID string, user *types.User, allowOwnerAndAdmin bool) error { m.ctrl.T.Helper() diff --git a/management/server/permissions/modules/module.go b/management/server/permissions/modules/module.go index 4c42b6190..3d021a235 100644 --- a/management/server/permissions/modules/module.go +++ b/management/server/permissions/modules/module.go @@ -17,3 +17,19 @@ const ( SetupKeys Module = "setup_keys" Pats Module = "pats" ) + +var All = map[Module]struct{}{ + Networks: {}, + Peers: {}, + Groups: {}, + Settings: {}, + Accounts: {}, + Dns: {}, + Nameservers: {}, + Events: {}, + Policies: {}, + Routes: {}, + Users: {}, + SetupKeys: {}, + Pats: {}, +} diff --git a/management/server/permissions/roles/network_admin.go b/management/server/permissions/roles/network_admin.go index 761933386..e95d58381 100644 --- a/management/server/permissions/roles/network_admin.go +++ b/management/server/permissions/roles/network_admin.go @@ -23,9 +23,9 @@ var NetworkAdmin = RolePermissions{ }, modules.Groups: { operations.Read: true, - operations.Create: false, - operations.Update: false, - operations.Delete: false, + operations.Create: true, + operations.Update: true, + operations.Delete: true, }, modules.Settings: { operations.Read: true, @@ -87,5 +87,11 @@ var NetworkAdmin = RolePermissions{ operations.Update: true, operations.Delete: true, }, + modules.Peers: { + operations.Read: true, + operations.Create: false, + operations.Update: false, + operations.Delete: false, + }, }, } diff --git a/management/server/types/user.go b/management/server/types/user.go index a2596b3cb..783fe14da 100644 --- a/management/server/types/user.go +++ b/management/server/types/user.go @@ -65,11 +65,6 @@ type UserInfo struct { LastLogin time.Time `json:"last_login"` Issued string `json:"issued"` IntegrationReference integration_reference.IntegrationReference `json:"-"` - Permissions UserPermissions `json:"permissions"` -} - -type UserPermissions struct { - DashboardView string `json:"dashboard_view"` } // User represents a user of the system @@ -132,21 +127,18 @@ func (u *User) IsRegularUser() bool { return !u.HasAdminPower() && !u.IsServiceUser } +// IsRestrictable checks whether a user is in a restrictable role. +func (u *User) IsRestrictable() bool { + return u.Role == UserRoleUser || u.Role == UserRoleBillingAdmin +} + // ToUserInfo converts a User object to a UserInfo object. -func (u *User) ToUserInfo(userData *idp.UserData, settings *Settings) (*UserInfo, error) { +func (u *User) ToUserInfo(userData *idp.UserData) (*UserInfo, error) { autoGroups := u.AutoGroups if autoGroups == nil { autoGroups = []string{} } - dashboardViewPermissions := "full" - if !u.HasAdminPower() { - dashboardViewPermissions = "limited" - if settings.RegularUsersViewBlocked { - dashboardViewPermissions = "blocked" - } - } - if userData == nil { return &UserInfo{ ID: u.Id, @@ -159,9 +151,6 @@ func (u *User) ToUserInfo(userData *idp.UserData, settings *Settings) (*UserInfo IsBlocked: u.Blocked, LastLogin: u.GetLastLogin(), Issued: u.Issued, - Permissions: UserPermissions{ - DashboardView: dashboardViewPermissions, - }, }, nil } if userData.ID != u.Id { @@ -184,9 +173,6 @@ func (u *User) ToUserInfo(userData *idp.UserData, settings *Settings) (*UserInfo IsBlocked: u.Blocked, LastLogin: u.GetLastLogin(), Issued: u.Issued, - Permissions: UserPermissions{ - DashboardView: dashboardViewPermissions, - }, }, nil } diff --git a/management/server/user.go b/management/server/user.go index b46ed24cf..44ad3b68f 100644 --- a/management/server/user.go +++ b/management/server/user.go @@ -12,6 +12,7 @@ import ( "github.com/netbirdio/netbird/management/server/activity" nbContext "github.com/netbirdio/netbird/management/server/context" + nbcontext "github.com/netbirdio/netbird/management/server/context" "github.com/netbirdio/netbird/management/server/idp" nbpeer "github.com/netbirdio/netbird/management/server/peer" "github.com/netbirdio/netbird/management/server/permissions/modules" @@ -19,6 +20,7 @@ import ( "github.com/netbirdio/netbird/management/server/status" "github.com/netbirdio/netbird/management/server/store" "github.com/netbirdio/netbird/management/server/types" + "github.com/netbirdio/netbird/management/server/users" "github.com/netbirdio/netbird/management/server/util" ) @@ -122,11 +124,6 @@ func (am *DefaultAccountManager) inviteNewUser(ctx context.Context, accountID, u CreatedAt: time.Now().UTC(), } - settings, err := am.Store.GetAccountSettings(ctx, store.LockingStrengthShare, accountID) - if err != nil { - return nil, err - } - if err = am.Store.SaveUser(ctx, store.LockingStrengthUpdate, newUser); err != nil { return nil, err } @@ -138,7 +135,7 @@ func (am *DefaultAccountManager) inviteNewUser(ctx context.Context, accountID, u am.StoreEvent(ctx, userID, newUser.Id, accountID, activity.UserInvited, nil) - return newUser.ToUserInfo(idpUser, settings) + return newUser.ToUserInfo(idpUser) } // createNewIdpUser validates the invite and creates a new user in the IdP @@ -360,6 +357,7 @@ func (am *DefaultAccountManager) CreatePAT(ctx context.Context, accountID string return nil, err } + // @note this is essential to prevent non admin users with Pats create permission frpm creating one for a service user if initiatorUserID != targetUserID && !(initiatorUser.HasAdminPower() && targetUser.IsServiceUser) { return nil, status.NewAdminPermissionError() } @@ -727,19 +725,14 @@ func handleOwnerRoleTransfer(ctx context.Context, transaction store.Store, initi // If the AccountManager has a non-nil idpManager and the User is not a service user, // it will attempt to look up the UserData from the cache. func (am *DefaultAccountManager) getUserInfo(ctx context.Context, user *types.User, accountID string) (*types.UserInfo, error) { - settings, err := am.Store.GetAccountSettings(ctx, store.LockingStrengthShare, accountID) - if err != nil { - return nil, err - } - if !isNil(am.idpManager) && !user.IsServiceUser { userData, err := am.lookupUserInCache(ctx, user.Id, accountID) if err != nil { return nil, err } - return user.ToUserInfo(userData, settings) + return user.ToUserInfo(userData) } - return user.ToUserInfo(nil, settings) + return user.ToUserInfo(nil) } // validateUserUpdate validates the update operation for a user. @@ -879,17 +872,12 @@ func (am *DefaultAccountManager) BuildUserInfosForAccount(ctx context.Context, a queriedUsers = append(queriedUsers, usersFromIntegration...) } - settings, err := am.Store.GetAccountSettings(ctx, store.LockingStrengthShare, accountID) - if err != nil { - return nil, err - } - userInfosMap := make(map[string]*types.UserInfo) // 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 accountUsers { - info, err := accountUser.ToUserInfo(nil, settings) + info, err := accountUser.ToUserInfo(nil) if err != nil { return nil, err } @@ -902,7 +890,7 @@ func (am *DefaultAccountManager) BuildUserInfosForAccount(ctx context.Context, a for _, localUser := range accountUsers { var info *types.UserInfo if queriedUser, contains := findUserInIDPUserdata(localUser.Id, queriedUsers); contains { - info, err = localUser.ToUserInfo(queriedUser, settings) + info, err = localUser.ToUserInfo(queriedUser) if err != nil { return nil, err } @@ -912,14 +900,6 @@ func (am *DefaultAccountManager) BuildUserInfosForAccount(ctx context.Context, a name = localUser.ServiceUserName } - dashboardViewPermissions := "full" - if !localUser.HasAdminPower() { - dashboardViewPermissions = "limited" - if settings.RegularUsersViewBlocked { - dashboardViewPermissions = "blocked" - } - } - info = &types.UserInfo{ ID: localUser.Id, Email: "", @@ -929,7 +909,6 @@ func (am *DefaultAccountManager) BuildUserInfosForAccount(ctx context.Context, a Status: string(types.UserStatusActive), IsServiceUser: localUser.IsServiceUser, NonDeletable: localUser.NonDeletable, - Permissions: types.UserPermissions{DashboardView: dashboardViewPermissions}, } } userInfosMap[info.ID] = info @@ -1239,8 +1218,10 @@ func validateUserInvite(invite *types.UserInfo) error { return nil } -// GetCurrentUserInfo retrieves the account's current user info -func (am *DefaultAccountManager) GetCurrentUserInfo(ctx context.Context, accountID, userID string) (*types.UserInfo, error) { +// GetCurrentUserInfo retrieves the account's current user info and permissions +func (am *DefaultAccountManager) GetCurrentUserInfo(ctx context.Context, userAuth nbcontext.UserAuth) (*users.UserInfoWithPermissions, error) { + accountID, userID := userAuth.AccountId, userAuth.UserId + user, err := am.Store.GetUserByUserID(ctx, store.LockingStrengthShare, userID) if err != nil { return nil, err @@ -1258,10 +1239,25 @@ func (am *DefaultAccountManager) GetCurrentUserInfo(ctx context.Context, account return nil, err } + settings, err := am.Store.GetAccountSettings(ctx, store.LockingStrengthShare, accountID) + if err != nil { + return nil, err + } + userInfo, err := am.getUserInfo(ctx, user, accountID) if err != nil { return nil, err } - return userInfo, nil + userWithPermissions := &users.UserInfoWithPermissions{ + UserInfo: userInfo, + Restricted: !userAuth.IsChild && user.IsRestrictable() && settings.RegularUsersViewBlocked, + } + + permissions, err := am.permissionsManager.GetPermissionsByRole(ctx, user.Role) + if err == nil { + userWithPermissions.Permissions = permissions + } + + return userWithPermissions, nil } diff --git a/management/server/user_test.go b/management/server/user_test.go index 83c5ac49a..66bdc1683 100644 --- a/management/server/user_test.go +++ b/management/server/user_test.go @@ -13,7 +13,10 @@ import ( nbcache "github.com/netbirdio/netbird/management/server/cache" nbcontext "github.com/netbirdio/netbird/management/server/context" "github.com/netbirdio/netbird/management/server/permissions" + "github.com/netbirdio/netbird/management/server/permissions/modules" + "github.com/netbirdio/netbird/management/server/permissions/roles" "github.com/netbirdio/netbird/management/server/status" + "github.com/netbirdio/netbird/management/server/users" "github.com/netbirdio/netbird/management/server/util" nbpeer "github.com/netbirdio/netbird/management/server/peer" @@ -1020,90 +1023,6 @@ func TestDefaultAccountManager_ListUsers(t *testing.T) { assert.Equal(t, 2, regular) } -func TestDefaultAccountManager_ListUsers_DashboardPermissions(t *testing.T) { - testCases := []struct { - name string - role types.UserRole - limitedViewSettings bool - expectedDashboardPermissions string - }{ - { - name: "Regular user, no limited view settings", - role: types.UserRoleUser, - limitedViewSettings: false, - expectedDashboardPermissions: "limited", - }, - { - name: "Admin user, no limited view settings", - role: types.UserRoleAdmin, - limitedViewSettings: false, - expectedDashboardPermissions: "full", - }, - { - name: "Owner, no limited view settings", - role: types.UserRoleOwner, - limitedViewSettings: false, - expectedDashboardPermissions: "full", - }, - { - name: "Regular user, limited view settings", - role: types.UserRoleUser, - limitedViewSettings: true, - expectedDashboardPermissions: "blocked", - }, - { - name: "Admin user, limited view settings", - role: types.UserRoleAdmin, - limitedViewSettings: true, - expectedDashboardPermissions: "full", - }, - { - name: "Owner, limited view settings", - role: types.UserRoleOwner, - limitedViewSettings: true, - expectedDashboardPermissions: "full", - }, - } - - for _, testCase := range testCases { - t.Run(testCase.name, func(t *testing.T) { - store, cleanup, err := store.NewTestStoreFromSQL(context.Background(), "", t.TempDir()) - if err != nil { - t.Fatalf("Error when creating store: %s", err) - } - t.Cleanup(cleanup) - - account := newAccountWithId(context.Background(), mockAccountID, mockUserID, "") - account.Users["normal_user1"] = types.NewUser("normal_user1", testCase.role, false, false, "", []string{}, types.UserIssuedAPI) - account.Settings.RegularUsersViewBlocked = testCase.limitedViewSettings - delete(account.Users, mockUserID) - - err = store.SaveAccount(context.Background(), account) - if err != nil { - t.Fatalf("Error when saving account: %s", err) - } - - permissionsManager := permissions.NewManager(store) - am := DefaultAccountManager{ - Store: store, - eventStore: &activity.InMemoryEventStore{}, - permissionsManager: permissionsManager, - } - - users, err := am.ListUsers(context.Background(), mockAccountID) - if err != nil { - t.Fatalf("Error when checking user role: %s", err) - } - - assert.Equal(t, 1, len(users)) - - userInfo, _ := users[0].ToUserInfo(nil, account.Settings) - assert.Equal(t, testCase.expectedDashboardPermissions, userInfo.Permissions.DashboardView) - }) - } - -} - func TestDefaultAccountManager_ExternalCache(t *testing.T) { store, cleanup, err := store.NewTestStoreFromSQL(context.Background(), "", t.TempDir()) if err != nil { @@ -1654,121 +1573,154 @@ func TestDefaultAccountManager_GetCurrentUserInfo(t *testing.T) { tt := []struct { name string - accountId string - userId string + userAuth nbcontext.UserAuth expectedErr error - expectedResult *types.UserInfo + expectedResult *users.UserInfoWithPermissions }{ { name: "not found", - accountId: account1.Id, - userId: "not-found", + userAuth: nbcontext.UserAuth{AccountId: account1.Id, UserId: "not-found"}, expectedErr: status.NewUserNotFoundError("not-found"), }, { name: "not part of account", - accountId: account1.Id, - userId: "account2Owner", + userAuth: nbcontext.UserAuth{AccountId: account1.Id, UserId: "account2Owner"}, expectedErr: status.NewUserNotPartOfAccountError(), }, { name: "blocked", - accountId: account1.Id, - userId: "blocked-user", + userAuth: nbcontext.UserAuth{AccountId: account1.Id, UserId: "blocked-user"}, expectedErr: status.NewUserBlockedError(), }, { name: "service user", - accountId: account1.Id, - userId: "service-user", + userAuth: nbcontext.UserAuth{AccountId: account1.Id, UserId: "service-user"}, expectedErr: status.NewPermissionDeniedError(), }, { - name: "owner user", - accountId: account1.Id, - userId: "account1Owner", - expectedResult: &types.UserInfo{ - ID: "account1Owner", - Name: "", - Role: "owner", - AutoGroups: []string{}, - Status: "active", - IsServiceUser: false, - IsBlocked: false, - NonDeletable: false, - LastLogin: time.Time{}, - Issued: "api", - IntegrationReference: integration_reference.IntegrationReference{}, - Permissions: types.UserPermissions{ - DashboardView: "full", + name: "owner user", + userAuth: nbcontext.UserAuth{AccountId: account1.Id, UserId: "account1Owner"}, + expectedResult: &users.UserInfoWithPermissions{ + UserInfo: &types.UserInfo{ + ID: "account1Owner", + Name: "", + Role: "owner", + AutoGroups: []string{}, + Status: "active", + IsServiceUser: false, + IsBlocked: false, + NonDeletable: false, + LastLogin: time.Time{}, + Issued: "api", + IntegrationReference: integration_reference.IntegrationReference{}, }, + Permissions: mergeRolePermissions(roles.Owner), }, }, { - name: "regular user", - accountId: account1.Id, - userId: "regular-user", - expectedResult: &types.UserInfo{ - ID: "regular-user", - Name: "", - Role: "user", - Status: "active", - IsServiceUser: false, - IsBlocked: false, - NonDeletable: false, - LastLogin: time.Time{}, - Issued: "api", - IntegrationReference: integration_reference.IntegrationReference{}, - Permissions: types.UserPermissions{ - DashboardView: "limited", + name: "regular user", + userAuth: nbcontext.UserAuth{AccountId: account1.Id, UserId: "regular-user"}, + expectedResult: &users.UserInfoWithPermissions{ + UserInfo: &types.UserInfo{ + ID: "regular-user", + Name: "", + Role: "user", + Status: "active", + IsServiceUser: false, + IsBlocked: false, + NonDeletable: false, + LastLogin: time.Time{}, + Issued: "api", + IntegrationReference: integration_reference.IntegrationReference{}, }, + Permissions: mergeRolePermissions(roles.User), }, }, { - name: "admin user", - accountId: account1.Id, - userId: "admin-user", - expectedResult: &types.UserInfo{ - ID: "admin-user", - Name: "", - Role: "admin", - Status: "active", - IsServiceUser: false, - IsBlocked: false, - NonDeletable: false, - LastLogin: time.Time{}, - Issued: "api", - IntegrationReference: integration_reference.IntegrationReference{}, - Permissions: types.UserPermissions{ - DashboardView: "full", + name: "admin user", + userAuth: nbcontext.UserAuth{AccountId: account1.Id, UserId: "admin-user"}, + expectedResult: &users.UserInfoWithPermissions{ + UserInfo: &types.UserInfo{ + ID: "admin-user", + Name: "", + Role: "admin", + Status: "active", + IsServiceUser: false, + IsBlocked: false, + NonDeletable: false, + LastLogin: time.Time{}, + Issued: "api", + IntegrationReference: integration_reference.IntegrationReference{}, }, + Permissions: mergeRolePermissions(roles.Admin), }, }, { - name: "settings blocked regular user", - accountId: account2.Id, - userId: "settings-blocked-user", - expectedResult: &types.UserInfo{ - ID: "settings-blocked-user", - Name: "", - Role: "user", - Status: "active", - IsServiceUser: false, - IsBlocked: false, - NonDeletable: false, - LastLogin: time.Time{}, - Issued: "api", - IntegrationReference: integration_reference.IntegrationReference{}, - Permissions: types.UserPermissions{ - DashboardView: "blocked", + name: "settings blocked regular user", + userAuth: nbcontext.UserAuth{AccountId: account2.Id, UserId: "settings-blocked-user"}, + expectedResult: &users.UserInfoWithPermissions{ + UserInfo: &types.UserInfo{ + ID: "settings-blocked-user", + Name: "", + Role: "user", + Status: "active", + IsServiceUser: false, + IsBlocked: false, + NonDeletable: false, + LastLogin: time.Time{}, + Issued: "api", + IntegrationReference: integration_reference.IntegrationReference{}, }, + Permissions: mergeRolePermissions(roles.User), + Restricted: true, + }, + }, + + { + name: "settings blocked regular user child account", + userAuth: nbcontext.UserAuth{AccountId: account2.Id, UserId: "settings-blocked-user", IsChild: true}, + expectedResult: &users.UserInfoWithPermissions{ + UserInfo: &types.UserInfo{ + ID: "settings-blocked-user", + Name: "", + Role: "user", + Status: "active", + IsServiceUser: false, + IsBlocked: false, + NonDeletable: false, + LastLogin: time.Time{}, + Issued: "api", + IntegrationReference: integration_reference.IntegrationReference{}, + }, + Permissions: mergeRolePermissions(roles.User), + Restricted: false, + }, + }, + { + name: "settings blocked owner user", + userAuth: nbcontext.UserAuth{AccountId: account2.Id, UserId: "account2Owner"}, + expectedResult: &users.UserInfoWithPermissions{ + UserInfo: &types.UserInfo{ + ID: "account2Owner", + Name: "", + Role: "owner", + AutoGroups: []string{}, + Status: "active", + IsServiceUser: false, + IsBlocked: false, + NonDeletable: false, + LastLogin: time.Time{}, + Issued: "api", + IntegrationReference: integration_reference.IntegrationReference{}, + }, + Permissions: mergeRolePermissions(roles.Owner), }, }, } for _, tc := range tt { t.Run(tc.name, func(t *testing.T) { - result, err := am.GetCurrentUserInfo(context.Background(), tc.accountId, tc.userId) + result, err := am.GetCurrentUserInfo(context.Background(), tc.userAuth) if tc.expectedErr != nil { assert.Equal(t, err, tc.expectedErr) @@ -1780,3 +1732,17 @@ func TestDefaultAccountManager_GetCurrentUserInfo(t *testing.T) { }) } } + +func mergeRolePermissions(role roles.RolePermissions) roles.Permissions { + permissions := roles.Permissions{} + + for k := range modules.All { + if rolePermissions, ok := role.Permissions[k]; ok { + permissions[k] = rolePermissions + continue + } + permissions[k] = role.AutoAllowNew + } + + return permissions +} diff --git a/management/server/users/user.go b/management/server/users/user.go new file mode 100644 index 000000000..2f2788271 --- /dev/null +++ b/management/server/users/user.go @@ -0,0 +1,14 @@ +package users + +import ( + "github.com/netbirdio/netbird/management/server/permissions/roles" + "github.com/netbirdio/netbird/management/server/types" +) + +// Wrapped UserInfo with Role Permissions +type UserInfoWithPermissions struct { + *types.UserInfo + + Permissions roles.Permissions + Restricted bool +}