From 83feade79422b6ef512429a09265567a7901e785 Mon Sep 17 00:00:00 2001 From: matthias Date: Tue, 30 Jul 2024 17:26:34 +0100 Subject: [PATCH 1/5] blocking & removing 'Manage Api Key' fragment of my profile page for sysadmin 'operating as'; guarding API endpoint for apiKey retrieval with new deployment property 'sysadmin.apikey.access' which must be explicitly changed to 'true' --- ...dminApi.java => UserDetailsSysAdminApi.java} | 17 ++++++++++------- .../UserDetailsSysAdminApiController.java | 14 ++++++++++---- .../controller/UserProfileController.java | 7 +++++++ .../deployments/defaultDeployment.properties | 2 ++ .../deployments/dev/deployment.properties | 4 +--- src/main/webapp/WEB-INF/pages/userform.jsp | 8 +------- src/main/webapp/scripts/pages/userform.js | 5 +++++ 7 files changed, 36 insertions(+), 21 deletions(-) rename src/main/java/com/researchspace/api/v1/{UserDetailsSyadminApi.java => UserDetailsSysAdminApi.java} (63%) diff --git a/src/main/java/com/researchspace/api/v1/UserDetailsSyadminApi.java b/src/main/java/com/researchspace/api/v1/UserDetailsSysAdminApi.java similarity index 63% rename from src/main/java/com/researchspace/api/v1/UserDetailsSyadminApi.java rename to src/main/java/com/researchspace/api/v1/UserDetailsSysAdminApi.java index 5a65a7b90..1c557cabc 100644 --- a/src/main/java/com/researchspace/api/v1/UserDetailsSyadminApi.java +++ b/src/main/java/com/researchspace/api/v1/UserDetailsSysAdminApi.java @@ -11,15 +11,18 @@ import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.bind.annotation.ResponseBody; -/** returns user details available to sysadmins - only sysadmins will be allowed to use this API */ -@RequestMapping("/api/v1/syadmin/userDetails") -public interface UserDetailsSyadminApi { +/** + * returns user details available to sysadmins - only sysadmins will be allowed to use this API, and + * only if sysadmin.apikey.access deployment property is set to true + */ +@RequestMapping("/api/v1/sysadmin/userDetails") +public interface UserDetailsSysAdminApi { - @GetMapping("/apiKeyInfo/all") - @ResponseBody /** - * returns a map of all users usernames to api keys. Request user must be a sysadmin or request - * will not be authorised + * returns a map of all users usernames to api keys. Request user must be a sysadmin, and + * sysadmin.apikey.access deployment property must be set to 'true', or request will be rejected */ + @GetMapping("/apiKeyInfo/all") + @ResponseBody Map getAllApiKeyInfo(@RequestAttribute(name = "user") User user); } diff --git a/src/main/java/com/researchspace/api/v1/controller/UserDetailsSysAdminApiController.java b/src/main/java/com/researchspace/api/v1/controller/UserDetailsSysAdminApiController.java index 223f0fc5e..7f4663a93 100644 --- a/src/main/java/com/researchspace/api/v1/controller/UserDetailsSysAdminApiController.java +++ b/src/main/java/com/researchspace/api/v1/controller/UserDetailsSysAdminApiController.java @@ -1,6 +1,6 @@ package com.researchspace.api.v1.controller; -import com.researchspace.api.v1.UserDetailsSyadminApi; +import com.researchspace.api.v1.UserDetailsSysAdminApi; import com.researchspace.model.SignupSource; import com.researchspace.model.User; import com.researchspace.model.UserApiKey; @@ -11,19 +11,25 @@ import java.util.Optional; import org.apache.shiro.authz.AuthorizationException; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.beans.factory.annotation.Value; import org.springframework.web.bind.annotation.RequestAttribute; @ApiController -public class UserDetailsSysAdminApiController implements UserDetailsSyadminApi { +public class UserDetailsSysAdminApiController implements UserDetailsSysAdminApi { @Autowired private UserManager userManager; @Autowired private UserApiKeyManager apiKeyMgr; + @Value("${sysadmin.apikey.access}") + private boolean sysadminApiKeyAccess; + @Override public Map getAllApiKeyInfo(@RequestAttribute(name = "user") User user) { - boolean isSysadmin = user.hasSysadminRole(); - if (!isSysadmin) { + if (!user.hasSysadminRole()) { throw new AuthorizationException("Only sysadmin can use this API"); } + if (!sysadminApiKeyAccess) { + throw new AuthorizationException("Reading apiKeys by sysadmin is not enabled on this server"); + } Map allUserInfo = new HashMap<>(); for (User aUser : userManager.getUsers()) { Optional optKey = apiKeyMgr.getKeyForUser(aUser); diff --git a/src/main/java/com/researchspace/webapp/controller/UserProfileController.java b/src/main/java/com/researchspace/webapp/controller/UserProfileController.java index 01fa8690a..97ad78ad6 100644 --- a/src/main/java/com/researchspace/webapp/controller/UserProfileController.java +++ b/src/main/java/com/researchspace/webapp/controller/UserProfileController.java @@ -105,6 +105,8 @@ import org.apache.commons.collections.CollectionUtils; import org.apache.commons.io.IOUtils; import org.apache.commons.lang.StringUtils; +import org.apache.shiro.SecurityUtils; +import org.apache.shiro.authz.AuthorizationException; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Qualifier; import org.springframework.dao.DataIntegrityViolationException; @@ -748,6 +750,10 @@ public static class ApiInfo { @GetMapping("/ajax/apiKeyInfo") public @ResponseBody AjaxReturnObject getApiKeyInfo() { + if (SecurityUtils.getSubject().isRunAs()) { + throw new AuthorizationException( + "apiKey cannot be accessed when 'operating' as another user"); + } User user = userManager.getAuthenticatedUserInSession(); Optional optKey = apiKeyMgr.getKeyForUser(user); ApiInfo rc = new ApiInfo(); @@ -761,6 +767,7 @@ public static class ApiInfo { if (!available.isSucceeded()) { rc.setMessage(available.getEntity()); } + SECURITY_LOG.info("User [{}] asked to see their apiKey", user.getUsername()); return new AjaxReturnObject<>(rc, null); } diff --git a/src/main/resources/deployments/defaultDeployment.properties b/src/main/resources/deployments/defaultDeployment.properties index 4d24b0708..1aceb60cf 100644 --- a/src/main/resources/deployments/defaultDeployment.properties +++ b/src/main/resources/deployments/defaultDeployment.properties @@ -121,6 +121,8 @@ sysadmin.delete.user=false sysadmin.delete.user.resourceList.folder=archive/deletedUserResourceListings # whether successful user deletion from DB should be immediately followed by filestore resources deletion sysadmin.delete.user.deleteResourcesImmediately=true +# whether sysadmin should be able to see users' API keys. this shouldn't be changed unless in very specific scenarios +sysadmin.apikey.access=false #Path to error log file sysadmin.errorfile.path=src/test/resources/TestResources/sampleLogs/RSLogs.txt diff --git a/src/main/resources/deployments/dev/deployment.properties b/src/main/resources/deployments/dev/deployment.properties index 4426ab7ec..404fce425 100644 --- a/src/main/resources/deployments/dev/deployment.properties +++ b/src/main/resources/deployments/dev/deployment.properties @@ -13,6 +13,7 @@ archive.folder.storagetime=1 importArchiveFromServer.enabled=true sysadmin.delete.user=true +sysadmin.apikey.access=true rs.dev.groupcreation=true ui.bannerImage.url=/workspace ui.bannerImage.loggedOutUrl=https://www.bbc.com/ @@ -63,9 +64,6 @@ nextcloud.secret= egnyte.client.id= egnyte.internal.app.client.id= -pyrat.url= -pyrat.client.token= - # clustermarket clustermarket.client.id= clustermarket.secret= diff --git a/src/main/webapp/WEB-INF/pages/userform.jsp b/src/main/webapp/WEB-INF/pages/userform.jsp index bc73d583c..56c4a163e 100644 --- a/src/main/webapp/WEB-INF/pages/userform.jsp +++ b/src/main/webapp/WEB-INF/pages/userform.jsp @@ -14,12 +14,6 @@ " /> -<%-- commenting as seems unused (mk - 22/09/16) --%> -<%-- --%> -<%-- --%> -<%-- --%> -<%-- --%> - @@ -305,7 +299,7 @@
- +
diff --git a/src/main/webapp/scripts/pages/userform.js b/src/main/webapp/scripts/pages/userform.js index 853e91594..006b46f81 100644 --- a/src/main/webapp/scripts/pages/userform.js +++ b/src/main/webapp/scripts/pages/userform.js @@ -500,6 +500,11 @@ function renderApiKeyMenu(serverResponse) { } function initApiKeyDisplay () { + + if ($('#apiKeyInfo').size() == 0) { + return; // fragment not displayed + } + function updateApiKeyMenu() { $.get('/userform/ajax/apiKeyInfo') .then(renderApiKeyMenu); From c069f150f3f564868ae401ce08200760f1da87e2 Mon Sep 17 00:00:00 2001 From: matthias Date: Wed, 31 Jul 2024 09:13:36 +0100 Subject: [PATCH 2/5] update the UI logic, api key is no longer implicitly loaded on 'my profile' page; also it should be fine for sysadmin to regenerate/revoke the key, just not to see pre-existing one --- .../controller/UserProfileController.java | 35 ++++++++++------ .../deployments/defaultDeployment.properties | 2 +- .../deployments/dev/deployment.properties | 1 - src/main/webapp/WEB-INF/pages/userform.jsp | 12 +++--- src/main/webapp/scripts/pages/userform.js | 34 +++++++++++----- .../UserProfileControllerMVCIT.java | 40 +++++++++++++++---- 6 files changed, 86 insertions(+), 38 deletions(-) diff --git a/src/main/java/com/researchspace/webapp/controller/UserProfileController.java b/src/main/java/com/researchspace/webapp/controller/UserProfileController.java index 97ad78ad6..66676d5c8 100644 --- a/src/main/java/com/researchspace/webapp/controller/UserProfileController.java +++ b/src/main/java/com/researchspace/webapp/controller/UserProfileController.java @@ -699,7 +699,7 @@ public AjaxReturnObject updatePreferenceValue( @PostMapping("/ajax/apiKey") @IgnoreInLoggingInterceptor(ignoreRequestParams = "password") - public @ResponseBody AjaxReturnObject generateApiKey( + public @ResponseBody AjaxReturnObject generateApiKey( @RequestParam("password") String pwd) { if (isEmpty(pwd)) { return new AjaxReturnObject<>( @@ -714,7 +714,8 @@ public AjaxReturnObject updatePreferenceValue( UserApiKey apiKey = apiKeyMgr.createKeyForUser(user); SECURITY_LOG.info("User {} created new API key", user.getUsername()); - return new AjaxReturnObject<>(new ApiInfo(apiKey.getApiKey(), true, true, true, "", 0L), null); + return new AjaxReturnObject<>( + new ApiKeyInfo(apiKey.getApiKey(), true, true, true, "", 0L), null); } @DeleteMapping("/ajax/apiKey") @@ -728,7 +729,7 @@ public AjaxReturnObject updatePreferenceValue( @Data @AllArgsConstructor @NoArgsConstructor - public static class ApiInfo { + public static class ApiKeyInfo { /** The actual API key */ private String key = null; @@ -748,17 +749,12 @@ public static class ApiInfo { private long age; } - @GetMapping("/ajax/apiKeyInfo") - public @ResponseBody AjaxReturnObject getApiKeyInfo() { - if (SecurityUtils.getSubject().isRunAs()) { - throw new AuthorizationException( - "apiKey cannot be accessed when 'operating' as another user"); - } + @GetMapping("/ajax/apiKeyDisplayInfo") + public @ResponseBody AjaxReturnObject getApiKeyDisplayInfo() { User user = userManager.getAuthenticatedUserInSession(); Optional optKey = apiKeyMgr.getKeyForUser(user); - ApiInfo rc = new ApiInfo(); + ApiKeyInfo rc = new ApiKeyInfo(); if (optKey.isPresent()) { - rc.setKey(optKey.get().getApiKey()); rc.setRevokable(true); rc.setAge(calculateAge(optKey.get())); } @@ -767,10 +763,25 @@ public static class ApiInfo { if (!available.isSucceeded()) { rc.setMessage(available.getEntity()); } - SECURITY_LOG.info("User [{}] asked to see their apiKey", user.getUsername()); return new AjaxReturnObject<>(rc, null); } + @GetMapping("/ajax/apiKeyValue") + public @ResponseBody AjaxReturnObject getApiKeyValue() { + if (SecurityUtils.getSubject().isRunAs()) { + throw new AuthorizationException( + "API key details cannot be accessed when 'operating' as another user"); + } + User user = userManager.getAuthenticatedUserInSession(); + SECURITY_LOG.info("User [{}] asked to see their apiKey", user.getUsername()); + + Optional optKey = apiKeyMgr.getKeyForUser(user); + if (!optKey.isPresent()) { + return new AjaxReturnObject<>(null, ErrorList.of("API key is not set")); + } + return new AjaxReturnObject<>(optKey.get().getApiKey(), null); + } + /** Shows a list of created OAuth apps on the user's profile page */ @GetMapping("/ajax/oAuthApps") @ResponseBody diff --git a/src/main/resources/deployments/defaultDeployment.properties b/src/main/resources/deployments/defaultDeployment.properties index 1aceb60cf..9573de3b0 100644 --- a/src/main/resources/deployments/defaultDeployment.properties +++ b/src/main/resources/deployments/defaultDeployment.properties @@ -121,7 +121,7 @@ sysadmin.delete.user=false sysadmin.delete.user.resourceList.folder=archive/deletedUserResourceListings # whether successful user deletion from DB should be immediately followed by filestore resources deletion sysadmin.delete.user.deleteResourcesImmediately=true -# whether sysadmin should be able to see users' API keys. this shouldn't be changed unless in very specific scenarios +# whether sysadmin should be able to see users' API keys; this shouldn't be changed unless in very specific scenarios sysadmin.apikey.access=false #Path to error log file diff --git a/src/main/resources/deployments/dev/deployment.properties b/src/main/resources/deployments/dev/deployment.properties index 404fce425..1a02b7e78 100644 --- a/src/main/resources/deployments/dev/deployment.properties +++ b/src/main/resources/deployments/dev/deployment.properties @@ -13,7 +13,6 @@ archive.folder.storagetime=1 importArchiveFromServer.enabled=true sysadmin.delete.user=true -sysadmin.apikey.access=true rs.dev.groupcreation=true ui.bannerImage.url=/workspace ui.bannerImage.loggedOutUrl=https://www.bbc.com/ diff --git a/src/main/webapp/WEB-INF/pages/userform.jsp b/src/main/webapp/WEB-INF/pages/userform.jsp index 56c4a163e..892a4e758 100644 --- a/src/main/webapp/WEB-INF/pages/userform.jsp +++ b/src/main/webapp/WEB-INF/pages/userform.jsp @@ -299,7 +299,7 @@
- +
@@ -561,14 +561,14 @@