From 07a78287a7f01cbc553b6d4afd0f547b7be88683 Mon Sep 17 00:00:00 2001 From: Ralf Haferkamp Date: Wed, 16 Nov 2022 15:34:18 +0100 Subject: [PATCH] Allow initial self-assignemnt of UserRole When using an external user management we need to allow users to self-assign the default role. This adds an explicit check for that to the settings service. This also means we no longer need to fiddle with the account id in the proxy upon first login. Fixes: #5045 --- .../unreleased/initial-role-assignment.md | 5 ++ services/proxy/pkg/user/backend/cs3.go | 4 +- services/settings/pkg/service/v0/service.go | 23 +++++++--- .../settings/pkg/service/v0/service_test.go | 46 +++++++++++++++---- 4 files changed, 58 insertions(+), 20 deletions(-) create mode 100644 changelog/unreleased/initial-role-assignment.md diff --git a/changelog/unreleased/initial-role-assignment.md b/changelog/unreleased/initial-role-assignment.md new file mode 100644 index 0000000000..6ca708342c --- /dev/null +++ b/changelog/unreleased/initial-role-assignment.md @@ -0,0 +1,5 @@ +Bugfix: Initial role assingment with external IDM + +We've the initial user role assignment when using an external LDAP server. + +https://github.com/owncloud/ocis/issues/5045 diff --git a/services/proxy/pkg/user/backend/cs3.go b/services/proxy/pkg/user/backend/cs3.go index 997ba33b92..bd848cc73a 100644 --- a/services/proxy/pkg/user/backend/cs3.go +++ b/services/proxy/pkg/user/backend/cs3.go @@ -87,9 +87,7 @@ func (c *cs3backend) GetUserByClaims(ctx context.Context, claim, value string, w // https://github.com/owncloud/ocis/v2/issues/1825 for more context. if user.Id.Type == cs3.UserType_USER_TYPE_PRIMARY { c.logger.Info().Str("userid", user.Id.OpaqueId).Msg("user has no role assigned, assigning default user role") - // Updating context to have the Account-ID field and suffixing with _init - // so that the safety check for setting users' own role doesn't fail - ctx = metadata.Set(ctx, middleware.AccountID, user.Id.OpaqueId+"_init") + ctx = metadata.Set(ctx, middleware.AccountID, user.Id.OpaqueId) _, err := c.settingsRoleService.AssignRoleToUser(ctx, &settingssvc.AssignRoleToUserRequest{ AccountUuid: user.Id.OpaqueId, RoleId: settingsService.BundleUUIDRoleUser, diff --git a/services/settings/pkg/service/v0/service.go b/services/settings/pkg/service/v0/service.go index 33c3cadd08..0b9955381c 100644 --- a/services/settings/pkg/service/v0/service.go +++ b/services/settings/pkg/service/v0/service.go @@ -15,6 +15,7 @@ import ( settingssvc "github.com/owncloud/ocis/v2/protogen/gen/ocis/services/settings/v0" "github.com/owncloud/ocis/v2/services/settings/pkg/config" "github.com/owncloud/ocis/v2/services/settings/pkg/settings" + "github.com/owncloud/ocis/v2/services/settings/pkg/store/defaults" filestore "github.com/owncloud/ocis/v2/services/settings/pkg/store/filesystem" metastore "github.com/owncloud/ocis/v2/services/settings/pkg/store/metadata" merrors "go-micro.dev/v4/errors" @@ -392,10 +393,6 @@ func (g Service) ListRoleAssignments(ctx context.Context, req *settingssvc.ListR // AssignRoleToUser implements the RoleServiceHandler interface func (g Service) AssignRoleToUser(ctx context.Context, req *settingssvc.AssignRoleToUserRequest, res *settingssvc.AssignRoleToUserResponse) error { - if !g.canManageRoles(ctx) { - return merrors.Forbidden(g.id, "user has no role management permission") - } - req.AccountUuid = getValidatedAccountUUID(ctx, req.AccountUuid) if validationError := validateAssignRoleToUser(req); validationError != nil { return merrors.BadRequest(g.id, validationError.Error()) @@ -406,9 +403,21 @@ func (g Service) AssignRoleToUser(ctx context.Context, req *settingssvc.AssignRo g.logger.Debug().Str("id", g.id).Msg("user not in context") return merrors.InternalServerError(g.id, "user not in context") } - if ownAccountUUID == req.AccountUuid { - g.logger.Debug().Str("id", g.id).Msg("Changing own role assignment forbidden") - return merrors.Forbidden(g.id, "Changing own role assignment forbidden") + + switch { + case ownAccountUUID == req.AccountUuid: + // Allow users to assign themself to the user role + // deny any other attempt to change the user's own assignment + if r, err := g.manager.ListRoleAssignments(req.AccountUuid); err == nil && len(r) > 0 { + return merrors.Forbidden(g.id, "Changing own role assignment forbidden") + } + if req.RoleId != defaults.BundleUUIDRoleUser { + return merrors.Forbidden(g.id, "Changing own role assignment forbidden") + } + g.logger.Debug().Str("userid", ownAccountUUID).Msg("Self-assignment for default 'user' role permitted") + case g.canManageRoles(ctx): + default: + return merrors.Forbidden(g.id, "user has no role management permission") } r, err := g.manager.WriteRoleAssignment(req.AccountUuid, req.RoleId) diff --git a/services/settings/pkg/service/v0/service_test.go b/services/settings/pkg/service/v0/service_test.go index d753664808..86b4ec479f 100644 --- a/services/settings/pkg/service/v0/service_test.go +++ b/services/settings/pkg/service/v0/service_test.go @@ -8,6 +8,7 @@ import ( settingsmsg "github.com/owncloud/ocis/v2/protogen/gen/ocis/messages/settings/v0" v0 "github.com/owncloud/ocis/v2/protogen/gen/ocis/services/settings/v0" "github.com/owncloud/ocis/v2/services/settings/pkg/settings/mocks" + "github.com/owncloud/ocis/v2/services/settings/pkg/store/defaults" "github.com/stretchr/testify/assert" "github.com/test-go/testify/mock" "go-micro.dev/v4/metadata" @@ -66,7 +67,35 @@ func TestGetValidatedAccountUUID(t *testing.T) { func TestEditOwnRoleAssignment(t *testing.T) { manager := &mocks.Manager{} - a := []*settingsmsg.UserRoleAssignment{ + svc := Service{ + manager: manager, + } + a := []*settingsmsg.UserRoleAssignment{} + manager.On("ListRoleAssignments", mock.Anything).Return(a, nil) + manager.On("WriteRoleAssignment", mock.Anything, mock.Anything).Return(nil, nil) + // Creating an initial self assignment is expected to succeed for UserRole when no assignment exists yet + req := v0.AssignRoleToUserRequest{ + AccountUuid: "61445573-4dbe-4d56-88dc-88ab47aceba7", + RoleId: defaults.BundleUUIDRoleUser, + } + res := v0.AssignRoleToUserResponse{} + err := svc.AssignRoleToUser(ctxWithUUID, &req, &res) + assert.Nil(t, err) + + // Creating an initial self assignment is expected to fail for non UserRole when no assignment exists yet + req = v0.AssignRoleToUserRequest{ + AccountUuid: "61445573-4dbe-4d56-88dc-88ab47aceba7", + RoleId: defaults.BundleUUIDRoleAdmin, + } + res = v0.AssignRoleToUserResponse{} + err = svc.AssignRoleToUser(ctxWithUUID, &req, &res) + assert.NotNil(t, err) + + manager = &mocks.Manager{} + svc = Service{ + manager: manager, + } + a = []*settingsmsg.UserRoleAssignment{ { Id: "00000000-0000-0000-0000-000000000001", AccountUuid: "61445573-4dbe-4d56-88dc-88ab47aceba7", @@ -79,21 +108,18 @@ func TestEditOwnRoleAssignment(t *testing.T) { } manager.On("ListRoleAssignments", mock.Anything).Return(a, nil) manager.On("ReadPermissionByID", mock.Anything, mock.Anything).Return(editRolePermission, nil) - svc := Service{ - manager: manager, - } - // Creating an self assignment is expect to fail - req := v0.AssignRoleToUserRequest{ + // Creating an self assignment is expect to fail if there is already an assingment + req = v0.AssignRoleToUserRequest{ AccountUuid: "61445573-4dbe-4d56-88dc-88ab47aceba7", - RoleId: "aceb15b8-7486-479f-ae32-c91118e07a39", + RoleId: defaults.BundleUUIDRoleUser, } - res := v0.AssignRoleToUserResponse{} - err := svc.AssignRoleToUser(ctxWithUUID, &req, &res) + res = v0.AssignRoleToUserResponse{} + err = svc.AssignRoleToUser(ctxWithUUID, &req, &res) assert.NotNil(t, err) manager.On("WriteRoleAssignment", mock.Anything, mock.Anything).Return(nil, nil) - // Creating an self assignment is expect to fail + // Creating an assignment for somebody else is expected to succeed, give the right permissions req = v0.AssignRoleToUserRequest{ AccountUuid: "00000000-0000-0000-0000-000000000000", RoleId: "aceb15b8-7486-479f-ae32-c91118e07a39",