Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Jun 24, 2024
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions changelog/unreleased/fix-sharingng-permission-listings.md
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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