Refactored canViewRecord based on comments.

Also fixed issue with /related generating error.
ianwallen · Sep 29, 2023 · 184e786 · 184e786
1 parent 6f0317f
commit 184e786
Show file tree

Hide file tree

Showing 5 changed files with 57 additions and 24 deletions.
diff --git a/services/src/main/java/org/fao/geonet/api/ApiUtils.java b/services/src/main/java/org/fao/geonet/api/ApiUtils.java
@@ -23,6 +23,8 @@
 
 package org.fao.geonet.api;
 
+import static org.fao.geonet.api.records.attachments.AbstractStore.getAndCheckMetadataId;
+
 import com.google.common.collect.Sets;
 import jeeves.constants.Jeeves;
 import jeeves.server.UserSession;
@@ -459,7 +461,7 @@ static public AbstractMetadata canChangeStatusRecord(String metadataUuid, Servic
     }
 
     /**
-     * Check if the current user can view this record.
+     * Check if the current user can view this approved record
      *
      * This method creates a temporary service context using the provided request to check record access,
      * if you have a service context already please use {@link #canViewRecord(String, ServiceContext)}.
@@ -470,11 +472,34 @@ static public AbstractMetadata canChangeStatusRecord(String metadataUuid, Servic
      * @throws SecurityException if user is not allowed to view
      */
     public static AbstractMetadata canViewRecord(String metadataUuid, HttpServletRequest request) throws Exception {
+        return canViewRecord(metadataUuid, true, request);
+    }
+    /**
+     * Check if the current user can view this record.
+     *
+     * This method creates a temporary service context using the provided request to check record access,
+     * if you have a service context already please use {@link #canViewRecord(String, ServiceContext)}.
+     *
+     * @param metadataUuid Look up metadata record
+     * @param request Request to identify current user
+     * @return metadata record
+     * @throws SecurityException if user is not allowed to view
+     */
+    public static AbstractMetadata canViewRecord(String metadataUuid, boolean approved, HttpServletRequest request) throws Exception {
+        String metadataId;
+        if (!approved) {
+            // If the record is not approved then we need to get the id of the record.
+            metadataId = String.valueOf(getAndCheckMetadataId(metadataUuid, approved));
+        } else {
+            // Otherwise use the uuid or id that was supplied.
+            metadataId = metadataUuid;
+        }
+
         ServiceContext previous = ServiceContext.get();
         if (previous != null) previous.clearAsThreadLocal();
 
         try (ServiceContext context = createServiceContext(request)) {
-            return canViewRecord(metadataUuid,context);
+            return canViewRecord(metadataId,context);
         }
         finally {
             if (previous != null) previous.setAsThreadLocal();
@@ -488,7 +513,27 @@ public static AbstractMetadata canViewRecord(String metadataUuid, HttpServletReq
      * @throws SecurityException if user is not allowed to view
      */
     public static AbstractMetadata canViewRecord(String metadataUuid, ServiceContext context) throws Exception {
-        AbstractMetadata metadata = getRecord(metadataUuid);
+        return canViewRecord(metadataUuid, true, context);
+    }
+
+    /**
+     * Check if the current user can view this record.
+     *
+     * @param metadataUuid Look up metadata record
+     * @return metadata record
+     * @throws SecurityException if user is not allowed to view
+     */
+    public static AbstractMetadata canViewRecord(String metadataUuid, boolean approved, ServiceContext context) throws Exception {
+        String metadataId;
+        if (!approved) {
+            // If the record is not approved then we need to get the id of the record.
+            metadataId = String.valueOf(getAndCheckMetadataId(metadataUuid, approved));
+        } else {
+            // Otherwise use the uuid or id that was supplied.
+            metadataId = metadataUuid;
+        }
+
+        AbstractMetadata metadata = getRecord(metadataId);
         try {
             Lib.resource.checkPrivilege(context, String.valueOf(metadata.getId()), ReservedOperation.view);
         } catch (Exception e) {

diff --git a/services/src/main/java/org/fao/geonet/api/records/MetadataApi.java b/services/src/main/java/org/fao/geonet/api/records/MetadataApi.java
@@ -565,13 +565,7 @@ public RelatedResponse getRelated(
       try (ServiceContext context = ApiUtils.createServiceContext(request)) {
         AbstractMetadata md;
         try {
-            int metadataId;
-            if (Lib.type.isInteger(metadataUuid)) {
-                metadataId = Integer.parseInt(metadataUuid);
-            } else {
-                metadataId = getAndCheckMetadataId(metadataUuid, approved);
-            }
-            md = ApiUtils.canViewRecord(String.valueOf(metadataId), request);
+            md = ApiUtils.canViewRecord(metadataUuid, approved, request);
         } catch (SecurityException e) {
             Log.debug(API.LOG_MODULE_NAME, e.getMessage(), e);
             throw new NotAllowedException(ApiParams.API_RESPONSE_NOT_ALLOWED_CAN_VIEW);
@@ -633,7 +627,7 @@ public FeatureResponse getFeatureCatalog(
         Map<String, String[]> decodeMap = new HashMap<>();
 
         try {
-            RelatedResponse related = getRelated(metadataUuid, type, approved,1, 100, request);
+            RelatedResponse related = getRelated(metadataUuid, type, approved, 1, 100, request);
 
             if (isIncludedAttributeTable(related.getFcats())) {
                 for (AttributeTable.Element element : related.getFcats().getItem().get(0).getFeatureType().getAttributeTable().getElement()) {

diff --git a/services/src/main/java/org/fao/geonet/api/related/Related.java b/services/src/main/java/org/fao/geonet/api/related/Related.java
@@ -135,13 +135,7 @@ public Map<String, RelatedResponse> getRelated(
 
         for(String uuid : uuids) {
             try{
-                int metadataId;
-                if (Lib.type.isInteger(uuid)) {
-                    metadataId = Integer.parseInt(uuid);
-                } else {
-                    metadataId = getAndCheckMetadataId(uuid, approved);
-                }
-                md = ApiUtils.canViewRecord(String.valueOf(metadataId), request);
+                md = ApiUtils.canViewRecord(uuid, approved, context);
                 Element raw = new Element("root").addContent(Arrays.asList(
                     new Element("gui").addContent(Arrays.asList(
                         new Element("language").setText(language.getISO3Language()),

diff --git a/web-ui/src/main/resources/catalog/components/edit/onlinesrc/OnlineSrcService.js b/web-ui/src/main/resources/catalog/components/edit/onlinesrc/OnlineSrcService.js
@@ -230,9 +230,9 @@
         getAllResources: function(types) {
 
           var defer = $q.defer();
-          var url = '../api/records/' + gnCurrentEdit.uuid + '/related?type=' +
-                      (angular.isArray(types) ? types.join('&type=') : '') + 
-                      (gnCurrentEdit.metadata.draft === 'y' ? '&approved=false' : '');
+          var url = '../api/records/' + gnCurrentEdit.uuid + '/related' +
+                      (angular.isArray(types) ? '?' + types.join('&type=') : '') +
+                      (gnCurrentEdit.metadata.draft === 'y' ? (angular.isArray(types) ? "&" : "?") + 'approved=false' : '');
           $http.get(url, {
             headers: {
               'Accept': 'application/json'

diff --git a/web-ui/src/main/resources/catalog/components/metadataactions/RelatedDirective.js b/web-ui/src/main/resources/catalog/components/metadataactions/RelatedDirective.js
@@ -54,11 +54,11 @@
       var canceller = $q.defer();
       var request = $http({
         method: 'get',
-        url: '../api/records/' + uuidOrId + '/related?type=' +
+        url: '../api/records/' + uuidOrId + '/related' +
             (types ?
-             types.split('|').join('&type=') :
+            '?type=' + types.split('|').join('&type=') :
             '') +
-          (approved === false ? "&approved=false" : ""),
+          (approved === false ? (types ? "&" : "?" ) + "approved=false" : ""),
         timeout: canceller.promise,
         cache: true
       });