Fixed Anonymous guestbook file download via persistentId

doc/release-notes/12245-anon-guestbook-download-via-persistentid.md

@@ -0,0 +1,4 @@
+## Bug
+Guest requesting file download using :persistentId with guestbook response is now working.
+
+"persistentId" will be replaced by the actual fileId in the signed url that is returned by the POST call containing the guestbook response.

doc/sphinx-guides/source/api/native-api.rst

@@ -8842,7 +8842,7 @@ A curl example using allowing access to a dataset's metadata
 
   curl -H "X-Dataverse-key:$API_KEY" -H 'Content-Type:application/json' -d "$JSON" "$SERVER_URL/api/admin/requestSignedUrl"
 
-Please see :ref:`dataverse.api.signature-secret` for the configuration option to add a shared secret, enabling extra
+Please see :ref:`dataverse.api.signing-secret` for the configuration option to add a shared secret, enabling extra
 security.
 
 .. _send-feedback-admin:

doc/sphinx-guides/source/installation/config.rst

@@ -3308,23 +3308,23 @@ Can also be set via *MicroProfile Config API* sources, e.g. the environment vari
 
 **Note:** This setting was previously called `dataverse.personOrOrg.orgPhraseArray` and expected a JsonArray of strings. Please update both the name and value format if using the old setting.
 
-.. _dataverse.api.signature-secret:
+.. _dataverse.api.signing-secret:
 
-dataverse.api.signature-secret
+dataverse.api.signing-secret
 ++++++++++++++++++++++++++++++
 
 Context: Dataverse has the ability to create "Signed URLs" for it's API calls. Using a signed URLs is more secure than
 providing API tokens, which are long-lived and give the holder all of the permissions of the user. In contrast, signed URLs
 are time limited and only allow the action of the API call in the URL. See :ref:`api-exttools-auth` and
 :ref:`api-native-signed-url` for more details. 
 
-The key used to sign a URL is created from the API token of the creating user plus a signature-secret provided by an administrator.
-**Using a signature-secret is highly recommended.** This setting defaults to an empty string. Using a non-empty 
-signature-secret makes it impossible for someone who knows an API token from forging signed URLs and provides extra security by 
+The key used to sign a URL is created from the API token of the creating user plus a signing-secret provided by an administrator.
+**Using a signing-secret is highly recommended.** This setting defaults to an empty string. Using a non-empty 
+signing-secret makes it impossible for someone who knows an API token from forging signed URLs and provides extra security by 
 making the overall signing key longer.
 
 **WARNING**:
-*Since the signature-secret is sensitive, you should treat it like a password.*
+*Since the signing-secret is sensitive, you should treat it like a password.*
 *See* :ref:`secure-password-storage` *to learn about ways to safeguard it.*
 
 Can also be set via any `supported MicroProfile Config API source`_, e.g. the environment variable ``DATAVERSE_API_SIGNATURE_SECRET`` (although you shouldn't use environment variables for passwords) .

src/main/java/edu/harvard/iq/dataverse/api/AbstractApiBean.java

@@ -77,7 +77,7 @@ public abstract class AbstractApiBean {
 
     private static final Logger logger = Logger.getLogger(AbstractApiBean.class.getName());
     private static final String DATAVERSE_KEY_HEADER_NAME = "X-Dataverse-key";
-    private static final String PERSISTENT_ID_KEY=":persistentId";
+    protected static final String PERSISTENT_ID_KEY=":persistentId";
     private static final String ALIAS_KEY=":alias";
     public static final String STATUS_WF_IN_PROGRESS = "WORKFLOW_IN_PROGRESS";
     public static final String DATAVERSE_WORKFLOW_INVOCATION_HEADER_NAME = "X-Dataverse-invocationID";

src/main/java/edu/harvard/iq/dataverse/api/Access.java

@@ -419,6 +419,11 @@ private String normalizeFileId(String fileId) {
         while (fId.lastIndexOf('/') == fId.length() - 1) {
             fId = fId.substring(0, fId.length() - 1);
         }
+        // Handle persistentId by converting it back to ID
+        if (fileId.equals(PERSISTENT_ID_KEY)) {
+            DataFile file = findDataFileOrDieWrapper(fileId);
+            fId = String.valueOf(file.getId());
+        }
 
         if (fId.indexOf('/') > -1) {
             // This is for embedding folder names into the Access API URLs;
@@ -448,15 +453,17 @@ private Response processDatafileWithGuestbookResponse(ContainerRequestContext cr
         // Handle Guestbook Responses
         String displayName = "";
         String gbrids = null;
-        Long datasetId = null;
+        List<String> fileIdList = new ArrayList<>();
+        String id = null;
         try {
             // since all files must be in the same Dataset we can generate a Guestbook Response once and just replace the DataFile for each file in the list
             DataFile firstDatafile = datafilesMap.values().size() > 0 ? (DataFile) Arrays.stream(datafilesMap.values().toArray()).findFirst().get() : null;
+            id = firstDatafile.getOwner().getId().toString();
             GuestbookResponse gbr = getGuestbookResponseFromBody(firstDatafile, GuestbookResponse.DOWNLOAD, jsonBody, user);
             boolean guestbookResponseRequired = checkGuestbookRequiredResponse(crc, uriInfo, firstDatafile, null);
             for (DataFile df : datafilesMap.values()) {
                 displayName = df.getDisplayName();
-                datasetId = df.getOwner().getId();
+                fileIdList.add(String.valueOf(df.getId()));
                 if (guestbookResponseRequired) {
                     if (gbr != null) {
                         gbr.setDataFile(df);
@@ -479,12 +486,17 @@ private Response processDatafileWithGuestbookResponse(ContainerRequestContext cr
             List<String> args = Arrays.asList(displayName, ex.getLocalizedMessage());
             return error(BAD_REQUEST, BundleUtil.getStringFromBundle("access.api.download.failure.guestbook.commandError", args));
         }
-        return returnSignedUrl(crc, uriInfo, user, datasetId.toString(), gbrids);
+        // Check if requesting datafile(s) or all files within dataset
+        if (!uriInfo.getPath().toLowerCase().contains("/dataset/")) {
+            id = String.join(",", fileIdList);
+        }
+        return returnSignedUrl(crc, uriInfo, user, id, gbrids);
     }
 
     private Map<Long, DataFile> getDatafilesMap(ContainerRequestContext crc, String fileIds) {
         String fileIdParams[] = getFileIdsCSV(fileIds);
         Map<Long, DataFile> datafilesMap = new HashMap<>();
+        Long datasetId = null;
         // Get and validate all the DataFiles first
         if (fileIdParams != null && fileIdParams.length > 0) {
             for (int i = 0; i < fileIdParams.length; i++) {
@@ -496,6 +508,16 @@ private Map<Long, DataFile> getDatafilesMap(ContainerRequestContext crc, String
                     // (nobody should ever be using this API on a harvested DataFile)!
                 }
 
+                // Make sure all files are from the same dataset
+                if (datasetId == null) {
+                    datasetId = df.getOwner().getId();
+                } else {
+                    if (!datasetId.equals(df.getOwner().getId())) {
+                        // All files must be from the same Dataset
+                        throw new BadRequestException(BundleUtil.getStringFromBundle("access.api.download.failure.multipleDatasets"));
+                    }
+                }
+
                 // This will throw a ForbiddenException if access isn't authorized:
                 checkAuthorization(crc, df);
 
@@ -526,7 +548,9 @@ private Response returnSignedUrl(ContainerRequestContext crc, UriInfo uriInfo, U
         } else {
             // Guest
             userIdentifier = "guest";
-            key = uriInfo.getAbsolutePath().toASCIIString(); //TODO find a better one for here and in SignedUrlAuthMechanism.java
+            // Note: In order for the key to match we need to replace ":persistentId" with the actual file id since that is what will be sent in via the signed url.
+            key = URLDecoder.decode(uriInfo.getAbsolutePath().toASCIIString())
+                    .replace(":persistentId", id); //TODO find a better one for here and in SignedUrlAuthMechanism.java
         }
 
         UriBuilder builder = UriBuilder.fromUri(uriInfo.getRequestUri());

src/test/java/edu/harvard/iq/dataverse/api/FilesIT.java

@@ -63,6 +63,11 @@ public static void setUpClass() {
         removePublicInstall.then().assertThat().statusCode(200);
     }
 
+    @AfterEach
+    public void resetClass() {
+        UtilIT.deleteSetting(SettingsServiceBean.Key.FilePIDsEnabled);
+    }
+
     @AfterAll
     public static void tearDownClass() {
         UtilIT.deleteSetting(SettingsServiceBean.Key.PublicInstall);
@@ -3869,6 +3874,7 @@ public void testUpdateWithEmptyFieldsAndVersionCheck() throws InterruptedExcepti
     @Test
     public void testDownloadFileWithGuestbookResponse() throws IOException, JsonParseException {
         msgt("testDownloadFileWithGuestbookResponse");
+        UtilIT.enableSetting(SettingsServiceBean.Key.FilePIDsEnabled);
         // Create superuser
         Response createUserResponse = UtilIT.createRandomUser();
         assertEquals(200, createUserResponse.getStatusCode());
@@ -3898,6 +3904,7 @@ public void testDownloadFileWithGuestbookResponse() throws IOException, JsonPars
         createDatasetResponse.then().assertThat().statusCode(CREATED.getStatusCode());
         Integer datasetId = JsonPath.from(createDatasetResponse.body().asString()).getInt("data.id");
         String persistentId = JsonPath.from(createDatasetResponse.body().asString()).getString("data.persistentId");
+        String directoryLabel = "data/store/" + persistentId.substring(4);
         Response getDatasetMetadata = UtilIT.nativeGet(datasetId, ownerApiToken);
         getDatasetMetadata.then().assertThat().statusCode(200);
 
@@ -3914,22 +3921,27 @@ public void testDownloadFileWithGuestbookResponse() throws IOException, JsonPars
         assertEquals(1, getGuestbooksResponse.getBody().jsonPath().getList("data").size());
 
         // Upload files
-        JsonObjectBuilder json1 = Json.createObjectBuilder().add("description", "my description1").add("directoryLabel", "data/subdir1").add("categories", Json.createArrayBuilder().add("Data"));
+        JsonObjectBuilder json1 = Json.createObjectBuilder().add("description", "my description1").add("directoryLabel", directoryLabel).add("categories", Json.createArrayBuilder().add("Data"));
         Response uploadResponse = UtilIT.uploadFileViaNative(datasetId.toString(), "src/main/webapp/resources/images/dataverseproject.png", json1.build(), ownerApiToken);
+        uploadResponse.prettyPrint();
         uploadResponse.then().assertThat().statusCode(OK.getStatusCode());
         Integer fileId1 = JsonPath.from(uploadResponse.body().asString()).getInt("data.files[0].dataFile.id");
-        JsonObjectBuilder json2 = Json.createObjectBuilder().add("description", "my description2").add("directoryLabel", "data/subdir1").add("categories", Json.createArrayBuilder().add("Data"));
-        uploadResponse = UtilIT.uploadFileViaNative(datasetId.toString(), "src/main/webapp/resources/images/orcid_16x16.png", json1.build(), ownerApiToken);
+        JsonObjectBuilder json2 = Json.createObjectBuilder().add("description", "my description2").add("directoryLabel", directoryLabel).add("categories", Json.createArrayBuilder().add("Data"));
+        uploadResponse = UtilIT.uploadFileViaNative(datasetId.toString(), "src/main/webapp/resources/images/orcid_16x16.png", json2.build(), ownerApiToken);
+        uploadResponse.prettyPrint();
         uploadResponse.then().assertThat().statusCode(OK.getStatusCode());
         Integer fileId2 = JsonPath.from(uploadResponse.body().asString()).getInt("data.files[0].dataFile.id");
-        JsonObjectBuilder json3 = Json.createObjectBuilder().add("description", "my description3").add("directoryLabel", "data/subdir1").add("categories", Json.createArrayBuilder().add("Data"));
-        uploadResponse = UtilIT.uploadFileViaNative(datasetId.toString(), "src/main/webapp/resources/images/cc0.png", json1.build(), ownerApiToken);
+        JsonObjectBuilder json3 = Json.createObjectBuilder().add("description", "my description3").add("directoryLabel", directoryLabel).add("categories", Json.createArrayBuilder().add("Data"));
+        uploadResponse = UtilIT.uploadFileViaNative(datasetId.toString(), "src/main/webapp/resources/images/cc0.png", json3.build(), ownerApiToken);
+        uploadResponse.prettyPrint();
         uploadResponse.then().assertThat().statusCode(OK.getStatusCode());
         Integer fileId3 = JsonPath.from(uploadResponse.body().asString()).getInt("data.files[0].dataFile.id");
-        JsonObjectBuilder json4 = Json.createObjectBuilder().add("description", "my description4").add("directoryLabel", "data/subdir1").add("categories", Json.createArrayBuilder().add("Data"));
-        uploadResponse = UtilIT.uploadFileViaNative(datasetId.toString(), "src/main/webapp/resources/images/Robot-Icon_2.png", json1.build(), ownerApiToken);
+        JsonObjectBuilder json4 = Json.createObjectBuilder().add("description", "my description4").add("directoryLabel", directoryLabel).add("categories", Json.createArrayBuilder().add("Data"));
+        uploadResponse = UtilIT.uploadFileViaNative(datasetId.toString(), "src/main/webapp/resources/images/Robot-Icon_2.png", json4.build(), ownerApiToken);
         uploadResponse.then().assertThat().statusCode(OK.getStatusCode());
+        uploadResponse.prettyPrint();
         Integer fileId4 = JsonPath.from(uploadResponse.body().asString()).getInt("data.files[0].dataFile.id");
+        String filePersistentId = JsonPath.from(uploadResponse.body().asString()).getString("data.files[0].dataFile.persistentId");
 
         // Restrict files
         Response restrictResponse = UtilIT.restrictFile(fileId1.toString(), true, ownerApiToken);
@@ -4025,6 +4037,14 @@ public void testDownloadFileWithGuestbookResponse() throws IOException, JsonPars
         downloadResponse = UtilIT.postDownloadDatafiles(jsonBody, apiToken);
         assertEquals(OK.getStatusCode(), downloadResponse.getStatusCode());
 
+        // Download all files in dataset with guestbook response using dataset persistentId
+        downloadResponse = UtilIT.downloadAllDatasetFilesWithGuestbookResponse(persistentId, apiToken, guestbookResponse);
+        downloadResponse.prettyPrint();
+        assertEquals(OK.getStatusCode(), signedUrlResponse.getStatusCode());
+        signedUrl = UtilIT.getSignedUrlFromResponse(downloadResponse);
+        signedUrlResponse = get(signedUrl);
+        assertEquals(OK.getStatusCode(), signedUrlResponse.getStatusCode());
+
         downloadResponse = UtilIT.downloadFilesUrlWithGuestbookResponse(new Integer[]{fileId1, fileId2, fileId3}, apiToken, guestbookResponse);
         signedUrl = UtilIT.getSignedUrlFromResponse(downloadResponse);
         signedUrlResponse = get(signedUrl);
@@ -4044,13 +4064,18 @@ public void testDownloadFileWithGuestbookResponse() throws IOException, JsonPars
         Response guestbookResponses = UtilIT.getGuestbookResponses(dataverseAlias, guestbook.getId(), ownerApiToken);
         assertTrue(guestbookResponses.prettyPrint().contains("My Name," + user2Email + ",My Institution,My Position"));
 
-        // Get Signed Download Url with guestbook response using persistentId
-        // POST /api/access/dataset/:persistentId?persistentId=doi:10.xxxx/FK2/ABC
-        downloadResponse = UtilIT.downloadFilesUrlWithGuestbookResponse(persistentId, apiToken, guestbookResponse);
+        // Get Signed Download Url for guest with guestbook response using file's persistentId
+        // POST /api/access/datafile/:persistentId?persistentId=
+        downloadResponse = UtilIT.downloadFilesUrlWithGuestbookResponse(filePersistentId, null, guestbookResponseForGuest);
         downloadResponse.prettyPrint();
         downloadResponse.then().assertThat()
                 .statusCode(OK.getStatusCode());
         signedUrl = UtilIT.getSignedUrlFromResponse(downloadResponse);
+        // verify that the fileId is correct
+        assertTrue(signedUrl.contains("/access/datafile/" + fileId4 + "?"));
+        // verify that the persistentId is no longer in the url
+        assertFalse(signedUrl.contains("persistentId"));
+        // verify that the signed url is good
         signedUrlResponse = get(signedUrl);
         assertEquals(OK.getStatusCode(), signedUrlResponse.getStatusCode());
     }
@@ -4141,6 +4166,7 @@ public void testGetFileCitationFormatted() {
     @Disabled
     public void testDownloadFileWithGuestbookResponseUsingBearerToken() throws IOException, JsonParseException {
         msgt("testDownloadFileWithGuestbookResponseUsingBearerToken");
+        UtilIT.enableSetting(SettingsServiceBean.Key.FilePIDsEnabled);
         // Create superuser
         Response createUserResponse = UtilIT.createRandomUser();
         assertEquals(200, createUserResponse.getStatusCode());
@@ -4166,7 +4192,7 @@ public void testDownloadFileWithGuestbookResponseUsingBearerToken() throws IOExc
         JsonObjectBuilder json1 = Json.createObjectBuilder().add("description", "my description1").add("directoryLabel", "data/subdir1").add("categories", Json.createArrayBuilder().add("Data"));
         Response uploadResponse = UtilIT.uploadFileViaNative(datasetId.toString(), "src/main/webapp/resources/images/dataverseproject.png", json1.build(), ownerApiToken);
         uploadResponse.then().assertThat().statusCode(OK.getStatusCode());
-        Integer fileId1 = JsonPath.from(uploadResponse.body().asString()).getInt("data.files[0].dataFile.id");
+        String filePersistentId = JsonPath.from(uploadResponse.body().asString()).getString("data.files[0].dataFile.persistentId");
 
         // Publish dataverse and dataset
         Response publishDataverse = UtilIT.publishDataverseViaNativeApi(dataverseAlias, ownerApiToken);
@@ -4220,7 +4246,7 @@ public void testDownloadFileWithGuestbookResponseUsingBearerToken() throws IOExc
         // POST with guestbook response
         String guestbookResponse = UtilIT.generateGuestbookResponse(guestbook).replace("\"guestbookResponse\": {",
                 "\"guestbookResponse\": { \"name\":\"My Name\", \"position\":\"My Position\", \"institution\":\"My Institution\",");
-        Response downloadResponse = UtilIT.downloadFilesUrlWithGuestbookResponse(persistentId, null, guestbookResponse, userWithClaimsAccessToken);
+        Response downloadResponse = UtilIT.downloadFilesUrlWithGuestbookResponse(filePersistentId,null, guestbookResponse, userWithClaimsAccessToken);
         downloadResponse.prettyPrint();
         downloadResponse.then().assertThat()
                 .statusCode(OK.getStatusCode());

src/test/java/edu/harvard/iq/dataverse/api/UtilIT.java

@@ -1302,8 +1302,19 @@ static Response downloadFilesUrlWithGuestbookResponse(String persistentId, Strin
         if (body != null) {
             requestSpecification.body(body);
         }
-        String getString = "/api/access/dataset/:persistentId?persistentId=" + persistentId;
-        return requestSpecification.post(getString);
+        String postString = "/api/access/datafile/:persistentId?persistentId=" + persistentId;
+        return requestSpecification.post(postString);
+    }
+    static Response downloadAllDatasetFilesWithGuestbookResponse(String persistentId, String apiToken, String body) {
+        RequestSpecification requestSpecification = given();
+        if (apiToken != null) {
+            requestSpecification.header(API_TOKEN_HTTP_HEADER, apiToken);
+        }
+        if (body != null) {
+            requestSpecification.body(body);
+        }
+        String postString = "/api/access/dataset/:persistentId?persistentId=" + persistentId;
+        return requestSpecification.post(postString);
     }
 
     static Response postDownloadDatafiles(String body, String apiToken) {