[Shares] New endpoint that announces (sharing) roles for clients

michaelstingl commented 2 years ago

oCIS comes with a new, roles-based sharing concept. It would be beneficial, if clients wouldn't need to hardcode the different roles and permissions, but instead could request them from the backend, to also support future changes. (new, dynamic roles?)

Hardcoded example from ownCloud web: https://github.com/owncloud/web/blob/master/packages/web-client/src/helpers/share/role.ts

For inspiration, maybe check the oC10 implementation for roles-based public links:

https://github.com/owncloud/core/pull/36024

/cc @TheOneRing @felix-schwarz @kulmann @micbar @tbsbdr @pmaier1

felix-schwarz commented 2 years ago

For the iOS SDK, I used role.ts and using the actual web UI as a guide to get an overview of the many different roles and how they technically differ under the hood depending on resource (folder, file, whole space, …) and target (user, group, link, …).

I ended up with this list of roles: https://github.com/owncloud/ios-sdk/blob/feature/graph-api/ownCloudSDK/Core/Sharing/OCCore%2BSharing.m#L360

They can all be described with these properties:

Property	Type	Description
`type`	string	abstract type for internal identification (`viewer`, `uploader`, `editor`, `contributor`, `manager`, `custom`)
`identifier`	string	unique identifier per role (f.ex. `Viewer`, `SpaceViewer`, …)
`shareTypes`	array/flags	types of share (`link`, `group`, `user`) covered by this scope
`permissions`	array/flags	permissions set on this share (`read`, `update`, `create`, `delete`, `share`)
`customizablePermissions`	array/flags	permissions contained in `permissions` that the user is allowed to change for this role. Typically empty/`0` except for the roles of `type` = `custom`. By including a permission (f.ex. `read`) in `permissions`, but not in `customizablePermissions`, the permission is set and can't be turned off by the user.
`locations`	array/flags	Locations that this role can be used with (`file`, `folder`, `drive`)
`iconName`	string	Name of the image resource to use as an icon.
`roleName`	string	Localized name of the scope.
`roleDescription`	string	Localized description for the scope.
`isLocalized`	bool	(optional) Indicates if `roleName` and `roleDescription` are localized. If `false`, these strings are in English and localization should take place on the respective client. (compromise / intermediate solution, see below)

What I imagine an API to look like:

General properties

JSON-based
uses arrays of strings instead of flags for properties such as permissions(f.ex. ["read", "share"] instead of 17 (= 1 | 16))
localized parts use the Accept-Language header to return strings in a language matching the client's UI. If server-side localization is too far off the roadmap at this point, a good compromise could be to return them in English and indicate via isLocalized to the client that it should run those strings through its native/local localization capabilities.

Actual API

One endpoint allowing two different types of query: 1) list: list of all available roles: requires no parameter to be passed 2) item: list of available roles for a particular item. The item is identified…

for items: by a pair of parameters: driveID and path (or driveID + fileID - whatever suits the server best).
for whole drives: just by driveID

That moves the logic which roles are available for which item entirely to the server.

Client usage

Clients can use the result of the item query to determine

which roles to offer to the user
which role is active for a specific share by going through the list of roles and 1) checking if type covers the share type 2) a) comparing share permissions (bitfield value) with the share's permissions. b) if that doesn't turn up any match, comparing if share permissions (bitfield value) are covered by a custom role and fully covered by the role's permissions

Alternative:

each role could gain an additional identifier property that's stable + unique for every role
the shares endpoints add the active roleID to the set of information they return for each share

micbar commented 2 years ago

@michaelstingl @felix-schwarz Thanks for the writeup.

The sharing roles and permissions should be maintained by the backend and are different per space. That is already the case today. A Viewer (sharing in personal spaces) already has different permissions than a SpaceViewer.

IMO we need unique identifiers per sharing role. The bitmask will completely disappear because we do not have a 1:1 relationship between bitmask and a role anymore.

micbar commented 2 years ago

@pmaier1 @tbsbdr For me this is a post GA topic.

tbsbdr commented 2 years ago

already on the (still raw) list

Roles and Permissions: allow customizations

felix-schwarz commented 2 years ago

@michaelstingl @felix-schwarz Thanks for the writeup.

The sharing roles and permissions should be maintained by the backend and are different per space. That is already the case today. A Viewer (sharing in personal spaces) already has different permissions than a SpaceViewer.

I just added identifier to the table in the proposal above.

IMO we need unique identifiers per sharing role. The bitmask will completely disappear because we do not have a 1:1 relationship between bitmask and a role anymore.

Yeah, I noticed the bitmask values are different depending on file/folder/space.

@pmaier1 @tbsbdr For me this is a post GA topic.

Ok. Until then, is there documentation available describing the current sharing API? (especially regarding changes from OC10's)

pmaier1 commented 2 years ago

already on the (still raw) list Roles and Permissions: allow customizations

This is a different topic. The point here is to allow the clients to retrieve the available roles in sharing from the server in order to avoid hardcoding them in every client.

Agree that it is a post-GA topic. It should be tackled together with enabling sharing on the native clients.

tbsbdr commented 1 year ago

highlevel meeting:

cross platform alignment before sprintstart needed @micbar
roles.ts from web should be listed in graph endpoint

michaelstingl commented 1 year ago

roles.ts from web should be listed in graph endpoint

@felix-schwarz would it help to expose what we currently have in role.ts in a new "official" endpoint, and add the nice stuff from https://github.com/owncloud/ocis/issues/4848#issuecomment-1283678879 in new versions?

butonic commented 1 year ago

After a deep dive into the ms graph appRoles and appRoleAssignments in https://github.com/owncloud/ocis/pull/5318#issuecomment-1376992173 I am quite sure that we will at some point have to overhaul the application permissions. We are currently misusing appRoles and appRoleAssignments or application permissions to do what delegated permissions are supposed to do.

That being said, neither concept is the right thing to misuse for sharing roles. I'd still prefer to have a graph API endpoint that extends the concept of the ms graph sharing roles property as defined in https://learn.microsoft.com/en-us/graph/api/resources/permission?view=graph-rest-1.0#properties as

Property	Type	Description
roles	Collection of String	The type of permission, for example, `read`. See below for the full list of roles. Read-only.

So, actually roles here corresponds to oCIS / oc10 sharing permissions as human readable strings.

I propose building on top of this api by introducing a new sharingRoles resource in the libregraph api which allows fetching a ̀ list of roles with a display_name and roles property, where roles is an array of strings, corresponding to the roles in the MS graph sharing permissions above.

If this endpoint does not exist or does not respond, clients can fall back to a hardcoded set or just display custom permissions. When the list of sharingRoles contains a role with matching set of 'roles' aka permissions the display_name is used instead.

butonic commented 1 year ago

hm then again, the sharingRoles might be specific to an application, eg collabora, onlyoffice in addition to the ocis internal permissions. so we may also need an `applicationID` property in the list of `sharingRoles`: Property	Type	Description
`display_name`	String	A descriptive human readable name, eg `Viewer`, `Editor`, `Manager` `File Drop` ...
`roles`	Collection of String	The type of permission, for example, `create`, `read`, `write`, `delete`, `share`, `comment` ...
`applicationID`	String	Corresponds to an `application` resource

Two problems:

display_name is not translated ... should the api return a translated name?
roles are string names not ids. While this allows seamlessly extending the existing MS Graph sharing roles I can understand the desire to use uuids ... also to prevent collisions between multiple applications ... or we extend applications so sharingRoles are a property of applications ...

🤔

felix-schwarz commented 1 year ago

@michaelstingl At least for the iOS client, I see no benefit in having the information from role.ts exposed via a new endpoint as the iOS app's SDK already has that info - and the information in role.ts is not in a format/shape right now that is easily JSON-encodable.

Better to focus on a stable API. Given that the iOS SDK's internal implementation of this is just around ~200 lines of code - with most of these lines spent on all the role definitions - I imagine the effort/time needed for an initial server-side implementation of what I outlined above should be relatively small.

One thing that we haven't discussed yet, however, is what clients should do when connecting to an OC 10 server. If clients should offer the same, role-based UI for these, I see two possibilities: 1) implement the same server-side API for OC10. But this would require everyone to upgrade their server version, so probably a no-go. 2) clients still ship with a built-in fallback set of roles as well as the ability to filter them to match an item/share. The logic the server uses to return possible roles for the item endpoint above should then be documented, so clients can implement the same logic as a fallback for (OC10) servers that don't offer the server-side capabilities.

michaelstingl commented 1 year ago

No changes planned in oC10. They'll keep the bitmask implementation. When oC10 changes, then they should add the new endpoint too.

butonic commented 1 year ago

In order to extend the ms graph resources with our concepts, eg. to use permission ids instead of roles (aka named permissions like read, write) I propose a fifth extension type, in addition to the four defined by ms graph:

Similar to Directory extensions libregraph properties we introduce should be prefixed with lg_. This is similar to the extension_{id}_ prefix of Directory extensions but uses a shorter prefix.

@micbar a lg_ prefix would also allow us to extend the normal drive resource with our space specific properties while making obvious were we diverge.

Discarded ideas:

Directory Extensions

They cannot be registered on permissions. They contain a random id which is generated when registering the extension. We could use extension_libregraph_ but I think that is too long for a prefix.

Use any other of the original four ways to extend ms graph

Each extension can only be applied to specific resource types. In this case permission is not extendable by any of them and the permissions roles property is using names instead of identifiers, so we would be changing the type and meaning if we would just list our permission ids.

Odata Instance Annotations

The example shows that annotations really are meant to add data outside the scope of the actual resource, like influencing rendering:

{
  "@context": "http://host/service/$metadata#Customers",
  "@com.example.customer.setkind": "VIPs",
  "value": [
    {
      "@com.example.display.highlight": true,
      "ID": "ALFKI",
      "CompanyName@com.example.display.style": { "title": true, "order": 1 },
      "CompanyName": "Alfreds Futterkiste",
      "Orders@com.example.display.style#simple": { "order": 2 }
    }
  ]
}

Dot or colon separated prefix

Using a prefix like lg: or lg. is unnecessarily hard or when trying to express json paths like root.permissions.lg_permissions.

butonic commented 1 year ago

odata supports localization via the Accept-Language header, and ms graph makes use of it eg. in the organizationalBranding

The RFC recommends not to return a 406 Not Acceptable status because end users may be able to use the page with translation software. I think on the libragraph API we should return a 406 if we have no translation for the accepted language. When the client wants to use his translation he can leave out the header, or he can sand a language header and retry if he gets a 406.

rhafer commented 8 months ago

We have https://owncloud.dev/libre-graph-api/#/roleManagement now so I guess this can be closed.

owncloud / ocis