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

enhancement: add graph beta listPermissions endpoint #7753

Merged
merged 5 commits into from
Nov 28, 2023

Conversation

fschade
Copy link
Contributor

@fschade fschade commented Nov 19, 2023

Description

implement graph listPermissions endpoint according to the spec.

the current implementation is an optimistic approach and needs discussions for the expected behaviour.

the following points are unclear or need further fine-tuning:

  • @libre.graph.permissions.roles.allowedValues, use of CS3ResourcePermissionsToUnifiedRole
  • result.value, to contain public links or not (currently contained) - yes
  • swagger spec return value vs ms spec (collectionOfPermissions or ListResponse) - CollectionOfPermissionsWithAllowedValues
  • display @libre.graph.permissions.roles.allowedValues.*.rolePermissions or not - no
  • meaning of @libre.graph.permissions.roles.allowedValues.*.weight in that context

ToDo

  • ✅ clarification on the above points
  • ✅ add unit tests
  • ✅ abstract shared code

Related Issue

Motivation and Context

listing share (drive) permissions is part of our openAPI and ms graph spec, this is needed for the ocs to graph migratioin.

How Has This Been Tested?

  • acceptance tests
  • e2e tests
  • manual testing

Example:

Request

GET {{host}}/graph/v1beta1/drives/{{drive-id}}/items/{{item-id}}/permissions
Authorization: Basic {{user_admin_login}} {{user_admin_password}}
Authorization: Basic {{user_marie_login}} {{user_marie_password}}

Response (owner)

{
  "@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": [
    {
      "expirationDateTime": "2055-07-15T16:00:00+02:00",
      "grantedToV2": {
        "user": {
          "displayName": "Marie Skłodowska Curie",
          "id": "f7fbf8c8-139b-4376-b307-cf0a8c2d0d9c"
        }
      },
      "id": "3496f80c-b4c7-40cd-9ee9-f7bfb77c671c:4353a0e1-a947-4588-8be2-7730ac879eda:6be053d3-6752-4112-a71a-3002b1c0addf",
      "roles": [
        "b1e2218d-eef8-4d4c-b82d-0f1a1b48f3b5"
      ]
    }
  ]
}

Response (grantee - viewer)

{
  "@libre.graph.permissions.actions.allowedValues": [
    "libre.graph/driveItem/permissions/create",
    "libre.graph/driveItem/path/read",
    "libre.graph/driveItem/quota/read",
    "libre.graph/driveItem/content/read",
    "libre.graph/driveItem/children/read",
    "libre.graph/driveItem/deleted/read",
    "libre.graph/driveItem/basic/read"
  ],
  "@libre.graph.permissions.roles.allowedValues": [
    {
      "@libre.graph.weight": 1,
      "description": "Allows reading the shared file or folder",
      "displayName": "Viewer",
      "id": "b1e2218d-eef8-4d4c-b82d-0f1a1b48f3b5"
    }
  ]
}

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests only (no source changes)

Checklist:

  • Code changes
  • Acceptance tests added
  • Documentation ticket raised:

Copy link

update-docs bot commented Nov 19, 2023

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

@fschade fschade self-assigned this Nov 19, 2023
@individual-it
Copy link
Member

As far as I understand weight is to help the client to sort the values when displaying them to the user. E.g. showing Viewer before Manager would make UX sense, so sorting alphabetically does not work.

Also according to the specs its weight and not @libre.graph.weight. Do we need to change the specs?

@rhafer
Copy link
Contributor

rhafer commented Nov 22, 2023

Also according to the specs its weight and not @libre.graph.weight. Do we need to change the specs?

Seems the example in the spec is wrong. The schema for the unifiedRoleDefinition is using the correct @libre.graph.weight. Should be fixed with: owncloud/libre-graph-api#139

@fschade fschade force-pushed the graph-beta-permissions branch 4 times, most recently from f6b1bd5 to 21677b7 Compare November 26, 2023 14:05
@fschade fschade marked this pull request as ready for review November 26, 2023 14:05
@fschade fschade force-pushed the graph-beta-permissions branch 2 times, most recently from 40a0b01 to 9ca0810 Compare November 26, 2023 18:23
besides the new api endpoint it includes several utilities to simplify the graph api development.

* resolve drive and item id from the request path
* generic pointer and value utilities
* space root detection
@fschade fschade force-pushed the graph-beta-permissions branch from 9ca0810 to ae5a228 Compare November 28, 2023 14:17
@fschade fschade requested review from micbar and rhafer November 28, 2023 14:19
@fschade fschade force-pushed the graph-beta-permissions branch from ae5a228 to adb7d11 Compare November 28, 2023 15:19
Copy link

sonarcloud bot commented Nov 28, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 6 Code Smells

74.2% 74.2% Coverage
0.0% 0.0% Duplication

@fschade fschade merged commit ad06a19 into owncloud:master Nov 28, 2023
4 checks passed
ownclouders pushed a commit that referenced this pull request Nov 28, 2023
* enhancement: add graph beta listPermissions endpoint

besides the new api endpoint it includes several utilities to simplify the graph api development.

* resolve drive and item id from the request path
* generic pointer and value utilities
* space root detection

* update GetDriveAndItemIDParam signature to return a error

* move errorcode package

* enhancement: add generic error code handling

* fix: rebase
@micbar micbar mentioned this pull request Nov 28, 2023
22 tasks
2403905 pushed a commit to 2403905/ocis that referenced this pull request Dec 4, 2023
* enhancement: add graph beta listPermissions endpoint

besides the new api endpoint it includes several utilities to simplify the graph api development.

* resolve drive and item id from the request path
* generic pointer and value utilities
* space root detection

* update GetDriveAndItemIDParam signature to return a error

* move errorcode package

* enhancement: add generic error code handling

* fix: rebase
2403905 pushed a commit to 2403905/ocis that referenced this pull request Dec 8, 2023
* enhancement: add graph beta listPermissions endpoint

besides the new api endpoint it includes several utilities to simplify the graph api development.

* resolve drive and item id from the request path
* generic pointer and value utilities
* space root detection

* update GetDriveAndItemIDParam signature to return a error

* move errorcode package

* enhancement: add generic error code handling

* fix: rebase
2403905 pushed a commit to 2403905/ocis that referenced this pull request Jan 24, 2024
* enhancement: add graph beta listPermissions endpoint

besides the new api endpoint it includes several utilities to simplify the graph api development.

* resolve drive and item id from the request path
* generic pointer and value utilities
* space root detection

* update GetDriveAndItemIDParam signature to return a error

* move errorcode package

* enhancement: add generic error code handling

* fix: rebase
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants