Merge pull request #4264 from stianst/KEYCLOAK-5074

KEYCLOAK-5074 Allow updating client secret through client registratio…

server-spi-private/src/main/java/org/keycloak/models/utils/RepresentationToModel.java

@@ -1201,6 +1201,7 @@ public class RepresentationToModel {
         if (rep.isUseTemplateScope() != null) resource.setUseTemplateScope(rep.isUseTemplateScope());
         if (rep.isUseTemplateMappers() != null) resource.setUseTemplateMappers(rep.isUseTemplateMappers());
 
+        if (rep.getSecret() != null) resource.setSecret(rep.getSecret());
 
         if (rep.getClientTemplate() != null) {
             if (rep.getClientTemplate().equals(ClientTemplateRepresentation.NONE)) {

services/src/main/java/org/keycloak/services/clientregistration/AbstractClientRegistrationProvider.java

@@ -97,9 +97,12 @@ public abstract class AbstractClientRegistrationProvider implements ClientRegist
         auth.requireView(client);
 
         ClientRepresentation rep = ModelToRepresentation.toRepresentation(client);
+        if (client.getSecret() != null) {
+            rep.setSecret(client.getSecret());
+        }
 
         if (auth.isRegistrationAccessToken()) {
-            String registrationAccessToken = ClientRegistrationTokenUtils.getRegistrationAccessToken(session, client, auth.getRegistrationAuth());
+            String registrationAccessToken = ClientRegistrationTokenUtils.updateTokenSignature(session, auth);
             rep.setRegistrationAccessToken(registrationAccessToken);
         }
 

services/src/main/java/org/keycloak/services/clientregistration/ClientRegistrationAuth.java

@@ -60,6 +60,8 @@ public class ClientRegistrationAuth {
     private RealmModel realm;
     private JsonWebToken jwt;
     private ClientInitialAccessModel initialAccessModel;
+    private String kid;
+    private String token;
 
     public ClientRegistrationAuth(KeycloakSession session, ClientRegistrationProvider provider, EventBuilder event) {
         this.session = session;
@@ -81,10 +83,13 @@ public class ClientRegistrationAuth {
             return;
         }
 
-        ClientRegistrationTokenUtils.TokenVerification tokenVerification = ClientRegistrationTokenUtils.verifyToken(session, realm, uri, split[1]);
+        token = split[1];
+
+        ClientRegistrationTokenUtils.TokenVerification tokenVerification = ClientRegistrationTokenUtils.verifyToken(session, realm, uri, token);
         if (tokenVerification.getError() != null) {
             throw unauthorized(tokenVerification.getError().getMessage());
         }
+        kid = tokenVerification.getKid();
         jwt = tokenVerification.getJwt();
 
         if (isInitialAccessToken()) {
@@ -95,6 +100,18 @@ public class ClientRegistrationAuth {
         }
     }
 
+    public String getToken() {
+        return token;
+    }
+
+    public String getKid() {
+        return kid;
+    }
+
+    public JsonWebToken getJwt() {
+        return jwt;
+    }
+
     private boolean isBearerToken() {
         return jwt != null && TokenUtil.TOKEN_TYPE_BEARER.equals(jwt.getType());
     }

services/src/main/java/org/keycloak/services/clientregistration/ClientRegistrationTokenUtils.java

@@ -44,11 +44,25 @@ public class ClientRegistrationTokenUtils {
     public static final String TYPE_INITIAL_ACCESS_TOKEN = "InitialAccessToken";
     public static final String TYPE_REGISTRATION_ACCESS_TOKEN = "RegistrationAccessToken";
 
-    public static String getRegistrationAccessToken(KeycloakSession session, ClientModel client, RegistrationAuth registrationAuth) {
+    public static String updateTokenSignature(KeycloakSession session, ClientRegistrationAuth auth) {
-        RegistrationAccessToken regToken = new RegistrationAccessToken();
+        KeyManager.ActiveRsaKey keys = session.keys().getActiveRsaKey(session.getContext().getRealm());
-        regToken.setRegistrationAuth(registrationAuth.toString().toLowerCase());
 
-        return setupToken(regToken, session, session.getContext().getRealm(), session.getContext().getUri(), client.getRegistrationToken(), TYPE_REGISTRATION_ACCESS_TOKEN, 0);
+        if (keys.getKid().equals(auth.getKid())) {
+            return auth.getToken();
+        } else {
+            RegistrationAccessToken regToken = new RegistrationAccessToken();
+            regToken.setRegistrationAuth(auth.getRegistrationAuth().toString().toLowerCase());
+
+            regToken.type(auth.getJwt().getType());
+            regToken.id(auth.getJwt().getId());
+            regToken.issuedAt(Time.currentTime());
+            regToken.expiration(0);
+            regToken.issuer(auth.getJwt().getIssuer());
+            regToken.audience(auth.getJwt().getIssuer());
+
+            String token = new JWSBuilder().kid(keys.getKid()).jsonContent(regToken).rsa256(keys.getPrivateKey());
+            return token;
+        }
     }
 
     public static String updateRegistrationAccessToken(KeycloakSession session, ClientModel client, RegistrationAuth registrationAuth) {
@@ -82,7 +96,8 @@ public class ClientRegistrationTokenUtils {
             return TokenVerification.error(new RuntimeException("Invalid token", e));
         }
 
-        PublicKey publicKey = session.keys().getRsaPublicKey(realm, input.getHeader().getKeyId());
+        String kid = input.getHeader().getKeyId();
+        PublicKey publicKey = session.keys().getRsaPublicKey(realm, kid);
 
         if (!RSAProvider.verify(input, publicKey)) {
             return TokenVerification.error(new RuntimeException("Failed verify token"));
@@ -109,7 +124,7 @@ public class ClientRegistrationTokenUtils {
             return TokenVerification.error(new RuntimeException("Invalid type of token"));
         }
 
-        return TokenVerification.success(jwt);
+        return TokenVerification.success(kid, jwt);
     }
 
     private static String setupToken(JsonWebToken jwt, KeycloakSession session, RealmModel realm, UriInfo uri, String id, String type, int expiration) {
@@ -134,22 +149,28 @@ public class ClientRegistrationTokenUtils {
 
     protected static class TokenVerification {
 
+        private final String kid;
         private final JsonWebToken jwt;
         private final RuntimeException error;
 
-        public static TokenVerification success(JsonWebToken jwt) {
+        public static TokenVerification success(String kid, JsonWebToken jwt) {
-            return new TokenVerification(jwt, null);
+            return new TokenVerification(kid, jwt, null);
         }
 
         public static TokenVerification error(RuntimeException error) {
-            return new TokenVerification(null, error);
+            return new TokenVerification(null,null, error);
         }
 
-        private TokenVerification(JsonWebToken jwt, RuntimeException error) {
+        private TokenVerification(String kid, JsonWebToken jwt, RuntimeException error) {
+            this.kid = kid;
             this.jwt = jwt;
             this.error = error;
         }
 
+        public String getKid() {
+            return kid;
+        }
+
         public JsonWebToken getJwt() {
             return jwt;
         }

testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/cli/registration/AbstractRegCliTest.java

@@ -455,7 +455,7 @@ public abstract class AbstractRegCliTest extends AbstractCliTest {
         ClientRepresentation client3 = JsonSerialization.readValue(exe.stdout(), ClientRepresentation.class);
         Assert.assertEquals("clientId", "test-client", client3.getClientId());
 
-        Assert.assertNotEquals("registrationAccessToken in returned json is different than one returned by create",
+        Assert.assertEquals("registrationAccessToken in returned json is different than one returned by create",
                 client.getRegistrationAccessToken(), client3.getRegistrationAccessToken());
 
         lastModified2 = configFile.exists() ? configFile.lastModified() : 0;

testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/ClientRegistrationTest.java

@@ -186,6 +186,23 @@ public class ClientRegistrationTest extends AbstractClientRegistrationTest {
         updateClient();
     }
 
+    @Test
+    public void updateClientSecret() throws ClientRegistrationException {
+        authManageClients();
+
+        registerClient();
+
+        ClientRepresentation client = reg.get(CLIENT_ID);
+        assertNotNull(client.getSecret());
+        client.setSecret("mysecret");
+
+        reg.update(client);
+
+        ClientRepresentation updatedClient = reg.get(CLIENT_ID);
+
+        assertEquals("mysecret", updatedClient.getSecret());
+    }
+
     @Test
     public void updateClientAsAdminWithCreateOnly() throws ClientRegistrationException {
         authCreateClients();

testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/RegistrationAccessTokenTest.java

@@ -82,8 +82,11 @@ public class RegistrationAccessTokenTest extends AbstractClientRegistrationTest
 
     @Test
     public void getClientWithRegistrationToken() throws ClientRegistrationException {
+        setTimeOffset(10);
+
         ClientRepresentation rep = reg.get(client.getClientId());
         assertNotNull(rep);
+
         assertEquals(client.getRegistrationAccessToken(), rep.getRegistrationAccessToken());
         assertNotNull(rep.getRegistrationAccessToken());
 

testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/keys/KeyRotationTest.java

@@ -23,6 +23,9 @@ import org.jboss.arquillian.graphene.page.Page;
 import org.junit.Rule;
 import org.junit.Test;
 import org.keycloak.RSATokenVerifier;
+import org.keycloak.client.registration.Auth;
+import org.keycloak.client.registration.ClientRegistration;
+import org.keycloak.client.registration.ClientRegistrationException;
 import org.keycloak.common.VerificationException;
 import org.keycloak.common.util.KeyUtils;
 import org.keycloak.common.util.MultivaluedHashMap;
@@ -31,6 +34,8 @@ import org.keycloak.keys.Attributes;
 import org.keycloak.keys.GeneratedHmacKeyProviderFactory;
 import org.keycloak.keys.KeyProvider;
 import org.keycloak.keys.ImportedRsaKeyProviderFactory;
+import org.keycloak.representations.idm.ClientInitialAccessCreatePresentation;
+import org.keycloak.representations.idm.ClientInitialAccessPresentation;
 import org.keycloak.representations.idm.ClientRepresentation;
 import org.keycloak.representations.idm.ComponentRepresentation;
 import org.keycloak.representations.idm.KeysMetadataRepresentation;
@@ -41,11 +46,11 @@ import org.keycloak.testsuite.admin.ApiUtil;
 import org.keycloak.testsuite.pages.AppPage;
 import org.keycloak.testsuite.pages.AppPage.RequestType;
 import org.keycloak.testsuite.pages.LoginPage;
+import org.keycloak.testsuite.util.ClientBuilder;
 import org.keycloak.testsuite.util.KeycloakModelUtils;
 import org.keycloak.testsuite.util.OAuthClient;
 import org.keycloak.testsuite.util.UserInfoClientUtil;
 
-import javax.ws.rs.client.ClientBuilder;
 import javax.ws.rs.core.Response;
 import java.io.IOException;
 import java.security.KeyPair;
@@ -127,12 +132,27 @@ public class KeyRotationTest extends AbstractKeycloakTest {
         assertTokenSignature(key1, response.getAccessToken());
         assertTokenSignature(key1, response.getRefreshToken());
 
+        // Create client with keys #1
+        ClientInitialAccessCreatePresentation initialToken = new ClientInitialAccessCreatePresentation();
+        initialToken.setCount(100);
+        initialToken.setExpiration(0);
+        ClientInitialAccessPresentation accessRep = adminClient.realm("test").clientInitialAccess().create(initialToken);
+        String initialAccessToken = accessRep.getToken();
+
+        ClientRegistration reg = ClientRegistration.create().url(suiteContext.getAuthServerInfo().getContextRoot() + "/auth", "test").build();
+        reg.auth(Auth.token(initialAccessToken));
+        ClientRepresentation clientRep = reg.create(ClientBuilder.create().clientId("test").build());
+
         // Userinfo with keys #1
         assertUserInfo(response.getAccessToken(), 200);
 
         // Token introspection with keys #1
         assertTokenIntrospection(response.getAccessToken(), true);
 
+        // Get client with keys #1 - registration access token should not have changed
+        ClientRepresentation clientRep2 = reg.auth(Auth.token(clientRep.getRegistrationAccessToken())).get("test");
+        assertEquals(clientRep.getRegistrationAccessToken(), clientRep2.getRegistrationAccessToken());
+
         // Create keys #2
         PublicKey key2 = createKeys2();
 
@@ -148,6 +168,10 @@ public class KeyRotationTest extends AbstractKeycloakTest {
         // Token introspection with keys #2
         assertTokenIntrospection(response.getAccessToken(), true);
 
+        // Get client with keys #2 - registration access token should be changed
+        ClientRepresentation clientRep3 = reg.auth(Auth.token(clientRep.getRegistrationAccessToken())).get("test");
+        assertNotEquals(clientRep.getRegistrationAccessToken(), clientRep3.getRegistrationAccessToken());
+
         // Drop key #1
         dropKeys1();
 
@@ -162,6 +186,17 @@ public class KeyRotationTest extends AbstractKeycloakTest {
         // Token introspection with keys #1 dropped
         assertTokenIntrospection(response.getAccessToken(), true);
 
+        // Get client with keys #1 - should fail
+        try {
+            reg.auth(Auth.token(clientRep.getRegistrationAccessToken())).get("test");
+            fail("Expected to fail");
+        } catch (ClientRegistrationException e) {
+        }
+
+        // Get client with keys #2 - should succeed
+        ClientRepresentation clientRep4 = reg.auth(Auth.token(clientRep3.getRegistrationAccessToken())).get("test");
+        assertNotEquals(clientRep2.getRegistrationAccessToken(), clientRep4.getRegistrationAccessToken());
+
         // Drop key #2
         dropKeys2();
 
@@ -292,7 +327,7 @@ public class KeyRotationTest extends AbstractKeycloakTest {
     }
 
     private void assertUserInfo(String token, int expectedStatus) {
-        Response userInfoResponse = UserInfoClientUtil.executeUserInfoRequest_getMethod(ClientBuilder.newClient(), token);
+        Response userInfoResponse = UserInfoClientUtil.executeUserInfoRequest_getMethod(javax.ws.rs.client.ClientBuilder.newClient(), token);
         assertEquals(expectedStatus, userInfoResponse.getStatus());
         userInfoResponse.close();
     }