Skip to content

Commit

Permalink
Validate form_redirect_uri parameter against the request host.
Browse files Browse the repository at this point in the history
- The form_redirect_uri is now only used if it redirects to the same
host as the receiving UAA server.

[#158222161] https://www.pivotaltracker.com/story/show/158222161

Signed-off-by: Jaskanwal Pawar <jpawar@pivotal.io>
  • Loading branch information
bruce-ricard authored and Pivotal committed Jun 12, 2018
1 parent d370654 commit 7a8f157
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@

import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import java.net.MalformedURLException;
import java.net.URL;

public class UaaSavedRequestAwareAuthenticationSuccessHandler extends SavedRequestAwareAuthenticationSuccessHandler {
public static final String SAVED_REQUEST_SESSION_ATTRIBUTE = "SPRING_SECURITY_SAVED_REQUEST";
Expand All @@ -38,10 +40,23 @@ public String determineTargetUrl(HttpServletRequest request, HttpServletResponse
if (redirectAttribute !=null) {
logger.debug("Returning redirectAttribute saved URI:"+redirectAttribute);
return (String) redirectAttribute;
} else if (redirectFormParam != null) {
} else if (isApprovedFormRedirectUri(request, redirectFormParam)) {
return redirectFormParam;
} else {
return super.determineTargetUrl(request, response);
}
}

private boolean isApprovedFormRedirectUri(HttpServletRequest request, String redirectUri) {
if (redirectUri == null) {
return false;
}

try {
URL url = new URL(redirectUri);
return request.getServerName().equals(url.getHost());
} catch (MalformedURLException e) {
return false;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -41,16 +41,26 @@ public void allow_url_override() {
assertEquals("http://test.com", handler.determineTargetUrl(request, new MockHttpServletResponse()));
}

@Test
public void form_parameter_works() {
request.setParameter(FORM_REDIRECT_PARAMETER, "http://test.com");
assertEquals("http://test.com", handler.determineTargetUrl(request, new MockHttpServletResponse()));
}

@Test
public void form_parameter_is_overridden() {
request.setParameter(FORM_REDIRECT_PARAMETER, "http://test.com");
request.setAttribute(URI_OVERRIDE_ATTRIBUTE, "http://override.test.com");
assertEquals("http://override.test.com", handler.determineTargetUrl(request, new MockHttpServletResponse()));
}
}

@Test
public void validFormRedirectIsReturned() {
String redirectUri = request.getScheme() + "://" + request.getServerName() + "/test";

request.setParameter(FORM_REDIRECT_PARAMETER, redirectUri);
assertEquals(redirectUri, handler.determineTargetUrl(request, new MockHttpServletResponse()));
}

@Test
public void invalidFormRedirectIsNotReturned() {
String redirectUri = "http://test.com/test";

request.setParameter(FORM_REDIRECT_PARAMETER, redirectUri);
assertEquals("/", handler.determineTargetUrl(request, new MockHttpServletResponse()));
}
}

0 comments on commit 7a8f157

Please sign in to comment.