SONAR-14424 fix SSF-140

server/sonar-webserver-auth/src/main/java/org/sonar/server/authentication/OAuth2AuthenticationParametersImpl.java

@@ -89,7 +89,8 @@ public class OAuth2AuthenticationParametersImpl implements OAuth2AuthenticationP
 
   @Override
   public Optional<String> getReturnTo(HttpServletRequest request) {
-    return getParameter(request, RETURN_TO_PARAMETER);
+    return getParameter(request, RETURN_TO_PARAMETER)
+      .flatMap(OAuth2AuthenticationParametersImpl::sanitizeRedirectUrl);
   }
 
   @Override

server/sonar-webserver-auth/src/test/java/org/sonar/server/authentication/OAuth2AuthenticationParametersImplTest.java

@@ -41,12 +41,11 @@ import static org.mockito.Mockito.when;
 public class OAuth2AuthenticationParametersImplTest {
 
   private static final String AUTHENTICATION_COOKIE_NAME = "AUTH-PARAMS";
-  private ArgumentCaptor<Cookie> cookieArgumentCaptor = ArgumentCaptor.forClass(Cookie.class);
+  private final ArgumentCaptor<Cookie> cookieArgumentCaptor = ArgumentCaptor.forClass(Cookie.class);
+  private final HttpServletResponse response = mock(HttpServletResponse.class);
+  private final HttpServletRequest request = mock(HttpServletRequest.class);
 
-  private HttpServletResponse response = mock(HttpServletResponse.class);
-  private HttpServletRequest request = mock(HttpServletRequest.class);
-
-  private OAuth2AuthenticationParameters underTest = new OAuth2AuthenticationParametersImpl();
+  private final OAuth2AuthenticationParameters underTest = new OAuth2AuthenticationParametersImpl();
 
   @Before
   public void setUp() {
@@ -98,27 +97,24 @@ public class OAuth2AuthenticationParametersImplTest {
 
   @Test
   @DataProvider({"http://example.com", "/\t/example.com", "//local_file", "/\\local_file", "something_else"})
-  public void return_to_is_not_set_when_not_local(String url) {
+  public void get_return_to_is_not_set_when_not_local(String url) {
     when(request.getParameter("return_to")).thenReturn(url);
 
-    underTest.init(request, response);
-
-    verify(response, never()).addCookie(any());
+    assertThat(underTest.getReturnTo(request)).isEmpty();
   }
 
   @Test
   public void get_return_to_parameter() {
-    when(request.getCookies()).thenReturn(new Cookie[] {new Cookie(AUTHENTICATION_COOKIE_NAME, "{\"return_to\":\"/settings\"}")});
+    when(request.getCookies()).thenReturn(new Cookie[]{new Cookie(AUTHENTICATION_COOKIE_NAME, "{\"return_to\":\"/settings\"}")});
 
     Optional<String> redirection = underTest.getReturnTo(request);
 
-    assertThat(redirection).isNotEmpty();
-    assertThat(redirection.get()).isEqualTo("/settings");
+    assertThat(redirection).contains("/settings");
   }
 
   @Test
   public void get_return_to_is_empty_when_no_cookie() {
-    when(request.getCookies()).thenReturn(new Cookie[] {});
+    when(request.getCookies()).thenReturn(new Cookie[]{});
 
     Optional<String> redirection = underTest.getReturnTo(request);
 
@@ -127,7 +123,7 @@ public class OAuth2AuthenticationParametersImplTest {
 
   @Test
   public void get_return_to_is_empty_when_no_value() {
-    when(request.getCookies()).thenReturn(new Cookie[] {new Cookie(AUTHENTICATION_COOKIE_NAME, "{}")});
+    when(request.getCookies()).thenReturn(new Cookie[]{new Cookie(AUTHENTICATION_COOKIE_NAME, "{}")});
 
     Optional<String> redirection = underTest.getReturnTo(request);
 
@@ -136,7 +132,7 @@ public class OAuth2AuthenticationParametersImplTest {
 
   @Test
   public void get_allowEmailShift_parameter() {
-    when(request.getCookies()).thenReturn(new Cookie[] {new Cookie(AUTHENTICATION_COOKIE_NAME, "{\"allowEmailShift\":\"true\"}")});
+    when(request.getCookies()).thenReturn(new Cookie[]{new Cookie(AUTHENTICATION_COOKIE_NAME, "{\"allowEmailShift\":\"true\"}")});
 
     Optional<Boolean> allowEmailShift = underTest.getAllowEmailShift(request);
 
@@ -146,7 +142,7 @@ public class OAuth2AuthenticationParametersImplTest {
 
   @Test
   public void get_allowEmailShift_is_empty_when_no_cookie() {
-    when(request.getCookies()).thenReturn(new Cookie[] {});
+    when(request.getCookies()).thenReturn(new Cookie[]{});
 
     Optional<Boolean> allowEmailShift = underTest.getAllowEmailShift(request);
 
@@ -155,7 +151,7 @@ public class OAuth2AuthenticationParametersImplTest {
 
   @Test
   public void get_allowEmailShift_is_empty_when_no_value() {
-    when(request.getCookies()).thenReturn(new Cookie[] {new Cookie(AUTHENTICATION_COOKIE_NAME, "{}")});
+    when(request.getCookies()).thenReturn(new Cookie[]{new Cookie(AUTHENTICATION_COOKIE_NAME, "{}")});
 
     Optional<Boolean> allowEmailShift = underTest.getAllowEmailShift(request);
 
@@ -164,7 +160,7 @@ public class OAuth2AuthenticationParametersImplTest {
 
   @Test
   public void getAllowUpdateLogin_is_empty_when_no_cookie() {
-    when(request.getCookies()).thenReturn(new Cookie[] {});
+    when(request.getCookies()).thenReturn(new Cookie[]{});
 
     Optional<Boolean> allowLoginUpdate = underTest.getAllowUpdateLogin(request);
 
@@ -173,7 +169,7 @@ public class OAuth2AuthenticationParametersImplTest {
 
   @Test
   public void getAllowUpdateLogin_is_empty_when_no_value() {
-    when(request.getCookies()).thenReturn(new Cookie[] {new Cookie(AUTHENTICATION_COOKIE_NAME, "{}")});
+    when(request.getCookies()).thenReturn(new Cookie[]{new Cookie(AUTHENTICATION_COOKIE_NAME, "{}")});
 
     Optional<Boolean> allowLoginUpdate = underTest.getAllowUpdateLogin(request);
 
@@ -182,7 +178,7 @@ public class OAuth2AuthenticationParametersImplTest {
 
   @Test
   public void delete() {
-    when(request.getCookies()).thenReturn(new Cookie[] {new Cookie(AUTHENTICATION_COOKIE_NAME, "{\"return_to\":\"/settings\"}")});
+    when(request.getCookies()).thenReturn(new Cookie[]{new Cookie(AUTHENTICATION_COOKIE_NAME, "{\"return_to\":\"/settings\"}")});
 
     underTest.delete(request, response);