feat(admin): add /reset-client-secret endpoint and fix config persist… (#7304)

DanielPalafox · amontenegro · web-flow · commit 5ecfe49bf494 · 2025-05-28T07:46:51.000-06:00
* feat(admin): add /reset-client-secret endpoint and fix config persistence in add-client

* test(admin): Add missing test for admin controller and fix typo

---------

Co-authored-by: Angel Montenegro &lt;a.montenegro@orcid.org&gt;
Co-authored-by: Daniel Palafox &lt;daniel.p4l4fox@gmail.com&gt;
diff --git a/orcid-core/src/main/java/org/orcid/core/manager/v3/ClientManager.java b/orcid-core/src/main/java/org/orcid/core/manager/v3/ClientManager.java
@@ -8,7 +8,11 @@ public interface ClientManager {
     
     Client createPublicClient(Client newClient);
 
+    Client createWithConfigValues(Client newClient);
+
     Client edit(Client existingClient, boolean updateConfigValues);
-    
+
     Boolean resetClientSecret(String clientId);
+
+    String resetAndGetClientSecret(String clientId);
 }
diff --git a/orcid-core/src/main/java/org/orcid/core/manager/v3/impl/ClientManagerImpl.java b/orcid-core/src/main/java/org/orcid/core/manager/v3/impl/ClientManagerImpl.java
@@ -82,16 +82,22 @@ public class ClientManagerImpl implements ClientManager {
     @Override
     @Transactional
     public Client create(Client newClient) throws IllegalArgumentException {
-        return create(newClient, false);
+        return create(newClient, false, false);
     }
 
     @Override
     @Transactional
     public Client createPublicClient(Client newClient) {
-        return create(newClient, true);
+        return create(newClient, true, false);
     }
 
-    private Client create(Client newClient, boolean publicClient) {
+    @Override
+    @Transactional
+    public Client createWithConfigValues(Client newClient) {
+        return create(newClient, false, true);
+    }
+
+    private Client create(Client newClient, boolean publicClient, boolean addConfigValues) {
         // If the member id comes in the newClient, use that one, if not, use the active source id
         String memberId = PojoUtil.isEmpty(newClient.getGroupProfileId()) ? null : newClient.getGroupProfileId();
         if(memberId == null) {
@@ -115,7 +121,6 @@ private Client create(Client newClient, boolean publicClient) {
         }
 
         ClientDetailsEntity newEntity = jpaJaxbClientAdapter.toEntity(newClient);
-        Date now = new Date();
         newEntity.setId(appIdGenerationManager.createNewAppId());
         newEntity.setClientSecretForJpa(encryptionManager.encryptForInternalUse(UUID.randomUUID().toString()), true);
         newEntity.setGroupProfileId(memberId);
@@ -177,6 +182,11 @@ private Client create(Client newClient, boolean publicClient) {
         }
         newEntity.setClientScopes(clientScopeEntities);
 
+        if (addConfigValues) {
+            newEntity.setUserOBOEnabled(newClient.isUserOBOEnabled());
+            refreshGrantTypesForObo(newEntity, newClient.isOboEnabled());
+        }
+
         try {
             clientDetailsDao.persist(newEntity);
         } catch (Exception e) {
@@ -247,7 +257,29 @@ public Boolean doInTransaction(TransactionStatus status) {
             }
         });
     }
-    
+
+    @Override
+    @Transactional
+    public String resetAndGetClientSecret(String clientId) {
+        try {
+            String newSecret = encryptionManager.encryptForInternalUse(UUID.randomUUID().toString());
+            clientSecretDao.revokeAllKeys(clientId);
+            boolean created  = clientSecretDao.createClientSecret(clientId, newSecret);
+            if (created) {
+                clientDetailsDao.updateLastModified(clientId);
+                String sourceId = sourceManager.retrieveActiveSourceId();
+                profileLastModifiedDao.updateLastModifiedDateWithoutResult(sourceId);
+            } else {
+                LOGGER.warn("Client secret creation failed for client {}", clientId);
+            }
+
+            return encryptionManager.decryptForInternalUse(newSecret);
+        } catch (Exception e) {
+            LOGGER.error("Unable to reset client secret for client {}", clientId, e);
+            throw e;
+        }
+    }
+
     private void refreshGrantTypesForObo(ClientDetailsEntity clientDetails, boolean enableObo) {
         boolean oboAlreadyEnabled = false;
         Iterator<ClientAuthorisedGrantTypeEntity> grantTypes = clientDetails.getClientAuthorizedGrantTypes().iterator();
diff --git a/orcid-core/src/test/java/org/orcid/core/manager/v3/ClientManagerTest.java b/orcid-core/src/test/java/org/orcid/core/manager/v3/ClientManagerTest.java
@@ -387,6 +387,74 @@ public void resetClientSecret() {
         assertEquals(secret3, client.getDecryptedSecret());
     }
 
+    @Test
+    @Transactional
+    public void saveClientWithConfigValuesTest() {
+        // Create a new client
+        String seed = RandomStringUtils.randomAlphanumeric(15);
+        Client client = getClient(seed, MEMBER_ID);
+
+        client.setAllowAutoDeprecate(false);
+        client.setUserOBOEnabled(true);
+        client.setOboEnabled(true);
+
+        Client newClient = clientManager.createWithConfigValues(client);
+        // Check config options where saved
+        assertTrue(newClient.getId().startsWith("APP-"));
+        assertEquals("authentication-provider-id " + seed, newClient.getAuthenticationProviderId());
+        assertFalse(newClient.isAllowAutoDeprecate());
+        assertTrue(newClient.isUserOBOEnabled());
+        assertTrue(newClient.isOboEnabled());
+
+        seed = RandomStringUtils.randomAlphanumeric(15);
+        client = getClient(seed, MEMBER_ID);
+
+        client.setAllowAutoDeprecate(true);
+        client.setUserOBOEnabled(true);
+        client.setOboEnabled(false);
+
+        newClient = clientManager.createWithConfigValues(client);
+        // Check config options where saved
+        assertTrue(newClient.getId().startsWith("APP-"));
+        assertEquals("authentication-provider-id " + seed, newClient.getAuthenticationProviderId());
+        assertTrue(newClient.isAllowAutoDeprecate());
+        assertTrue(newClient.isUserOBOEnabled());
+        assertFalse(newClient.isOboEnabled());
+    }
+
+    @Test
+    public void resetAndGetClientSecretTest() {
+        String clientId = "APP-5555555555555556";
+        // Get an existing client
+        Client client = clientManagerReadOnly.get(clientId);
+        assertNotNull(client);
+        assertNotNull(client.getDecryptedSecret());
+        assertFalse(PojoUtil.isEmpty(client.getDecryptedSecret()));
+        assertTrue(client.getDecryptedSecret().length() > 1);
+        String secret1 = client.getDecryptedSecret();
+
+        // Reset it one time
+        String newSecret = clientManager.resetAndGetClientSecret(clientId);
+        assertFalse(PojoUtil.isEmpty(newSecret));
+        assertNotEquals(secret1, newSecret);
+        String secret2 = newSecret;
+
+        // Reset it the second time
+        newSecret = clientManager.resetAndGetClientSecret(clientId);
+        assertFalse(PojoUtil.isEmpty(client.getDecryptedSecret()));
+        assertNotEquals(secret1, newSecret);
+        assertNotEquals(secret2, newSecret);
+        String secret3 = newSecret;
+
+        // Update the client and fetch secret again to confirm it didnt changed
+        client.setName("Updated name");
+        client = clientManager.edit(client, false);
+        assertEquals("Updated name", client.getName());
+        assertFalse(PojoUtil.isEmpty(client.getDecryptedSecret()));
+        assertTrue(client.getDecryptedSecret().length() > 1);
+        assertEquals(secret3, client.getDecryptedSecret());
+    }
+
     private void validateClientConfigSettings(ClientDetailsEntity entity, Date lastTimeEntityWasModified) {
         assertNotNull(entity.getAuthorizedGrantTypes());
         assertEquals(4, entity.getClientAuthorizedGrantTypes().size());
diff --git a/orcid-web/src/main/java/org/orcid/frontend/web/controllers/AdminController.java b/orcid-web/src/main/java/org/orcid/frontend/web/controllers/AdminController.java
@@ -27,8 +27,8 @@
 import org.orcid.core.manager.ProfileEntityCacheManager;
 import org.orcid.core.manager.TwoFactorAuthenticationManager;
 import org.orcid.core.manager.v3.*;
+import org.orcid.core.manager.v3.read_only.ClientManagerReadOnly;
 import org.orcid.core.manager.v3.read_only.RecordNameManagerReadOnly;
-import org.orcid.core.utils.PasswordResetToken;
 import org.orcid.core.utils.VerifyEmailUtils;
 import org.orcid.frontend.email.RecordEmailSender;
 import org.orcid.frontend.web.util.PasswordConstants;
@@ -56,12 +56,15 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.springframework.beans.factory.annotation.Value;
+import org.springframework.http.HttpStatus;
+import org.springframework.http.ResponseEntity;
 import org.springframework.stereotype.Controller;
 import org.springframework.web.bind.annotation.ModelAttribute;
 import org.springframework.web.bind.annotation.RequestBody;
 import org.springframework.web.bind.annotation.RequestMapping;
 import org.springframework.web.bind.annotation.RequestMethod;
 import org.springframework.web.bind.annotation.ResponseBody;
+import org.springframework.web.bind.annotation.PostMapping;
 import org.springframework.web.servlet.ModelAndView;
 
 /**
@@ -107,6 +110,9 @@ public class AdminController extends BaseController {
     @Resource(name = "clientManagerV3")
     private ClientManager clientManager;
 
+    @Resource(name = "clientManagerReadOnlyV3")
+    private ClientManagerReadOnly clientManagerReadOnly;
+
     @Resource(name = "profileDaoReadOnly")
     private ProfileDao profileDaoReadOnly;
 
@@ -686,7 +692,7 @@ public Map<String, String> findIdByEmailHelper(String csvEmails) {
 
     /**
      * Reset password validate
-     * 
+     *
      * @throws IllegalAccessException
      * @throws UnsupportedEncodingException
      */
@@ -1262,45 +1268,92 @@ private String getOrcidFromParam(String orcidOrEmail) {
     }
 
     @RequestMapping(value = "/add-client.json", method = RequestMethod.POST)
-    @Produces(value = { MediaType.APPLICATION_JSON })
-    public @ResponseBody Client createClient(HttpServletRequest serverRequest, HttpServletResponse response, @RequestBody Client client) throws IllegalAccessException {
+    @Produces(value = {MediaType.APPLICATION_JSON})
+    @ResponseBody
+    public Client createClient(HttpServletRequest serverRequest, HttpServletResponse response, @RequestBody Client client) throws IllegalAccessException {
         isAdmin(serverRequest, response);
-        if(client == null) {
+
+        List<String> errors = new ArrayList<>();
+
+        if (client == null) {
             client = new Client();
-            client.getErrors().add("Client object cannot be null");
-        } else if(client.getMemberId() == null || PojoUtil.isEmpty(client.getMemberId())) {
-            client.getErrors().add("Member ID is requiered");
+            errors.add("Client object cannot be null");
+        } else if (client.getMemberId() == null || PojoUtil.isEmpty(client.getMemberId())) {
+            errors.add("Member ID is required");
         } else if (profileDaoReadOnly.getGroupType(client.getMemberId().getValue()) == null) {
-            client.getErrors().add("Member with ID " + client.getMemberId().getValue() + " does not exists");
-        } else if(client.getDisplayName() == null || PojoUtil.isEmpty(client.getDisplayName())) {
-            client.getErrors().add("Display name is requiered");
-        } else if(client.getShortDescription() == null || PojoUtil.isEmpty(client.getShortDescription())) {
-            client.getErrors().add("Description is requiered");
-        } else if(client.getWebsite() == null || PojoUtil.isEmpty(client.getWebsite())) {
-            client.getErrors().add("Website is requiered");
-        } else if(client.getRedirectUris() == null || client.getRedirectUris().isEmpty()) {
-            client.getErrors().add("Redirect URIs are requiered");
+            errors.add("Member with ID " + client.getMemberId().getValue() + " does not exists");
+        } else if (client.getDisplayName() == null || PojoUtil.isEmpty(client.getDisplayName())) {
+            errors.add("Display name is required");
+        } else if (client.getShortDescription() == null || PojoUtil.isEmpty(client.getShortDescription())) {
+            errors.add("Description is required");
+        } else if (client.getWebsite() == null || PojoUtil.isEmpty(client.getWebsite())) {
+            errors.add("Website is required");
+        } else if (client.getRedirectUris() == null || client.getRedirectUris().isEmpty()) {
+            errors.add("Redirect URIs are required");
         } else {
             // Validate the redirect uris are valid
-            for(RedirectUri r : client.getRedirectUris()) {
-                if(r.getType() == null || PojoUtil.isEmpty(r.getType())) {
-                    client.getErrors().add("Redirect uri type missing on redirect uri " + r.getValue().getValue());
+            for (RedirectUri r : client.getRedirectUris()) {
+                if (r.getType() == null || PojoUtil.isEmpty(r.getType())) {
+                    errors.add("Redirect uri type missing on redirect uri " + r.getValue().getValue());
                 }
             }
-            if(client.getErrors().isEmpty()) {
+
+            if (errors.isEmpty()) {
                 org.orcid.jaxb.model.v3.release.client.Client newClient = client.toModelObject();
                 try {
-                    newClient = clientManager.create(newClient);
+                    newClient = clientManager.createWithConfigValues(newClient);
                 } catch (Exception e) {
                     LOGGER.error(e.getMessage());
-                    String errorDesciption = getMessage("manage.developer_tools.group.cannot_create_client") + " " + e.getMessage();
+                    String errorDescription = getMessage("manage.developer_tools.group.cannot_create_client") + " " + e.getMessage();
                     client.setErrors(new ArrayList<String>());
-                    client.getErrors().add(errorDesciption);
+                    client.getErrors().add(errorDescription);
                     return client;
                 }
                 client = Client.fromModelObject(newClient);
             }
         }
+
+        if (!errors.isEmpty()) {
+            client.setErrors(errors);
+            return client;
+        }
+
+
         return client;
     }
+
+
+    @PostMapping(value = "/reset-client-secret.json", produces = MediaType.APPLICATION_JSON)
+    public ResponseEntity<Map<String, String>> resetClientSecret(HttpServletRequest serverRequest, HttpServletResponse response, @RequestBody Client client) throws IllegalAccessException {
+        isAdmin(serverRequest, response);
+
+        if (client == null || PojoUtil.isEmpty(client.getClientId())) {
+            return ResponseEntity
+                    .badRequest()
+                    .body(Map.of("error", "Client ID is required"));
+        }
+
+        String clientId = client.getClientId().getValue();
+
+        org.orcid.jaxb.model.v3.release.client.Client existingClient = clientManagerReadOnly.get(clientId);
+        if (existingClient == null) {
+            return ResponseEntity
+                    .status(HttpStatus.NOT_FOUND)
+                    .body(Map.of("error", "Client “" + clientId + "” not found"));
+        }
+
+        try {
+            String newSecret = clientManager.resetAndGetClientSecret(clientId);
+            return ResponseEntity
+                    .ok(Map.of(
+                            "clientId", clientId,
+                            "newSecret", newSecret
+                    ));
+        } catch (Exception e) {
+            LOGGER.error("Error resetting secret for client {}", clientId, e);
+            return ResponseEntity
+                    .status(HttpStatus.INTERNAL_SERVER_ERROR)
+                    .body(Map.of("error", "Failed to reset client secret"));
+        }
+    }
 }
diff --git a/orcid-web/src/test/java/org/orcid/frontend/web/controllers/AdminControllerTest.java b/orcid-web/src/test/java/org/orcid/frontend/web/controllers/AdminControllerTest.java
