From 11fb3eb3c0d0890c8ffbce348e4e5482a1bb5e15 Mon Sep 17 00:00:00 2001 From: Florian Schade Date: Fri, 21 Jun 2024 13:57:24 +0200 Subject: [PATCH] fix(sharing-ng): permission listings for personal and virtual drive items --- .../fix-sharingng-permission-listings.md | 6 + .../service/v0/api_driveitem_permissions.go | 11 +- services/graph/pkg/service/v0/drives.go | 16 +- .../graph/pkg/service/v0/educationuser.go | 9 +- services/graph/pkg/service/v0/users.go | 11 +- services/graph/pkg/service/v0/utils.go | 2 +- .../apiSharingNg/listPermissions.feature | 183 +++++++++++++++--- 7 files changed, 188 insertions(+), 50 deletions(-) create mode 100644 changelog/unreleased/fix-sharingng-permission-listings.md diff --git a/changelog/unreleased/fix-sharingng-permission-listings.md b/changelog/unreleased/fix-sharingng-permission-listings.md new file mode 100644 index 000000000..671e70431 --- /dev/null +++ b/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 diff --git a/services/graph/pkg/service/v0/api_driveitem_permissions.go b/services/graph/pkg/service/v0/api_driveitem_permissions.go index 45ad50083..6187e4cd7 100644 --- a/services/graph/pkg/service/v0/api_driveitem_permissions.go +++ b/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") } diff --git a/services/graph/pkg/service/v0/drives.go b/services/graph/pkg/service/v0/drives.go index f1274b7b4..28b12ef56 100644 --- a/services/graph/pkg/service/v0/drives.go +++ b/services/graph/pkg/service/v0/drives.go @@ -37,9 +37,11 @@ import ( ) const ( - _spaceTypePersonal = "personal" - _spaceTypeProject = "project" - _spaceStateTrashed = "trashed" + _spaceTypePersonal = "personal" + _spaceTypeProject = "project" + _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 = "a 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 diff --git a/services/graph/pkg/service/v0/educationuser.go b/services/graph/pkg/service/v0/educationuser.go index d63697312..629eead6e 100644 --- a/services/graph/pkg/service/v0/educationuser.go +++ b/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, diff --git a/services/graph/pkg/service/v0/users.go b/services/graph/pkg/service/v0/users.go index b19313baa..f492486a0 100644 --- a/services/graph/pkg/service/v0/users.go +++ b/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, diff --git a/services/graph/pkg/service/v0/utils.go b/services/graph/pkg/service/v0/utils.go index 3daf87b2a..12353b056 100644 --- a/services/graph/pkg/service/v0/utils.go +++ b/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, diff --git a/tests/acceptance/features/apiSharingNg/listPermissions.feature b/tests/acceptance/features/apiSharingNg/listPermissions.feature index ec7f8fd31..a5b2668d8 100644 --- a/tests/acceptance/features/apiSharingNg/listPermissions.feature +++ b/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 "" using root endpoint of the Graph API - Then the HTTP status code should be "400" + 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 "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": { - "type": "object", - "required": [ - "code", - "innererror", - "message" - ], - "properties": { - "code": { - "const": "invalidRequest" - }, - "innererror": { - "type": "object", - "required": [ - "date", - "request-id" - ] - }, - "message": { - "const": "unsupported space type" - } + "@libre.graph.permissions.actions.allowedValues": { + "const": [ + "libre.graph/driveItem/permissions/create", + "libre.graph/driveItem/children/create", + "libre.graph/driveItem/standard/delete", + "libre.graph/driveItem/path/read", + "libre.graph/driveItem/quota/read", + "libre.graph/driveItem/content/read", + "libre.graph/driveItem/upload/create", + "libre.graph/driveItem/permissions/read", + "libre.graph/driveItem/children/read", + "libre.graph/driveItem/versions/read", + "libre.graph/driveItem/deleted/read", + "libre.graph/driveItem/path/update", + "libre.graph/driveItem/permissions/delete", + "libre.graph/driveItem/deleted/delete", + "libre.graph/driveItem/versions/update", + "libre.graph/driveItem/deleted/update", + "libre.graph/driveItem/basic/read", + "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 | - | Shares | + + + Scenario: try to lists the permissions of a Shares 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 "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