Skip to content

Commit

Permalink
Merge pull request #9438 from fschade/fix-sharingng-permission-listings
Browse files Browse the repository at this point in the history 
fix(sharing-ng): permission listings for personal and virtual drive items
  • Loading branch information
@fschade
fschade authored Jun 24, 2024
2 parents 4f1f90d + 11fb3eb commit 77c4f72
Show file tree
Hide file tree
Showing 7 changed files with 188 additions and 50 deletions.
6 changes: 6 additions & 0 deletions changelog/unreleased/fix-sharingng-permission-listings.md
View file
Original file line number Diff line number Diff line change
@@ -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
11 changes: 7 additions & 4 deletions services/graph/pkg/service/v0/api_driveitem_permissions.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,21 +4,23 @@ 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"
userpb "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1"
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"
Expand Down Expand Up @@ -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")
}

Expand Down
16 changes: 9 additions & 7 deletions services/graph/pkg/service/v0/drives.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,11 @@ import (
)

const (
_spaceTypePersonal = "personal"
_spaceTypeProject = "project"
_spaceStateTrashed = "trashed"
_spaceTypePersonal = "personal"
_spaceTypeProject = "project"
_spaceTypeVirtual = "virtual"
_spaceTypeMountpoint = "mountpoint"
_spaceStateTrashed = "trashed"

_sortDescending = "desc"
)
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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"),
Expand Down Expand Up @@ -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
Expand Down
9 changes: 5 additions & 4 deletions services/graph/pkg/service/v0/educationuser.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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,
Expand Down
11 changes: 6 additions & 5 deletions services/graph/pkg/service/v0/users.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand All @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion services/graph/pkg/service/v0/utils.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
183 changes: 154 additions & 29 deletions tests/acceptance/features/apiSharingNg/listPermissions.feature
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
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
Expand Down

0 comments on commit 77c4f72

Please sign in to comment.