Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make Redirect Status Code Configurable #12817

Merged
merged 2 commits into from
Oct 16, 2023
Merged

Make Redirect Status Code Configurable #12817

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
995a3f2
Allow redirect strategy to be customized
mches Sep 5, 2023
6d91c00
Allow redirect status code to be customized
mches Sep 5, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 28 additions & 2 deletions web/src/main/java/org/springframework/security/web/DefaultRedirectStrategy.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2016 the original author or authors.
* Copyright 2002-2023 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -24,6 +24,8 @@
import org.apache.commons.logging.LogFactory;

import org.springframework.core.log.LogMessage;
import org.springframework.http.HttpHeaders;
import org.springframework.http.HttpStatus;
import org.springframework.security.web.util.UrlUtils;
import org.springframework.util.Assert;

Expand All @@ -32,6 +34,7 @@
* the framework.
*
* @author Luke Taylor
* @author Mark Chesney
* @since 3.0
*/
public class DefaultRedirectStrategy implements RedirectStrategy {
Expand All @@ -40,6 +43,8 @@ public class DefaultRedirectStrategy implements RedirectStrategy {

private boolean contextRelative;

private HttpStatus statusCode = HttpStatus.FOUND;

/**
* Redirects the response to the supplied URL.
* <p>
Expand All @@ -55,7 +60,14 @@ public void sendRedirect(HttpServletRequest request, HttpServletResponse respons
if (this.logger.isDebugEnabled()) {
this.logger.debug(LogMessage.format("Redirecting to %s", redirectUrl));
}
response.sendRedirect(redirectUrl);
if (this.statusCode == HttpStatus.FOUND) {
response.sendRedirect(redirectUrl);
}
else {
response.setHeader(HttpHeaders.LOCATION, redirectUrl);
response.setStatus(this.statusCode.value());
jzheaux marked this conversation as resolved.
Show resolved Hide resolved
response.getWriter().flush();
}
}

protected String calculateRedirectUrl(String contextPath, String url) {
Expand Down Expand Up @@ -96,4 +108,18 @@ protected boolean isContextRelative() {
return this.contextRelative;
}

/**
* Sets the HTTP status code to use. The default is {@link HttpStatus#FOUND}.
* <p>
* Note that according to RFC 7231, with {@link HttpStatus#FOUND}, a user agent MAY
* change the request method from POST to GET for the subsequent request. If this
* behavior is undesired, {@link HttpStatus#TEMPORARY_REDIRECT} can be used instead.
* @param statusCode the HTTP status code to use.
* @since 6.2
*/
public void setStatusCode(HttpStatus statusCode) {
jzheaux marked this conversation as resolved.
Show resolved Hide resolved
Assert.notNull(statusCode, "statusCode cannot be null");
this.statusCode = statusCode;
jzheaux marked this conversation as resolved.
Show resolved Hide resolved
}

}
16 changes: 14 additions & 2 deletions .../org/springframework/security/web/session/RequestedUrlRedirectInvalidSessionStrategy.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2016 the original author or authors.
* Copyright 2002-2023 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -25,19 +25,21 @@

import org.springframework.security.web.DefaultRedirectStrategy;
import org.springframework.security.web.RedirectStrategy;
import org.springframework.util.Assert;
import org.springframework.web.servlet.support.ServletUriComponentsBuilder;

/**
* Performs a redirect to the original request URL when an invalid requested session is
* detected by the {@code SessionManagementFilter}.
*
* @author Craig Andrews
* @author Mark Chesney
*/
public final class RequestedUrlRedirectInvalidSessionStrategy implements InvalidSessionStrategy {

private final Log logger = LogFactory.getLog(getClass());

private final RedirectStrategy redirectStrategy = new DefaultRedirectStrategy();
private RedirectStrategy redirectStrategy = new DefaultRedirectStrategy();

private boolean createNewSession = true;

Expand Down Expand Up @@ -68,4 +70,14 @@ public void setCreateNewSession(boolean createNewSession) {
this.createNewSession = createNewSession;
}

/**
* Sets the redirect strategy to use. The default is {@link DefaultRedirectStrategy}.
* @param redirectStrategy the redirect strategy to use.
* @since 6.2
*/
public void setRedirectStrategy(RedirectStrategy redirectStrategy) {
Assert.notNull(redirectStrategy, "redirectStrategy cannot be null");
this.redirectStrategy = redirectStrategy;
}

}
21 changes: 20 additions & 1 deletion web/src/test/java/org/springframework/security/web/DefaultRedirectStrategyTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2016 the original author or authors.
* Copyright 2002-2023 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -18,6 +18,7 @@

import org.junit.jupiter.api.Test;

import org.springframework.http.HttpStatus;
import org.springframework.mock.web.MockHttpServletRequest;
import org.springframework.mock.web.MockHttpServletResponse;

Expand All @@ -26,6 +27,7 @@

/**
* @author Luke Taylor
* @author Mark Chesney
* @since 3.0
*/
public class DefaultRedirectStrategyTests {
Expand Down Expand Up @@ -64,4 +66,21 @@ public void contextRelativeShouldThrowExceptionIfURLDoesNotContainContextPath()
.isThrownBy(() -> rds.sendRedirect(request, response, "https://redirectme.somewhere.else"));
}

@Test
public void statusCodeIsHandledCorrectly() throws Exception {
// given
DefaultRedirectStrategy redirectStrategy = new DefaultRedirectStrategy();
redirectStrategy.setStatusCode(HttpStatus.TEMPORARY_REDIRECT);
MockHttpServletRequest request = new MockHttpServletRequest();
MockHttpServletResponse response = new MockHttpServletResponse();

// when
redirectStrategy.sendRedirect(request, response, "/requested");

// then
assertThat(response.isCommitted()).isTrue();
assertThat(response.getRedirectedUrl()).isEqualTo("/requested");
assertThat(response.getStatus()).isEqualTo(307);
}

}
70 changes: 69 additions & 1 deletion web/src/test/java/org/springframework/security/web/session/SessionManagementFilterTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2016 the original author or authors.
* Copyright 2002-2023 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -20,15 +20,18 @@
import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletResponse;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;

import org.springframework.http.HttpStatus;
import org.springframework.mock.web.MockFilterChain;
import org.springframework.mock.web.MockHttpServletRequest;
import org.springframework.mock.web.MockHttpServletResponse;
import org.springframework.security.authentication.AuthenticationTrustResolver;
import org.springframework.security.authentication.TestingAuthenticationToken;
import org.springframework.security.core.Authentication;
import org.springframework.security.core.context.SecurityContextHolder;
import org.springframework.security.web.DefaultRedirectStrategy;
import org.springframework.security.web.authentication.AuthenticationFailureHandler;
import org.springframework.security.web.authentication.session.SessionAuthenticationException;
import org.springframework.security.web.authentication.session.SessionAuthenticationStrategy;
Expand All @@ -46,9 +49,11 @@
/**
* @author Luke Taylor
* @author Rob Winch
* @author Mark Chesney
*/
public class SessionManagementFilterTests {

@BeforeEach
Copy link
Contributor Author

@mches mches Oct 7, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jzheaux SecurityContextHolderAwareRequestWrapperTests, and perhaps other tests, don't clear the context after each/all tests, so it becomes necessary to counteract side effects that crop up when such tests are run together. Adding @BeforeEach here does the trick, but I'm open to other options.

SecurityContextHolderAwareRequestWrapperTests.java for reference:

…
package org.springframework.security.web.servletapi;
…
public class SecurityContextHolderAwareRequestWrapperTests {

	@BeforeEach
	public void tearDown() {
		SecurityContextHolder.clearContext();
	}
…
	@Test
	public void testGetRemoteUserStringWithAuthenticatedPrincipal() {
		…
		SecurityContextHolder.getContext().setAuthentication(auth);
		…
	}

}

@AfterEach
public void clearContext() {
SecurityContextHolder.clearContext();
Expand Down Expand Up @@ -174,6 +179,69 @@ public void responseIsRedirectedToRequestedUrlIfSetAndSessionIsInvalid() throws
assertThat(response.getRedirectedUrl()).isEqualTo("/requested");
}

@Test
public void responseIsRedirectedToRequestedUrlIfContextPathIsSetAndSessionIsInvalid() throws Exception {
// given
DefaultRedirectStrategy redirectStrategy = new DefaultRedirectStrategy();
redirectStrategy.setContextRelative(true);
RequestedUrlRedirectInvalidSessionStrategy invalidSessionStrategy = new RequestedUrlRedirectInvalidSessionStrategy();
invalidSessionStrategy.setCreateNewSession(true);
invalidSessionStrategy.setRedirectStrategy(redirectStrategy);
SecurityContextRepository securityContextRepository = mock(SecurityContextRepository.class);
SessionAuthenticationStrategy sessionAuthenticationStrategy = mock(SessionAuthenticationStrategy.class);
SessionManagementFilter filter = new SessionManagementFilter(securityContextRepository,
sessionAuthenticationStrategy);
filter.setInvalidSessionStrategy(invalidSessionStrategy);
MockHttpServletRequest request = new MockHttpServletRequest();
request.setContextPath("/context");
request.setRequestedSessionId("xxx");
request.setRequestedSessionIdValid(false);
request.setRequestURI("/context/requested");
MockHttpServletResponse response = new MockHttpServletResponse();
FilterChain chain = mock(FilterChain.class);

// when
filter.doFilter(request, response, chain);

// then
verify(securityContextRepository).containsContext(request);
verifyNoMoreInteractions(securityContextRepository, sessionAuthenticationStrategy, chain);
assertThat(response.isCommitted()).isTrue();
assertThat(response.getRedirectedUrl()).isEqualTo("/context/requested");
assertThat(response.getStatus()).isEqualTo(302);
}

@Test
public void responseIsRedirectedToRequestedUrlIfStatusCodeIsSetAndSessionIsInvalid() throws Exception {
// given
DefaultRedirectStrategy redirectStrategy = new DefaultRedirectStrategy();
redirectStrategy.setStatusCode(HttpStatus.TEMPORARY_REDIRECT);
RequestedUrlRedirectInvalidSessionStrategy invalidSessionStrategy = new RequestedUrlRedirectInvalidSessionStrategy();
invalidSessionStrategy.setCreateNewSession(true);
invalidSessionStrategy.setRedirectStrategy(redirectStrategy);
SecurityContextRepository securityContextRepository = mock(SecurityContextRepository.class);
SessionAuthenticationStrategy sessionAuthenticationStrategy = mock(SessionAuthenticationStrategy.class);
SessionManagementFilter filter = new SessionManagementFilter(securityContextRepository,
sessionAuthenticationStrategy);
filter.setInvalidSessionStrategy(invalidSessionStrategy);
MockHttpServletRequest request = new MockHttpServletRequest();
request.setRequestedSessionId("xxx");
request.setRequestedSessionIdValid(false);
request.setRequestURI("/requested");
MockHttpServletResponse response = new MockHttpServletResponse();
FilterChain chain = mock(FilterChain.class);

// when
filter.doFilter(request, response, chain);

// then
verify(securityContextRepository).containsContext(request);
verifyNoMoreInteractions(securityContextRepository, sessionAuthenticationStrategy, chain);
assertThat(response.isCommitted()).isTrue();
assertThat(response.getRedirectedUrl()).isEqualTo("/requested");
assertThat(response.getStatus()).isEqualTo(307);
}

@Test
public void customAuthenticationTrustResolver() throws Exception {
AuthenticationTrustResolver trustResolver = mock(AuthenticationTrustResolver.class);
Expand Down
Loading