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.

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

Any role can be set during share invite request #8131

Closed
amrita-shrestha opened this issue Jan 4, 2024 · 7 comments
Closed

Any role can be set during share invite request #8131

amrita-shrestha opened this issue Jan 4, 2024 · 7 comments
Assignees
Labels
Milestone

Comments

@amrita-shrestha
Copy link
Contributor

amrita-shrestha commented Jan 4, 2024

Describe the bug

Creating a share with roles such as Uploader, Viewer, Editor, Manager, or Co-owner is possible. However, when attempting to send an API request to update roles, a "400 Bad Request" error is encountered.

Steps to reproduce

  1. send share invitation of file.txt with any roles result successful share invite
  2. try to update the role to manager/co-owner/uploader/Editor(for file or folder) results 400 status code
try to change role from Co Owner to Uploader
curl --location --request PATCH 'https://ocis.owncloud.test/graph/v1beta1/drives/drive-id/items/item-id/permissions/permission-id' \
--header 'Authorization: Bearer eyJhbVKA' \
--data '{
    "roles": [
        "1c996275-f1c9-4e71-abdf-a42f6495e960"
    ]
}'
{
    "error": {
        "code": "invalidRequest",
        "innererror": {
            "date": "2024-01-04T11:55:00Z",
            "request-id": "8bce2c34615e/b9ocmAsArN-002634"
        },
        "message": "cannot set the requested permissions on that type of resource"
    }
}

Cases

Role while creating share Role to update Result
Manager/Viewer/Co Owner Editor ✅ Success
Manager/Editor/Co Owner Viewer ✅ Success
Manager/Editor/Viewer/Uploader Co-owner 400 Status
Manager/Co Owner/Editor/Viewer Uploader 400 Status

Expected behavior

We cannot use all roles on all resource types.
The roles have conditions. Some can only be applied to space roots some only to shares.
During share invite, only allowed role should be set

Actual behavior

Can set any role during share invite.
Personal Drive File resource should not allowed to create share invite with role Co owner/Uploader

Setup

Please describe how you started the server and provide a list of relevant environment variables or configuration files.

OCIS_Version= 5.1.0-prealpha+0f9f996ab

Additional context

Add any other context about the problem here.

@micbar
Copy link
Contributor

micbar commented Jan 4, 2024

The expected behavior is not correct.

We cannot use all roles on all resource types.

The roles have conditions. Some can only be applied to space roots some only to shares.

but the clients do not need any Knowledge about that. Before creating or updating a permission, they should check what is possible on that specific resource. This is the needed api call https://owncloud.dev/libre-graph-api/#/drives.permissions/ListPermissions

@amrita-shrestha
Copy link
Contributor Author

amrita-shrestha commented Jan 5, 2024

@micbar
If roles are specified according to resource type then during share invite user should not allowed to set role that resource doesn't support. Isn't it?

Resource Type Role Allowed Role Not Allowed
Personal Drive File Editor/Viewer Manager/Co Owner/Uploader/Editor(File or Folder)
Personal Drive Folder Uploader/Viewer/Editor(File or Folder) Manager/Co owner

@amrita-shrestha amrita-shrestha changed the title Update role gives 400 error for Manger/Co Owner/Uploader/Editor Any role can be set during share invite request Jan 5, 2024
@micbar
Copy link
Contributor

micbar commented Jan 5, 2024

What is the response of api call https://owncloud.dev/libre-graph-api/#/drives.permissions/ListPermissions on the two different cases?

IMO the test should use the response from api call https://owncloud.dev/libre-graph-api/#/drives.permissions/ListPermissions and check if the allowed roles are possible to apply.

@amrita-shrestha
Copy link
Contributor Author

amrita-shrestha commented Jan 10, 2024

@micbar

  1. create file abc.txt
  2. send list permission API request response with role Co-Owner/Manger/Uploader/Editor/Viewer
    curl -vk 'https://host.docker.internal:9200/graph/v1beta1/drives/{drives-id}/items/{items-id}/permissions'
{
    "@libre.graph.permissions.actions.allowedValues": [
        "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": [
        {
            "@libre.graph.weight": 1,
            "description": "Allows upload file or folder",
            "displayName": "Uploader",
            "id": "1c996275-f1c9-4e71-abdf-a42f6495e960"
        },
        {
            "@libre.graph.weight": 2,
            "description": "Allows reading the shared file or folder",
            "displayName": "Viewer",
            "id": "b1e2218d-eef8-4d4c-b82d-0f1a1b48f3b5"
        },
        {
            "@libre.graph.weight": 3,
            "description": "Allows reading and updating file",
            "displayName": "Editor",
            "id": "2d00ce52-1fc2-4dbc-8b95-a73b73395f5a"
        },
        {
            "@libre.graph.weight": 4,
            "description": "Allows creating, reading, updating and deleting the shared file or folder",
            "displayName": "Editor",
            "id": "fb6c3e19-e378-47e5-b277-9732f9de6e21"
        },
        {
            "@libre.graph.weight": 5,
            "description": "Grants co-owner permissions on a resource",
            "displayName": "Co Owner",
            "id": "3a4ba8e9-6a0d-4235-9140-0e7a34007abe"
        },
        {
            "@libre.graph.weight": 6,
            "description": "Grants manager permissions on a resource. Semantically equivalent to co-owner",
            "displayName": "Manager",
            "id": "312c0871-5ef7-4b3a-85b6-0e4074c64049"
        }
    ]
}
  1. send invite with role Co-Owner
    curl -vk 'https://host.docker.internal:9200/graph/v1beta1/drives/{drive-id}/items/{item-id}/invite' \ --data '{ "recipients": [ { "objectId": "dc3b93f7-f77f-4551-a783-625504af7d77" } ], "roles": [ "3a4ba8e9-6a0d-4235-9140-0e7a34007abe" ] }'
{
    "value": [
        {
            "grantedToV2": {
                "user": {
                    "displayName": "amy stha",
                    "id": "dc3b93f7-f77f-4551-a783-625504af7d77"
                }
            },
            "id": "3802a234-6247-4ab1-8dfd-a74d53a99809:48762040-79ba-4933-b433-ecb2474fc367:230ecb1d-d30e-4f61-8fc8-c1b6bfe73ede",
            "roles": [
                "3a4ba8e9-6a0d-4235-9140-0e7a34007abe"
            ]
        }
    ]
}
  1. again send request listpermission then co-owner role will be displayed as set role
    curl -vk 'https://host.docker.internal:9200/graph/v1beta1/drives/{drives-id}/items/{items-id}/permissions
{
    "@libre.graph.permissions.actions.allowedValues": [
        "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": [
        {
            "@libre.graph.weight": 1,
            "description": "Allows upload file or folder",
            "displayName": "Uploader",
            "id": "1c996275-f1c9-4e71-abdf-a42f6495e960"
        },
        {
            "@libre.graph.weight": 2,
            "description": "Allows reading the shared file or folder",
            "displayName": "Viewer",
            "id": "b1e2218d-eef8-4d4c-b82d-0f1a1b48f3b5"
        },
        {
            "@libre.graph.weight": 3,
            "description": "Allows reading and updating file",
            "displayName": "Editor",
            "id": "2d00ce52-1fc2-4dbc-8b95-a73b73395f5a"
        },
        {
            "@libre.graph.weight": 4,
            "description": "Allows creating, reading, updating and deleting the shared file or folder",
            "displayName": "Editor",
            "id": "fb6c3e19-e378-47e5-b277-9732f9de6e21"
        },
        {
            "@libre.graph.weight": 5,
            "description": "Grants co-owner permissions on a resource",
            "displayName": "Co Owner",
            "id": "3a4ba8e9-6a0d-4235-9140-0e7a34007abe"
        },
        {
            "@libre.graph.weight": 6,
            "description": "Grants manager permissions on a resource. Semantically equivalent to co-owner",
            "displayName": "Manager",
            "id": "312c0871-5ef7-4b3a-85b6-0e4074c64049"
        }
    ],
    "value": [
        {
            "grantedToV2": {
                "user": {
                    "displayName": "amy stha",
                    "id": "dc3b93f7-f77f-4551-a783-625504af7d77"
                }
            },
            "id": "3802a234-6247-4ab1-8dfd-a74d53a99809:48762040-79ba-4933-b433-ecb2474fc367:230ecb1d-d30e-4f61-8fc8-c1b6bfe73ede",
            "roles": [
                "3a4ba8e9-6a0d-4235-9140-0e7a34007abe"
            ]
        }
    ]
}

  1. create folder abc
  2. send listpermission API request gets same role like file
{
    "@libre.graph.permissions.actions.allowedValues": [
        "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": [
        {
            "@libre.graph.weight": 1,
            "description": "Allows upload file or folder",
            "displayName": "Uploader",
            "id": "1c996275-f1c9-4e71-abdf-a42f6495e960"
        },
        {
            "@libre.graph.weight": 2,
            "description": "Allows reading the shared file or folder",
            "displayName": "Viewer",
            "id": "b1e2218d-eef8-4d4c-b82d-0f1a1b48f3b5"
        },
        {
            "@libre.graph.weight": 3,
            "description": "Allows reading and updating file",
            "displayName": "Editor",
            "id": "2d00ce52-1fc2-4dbc-8b95-a73b73395f5a"
        },
        {
            "@libre.graph.weight": 4,
            "description": "Allows creating, reading, updating and deleting the shared file or folder",
            "displayName": "Editor",
            "id": "fb6c3e19-e378-47e5-b277-9732f9de6e21"
        },
        {
            "@libre.graph.weight": 5,
            "description": "Grants co-owner permissions on a resource",
            "displayName": "Co Owner",
            "id": "3a4ba8e9-6a0d-4235-9140-0e7a34007abe"
        },
        {
            "@libre.graph.weight": 6,
            "description": "Grants manager permissions on a resource. Semantically equivalent to co-owner",
            "displayName": "Manager",
            "id": "312c0871-5ef7-4b3a-85b6-0e4074c64049"
        }
    ]
}

@micbar
Copy link
Contributor

micbar commented Jan 10, 2024

@rhafer seems that our allowed roles are not yet scoped.

@rhafer
Copy link
Contributor

rhafer commented Jan 17, 2024

related #6993

@rhafer rhafer added the Priority:p3-medium Normal priority label Jan 17, 2024
@rhafer rhafer self-assigned this Jan 17, 2024
@rhafer
Copy link
Contributor

rhafer commented Jan 18, 2024

Seems there are two bugs here:

  1. In reva when creating a share we're missing a plausibility check, to prevent creating shares with the CreateContainer, Move or Delete grants on File resources (those grants are only meant for Directories)
  2. In the graph API there's bug on the definition of the constraints for the Space specific roles.

rhafer added a commit to rhafer/reva that referenced this issue Jan 18, 2024
…files

It was possible to set the 'CreateContainer', 'Move' or 'Delete' permissions on
file resources with a CreateShare request. These permissions are meant to be only
set on container resources. The UpdateShare request already has a similar check.

Parial Fix: owncloud/ocis#8131
rhafer added a commit to rhafer/reva that referenced this issue Jan 18, 2024
…files

It was possible to set the 'CreateContainer', 'Move' or 'Delete' permissions on
file resources with a CreateShare request. These permissions are meant to be only
set on container resources. The UpdateShare request already has a similar check.

Parial Fix: owncloud/ocis#8131
rhafer added a commit to rhafer/reva that referenced this issue Jan 18, 2024
…files

It was possible to set the 'CreateContainer', 'Move' or 'Delete' permissions on
file resources with a CreateShare request. These permissions are meant to be only
set on container resources. The UpdateShare request already has a similar check.

Partial Fix: owncloud/ocis#8131
rhafer added a commit to cs3org/reva that referenced this issue Jan 19, 2024
…files

It was possible to set the 'CreateContainer', 'Move' or 'Delete' permissions on
file resources with a CreateShare request. These permissions are meant to be only
set on container resources. The UpdateShare request already has a similar check.

Partial Fix: owncloud/ocis#8131
@micbar micbar added this to the Release 5.0.0 milestone Jan 22, 2024
rhafer added a commit to rhafer/ocis that referenced this issue Jan 23, 2024
Use the condition from the unifiedrole to check if the requested role
is actually applicable to the selected resource.

Fixes: owncloud#8131
rhafer added a commit to rhafer/ocis that referenced this issue Jan 24, 2024
Use the condition from the unifiedrole to check if the requested role
is actually applicable to the selected resource.

Fixes: owncloud#8131
rhafer added a commit to rhafer/ocis that referenced this issue Jan 24, 2024
Use the condition from the unifiedrole to check if the requested role
is actually applicable to the selected resource.

Fixes: owncloud#8131
rhafer added a commit to rhafer/ocis that referenced this issue Jan 24, 2024
Use the condition from the unifiedrole to check if the requested role
is actually applicable to the selected resource.

Fixes: owncloud#8131
@rhafer rhafer closed this as completed in d9fc4af Jan 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Done
Development

No branches or pull requests

3 participants