issue-73: reworking API key display logic and ways for sysadmin to access it

src/main/java/com/researchspace/api/v1/UserDetailsSyadminApi.java → 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<String, String> getAllApiKeyInfo(@RequestAttribute(name = "user") User user);
 }

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<String, String> 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<String, String> allUserInfo = new HashMap<>();
     for (User aUser : userManager.getUsers()) {
       Optional<UserApiKey> optKey = apiKeyMgr.getKeyForUser(aUser);

src/main/java/com/researchspace/webapp/controller/UserProfileController.java

@@ -105,6 +105,7 @@
 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.springframework.beans.factory.annotation.Autowired;
 import org.springframework.beans.factory.annotation.Qualifier;
 import org.springframework.dao.DataIntegrityViolationException;
@@ -697,7 +698,7 @@ public AjaxReturnObject<String> updatePreferenceValue(
 
   @PostMapping("/ajax/apiKey")
   @IgnoreInLoggingInterceptor(ignoreRequestParams = "password")
-  public @ResponseBody AjaxReturnObject<ApiInfo> generateApiKey(
+  public @ResponseBody AjaxReturnObject<ApiKeyInfo> generateApiKey(
       @RequestParam("password") String pwd) {
     if (isEmpty(pwd)) {
       return new AjaxReturnObject<>(
@@ -712,7 +713,8 @@ public AjaxReturnObject<String> 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")
@@ -726,7 +728,7 @@ public AjaxReturnObject<String> updatePreferenceValue(
   @Data
   @AllArgsConstructor
   @NoArgsConstructor
-  public static class ApiInfo {
+  public static class ApiKeyInfo {
     /** The actual API key */
     private String key = null;
 
@@ -746,13 +748,12 @@ public static class ApiInfo {
     private long age;
   }
 
-  @GetMapping("/ajax/apiKeyInfo")
-  public @ResponseBody AjaxReturnObject<ApiInfo> getApiKeyInfo() {
+  @GetMapping("/ajax/apiKeyDisplayInfo")
+  public @ResponseBody AjaxReturnObject<ApiKeyInfo> getApiKeyDisplayInfo() {
     User user = userManager.getAuthenticatedUserInSession();
     Optional<UserApiKey> 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()));
     }
@@ -764,6 +765,22 @@ public static class ApiInfo {
     return new AjaxReturnObject<>(rc, null);
   }
 
+  @GetMapping("/ajax/apiKeyValue")
+  public @ResponseBody AjaxReturnObject<String> getApiKeyValue() {
+    if (SecurityUtils.getSubject().isRunAs()) {
+      return new AjaxReturnObject<>(
+          null, ErrorList.of("API key value cannot be accessed when 'operating as' another user"));
+    }
+    User user = userManager.getAuthenticatedUserInSession();
+    SECURITY_LOG.info("User [{}] asked to see their API key", user.getUsername());
+
+    Optional<UserApiKey> 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

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

src/main/webapp/WEB-INF/pages/userform.jsp

@@ -14,12 +14,6 @@
 
 <input type="hidden" name="from" value="<c:out value=" ${param.from}" />" />
 
-<%-- commenting as seems unused (mk - 22/09/16) --%>
-<%-- <c:if test="${cookieLogin == 'true'}"> --%>
-<%-- 	<form:hidden path="password" /> --%>
-<%-- 	<form:hidden path="confirmPassword" /> --%>
-<%-- </c:if> --%>
-
 <c:if test="${empty user.version}">
 	<input type="hidden" name="encryptPass" value="true" />
 </c:if>
@@ -567,14 +561,14 @@
 <script type="text/template" id="apiKeyDetailsTemplate">
 	{{#enabled}}
     <div class="api-menu__key col-xs-8">
-      {{#key}}
-        <strong>Key</strong>: <span id="api-menu__keyValue">{{key}}</span>
+      {{#revokable}}
+        <strong>Key</strong>: <span id="api-menu__keyValue"> &lt;hidden&gt; </span>
           <a href="#" id="api-menu__showKey" onclick="return false;">Show Key</a>
           <a href="#" id="api-menu__hideKey" onclick="return false;">Hide Key</a>
-      {{/key}}
-      {{^key}}
+      {{/revokable}}
+      {{^revokable}}
         <strong>Key</strong>: Empty.
-      {{/key}}
+      {{/revokable}}
 
       <br />
       See <a href="/public/apiDocs" target="_blank">API Documentation</a>.

src/main/webapp/scripts/pages/userform.js

@@ -389,7 +389,7 @@ function initPwdConfirmDlg () {
           var jqxhr = $.post('/userform/ajax/apiKey', {"password":pwd})
                        .done(function(data) {
                            showRSApiKey = true;
-                           renderApiKeyMenu(data); 
+                           renderApiKeyMenu(data);
                        })
                        .fail(function() {
                           RS.ajaxFailed("Creating a new API key", false, jqxhr);
@@ -495,19 +495,32 @@ function renderApiKeyMenu(serverResponse) {
   var keyInfoData = serverResponse.data;
   var htmlData = Mustache.render(apiKeyInfoTemplate, keyInfoData);
   $apiKeyInfo.html(htmlData);
+  if (keyInfoData.key) {
+    $('#api-menu__keyValue').text(keyInfoData.key);
+  }
+  $('#api-menu__keyValue, #api-menu__hideKey').toggle(showRSApiKey);
+  $('#api-menu__showKey').toggle(!showRSApiKey);
+}
+
+function showApiKeyValue(serverResponse) {
+  if (!serverResponse.data) {
+    apprise(getValidationErrorString(serverResponse.errorMsg));
+    return;
+  }
+  $('#api-menu__keyValue').text(serverResponse.data);
   $('#api-menu__keyValue, #api-menu__hideKey').toggle(showRSApiKey);
   $('#api-menu__showKey').toggle(!showRSApiKey);
 }
 
 function initApiKeyDisplay () {
+
   function updateApiKeyMenu() {
-    $.get('/userform/ajax/apiKeyInfo')
+    $.get('/userform/ajax/apiKeyDisplayInfo')
      .then(renderApiKeyMenu);
-  }  
-  
+  }
+
   $(document).on("click", "#apiKeyRegenerateBtn", function(e) {
     e.preventDefault();
-    e.stopPropagation();
 
 	  $.get('/vfpwd/ajax/checkVerificationPasswordNeeded', function(response) {  
       	if (response.data) {
@@ -518,7 +531,9 @@ function initApiKeyDisplay () {
 	  });
   });
 
-  $(document).on("click", "#apiKeyRevokeBtn", function(e) { 
+  $(document).on("click", "#apiKeyRevokeBtn", function(e) {
+    e.preventDefault();
+
     $.post('/userform/ajax/apiKey', {"_method":"DELETE"}, function (intDeleted){
       if (intDeleted > 0) {
         RS.defaultConfirm("Key deleted");
@@ -530,8 +545,13 @@ function initApiKeyDisplay () {
   });
 
   $(document).on("click", "#api-menu__showKey, #api-menu__hideKey", function() {
-	  showRSApiKey = !showRSApiKey;
-	  updateApiKeyMenu();
+    showRSApiKey = !showRSApiKey;
+    if (showRSApiKey) {
+      $.get('/userform/ajax/apiKeyValue')
+        .then(showApiKeyValue);
+    }
+    $('#api-menu__keyValue, #api-menu__hideKey').toggle(showRSApiKey);
+    $('#api-menu__showKey').toggle(!showRSApiKey);
   });
 
   updateApiKeyMenu();

src/test/java/com/researchspace/webapp/controller/UserProfileControllerMVCIT.java

@@ -53,7 +53,7 @@
 import com.researchspace.service.UserProfileManager;
 import com.researchspace.testutils.RSpaceTestUtils;
 import com.researchspace.testutils.TestGroup;
-import com.researchspace.webapp.controller.UserProfileController.ApiInfo;
+import com.researchspace.webapp.controller.UserProfileController.ApiKeyInfo;
 import java.io.IOException;
 import java.security.Principal;
 import java.util.Date;
@@ -616,39 +616,63 @@ public void apiKeyManagement() throws Exception {
     User user = createAndSaveUser(CoreTestUtils.getRandomName(8));
     logoutAndLoginAs(user);
     mockPrincipal = new MockPrincipal(user.getUsername());
-    MvcResult result =
-        mockMvc.perform(get("/userform/ajax/apiKeyInfo").principal(mockPrincipal)).andReturn();
-    ApiInfo info = getFromJsonAjaxReturnObject(result, UserProfileController.ApiInfo.class);
+
+    // check responses before key is generated
+    MvcResult noKeyDetails =
+        mockMvc
+            .perform(get("/userform/ajax/apiKeyDisplayInfo").principal(mockPrincipal))
+            .andReturn();
+    ApiKeyInfo info = getFromJsonAjaxReturnObject(noKeyDetails, ApiKeyInfo.class);
     assertNull(info.getKey());
     assertTrue(info.isRegenerable());
     assertFalse(info.isRevokable());
+    noKeyDetails =
+        mockMvc.perform(get("/userform/ajax/apiKeyValue").principal(mockPrincipal)).andReturn();
+    ErrorList el = getErrorListFromAjaxReturnObject(noKeyDetails);
+    assertEquals("API key is not set", el.getAllErrorMessagesAsStringsSeparatedBy(""));
 
+    // create new API key
     MvcResult createdKey =
         mockMvc
             .perform(
                 post("/userform/ajax/apiKey")
                     .principal(mockPrincipal)
                     .param("password", TESTPASSWD))
             .andReturn();
-    ApiInfo created = getFromJsonAjaxReturnObject(createdKey, UserProfileController.ApiInfo.class);
+    ApiKeyInfo created = getFromJsonAjaxReturnObject(createdKey, ApiKeyInfo.class);
     assertNotNull(created.getKey());
     assertTrue(created.isRegenerable());
     assertTrue(created.isRevokable());
 
+    // retrieve details on created key
+    MvcResult retrievedKeyDetails =
+        mockMvc
+            .perform(get("/userform/ajax/apiKeyDisplayInfo").principal(mockPrincipal))
+            .andReturn();
+    info = getFromJsonAjaxReturnObject(retrievedKeyDetails, ApiKeyInfo.class);
+    assertNull(info.getKey()); // key not included in display info
+    assertTrue(info.isRegenerable());
+    assertTrue(info.isRevokable());
+    retrievedKeyDetails =
+        mockMvc.perform(get("/userform/ajax/apiKeyValue").principal(mockPrincipal)).andReturn();
+    String retrievedApiKey = getFromJsonAjaxReturnObject(retrievedKeyDetails, String.class);
+    assertEquals(created.getKey(), retrievedApiKey);
+
+    // delete key
     MvcResult deletedKey =
         mockMvc.perform(delete("/userform/ajax/apiKey").principal(mockPrincipal)).andReturn();
     Long revoked = Long.parseLong(deletedKey.getResponse().getContentAsString());
     assertTrue(revoked >= 1);
 
+    // error scenario - no password when generating the key
     MvcResult noPwd =
         mockMvc
             .perform(post("/userform/ajax/apiKey").principal(mockPrincipal).param("password", ""))
-
-            // .andExpect(status().is4xxClientError())
             .andReturn();
-    ErrorList el = getErrorListFromAjaxReturnObject(noPwd);
+    el = getErrorListFromAjaxReturnObject(noPwd);
     assertTrue(el.hasErrorMessages());
 
+    // error scenario - wrong password when generating the key
     MvcResult wrongPwd =
         mockMvc
             .perform(