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.

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

API: deprecate toggle of superuser status in favor of new idempotent endpoint, update setup-all.sh #10440

Merged
merged 22 commits into from
Apr 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
25cf3ff
9887: Adding new endpoint to make superuser call idempotent
zearaujo25 Mar 27, 2024
16ed8cc
9887: Adding documentation and deprecation tag
zearaujo25 Mar 28, 2024
5570bec
9887: Fixing documentation
zearaujo25 Mar 28, 2024
e1bed70
Update doc/sphinx-guides/source/api/native-api.rst
zearaujo25 Mar 29, 2024
e5aca34
Update doc/sphinx-guides/source/api/native-api.rst
zearaujo25 Mar 29, 2024
4444c0a
Update doc/sphinx-guides/source/api/native-api.rst
zearaujo25 Mar 29, 2024
7008b35
Update doc/release-notes/9887-new-superuser-status-endpoint.md
zearaujo25 Mar 29, 2024
532e7f0
9887: code review
zearaujo25 Mar 29, 2024
74f6c4a
Update doc/sphinx-guides/source/api/changelog.rst
zearaujo25 Mar 29, 2024
9577295
9887: code review
zearaujo25 Mar 29, 2024
12fe4cd
9887: renaming function
zearaujo25 Mar 29, 2024
0806eab
Merge branch 'develop' into feat/9887 #9887
pdurbin Apr 11, 2024
4788c10
Adding header to documentation
zearaujo25 Apr 12, 2024
0d9e8df
Changing header to text/plain
zearaujo25 Apr 12, 2024
6684ae6
9887: Changin body to string to allow empty Content-Type
zearaujo25 Apr 12, 2024
e3ecf8b
9887: changing installation scrpt to sue new endpoint
zearaujo25 Apr 12, 2024
a418b58
9887: Changin doc
zearaujo25 Apr 12, 2024
3f89c32
9887: renaming variable
zearaujo25 Apr 12, 2024
95ce8e0
tweak docs #9887
pdurbin Apr 12, 2024
074498f
switch BagIT to new "make superuser" API #9887
pdurbin Apr 12, 2024
a97b931
reformat code (no-op) #9887
pdurbin Apr 12, 2024
80ae69b
make action log match method name, comment tweak #9887
pdurbin Apr 12, 2024
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
1 change: 1 addition & 0 deletions doc/release-notes/9887-new-superuser-status-endpoint.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
The existing API endpoint for toggling the superuser status of a user has been deprecated in favor of a new API endpoint that allows you to explicitly and idempotently set the status as true or false. For details, see [the guides](https://dataverse-guide--10440.org.readthedocs.build/en/10440/api/native-api.html), #9887 and #10440.
5 changes: 5 additions & 0 deletions doc/sphinx-guides/source/api/changelog.rst
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,11 @@ This API changelog is experimental and we would love feedback on its usefulness.
:local:
:depth: 1

v6.3
----

- **/api/admin/superuser/{identifier}**: The POST endpoint that toggles superuser status has been deprecated in favor of a new PUT endpoint that allows you to specify true or false. See :ref:`set-superuser-status`.

v6.2
----

Expand Down
42 changes: 38 additions & 4 deletions doc/sphinx-guides/source/api/native-api.rst
Original file line number Diff line number Diff line change
Expand Up @@ -5412,12 +5412,46 @@ Example: ``curl -H "X-Dataverse-key: $API_TOKEN" -X POST "https://demo.datavers

This action changes the identifier of user johnsmith to jsmith.

Make User a SuperUser
~~~~~~~~~~~~~~~~~~~~~
Toggle Superuser Status
~~~~~~~~~~~~~~~~~~~~~~~

Toggle the superuser status of a user.

.. note:: This endpoint is deprecated as explained in :doc:`/api/changelog`. Please use the :ref:`set-superuser-status` endpoint instead.

.. code-block:: bash

export SERVER_URL=http://localhost:8080
export USERNAME=jdoe
curl -X POST "$SERVER_URL/api/admin/superuser/$USERNAME"

The fully expanded example above (without environment variables) looks like this:

zearaujo25 marked this conversation as resolved.
Show resolved Hide resolved
.. code-block:: bash

curl -X POST "http://localhost:8080/api/admin/superuser/jdoe"

Toggles superuser mode on the ``AuthenticatedUser`` whose ``identifier`` (without the ``@`` sign) is passed. ::
.. _set-superuser-status:

Set Superuser Status
~~~~~~~~~~~~~~~~~~~~

Specify the superuser status of a user with a boolean value (``true`` or ``false``).

.. note:: See :ref:`curl-examples-and-environment-variables` if you are unfamiliar with the use of ``export`` below.

.. code-block:: bash

export SERVER_URL=http://localhost:8080
export USERNAME=jdoe
export IS_SUPERUSER=true
curl -X PUT "$SERVER_URL/api/admin/superuser/$USERNAME" -d "$IS_SUPERUSER"

The fully expanded example above (without environment variables) looks like this:

.. code-block:: bash

POST http://$SERVER/api/admin/superuser/$identifier
curl -X PUT "http://localhost:8080/api/admin/superuser/jdoe" -d true

zearaujo25 marked this conversation as resolved.
Show resolved Hide resolved
.. _delete-a-user:

Expand Down
2 changes: 1 addition & 1 deletion scripts/api/setup-all.sh
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ echo
echo "Setting up the admin user (and as superuser)"
adminResp=$(curl -s -H "Content-type:application/json" -X POST -d @"$SCRIPT_PATH"/data/user-admin.json "${DATAVERSE_URL}/api/builtin-users?password=$DV_SU_PASSWORD&key=burrito")
echo "$adminResp"
curl -X POST "${DATAVERSE_URL}/api/admin/superuser/dataverseAdmin"
curl -X PUT "${DATAVERSE_URL}/api/admin/superuser/dataverseAdmin" -d "true"
echo

echo "Setting up the root dataverse"
Expand Down
63 changes: 42 additions & 21 deletions src/main/java/edu/harvard/iq/dataverse/api/Admin.java
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import edu.harvard.iq.dataverse.DvObjectServiceBean;
import edu.harvard.iq.dataverse.api.auth.AuthRequired;
import edu.harvard.iq.dataverse.settings.JvmSettings;
import edu.harvard.iq.dataverse.util.StringUtil;
import edu.harvard.iq.dataverse.validation.EMailValidator;
import edu.harvard.iq.dataverse.EjbDataverseEngine;
import edu.harvard.iq.dataverse.Template;
Expand Down Expand Up @@ -1029,29 +1030,49 @@ public Response deleteRole(@Context ContainerRequestContext crc, @PathParam("id"
}, getRequestUser(crc));
}

@Path("superuser/{identifier}")
@POST
public Response toggleSuperuser(@PathParam("identifier") String identifier) {
ActionLogRecord alr = new ActionLogRecord(ActionLogRecord.ActionType.Admin, "toggleSuperuser")
.setInfo(identifier);
try {
AuthenticatedUser user = authSvc.getAuthenticatedUser(identifier);
if (user.isDeactivated()) {
return error(Status.BAD_REQUEST, "You cannot make a deactivated user a superuser.");
}
@Path("superuser/{identifier}")
@Deprecated
@POST
public Response toggleSuperuser(@PathParam("identifier") String identifier) {
ActionLogRecord alr = new ActionLogRecord(ActionLogRecord.ActionType.Admin, "toggleSuperuser")
.setInfo(identifier);
try {
final AuthenticatedUser user = authSvc.getAuthenticatedUser(identifier);
return setSuperuserStatus(user, !user.isSuperuser());
} catch (Exception e) {
alr.setActionResult(ActionLogRecord.Result.InternalError);
alr.setInfo(alr.getInfo() + "// " + e.getMessage());
return error(Response.Status.INTERNAL_SERVER_ERROR, e.getMessage());
} finally {
actionLogSvc.log(alr);
}
}

user.setSuperuser(!user.isSuperuser());
private Response setSuperuserStatus(AuthenticatedUser user, Boolean isSuperuser) {
if (user.isDeactivated()) {
return error(Status.BAD_REQUEST, "You cannot make a deactivated user a superuser.");
}
user.setSuperuser(isSuperuser);
return ok("User " + user.getIdentifier() + " " + (user.isSuperuser() ? "set" : "removed")
+ " as a superuser.");
}

return ok("User " + user.getIdentifier() + " " + (user.isSuperuser() ? "set" : "removed")
+ " as a superuser.");
} catch (Exception e) {
alr.setActionResult(ActionLogRecord.Result.InternalError);
alr.setInfo(alr.getInfo() + "// " + e.getMessage());
return error(Response.Status.INTERNAL_SERVER_ERROR, e.getMessage());
} finally {
actionLogSvc.log(alr);
}
}
@Path("superuser/{identifier}")
@PUT
// Using string instead of boolean so user doesn't need to add a Content-type header in their request
public Response setSuperuserStatus(@PathParam("identifier") String identifier, String isSuperuser) {
ActionLogRecord alr = new ActionLogRecord(ActionLogRecord.ActionType.Admin, "setSuperuserStatus")
.setInfo(identifier + ":" + isSuperuser);
try {
return setSuperuserStatus(authSvc.getAuthenticatedUser(identifier), StringUtil.isTrue(isSuperuser));
} catch (Exception e) {
alr.setActionResult(ActionLogRecord.Result.InternalError);
alr.setInfo(alr.getInfo() + "// " + e.getMessage());
return error(Response.Status.INTERNAL_SERVER_ERROR, e.getMessage());
} finally {
actionLogSvc.log(alr);
}
}

@GET
@Path("validate/datasets")
Expand Down
14 changes: 13 additions & 1 deletion src/test/java/edu/harvard/iq/dataverse/api/AdminIT.java
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@
import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.BeforeAll;

import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.ValueSource;
import java.util.Map;
import java.util.UUID;
import java.util.logging.Logger;
Expand Down Expand Up @@ -895,4 +896,15 @@ private String createTestNonSuperuserApiToken() {
createUserResponse.then().assertThat().statusCode(OK.getStatusCode());
return UtilIT.getApiTokenFromResponse(createUserResponse);
}

@ParameterizedTest
@ValueSource(booleans={true,false})
public void testSetSuperUserStatus(Boolean status) {
Response createUser = UtilIT.createRandomUser();
createUser.then().assertThat().statusCode(OK.getStatusCode());
String username = UtilIT.getUsernameFromResponse(createUser);
Response toggleSuperuser = UtilIT.setSuperuserStatus(username, status);
toggleSuperuser.then().assertThat()
.statusCode(OK.getStatusCode());
}
}
4 changes: 2 additions & 2 deletions src/test/java/edu/harvard/iq/dataverse/api/BagIT.java
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,8 @@ public void testBagItExport() throws IOException {
createUser.then().assertThat().statusCode(OK.getStatusCode());
String username = UtilIT.getUsernameFromResponse(createUser);
String apiToken = UtilIT.getApiTokenFromResponse(createUser);
Response toggleSuperuser = UtilIT.makeSuperUser(username);
toggleSuperuser.then().assertThat()
Response makeSuperuser = UtilIT.setSuperuserStatus(username, true);
makeSuperuser.then().assertThat()
.statusCode(OK.getStatusCode());

Response createDataverse = UtilIT.createRandomDataverse(apiToken);
Expand Down
2 changes: 1 addition & 1 deletion src/test/java/edu/harvard/iq/dataverse/api/RolesIT.java
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
*/
public class RolesIT {

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

@BeforeAll
public static void setUp() {
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 @@ -1535,11 +1535,17 @@ static Response getMetadataBlockFromDatasetVersion(String persistentId, String v
.get("/api/datasets/:persistentId/versions/" + DS_VERSION_LATEST_PUBLISHED + "/metadata/citation?persistentId=" + persistentId);
}

@Deprecated
static Response makeSuperUser(String username) {
Response response = given().post("/api/admin/superuser/" + username);
return response;
zearaujo25 marked this conversation as resolved.
Show resolved Hide resolved
}

static Response setSuperuserStatus(String username, Boolean isSuperUser) {
Response response = given().body(isSuperUser).put("/api/admin/superuser/" + username);
return response;
}

static Response reindexDataset(String persistentId) {
Response response = given()
.get("/api/admin/index/dataset?persistentId=" + persistentId);
Expand Down
Loading