Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Allow initial self-assignemnt of UserRole #5065

Merged
merged 1 commit into from
Nov 21, 2022
Merged

Allow initial self-assignemnt of UserRole #5065

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions changelog/unreleased/initial-role-assignment.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
4 changes: 1 addition & 3 deletions services/proxy/pkg/user/backend/cs3.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
23 changes: 16 additions & 7 deletions services/settings/pkg/service/v0/service.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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())
Expand All @@ -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)
Expand Down
46 changes: 36 additions & 10 deletions services/settings/pkg/service/v0/service_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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",
Expand All @@ -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",
Expand Down