Skip to content

Commit

Permalink
Private URL: add tests to exercise #3200 and #3201
Browse files Browse the repository at this point in the history
  • Loading branch information
pdurbin committed Jul 8, 2016
1 parent 9816361 commit 4015d35
Show file tree
Hide file tree
Showing 5 changed files with 71 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,11 @@ public <R> R submit(Command<R> aCommand) throws CommandException {
if (!granted.containsAll(required)) {
required.removeAll(granted);
logRec.setActionResult(ActionLogRecord.Result.PermissionError);
/**
* @todo Is there any harm in showing the "granted" set
* since we already show "required"? It would help people
* reason about the mismatch.
*/
throw new PermissionException("Can't execute command " + aCommand
+ ", because request " + aCommand.getRequest()
+ " is missing permissions " + required
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -337,6 +337,11 @@ protected <T> T execCommand( Command<T> cmd ) throws WrappedResponse {
throw new WrappedResponse( ex, errorResponse(Response.Status.BAD_REQUEST, ex.getMessage() ) );

} catch (PermissionException ex) {
/**
* @todo Is there any harm in exposing ex.getLocalizedMessage()?
* There's valuable information in there that can help people reason
* about permissions!
*/
throw new WrappedResponse(errorResponse(Response.Status.UNAUTHORIZED,
"User " + cmd.getRequest().getUser().getIdentifier() + " is not permitted to perform requested action.") );

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,11 @@ public class DataverseRole implements Serializable {
public static final String FULL_CONTRIBUTOR = "fullContributor";
public static final String DV_CONTRIBUTOR = "dvContributor";
public static final String DS_CONTRIBUTOR = "dsContributor";
/**
* Heads up that this says "editor" which comes from
* scripts/api/data/role-editor.json but the name is "Contributor". The
* *alias* is "editor". Don't be fooled!
*/
public static final String EDITOR = "editor";
public static final String MANAGER = "manager";
public static final String CURATOR = "curator";
Expand Down
61 changes: 50 additions & 11 deletions src/test/java/edu/harvard/iq/dataverse/api/DatasetsIT.java
Original file line number Diff line number Diff line change
Expand Up @@ -161,15 +161,16 @@ public void testPrivateUrl() {
String contributorUsername = UtilIT.getUsernameFromResponse(createContributorResponse);
String contributorApiToken = UtilIT.getApiTokenFromResponse(createContributorResponse);
UtilIT.getRoleAssignmentsOnDataverse(dataverseAlias, apiToken).prettyPrint();
Response grantRoleShouldFail = UtilIT.grantRoleOnDataverse(dataverseAlias, DataverseRole.DS_CONTRIBUTOR.toString(), "doesNotExist", apiToken);
Response grantRoleShouldFail = UtilIT.grantRoleOnDataverse(dataverseAlias, DataverseRole.EDITOR.toString(), "doesNotExist", apiToken);
grantRoleShouldFail.then().assertThat()
.statusCode(BAD_REQUEST.getStatusCode())
.body("message", equalTo("Assignee not found"));
/**
* dsContributor only has AddDataset per
* scripts/api/data/role-dsContributor.json
* editor (a.k.a. Contributor) has "ViewUnpublishedDataset",
* "EditDataset", "DownloadFile", and "DeleteDatasetDraft" per
* scripts/api/data/role-editor.json
*/
Response grantRole = UtilIT.grantRoleOnDataverse(dataverseAlias, DataverseRole.DS_CONTRIBUTOR.toString(), "@" + contributorUsername, apiToken);
Response grantRole = UtilIT.grantRoleOnDataverse(dataverseAlias, DataverseRole.EDITOR.toString(), "@" + contributorUsername, apiToken);
grantRole.prettyPrint();
assertEquals(OK.getStatusCode(), grantRole.getStatusCode());
UtilIT.getRoleAssignmentsOnDataverse(dataverseAlias, apiToken).prettyPrint();
Expand Down Expand Up @@ -332,13 +333,51 @@ public void testPrivateUrl() {
createPrivateUrlForPostVersionOneDraft.prettyPrint();
assertEquals(OK.getStatusCode(), createPrivateUrlForPostVersionOneDraft.getStatusCode());

/**
* @todo Make this a more explicit delete of the draft version rather
* than the latest version which we happen to know is a draft.
*/
Response deleteDraftVersion = UtilIT.deleteLatestDatasetVersionViaSwordApi(dataset1PersistentId, apiToken);
deleteDraftVersion.prettyPrint();
assertEquals(OK.getStatusCode(), createPrivateUrlForPostVersionOneDraft.getStatusCode());
// A Contributor has DeleteDatasetDraft
Response deleteDraftVersionAsContributor = UtilIT.deleteDatasetVersionViaNativeApi(datasetId, ":draft", contributorApiToken);
deleteDraftVersionAsContributor.prettyPrint();
boolean bugs3200and3201haveBeenFixed = false;
if (bugs3200and3201haveBeenFixed) {
deleteDraftVersionAsContributor.then().assertThat()
.statusCode(OK.getStatusCode());
} else {
/**
* The problem here is that the deletion of the dataset version by
* the contributor only half worked! The version is gone but the
* Private URL was not deleted and reindexing didn't happen. The
* DeleteDatasetVersionCommand exited early. If you allow more
* information to be reported, you will see "status:ERROR" and this
* message:
*
* "User @userbaed3c79 is not permitted to perform requested action.
* Can't execute command
* edu.harvard.iq.dataverse.engine.command.impl.GetPrivateUrlCommand@70d1d3e4,
* because request [DataverseRequest
* user:edu.harvard.iq.dataverse.authorization.users.AuthenticatedUser@69@127.0.0.1]
* is missing permissions [ManageDatasetPermissions] on Object
* Darwin's Finches. granted: [ViewUnpublishedDataset, DownloadFile,
* EditDataset, DeleteDatasetDraft]"
*
* This is fairly awful because you are left with the impression
* that the deletion of the dataset version did not happen
* (UNAUTHORIZED) but it really did! Yikes!
*
* To get the dataset out of its invalid state (indexing cruft left
* behind and a Private URL on a published version), we create a new
* draft.
*/
deleteDraftVersionAsContributor.then().assertThat()
.statusCode(UNAUTHORIZED.getStatusCode())
.body("message", equalTo("User @" + contributorUsername + " is not permitted to perform requested action."));
String workaroundTitle = "I am creating a new draft as a workaround while bugs 3200 and 3100 exist. This way the creator can delete it cleanly.";
Response workAroundSoWeCanDeleteDraft = UtilIT.updateDatasetTitleViaSword(dataset1PersistentId, workaroundTitle, apiToken);
workAroundSoWeCanDeleteDraft.prettyPrint();
assertEquals(OK.getStatusCode(), workAroundSoWeCanDeleteDraft.getStatusCode());
Response deleteDraftVersionAsCreator = UtilIT.deleteDatasetVersionViaNativeApi(datasetId, ":draft", apiToken);
deleteDraftVersionAsCreator.prettyPrint();
deleteDraftVersionAsCreator.then().assertThat()
.statusCode(OK.getStatusCode());
}

Response privateUrlRoleAssignmentShouldBeGoneAfterDraftDeleted = UtilIT.getRoleAssignmentsOnDataset(datasetId.toString(), null, apiToken);
privateUrlRoleAssignmentShouldBeGoneAfterDraftDeleted.prettyPrint();
Expand Down
6 changes: 6 additions & 0 deletions src/test/java/edu/harvard/iq/dataverse/api/UtilIT.java
Original file line number Diff line number Diff line change
Expand Up @@ -375,6 +375,12 @@ public static Response deleteDatasetViaNativeApi(Integer datasetId, String apiTo
.delete("/api/datasets/" + datasetId);
}

public static Response deleteDatasetVersionViaNativeApi(Integer datasetId, String versionId, String apiToken) {
return given()
.header(API_TOKEN_HTTP_HEADER, apiToken)
.delete("/api/datasets/" + datasetId + "/versions/" + versionId);
}

static Response deleteLatestDatasetVersionViaSwordApi(String persistentId, String apiToken) {
return given()
.auth().basic(apiToken, EMPTY_STRING)
Expand Down

0 comments on commit 4015d35

Please sign in to comment.