diff --git a/src/main/java/com/auth0/AuthorizeUrl.java b/src/main/java/com/auth0/AuthorizeUrl.java index 2d41b10..abb2194 100644 --- a/src/main/java/com/auth0/AuthorizeUrl.java +++ b/src/main/java/com/auth0/AuthorizeUrl.java @@ -252,7 +252,7 @@ private void storeTransient() { SameSite sameSiteValue = containsFormPost() ? SameSite.NONE : SameSite.LAX; TransientCookieStore.storeState(response, state, sameSiteValue, useLegacySameSiteCookie, setSecureCookie, cookiePath); - TransientCookieStore.storeNonce(response, nonce, sameSiteValue, useLegacySameSiteCookie, setSecureCookie, cookiePath); + TransientCookieStore.storeNonce(response, nonce, state, sameSiteValue, useLegacySameSiteCookie, setSecureCookie, cookiePath); // Store HMAC-signed origin domain bound to this transaction's state if (originDomain != null && clientSecret != null && state != null) { diff --git a/src/main/java/com/auth0/RequestProcessor.java b/src/main/java/com/auth0/RequestProcessor.java index 5616114..36a53cc 100644 --- a/src/main/java/com/auth0/RequestProcessor.java +++ b/src/main/java/com/auth0/RequestProcessor.java @@ -240,7 +240,7 @@ Tokens process(HttpServletRequest request, HttpServletResponse response) throws throw new InvalidRequestException(MISSING_ACCESS_TOKEN, "Access Token is missing from the response."); } - return getVerifiedTokens(request, response, frontChannelTokens, responseTypeList, originDomain, originIssuer); + return getVerifiedTokens(request, response, frontChannelTokens, responseTypeList, originDomain, originIssuer, state); } static boolean requiresFormPostResponseMode(List responseType) { @@ -256,13 +256,13 @@ static boolean requiresFormPostResponseMode(List responseType) { * @return a Tokens object that wraps the values obtained from the front-channel and/or the code request response. * @throws IdentityVerificationException */ - private Tokens getVerifiedTokens(HttpServletRequest request, HttpServletResponse response, Tokens frontChannelTokens, List responseTypeList, String originDomain, String originIssuer) + private Tokens getVerifiedTokens(HttpServletRequest request, HttpServletResponse response, Tokens frontChannelTokens, List responseTypeList, String originDomain, String originIssuer, String state) throws IdentityVerificationException { String authorizationCode = request.getParameter(KEY_CODE); Tokens codeExchangeTokens = null; - String nonce = TransientCookieStore.getNonce(request, response); + String nonce = TransientCookieStore.getNonce(request, response, state); try { if (responseTypeList.contains(KEY_ID_TOKEN)) { diff --git a/src/main/java/com/auth0/StorageUtils.java b/src/main/java/com/auth0/StorageUtils.java index 3c41de5..4a3b4d5 100644 --- a/src/main/java/com/auth0/StorageUtils.java +++ b/src/main/java/com/auth0/StorageUtils.java @@ -12,6 +12,27 @@ private StorageUtils() {} static final String NONCE_KEY = "com.auth0.nonce"; static final String ORIGIN_DOMAIN_KEY = "com.auth0.origin_domain"; + /** + * Constructs a transaction-keyed state cookie name. + * Each login transaction gets its own cookie, preventing multi-tab overwrites. + * + * @param state the state value for this transaction + * @return the cookie name in the form "com.auth0.state.{state}" + */ + static String transactionStateKey(String state) { + return STATE_KEY + "." + state; + } + + /** + * Constructs a transaction-keyed nonce cookie name. + * + * @param state the state value for this transaction (used as key, not the nonce itself) + * @return the cookie name in the form "com.auth0.nonce.{state}" + */ + static String transactionNonceKey(String state) { + return NONCE_KEY + "." + state; + } + /** * Generates a new random string using {@link SecureRandom}. * The output can be used as State or Nonce values for API requests. diff --git a/src/main/java/com/auth0/TransientCookieStore.java b/src/main/java/com/auth0/TransientCookieStore.java index 7fb6b34..4e3a40c 100644 --- a/src/main/java/com/auth0/TransientCookieStore.java +++ b/src/main/java/com/auth0/TransientCookieStore.java @@ -10,7 +10,10 @@ import java.nio.charset.StandardCharsets; /** - * Allows storage and retrieval/removal of cookies. + * Allows storage and retrieval/removal of transient cookies used during the OAuth transaction. + * + *

Each login transaction gets its own uniquely-named cookies (keyed by state value), + * preventing multi-tab race conditions where concurrent logins would overwrite each other's state.

*/ class TransientCookieStore { @@ -19,50 +22,91 @@ private TransientCookieStore() {} /** - * Stores a state value as a cookie on the response. + * Stores a state value as a transaction-keyed cookie on the response. + * The cookie name includes the state value itself, ensuring each login flow + * gets its own isolated cookie (e.g., "com.auth0.state.{state_value}"). * * @param response the response object to set the cookie on * @param state the value for the state cookie. If null, no cookie will be set. * @param sameSite the value for the SameSite attribute on the cookie * @param useLegacySameSiteCookie whether to set a fallback cookie or not * @param isSecureCookie whether to always set the Secure cookie attribute or not + * @param cookiePath the path for the cookie */ static void storeState(HttpServletResponse response, String state, SameSite sameSite, boolean useLegacySameSiteCookie, boolean isSecureCookie, String cookiePath) { - store(response, StorageUtils.STATE_KEY, state, sameSite, useLegacySameSiteCookie, isSecureCookie, cookiePath); + if (state == null) { + return; + } + store(response, StorageUtils.transactionStateKey(state), state, sameSite, useLegacySameSiteCookie, isSecureCookie, cookiePath); } /** - * Stores a nonce value as a cookie on the response. + * Stores a nonce value as a transaction-keyed cookie on the response. + * The cookie is keyed by the state value (not the nonce), so it can be + * retrieved during callback using the state parameter from the URL. * * @param response the response object to set the cookie on * @param nonce the value for the nonce cookie. If null, no cookie will be set. + * @param state the state value for this transaction (used as key in cookie name) * @param sameSite the value for the SameSite attribute on the cookie * @param useLegacySameSiteCookie whether to set a fallback cookie or not * @param isSecureCookie whether to always set the Secure cookie attribute or not + * @param cookiePath the path for the cookie */ - static void storeNonce(HttpServletResponse response, String nonce, SameSite sameSite, boolean useLegacySameSiteCookie, boolean isSecureCookie, String cookiePath) { - store(response, StorageUtils.NONCE_KEY, nonce, sameSite, useLegacySameSiteCookie, isSecureCookie, cookiePath); + static void storeNonce(HttpServletResponse response, String nonce, String state, SameSite sameSite, boolean useLegacySameSiteCookie, boolean isSecureCookie, String cookiePath) { + if (nonce == null || state == null) { + return; + } + store(response, StorageUtils.transactionNonceKey(state), nonce, sameSite, useLegacySameSiteCookie, isSecureCookie, cookiePath); } /** - * Gets the value associated with the state cookie and removes it. + * Gets the value associated with the state cookie for this transaction and removes it. + * Uses the state parameter from the callback request to look up the correct transaction cookie. + * Falls back to the legacy fixed-name cookie for backward compatibility during rolling upgrades. * * @param request the request object * @param response the response object * @return the value of the state cookie, if it exists */ static String getState(HttpServletRequest request, HttpServletResponse response) { + String stateParam = request.getParameter("state"); + if (stateParam == null) { + return null; + } + + // Try transaction-keyed cookie first (new behavior) + String value = getOnce(StorageUtils.transactionStateKey(stateParam), request, response); + if (value != null) { + return value; + } + + // Fallback: legacy fixed-name cookie (for in-flight transactions during upgrade from v1) return getOnce(StorageUtils.STATE_KEY, request, response); } /** - * Gets the value associated with the nonce cookie and removes it. + * Gets the value associated with the nonce cookie for this transaction and removes it. + * Uses the state parameter to look up the correct transaction-keyed nonce cookie. + * Falls back to the legacy fixed-name cookie for backward compatibility. * * @param request the request object * @param response the response object + * @param state the state value from the callback (used to find the correct nonce cookie) * @return the value of the nonce cookie, if it exists */ - static String getNonce(HttpServletRequest request, HttpServletResponse response) { + static String getNonce(HttpServletRequest request, HttpServletResponse response, String state) { + if (state == null) { + return null; + } + + // Try transaction-keyed cookie first (new behavior) + String value = getOnce(StorageUtils.transactionNonceKey(state), request, response); + if (value != null) { + return value; + } + + // Fallback: legacy fixed-name cookie (for in-flight transactions during upgrade from v1) return getOnce(StorageUtils.NONCE_KEY, request, response); } diff --git a/src/test/java/com/auth0/AuthorizeUrlTest.java b/src/test/java/com/auth0/AuthorizeUrlTest.java index d8058ec..5ccea63 100644 --- a/src/test/java/com/auth0/AuthorizeUrlTest.java +++ b/src/test/java/com/auth0/AuthorizeUrlTest.java @@ -81,27 +81,29 @@ public void shouldSetAudience() { @Test public void shouldSetNonceSameSiteAndLegacyCookieByDefault() { String url = new AuthorizeUrl(client, response, "https://redirect.to/me", "id_token token") + .withState("stateVal") .withNonce("asdfghjkl") .build(); assertThat(HttpUrl.parse(url).queryParameter("nonce"), is("asdfghjkl")); Collection headers = response.getHeaders("Set-Cookie"); - assertThat(headers.size(), is(2)); - assertThat(headers, hasItem(matchesPattern("com\\.auth0\\.nonce=asdfghjkl; Max-Age=600; Expires=.*?; Secure; HttpOnly; SameSite=None"))); - assertThat(headers, hasItem(matchesPattern("_com\\.auth0\\.nonce=asdfghjkl; Max-Age=600; Expires=.*?; HttpOnly"))); + // state (2: main + legacy) + nonce (2: main + legacy) = 4 + assertThat(headers, hasItem(containsString("com.auth0.nonce.stateVal=asdfghjkl"))); + assertThat(headers, hasItem(containsString("_com.auth0.nonce.stateVal=asdfghjkl"))); } @Test public void shouldSetNonceSameSiteAndNotLegacyCookieWhenConfigured() { String url = new AuthorizeUrl(client, response, "https://redirect.to/me", "id_token token") + .withState("stateVal") .withNonce("asdfghjkl") .withLegacySameSiteCookie(false) .build(); assertThat(HttpUrl.parse(url).queryParameter("nonce"), is("asdfghjkl")); Collection headers = response.getHeaders("Set-Cookie"); - assertThat(headers.size(), is(1)); - assertThat(headers, hasItem(matchesPattern("com\\.auth0\\.nonce=asdfghjkl; Max-Age=600; Expires=.*?; Secure; HttpOnly; SameSite=None"))); + assertThat(headers, hasItem(containsString("com.auth0.nonce.stateVal=asdfghjkl"))); + assertThat(headers, not(hasItem(containsString("_com.auth0.nonce")))); } @Test @@ -113,8 +115,8 @@ public void shouldSetStateSameSiteAndLegacyCookieByDefault() { Collection headers = response.getHeaders("Set-Cookie"); assertThat(headers.size(), is(2)); - assertThat(headers, hasItem(matchesPattern("com\\.auth0\\.state=asdfghjkl; Max-Age=600; Expires=.*?; Secure; HttpOnly; SameSite=None"))); - assertThat(headers, hasItem(matchesPattern("_com\\.auth0\\.state=asdfghjkl; Max-Age=600; Expires=.*?; HttpOnly"))); + assertThat(headers, hasItem(containsString("com.auth0.state.asdfghjkl=asdfghjkl"))); + assertThat(headers, hasItem(containsString("_com.auth0.state.asdfghjkl=asdfghjkl"))); } @Test @@ -127,7 +129,7 @@ public void shouldSetStateSameSiteAndNotLegacyCookieWhenConfigured() { Collection headers = response.getHeaders("Set-Cookie"); assertThat(headers.size(), is(1)); - assertThat(headers, hasItem(matchesPattern("com\\.auth0\\.state=asdfghjkl; Max-Age=600; Expires=.*?; Secure; HttpOnly; SameSite=None"))); + assertThat(headers, hasItem(containsString("com.auth0.state.asdfghjkl=asdfghjkl"))); } @Test @@ -140,7 +142,7 @@ public void shouldSetSecureCookieWhenConfiguredTrue() { Collection headers = response.getHeaders("Set-Cookie"); assertThat(headers.size(), is(1)); - assertThat(headers, hasItem(matchesPattern("com\\.auth0\\.state=asdfghjkl; Max-Age=600; Expires=.*?; Secure; HttpOnly; SameSite=Lax"))); + assertThat(headers, hasItem(allOf(containsString("com.auth0.state.asdfghjkl=asdfghjkl"), containsString("Secure"), containsString("SameSite=Lax")))); } @Test @@ -153,8 +155,8 @@ public void shouldSetSecureCookieWhenConfiguredFalseAndSameSiteNone() { Collection headers = response.getHeaders("Set-Cookie"); assertThat(headers.size(), is(2)); - assertThat(headers, hasItem(matchesPattern("com\\.auth0\\.state=asdfghjkl; Max-Age=600; Expires=.*?; Secure; HttpOnly; SameSite=None"))); - assertThat(headers, hasItem(matchesPattern("_com\\.auth0\\.state=asdfghjkl; Max-Age=600; Expires=.*?; HttpOnly"))); + assertThat(headers, hasItem(allOf(containsString("com.auth0.state.asdfghjkl=asdfghjkl"), containsString("Secure"), containsString("SameSite=None")))); + assertThat(headers, hasItem(containsString("_com.auth0.state.asdfghjkl=asdfghjkl"))); } @Test diff --git a/src/test/java/com/auth0/TransientCookieStoreTest.java b/src/test/java/com/auth0/TransientCookieStoreTest.java index d210cb8..44ff9e4 100644 --- a/src/test/java/com/auth0/TransientCookieStoreTest.java +++ b/src/test/java/com/auth0/TransientCookieStoreTest.java @@ -36,23 +36,38 @@ public void shouldNotSetCookieIfStateIsNull() { @Test public void shouldNotSetCookieIfNonceIsNull() { - TransientCookieStore.storeNonce(response, null, SameSite.NONE, true, false, null); + TransientCookieStore.storeNonce(response, null, "someState", SameSite.NONE, true, false, null); List headers = response.getHeaders("Set-Cookie"); assertThat(headers.size(), is(0)); } @Test - public void shouldHandleSpecialCharsWhenStoringState() throws Exception { - String stateVal = ";state = ,va\\lu;e\""; - TransientCookieStore.storeState(response, stateVal, SameSite.NONE, true, false, null); + public void shouldNotSetNonceCookieIfStateIsNull() { + TransientCookieStore.storeNonce(response, "nonceValue", null, SameSite.NONE, true, false, null); List headers = response.getHeaders("Set-Cookie"); - assertThat(headers.size(), is(2)); + assertThat(headers.size(), is(0)); + } + + @Test + public void shouldSetStateCookieWithTransactionKey() { + TransientCookieStore.storeState(response, "myState123", SameSite.LAX, false, false, null); - String expectedEncodedState = URLEncoder.encode(stateVal, "UTF-8").replaceAll("\\+", "\\\\+"); - assertThat(headers, hasItem(matchesPattern(String.format("com\\.auth0\\.state=%s; Max-Age=600; Expires=.*?; Secure; HttpOnly; SameSite=None", expectedEncodedState)))); - assertThat(headers, hasItem(matchesPattern(String.format("_com\\.auth0\\.state=%s; Max-Age=600; Expires=.*?; HttpOnly", expectedEncodedState)))); + List headers = response.getHeaders("Set-Cookie"); + assertThat(headers.size(), is(1)); + // Cookie name should be "com.auth0.state.myState123" + assertThat(headers.get(0), containsString("com.auth0.state.myState123=myState123")); + } + + @Test + public void shouldSetNonceCookieWithTransactionKey() { + TransientCookieStore.storeNonce(response, "nonceVal", "myState123", SameSite.LAX, false, false, null); + + List headers = response.getHeaders("Set-Cookie"); + assertThat(headers.size(), is(1)); + // Cookie name should be "com.auth0.nonce.myState123" + assertThat(headers.get(0), containsString("com.auth0.nonce.myState123=nonceVal")); } @Test @@ -62,8 +77,10 @@ public void shouldSetStateSameSiteCookieAndFallbackCookie() { List headers = response.getHeaders("Set-Cookie"); assertThat(headers.size(), is(2)); - assertThat(headers, hasItem(matchesPattern("com\\.auth0\\.state=123456; Max-Age=600; Expires=.*?; Secure; HttpOnly; SameSite=None"))); - assertThat(headers, hasItem(matchesPattern("_com\\.auth0\\.state=123456; Max-Age=600; Expires=.*?; HttpOnly"))); + assertThat(headers.get(0), containsString("com.auth0.state.123456=123456")); + assertThat(headers.get(0), containsString("SameSite=None")); + assertThat(headers.get(0), containsString("Secure")); + assertThat(headers.get(1), containsString("_com.auth0.state.123456=123456")); } @Test @@ -73,7 +90,8 @@ public void shouldSetStateSameSiteCookieAndNoFallbackCookie() { List headers = response.getHeaders("Set-Cookie"); assertThat(headers.size(), is(1)); - assertThat(headers, hasItem(matchesPattern("com\\.auth0\\.state=123456; Max-Age=600; Expires=.*?; Secure; HttpOnly; SameSite=None"))); + assertThat(headers.get(0), containsString("com.auth0.state.123456=123456")); + assertThat(headers.get(0), containsString("SameSite=None")); } @Test @@ -83,7 +101,8 @@ public void shouldSetSecureCookieWhenSameSiteLaxAndConfigured() { List headers = response.getHeaders("Set-Cookie"); assertThat(headers.size(), is(1)); - assertThat(headers, hasItem(matchesPattern("com\\.auth0\\.state=123456; Max-Age=600; Expires=.*?; Secure; HttpOnly; SameSite=Lax"))); + assertThat(headers.get(0), containsString("Secure")); + assertThat(headers.get(0), containsString("SameSite=Lax")); } @Test @@ -93,66 +112,89 @@ public void shouldSetSecureFallbackCookieWhenSameSiteNoneAndConfigured() { List headers = response.getHeaders("Set-Cookie"); assertThat(headers.size(), is(2)); - assertThat(headers, hasItem(matchesPattern("com\\.auth0\\.state=123456; Max-Age=600; Expires=.*?; Secure; HttpOnly; SameSite=None"))); - assertThat(headers, hasItem(matchesPattern("_com\\.auth0\\.state=123456; Max-Age=600; Expires=.*?; Secure; HttpOnly"))); + assertThat(headers.get(0), containsString("SameSite=None")); + assertThat(headers.get(0), containsString("Secure")); + assertThat(headers.get(1), containsString("Secure")); } @Test - public void shouldNotSetSecureCookieWhenSameSiteLaxAndConfigured() { + public void shouldNotSetSecureCookieWhenSameSiteLaxAndNotConfigured() { TransientCookieStore.storeState(response, "123456", SameSite.LAX, true, false, null); List headers = response.getHeaders("Set-Cookie"); assertThat(headers.size(), is(1)); - assertThat(headers, hasItem(matchesPattern("com\\.auth0\\.state=123456; Max-Age=600; Expires=.*?; HttpOnly; SameSite=Lax"))); + assertThat(headers.get(0), not(containsString("Secure"))); + assertThat(headers.get(0), containsString("SameSite=Lax")); } @Test public void shouldSetNonceSameSiteCookieAndFallbackCookie() { - TransientCookieStore.storeNonce(response, "123456", SameSite.NONE, true, false, null); + TransientCookieStore.storeNonce(response, "nonceVal", "stateVal", SameSite.NONE, true, false, null); List headers = response.getHeaders("Set-Cookie"); assertThat(headers.size(), is(2)); - assertThat(headers, hasItem(matchesPattern("com\\.auth0\\.nonce=123456; Max-Age=600; Expires=.*?; Secure; HttpOnly; SameSite=None"))); - assertThat(headers, hasItem(matchesPattern("_com\\.auth0\\.nonce=123456; Max-Age=600; Expires=.*?; HttpOnly"))); + assertThat(headers.get(0), containsString("com.auth0.nonce.stateVal=nonceVal")); + assertThat(headers.get(0), containsString("SameSite=None")); + assertThat(headers.get(1), containsString("_com.auth0.nonce.stateVal=nonceVal")); } @Test public void shouldSetNonceSameSiteCookieAndNoFallbackCookie() { - TransientCookieStore.storeNonce(response, "123456", SameSite.NONE, false, false, null); + TransientCookieStore.storeNonce(response, "nonceVal", "stateVal", SameSite.NONE, false, false, null); List headers = response.getHeaders("Set-Cookie"); assertThat(headers.size(), is(1)); - assertThat(headers, hasItem(matchesPattern("com\\.auth0\\.nonce=123456; Max-Age=600; Expires=.*?; Secure; HttpOnly; SameSite=None"))); + assertThat(headers.get(0), containsString("com.auth0.nonce.stateVal=nonceVal")); } - @Test - public void shouldRemoveStateSameSiteCookieAndFallbackCookie() { - Cookie cookie1 = new Cookie("com.auth0.state", "123456"); - Cookie cookie2 = new Cookie("_com.auth0.state", "123456"); + // --- State retrieval tests (transaction-keyed) --- - request.setCookies(cookie1, cookie2); + @Test + public void shouldRetrieveTransactionKeyedStateCookie() { + Cookie cookie = new Cookie("com.auth0.state.myState", "myState"); + request.setCookies(cookie); + request.setParameter("state", "myState"); String state = TransientCookieStore.getState(request, response); - assertThat(state, is("123456")); + assertThat(state, is("myState")); + // Should delete the cookie Cookie[] cookies = response.getCookies(); assertThat(cookies, is(notNullValue())); + assertThat(cookies[0].getMaxAge(), is(0)); + } - List cookieList = Arrays.asList(cookies); - assertThat(cookieList.size(), is(2)); + @Test + public void shouldFallbackToLegacyFixedStateCookie() { + // Legacy cookie (from v1 SDK or in-flight transaction during upgrade) + Cookie cookie = new Cookie("com.auth0.state", "legacyState"); + request.setCookies(cookie); + request.setParameter("state", "legacyState"); - assertThat(Arrays.asList(cookies), everyItem(HasPropertyWithValue.hasProperty("value", is("")))); - assertThat(Arrays.asList(cookies), everyItem(HasPropertyWithValue.hasProperty("maxAge", is(0)))); + String state = TransientCookieStore.getState(request, response); + assertThat(state, is("legacyState")); } @Test - public void shouldRemoveStateSameSiteCookie() { - Cookie cookie1 = new Cookie("com.auth0.state", "123456"); + public void shouldPreferTransactionKeyedOverLegacy() { + Cookie txCookie = new Cookie("com.auth0.state.txState", "txState"); + Cookie legacyCookie = new Cookie("com.auth0.state", "oldState"); + request.setCookies(txCookie, legacyCookie); + request.setParameter("state", "txState"); - request.setCookies(cookie1); + String state = TransientCookieStore.getState(request, response); + assertThat(state, is("txState")); + } + + @Test + public void shouldRemoveStateSameSiteCookieAndFallbackCookie() { + Cookie cookie1 = new Cookie("com.auth0.state.123456", "123456"); + Cookie cookie2 = new Cookie("_com.auth0.state.123456", "123456"); + request.setCookies(cookie1, cookie2); + request.setParameter("state", "123456"); String state = TransientCookieStore.getState(request, response); assertThat(state, is("123456")); @@ -161,38 +203,66 @@ public void shouldRemoveStateSameSiteCookie() { assertThat(cookies, is(notNullValue())); List cookieList = Arrays.asList(cookies); - assertThat(cookieList.size(), is(1)); + assertThat(cookieList.size(), is(2)); + assertThat(Arrays.asList(cookies), everyItem(HasPropertyWithValue.hasProperty("value", is("")))); assertThat(Arrays.asList(cookies), everyItem(HasPropertyWithValue.hasProperty("maxAge", is(0)))); } @Test - public void shouldRemoveStateFallbackCookie() { - Cookie cookie1 = new Cookie("_com.auth0.state", "123456"); + public void shouldReturnNullStateWhenNoCookies() { + request.setParameter("state", "someState"); + String state = TransientCookieStore.getState(request, response); + assertThat(state, is(nullValue())); + } + + @Test + public void shouldReturnNullStateWhenNoStateParam() { + // No state parameter in request → null + String state = TransientCookieStore.getState(request, response); + assertThat(state, is(nullValue())); + } + @Test + public void shouldReturnEmptyWhenNoStateCookie() { + Cookie cookie1 = new Cookie("someCookie", "123456"); request.setCookies(cookie1); + request.setParameter("state", "someState"); String state = TransientCookieStore.getState(request, response); - assertThat(state, is("123456")); + assertThat(state, is(nullValue())); + } + + @Test + public void shouldRetrieveTransactionKeyedNonceCookie() { + Cookie cookie = new Cookie("com.auth0.nonce.myState", "nonceValue"); + request.setCookies(cookie); + + String nonce = TransientCookieStore.getNonce(request, response, "myState"); + assertThat(nonce, is("nonceValue")); Cookie[] cookies = response.getCookies(); assertThat(cookies, is(notNullValue())); + assertThat(cookies[0].getMaxAge(), is(0)); + } - List cookieList = Arrays.asList(cookies); - assertThat(cookieList.size(), is(1)); - assertThat(Arrays.asList(cookies), everyItem(HasPropertyWithValue.hasProperty("value", is("")))); - assertThat(Arrays.asList(cookies), everyItem(HasPropertyWithValue.hasProperty("maxAge", is(0)))); + @Test + public void shouldFallbackToLegacyFixedNonceCookie() { + Cookie cookie = new Cookie("com.auth0.nonce", "legacyNonce"); + request.setCookies(cookie); + + String nonce = TransientCookieStore.getNonce(request, response, "someState"); + assertThat(nonce, is("legacyNonce")); } @Test public void shouldRemoveNonceSameSiteCookieAndFallbackCookie() { - Cookie cookie1 = new Cookie("com.auth0.nonce", "123456"); - Cookie cookie2 = new Cookie("_com.auth0.nonce", "123456"); - + Cookie cookie1 = new Cookie("com.auth0.nonce.stateVal", "nonceVal"); + Cookie cookie2 = new Cookie("_com.auth0.nonce.stateVal", "nonceVal"); request.setCookies(cookie1, cookie2); - String state = TransientCookieStore.getNonce(request, response); - assertThat(state, is("123456")); + String nonce = TransientCookieStore.getNonce(request, response, "stateVal"); + assertThat(nonce, is("nonceVal")); Cookie[] cookies = response.getCookies(); assertThat(cookies, is(notNullValue())); @@ -204,72 +274,169 @@ public void shouldRemoveNonceSameSiteCookieAndFallbackCookie() { } @Test - public void shouldRemoveNonceSameSiteCookie() { - Cookie cookie1 = new Cookie("com.auth0.nonce", "123456"); - - request.setCookies(cookie1); - - String state = TransientCookieStore.getNonce(request, response); - assertThat(state, is("123456")); - - Cookie[] cookies = response.getCookies(); - assertThat(cookies, is(notNullValue())); - - List cookieList = Arrays.asList(cookies); - assertThat(cookieList.size(), is(1)); - assertThat(Arrays.asList(cookies), everyItem(HasPropertyWithValue.hasProperty("value", is("")))); - assertThat(Arrays.asList(cookies), everyItem(HasPropertyWithValue.hasProperty("maxAge", is(0)))); + public void shouldReturnNullNonceWhenNoCookies() { + String nonce = TransientCookieStore.getNonce(request, response, "someState"); + assertThat(nonce, is(nullValue())); } @Test - public void shouldRemoveNonceFallbackCookie() { - Cookie cookie1 = new Cookie("_com.auth0.nonce", "123456"); + public void shouldReturnNullNonceWhenStateIsNull() { + String nonce = TransientCookieStore.getNonce(request, response, null); + assertThat(nonce, is(nullValue())); + } + @Test + public void shouldReturnNullWhenNoNonceCookie() { + Cookie cookie1 = new Cookie("someCookie", "123456"); request.setCookies(cookie1); - String state = TransientCookieStore.getNonce(request, response); - assertThat(state, is("123456")); + String nonce = TransientCookieStore.getNonce(request, response, "someState"); + assertThat(nonce, is(nullValue())); + } - Cookie[] cookies = response.getCookies(); - assertThat(cookies, is(notNullValue())); + @Test + public void shouldIsolateMultipleTransactions() { + // Simulate two tabs storing state cookies + TransientCookieStore.storeState(response, "stateA", SameSite.LAX, false, false, null); + TransientCookieStore.storeState(response, "stateB", SameSite.LAX, false, false, null); - List cookieList = Arrays.asList(cookies); - assertThat(cookieList.size(), is(1)); - assertThat(Arrays.asList(cookies), everyItem(HasPropertyWithValue.hasProperty("value", is("")))); - assertThat(Arrays.asList(cookies), everyItem(HasPropertyWithValue.hasProperty("maxAge", is(0)))); + List headers = response.getHeaders("Set-Cookie"); + assertThat(headers.size(), is(2)); + assertThat(headers.get(0), containsString("com.auth0.state.stateA=stateA")); + assertThat(headers.get(1), containsString("com.auth0.state.stateB=stateB")); } @Test - public void shouldReturnEmptyStateWhenNoCookies() { - String state = TransientCookieStore.getState(request, response); - assertThat(state, is(nullValue())); + public void shouldRetrieveCorrectStateForEachTransaction() { + // Both transaction cookies exist + Cookie cookieA = new Cookie("com.auth0.state.stateA", "stateA"); + Cookie cookieB = new Cookie("com.auth0.state.stateB", "stateB"); + request.setCookies(cookieA, cookieB); + + // Tab A callback + request.setParameter("state", "stateA"); + String stateA = TransientCookieStore.getState(request, response); + assertThat(stateA, is("stateA")); + + // Tab B callback (new request) + MockHttpServletRequest requestB = new MockHttpServletRequest(); + MockHttpServletResponse responseB = new MockHttpServletResponse(); + requestB.setCookies(cookieA, cookieB); + requestB.setParameter("state", "stateB"); + String stateB = TransientCookieStore.getState(requestB, responseB); + assertThat(stateB, is("stateB")); } @Test - public void shouldReturnEmptyNonceWhenNoCookies() { - String nonce = TransientCookieStore.getNonce(request, response); - assertThat(nonce, is(nullValue())); + public void shouldNotDeleteOtherTransactionCookies() { + Cookie cookieA = new Cookie("com.auth0.state.stateA", "stateA"); + Cookie cookieB = new Cookie("com.auth0.state.stateB", "stateB"); + request.setCookies(cookieA, cookieB); + request.setParameter("state", "stateA"); + + TransientCookieStore.getState(request, response); + + // Only stateA's cookie should be deleted + Cookie[] deletedCookies = response.getCookies(); + assertThat(deletedCookies.length, is(1)); + assertThat(deletedCookies[0].getName(), is("com.auth0.state.stateA")); + assertThat(deletedCookies[0].getMaxAge(), is(0)); } - @Test - public void shouldReturnEmptyWhenNoStateCookie() { - Cookie cookie1 = new Cookie("someCookie", "123456"); - request.setCookies(cookie1); + // --- Nonce multi-tab isolation tests --- - String state = TransientCookieStore.getState(request, response); - assertThat(state, is(nullValue())); + @Test + public void shouldIsolateMultipleNonceTransactionsOnRetrieval() { + // Both nonce cookies exist (two concurrent tabs) + Cookie nonceA = new Cookie("com.auth0.nonce.stateA", "nonceA"); + Cookie nonceB = new Cookie("com.auth0.nonce.stateB", "nonceB"); + request.setCookies(nonceA, nonceB); + + // Retrieve nonce for Tab A — should get nonceA without touching nonceB + String resultA = TransientCookieStore.getNonce(request, response, "stateA"); + assertThat(resultA, is("nonceA")); + + // Only stateA's nonce cookie should be deleted + Cookie[] deletedCookies = response.getCookies(); + assertThat(deletedCookies.length, is(1)); + assertThat(deletedCookies[0].getName(), is("com.auth0.nonce.stateA")); + assertThat(deletedCookies[0].getMaxAge(), is(0)); + + // Tab B can still retrieve its nonce independently (new request) + MockHttpServletRequest requestB = new MockHttpServletRequest(); + MockHttpServletResponse responseB = new MockHttpServletResponse(); + requestB.setCookies(nonceA, nonceB); + + String resultB = TransientCookieStore.getNonce(requestB, responseB, "stateB"); + assertThat(resultB, is("nonceB")); } @Test - public void shouldReturnEmptyWhenNoNonceCookie() { - Cookie cookie1 = new Cookie("someCookie", "123456"); - request.setCookies(cookie1); + public void shouldPreferTransactionKeyedNonceOverLegacyAndNotDeleteLegacy() { + Cookie txCookie = new Cookie("com.auth0.nonce.stateA", "txNonce"); + Cookie legacyCookie = new Cookie("com.auth0.nonce", "legacyNonce"); + request.setCookies(txCookie, legacyCookie); + + String nonce = TransientCookieStore.getNonce(request, response, "stateA"); + assertThat(nonce, is("txNonce")); + + // Transaction-keyed cookie was consumed; legacy cookie should NOT be deleted + // (it may belong to another in-flight flow upgrading from v1) + Cookie[] deletedCookies = response.getCookies(); + assertThat(deletedCookies.length, is(1)); + assertThat(deletedCookies[0].getName(), is("com.auth0.nonce.stateA")); + } - String nonce = TransientCookieStore.getNonce(request, response); - assertThat(nonce, is(nullValue())); - assertThat(nonce, is(nullValue())); + @Test + public void shouldRetrieveCorrectStateAndNonceForEachTabEndToEnd() { + // Simulate two concurrent login flows storing state + nonce + MockHttpServletResponse storeResponse = new MockHttpServletResponse(); + TransientCookieStore.storeState(storeResponse, "stateA", SameSite.LAX, false, false, null); + TransientCookieStore.storeNonce(storeResponse, "nonceA", "stateA", SameSite.LAX, false, false, null); + TransientCookieStore.storeState(storeResponse, "stateB", SameSite.LAX, false, false, null); + TransientCookieStore.storeNonce(storeResponse, "nonceB", "stateB", SameSite.LAX, false, false, null); + + // Tab A callback + MockHttpServletRequest requestA = new MockHttpServletRequest(); + MockHttpServletResponse responseA = new MockHttpServletResponse(); + requestA.setCookies( + new Cookie("com.auth0.state.stateA", "stateA"), + new Cookie("com.auth0.nonce.stateA", "nonceA"), + new Cookie("com.auth0.state.stateB", "stateB"), + new Cookie("com.auth0.nonce.stateB", "nonceB") + ); + requestA.setParameter("state", "stateA"); + + String stateA = TransientCookieStore.getState(requestA, responseA); + String nonceA = TransientCookieStore.getNonce(requestA, responseA, "stateA"); + assertThat(stateA, is("stateA")); + assertThat(nonceA, is("nonceA")); + + // Tab B callback + MockHttpServletRequest requestB = new MockHttpServletRequest(); + MockHttpServletResponse responseB = new MockHttpServletResponse(); + requestB.setCookies( + new Cookie("com.auth0.state.stateA", "stateA"), + new Cookie("com.auth0.nonce.stateA", "nonceA"), + new Cookie("com.auth0.state.stateB", "stateB"), + new Cookie("com.auth0.nonce.stateB", "nonceB") + ); + requestB.setParameter("state", "stateB"); + + String stateB = TransientCookieStore.getState(requestB, responseB); + String nonceB = TransientCookieStore.getNonce(requestB, responseB, "stateB"); + assertThat(stateB, is("stateB")); + assertThat(nonceB, is("nonceB")); + + // Verify Tab A's retrieval didn't affect Tab B's cookies + Cookie[] deletedByA = responseA.getCookies(); + for (Cookie c : deletedByA) { + assertThat(c.getName(), not(containsString("stateB"))); + } } + // --- Origin domain tests --- + private static final String TEST_SECRET = "testClientSecret123"; private static final String TEST_DOMAIN = "tenant-a.auth0.com"; private static final String TEST_STATE = "abc123state"; @@ -305,7 +472,6 @@ public void shouldRetrieveAndVerifySignedOriginDomain() { request.setCookies(cookie); String domain = TransientCookieStore.getSignedOriginDomain(request, response, TEST_STATE, TEST_SECRET); - assertThat(domain, is(TEST_DOMAIN)); } @@ -316,14 +482,12 @@ public void shouldReturnNullForTamperedOriginDomain() { request.setCookies(cookie); String domain = TransientCookieStore.getSignedOriginDomain(request, response, TEST_STATE, TEST_SECRET); - assertThat(domain, is(nullValue())); } @Test public void shouldReturnNullForMissingOriginDomainCookie() { String domain = TransientCookieStore.getSignedOriginDomain(request, response, TEST_STATE, TEST_SECRET); - assertThat(domain, is(nullValue())); } @@ -334,7 +498,6 @@ public void shouldReturnNullForWrongSecret() { request.setCookies(cookie); String domain = TransientCookieStore.getSignedOriginDomain(request, response, TEST_STATE, "wrong-secret"); - assertThat(domain, is(nullValue())); } @@ -345,7 +508,6 @@ public void shouldReturnNullForWrongState() { request.setCookies(cookie); String domain = TransientCookieStore.getSignedOriginDomain(request, response, "different-state", TEST_SECRET); - assertThat(domain, is(nullValue())); } @@ -362,7 +524,7 @@ public void shouldDeleteOriginDomainCookieAfterReading() { assertThat(responseCookies, is(notNullValue())); boolean foundDeleted = false; for (Cookie c : responseCookies) { - if ("com.auth0.origin_domain".equals(c.getName())) { + if (c.getName().equals("com.auth0.origin_domain")) { assertThat(c.getMaxAge(), is(0)); assertThat(c.getValue(), is("")); foundDeleted = true;