From 3ab83efbcd586b9543d6e2cf1af75679faa46dcd Mon Sep 17 00:00:00 2001 From: Felix <23635466+its-felix@users.noreply.github.com> Date: Mon, 23 Oct 2023 01:00:46 +0200 Subject: [PATCH] fix: delete REDIRECT_URI cookie on successful login --- pom.xml | 2 +- .../configuration/SecurityConfiguration.java | 1 + .../service/user/Gw2AuthTokenUserService.java | 2 + .../gw2auth/oauth2/server/util/Constants.java | 17 ++++ .../oauth2/server/util/CookieHelper.java | 13 +++ .../gw2auth/oauth2/server/SessionHandle.java | 6 +- .../server/oauth2/OAuth2ServerTest.java | 87 +++++++++++++++++++ 7 files changed, 124 insertions(+), 4 deletions(-) diff --git a/pom.xml b/pom.xml index 8fcabcd7..b1e00321 100644 --- a/pom.xml +++ b/pom.xml @@ -6,7 +6,7 @@ com.gw2auth oauth2-server - 1.58.0 + 1.58.1 jar diff --git a/src/main/java/com/gw2auth/oauth2/server/configuration/SecurityConfiguration.java b/src/main/java/com/gw2auth/oauth2/server/configuration/SecurityConfiguration.java index 193c24f8..d2937577 100644 --- a/src/main/java/com/gw2auth/oauth2/server/configuration/SecurityConfiguration.java +++ b/src/main/java/com/gw2auth/oauth2/server/configuration/SecurityConfiguration.java @@ -120,6 +120,7 @@ public Customizer> oauth2LoginCustomizer(@Qu } delegate.onAuthenticationSuccess(request, response, authentication); + requestCache.removeRequest(request, response); }); }; } diff --git a/src/main/java/com/gw2auth/oauth2/server/service/user/Gw2AuthTokenUserService.java b/src/main/java/com/gw2auth/oauth2/server/service/user/Gw2AuthTokenUserService.java index 207bce59..f68da731 100644 --- a/src/main/java/com/gw2auth/oauth2/server/service/user/Gw2AuthTokenUserService.java +++ b/src/main/java/com/gw2auth/oauth2/server/service/user/Gw2AuthTokenUserService.java @@ -116,6 +116,8 @@ public Optional resolveUserForToken(HttpServletRequest request, S user = new Gw2AuthUserV2(account.id(), accountFederation.issuer(), accountFederation.idAtIssuer(), sessionId, currentSessionMetadata, encryptionKeyBytes); request.setAttribute(REQUEST_ATTRIBUTE_NAME, user); + CookieHelper.clearCookieIfPresent(request, AuthenticationHelper.getCurrentResponse().orElseThrow(), Constants.REDIRECT_URI_COOKIE_NAME); + return Optional.of(user); } } diff --git a/src/main/java/com/gw2auth/oauth2/server/util/Constants.java b/src/main/java/com/gw2auth/oauth2/server/util/Constants.java index 76d8e1d4..5415dda3 100644 --- a/src/main/java/com/gw2auth/oauth2/server/util/Constants.java +++ b/src/main/java/com/gw2auth/oauth2/server/util/Constants.java @@ -1,7 +1,24 @@ package com.gw2auth.oauth2.server.util; +import org.springframework.security.web.savedrequest.CookieRequestCache; + +import java.lang.reflect.Field; + public final class Constants { public static final String LOGOUT_URL = "/auth/logout"; public static final String ACCESS_TOKEN_COOKIE_NAME = "BEARER"; + public static final String REDIRECT_URI_COOKIE_NAME; + + static { + try { + final Field field = CookieRequestCache.class.getDeclaredField("COOKIE_NAME"); + final boolean wasAccessible = field.canAccess(null); + field.setAccessible(true); + REDIRECT_URI_COOKIE_NAME = (String) field.get(null); + field.setAccessible(wasAccessible); + } catch (ReflectiveOperationException e) { + throw new RuntimeException(e); + } + } } diff --git a/src/main/java/com/gw2auth/oauth2/server/util/CookieHelper.java b/src/main/java/com/gw2auth/oauth2/server/util/CookieHelper.java index 05a6936a..0acb5c54 100644 --- a/src/main/java/com/gw2auth/oauth2/server/util/CookieHelper.java +++ b/src/main/java/com/gw2auth/oauth2/server/util/CookieHelper.java @@ -29,6 +29,19 @@ public static void clearCookie(HttpServletRequest request, HttpServletResponse r response.addCookie(cookie); } + public static void clearCookieIfPresent(HttpServletRequest request, HttpServletResponse response, String name) { + final Cookie[] cookies = request.getCookies(); + if (cookies != null) { + for (Cookie cookie : cookies) { + if (cookie.getName().equals(name)) { + cookie.setMaxAge(0); + response.addCookie(cookie); + break; + } + } + } + } + private static String getRequestContext(HttpServletRequest request) { String contextPath = request.getContextPath(); return contextPath.isEmpty() ? "/" : contextPath; diff --git a/src/test/java/com/gw2auth/oauth2/server/SessionHandle.java b/src/test/java/com/gw2auth/oauth2/server/SessionHandle.java index 890b8855..1a490c7e 100644 --- a/src/test/java/com/gw2auth/oauth2/server/SessionHandle.java +++ b/src/test/java/com/gw2auth/oauth2/server/SessionHandle.java @@ -34,7 +34,7 @@ public SessionHandle(String countryCode, String city, Double latitude, Double lo } public void addCookie(Cookie cookie) { - if (cookie.getMaxAge() > 0) { + if (cookie.getMaxAge() != 0) { this.cookies.put(cookie.getName(), cookie); } else { this.cookies.remove(cookie.getName()); @@ -72,14 +72,14 @@ public MockHttpServletRequest postProcessRequest(MockHttpServletRequest request) if (existingCookies != null) { for (Cookie cookie : existingCookies) { - if (cookie.getMaxAge() > 0) { + if (cookie.getMaxAge() != 0) { totalCookies.putIfAbsent(cookie.getName(), cookie); } } } for (Map.Entry entry : this.cookies.entrySet()) { - if (entry.getValue().getMaxAge() > 0) { + if (entry.getValue().getMaxAge() != 0) { totalCookies.putIfAbsent(entry.getKey(), entry.getValue()); } } diff --git a/src/test/java/com/gw2auth/oauth2/server/oauth2/OAuth2ServerTest.java b/src/test/java/com/gw2auth/oauth2/server/oauth2/OAuth2ServerTest.java index 58347ab5..3e298bd8 100644 --- a/src/test/java/com/gw2auth/oauth2/server/oauth2/OAuth2ServerTest.java +++ b/src/test/java/com/gw2auth/oauth2/server/oauth2/OAuth2ServerTest.java @@ -2133,6 +2133,93 @@ public void refreshWithLegacyAttributes(SessionHandle sessionHandle, OAuth2Clien assertNotEquals(refreshToken, tokenResponse.get("refresh_token").textValue()); } + @Test + public void unauthenticatedRequestShouldRememberURLAndRedirectUponLogin() throws Exception { + final ApplicationClientCreation applicationClientCreation = createApplicationClient(OAuth2ClientApiVersion.V0, OAuth2ClientType.CONFIDENTIAL); + final ApplicationClient applicationClient = applicationClientCreation.client(); + final String clientSecret = applicationClientCreation.clientSecret(); + + // perform authorization request (should redirect to the login page) + final SessionHandle sessionHandle = new SessionHandle(); + MvcResult result = performAuthorizeWithClient(sessionHandle, applicationClient, Set.of(OAuth2Scope.GW2_ACCOUNT)).andReturn(); + + // verify the redirect URI was saved + assertNotNull(sessionHandle.getCookie("REDIRECT_URI")); + + // login + result = this.gw2AuthLoginExtension.login(sessionHandle, "issuer", UUID.randomUUID().toString()) + .andExpectAll(this.gw2AuthLoginExtension.expectLoginSuccess()) + .andReturn(); + + // verify the redirect URI was removed + assertNull(sessionHandle.getCookie("REDIRECT_URI")); + + // follow redirect + result = this.mockMvc.perform( + get(URI.create(Objects.requireNonNull(result.getResponse().getRedirectedUrl()))) + .with(sessionHandle) + ) + .andDo(sessionHandle) + .andReturn(); + + // submit the consent + final String token = TestHelper.randomRootToken(); + result = performSubmitConsent( + sessionHandle, + applicationClient, + URI.create(Objects.requireNonNull(result.getResponse().getRedirectedUrl())), + Map.of(this.gw2AccountId1st, "First.1234"), + Map.of(this.gw2AccountId1st, "First"), + Map.of(this.gw2AccountId1st, token), + Map.of(this.gw2AccountId1st, Set.of(Gw2ApiPermission.ACCOUNT)), + Set.of(Gw2ApiPermission.ACCOUNT) + ).andReturn(); + + // verify the consent has been saved + final UUID accountId = this.testHelper.getAccountIdForCookie(sessionHandle).orElseThrow(); + final ApplicationClientAccountEntity applicationClientAccountEntity = this.applicationClientAccountRepository.findByApplicationClientIdAndAccountId( + applicationClient.id(), + accountId + ).orElse(null); + assertNotNull(applicationClientAccountEntity); + assertEquals(Set.of(OAuth2Scope.GW2_ACCOUNT.oauth2()), applicationClientAccountEntity.authorizedScopes()); + + // verify the authorization has been saved + final List authorizations = this.applicationClientAuthorizationRepository.findAllWithGw2AccountIdsByAccountIdAndApplicationClientId(accountId, applicationClient.id()); + assertEquals(1, authorizations.size()); + + final ApplicationClientAuthorizationWithGw2AccountIdsEntity clientAuthorization = authorizations.get(0); + assertEquals(Set.of(OAuth2Scope.GW2_ACCOUNT.oauth2()), clientAuthorization.authorization().authorizedScopes()); + assertEquals(Set.of(this.gw2AccountId1st), clientAuthorization.gw2AccountIds()); + + // verify tokens have been saved + List clientAuthorizationTokenEntities = this.applicationClientAuthorizationTokenRepository.findAllByApplicationClientAuthorizationIdAndAccountId(clientAuthorization.authorization().id(), accountId); + assertEquals(1, clientAuthorizationTokenEntities.size()); + + // set testing clock to token customizer + final Clock testingClock = Clock.fixed(Instant.now(), ZoneId.systemDefault()); + this.gw2AuthClockedExtension.setClock(testingClock); + + final String dummySubtoken = TestHelper.createSubtokenJWT(this.gw2AccountId1st, Set.of(Gw2ApiPermission.ACCOUNT), testingClock.instant(), Duration.ofMinutes(30L)); + + result = performRetrieveTokenByCode( + applicationClient, + clientSecret, + URI.create(Objects.requireNonNull(result.getResponse().getRedirectedUrl())), + Map.of(token, dummySubtoken), + Set.of(Gw2ApiPermission.ACCOUNT) + ) + .andExpectAll(expectValidTokenResponse(OAuth2Scope.GW2_ACCOUNT)) + .andReturn(); + + // verify the access token + assertTokenResponseV0( + result, + Map.of(this.gw2AccountId1st, Map.of("name", "First", "token", dummySubtoken)), + Set.of(OAuth2Scope.GW2_ACCOUNT) + ); + } + private ResultActions performRetrieveTokensByRefreshTokenAndExpectValid(ApplicationClient applicationClient, String clientSecret, String refreshToken) throws Exception { return performRetrieveTokensByRefreshToken(applicationClient, clientSecret, refreshToken) .andExpectAll(expectValidTokenResponse());