Skip to content
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

Define/use an additional secret key, Refactor token replacement for signed urls #8802

Merged
merged 2 commits into from
Jun 23, 2022
Merged

Define/use an additional secret key, Refactor token replacement for signed urls #8802

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
881e3db
define/use an additional secret key
qqmyers Jun 14, 2022
208ab95
refactor to allow URL token substitution outside tools framework
qqmyers Jun 21, 2022
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: 1 addition & 1 deletion src/main/java/edu/harvard/iq/dataverse/api/AbstractApiBean.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -440,7 +440,7 @@ private AuthenticatedUser getAuthenticatedUserFromSignedUrl() {
//ToDo - add null checks/ verify that calling methods catch things.
String user = httpRequest.getParameter("user");
AuthenticatedUser targetUser = authSvc.getAuthenticatedUser(user);
String key = authSvc.findApiTokenByUser(targetUser).getTokenString();
String key = System.getProperty(SystemConfig.API_SIGNING_SECRET,"") + authSvc.findApiTokenByUser(targetUser).getTokenString();
String signedUrl = httpRequest.getRequestURL().toString();
String method = httpRequest.getMethod();

Expand Down
2 changes: 1 addition & 1 deletion src/main/java/edu/harvard/iq/dataverse/api/Admin.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2088,7 +2088,7 @@ public Response getSignedUrl(JsonObject urlInfo) throws WrappedResponse {
userId=superuser.getIdentifier();
//We ~know this exists - the superuser just used it and it was unexpired/not disabled. (ToDo - if we want this to work with workflow tokens (or as a signed URL, we should do more checking as for the user above))
}
key = authSvc.findApiTokenByUser(superuser).getTokenString();
key = System.getProperty(SystemConfig.API_SIGNING_SECRET,"") + authSvc.findApiTokenByUser(superuser).getTokenString();
}
if(key==null) {
return error(Response.Status.CONFLICT, "Do not have a valid user with apiToken");
Expand Down
58 changes: 0 additions & 58 deletions src/main/java/edu/harvard/iq/dataverse/externaltools/ExternalTool.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -299,64 +299,6 @@ public JsonObjectBuilder toJson() {
return jab;
}

public enum ReservedWord {

// TODO: Research if a format like "{reservedWord}" is easily parse-able or if another format would be
// better. The choice of curly braces is somewhat arbitrary, but has been observed in documenation for
// various REST APIs. For example, "Variable substitutions will be made when a variable is named in {brackets}."
// from https://swagger.io/specification/#fixed-fields-29 but that's for URLs.
FILE_ID("fileId"),
FILE_PID("filePid"),
SITE_URL("siteUrl"),
API_TOKEN("apiToken"),
// datasetId is the database id
DATASET_ID("datasetId"),
// datasetPid is the DOI or Handle
DATASET_PID("datasetPid"),
DATASET_VERSION("datasetVersion"),
FILE_METADATA_ID("fileMetadataId"),
LOCALE_CODE("localeCode"),
ALLOWED_URLS("allowedUrls");

private final String text;
private final String START = "{";
private final String END = "}";

private ReservedWord(final String text) {
this.text = START + text + END;
}

/**
* This is a centralized method that enforces that only reserved words
* are allowed to be used by external tools. External tool authors
* cannot pass their own query parameters through Dataverse such as
* "mode=mode1".
*
* @throws IllegalArgumentException
*/
public static ReservedWord fromString(String text) throws IllegalArgumentException {
if (text != null) {
for (ReservedWord reservedWord : ReservedWord.values()) {
if (text.equals(reservedWord.text)) {
return reservedWord;
}
}
}
// TODO: Consider switching to a more informative message that enumerates the valid reserved words.
boolean moreInformativeMessage = false;
if (moreInformativeMessage) {
throw new IllegalArgumentException("Unknown reserved word: " + text + ". A reserved word must be one of these values: " + Arrays.asList(ReservedWord.values()) + ".");
} else {
throw new IllegalArgumentException("Unknown reserved word: " + text);
}
}

@Override
public String toString() {
return text;
}
}

public String getDescriptionLang() {
String description = "";
if (this.toolName != null) {
Expand Down
116 changes: 6 additions & 110 deletions src/main/java/edu/harvard/iq/dataverse/externaltools/ExternalToolHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,9 @@
import edu.harvard.iq.dataverse.DataFile;
import edu.harvard.iq.dataverse.Dataset;
import edu.harvard.iq.dataverse.FileMetadata;
import edu.harvard.iq.dataverse.GlobalId;
import edu.harvard.iq.dataverse.authorization.users.ApiToken;
import edu.harvard.iq.dataverse.externaltools.ExternalTool.ReservedWord;
import edu.harvard.iq.dataverse.util.SystemConfig;
import edu.harvard.iq.dataverse.util.URLTokenUtil;

import edu.harvard.iq.dataverse.util.UrlSignerUtil;
import java.io.IOException;
import java.io.StringReader;
Expand All @@ -19,6 +18,7 @@
import java.util.List;
import java.util.logging.Level;
import java.util.logging.Logger;

import javax.json.Json;
import javax.json.JsonArray;
import javax.json.JsonObject;
Expand All @@ -31,8 +31,7 @@
* instantiated. Applies logic based on an {@link ExternalTool} specification,
* such as constructing a URL to access that file.
*/
public class ExternalToolHandler {

public class ExternalToolHandler extends URLTokenUtil {
/**
* @return the allowedUrls
*/
Expand All @@ -54,15 +53,8 @@ public void setUser(String user) {
this.user = user;
}

private static final Logger logger = Logger.getLogger(ExternalToolHandler.class.getCanonicalName());

private final ExternalTool externalTool;
private final DataFile dataFile;
private final Dataset dataset;
private final FileMetadata fileMetadata;

private ApiToken apiToken;
private String localeCode;
private String requestMethod;
private String toolContext;
private String user;
Expand All @@ -78,23 +70,9 @@ public void setUser(String user) {
* used anonymously.
*/
public ExternalToolHandler(ExternalTool externalTool, DataFile dataFile, ApiToken apiToken, FileMetadata fileMetadata, String localeCode) {
super(dataFile, apiToken, fileMetadata, localeCode);
this.externalTool = externalTool;
toolContext = externalTool.getToolUrl();
if (dataFile == null) {
String error = "A DataFile is required.";
logger.warning("Error in ExternalToolHandler constructor: " + error);
throw new IllegalArgumentException(error);
}
if (fileMetadata == null) {
String error = "A FileMetadata is required.";
logger.warning("Error in ExternalToolHandler constructor: " + error);
throw new IllegalArgumentException(error);
}
this.dataFile = dataFile;
this.apiToken = apiToken;
this.fileMetadata = fileMetadata;
dataset = fileMetadata.getDatasetVersion().getDataset();
this.localeCode = localeCode;
}

/**
Expand All @@ -106,33 +84,8 @@ public ExternalToolHandler(ExternalTool externalTool, DataFile dataFile, ApiToke
* used anonymously.
*/
public ExternalToolHandler(ExternalTool externalTool, Dataset dataset, ApiToken apiToken, String localeCode) {
super(dataset, apiToken, localeCode);
this.externalTool = externalTool;
if (dataset == null) {
String error = "A Dataset is required.";
logger.warning("Error in ExternalToolHandler constructor: " + error);
throw new IllegalArgumentException(error);
}
this.dataset = dataset;
this.apiToken = apiToken;
this.dataFile = null;
this.fileMetadata = null;
this.localeCode = localeCode;
}

public DataFile getDataFile() {
return dataFile;
}

public FileMetadata getFileMetadata() {
return fileMetadata;
}

public ApiToken getApiToken() {
return apiToken;
}

public String getLocaleCode() {
return localeCode;
}

// TODO: rename to handleRequest() to someday handle sending headers as well as query parameters.
Expand Down Expand Up @@ -175,63 +128,6 @@ public String handleRequest(boolean preview) {
}
}

private String getQueryParam(String key, String value) {
ReservedWord reservedWord = ReservedWord.fromString(value);
switch (reservedWord) {
case FILE_ID:
// getDataFile is never null for file tools because of the constructor
return key + "=" + getDataFile().getId();
case FILE_PID:
GlobalId filePid = getDataFile().getGlobalId();
if (filePid != null) {
return key + "=" + getDataFile().getGlobalId();
}
break;
case SITE_URL:
siteUrl = SystemConfig.getDataverseSiteUrlStatic();
return key + "=" + siteUrl;
case API_TOKEN:
String apiTokenString = null;
ApiToken theApiToken = getApiToken();
if (theApiToken != null) {
apiTokenString = theApiToken.getTokenString();
return key + "=" + apiTokenString;
}
break;
case DATASET_ID:
return key + "=" + dataset.getId();
case DATASET_PID:
return key + "=" + dataset.getGlobalId().asString();
case DATASET_VERSION:
String versionString = null;
if(fileMetadata!=null) { //true for file case
versionString = fileMetadata.getDatasetVersion().getFriendlyVersionNumber();
} else { //Dataset case - return the latest visible version (unless/until the dataset case allows specifying a version)
if (getApiToken() != null) {
versionString = dataset.getLatestVersion().getFriendlyVersionNumber();
} else {
versionString = dataset.getLatestVersionForCopy().getFriendlyVersionNumber();
}
}
if (("DRAFT").equals(versionString)) {
versionString = ":draft"; // send the token needed in api calls that can be substituted for a numeric
// version.
}
return key + "=" + versionString;
case FILE_METADATA_ID:
if(fileMetadata!=null) { //true for file case
return key + "=" + fileMetadata.getId();
}
case LOCALE_CODE:
return key + "=" + getLocaleCode();
case ALLOWED_URLS:
return key + "=" + getAllowedUrls();
default:
break;
}
return null;
}


private String postFormData(Integer timeout,List<String> params ) throws IOException, InterruptedException{
String url = "";
Expand Down
3 changes: 2 additions & 1 deletion src/main/java/edu/harvard/iq/dataverse/externaltools/ExternalToolServiceBean.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,9 @@
import edu.harvard.iq.dataverse.DataFile;
import edu.harvard.iq.dataverse.DataFileServiceBean;
import edu.harvard.iq.dataverse.authorization.users.ApiToken;
import edu.harvard.iq.dataverse.externaltools.ExternalTool.ReservedWord;
import edu.harvard.iq.dataverse.externaltools.ExternalTool.Type;
import edu.harvard.iq.dataverse.util.URLTokenUtil;
import edu.harvard.iq.dataverse.util.URLTokenUtil.ReservedWord;
import edu.harvard.iq.dataverse.externaltools.ExternalTool.Scope;

import java.io.StringReader;
Expand Down
5 changes: 5 additions & 0 deletions src/main/java/edu/harvard/iq/dataverse/util/SystemConfig.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,11 @@ public class SystemConfig {
public final static String DEFAULTCURATIONLABELSET = "DEFAULT";
public final static String CURATIONLABELSDISABLED = "DISABLED";

// A secret used in signing URLs - individual urls are signed using this and the
// intended user's apiKey, creating an aggregate key that is unique to the user
// but not known to the user (as their apiKey is)
public final static String API_SIGNING_SECRET = "dataverse.api-signing-secret;";

public String getVersion() {
return getVersion(false);
}
Expand Down
Loading