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

Mismatch of cookie path name between Spring Core and Spring Session #1004

Closed
michaelwiles opened this issue Feb 26, 2018 · 4 comments
Closed
Assignees

Comments

@michaelwiles
Copy link

The path name for a cookie when using Spring Session cookie management is:

<CONTEXT_PATH>/

The code is provided in DefaultCookieSerializer:

private String getCookiePath(HttpServletRequest request) {
		if (this.cookiePath == null) {
			return request.getContextPath() + "/";
		}
		return this.cookiePath;
	}

Notice how the path ends with a forward slash.

Now when I logout I want to delete this cookie:

So I configure spring security to do exactly that (the session is SESSION)

http.logout()
.defaultLogoutSuccessHandlerFor(new HttpStatusReturningLogoutSuccessHandler(), new 
     AntPathRequestMatcher("/logout"))
.deleteCookies("SESSION");

Doing this invokes the following code from CookieClearingLogoutHandler on logout (you'll see this from the source code of deleteCookies):

	public void logout(HttpServletRequest request, HttpServletResponse response,
			Authentication authentication) {
		for (String cookieName : cookiesToClear) {
			Cookie cookie = new Cookie(cookieName, null);
			String cookiePath = request.getContextPath();
			if (!StringUtils.hasLength(cookiePath)) {
				cookiePath = "/";
			}
			cookie.setPath(cookiePath);
			cookie.setMaxAge(0);
			response.addCookie(cookie);
		}
	}

Notice that the cookie path is simply set to requestContext.getContextPath()

No trailing slash.

Thus as things stand spring's logout handler does not delete the cookie as for the cookie to be deleted the paths have to match 😞

Is there a reason why spring session appends the / to the context path?
Is it possible to have this extra slash removed so it is in sync with core springs CookieClearingLogoutHandler?

@vpavic vpavic self-assigned this Feb 28, 2018
@vpavic
Copy link
Contributor

vpavic commented Feb 28, 2018

Thanks for the report @michaelwiles.

I see your point that DefaultCookieSerializer and CookieClearingLogoutHandler are having somewhat different handling of cookie path. Does configuring cookiePath on DefaultCookieSerializer explicitly help you as a workaround?

Ping @rwinch, can you provide more insight?

@michaelwiles
Copy link
Author

The glory of Spring and open source is that you can always make a plan.

I couldn't really go the custom cookie path route as I did not want to hard code my context path (I guess I could have pursued this route further but found the path of least resistance on the Spring core side.

I'm using the spring security configurer to configure my cookie handling. When I found the issue I had the following code:

http is of type org.springframework.security.config.annotation.web.builders.HttpSecurity.

http.logout().deleteCookies("SESSION");

And deleteCookies does this:
addLogoutHandler(new CookieClearingLogoutHandler(cookieNamesToClear));

It is in CookieClearingLogoutHandler that actually uses the context path (and doesn't suffix the /).

So all I did was create my own CookieClearingLogoutHandler which uses the different cookie path.

@vpavic
Copy link
Contributor

vpavic commented Mar 15, 2018

I stumbled upon spring-projects/spring-security#2325 recently which is effectively the issue you're reporting here.

Since it's marked as a bug at Spring Security side, I'm inclined to close this as duplicate.

@vpavic
Copy link
Contributor

vpavic commented Mar 19, 2018

@michaelwiles This has been addressed via spring-projects/spring-security#2325. The fixed has been backported to 5.0.x and 4.2.x branches of Spring Security.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants