Enforce permissions in public share apps (#2214)

changelog/unreleased/fix-view-only-wopi.md

@@ -0,0 +1,8 @@
+Bugfix: Enforce permissions in public share apps
+
+A receiver of a read-only public share could still edit files via apps like Collabora.
+These changes enforce the share permissions in apps used on publicly shared resources.
+
+https://github.com/owncloud/web/issues/5776
+https://github.com/owncloud/ocis/issues/2479
+https://github.com/cs3org/reva/pull/22142214

internal/grpc/interceptors/auth/scope.go

@@ -50,7 +50,14 @@ func expandAndVerifyScope(ctx context.Context, req interface{}, tokenScope map[s
 		return err
 	}
 
-	if ref, ok := extractRef(req); ok {
+	hasEditorRole := false
+	for _, v := range tokenScope {
+		if v.Role == authpb.Role_ROLE_EDITOR {
+			hasEditorRole = true
+		}
+	}
+
+	if ref, ok := extractRef(req, hasEditorRole); ok {
 		// Check if req is of type *provider.Reference_Path
 		// If yes, the request might be coming from a share where the accessor is
 		// trying to impersonate the owner, since the share manager doesn't know the
@@ -205,36 +212,46 @@ func checkIfNestedResource(ctx context.Context, ref *provider.Reference, parent
 
 }
 
-func extractRef(req interface{}) (*provider.Reference, bool) {
+func extractRef(req interface{}, hasEditorRole bool) (*provider.Reference, bool) {
 	switch v := req.(type) {
+	// Read requests
 	case *registry.GetStorageProvidersRequest:
 		return v.GetRef(), true
 	case *provider.StatRequest:
 		return v.GetRef(), true
 	case *provider.ListContainerRequest:
 		return v.GetRef(), true
+	case *provider.InitiateFileDownloadRequest:
+		return v.GetRef(), true
+	case *appprovider.OpenInAppRequest:
+		return &provider.Reference{ResourceId: v.ResourceInfo.Id}, true
+	case *gateway.OpenInAppRequest:
+		return v.GetRef(), true
+
+		// App provider requests
+	case *appregistry.GetAppProvidersRequest:
+		return &provider.Reference{ResourceId: v.ResourceInfo.Id}, true
+	}
+
+	if !hasEditorRole {
+		return nil, false
+	}
+
+	switch v := req.(type) {
+	// Write Requests
 	case *provider.CreateContainerRequest:
 		return v.GetRef(), true
 	case *provider.DeleteRequest:
 		return v.GetRef(), true
 	case *provider.MoveRequest:
 		return v.GetSource(), true
-	case *provider.InitiateFileDownloadRequest:
-		return v.GetRef(), true
 	case *provider.InitiateFileUploadRequest:
 		return v.GetRef(), true
-	case *appprovider.OpenInAppRequest:
-		return &provider.Reference{ResourceId: v.ResourceInfo.Id}, true
-	case *gateway.OpenInAppRequest:
-		return v.GetRef(), true
 	case *provider.SetArbitraryMetadataRequest:
 		return v.GetRef(), true
 	case *provider.UnsetArbitraryMetadataRequest:
 		return v.GetRef(), true
 
-		// App provider requests
-	case *appregistry.GetAppProvidersRequest:
-		return &provider.Reference{ResourceId: v.ResourceInfo.Id}, true
 	}
 	return nil, false
 }

internal/grpc/services/gateway/storageprovider.go

@@ -46,6 +46,9 @@ import (
 	"github.com/golang-jwt/jwt"
 	"github.com/google/uuid"
 	"github.com/pkg/errors"
+
+	"google.golang.org/grpc/codes"
+	gstatus "google.golang.org/grpc/status"
 )
 
 // transferClaims are custom claims for a JWT token to be used between the metadata and data gateways.
@@ -481,6 +484,9 @@ func (s *svc) initiateFileDownload(ctx context.Context, req *provider.InitiateFi
 
 	storageRes, err := c.InitiateFileDownload(ctx, req)
 	if err != nil {
+		if gstatus.Code(err) == codes.PermissionDenied {
+			return &gateway.InitiateFileDownloadResponse{Status: &rpc.Status{Code: rpc.Code_CODE_PERMISSION_DENIED}}, nil
+		}
 		return nil, errors.Wrap(err, "gateway: error calling InitiateFileDownload")
 	}
 
@@ -679,6 +685,9 @@ func (s *svc) initiateFileUpload(ctx context.Context, req *provider.InitiateFile
 
 	storageRes, err := c.InitiateFileUpload(ctx, req)
 	if err != nil {
+		if gstatus.Code(err) == codes.PermissionDenied {
+			return &gateway.InitiateFileUploadResponse{Status: &rpc.Status{Code: rpc.Code_CODE_PERMISSION_DENIED}}, nil
+		}
 		return nil, errors.Wrap(err, "gateway: error calling InitiateFileUpload")
 	}
 
@@ -830,6 +839,9 @@ func (s *svc) createContainer(ctx context.Context, req *provider.CreateContainer
 
 	res, err := c.CreateContainer(ctx, req)
 	if err != nil {
+		if gstatus.Code(err) == codes.PermissionDenied {
+			return &provider.CreateContainerResponse{Status: &rpc.Status{Code: rpc.Code_CODE_PERMISSION_DENIED}}, nil
+		}
 		return nil, errors.Wrap(err, "gateway: error calling CreateContainer")
 	}
 
@@ -986,6 +998,9 @@ func (s *svc) delete(ctx context.Context, req *provider.DeleteRequest) (*provide
 
 	res, err := c.Delete(ctx, req)
 	if err != nil {
+		if gstatus.Code(err) == codes.PermissionDenied {
+			return &provider.DeleteResponse{Status: &rpc.Status{Code: rpc.Code_CODE_PERMISSION_DENIED}}, nil
+		}
 		return nil, errors.Wrap(err, "gateway: error calling Delete")
 	}
 
@@ -1161,6 +1176,9 @@ func (s *svc) SetArbitraryMetadata(ctx context.Context, req *provider.SetArbitra
 
 	res, err := c.SetArbitraryMetadata(ctx, req)
 	if err != nil {
+		if gstatus.Code(err) == codes.PermissionDenied {
+			return &provider.SetArbitraryMetadataResponse{Status: &rpc.Status{Code: rpc.Code_CODE_PERMISSION_DENIED}}, nil
+		}
 		return nil, errors.Wrap(err, "gateway: error calling Stat")
 	}
 
@@ -1178,6 +1196,9 @@ func (s *svc) UnsetArbitraryMetadata(ctx context.Context, req *provider.UnsetArb
 
 	res, err := c.UnsetArbitraryMetadata(ctx, req)
 	if err != nil {
+		if gstatus.Code(err) == codes.PermissionDenied {
+			return &provider.UnsetArbitraryMetadataResponse{Status: &rpc.Status{Code: rpc.Code_CODE_PERMISSION_DENIED}}, nil
+		}
 		return nil, errors.Wrap(err, "gateway: error calling Stat")
 	}