Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
stevenwinship wants to merge 8 commits into
base: develop
Choose a base branch
from
+26 −27
Open

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
c38c37e
Allow Preview User to download files without requiring a Guestbook Re…
stevenwinship Aug 3, 2026
7723ac1
fix npe SessionCookieAuthMechanism
stevenwinship Aug 3, 2026
aa4a1c8
change perm to viewUnpublishedDataset from editDataset
stevenwinship Aug 3, 2026
5d04c58
refactor isAccessApi
stevenwinship Aug 4, 2026
9e953bc
fix for npe in unit test
stevenwinship Aug 4, 2026
131ecb6
add LF fix info
qqmyers Aug 5, 2026
cf7d78c
adding fixes per review comments
stevenwinship Aug 5, 2026
f01525d
broaden access api to access/ without limiting to access/datafile
stevenwinship Aug 7, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions doc/release-notes/12535-download-without-guestbook-response-for-preview-user2.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
## Bug ##
Preview URL users could not download files from the dataset being previewed if a guestbook was assigned to that dataset. This is now fixed. A similar issue with Locally FAIR content is also fixed.
41 changes: 15 additions & 26 deletions src/main/java/edu/harvard/iq/dataverse/api/Access.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,7 @@
import edu.harvard.iq.dataverse.authorization.DataverseRole;
import edu.harvard.iq.dataverse.authorization.Permission;
import edu.harvard.iq.dataverse.authorization.RoleAssignee;
import edu.harvard.iq.dataverse.authorization.users.ApiToken;
import edu.harvard.iq.dataverse.authorization.users.AuthenticatedUser;
import edu.harvard.iq.dataverse.authorization.users.GuestUser;
import edu.harvard.iq.dataverse.authorization.users.User;
import edu.harvard.iq.dataverse.authorization.users.*;
import edu.harvard.iq.dataverse.dataaccess.*;
import edu.harvard.iq.dataverse.datavariable.DataVariable;
import edu.harvard.iq.dataverse.datavariable.VariableServiceBean;
Expand Down Expand Up @@ -197,7 +194,7 @@ public BundleDownloadInstance datafileBundle(@Context ContainerRequestContext cr

if (gbrecs != true && df.isReleased()) {
// Write Guestbook record if not done previously and file is released
GuestbookResponse gbr = guestbookResponseService.initAPIGuestbookResponse(df.getOwner(), df, session, getRequestor(req.getUser()));
GuestbookResponse gbr = guestbookResponseService.initAPIGuestbookResponse(df.getOwner(), df, session, req.getUser());
guestbookResponseService.save(gbr);
MakeDataCountEntry entry = new MakeDataCountEntry(uriInfo, headers, dvRequestService, df);
mdcLogService.logEntry(entry);
Expand Down Expand Up @@ -327,7 +324,7 @@ public Response datafile(@Context ContainerRequestContext crc,

if (gbrecs != true && df.isReleased()){
// Write Guestbook record if not done previously and file is released
gbr = guestbookResponseService.initAPIGuestbookResponse(df.getOwner(), df, session, getRequestor(req.getUser()));
gbr = guestbookResponseService.initAPIGuestbookResponse(df.getOwner(), df, session, req.getUser());
}

DownloadInfo dInfo = new DownloadInfo(df);
Expand Down Expand Up @@ -1234,7 +1231,7 @@ private Response downloadDatafiles(ContainerRequestContext crc, String body, boo
String customZipServiceUrl = settingsService.getValueForKey(SettingsServiceBean.Key.CustomZipDownloadServiceUrl);
boolean useCustomZipService = customZipServiceUrl != null;

User user = getRequestor(getRequestUser(crc));
User user = getRequestUser(crc);
DataverseRequest req = createDataverseRequest(user);

Boolean getOrig = false;
Expand Down Expand Up @@ -2236,29 +2233,30 @@ public Response getUserPermissionsOnFile(@Context ContainerRequestContext crc,

private boolean checkGuestbookRequiredResponse(User user, UriInfo uriInfo, DataFile df, String gbrids) throws WebApplicationException {
// Check if guestbook response is required
Dataset d = df.getOwner();
boolean required = df.getOwner().hasEnabledGuestbook() && !d.getEffectiveGuestbookEntryAtRequest();
Dataset ds = df.getOwner();
boolean required = ds.hasEnabledGuestbook() && !ds.getEffectiveGuestbookEntryAtRequest();
boolean wasWrittenInPost = false;
if (required) {
User requestor = getRequestor(user);
if (requestor instanceof AuthenticatedUser && permissionService.userOn(requestor, df.getOwner()).has(Permission.EditDataset)) {
required = false;
checkAuthorization(user, df);
// PrivateUrlUsers are exempt from this requirement
if (user instanceof PrivateUrlUser) {
return false;

@qqmyers qqmyers Aug 5, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@stevenwinship stevenwinship Aug 5, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added checkAuthorization

@qqmyers qqmyers Aug 6, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@qqmyers qqmyers Aug 7, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

}
// Check if we are downloading a thumbnail image which doesn't require a guestbook response
boolean imageThumb = uriInfo.getQueryParameters().containsKey("imageThumb");
if (imageThumb) {
required = false;
return false;
}

if (required && gbrids != null && !gbrids.isEmpty()) {
if (gbrids != null && !gbrids.isEmpty()) {
try {
// verify that this id is good
GuestbookResponse gbr = guestbookResponseService.findById(Long.valueOf(gbrids));
if (gbr == null) {
throw new NotFoundException("GuestbookResponse Not Found for id:" + gbrids);
}
Long delta = Instant.now().toEpochMilli() - gbr.getResponseTime().getTime();
wasWrittenInPost = gbr.getDataset().getId().equals(df.getOwner().getId()) && delta <= (GUESTBOOK_RESPONSE_SIGNEDURL_TIMEOUT_MINUTES * 60000L);
wasWrittenInPost = gbr.getDataset().getId().equals(ds.getId()) && delta <= (GUESTBOOK_RESPONSE_SIGNEDURL_TIMEOUT_MINUTES * 60000L);
} catch (NumberFormatException | DateTimeParseException ex) {
throw new BadRequestException(ex.getMessage());
}
Expand All @@ -2283,20 +2281,11 @@ private GuestbookResponse getGuestbookResponseFromBody(DataFile dataFile, String

// checkAuthorization is a convenience method; it calls the boolean method
// isAccessAuthorized(), the actual workhorse, and throws a 403 exception if not.
private void checkAuthorization(User initialUser, DataFile df) throws WebApplicationException {
User user = getRequestor(initialUser);
if (!isAccessAuthorized(user, df)) {
private void checkAuthorization(User requestUser, DataFile df) throws WebApplicationException {
if (!isAccessAuthorized(requestUser, df)) {
throw new ForbiddenException();
}
}
private User getRequestor(User user) {
// CompoundAuthMechanism should find the user by API Key/Token, Workflow, etc. And for SPA the Bearer Token
// For JSF check if CompoundAuthMechanism couldn't find the user then try to get it from the session
if (session!=null && user instanceof GuestUser) {
user = session.getUser();
}
return user;
}

private boolean isAccessAuthorized(User requestUser, DataFile df) {
// First, check if the file belongs to a released Dataset version:
Expand Down
10 changes: 9 additions & 1 deletion src/main/java/edu/harvard/iq/dataverse/api/auth/SessionCookieAuthMechanism.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,19 @@ public class SessionCookieAuthMechanism implements AuthMechanism {
@Inject
DataverseSession session;

public static final String ACCESS_PATH_PREFIX = "access/";

@Override
public User findUserFromRequest(ContainerRequestContext containerRequestContext) throws WrappedAuthErrorResponse {
if (FeatureFlags.API_SESSION_AUTH.enabled()) {
if (FeatureFlags.API_SESSION_AUTH.enabled() || isAccessApi(containerRequestContext)) {
return session.getUser();
}
return null;
}

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

@qqmyers qqmyers Aug 7, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

requestPath.startsWith(ACCESS_PATH_PREFIX));
}
}
Loading