Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
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.
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import edu.harvard.iq.dataverse.authorization.Permission;
import edu.harvard.iq.dataverse.authorization.users.ApiToken;
import edu.harvard.iq.dataverse.authorization.users.AuthenticatedUser;
import edu.harvard.iq.dataverse.authorization.users.PrivateUrlUser;
import edu.harvard.iq.dataverse.authorization.users.User;
import edu.harvard.iq.dataverse.dataaccess.DataAccess;
import edu.harvard.iq.dataverse.dataaccess.StorageIO;
Expand Down Expand Up @@ -31,7 +32,6 @@
import java.sql.Timestamp;
import java.util.*;
import java.util.logging.Logger;
//import org.primefaces.context.RequestContext;

/**
*
Expand Down Expand Up @@ -314,6 +314,10 @@
} else {
logger.fine("Redirecting to file download url: " + fileDownloadUrl);
try {
User user = session.getUser();
if (user != null && (user instanceof PrivateUrlUser)) {

Check warning on line 318 in src/main/java/edu/harvard/iq/dataverse/FileDownloadServiceBean.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Replace this instanceof check and cast with 'instanceof PrivateUrlUser privateurluser'

See more on https://sonarcloud.io/project/issues?id=IQSS_dataverse&issues=AZ-6Eli_HtWh4JupVvYJ&open=AZ-6Eli_HtWh4JupVvYJ&pullRequest=12548

Check warning on line 318 in src/main/java/edu/harvard/iq/dataverse/FileDownloadServiceBean.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Remove this unnecessary null check; "instanceof" returns false for nulls.

See more on https://sonarcloud.io/project/issues?id=IQSS_dataverse&issues=AZ-6Eli_HtWh4JupVvYK&open=AZ-6Eli_HtWh4JupVvYK&pullRequest=12548
fileDownloadUrl = fileDownloadUrl + ((fileDownloadUrl.contains("?")) ? "&" : "?") + "key=" + ((PrivateUrlUser) user).getToken();
}
FacesContext.getCurrentInstance().getExternalContext().redirect(fileDownloadUrl);
} catch (IOException ex) {
logger.info("Failed to issue a redirect to file download url (" + fileDownloadUrl + "): " + ex);
Expand All @@ -337,7 +341,7 @@
logger.info("Failed to issue a redirect to aux file download url (" + fileDownloadUrl + "): " + ex);
}
}

/**
* Launch an "explore" tool which is a type of ExternalTool such as
* Data Explorer. This method may be invoked directly from the
Expand Down
12 changes: 7 additions & 5 deletions src/main/java/edu/harvard/iq/dataverse/api/Access.java
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 @@ -2237,7 +2234,12 @@
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();
boolean exempt = false;
// PrivateUrlUser access to draft files is exempt from guestbook responses in JSF https://github.com/IQSS/dataverse/issues/12535
if (user instanceof PrivateUrlUser) {

Check warning on line 2239 in src/main/java/edu/harvard/iq/dataverse/api/Access.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Replace this instanceof check and cast with 'instanceof PrivateUrlUser privateurluser'

See more on https://sonarcloud.io/project/issues?id=IQSS_dataverse&issues=AZ-6ElgBHtWh4JupVvYI&open=AZ-6ElgBHtWh4JupVvYI&pullRequest=12548

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 decided to run a debugger - here's the core issue. For both Authenticated and Private URL users, user is Guest at this point (so exempt is always false). Before the check to see if there's an authenticated user with permissions below, getRequestor(user) is called, which returns the authenticateduser in that case. In the privateUrl user case, getRequestor(user) will return the PrivateUrlUser, which would allow the check currently on line 2239 to succeed for that case. So I'd suggest a reshuffle - go back to seeing if a guestbook response is required first, and, if it is, get the requestor and make both the authenticated and private url user checks against the requestor.

With this change, there's no need to store the PrivateUrlUser token or generate a URL with the key in it and those changes can be removed.

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.

The private user token needs to be passed to the api as &key= in order for the ApiKeyAuthMechanism to recognize the user is a private user instead of a guest.

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.

See #12569 - unless I made a typo merging this from QDR, I think this solves the issue with downloads from within JSF, building on your work in #12110, that you were trying to address here via signing or now adding the privateURL key.

exempt = (df.getOwner().getId() == ((PrivateUrlUser) user).getDatasetId());
}
boolean required = !exempt && df.getOwner().hasEnabledGuestbook() && !d.getEffectiveGuestbookEntryAtRequest();
boolean wasWrittenInPost = false;
if (required) {
User requestor = getRequestor(user);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@ public class PrivateUrlUser implements User {
* is a DvObject.
*/
private final long datasetId;
private final boolean anonymizedAccess;
private final boolean anonymizedAccess;
private String token = null;

public PrivateUrlUser(long datasetId) {
this(datasetId, false);
Expand All @@ -38,7 +39,14 @@ public long getDatasetId() {
public boolean hasAnonymizedAccess() {
return anonymizedAccess;
}


public String getToken() {
return token;
}
public void setToken(String token) {
this.token = token;
}

/**
* By always returning false for isAuthenticated(), we prevent a
* name from appearing in the corner as well as preventing an account page
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,15 +51,23 @@ public PrivateUrl getPrivateUrlFromDatasetId(long datasetId) {
* @return A PrivateUrlUser if one can be found using the token or null.
*/
public PrivateUrlUser getPrivateUrlUserFromToken(String token) {
return PrivateUrlUtil.getPrivateUrlUserFromRoleAssignment(getRoleAssignmentFromPrivateUrlToken(token));
PrivateUrlUser user = PrivateUrlUtil.getPrivateUrlUserFromRoleAssignment(getRoleAssignmentFromPrivateUrlToken(token));
if (user != null) {
user.setToken(token);
}
return user;
}

/**
* @return PrivateUrlRedirectData if it can be found using the token or
* null.
*/
public PrivateUrlRedirectData getPrivateUrlRedirectDataFromToken(String token) {
return PrivateUrlUtil.getPrivateUrlRedirectData(getRoleAssignmentFromPrivateUrlToken(token));
PrivateUrlRedirectData privateUrlRedirectData = PrivateUrlUtil.getPrivateUrlRedirectData(getRoleAssignmentFromPrivateUrlToken(token));
if (privateUrlRedirectData != null && privateUrlRedirectData.getPrivateUrlUser() != null) {
privateUrlRedirectData.getPrivateUrlUser().setToken(token);
}
return privateUrlRedirectData;
}

/**
Expand Down
Loading