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.

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

merged 2 commits into from
Oct 16, 2023

Conversation

mches
Copy link
Contributor

@mches mches commented Mar 2, 2023

  • Makes it possible to customize the redirect strategy of RequestedUrlRedirectInvalidSessionStrategy, so that the servlet context path can be taken into account if desired.
  • Makes the HTTP status code used by DefaultRedirectStrategy configurable, so that 307 Temporary Redirect can be chosen if desired.

Closes gh-12795
Closes gh-12797

Copy link
Contributor

@jzheaux jzheaux left a comment

Choose a reason for hiding this comment

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

Thanks for the PR, @mches! And sorry for the delay; I think this PR got lost in the shuffle.

I've left some inline feedback and look forward to your updates.

@jzheaux jzheaux self-assigned this Aug 28, 2023
@jzheaux jzheaux added in: web An issue in web modules (web, webmvc) type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged labels Aug 28, 2023
@jzheaux
Copy link
Contributor

jzheaux commented Aug 28, 2023

Also, since this is a new feature, @mches, will you please change the branch to main and ensure that any new APIs have a @since 6.2 JavaDoc?

@mches
Copy link
Contributor Author

mches commented Aug 29, 2023

Also, since this is a new feature, @mches, will you please change the branch to main and ensure that any new APIs have a @since 6.2 JavaDoc?

I was hoping for this to be usable with Spring Boot 2.7.x, but I get it.

@mches
Copy link
Contributor Author

mches commented Aug 29, 2023

Thanks for the PR, @mches! And sorry for the delay; I think this PR got lost in the shuffle.

I've left some inline feedback and look forward to your updates.

Thank you for the review, @jzheaux. I've responded inline.

I've also rebased, force-pushed, retargeted the PR and updated the PR documentation.

@mches mches changed the base branch from 5.7.x to main September 5, 2023 03:56
@mches mches changed the title Take servlet context path into account and ensure HTTP method is preserved on redirect to requested URL Make it possible to take servlet context path into account and ensure HTTP method is preserved on redirect to requested URL Sep 5, 2023
*/
public class SessionManagementFilterTests {

@BeforeEach
Copy link
Contributor Author

@mches mches Oct 7, 2023

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);
		…
	}

}

@jzheaux jzheaux added this to the 6.2.0-RC2 milestone Oct 16, 2023
@jzheaux jzheaux added the status: duplicate A duplicate of another issue label Oct 16, 2023
@jzheaux jzheaux merged commit d9399df into spring-projects:main Oct 16, 2023
1 check passed
@mches mches deleted the gh-12795+gh-12797-fix-redirect-bugs branch October 16, 2023 22:40
@jzheaux jzheaux changed the title Make it possible to take servlet context path into account and ensure HTTP method is preserved on redirect to requested URL Make Redirect Status Code Configurable Oct 31, 2023
jzheaux added a commit that referenced this pull request Oct 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web An issue in web modules (web, webmvc) status: duplicate A duplicate of another issue type: enhancement A general enhancement
Projects
Status: Done
3 participants