diff --git a/management/client/rest/users_test.go b/management/client/rest/users_test.go index 7a23ec11c..715eb1661 100644 --- a/management/client/rest/users_test.go +++ b/management/client/rest/users_test.go @@ -30,14 +30,8 @@ var ( Issued: ptr("api"), LastLogin: &time.Time{}, Name: "M. Essam", - Permissions: &api.UserPermissions{ - Default: map[string]bool{ - "read": false, - "write": false, - }, - }, - Role: "user", - Status: api.UserStatusActive, + Role: "user", + Status: api.UserStatusActive, } ) diff --git a/management/server/http/api/openapi.yml b/management/server/http/api/openapi.yml index 82a55ddd7..bf1a37bc3 100644 --- a/management/server/http/api/openapi.yml +++ b/management/server/http/api/openapi.yml @@ -211,17 +211,8 @@ components: - read - write example: {"networks": { "read": true, "write": false}, "peers": { "read": false, "write": false} } - default: - type: object - additionalProperties: - type: boolean - propertyNames: - type: string - enum: - - read - - write required: - - default + - modules - is_restricted UserRequest: type: object diff --git a/management/server/http/api/types.gen.go b/management/server/http/api/types.gen.go index 3c1697339..a030bf2a6 100644 --- a/management/server/http/api/types.gen.go +++ b/management/server/http/api/types.gen.go @@ -1710,11 +1710,9 @@ type UserCreateRequest struct { // UserPermissions defines model for UserPermissions. type UserPermissions struct { - Default map[string]bool `json:"default"` - // IsRestricted Indicates whether this User's Peers view is restricted - IsRestricted bool `json:"is_restricted"` - Modules *map[string]map[string]bool `json:"modules,omitempty"` + IsRestricted bool `json:"is_restricted"` + Modules map[string]map[string]bool `json:"modules"` } // UserRequest defines model for UserRequest. diff --git a/management/server/http/handlers/users/users_handler.go b/management/server/http/handlers/users/users_handler.go index 1aa9dffc9..87a2eae92 100644 --- a/management/server/http/handlers/users/users_handler.go +++ b/management/server/http/handlers/users/users_handler.go @@ -328,45 +328,21 @@ func toRolesResponse(roles map[types.UserRole]roles.RolePermissions) []api.RoleP func toUserWithPermissionsResponse(user *users.UserInfoWithPermissions, userID string) *api.User { response := toUserResponse(user.UserInfo, userID) - response.Permissions = &api.UserPermissions{ - IsRestricted: user.Restricted, - Default: toOperationsMapResponse(user.Permissions.AutoAllowNew), - Modules: toModulesMapResponse(user.Permissions.Permissions), - } - - return response -} - -func toOperationsMapResponse[K ~string, V any](input map[K]V) map[string]V { - if len(input) == 0 { - return nil - } - - result := make(map[string]V, len(input)) - for k, v := range input { - result[string(k)] = v - } - - return result -} - -func toModulesMapResponse[K, L ~string](input map[K]map[L]bool) *map[string]map[string]bool { - if len(input) == 0 { - return nil - } - + // stringify modules and operations keys modules := make(map[string]map[string]bool) - for module, operations := range input { - if converted := toOperationsMapResponse(operations); len(converted) > 0 { - modules[string(module)] = converted + for module, operations := range user.Permissions { + modules[string(module)] = make(map[string]bool) + for op, val := range operations { + modules[string(module)][string(op)] = val } } - if len(modules) == 0 { - return nil + response.Permissions = &api.UserPermissions{ + IsRestricted: user.Restricted, + Modules: modules, } - return &modules + return response } func toUserResponse(user *types.UserInfo, currenUserID string) *api.User { diff --git a/management/server/http/handlers/users/users_handler_test.go b/management/server/http/handlers/users/users_handler_test.go index 94f2df8fc..55a4466cf 100644 --- a/management/server/http/handlers/users/users_handler_test.go +++ b/management/server/http/handlers/users/users_handler_test.go @@ -18,6 +18,7 @@ import ( 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" @@ -149,7 +150,7 @@ func initUsersTestData() *handler { NonDeletable: false, Issued: "api", }, - Permissions: roles.Owner, + Permissions: mergeRolePermissions(roles.Owner), }, nil case "regular-user": return &users.UserInfoWithPermissions{ @@ -163,7 +164,7 @@ func initUsersTestData() *handler { NonDeletable: false, Issued: "api", }, - Permissions: roles.User, + Permissions: mergeRolePermissions(roles.User), }, nil case "admin-user": @@ -179,7 +180,7 @@ func initUsersTestData() *handler { LastLogin: time.Time{}, Issued: "api", }, - Permissions: roles.Admin, + Permissions: mergeRolePermissions(roles.Admin), }, nil case "restricted-user": return &users.UserInfoWithPermissions{ @@ -194,7 +195,7 @@ func initUsersTestData() *handler { LastLogin: time.Time{}, Issued: "api", }, - Permissions: roles.User, + Permissions: mergeRolePermissions(roles.User), Restricted: true, }, nil } @@ -606,13 +607,7 @@ func TestCurrentUser(t *testing.T) { Issued: ptr("api"), LastLogin: ptr(time.Time{}), Permissions: &api.UserPermissions{ - IsRestricted: false, - Default: map[string]bool{ - "read": true, - "create": true, - "update": true, - "delete": true, - }, + Modules: stringifyPermissionsKeys(mergeRolePermissions(roles.Owner)), }, }, }, @@ -631,12 +626,7 @@ func TestCurrentUser(t *testing.T) { Issued: ptr("api"), LastLogin: ptr(time.Time{}), Permissions: &api.UserPermissions{ - Default: map[string]bool{ - "read": false, - "create": false, - "update": false, - "delete": false, - }, + Modules: stringifyPermissionsKeys(mergeRolePermissions(roles.User)), }, }, }, @@ -655,21 +645,7 @@ func TestCurrentUser(t *testing.T) { Issued: ptr("api"), LastLogin: ptr(time.Time{}), Permissions: &api.UserPermissions{ - IsRestricted: false, - Default: map[string]bool{ - "read": true, - "create": true, - "update": true, - "delete": true, - }, - Modules: ptr(map[string]map[string]bool{ - "accounts": { - "read": true, - "create": false, - "update": false, - "delete": false, - }, - }), + Modules: stringifyPermissionsKeys(mergeRolePermissions(roles.Admin)), }, }, }, @@ -689,12 +665,7 @@ func TestCurrentUser(t *testing.T) { LastLogin: ptr(time.Time{}), Permissions: &api.UserPermissions{ IsRestricted: true, - Default: map[string]bool{ - "read": false, - "create": false, - "update": false, - "delete": false, - }, + Modules: stringifyPermissionsKeys(mergeRolePermissions(roles.User)), }, }, }, @@ -729,3 +700,28 @@ func TestCurrentUser(t *testing.T) { 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/permissions/manager.go b/management/server/permissions/manager.go index 5149659f4..30b6b8506 100644 --- a/management/server/permissions/manager.go +++ b/management/server/permissions/manager.go @@ -21,7 +21,7 @@ type Manager interface { 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 - GetRolePermissions(ctx context.Context, role types.UserRole) (roles.RolePermissions, error) + GetRolePermissions(ctx context.Context, role types.UserRole) (roles.Permissions, error) GetPermissions(ctx context.Context) map[types.UserRole]roles.RolePermissions } @@ -67,9 +67,9 @@ func (m *managerImpl) ValidateUserPermissions( return true, nil // this should be replaced by proper granular access role } - role, err := m.GetRolePermissions(ctx, user.Role) - if err != nil { - return false, err + role, ok := roles.RolesMap[user.Role] + if !ok { + return false, status.NewUserRoleNotFoundError(string(user.Role)) } return m.ValidateRoleModuleAccess(ctx, accountID, role, module, operation), nil @@ -100,10 +100,20 @@ func (m *managerImpl) ValidateAccountAccess(ctx context.Context, accountID strin return nil } -func (m *managerImpl) GetRolePermissions(ctx context.Context, role types.UserRole) (roles.RolePermissions, error) { - permissions, ok := roles.RolesMap[role] +func (m *managerImpl) GetRolePermissions(ctx context.Context, role types.UserRole) (roles.Permissions, error) { + roleMap, ok := roles.RolesMap[role] if !ok { - return roles.RolePermissions{}, status.NewUserRoleNotFoundError(string(role)) + 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 6d7403d1c..d862abaf1 100644 --- a/management/server/permissions/manager_mock.go +++ b/management/server/permissions/manager_mock.go @@ -53,10 +53,10 @@ func (mr *MockManagerMockRecorder) GetPermissions(ctx interface{}) *gomock.Call } // GetRolePermissions mocks base method. -func (m *MockManager) GetRolePermissions(ctx context.Context, role types.UserRole) (roles.RolePermissions, error) { +func (m *MockManager) GetRolePermissions(ctx context.Context, role types.UserRole) (roles.Permissions, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "GetRolePermissions", ctx, role) - ret0, _ := ret[0].(roles.RolePermissions) + ret0, _ := ret[0].(roles.Permissions) ret1, _ := ret[1].(error) return ret0, ret1 } 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/user_test.go b/management/server/user_test.go index b74b76fd3..9bda09588 100644 --- a/management/server/user_test.go +++ b/management/server/user_test.go @@ -13,6 +13,7 @@ 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" @@ -1619,7 +1620,7 @@ func TestDefaultAccountManager_GetCurrentUserInfo(t *testing.T) { Issued: "api", IntegrationReference: integration_reference.IntegrationReference{}, }, - Permissions: roles.Owner, + Permissions: mergeRolePermissions(roles.Owner), }, }, { @@ -1639,7 +1640,7 @@ func TestDefaultAccountManager_GetCurrentUserInfo(t *testing.T) { Issued: "api", IntegrationReference: integration_reference.IntegrationReference{}, }, - Permissions: roles.User, + Permissions: mergeRolePermissions(roles.User), }, }, { @@ -1659,7 +1660,7 @@ func TestDefaultAccountManager_GetCurrentUserInfo(t *testing.T) { Issued: "api", IntegrationReference: integration_reference.IntegrationReference{}, }, - Permissions: roles.Admin, + Permissions: mergeRolePermissions(roles.Admin), }, }, { @@ -1679,7 +1680,7 @@ func TestDefaultAccountManager_GetCurrentUserInfo(t *testing.T) { Issued: "api", IntegrationReference: integration_reference.IntegrationReference{}, }, - Permissions: roles.User, + Permissions: mergeRolePermissions(roles.User), Restricted: true, }, }, @@ -1701,7 +1702,7 @@ func TestDefaultAccountManager_GetCurrentUserInfo(t *testing.T) { Issued: "api", IntegrationReference: integration_reference.IntegrationReference{}, }, - Permissions: roles.Owner, + Permissions: mergeRolePermissions(roles.Owner), }, }, } @@ -1720,3 +1721,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 index a0d6144ba..2f2788271 100644 --- a/management/server/users/user.go +++ b/management/server/users/user.go @@ -9,6 +9,6 @@ import ( type UserInfoWithPermissions struct { *types.UserInfo - Permissions roles.RolePermissions + Permissions roles.Permissions Restricted bool }