Verify holder of the device code (#21)

Closes https://github.com/keycloak/security/issues/32

Co-authored-by: Stian Thorgersen <stianst@gmail.com>
Conflicts:
    services/src/main/java/org/keycloak/protocol/oidc/grants/device/DeviceGrantType.java

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

@@ -193,6 +193,8 @@ public class OAuth2DeviceCodeModel {
         Map<String, String> result = new HashMap<>();
 
         result.put(REALM_ID, realm.getId());
+        result.put(CLIENT_ID, clientId);
+
         if (clientNotificationToken != null) {
             result.put(CLIENT_NOTIFICATION_TOKEN_NOTE, clientNotificationToken);
         }
@@ -201,7 +203,6 @@ public class OAuth2DeviceCodeModel {
         }
 
         if (denied == null) {
-            result.put(CLIENT_ID, clientId);
             result.put(EXPIRATION_NOTE, String.valueOf(expiration));
             result.put(POLLING_INTERVAL_NOTE, String.valueOf(pollingInterval));
             result.put(SCOPE_NOTE, scope);

services/src/main/java/org/keycloak/protocol/oidc/grants/ciba/CibaGrantType.java

@@ -161,19 +161,13 @@ public class CibaGrantType {
             throw new CorsErrorResponseException(cors, OAuthErrorException.INVALID_GRANT, cpe.getErrorDetail(), Response.Status.BAD_REQUEST);
         }
 
-        OAuth2DeviceCodeModel deviceCode = DeviceGrantType.getDeviceByDeviceCode(session, realm, request.getId());
+        OAuth2DeviceCodeModel deviceCode = DeviceGrantType.getDeviceByDeviceCode(session, realm, client, event, request.getId());
 
         if (deviceCode == null) {
             // Auth Req ID has not put onto cache, no need to remove Auth Req ID.
             throw new CorsErrorResponseException(cors, OAuthErrorException.INVALID_GRANT, "Invalid " + AUTH_REQ_ID, Response.Status.BAD_REQUEST);
         }
 
-        if (!request.getIssuedFor().equals(client.getClientId())) {
-            logDebug("invalid client.", request);
-            // the client sending this Auth Req ID does not match the client to which keycloak had issued Auth Req ID.
-            throw new CorsErrorResponseException(cors, OAuthErrorException.INVALID_GRANT, "unauthorized client", Response.Status.BAD_REQUEST);
-        }
-
         if (deviceCode.isExpired()) {
             logDebug("expired.", request);
             throw new CorsErrorResponseException(cors, OAuthErrorException.EXPIRED_TOKEN, "authentication timed out", Response.Status.BAD_REQUEST);

services/src/main/java/org/keycloak/protocol/oidc/grants/device/DeviceGrantType.java

@@ -48,6 +48,7 @@ import org.keycloak.protocol.oidc.grants.device.clientpolicy.context.DeviceToken
 import org.keycloak.protocol.oidc.grants.device.endpoints.DeviceEndpoint;
 import org.keycloak.protocol.oidc.utils.PkceUtils;
 import org.keycloak.services.CorsErrorResponseException;
+import org.keycloak.services.ErrorResponseException;
 import org.keycloak.services.clientpolicy.ClientPolicyException;
 import org.keycloak.services.managers.AuthenticationManager;
 import org.keycloak.services.managers.UserSessionCrossDCManager;
@@ -145,10 +146,20 @@ public class DeviceGrantType {
         return flow != null;
     }
 
-    public static OAuth2DeviceCodeModel getDeviceByDeviceCode(KeycloakSession session, RealmModel realm, String deviceCode) {
+    public static OAuth2DeviceCodeModel getDeviceByDeviceCode(KeycloakSession session, RealmModel realm, ClientModel client, EventBuilder event, String deviceCode) {
         SingleUseObjectProvider singleUseStore = session.singleUseObjects();
         Map<String, String> notes = singleUseStore.get(OAuth2DeviceCodeModel.createKey(deviceCode));
-        return notes != null ? OAuth2DeviceCodeModel.fromCache(realm, deviceCode, notes) : null;
+        OAuth2DeviceCodeModel deviceCodeModel = notes != null ? OAuth2DeviceCodeModel.fromCache(realm, deviceCode, notes) : null;
+
+        if (deviceCodeModel != null) {
+            if (!client.getClientId().equals(deviceCodeModel.getClientId())) {
+                event.error(Errors.INVALID_OAUTH2_DEVICE_CODE);
+                throw new ErrorResponseException(OAuthErrorException.INVALID_GRANT, "unauthorized client",
+                        Response.Status.BAD_REQUEST);
+            }
+        }
+
+        return deviceCodeModel;
     }
 
     public static void removeDeviceByDeviceCode(KeycloakSession session, String deviceCode) {
@@ -227,7 +238,7 @@ public class DeviceGrantType {
                 "Missing parameter: " + OAuth2Constants.DEVICE_CODE, Response.Status.BAD_REQUEST);
         }
 
-        OAuth2DeviceCodeModel deviceCodeModel = getDeviceByDeviceCode(session, realm, deviceCode);
+        OAuth2DeviceCodeModel deviceCodeModel = getDeviceByDeviceCode(session, realm, client, event, deviceCode);
 
         if (deviceCodeModel == null) {
             event.error(Errors.INVALID_OAUTH2_DEVICE_CODE);

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

@@ -1104,6 +1104,52 @@ public class CIBATest extends AbstractClientPoliciesTest {
         }
     }
 
+    @Test
+    public void testVerifyHolderOfAuthenticationRequestRef() throws Exception {
+        ClientResource firstClientResource = null;
+        ClientResource secondClientResource = null;
+        ClientRepresentation firstClientRep = null;
+        ClientRepresentation secondClientRep = null;
+        try {
+            final String username = "nutzername-gelb";
+            final String firstClientName = "test-app-scope"; // see testrealm.json
+            final String secondClientName = TEST_CLIENT_NAME;
+            final String firstClientPassword = "password";
+            final String secondClientPassword = TEST_CLIENT_PASSWORD;
+            String firstClientAuthReqId = null;
+
+            firstClientResource = ApiUtil.findClientByClientId(adminClient.realm(TEST_REALM_NAME), firstClientName);
+            assertThat(firstClientResource, notNullValue());
+
+            firstClientRep = firstClientResource.toRepresentation();
+            prepareCIBASettings(firstClientResource, firstClientRep);
+
+            secondClientResource = ApiUtil.findClientByClientId(adminClient.realm(TEST_REALM_NAME), secondClientName);
+            assertThat(secondClientResource, notNullValue());
+
+            secondClientRep = secondClientResource.toRepresentation();
+            prepareCIBASettings(secondClientResource, secondClientRep);
+
+            // first client Backchannel Authentication Request
+            AuthenticationRequestAcknowledgement response = doBackchannelAuthenticationRequest(firstClientName, firstClientPassword, username, "asdfghjkl");
+            firstClientAuthReqId = response.getAuthReqId();
+
+            // first client Authentication Channel Request
+            TestAuthenticationChannelRequest firstClientAuthenticationChannelReq = doAuthenticationChannelRequest("asdfghjkl");
+
+            // first client Authentication Channel completed
+            doAuthenticationChannelCallback(firstClientAuthenticationChannelReq);
+
+            // second client Token Request
+            OAuthClient.AccessTokenResponse tokenRes = oauth.doBackchannelAuthenticationTokenRequest(secondClientName, secondClientPassword, firstClientAuthReqId);
+            assertEquals(400, tokenRes.getStatusCode());
+            assertEquals("unauthorized client", tokenRes.getErrorDescription());
+        } finally {
+            revertCIBASettings(firstClientResource, firstClientRep);
+            revertCIBASettings(secondClientResource, secondClientRep);
+        }
+    }
+
     @Test
     public void testRequestTokenBeforeAuthenticationNotCompleted() throws Exception {
         ClientResource clientResource = null;

testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/OAuth2DeviceAuthorizationGrantTest.java

@@ -16,6 +16,7 @@
  */
 package org.keycloak.testsuite.oauth;
 
+import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
 import static org.keycloak.models.OAuth2DeviceConfig.DEFAULT_OAUTH2_DEVICE_CODE_LIFESPAN;
@@ -59,11 +60,13 @@ import org.apache.http.impl.client.HttpClientBuilder;
 import org.apache.http.message.BasicNameValuePair;
 import org.apache.http.impl.client.CloseableHttpClient;
 import org.keycloak.util.BasicAuthHelper;
+import org.openqa.selenium.Cookie;
 
 import java.util.List;
 import java.util.LinkedList;
 
 import java.io.UnsupportedEncodingException;
+import java.util.concurrent.TimeUnit;
 
 /**
  * @author <a href="mailto:h2-wada@nri.co.jp">Hiroyuki Wada</a>
@@ -106,6 +109,7 @@ public class OAuth2DeviceAuthorizationGrantTest extends AbstractKeycloakTest {
 
         ClientRepresentation appPublic = ClientBuilder.create().id(KeycloakModelUtils.generateId()).publicClient()
             .clientId(DEVICE_APP_PUBLIC).attribute(OAuth2DeviceConfig.OAUTH2_DEVICE_AUTHORIZATION_GRANT_ENABLED, "true")
+            .redirectUris(OAuthClient.APP_ROOT + "/auth")
             .build();
         realm.client(appPublic);
 
@@ -243,6 +247,72 @@ public class OAuth2DeviceAuthorizationGrantTest extends AbstractKeycloakTest {
         }
     }
 
+    @Test
+    public void testVerifyHolderOfDeviceCode() throws Exception {
+        // Device Authorization Request from device
+        oauth.realm(REALM_NAME);
+        oauth.clientId(DEVICE_APP_PUBLIC);
+        OAuthClient.DeviceAuthorizationResponse response = oauth.doDeviceAuthorizationRequest(DEVICE_APP_PUBLIC, null);
+
+        assertEquals(200, response.getStatusCode());
+        assertNotNull(response.getDeviceCode());
+        assertNotNull(response.getUserCode());
+        assertNotNull(response.getVerificationUri());
+        assertNotNull(response.getVerificationUriComplete());
+        assertEquals(60, response.getExpiresIn());
+        assertEquals(5, response.getInterval());
+
+        openVerificationPage(response.getVerificationUriComplete());
+
+        // Do Login
+        oauth.fillLoginForm("device-login", "password");
+
+        // Consent
+        grantPage.accept();
+
+        // Token request from device
+        OAuthClient.AccessTokenResponse tokenResponse = oauth.doDeviceTokenRequest(DEVICE_APP_PUBLIC, null, response.getDeviceCode());
+
+        assertEquals(200, tokenResponse.getStatusCode());
+
+        String tokenString = tokenResponse.getAccessToken();
+        assertNotNull(tokenString);
+        AccessToken token = oauth.verifyToken(tokenString);
+
+        assertNotNull(token);
+
+        for (Cookie cookie : driver.manage().getCookies()) {
+            driver.manage().deleteCookie(cookie);
+        }
+
+        oauth.openLoginForm();
+
+        oauth.realm(REALM_NAME);
+        oauth.clientId(DEVICE_APP_PUBLIC_CUSTOM_CONSENT);
+
+        oauth.fillLoginForm("device-login", "password");
+
+        for (Cookie cookie : driver.manage().getCookies()) {
+            driver.manage().deleteCookie(cookie);
+        }
+
+        oauth.openLoginForm();
+
+        response = oauth.doDeviceAuthorizationRequest(DEVICE_APP_PUBLIC_CUSTOM_CONSENT, null);
+
+        openVerificationPage(response.getVerificationUriComplete());
+
+        // Consent
+        Assert.assertTrue(grantPage.getDisplayedGrants().contains("This is the custom consent screen text."));
+        grantPage.accept();
+
+        // Token request from device
+        tokenResponse = oauth.doDeviceTokenRequest(DEVICE_APP_PUBLIC, null, response.getDeviceCode());
+
+        assertEquals(400, tokenResponse.getStatusCode());
+        assertEquals("unauthorized client", tokenResponse.getErrorDescription());
+    }
+
     @Test
     public void testPublicClientOptionalScope() throws Exception {
         // Device Authorization Request from device - check giving optional scope phone