Merge pull request #5065 from rhafer/issue/5045

Allow initial self-assignemnt of UserRole

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

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,

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)

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",