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
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: 25.005% (+0.003%) from 25.002% — 12535-download-without-guestbook-response-for-preview-user2 into develop

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

Test Results

403 tests ±0 388 ✅ +1 33m 40s ⏱️ - 5m 0s
55 suites ±0 15 💤 ±0
55 files ±0 0 ❌ - 1

Results for commit f01525d. ± Comparison against base commit ad7ada1.

♻️ 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-05T15:02:23Z

-                required = false;
+            // PrivateUrlUsers are exempt from this requirement
+            if (user instanceof PrivateUrlUser) {
+                return false;


I think you still need a check for Authenticated users though, since they may have gotten here with either ViewUnpublished (where they wouldn't make a gbr) or FIle download perms (where they would be required). (As you said, PrivateUrlUsers can skip that check because the checkAuth guarantees they have the ViewUnpublished perm).

added checkAuthorization

Not seeing a change? I'd expect something like if ((user instanceof PrivateUrlUser) || (user instanceof AuthenticatedUser && permissionService.userOn(user, ds).has(Permission.ViewUnpublishedDataset)))

I see checkAuthorization - that assures that an AuthenticatedUser has either ViewUnpublished (which means no gbr required) or FileDownload (which would). So calling it here doesn't help distinguish the two cases and you need to explicitly check for the perm that would result in returning false.

…sponse

qqmyers · 2026-08-06T14:41:40Z

    @Inject
    DataverseSession session;

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


I think you need to allow broader access, e.g. to the access/dataset path as well (that could apply to the api key issue as well)? Since the access api overall is nominally about files, I'm not sure you need to check beyond "access/".

sonarqubecloud · 2026-08-07T13:27:27Z

Quality Gate failed

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

See analysis details on SonarQube Cloud

github-actions · 2026-08-07T13:40:50Z

📦 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.

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.)

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

This comment has been minimized.

Sign in to view

stevenwinship moved this from In Progress 💻 to Ready for Review ⏩ in IQSS Dataverse Project Aug 4, 2026

stevenwinship removed their assignment Aug 4, 2026

qqmyers requested changes Aug 5, 2026

View reviewed changes

qqmyers moved this from Ready for Review ⏩ to In Review 🔎 in IQSS Dataverse Project Aug 5, 2026

qqmyers assigned stevenwinship Aug 5, 2026

This comment has been minimized.

Sign in to view

stevenwinship force-pushed the 12535-download-without-guestbook-response-for-preview-user2 branch from aaf3218 to 477ed98 Compare August 5, 2026 19:03

This comment has been minimized.

Sign in to view

stevenwinship added 2 commits August 5, 2026 17:09

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

c38c37e

…sponse

fix npe SessionCookieAuthMechanism

7723ac1

stevenwinship and others added 5 commits August 5, 2026 17:09

change perm to viewUnpublishedDataset from editDataset

aa4a1c8

refactor isAccessApi

5d04c58

fix for npe in unit test

9e953bc

add LF fix info

131ecb6

adding fixes per review comments

cf7d78c

stevenwinship force-pushed the 12535-download-without-guestbook-response-for-preview-user2 branch from 477ed98 to cf7d78c Compare August 5, 2026 21:09

This comment has been minimized.

Sign in to view

stevenwinship assigned qqmyers and unassigned stevenwinship Aug 6, 2026

qqmyers reviewed Aug 6, 2026

View reviewed changes

qqmyers assigned stevenwinship Aug 6, 2026

broaden access api to access/ without limiting to access/datafile

f01525d

stevenwinship removed their assignment Aug 7, 2026

qqmyers reviewed Aug 7, 2026

View reviewed changes

qqmyers mentioned this pull request Aug 7, 2026

Write batches of GuestBookResponses for performance #12479

Open

Uh oh!

Conversation

stevenwinship commented Aug 3, 2026

Uh oh!

stevenwinship commented Aug 3, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stevenwinship Aug 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

coveralls commented Aug 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment has been minimized.

This comment has been minimized.

github-actions Bot commented Aug 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Results

Uh oh!

This comment has been minimized.

This comment has been minimized.

qqmyers left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

qqmyers Aug 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

qqmyers Aug 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud Bot commented Aug 7, 2026

Quality Gate failed

Uh oh!

github-actions Bot commented Aug 7, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

stevenwinship Aug 3, 2026 •

edited

Loading

coveralls commented Aug 3, 2026 •

edited

Loading

github-actions Bot commented Aug 3, 2026 •

edited

Loading

qqmyers Aug 5, 2026 •

edited

Loading

qqmyers Aug 6, 2026 •

edited

Loading