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 93364af13..a66c58a62 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 d576aabee..31060b19c 100644 --- a/management/server/http/api/types.gen.go +++ b/management/server/http/api/types.gen.go @@ -1703,11 +1703,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 98074fb2a..673a6936c 100644 --- a/management/server/http/handlers/users/users_handler.go +++ b/management/server/http/handlers/users/users_handler.go @@ -287,35 +287,20 @@ func (h *handler) getCurrentUser(w http.ResponseWriter, r *http.Request) { func toUserWithPermissionsResponse(user *users.UserInfoWithPermissions, userID string) *api.User { response := toUserResponse(user.UserInfo, userID) - permissions := api.UserPermissions{ + // 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, } - if len(user.Permissions.AutoAllowNew) > 0 { - permissions.Default = make(map[string]bool) - for k, v := range user.Permissions.AutoAllowNew { - permissions.Default[string(k)] = v - } - } - - if len(user.Permissions.Permissions) > 0 { - modules := make(map[string]map[string]bool) - for module, operations := range user.Permissions.Permissions { - if len(operations) == 0 { - continue - } - - access := make(map[string]bool) - for k, v := range operations { - access[string(k)] = v - } - modules[string(module)] = access - } - permissions.Modules = &modules - } - - response.Permissions = &permissions - return response } 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 2b81971c8..d12d8d5f6 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) } type managerImpl struct { @@ -66,9 +66,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 @@ -99,10 +99,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 840b007cd..18cc13c31 100644 --- a/management/server/permissions/manager_mock.go +++ b/management/server/permissions/manager_mock.go @@ -39,10 +39,10 @@ func (m *MockManager) EXPECT() *MockManagerMockRecorder { } // 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 }