Merge pull request #9532 from owncloud/polish-secure-view

fix: polish secure view

.drone.star

@@ -346,11 +346,19 @@ def buildWebCache(ctx):
 def testOcisAndUploadResults(ctx):
     pipeline = testOcis(ctx)
 
+    ######################################################################
+    # The triggers have been disabled for now, since the govulncheck can #
+    # not silence single, acceptable vulnerabilities.                    #
+    # See https://github.com/owncloud/ocis/issues/9527 for more details. #
+    # FIXME: RE-ENABLE THIS ASAP!!!                                      #
+    ######################################################################
+
     scan_result_upload = uploadScanResults(ctx)
     scan_result_upload["depends_on"] = getPipelineNames([pipeline])
 
-    security_scan = scanOcis(ctx)
+    #security_scan = scanOcis(ctx)
-    return [security_scan, pipeline, scan_result_upload]
+    #return [security_scan, pipeline, scan_result_upload]
+    return [pipeline, scan_result_upload]
 
 def testPipelines(ctx):
     pipelines = []

changelog/unreleased/fix-secure-view.md

@@ -0,0 +1,5 @@
+Bugfix: Polish secure view
+
+We fixed a bug where viewing pdf files in secure view mode was not possible. Secure view access on space roots was dropped because of unwanted side effects.
+
+https://github.com/owncloud/ocis/pull/9532

services/collaboration/pkg/helpers/discovery.go

@@ -77,7 +77,7 @@ func parseWopiDiscovery(body io.Reader) (map[string]map[string]string, error) {
 			for _, app := range netzone.SelectElements("app") {
 				for _, action := range app.SelectElements("action") {
 					access := action.SelectAttrValue("name", "")
-					if access == "view" || access == "edit" {
+					if access == "view" || access == "edit" || access == "view_comment" {
 						ext := action.SelectAttrValue("ext", "")
 						urlString := action.SelectAttrValue("urlsrc", "")
 

services/collaboration/pkg/service/grpc/v0/service.go

@@ -89,8 +89,14 @@ func (s *Service) OpenInApp(
 	// get the file extension to use the right wopi app url
 	fileExt := path.Ext(req.GetResourceInfo().GetPath())
 
+	var viewCommentAppURL string
 	var viewAppURL string
 	var editAppURL string
+	if viewCommentAppURLs, ok := s.appURLs["view_comment"]; ok {
+		if url := viewCommentAppURLs[fileExt]; ok {
+			viewCommentAppURL = url
+		}
+	}
 	if viewAppURLs, ok := s.appURLs["view"]; ok {
 		if url := viewAppURLs[fileExt]; ok {
 			viewAppURL = url
@@ -101,7 +107,7 @@ func (s *Service) OpenInApp(
 			editAppURL = url
 		}
 	}
-	if editAppURL == "" && viewAppURL == "" {
+	if editAppURL == "" && viewAppURL == "" && viewCommentAppURL == "" {
 		err := fmt.Errorf("OpenInApp: neither edit nor view app url found")
 		s.logger.Error().
 			Err(err).
@@ -122,7 +128,15 @@ func (s *Service) OpenInApp(
 		// the URL of the end-user application in view mode when different (defaults to edit mod URL)
 		viewAppURL = editAppURL
 	}
-
+	// TODO: check if collabora will support an "edit" url in the future
+	if viewAppURL == "" && editAppURL == "" && viewCommentAppURL != "" {
+		// there are rare cases where neither view nor edit is supported but view_comment is
+		viewAppURL = viewCommentAppURL
+		// that can be the case for editable and viewable files
+		if req.GetViewMode() == appproviderv1beta1.ViewMode_VIEW_MODE_READ_WRITE {
+			editAppURL = viewCommentAppURL
+		}
+	}
 	wopiSrcURL, err := url.Parse(s.config.Wopi.WopiSrc)
 	if err != nil {
 		return nil, err

services/graph/pkg/unifiedrole/unifiedrole.go

@@ -210,10 +210,6 @@ func NewSecureViewerUnifiedRole() *libregraph.UnifiedRoleDefinition {
 				AllowedResourceActions: convert(r),
 				Condition:              proto.String(UnifiedRoleConditionFolder),
 			},
-			{
-				AllowedResourceActions: convert(r),
-				Condition:              proto.String(UnifiedRoleConditionDrive),
-			},
 		},
 		LibreGraphWeight: proto.Int32(0),
 	}

services/graph/pkg/unifiedrole/unifiedrole_test.go

@@ -33,7 +33,6 @@ var _ = Describe("unifiedroles", func() {
 		Entry(rConversions.RoleSpaceEditor, rConversions.NewSpaceEditorRole(), unifiedrole.NewSpaceEditorUnifiedRole(), unifiedrole.UnifiedRoleConditionDrive),
 		Entry(rConversions.RoleSecureViewer, rConversions.NewSecureViewerRole(), unifiedrole.NewSecureViewerUnifiedRole(), unifiedrole.UnifiedRoleConditionFile),
 		Entry(rConversions.RoleSecureViewer, rConversions.NewSecureViewerRole(), unifiedrole.NewSecureViewerUnifiedRole(), unifiedrole.UnifiedRoleConditionFolder),
-		Entry(rConversions.RoleSecureViewer, rConversions.NewSecureViewerRole(), unifiedrole.NewSecureViewerUnifiedRole(), unifiedrole.UnifiedRoleConditionDrive),
 	)
 
 	DescribeTable("UnifiedRolePermissionsToCS3ResourcePermissions",
@@ -205,7 +204,6 @@ var _ = Describe("unifiedroles", func() {
 				rolesToAction(unifiedrole.GetBuiltinRoleDefinitionList()...),
 				unifiedrole.UnifiedRoleConditionDrive,
 				[]*libregraph.UnifiedRoleDefinition{
-					unifiedrole.NewSecureViewerUnifiedRole(),
 					unifiedrole.NewSpaceViewerUnifiedRole(),
 					unifiedrole.NewSpaceEditorUnifiedRole(),
 					unifiedrole.NewManagerUnifiedRole(),

tests/acceptance/features/apiSharingNg/listPermissions.feature

@@ -242,8 +242,8 @@ Feature: List a sharing permissions
           },
           "@libre.graph.permissions.roles.allowedValues": {
             "type": "array",
-            "minItems": 4,
+            "minItems": 3,
-            "maxItems": 4,
+            "maxItems": 3,
             "uniqueItems": true,
             "items": {
               "oneOf":  [
@@ -259,29 +259,6 @@ Feature: List a sharing permissions
                     "@libre.graph.weight": {
                       "const": 1
                     },
-                    "description": {
-                      "const": "View only documents, images and PDFs. Watermarks will be applied."
-                    },
-                    "displayName": {
-                      "const": "Can view (secure)"
-                    },
-                    "id": {
-                      "const": "aa97fe03-7980-45ac-9e50-b325749fd7e6"
-                    }
-                  }
-                },
-                {
-                  "type": "object",
-                  "required": [
-                    "@libre.graph.weight",
-                    "description",
-                    "displayName",
-                    "id"
-                  ],
-                  "properties": {
-                    "@libre.graph.weight": {
-                      "const": 2
-                    },
                     "description": {
                       "const": "View and download."
                     },
@@ -303,7 +280,7 @@ Feature: List a sharing permissions
                   ],
                   "properties": {
                     "@libre.graph.weight": {
-                      "const": 3
+                      "const": 2
                     },
                     "description": {
                       "const": "View, download, upload, edit, add and delete."
@@ -326,7 +303,7 @@ Feature: List a sharing permissions
                   ],
                   "properties": {
                     "@libre.graph.weight": {
-                      "const": 4
+                      "const": 3
                     },
                     "description": {
                       "const": "View, download, upload, edit, add, delete and manage members."
@@ -398,8 +375,8 @@ Feature: List a sharing permissions
           },
           "@libre.graph.permissions.roles.allowedValues": {
             "type": "array",
-            "minItems": 4,
+            "minItems": 3,
-            "maxItems": 4,
+            "maxItems": 3,
             "uniqueItems": true,
             "items": {
               "oneOf":  [
@@ -415,29 +392,6 @@ Feature: List a sharing permissions
                     "@libre.graph.weight": {
                       "const": 1
                     },
-                    "description": {
-                      "const": "View only documents, images and PDFs. Watermarks will be applied."
-                    },
-                    "displayName": {
-                      "const": "Can view (secure)"
-                    },
-                    "id": {
-                      "const": "aa97fe03-7980-45ac-9e50-b325749fd7e6"
-                    }
-                  }
-                },
-                {
-                  "type": "object",
-                  "required": [
-                    "@libre.graph.weight",
-                    "description",
-                    "displayName",
-                    "id"
-                  ],
-                  "properties": {
-                    "@libre.graph.weight": {
-                      "const": 2
-                    },
                     "description": {
                       "const": "View and download."
                     },
@@ -459,7 +413,7 @@ Feature: List a sharing permissions
                   ],
                   "properties": {
                     "@libre.graph.weight": {
-                      "const": 3
+                      "const": 2
                     },
                     "description": {
                       "const": "View, download, upload, edit, add and delete."
@@ -482,7 +436,7 @@ Feature: List a sharing permissions
                   ],
                   "properties": {
                     "@libre.graph.weight": {
-                      "const": 4
+                      "const": 3
                     },
                     "description": {
                       "const": "View, download, upload, edit, add, delete and manage members."
@@ -645,7 +599,6 @@ Feature: List a sharing permissions
       | Space Viewer     |
       | Space Editor     |
       | Manager          |
-      | Secure viewer    |
 
   @issues-8331
   Scenario: user lists permissions of a file in personal space
@@ -1110,8 +1063,8 @@ Feature: List a sharing permissions
           },
           "@libre.graph.permissions.roles.allowedValues": {
             "type": "array",
-            "minItems": 4,
+            "minItems": 3,
-            "maxItems": 4,
+            "maxItems": 3,
             "uniqueItems": true,
             "items": {
               "oneOf":  [
@@ -1127,29 +1080,6 @@ Feature: List a sharing permissions
                     "@libre.graph.weight": {
                       "const": 1
                     },
-                    "description": {
-                      "const": "View only documents, images and PDFs. Watermarks will be applied."
-                    },
-                    "displayName": {
-                      "const": "Can view (secure)"
-                    },
-                    "id": {
-                      "const": "aa97fe03-7980-45ac-9e50-b325749fd7e6"
-                    }
-                  }
-                },
-                {
-                  "type": "object",
-                  "required": [
-                    "@libre.graph.weight",
-                    "description",
-                    "displayName",
-                    "id"
-                  ],
-                  "properties": {
-                    "@libre.graph.weight": {
-                      "const": 2
-                    },
                     "description": {
                       "const": "View and download."
                     },
@@ -1171,7 +1101,7 @@ Feature: List a sharing permissions
                   ],
                   "properties": {
                     "@libre.graph.weight": {
-                      "const": 3
+                      "const": 2
                     },
                     "description": {
                       "const": "View, download, upload, edit, add and delete."
@@ -1194,7 +1124,7 @@ Feature: List a sharing permissions
                   ],
                   "properties": {
                     "@libre.graph.weight": {
-                      "const": 4
+                      "const": 3
                     },
                     "description": {
                       "const": "View, download, upload, edit, add, delete and manage members."
@@ -1255,8 +1185,8 @@ Feature: List a sharing permissions
           },
           "@libre.graph.permissions.roles.allowedValues": {
             "type": "array",
-            "minItems": 4,
+            "minItems": 3,
-            "maxItems": 4,
+            "maxItems": 3,
             "uniqueItems": true,
             "items": {
               "oneOf":  [
@@ -1272,29 +1202,6 @@ Feature: List a sharing permissions
                     "@libre.graph.weight": {
                       "const": 1
                     },
-                    "description": {
-                      "const": "View only documents, images and PDFs. Watermarks will be applied."
-                    },
-                    "displayName": {
-                      "const": "Can view (secure)"
-                    },
-                    "id": {
-                      "const": "aa97fe03-7980-45ac-9e50-b325749fd7e6"
-                    }
-                  }
-                },
-                {
-                  "type": "object",
-                  "required": [
-                    "@libre.graph.weight",
-                    "description",
-                    "displayName",
-                    "id"
-                  ],
-                  "properties": {
-                    "@libre.graph.weight": {
-                      "const": 2
-                    },
                     "description": {
                       "const": "View and download."
                     },
@@ -1316,7 +1223,7 @@ Feature: List a sharing permissions
                   ],
                   "properties": {
                     "@libre.graph.weight": {
-                      "const": 3
+                      "const": 2
                     },
                     "description": {
                       "const": "View, download, upload, edit, add and delete."
@@ -1339,7 +1246,7 @@ Feature: List a sharing permissions
                   ],
                   "properties": {
                     "@libre.graph.weight": {
-                      "const": 4
+                      "const": 3
                     },
                     "description": {
                       "const": "View, download, upload, edit, add, delete and manage members."
@@ -1560,8 +1467,8 @@ Feature: List a sharing permissions
           },
           "@libre.graph.permissions.roles.allowedValues": {
             "type": "array",
-            "minItems": 4,
+            "minItems": 3,
-            "maxItems": 4,
+            "maxItems": 3,
             "uniqueItems": true,
             "items": {
               "oneOf":  [
@@ -1577,29 +1484,6 @@ Feature: List a sharing permissions
                     "@libre.graph.weight": {
                       "const": 1
                     },
-                    "description": {
-                      "const": "View only documents, images and PDFs. Watermarks will be applied."
-                    },
-                    "displayName": {
-                      "const": "Can view (secure)"
-                    },
-                    "id": {
-                      "const": "aa97fe03-7980-45ac-9e50-b325749fd7e6"
-                    }
-                  }
-                },
-                {
-                  "type": "object",
-                  "required": [
-                    "@libre.graph.weight",
-                    "description",
-                    "displayName",
-                    "id"
-                  ],
-                  "properties": {
-                    "@libre.graph.weight": {
-                      "const": 2
-                    },
                     "description": {
                       "const": "View and download."
                     },
@@ -1621,7 +1505,7 @@ Feature: List a sharing permissions
                   ],
                   "properties": {
                     "@libre.graph.weight": {
-                      "const": 3
+                      "const": 2
                     },
                     "description": {
                       "const": "View, download, upload, edit, add and delete."
@@ -1644,7 +1528,7 @@ Feature: List a sharing permissions
                   ],
                   "properties": {
                     "@libre.graph.weight": {
-                      "const": 4
+                      "const": 3
                     },
                     "description": {
                       "const": "View, download, upload, edit, add, delete and manage members."
@@ -1807,7 +1691,6 @@ Feature: List a sharing permissions
       | Space Viewer     |
       | Space Editor     |
       | Manager          |
-      | Secure viewer    |
 
 
   Scenario: user sends share invitation with all allowed roles for a project space using root endpoint
@@ -1882,4 +1765,4 @@ Feature: List a sharing permissions
           }
         }
       }
-      """
+      """