diff --git a/openam-oauth2/src/main/java/org/forgerock/oauth2/core/DeviceCodeGrantTypeHandler.java b/openam-oauth2/src/main/java/org/forgerock/oauth2/core/DeviceCodeGrantTypeHandler.java index 7f9de61997..b7bc479788 100644 --- a/openam-oauth2/src/main/java/org/forgerock/oauth2/core/DeviceCodeGrantTypeHandler.java +++ b/openam-oauth2/src/main/java/org/forgerock/oauth2/core/DeviceCodeGrantTypeHandler.java @@ -12,7 +12,7 @@ * information: "Portions copyright [year] [name of copyright owner]". * * Copyright 2015-2016 ForgeRock AS. - * Portions copyright 2025 3A Systems LLC. + * Portions copyright 2025-2026 3A Systems LLC. */ package org.forgerock.oauth2.core; @@ -91,7 +91,13 @@ protected AccessToken handle(OAuth2Request request, ClientRegistration client, !request.getParameter(REALM).equals(deviceCode.getRealm())) { throw new AuthorizationDeclinedException(); } - + + // RFC 8628 section 3.5: an expired device code gets expired_token even if the user has approved it + if (deviceCode.getExpiryTime() < currentTimeMillis()) { + tryDeleteDeviceCode(clientId, code, request); + throw new ExpiredTokenException(); + } + if (deviceCode.isAuthorized()) { String grantType = request.getParameter(OAuth2Constants.Params.GRANT_TYPE); Set scope = deviceCode.getScope(); @@ -109,14 +115,6 @@ protected AccessToken handle(OAuth2Request request, ClientRegistration client, return accessToken; } - - // only reachable when not authorized - the branch above returns - if (deviceCode.getExpiryTime() < currentTimeMillis()) { - tryDeleteDeviceCode(clientId, code, request); - throw new ExpiredTokenException(); - } - - try { final long lastPollTime = deviceCode.getLastPollTime(); if (lastPollTime + (providerSettings.getDeviceCodePollInterval() * 1000) > currentTimeMillis()) { diff --git a/openam-oauth2/src/main/java/org/forgerock/oauth2/restlet/DeviceCodeVerificationResource.java b/openam-oauth2/src/main/java/org/forgerock/oauth2/restlet/DeviceCodeVerificationResource.java index 5ccec6cec6..72349f006b 100644 --- a/openam-oauth2/src/main/java/org/forgerock/oauth2/restlet/DeviceCodeVerificationResource.java +++ b/openam-oauth2/src/main/java/org/forgerock/oauth2/restlet/DeviceCodeVerificationResource.java @@ -18,6 +18,7 @@ import static org.forgerock.openam.oauth2.OAuth2Constants.Params.CLIENT_ID; import static org.forgerock.openam.oauth2.OAuth2Constants.Params.SCOPE; +import static org.forgerock.openam.utils.Time.currentTimeMillis; import jakarta.inject.Inject; import jakarta.inject.Named; @@ -142,7 +143,9 @@ public Representation verify(Representation body) throws ServerException, NotFou return getTemplateRepresentation(FORM, request, "not_found"); } - if (deviceCode == null || deviceCode.isIssued()) { + // An expired code, or one that has already been approved, is treated as an unknown user code + if (deviceCode == null || deviceCode.isIssued() || deviceCode.isAuthorized() + || deviceCode.getExpiryTime() < currentTimeMillis()) { return getTemplateRepresentation(FORM, request, "not_found"); } diff --git a/openam-oauth2/src/test/java/org/forgerock/oauth2/core/DeviceCodeGrantTypeHandlerTest.java b/openam-oauth2/src/test/java/org/forgerock/oauth2/core/DeviceCodeGrantTypeHandlerTest.java index fcf0dd10f0..fe39deb80e 100644 --- a/openam-oauth2/src/test/java/org/forgerock/oauth2/core/DeviceCodeGrantTypeHandlerTest.java +++ b/openam-oauth2/src/test/java/org/forgerock/oauth2/core/DeviceCodeGrantTypeHandlerTest.java @@ -137,6 +137,7 @@ public void shouldGenerateAccessTokenAndDeleteDeviceCodeWhenAuthorized() given(deviceCode.getClientId()).willReturn("CLIENT_ID"); given(deviceCode.getRealm()).willReturn("/REALM"); given(deviceCode.isAuthorized()).willReturn(true); + given(deviceCode.getExpiryTime()).willReturn(currentTimeMillis() + 10000); given(deviceCode.getScope()).willReturn(scope); given(deviceCode.getResourceOwnerId()).willReturn("RESOURCE_OWNER"); given(deviceCode.getNonce()).willReturn("NONCE"); @@ -171,8 +172,8 @@ public void shouldGenerateAccessTokenAndDeleteDeviceCodeWhenAuthorized() assertEquals(actualAccessToken, accessToken); } - @Test(expectedExceptions = ExpiredTokenException.class) - public void handleShouldThrowExpiredTokenExceptionWhenDeviceCodeHasExpired() + @Test + public void handleShouldThrowExpiredTokenExceptionAndDeleteDeviceCodeWhenDeviceCodeHasExpired() throws Exception { // Given @@ -194,10 +195,47 @@ public void handleShouldThrowExpiredTokenExceptionWhenDeviceCodeHasExpired() given(deviceCode.getExpiryTime()).willReturn(currentTimeMillis() - 100); // When - grantTypeHandler.handle(request, client, providerSettings); + try { + grantTypeHandler.handle(request, client, providerSettings); + fail("Expected ExpiredTokenException"); + } catch (ExpiredTokenException e) { + // Then + verify(tokenStore).deleteDeviceCode("CLIENT_ID", "DEVICE_CODE", request); + } + } - // Then - // Expect ExpiredTokenException + @Test + public void handleShouldNotIssueAccessTokenWhenAuthorizedDeviceCodeHasExpired() + throws Exception { + + // Given + OAuth2Request request = mock(OAuth2Request.class); + ClientRegistration client = mock(ClientRegistration.class); + DeviceCode deviceCode = mock(DeviceCode.class); + + given(request.getParameter(DEVICE_CODE)).willReturn("DEVICE_CODE"); + given(request.getParameter(REALM)).willReturn("/REALM"); + + given(client.getClientId()).willReturn("CLIENT_ID"); + + given(tokenStore.readDeviceCode("CLIENT_ID", "DEVICE_CODE", request)) + .willReturn(deviceCode); + + given(deviceCode.getClientId()).willReturn("CLIENT_ID"); + given(deviceCode.getRealm()).willReturn("/REALM"); + given(deviceCode.isAuthorized()).willReturn(true); + given(deviceCode.getResourceOwnerId()).willReturn("RESOURCE_OWNER"); + given(deviceCode.getExpiryTime()).willReturn(currentTimeMillis() - 100); + + // When + try { + grantTypeHandler.handle(request, client, providerSettings); + fail("Expected ExpiredTokenException"); + } catch (ExpiredTokenException e) { + // Then + verifyZeroInteractions(accessTokenGenerator); + verify(tokenStore).deleteDeviceCode("CLIENT_ID", "DEVICE_CODE", request); + } } @Test @@ -259,6 +297,7 @@ public void shouldNotDeleteDeviceCodeWhenAccessTokenGenerationFails() given(deviceCode.getClientId()).willReturn("CLIENT_ID"); given(deviceCode.getRealm()).willReturn("/REALM"); given(deviceCode.isAuthorized()).willReturn(true); + given(deviceCode.getExpiryTime()).willReturn(currentTimeMillis() + 10000); given(deviceCode.getScope()).willReturn(scope); given(deviceCode.getResourceOwnerId()).willReturn("RESOURCE_OWNER"); diff --git a/openam-oauth2/src/test/java/org/forgerock/oauth2/restlet/DeviceCodeVerificationResourceTest.java b/openam-oauth2/src/test/java/org/forgerock/oauth2/restlet/DeviceCodeVerificationResourceTest.java new file mode 100644 index 0000000000..3d1aa2d860 --- /dev/null +++ b/openam-oauth2/src/test/java/org/forgerock/oauth2/restlet/DeviceCodeVerificationResourceTest.java @@ -0,0 +1,179 @@ +/* + * The contents of this file are subject to the terms of the Common Development and + * Distribution License (the License). You may not use this file except in compliance with the + * License. + * + * You can obtain a copy of the License at legal/CDDLv1.0.txt. See the License for the + * specific language governing permission and limitations under the License. + * + * When distributing Covered Software, include this CDDL Header Notice in each file and include + * the License file at legal/CDDLv1.0.txt. If applicable, add the following below the CDDL + * Header, with the fields enclosed by brackets [] replaced by your own identifying + * information: "Portions copyright [year] [name of copyright owner]". + * + * Copyright 2026 3A Systems, LLC. + */ + +package org.forgerock.oauth2.restlet; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.forgerock.openam.utils.Time.currentTimeMillis; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.BDDMockito.given; +import static org.mockito.Mockito.*; + +import java.util.Collections; +import java.util.Map; + +import org.forgerock.oauth2.core.AuthorizationService; +import org.forgerock.oauth2.core.ClientRegistration; +import org.forgerock.oauth2.core.ClientRegistrationStore; +import org.forgerock.oauth2.core.CsrfProtection; +import org.forgerock.oauth2.core.DeviceCode; +import org.forgerock.oauth2.core.OAuth2ProviderSettings; +import org.forgerock.oauth2.core.OAuth2ProviderSettingsFactory; +import org.forgerock.oauth2.core.OAuth2Request; +import org.forgerock.oauth2.core.OAuth2RequestFactory; +import org.forgerock.oauth2.core.ResourceOwner; +import org.forgerock.oauth2.core.ResourceOwnerSessionValidator; +import org.forgerock.oauth2.core.TokenStore; +import org.forgerock.openam.oauth2.OAuth2Constants; +import org.forgerock.openam.oauth2.OAuth2Utils; +import org.forgerock.openam.services.baseurl.BaseURLProvider; +import org.forgerock.openam.services.baseurl.BaseURLProviderFactory; +import org.forgerock.openam.xui.XUIState; +import org.restlet.Context; +import org.restlet.Request; +import org.restlet.Response; +import org.restlet.Restlet; +import org.restlet.data.Status; +import org.restlet.ext.freemarker.TemplateRepresentation; +import org.restlet.representation.Representation; +import org.restlet.routing.Router; +import org.testng.annotations.BeforeMethod; +import org.testng.annotations.Test; + +/** + * Tests for DeviceCodeVerificationResource. + */ +public class DeviceCodeVerificationResourceTest { + + private static final String USER_CODE = "USER_CODE"; + + private DeviceCodeVerificationResource resource; + private OAuth2Request o2request; + private TokenStore tokenStore; + private AuthorizationService authorizationService; + private ResourceOwnerSessionValidator resourceOwnerSessionValidator; + private DeviceCode deviceCode; + + @BeforeMethod + public void setup() throws Exception { + Request request = new Request(); + OAuth2RequestFactory requestFactory = mock(OAuth2RequestFactory.class); + o2request = mock(OAuth2Request.class); + given(requestFactory.create(request)).willReturn(o2request); + given(o2request.getParameter(OAuth2Constants.DeviceCode.USER_CODE)).willReturn(USER_CODE); + given(o2request.getParameter("realm")).willReturn("/"); + given(o2request.getRequest()).willReturn(request); + + BaseURLProviderFactory baseURLProviderFactory = mock(BaseURLProviderFactory.class); + given(baseURLProviderFactory.get(anyString())).willReturn(mock(BaseURLProvider.class)); + + tokenStore = mock(TokenStore.class); + deviceCode = mock(DeviceCode.class); + given(deviceCode.getObject()).willReturn(Collections.emptyMap()); + given(tokenStore.readDeviceCode(USER_CODE, o2request)).willReturn(deviceCode); + + OAuth2ProviderSettingsFactory providerSettingsFactory = mock(OAuth2ProviderSettingsFactory.class); + OAuth2ProviderSettings providerSettings = mock(OAuth2ProviderSettings.class); + given(providerSettingsFactory.get(o2request)).willReturn(providerSettings); + given(providerSettings.clientsCanSkipConsent()).willReturn(true); + + ClientRegistrationStore clientRegistrationStore = mock(ClientRegistrationStore.class); + ClientRegistration clientRegistration = mock(ClientRegistration.class); + given(clientRegistrationStore.get(any(), any())).willReturn(clientRegistration); + given(clientRegistration.isConsentImplied()).willReturn(true); + + resourceOwnerSessionValidator = mock(ResourceOwnerSessionValidator.class); + ResourceOwner resourceOwner = mock(ResourceOwner.class); + given(resourceOwner.getId()).willReturn("RESOURCE_OWNER"); + given(resourceOwnerSessionValidator.validate(o2request)).willReturn(resourceOwner); + + authorizationService = mock(AuthorizationService.class); + + resource = spy(new DeviceCodeVerificationResource(mock(XUIState.class), mock(Router.class), + baseURLProviderFactory, mock(OAuth2Representation.class), tokenStore, requestFactory, + authorizationService, providerSettingsFactory, mock(ExceptionHandler.class), + resourceOwnerSessionValidator, clientRegistrationStore, mock(OAuth2Utils.class), + mock(CsrfProtection.class))); + doReturn(request).when(resource).getRequest(); + doReturn(templatesFromClasspathContext()).when(resource).getContext(); + } + + @Test + public void shouldAuthorizeValidDeviceCode() throws Exception { + // Given + given(deviceCode.getExpiryTime()).willReturn(currentTimeMillis() + 10000); + + // When + Representation result = resource.verify(null); + + // Then + assertThat(errorCode(result)).isNull(); + verify(deviceCode).setResourceOwnerId("RESOURCE_OWNER"); + verify(deviceCode).setAuthorized(true); + verify(tokenStore).updateDeviceCode(deviceCode, o2request); + } + + @Test + public void shouldRejectExpiredDeviceCode() throws Exception { + // Given + given(deviceCode.getExpiryTime()).willReturn(currentTimeMillis() - 100); + + // When + Representation result = resource.verify(null); + + // Then + assertThat(errorCode(result)).isEqualTo("not_found"); + verify(deviceCode, never()).setAuthorized(anyBoolean()); + verify(tokenStore, never()).updateDeviceCode(any(), any()); + verifyZeroInteractions(resourceOwnerSessionValidator, authorizationService); + } + + @Test + public void shouldRejectDeviceCodeThatIsAlreadyAuthorized() throws Exception { + // Given + given(deviceCode.getExpiryTime()).willReturn(currentTimeMillis() + 10000); + given(deviceCode.isAuthorized()).willReturn(true); + + // When + Representation result = resource.verify(null); + + // Then + assertThat(errorCode(result)).isEqualTo("not_found"); + verify(deviceCode, never()).setResourceOwnerId(anyString()); + verify(tokenStore, never()).updateDeviceCode(any(), any()); + verifyZeroInteractions(resourceOwnerSessionValidator, authorizationService); + } + + /** + * A context whose clap:// lookups find nothing, so the templates load from the classpath. + */ + private static Context templatesFromClasspathContext() { + Context context = new Context(); + context.setClientDispatcher(new Restlet() { + @Override + public void handle(Request request, Response response) { + response.setStatus(Status.CLIENT_ERROR_NOT_FOUND); + } + }); + return context; + } + + @SuppressWarnings("unchecked") + private static String errorCode(Representation representation) { + return ((Map) ((TemplateRepresentation) representation).getDataModel()).get("errorCode"); + } +}