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 00000000000..671e70431fc --- /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 45ad500834c..6187e4cd717 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 f1274b7b41c..28b12ef5679 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 d6369731213..629eead6e33 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 b19313baae0..f492486a010 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 3daf87b2a17..12353b05601 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 ec7f8fd3195..a5b2668d89c 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