Allow Preview User to download files without requiring a Guestbook Re… by stevenwinship · Pull Request #12584 · IQSS/dataverse

stevenwinship · 2026-08-03T20:26:44Z

The guestbook popup does not appear to collect a response before the download attempt is made. Disabling the guestbook on the dataset resolves the issue and files download normally through the Preview URL. This fix reinstates the behavior of the JSF UI from prior versions of Dataverse.

Which issue(s) this PR closes:#12535

Closes #12535
Closes #12579
Special notes for your reviewer:

Suggestions on how to test this: Create a dataset with a guestbook. Generate a Preview URL. Using the preview url try to download files and dataset zip file. This should work without requiring the guestbook response.

Does this PR introduce a user interface change? If mockups are available, please link/include them here:

Is there a release notes update needed for this change?: Included

Additional documentation:

stevenwinship · 2026-08-03T20:30:01Z

This PR replaces #12548

qqmyers · 2026-08-03T20:31:51Z

-        Dataset d = df.getOwner();
-        boolean required = df.getOwner().hasEnabledGuestbook() && !d.getEffectiveGuestbookEntryAtRequest();
+        Dataset ds = df.getOwner();
+        boolean required = ds.hasEnabledGuestbook() && !ds.getEffectiveGuestbookEntryAtRequest() && !(user instanceof PrivateUrlUser);


I think this skips checking if the PrivateUrlUser has access to this dataset?

That happens in checkAuthorization which is called before checkGuestbookRequiredResponse

OK - fair. I'd suggest moving this down so required is just about the guestbooks and then, in your if (required) block, handle the PreviewUrlUser and Authenticated user separately with a note - PreviewUrlUsers don't need another check because they can only be downloading because they have the ViewUnpublished perm whereas authenticatedUsers who can download may have that perm or FileDownload perm, so need to distinguish those two cases here.

Made a change

qqmyers · 2026-08-03T20:33:20Z

        if (required) {
-            User requestor = getRequestor(user);
-            if (requestor instanceof AuthenticatedUser && permissionService.userOn(requestor, df.getOwner()).has(Permission.EditDataset)) {
+            if (user instanceof AuthenticatedUser && permissionService.userOn(user, ds).has(Permission.EditDataset)) {


Versus if you change here to drop the instanceof AuthenticatedUser part, this permissionService check should verify the PrivateUrlUser has an assignment on this dataset. (And this would be the perm I suggested might be ViewUnpublishedDataset to match the overall access check.)

changed to ViewUnpublishedDataset

coveralls · 2026-08-03T21:02:41Z

Coverage is 25.013% — 12535-download-without-guestbook-response-for-preview-user2 into develop. No base build found for develop.

github-actions · 2026-08-03T21:37:42Z

Test Results

404 tests ±0 389 ✅ ±0 37m 11s ⏱️ +11s
55 suites ±0 15 💤 ±0
55 files ±0 0 ❌ ±0

Results for commit 43c47e1. ± Comparison against base commit e28af3f.

♻️ This comment has been updated with latest results.

qqmyers

Getting close - this simplifies and fixes the checkGuestbook required logic which should fix both the previewUrlUser and LFAIR issues (so this should close the LFAIR issue too).
I noted two issues to fix in other comments and added LFAIR to the release note.

qqmyers · 2026-08-05T14:58:18Z

+    private boolean isAccessApi(ContainerRequestContext containerRequestContext) {
+        return "GET".equalsIgnoreCase(containerRequestContext.getMethod())
+                && containerRequestContext.getUriInfo() != null
+                && containerRequestContext.getUriInfo().getPath().toLowerCase().startsWith("access/");


The ApiKey auth checks for the path starting with / - is that needed here too? (using final static constant as well)

dataverse/src/main/java/edu/harvard/iq/dataverse/api/auth/ApiKeyAuthMechanism.java

Line 24 in 2d2867d

public static final String ACCESS_DATAFILE_PATH_PREFIX = "/access/datafile/";

How does that even work? The path doesn't have a leading '/'

containerRequestContext.getUriInfo().getPath() access/datafile/4|#]

Good catch! Not sure it does - could be review/QA 0f #9303 didn't catch the problem. If you're not seeing a leading /, we are probably leaving the other APIs open to users of anonymized preview url users when we shouldn't - up to you whether you want to try fixing that here (just removing the / I think) and adding that to QA or an automated test scenario, or just reporting as a separate bug.

fixed SessionCookie but not ApiKeyAuth

I'm looking into ApiKeyAuth separately

qqmyers · 2026-08-07T17:33:23Z

+
+    private boolean isAccessApi(ContainerRequestContext containerRequestContext) {
+        String requestPath = containerRequestContext.getUriInfo() != null ? containerRequestContext.getUriInfo().getPath() : "";
+        return ("GET".equalsIgnoreCase(containerRequestContext.getMethod()) &&


At least the main Access Dataset /DownloadZip menu item calls the POST API, so PrivateURLUsers also need access to that. (Since the browser blocks other sites from sending our cookie for POST/PUT/etc. this shouldn't raise cross-site issues.)

Fixed in #12479 which pulls in the commits from here.

…sponse

sonarqubecloud · 2026-08-14T19:40:00Z

Quality Gate failed

Failed conditions
19.6% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud

github-actions · 2026-08-14T19:51:46Z

📦 Pushed preview images as

ghcr.io/gdcc/dataverse:12535-download-without-guestbook-response-for-preview-user2

ghcr.io/gdcc/configbaker:12535-download-without-guestbook-response-for-preview-user2

🚢 See on GHCR. Use by referencing with full name as printed above, mind the registry name.

jp-tosca · 2026-08-17T15:14:06Z

There was a comment on 8/17 that this PR #12479 could close any issues related to this one.

landreev · 2026-08-17T20:48:33Z

To confirm, we were not planning to merge #12479 instead of this pr, were we? - It looks like there was more done here, since the branch was merged into GlobalDataverseCommunityConsortium:WriteMultipleGBsAtOnce.

qqmyers · 2026-08-17T21:49:03Z

@landreev - yes, I did suggest not merging this one. As far as I can tell, the changes after I merged are behind #12479, e.g. the changes in this commit were what I started from in working on c43acb3 and #12479 has a fix to the Filter logic that is still an open comment/request in #12584: https://github.com/IQSS/dataverse/pull/12584/changes#r3759592780 .

For QA, #12479 should be checked against the same two issues as #12584 was for, along with performance testing writing guestbookresponses for many files.

landreev · 2026-08-17T22:29:38Z

If we can avoid merging this pr, that's great news. Otherwise it would be a recipe for merge conflicts (I think?) - that was the context of my question.

For QA, #12479 should be checked against the same two issues as #12584 was for, along with performance testing writing guestbookresponses for many files.

OK, I see now that #12479 closes the same 2 issues as this pr.
I'll go ahead and copy-and-paste the line from under "How to test" here into #12479 as well.

@landreev - yes, I did suggest not merging this one. As far as I can tell, the changes after I merged are behind #12479, e.g. the changes in this commit were what I started from in working on c43acb3 and #12479 has a fix to the Filter logic that is still an open comment/request in #12584: https://github.com/IQSS/dataverse/pull/12584/changes#r3759592780 .

stevenwinship self-assigned this Aug 3, 2026

stevenwinship added this to IQSS Dataverse Project Aug 3, 2026

github-actions Bot added FY27 Sprint 1 FY27 Sprint 1 (2026-07-01 - 2026-07-15) FY27 Sprint 2 FY27 Sprint 2 (2026-07-15 - 2026-07-29) Original size: 20 Type: Bug a defect labels Aug 3, 2026

stevenwinship moved this to In Progress 💻 in IQSS Dataverse Project Aug 3, 2026

stevenwinship added this to the 6.12 milestone Aug 3, 2026

stevenwinship mentioned this pull request Aug 3, 2026

Allow Preview User to download files without requiring a Guestbook Re… #12548

Closed

qqmyers reviewed Aug 3, 2026

View reviewed changes

github-advanced-security AI found potential problems Aug 3, 2026

View reviewed changes

Comment thread src/main/java/edu/harvard/iq/dataverse/api/auth/SessionCookieAuthMechanism.java Fixed