From edccafca8444178f43f1609a96fb54712ade6a58 Mon Sep 17 00:00:00 2001
From: Johnny Lim <izeye@naver.com>
Date: Sat, 18 Nov 2017 00:06:06 +0900
Subject: [PATCH] Create OAuth2AuthorizationResponse lazily

This commit creates `OAuth2AuthorizationResponse` as lazily as possible to prevent the creation when `authorizationRequest` is `null`.

Fixes gh-4848
---
 .../web/OAuth2LoginAuthenticationFilter.java  |  3 ++-
 .../OAuth2LoginAuthenticationFilterTests.java | 24 ++++++++++++++++++-
 2 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/OAuth2LoginAuthenticationFilter.java b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/OAuth2LoginAuthenticationFilter.java
index 769efce4d5..0230a6b593 100644
--- a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/OAuth2LoginAuthenticationFilter.java
+++ b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/OAuth2LoginAuthenticationFilter.java
@@ -108,7 +108,6 @@ public class OAuth2LoginAuthenticationFilter extends AbstractAuthenticationProce
 			OAuth2Error oauth2Error = new OAuth2Error(OAuth2ErrorCodes.INVALID_REQUEST);
 			throw new OAuth2AuthenticationException(oauth2Error, oauth2Error.toString());
 		}
-		OAuth2AuthorizationResponse authorizationResponse = this.convert(request);
 
 		OAuth2AuthorizationRequest authorizationRequest = this.authorizationRequestRepository.loadAuthorizationRequest(request);
 		if (authorizationRequest == null) {
@@ -120,6 +119,8 @@ public class OAuth2LoginAuthenticationFilter extends AbstractAuthenticationProce
 		String registrationId = (String) authorizationRequest.getAdditionalParameters().get(OAuth2ParameterNames.REGISTRATION_ID);
 		ClientRegistration clientRegistration = this.clientRegistrationRepository.findByRegistrationId(registrationId);
 
+		OAuth2AuthorizationResponse authorizationResponse = this.convert(request);
+
 		OAuth2LoginAuthenticationToken authenticationRequest = new OAuth2LoginAuthenticationToken(
 				clientRegistration, new OAuth2AuthorizationExchange(authorizationRequest, authorizationResponse));
 		authenticationRequest.setDetails(this.authenticationDetailsSource.buildDetails(request));
diff --git a/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/web/OAuth2LoginAuthenticationFilterTests.java b/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/web/OAuth2LoginAuthenticationFilterTests.java
index 3f9a2fe998..d382d1ff5c 100644
--- a/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/web/OAuth2LoginAuthenticationFilterTests.java
+++ b/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/web/OAuth2LoginAuthenticationFilterTests.java
@@ -19,6 +19,7 @@ import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.mockito.ArgumentCaptor;
+import org.powermock.api.mockito.PowerMockito;
 import org.powermock.core.classloader.annotations.PowerMockIgnore;
 import org.powermock.core.classloader.annotations.PrepareForTest;
 import org.powermock.modules.junit4.PowerMockRunner;
@@ -53,8 +54,10 @@ import java.util.HashMap;
 import java.util.Map;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.Assert.fail;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.*;
+import static org.powermock.api.mockito.PowerMockito.verifyPrivate;
 
 /**
  * Tests for {@link OAuth2LoginAuthenticationFilter}.
@@ -62,7 +65,7 @@ import static org.mockito.Mockito.*;
  * @author Joe Grandja
  */
 @PowerMockIgnore("javax.security.*")
-@PrepareForTest({OAuth2AuthorizationRequest.class, OAuth2AuthorizationExchange.class})
+@PrepareForTest({OAuth2AuthorizationRequest.class, OAuth2AuthorizationExchange.class, OAuth2LoginAuthenticationFilter.class})
 @RunWith(PowerMockRunner.class)
 public class OAuth2LoginAuthenticationFilterTests {
 	private ClientRegistration registration1;
@@ -263,6 +266,25 @@ public class OAuth2LoginAuthenticationFilterTests {
 		verify(this.filter).attemptAuthentication(any(HttpServletRequest.class), any(HttpServletResponse.class));
 	}
 
+	@Test
+	public void attemptAuthenticationWhenAuthorizationRequestIsNullThenAuthorizationResponseNotCreated() throws Exception {
+		OAuth2LoginAuthenticationFilter filter = PowerMockito.spy(new OAuth2LoginAuthenticationFilter(
+			this.clientRegistrationRepository, this.authorizedClientService));
+
+		MockHttpServletRequest request = new MockHttpServletRequest();
+		request.addParameter(OAuth2ParameterNames.CODE, "code");
+		request.addParameter(OAuth2ParameterNames.STATE, "state");
+
+		MockHttpServletResponse response = new MockHttpServletResponse();
+
+		try {
+			filter.attemptAuthentication(request, response);
+			fail();
+		} catch (OAuth2AuthenticationException ex) {
+			verifyPrivate(filter, never()).invoke("convert", any(HttpServletRequest.class));
+		}
+	}
+
 	private void setUpAuthorizationRequest(HttpServletRequest request, HttpServletResponse response,
 											ClientRegistration registration) {
 		OAuth2AuthorizationRequest authorizationRequest = mock(OAuth2AuthorizationRequest.class);