Merge pull request #1092 from owncloud/settings-permission-checks

Introduce permission checks for WRITE access via http

changelog/unreleased/settings-permissions.md

@@ -0,0 +1,7 @@
+Bugfix: Permission checks for settings write access
+
+Tags: settings
+
+There were several endpoints with write access to the settings service that were not protected by permission checks. We introduced a generic settings management permission to fix this for now. Will be more fine grained later on.
+
+https://github.com/owncloud/ocis/pull/1092

ocis-pkg/middleware/account.go

@@ -2,6 +2,7 @@ package middleware
 
 import (
 	"context"
+	"encoding/json"
 	"net/http"
 
 	"github.com/cs3org/reva/pkg/token/manager/jwt"
@@ -46,7 +47,9 @@ func ExtractAccountUUID(opts ...account.Option) func(http.Handler) http.Handler
 		return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 			token := r.Header.Get("x-access-token")
 			if len(token) == 0 {
-				next.ServeHTTP(w, r)
+				roleIDsJSON, _ := json.Marshal([]string{})
+				ctx := metadata.Set(r.Context(), RoleIDs, string(roleIDsJSON))
+				next.ServeHTTP(w, r.WithContext(ctx))
 				return
 			}
 

settings/pkg/proto/v0/settings.pb.micro_test.go

@@ -382,7 +382,7 @@ func TestGetBundleHavingFullPermissionsOnAnotherRole(t *testing.T) {
 	defer teardown()
 
 	ctx := metadata.Set(context.Background(), middleware.AccountID, testAccountID)
-	ctx = metadata.Set(ctx, middleware.RoleIDs, getRoleIDAsJSON(svc.BundleUUIDRoleUser))
+	ctx = metadata.Set(ctx, middleware.RoleIDs, getRoleIDAsJSON(svc.BundleUUIDRoleAdmin))
 
 	saveRequest := proto.SaveBundleRequest{
 		Bundle: &bundleStub,
@@ -392,6 +392,8 @@ func TestGetBundleHavingFullPermissionsOnAnotherRole(t *testing.T) {
 	assert.Equal(t, bundleStub.Id, saveResponse.Bundle.Id)
 
 	setFullReadWriteOnBundleForAdmin(ctx, t, bundleStub.Id)
+
+	ctx = metadata.Set(ctx, middleware.RoleIDs, getRoleIDAsJSON(svc.BundleUUIDRoleUser))
 	getRequest := proto.GetBundleRequest{BundleId: bundleStub.Id}
 	getResponse, err := bundleService.GetBundle(ctx, &getRequest)
 	assert.Empty(t, getResponse)

settings/pkg/service/v0/service.go

@@ -72,9 +72,13 @@ func (g Service) RegisterDefaultRoles() {
 // SaveBundle implements the BundleServiceHandler interface
 func (g Service) SaveBundle(ctx context.Context, req *proto.SaveBundleRequest, res *proto.SaveBundleResponse) error {
 	cleanUpResource(ctx, req.Bundle.Resource)
+	if err := g.checkStaticPermissionsByBundleType(ctx, req.Bundle.Type); err != nil {
+		return err
+	}
 	if validationError := validateSaveBundle(req); validationError != nil {
 		return merrors.BadRequest(g.id, "%s", validationError)
 	}
+
 	r, err := g.manager.WriteBundle(req.Bundle)
 	if err != nil {
 		return merrors.BadRequest(g.id, "%s", err)
@@ -164,9 +168,13 @@ func (g Service) getFilteredBundle(roleIDs []string, bundle *proto.Bundle) *prot
 // AddSettingToBundle implements the BundleServiceHandler interface
 func (g Service) AddSettingToBundle(ctx context.Context, req *proto.AddSettingToBundleRequest, res *proto.AddSettingToBundleResponse) error {
 	cleanUpResource(ctx, req.Setting.Resource)
+	if err := g.checkStaticPermissionsByBundleID(ctx, req.BundleId); err != nil {
+		return err
+	}
 	if validationError := validateAddSettingToBundle(req); validationError != nil {
 		return merrors.BadRequest(g.id, "%s", validationError)
 	}
+
 	r, err := g.manager.AddSettingToBundle(req.BundleId, req.Setting)
 	if err != nil {
 		return merrors.BadRequest(g.id, "%s", err)
@@ -177,9 +185,13 @@ func (g Service) AddSettingToBundle(ctx context.Context, req *proto.AddSettingTo
 
 // RemoveSettingFromBundle implements the BundleServiceHandler interface
 func (g Service) RemoveSettingFromBundle(ctx context.Context, req *proto.RemoveSettingFromBundleRequest, _ *empty.Empty) error {
+	if err := g.checkStaticPermissionsByBundleID(ctx, req.BundleId); err != nil {
+		return err
+	}
 	if validationError := validateRemoveSettingFromBundle(req); validationError != nil {
 		return merrors.BadRequest(g.id, "%s", validationError)
 	}
+
 	if err := g.manager.RemoveSettingFromBundle(req.BundleId, req.SettingId); err != nil {
 		return merrors.BadRequest(g.id, "%s", err)
 	}
@@ -297,8 +309,8 @@ func (g Service) ListRoleAssignments(ctx context.Context, req *proto.ListRoleAss
 
 // AssignRoleToUser implements the RoleServiceHandler interface
 func (g Service) AssignRoleToUser(ctx context.Context, req *proto.AssignRoleToUserRequest, res *proto.AssignRoleToUserResponse) error {
-	if !g.hasRoleManagementPermission(ctx) {
-		return merrors.Forbidden(g.id, "the user is not allowed to assign roles")
+	if err := g.checkStaticPermissionsByBundleType(ctx, proto.Bundle_TYPE_ROLE); err != nil {
+		return err
 	}
 
 	req.AccountUuid = getValidatedAccountUUID(ctx, req.AccountUuid)
@@ -315,8 +327,8 @@ func (g Service) AssignRoleToUser(ctx context.Context, req *proto.AssignRoleToUs
 
 // RemoveRoleFromUser implements the RoleServiceHandler interface
 func (g Service) RemoveRoleFromUser(ctx context.Context, req *proto.RemoveRoleFromUserRequest, _ *empty.Empty) error {
-	if !g.hasRoleManagementPermission(ctx) {
-		return merrors.Forbidden(g.id, "the user is not allowed to assign roles")
+	if err := g.checkStaticPermissionsByBundleType(ctx, proto.Bundle_TYPE_ROLE); err != nil {
+		return err
 	}
 
 	if validationError := validateRemoveRoleFromUser(req); validationError != nil {
@@ -406,7 +418,7 @@ func (g Service) getValueWithIdentifier(value *proto.Value) (*proto.ValueWithIde
 	}, nil
 }
 
-func (g Service) hasRoleManagementPermission(ctx context.Context) bool {
+func (g Service) hasStaticPermission(ctx context.Context, permissionID string) bool {
 	roleIDs, ok := roles.ReadRoleIDsFromContext(ctx)
 	if !ok {
 		/**
@@ -420,6 +432,27 @@ func (g Service) hasRoleManagementPermission(ctx context.Context) bool {
 		// tracked as OCIS-454
 		return true
 	}
-	p, err := g.manager.ReadPermissionByID(RoleManagementPermissionID, roleIDs)
+	p, err := g.manager.ReadPermissionByID(permissionID, roleIDs)
 	return err == nil && p != nil
 }
+
+func (g Service) checkStaticPermissionsByBundleID(ctx context.Context, bundleID string) error {
+	bundle, err := g.manager.ReadBundle(bundleID)
+	if err != nil {
+		return merrors.NotFound(g.id, "bundle not found: %s", err)
+	}
+	return g.checkStaticPermissionsByBundleType(ctx, bundle.Type)
+}
+
+func (g Service) checkStaticPermissionsByBundleType(ctx context.Context, bundleType proto.Bundle_Type) error {
+	if bundleType == proto.Bundle_TYPE_ROLE {
+		if !g.hasStaticPermission(ctx, RoleManagementPermissionID) {
+			return merrors.Forbidden(g.id, "user has no role management permission")
+		}
+		return nil
+	}
+	if !g.hasStaticPermission(ctx, SettingsManagementPermissionID) {
+		return merrors.Forbidden(g.id, "user has no settings management permission")
+	}
+	return nil
+}

settings/pkg/service/v0/settings.go

@@ -16,6 +16,11 @@ const (
 	RoleManagementPermissionID string = "a53e601e-571f-4f86-8fec-d4576ef49c62"
 	// RoleManagementPermissionName is the hardcoded setting name for the role management permission
 	RoleManagementPermissionName string = "role-management"
+
+	// SettingsManagementPermissionID is the hardcoded setting UUID for the settings management permission
+	SettingsManagementPermissionID string = "79e13b30-3e22-11eb-bc51-0b9f0bad9a58"
+	// SettingsManagementPermissionName is the hardcoded setting name for the settings management permission
+	SettingsManagementPermissionName string = "settings-management"
 )
 
 // generateBundlesDefaultRoles bootstraps the default roles.
@@ -90,5 +95,24 @@ func generatePermissionRequests() []*settings.AddSettingToBundleRequest {
 				},
 			},
 		},
+		{
+			BundleId: BundleUUIDRoleAdmin,
+			Setting: &settings.Setting{
+				Id:          SettingsManagementPermissionID,
+				Name:        SettingsManagementPermissionName,
+				DisplayName: "Settings Management",
+				Description: "This permission gives full access to everything that is related to settings management.",
+				Resource: &settings.Resource{
+					Type: settings.Resource_TYPE_USER,
+					Id:   "all",
+				},
+				Value: &settings.Setting_PermissionValue{
+					PermissionValue: &settings.Permission{
+						Operation:  settings.Permission_OPERATION_READWRITE,
+						Constraint: settings.Permission_CONSTRAINT_ALL,
+					},
+				},
+			},
+		},
 	}
 }