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

Set cookie domain for cancel remember-me #3871

Closed
asauvez opened this issue May 6, 2016 · 4 comments
Closed

Set cookie domain for cancel remember-me #3871

asauvez opened this issue May 6, 2016 · 4 comments
Assignees
Labels
in: web An issue in web modules (web, webmvc)
Milestone

Comments

@asauvez
Copy link

asauvez commented May 6, 2016

Summary

Following https://jira.spring.io/browse/SEC-3210, we now have the ability to specify the remember-me cookie domain, in order to have the same cookie on en.sample.org and fr.sample.org by example. But If you apply a domain in AbstractRememberMeServices.setCookie(), you should also apply it to cancelCookie().

In order to delete a cookie, one must specify the same domain.

Actual Behavior

  1. Specify a domain http.rememberMe().rememberMeCookieDomain(".sample.org");
  2. Login with remember me
  3. Logout
  4. You are still logged, since the remember-me cookie was not deleted.

Expected Behavior

After 3., you should be disconnected.

Version

Spring Security 4.1.0.

Suggested Correction

Pull request asauvez#1

protected void cancelCookie(HttpServletRequest request, HttpServletResponse response) {
    logger.debug("Cancelling cookie");
    Cookie cookie = new Cookie(cookieName, null);
    cookie.setMaxAge(0);
    if (cookieDomain != null) {
        cookie.setDomain(cookieDomain);
    }
    cookie.setPath(getCookiePath(request));

    response.addCookie(cookie);
}
@jgrandja jgrandja self-assigned this May 9, 2016
@jgrandja jgrandja added the in: web An issue in web modules (web, webmvc) label May 9, 2016
@jgrandja jgrandja added this to the 4.1.1 milestone May 9, 2016
@jgrandja
Copy link
Contributor

jgrandja commented May 9, 2016

Thanks for catching this @asauvez. Would you be able to write a test in AbstractRememberMeServicesTests for this issue?

@jgrandja
Copy link
Contributor

jgrandja commented May 9, 2016

Also, I noticed the referenced PR is in your forked repo. Would you like to submit the PR to the Spring Security repo?

@asauvez
Copy link
Author

asauvez commented May 10, 2016

Done here:
#3878
Thank you for your time and have a nice day.

@rwinch rwinch changed the title Appli remember-me cookie domain on cancelCookie also Set cookie domain for cancel remember-me May 13, 2016
@rwinch rwinch closed this as completed May 13, 2016
rwinch pushed a commit that referenced this issue May 13, 2016
@rwinch rwinch assigned rwinch and unassigned jgrandja May 13, 2016
@rwinch
Copy link
Member

rwinch commented May 13, 2016

Thanks @asauvez! This is now merged into master w/ polish to the formatting and commit comment

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)
Projects
None yet
Development

No branches or pull requests

3 participants