owncloud / ocis

:atom_symbol: ownCloud Infinite Scale Stack
https://doc.owncloud.com/ocis/next/
Apache License 2.0
1.4k stars 182 forks source link

Research: Secure View - Time Boxed - 3pd #8855

Closed micbar closed 6 months ago

micbar commented 6 months ago

Description

User Stories

Value

Acceptance Criteria

Definition of ready

Definition of done

dragonchaser commented 6 months ago

Why is there already a merged PR? As I read it there should be an ADR first....

kobergj commented 6 months ago

That just added the role - this ticket is about figuring out how to use it.

dragonchaser commented 6 months ago

We need to set the Attributes in https://github.com/owncloud/ocis/blob/1fad9acf1a11cba4a701d69b3e0ade758800527c/services/collaboration/pkg/connector/fileconnector.go#L500-L522 in the same way they are set in the richdocuments app in oc10 (see: https://github.com/owncloud/richdocuments/blob/d9d2c2c624649fe6106617f2f416b40a40b98351/lib/Controller/WopiController.php#L244-L268 )

dragonchaser commented 6 months ago

https://github.com/owncloud/ocis/blob/1fad9acf1a11cba4a701d69b3e0ade758800527c/services/collaboration/pkg/connector/contentconnector.go#L62 needs to be adapted accordingly to respect the permissions set. TODO: We need to figure out how to create a permission scope that is limited to the specific item in the share.

butonic commented 6 months ago

The storageprovider has to be able to decide if a InitiateFileDownload request is allowed. It can only look at the context, which currently only contains the user info. If the user does not have the InitiateDownload permission he currently cannot download the file. We cannot add a view-only permission to the scope, that would blow up the scope and it would have to hapen during login.

The storageprovider could check a Permission, but then any client making a CS3 RPC could make the same request and download the file.

Instead of a permission check we could add a ViewOnly Permission to the Permission set, but then we still need to check if the request was made by a trusted client, like a WOPI server.

AFAICT we need to carry the application in the scope. In this case WOPI (or to be specific only 'Collabora') supports 'secure view'. That trusted application could be put into the scope when the WOPI server makes the RPC request.

Collabora documented their WOPI extensions in https://sdk.collaboraonline.com/docs/advanced_integration.html#checkfileinfo-extended-response-properties

🤔

butonic commented 6 months ago

The CS3 API already has view modes for the open in app rpc:

enum ViewMode {
VIEW_MODE_INVALID = 0;
// The resource can be opened but not downloaded.
VIEW_MODE_VIEW_ONLY = 1;
// The resource can be downloaded.
VIEW_MODE_READ_ONLY = 2;
// The resource can be downloaded and updated. The underlying application
// MUST be a fully capable editor to support this mode.
VIEW_MODE_READ_WRITE = 3;
// The resource can be downloaded and updated, but must be shown in
// preview mode. If the underlying application does not support a preview mode,
// or if in a view-only mode users are not allowed to switch to edit mode,
// then this mode MUST fall back to READ_WRITE.
VIEW_MODE_PREVIEW = 4;
}
butonic commented 6 months ago

In the libregraph API we have mapped the CS3 permissions to actions:

I don't like to put all permissions into the permissions set. It forces all storage drivers to implement that permission.

🤔

The naming of the action should be a camelCased CRUD verb: https://learn.microsoft.com/en-us/graph/api/resources/unifiedrolepermission?view=graph-rest-1.0#allowedresourceactions-property

But I don't see a good way of mapping viewOnly to CRUD verbs.

butonic commented 6 months ago

Two approaches:

1. let WOPI use a service account with a limited scope

Let WOPI mint a token with scope to limit the request to that specific resource.

2. attach app password to share in file metadata

Creates a specific token that allows downloading the file, similar to a presigned url

3. Introduce ViewOnly permission

Create a new CS3 permission that allows the InitiateFileDownload call for trusted clients, like the wopi server. The trust relationship can be established using a shared secret, a new scope, client secrets ... tokens ... whatever

4. Use CS3 Permission Check call

Instead of a ViewOnly permission we could make a call to the permissions service. I'm not sure if that request can also carry the client information. 👀

5. Create two shares

Or the wopi server is an application in the context, additionally to the user in the context.

The crux is that the application has more permissions than the user. Which I assume is the reason why MS Graph has application permissions.

When sharing a file the application does - in this case - not act like a constraint. Both conditions have to be true:

  1. The user must have stat permissions
  2. The application must have initiate file download permission For this we would need 2 ACEs per share ... 🤔

6. Let the gateway mint an additional viewOnlyToken for the approvider

The gatway can mint an additional viewOnlyToken when a user makes an OpenInApp call having view only permissions. see https://github.com/owncloud/ocis/issues/8855#issuecomment-2085580566

butonic commented 6 months ago

Permission Checks:

message CheckPermissionRequest {
    // REQUIRED.
    // The permission to check.
    string permission = 1;
    // REQUIRED.
    // The subject holding the permission.
    SubjectReference subject_ref = 2;
    // OPTIONAL.
    // The target resource of the permission.
    cs3.storage.provider.v1beta1.Reference ref = 3;
}

message SubjectReference {
    oneof spec {
        cs3.identity.user.v1beta1.UserId user_id = 1;
        cs3.identity.group.v1beta1.GroupId group_id = 2;
    }
}

So currently the request cannot carry any client/application information.

butonic commented 6 months ago

The storageprovider has to determine access basedon the user, the application and the sharing permissions if a download can be initiated.

micbar commented 6 months ago

Permission Checks:

message CheckPermissionRequest {
  // REQUIRED.
  // The permission to check.
  string permission = 1;
  // REQUIRED.
  // The subject holding the permission.
  SubjectReference subject_ref = 2;
  // OPTIONAL.
  // The target resource of the permission.
  cs3.storage.provider.v1beta1.Reference ref = 3;
}

message SubjectReference {
  oneof spec {
      cs3.identity.user.v1beta1.UserId user_id = 1;
      cs3.identity.group.v1beta1.GroupId group_id = 2;
  }
}

So currently the request cannot carry any client/application information.

Please do not mix the user permissions with file permissions.

butonic commented 6 months ago

We could mint an additional viewOnlyToken in the gateway and send it as part of the OpenInApp call. For now we can use Opaque ... should be come a distinct property. The collaboration service aka CS3 appprovider can then use that token to make the InitiateFileDownload request.

The gateway already does a stat request. To decide if we should mint a viewOnlyToken (which effectively allows downloading the file content) We can either make a call to the share manager, or we introduce a ViewOnly Permission. The former would rely on the share manager that captures the share intent. The latter would need a code change ... and it needs special handling in every storage provider implementation.

diff --git a/internal/grpc/services/gateway/appprovider.go b/internal/grpc/services/gateway/appprovider.go
index 3a91e12a0..442e5896a 100644
--- a/internal/grpc/services/gateway/appprovider.go
+++ b/internal/grpc/services/gateway/appprovider.go
@@ -173,7 +173,8 @@ func (s *svc) openLocalResources(ctx context.Context, ri *storageprovider.Resour
                ResourceInfo: ri,
                ViewMode:     providerpb.ViewMode(vm),
                AccessToken:  accessToken,
-               Opaque:       opaque,
+               // ViewOnlyToken: viewOnlyToken // scoped to the shared resource if the stat response hase a ViewOnly permission
+               Opaque: opaque,
        }

        res, err := appProviderClient.OpenInApp(ctx, appProviderReq)

Always minting a ViewOnlyToken when a user has the stat permission is far too permissive IMO.

But adding a new ViewOnly permission would require code in every storage provider implementation.

And since the posix world has no use for that permission I am leaning towards the additional request to the share manager.

The gateway will need a service account that it can use to mint a ViewOnlyToken for the OpenInApp Request.

butonic commented 6 months ago

the current ocis collaboration service also needs a few changes

micbar commented 6 months ago

Follow ups created. Closing.