Merge pull request #9438 from fschade/fix-sharingng-permission-listings

fix(sharing-ng): permission listings for personal and virtual drive items

changelog/unreleased/fix-sharingng-permission-listings.md

@@ -0,0 +1,6 @@
+Bugfix: Fix sharing-ng permission listings for personal and virtual drive items
+
+Fixes an issue where the sharing-ng service was not able to list permissions for personal and virtual drive items.
+
+https://github.com/owncloud/ocis/pull/9438
+https://github.com/owncloud/ocis/issues/8922

services/graph/pkg/service/v0/api_driveitem_permissions.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"net/http"
 	"net/url"
+	"slices"
 
 	gateway "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1"
 	grouppb "github.com/cs3org/go-cs3apis/cs3/identity/group/v1beta1"
@@ -11,14 +12,15 @@ import (
 	collaboration "github.com/cs3org/go-cs3apis/cs3/sharing/collaboration/v1beta1"
 	link "github.com/cs3org/go-cs3apis/cs3/sharing/link/v1beta1"
 	storageprovider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"
+	"github.com/go-chi/chi/v5"
+	"github.com/go-chi/render"
+	libregraph "github.com/owncloud/libre-graph-api-go"
+
 	"github.com/cs3org/reva/v2/pkg/publicshare"
 	"github.com/cs3org/reva/v2/pkg/rgrpc/todo/pool"
 	"github.com/cs3org/reva/v2/pkg/share"
 	"github.com/cs3org/reva/v2/pkg/storagespace"
 	"github.com/cs3org/reva/v2/pkg/utils"
-	"github.com/go-chi/chi/v5"
-	"github.com/go-chi/render"
-	libregraph "github.com/owncloud/libre-graph-api-go"
 
 	"github.com/owncloud/ocis/v2/ocis-pkg/conversions"
 	"github.com/owncloud/ocis/v2/ocis-pkg/log"
@@ -301,7 +303,8 @@ func (s DriveItemPermissionsService) ListSpaceRootPermissions(ctx context.Contex
 		return collectionOfPermissions, errorcode.FromUtilsStatusCodeError(err)
 	}
 
-	if space.SpaceType != _spaceTypeProject {
+	isSupportedSpaceType := slices.Contains([]string{_spaceTypeProject, _spaceTypePersonal, _spaceTypeVirtual}, space.GetSpaceType())
+	if !isSupportedSpaceType {
 		return collectionOfPermissions, errorcode.New(errorcode.InvalidRequest, "unsupported space type")
 	}
 

services/graph/pkg/service/v0/drives.go

@@ -37,9 +37,11 @@ import (
 )
 
 const (
-	_spaceTypePersonal = "personal"
+	_spaceTypePersonal   = "personal"
-	_spaceTypeProject  = "project"
+	_spaceTypeProject    = "project"
-	_spaceStateTrashed = "trashed"
+	_spaceTypeVirtual    = "virtual"
+	_spaceTypeMountpoint = "mountpoint"
+	_spaceStateTrashed   = "trashed"
 
 	_sortDescending = "desc"
 )
@@ -650,9 +652,9 @@ func (g Graph) formatDrives(ctx context.Context, baseURL *url.URL, storageSpaces
 				}
 
 				// can't access disabled space
-				if utils.ReadPlainFromOpaque(storageSpace.Opaque, "trashed") != _spaceStateTrashed {
+				if utils.ReadPlainFromOpaque(storageSpace.Opaque, _spaceStateTrashed) != _spaceStateTrashed {
 					res.Special = g.getSpecialDriveItems(ctx, baseURL, storageSpace)
-					if storageSpace.SpaceType != "mountpoint" && storageSpace.SpaceType != "virtual" {
+					if storageSpace.SpaceType != _spaceTypeMountpoint && storageSpace.SpaceType != _spaceTypeVirtual {
 						quota, err := g.getDriveQuota(ctx, storageSpace)
 						res.Quota = &quota
 						if err != nil {
@@ -759,7 +761,7 @@ func (g Graph) cs3StorageSpaceToDrive(ctx context.Context, baseURL *url.URL, spa
 			Permissions: permissions,
 		},
 	}
-	if space.SpaceType == "mountpoint" {
+	if space.SpaceType == _spaceTypeMountpoint {
 		var remoteItem *libregraph.RemoteItem
 		grantID := storageprovider.ResourceId{
 			StorageId: utils.ReadPlainFromOpaque(space.Opaque, "grantStorageID"),
@@ -787,7 +789,7 @@ func (g Graph) cs3StorageSpaceToDrive(ctx context.Context, baseURL *url.URL, spa
 			drive.DriveAlias = libregraph.PtrString(string(alias.Value))
 		}
 
-		if v, ok := space.Opaque.Map["trashed"]; ok {
+		if v, ok := space.Opaque.Map[_spaceStateTrashed]; ok {
 			deleted := &libregraph.Deleted{}
 			deleted.SetState(string(v.Value))
 			drive.Root.Deleted = deleted

services/graph/pkg/service/v0/educationuser.go

@@ -9,12 +9,13 @@ import (
 
 	"github.com/CiscoM31/godata"
 	storageprovider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"
-	revactx "github.com/cs3org/reva/v2/pkg/ctx"
-	"github.com/cs3org/reva/v2/pkg/events"
-	"github.com/cs3org/reva/v2/pkg/utils"
 	"github.com/go-chi/chi/v5"
 	"github.com/go-chi/render"
 	libregraph "github.com/owncloud/libre-graph-api-go"
+
+	revactx "github.com/cs3org/reva/v2/pkg/ctx"
+	"github.com/cs3org/reva/v2/pkg/events"
+	"github.com/cs3org/reva/v2/pkg/utils"
 	"github.com/owncloud/ocis/v2/services/graph/pkg/errorcode"
 )
 
@@ -249,7 +250,7 @@ func (g Graph) DeleteEducationUser(w http.ResponseWriter, r *http.Request) {
 
 			// Deleting a space a two step process (1. disabling/trashing, 2. purging)
 			// Do the "disable/trash" step only if the space is not marked as trashed yet:
-			if _, ok := sp.Opaque.Map["trashed"]; !ok {
+			if _, ok := sp.Opaque.Map[_spaceStateTrashed]; !ok {
 				_, err := client.DeleteStorageSpace(r.Context(), &storageprovider.DeleteStorageSpaceRequest{
 					Id: &storageprovider.StorageSpaceId{
 						OpaqueId: sp.Id.OpaqueId,

services/graph/pkg/service/v0/users.go

@@ -20,13 +20,14 @@ import (
 	"github.com/CiscoM31/godata"
 	cs3rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1"
 	storageprovider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"
+	"github.com/go-chi/chi/v5"
+	"github.com/go-chi/render"
+	libregraph "github.com/owncloud/libre-graph-api-go"
+
 	revactx "github.com/cs3org/reva/v2/pkg/ctx"
 	"github.com/cs3org/reva/v2/pkg/events"
 	"github.com/cs3org/reva/v2/pkg/rgrpc/status"
 	"github.com/cs3org/reva/v2/pkg/utils"
-	"github.com/go-chi/chi/v5"
-	"github.com/go-chi/render"
-	libregraph "github.com/owncloud/libre-graph-api-go"
 
 	settingsmsg "github.com/owncloud/ocis/v2/protogen/gen/ocis/messages/settings/v0"
 	settings "github.com/owncloud/ocis/v2/protogen/gen/ocis/services/settings/v0"
@@ -640,7 +641,7 @@ func (g Graph) DeleteUser(w http.ResponseWriter, r *http.Request) {
 			return
 		}
 		for _, sp := range lspr.GetStorageSpaces() {
-			if !(sp.SpaceType == "personal" && sp.Owner.Id.OpaqueId == user.GetId()) {
+			if !(sp.SpaceType == _spaceTypePersonal && sp.Owner.Id.OpaqueId == user.GetId()) {
 				continue
 			}
 			// TODO: check if request contains a homespace and if, check if requesting user has the privilege to
@@ -649,7 +650,7 @@ func (g Graph) DeleteUser(w http.ResponseWriter, r *http.Request) {
 
 			// Deleting a space a two step process (1. disabling/trashing, 2. purging)
 			// Do the "disable/trash" step only if the space is not marked as trashed yet:
-			if _, ok := sp.Opaque.Map["trashed"]; !ok {
+			if _, ok := sp.Opaque.Map[_spaceStateTrashed]; !ok {
 				_, err := client.DeleteStorageSpace(r.Context(), &storageprovider.DeleteStorageSpaceRequest{
 					Id: &storageprovider.StorageSpaceId{
 						OpaqueId: sp.Id.OpaqueId,

services/graph/pkg/service/v0/utils.go

@@ -241,7 +241,7 @@ func cs3ReceivedSharesToDriveItems(ctx context.Context,
 			// the parentReference of the outer driveItem should be the drive
 			// containing the mountpoint i.e. the share jail
 			driveItem.ParentReference = libregraph.NewItemReference()
-			driveItem.ParentReference.SetDriveType("virtual")
+			driveItem.ParentReference.SetDriveType(_spaceTypeVirtual)
 			driveItem.ParentReference.SetDriveId(storagespace.FormatStorageID(utils.ShareStorageProviderID, utils.ShareStorageSpaceID))
 			driveItem.ParentReference.SetId(storagespace.FormatResourceID(storageprovider.ResourceId{
 				StorageId: utils.ShareStorageProviderID,

tests/acceptance/features/apiSharingNg/listPermissions.feature

@@ -1215,48 +1215,173 @@ Feature: List a sharing permissions
       """
 
 
-  Scenario Outline: try to lists the permissions of a Personal/Shares drive using root endpoint
+  Scenario: try to lists the permissions of a Personal drive using root endpoint
     Given using spaces DAV path
     And the administrator has assigned the role "Space Admin" to user "Alice" using the Graph API
     And user "Alice" has created a space "new-space" with the default quota using the Graph API
-    When user "Alice" tries to list the permissions of space "<drive>" using root endpoint of the Graph API
+    When user "Alice" tries to list the permissions of space "Personal" using root endpoint of the Graph API
-    Then the HTTP status code should be "400"
+    Then the HTTP status code should be "200"
     And the JSON data of the response should match
       """
       {
         "type": "object",
-        "required": ["error"],
+        "required": [
+          "@libre.graph.permissions.actions.allowedValues",
+          "@libre.graph.permissions.roles.allowedValues"
+        ],
         "properties": {
-          "error": {
+          "@libre.graph.permissions.actions.allowedValues": {
-            "type": "object",
+            "const": [
-            "required": [
+              "libre.graph/driveItem/permissions/create",
-              "code",
+              "libre.graph/driveItem/children/create",
-              "innererror",
+              "libre.graph/driveItem/standard/delete",
-              "message"
+              "libre.graph/driveItem/path/read",
-            ],
+              "libre.graph/driveItem/quota/read",
-            "properties": {
+              "libre.graph/driveItem/content/read",
-              "code": {
+              "libre.graph/driveItem/upload/create",
-                "const": "invalidRequest"
+              "libre.graph/driveItem/permissions/read",
-              },
+              "libre.graph/driveItem/children/read",
-              "innererror": {
+              "libre.graph/driveItem/versions/read",
-                "type": "object",
+              "libre.graph/driveItem/deleted/read",
-                "required": [
+              "libre.graph/driveItem/path/update",
-                  "date",
+              "libre.graph/driveItem/permissions/delete",
-                  "request-id"
+              "libre.graph/driveItem/deleted/delete",
-                ]
+              "libre.graph/driveItem/versions/update",
-              },
+              "libre.graph/driveItem/deleted/update",
-              "message": {
+              "libre.graph/driveItem/basic/read",
-                "const": "unsupported space type"
+              "libre.graph/driveItem/permissions/update",
-              }
+              "libre.graph/driveItem/permissions/deny"
+            ]
+          },
+          "@libre.graph.permissions.roles.allowedValues": {
+            "type": "array",
+            "minItems": 4,
+            "maxItems": 4,
+            "uniqueItems": true,
+            "items": {
+              "oneOf":  [
+                {
+                  "type": "object",
+                  "required": [
+                    "@libre.graph.weight",
+                    "description",
+                    "displayName",
+                    "id"
+                  ],
+                  "properties": {
+                    "@libre.graph.weight": {
+                      "const": 1
+                    },
+                    "description": {
+                      "const": "View only documents, images and PDFs. Watermarks will be applied."
+                    },
+                    "displayName": {
+                      "const": "Can view (secure)"
+                    },
+                    "id": {
+                      "const": "aa97fe03-7980-45ac-9e50-b325749fd7e6"
+                    }
+                  }
+                },
+                {
+                  "type": "object",
+                  "required": [
+                    "@libre.graph.weight",
+                    "description",
+                    "displayName",
+                    "id"
+                  ],
+                  "properties": {
+                    "@libre.graph.weight": {
+                      "const": 2
+                    },
+                    "description": {
+                      "const": "View and download."
+                    },
+                    "displayName": {
+                      "const": "Can view"
+                    },
+                    "id": {
+                      "const": "a8d5fe5e-96e3-418d-825b-534dbdf22b99"
+                    }
+                  }
+                },
+                {
+                  "type": "object",
+                  "required": [
+                    "@libre.graph.weight",
+                    "description",
+                    "displayName",
+                    "id"
+                  ],
+                  "properties": {
+                    "@libre.graph.weight": {
+                      "const": 3
+                    },
+                    "description": {
+                      "const": "View, download, upload, edit, add and delete."
+                    },
+                    "displayName": {
+                      "const": "Can edit"
+                    },
+                    "id": {
+                      "const": "58c63c02-1d89-4572-916a-870abc5a1b7d"
+                    }
+                  }
+                },
+                {
+                  "type": "object",
+                  "required": [
+                    "@libre.graph.weight",
+                    "description",
+                    "displayName",
+                    "id"
+                  ],
+                  "properties": {
+                    "@libre.graph.weight": {
+                      "const": 4
+                    },
+                    "description": {
+                      "const": "View, download, upload, edit, add, delete and manage members."
+                    },
+                    "displayName": {
+                      "const": "Can manage"
+                    },
+                    "id": {
+                      "const": "312c0871-5ef7-4b3a-85b6-0e4074c64049"
+                    }
+                  }
+                }
+              ]
             }
           }
         }
       }
       """
-    Examples:
+
-      | drive    |
+
-      | Personal |
+  Scenario: try to lists the permissions of a Shares drive using root endpoint
-      | Shares   |
+    Given using spaces DAV path
+    And the administrator has assigned the role "Space Admin" to user "Alice" using the Graph API
+    And user "Alice" has created a space "new-space" with the default quota using the Graph API
+    When user "Alice" tries to list the permissions of space "Shares" using root endpoint of the Graph API
+    Then the HTTP status code should be "200"
+    And the JSON data of the response should match
+      """
+      {
+        "type": "object",
+        "required": [
+          "@libre.graph.permissions.roles.allowedValues"
+        ],
+        "properties": {
+          "@libre.graph.permissions.roles.allowedValues": {
+            "type": "array",
+            "minItems": 0,
+            "maxItems": 0
+          }
+        }
+      }
+      """
 
 
   Scenario: space admin invites to a project space with all allowed roles