[SDK-4670] Improved state handling errors (#140)

src/main/java/com/auth0/RequestProcessor.java

@@ -292,8 +292,17 @@ private void assertNoError(HttpServletRequest request) throws InvalidRequestExce
      * @throws InvalidRequestException if the request contains a different state from the expected one
      */
     private void assertValidState(HttpServletRequest request, HttpServletResponse response) throws InvalidRequestException {
+        // TODO in v2:
+        //  - only store state/nonce in cookies, remove session storage
+        //  - create specific exception classes for various state validation failures (missing from auth response, missing
+        //    state cookie, mismatch)
+
         String stateFromRequest = request.getParameter(KEY_STATE);
 
+        if (stateFromRequest == null) {
+            throw new InvalidRequestException(INVALID_STATE_ERROR, "The received state doesn't match the expected one. No state parameter was found on the authorization response.");
+        }
+
         // If response is null, check the Session.
         // This can happen when the deprecated handle method that only takes the request parameter is called
         if (response == null) {
@@ -306,6 +315,9 @@ private void assertValidState(HttpServletRequest request, HttpServletResponse re
         // Just in case state was stored in Session by building auth URL with deprecated method, but then called the
         // supported handle method with the request and response
         if (cookieState == null) {
+            if (SessionUtils.get(request, StorageUtils.STATE_KEY) == null) {
+                throw new InvalidRequestException(INVALID_STATE_ERROR, "The received state doesn't match the expected one. No state cookie or state session attribute found. Check that you are using non-deprecated methods and that cookies are not being removed on the server.");
+            }
             checkSessionState(request, stateFromRequest);
             return;
         }

src/test/java/com/auth0/RandomStorageTest.java

@@ -24,6 +24,13 @@ public void shouldAcceptBothNullStates() {
         assertThat(validState, is(true));
     }
 
+    @Test
+    public void shouldFailIfSessionStateIsNullButCurrentStateNotNull() {
+        MockHttpServletRequest req = new MockHttpServletRequest();
+        boolean validState = RandomStorage.checkSessionState(req, "12345");
+        assertThat(validState, is(false));
+    }
+
     @Test
     public void shouldCheckAndRemoveInvalidState() {
         MockHttpServletRequest req = new MockHttpServletRequest();

src/test/java/com/auth0/RequestProcessorTest.java

@@ -107,7 +107,7 @@ public void shouldThrowOnProcessIfRequestHasMissingStateParameter() throws Excep
                 .build();
         InvalidRequestException e = assertThrows(InvalidRequestException.class, () -> handler.process(request, response));
         assertThat(e, InvalidRequestExceptionMatcher.hasCode("a0.invalid_state"));
-        assertEquals("The received state doesn't match the expected one.", e.getMessage());
+        assertEquals("The received state doesn't match the expected one. No state parameter was found on the authorization response.", e.getMessage());
     }
 
     @Test
@@ -120,7 +120,7 @@ public void shouldThrowOnProcessIfRequestHasMissingStateCookie() throws Exceptio
                 .build();
         InvalidRequestException e = assertThrows(InvalidRequestException.class, () -> handler.process(request, response));
         assertThat(e, InvalidRequestExceptionMatcher.hasCode("a0.invalid_state"));
-        assertEquals("The received state doesn't match the expected one.", e.getMessage());
+        assertEquals("The received state doesn't match the expected one. No state cookie or state session attribute found. Check that you are using non-deprecated methods and that cookies are not being removed on the server.", e.getMessage());
     }
 
     @Test
@@ -288,6 +288,80 @@ public void shouldReturnTokensOnProcessIfIdTokenCodeRequestPassesIdTokenVerifica
         assertThat(tokens.getExpiresIn(), is(8400L));
     }
 
+    @Test
+    public void shouldReturnTokensOnProcessIfIdTokenCodeRequestPassesIdTokenVerificationWhenUsingSessionStorage() throws Exception {
+        doNothing().when(tokenVerifier).verify(eq("frontIdToken"), eq(verifyOptions));
+
+        Map<String, Object> params = new HashMap<>();
+        params.put("code", "abc123");
+        params.put("state", "1234");
+        params.put("id_token", "frontIdToken");
+        params.put("expires_in", "8400");
+        params.put("token_type", "frontTokenType");
+        MockHttpServletRequest request = getRequest(params);
+        request.getSession().setAttribute("com.auth0.state", "1234");
+
+        TokenRequest codeExchangeRequest = mock(TokenRequest.class);
+        TokenHolder tokenHolder = mock(TokenHolder.class);
+        when(tokenHolder.getIdToken()).thenReturn("backIdToken");
+        when(tokenHolder.getExpiresIn()).thenReturn(4800L);
+        when(tokenHolder.getTokenType()).thenReturn("backTokenType");
+        when(codeExchangeRequest.execute()).thenReturn(tokenHolder);
+        when(client.exchangeCode("abc123", "https://me.auth0.com:80/callback")).thenReturn(codeExchangeRequest);
+
+        RequestProcessor handler = new RequestProcessor.Builder(client, "id_token code", verifyOptions)
+                .withIdTokenVerifier(tokenVerifier)
+                .build();
+        Tokens tokens = handler.process(request, response);
+
+        //Should not verify the ID Token twice
+        verify(tokenVerifier).verify("frontIdToken", verifyOptions);
+        verify(tokenVerifier, never()).verify("backIdToken", verifyOptions);
+        verifyNoMoreInteractions(tokenVerifier);
+
+        assertThat(tokens, is(notNullValue()));
+        assertThat(tokens.getIdToken(), is("frontIdToken"));
+        assertThat(tokens.getType(), is("frontTokenType"));
+        assertThat(tokens.getExpiresIn(), is(8400L));
+    }
+
+    @Test
+    public void shouldReturnTokensOnProcessIfIdTokenCodeRequestPassesIdTokenVerificationWhenUsingSessionStorageWithNullSession() throws Exception {
+        doNothing().when(tokenVerifier).verify(eq("frontIdToken"), eq(verifyOptions));
+
+        Map<String, Object> params = new HashMap<>();
+        params.put("code", "abc123");
+        params.put("state", "1234");
+        params.put("id_token", "frontIdToken");
+        params.put("expires_in", "8400");
+        params.put("token_type", "frontTokenType");
+        MockHttpServletRequest request = getRequest(params);
+        request.getSession().setAttribute("com.auth0.state", "1234");
+
+        TokenRequest codeExchangeRequest = mock(TokenRequest.class);
+        TokenHolder tokenHolder = mock(TokenHolder.class);
+        when(tokenHolder.getIdToken()).thenReturn("backIdToken");
+        when(tokenHolder.getExpiresIn()).thenReturn(4800L);
+        when(tokenHolder.getTokenType()).thenReturn("backTokenType");
+        when(codeExchangeRequest.execute()).thenReturn(tokenHolder);
+        when(client.exchangeCode("abc123", "https://me.auth0.com:80/callback")).thenReturn(codeExchangeRequest);
+
+        RequestProcessor handler = new RequestProcessor.Builder(client, "id_token code", verifyOptions)
+                .withIdTokenVerifier(tokenVerifier)
+                .build();
+        Tokens tokens = handler.process(request, null);
+
+        //Should not verify the ID Token twice
+        verify(tokenVerifier).verify("frontIdToken", verifyOptions);
+        verify(tokenVerifier, never()).verify("backIdToken", verifyOptions);
+        verifyNoMoreInteractions(tokenVerifier);
+
+        assertThat(tokens, is(notNullValue()));
+        assertThat(tokens.getIdToken(), is("frontIdToken"));
+        assertThat(tokens.getType(), is("frontTokenType"));
+        assertThat(tokens.getExpiresIn(), is(8400L));
+    }
+
     @Test
     public void shouldReturnTokensOnProcessIfTokenIdTokenCodeRequestPassesIdTokenVerification() throws Exception {
         doNothing().when(tokenVerifier).verify(eq("frontIdToken"), eq(verifyOptions));