Repository: owncloud/ocis
Pull request #5065 (2022-11-16 14:40:23)

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

ownclouders: :boom: Acceptance test [<strong>settingsUITests</strong>](https://drone.owncloud.com/owncloud/ocis/16849/60/1) failed. Further test are cancelled...
<!-- id: pr-5065 -->

sonarcloud[bot]: SonarCloud Quality Gate failed.&nbsp; &nbsp; [![Quality Gate failed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/failed-16px.png 'Quality Gate failed')](https://sonarcloud.io/dashboard?id=owncloud_ocis&pullRequest=5065)

[![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=owncloud_ocis&pullRequest=5065&resolved=false&types=BUG) [![C](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/C-16px.png 'C')](https://sonarcloud.io/project/issues?id=owncloud_ocis&pullRequest=5065&resolved=false&types=BUG) [1 Bug](https://sonarcloud.io/project/issues?id=owncloud_ocis&pullRequest=5065&resolved=false&types=BUG)  
[![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=owncloud_ocis&pullRequest=5065&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=owncloud_ocis&pullRequest=5065&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=owncloud_ocis&pullRequest=5065&resolved=false&types=VULNERABILITY)  
[![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=owncloud_ocis&pullRequest=5065&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=owncloud_ocis&pullRequest=5065&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=owncloud_ocis&pullRequest=5065&resolved=false&types=SECURITY_HOTSPOT)  
[![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=owncloud_ocis&pullRequest=5065&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=owncloud_ocis&pullRequest=5065&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=owncloud_ocis&pullRequest=5065&resolved=false&types=CODE_SMELL)

[![77.8%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/60-16px.png '77.8%')](https://sonarcloud.io/component_measures?id=owncloud_ocis&pullRequest=5065&metric=new_coverage&view=list) [77.8% Coverage](https://sonarcloud.io/component_measures?id=owncloud_ocis&pullRequest=5065&metric=new_coverage&view=list)  
[![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=owncloud_ocis&pullRequest=5065&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=owncloud_ocis&pullRequest=5065&metric=new_duplicated_lines_density&view=list)



Patch:
diff --git a/changelog/unreleased/initial-role-assignment.md b/changelog/unreleased/initial-role-assignment.md
new file mode 100644
index 00000000000..6ca708342cb
--- /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/go.mod b/go.mod
index 6c7985fa624..c7bac66e687 100644
--- a/go.mod
+++ b/go.mod
@@ -21,7 +21,7 @@ require (
 	github.com/go-micro/plugins/v4/client/grpc v1.1.0
 	github.com/go-micro/plugins/v4/events/natsjs v1.1.0
 	github.com/go-micro/plugins/v4/logger/zerolog v1.1.0
-	github.com/go-micro/plugins/v4/registry/consul v1.2.0
+	github.com/go-micro/plugins/v4/registry/consul v1.1.0
 	github.com/go-micro/plugins/v4/registry/etcd v1.2.0
 	github.com/go-micro/plugins/v4/registry/kubernetes v1.1.0
 	github.com/go-micro/plugins/v4/registry/mdns v1.1.0
diff --git a/go.sum b/go.sum
index 4e99fb479f4..b834068b487 100644
--- a/go.sum
+++ b/go.sum
@@ -478,8 +478,8 @@ github.com/go-micro/plugins/v4/events/natsjs v1.1.0 h1:eQ3JbN0mnLuiExBM29pRi/Ehl
 github.com/go-micro/plugins/v4/events/natsjs v1.1.0/go.mod h1:ibNwJyP1Z5y8unW4sryIn8EkdwGkrcaURGwm0DVeMcY=
 github.com/go-micro/plugins/v4/logger/zerolog v1.1.0 h1:sG20ktK9YN/h0TlmQXTuL4/yavxBy5QbzifcVARN5ww=
 github.com/go-micro/plugins/v4/logger/zerolog v1.1.0/go.mod h1:lUEQAHt8I+gJDmx3DbpU7aqwM3+MwFmFRnATxlp650w=
-github.com/go-micro/plugins/v4/registry/consul v1.2.0 h1:nqrTzfWUTWKAy+M+i2FazbHnQn4m77EOtoch57kbCH4=
-github.com/go-micro/plugins/v4/registry/consul v1.2.0/go.mod h1:wTat7/K9XQ+i64VbbcMYFcEwipYfSgJM51HcA/sgsM4=
+github.com/go-micro/plugins/v4/registry/consul v1.1.0 h1:YZQ4ddLOK7YsfyjIHiA0SEDzaDYc6vVXrV+8Z6oF5aY=
+github.com/go-micro/plugins/v4/registry/consul v1.1.0/go.mod h1:57TUMkEggnfVEFoWA5JH/nCYCW/c4oRZHC+Ureecgs8=
 github.com/go-micro/plugins/v4/registry/etcd v1.2.0 h1:tcHlU1GzvX3oZa8WQH8ylMCGie5qD5g98YWTESJjeqQ=
 github.com/go-micro/plugins/v4/registry/etcd v1.2.0/go.mod h1:CQeTHkjN3xMtIQsynaTTquMz2sHEdsTfRIfFzrX7aug=
 github.com/go-micro/plugins/v4/registry/kubernetes v1.1.0 h1:JfkrimhACDviPtKT7Fhqms7oeM9sgy0I37bzvn4AsI4=
diff --git a/services/proxy/pkg/user/backend/cs3.go b/services/proxy/pkg/user/backend/cs3.go
index 997ba33b925..bd848cc73ad 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 33c3cadd085..0b9955381cd 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)


Test patch:
diff --git a/services/settings/pkg/service/v0/service_test.go b/services/settings/pkg/service/v0/service_test.go
index d7536648086..86b4ec479f7 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",