From 4fa940a31ef2ee0e5f12c1987bc7c6294b2ad6d5 Mon Sep 17 00:00:00 2001 From: graziang Date: Mon, 4 Mar 2024 09:47:03 +0100 Subject: [PATCH] Device verification flow always requires consent Force consent for device verification flow when there are no client scopes to approve by adding a default client scope to approve Closes #26100 Signed-off-by: graziang --- .../managers/AuthenticationManager.java | 10 +++-- .../OAuth2DeviceAuthorizationGrantTest.java | 41 ++++++++++++++++++- 2 files changed, 47 insertions(+), 4 deletions(-) diff --git a/services/src/main/java/org/keycloak/services/managers/AuthenticationManager.java b/services/src/main/java/org/keycloak/services/managers/AuthenticationManager.java index 3bc4784c63..409c74b5ef 100755 --- a/services/src/main/java/org/keycloak/services/managers/AuthenticationManager.java +++ b/services/src/main/java/org/keycloak/services/managers/AuthenticationManager.java @@ -1058,7 +1058,7 @@ public class AuthenticationManager { UserConsentModel grantedConsent = getEffectiveGrantedConsent(session, authSession); // See if any clientScopes need to be approved on consent screen - List clientScopesToApprove = getClientScopesToApproveOnConsentScreen(grantedConsent, session); + List clientScopesToApprove = getClientScopesToApproveOnConsentScreen(grantedConsent, session, authSession); if (!clientScopesToApprove.isEmpty()) { return CommonClientSessionModel.Action.OAUTH_GRANT.name(); } @@ -1122,7 +1122,7 @@ public class AuthenticationManager { UserConsentModel grantedConsent = getEffectiveGrantedConsent(session, authSession); - List clientScopesToApprove = getClientScopesToApproveOnConsentScreen(grantedConsent, session); + List clientScopesToApprove = getClientScopesToApproveOnConsentScreen(grantedConsent, session, authSession); // Skip grant screen if everything was already approved by this user if (clientScopesToApprove.size() > 0) { @@ -1149,7 +1149,7 @@ public class AuthenticationManager { } - private static List getClientScopesToApproveOnConsentScreen(UserConsentModel grantedConsent, KeycloakSession session) { + private static List getClientScopesToApproveOnConsentScreen(UserConsentModel grantedConsent, KeycloakSession session, AuthenticationSessionModel authSession) { // Client Scopes to be displayed on consent screen List clientScopesToDisplay = new LinkedList<>(); @@ -1165,6 +1165,10 @@ public class AuthenticationManager { clientScopesToDisplay.add(authDetails); } } + //force consent when running a verification flow of OAuth 2.0 Device Authorization Grant + if(clientScopesToDisplay.isEmpty() && isOAuth2DeviceVerificationFlow(authSession)) { + clientScopesToDisplay.add(new AuthorizationDetails(authSession.getClient())); + } return clientScopesToDisplay; } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/OAuth2DeviceAuthorizationGrantTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/OAuth2DeviceAuthorizationGrantTest.java index 5cac4f58b4..1ecce110de 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/OAuth2DeviceAuthorizationGrantTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/OAuth2DeviceAuthorizationGrantTest.java @@ -30,6 +30,7 @@ import org.junit.Test; import org.keycloak.OAuth2Constants; import org.keycloak.OAuthErrorException; import org.keycloak.admin.client.resource.ClientResource; +import org.keycloak.admin.client.resource.ClientsResource; import org.keycloak.admin.client.resource.RealmResource; import org.keycloak.common.Profile; import org.keycloak.events.Errors; @@ -42,6 +43,7 @@ import org.keycloak.protocol.oidc.representations.OIDCConfigurationRepresentatio import org.keycloak.representations.AccessToken; import org.keycloak.representations.UserInfo; import org.keycloak.representations.idm.ClientRepresentation; +import org.keycloak.representations.idm.ClientScopeRepresentation; import org.keycloak.representations.idm.RealmRepresentation; import org.keycloak.representations.idm.UserRepresentation; import org.keycloak.testsuite.AbstractKeycloakTest; @@ -83,6 +85,7 @@ public class OAuth2DeviceAuthorizationGrantTest extends AbstractKeycloakTest { private static final String DEVICE_APP = "test-device"; private static final String DEVICE_APP_PUBLIC = "test-device-public"; private static final String DEVICE_APP_PUBLIC_CUSTOM_CONSENT = "test-device-public-custom-consent"; + private static final String DEVICE_APP_WITHOUT_SCOPES = "test-device-without-scopes"; private static final String SHORT_DEVICE_FLOW_URL = "https://keycloak.org/device"; @Rule @@ -105,7 +108,7 @@ public class OAuth2DeviceAuthorizationGrantTest extends AbstractKeycloakTest { ClientRepresentation app = ClientBuilder.create() .id(KeycloakModelUtils.generateId()) - .clientId("test-device") + .clientId(DEVICE_APP) .secret("secret") .attribute(OAuth2DeviceConfig.OAUTH2_DEVICE_AUTHORIZATION_GRANT_ENABLED, "true") .build(); @@ -125,6 +128,13 @@ public class OAuth2DeviceAuthorizationGrantTest extends AbstractKeycloakTest { .build(); realm.client(appPublicCustomConsent); + ClientRepresentation appWithoutScopes = ClientBuilder.create().publicClient() + .id(KeycloakModelUtils.generateId()) + .clientId(DEVICE_APP_WITHOUT_SCOPES) + .attribute(OAuth2DeviceConfig.OAUTH2_DEVICE_AUTHORIZATION_GRANT_ENABLED, "true") + .build(); + realm.client(appWithoutScopes); + userId = KeycloakModelUtils.generateId(); UserRepresentation user = UserBuilder.create() .id(userId) @@ -478,6 +488,35 @@ public class OAuth2DeviceAuthorizationGrantTest extends AbstractKeycloakTest { assertNotNull(token); } + @Test + public void testPublicClientConsentWithoutScopes() throws Exception { + + ClientsResource clients = realmsResouce().realm(REALM_NAME).clients(); + ClientRepresentation clientRep = clients.findByClientId(DEVICE_APP_WITHOUT_SCOPES).get(0); + ClientResource client = clients.get(clientRep.getId()); + + List defaultClientScopes = client.getDefaultClientScopes(); + defaultClientScopes.forEach(scope -> client.removeDefaultClientScope(scope.getId())); + + List optionalClientScopes = client.getOptionalClientScopes(); + optionalClientScopes.forEach(scope -> client.removeOptionalClientScope(scope.getId())); + + // Device Authorization Request from device + oauth.realm(REALM_NAME); + oauth.clientId(DEVICE_APP_WITHOUT_SCOPES); + OAuthClient.DeviceAuthorizationResponse response = oauth.doDeviceAuthorizationRequest(DEVICE_APP_WITHOUT_SCOPES, null); + + Assert.assertEquals(200, response.getStatusCode()); + assertNotNull(response.getVerificationUriComplete()); + openVerificationPage(response.getVerificationUriComplete()); + loginPage.assertCurrent(); + + // Do Login + oauth.fillLoginForm("device-login", "password"); + // Consent + grantPage.assertCurrent(); + } + @Test public void testNoRefreshToken() throws Exception { ClientResource client = ApiUtil.findClientByClientId(adminClient.realm(REALM_NAME), DEVICE_APP);