Skip to content

Set secure on cookie when logging out #7764

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

Conversation

okohub
Copy link
Contributor

@okohub okohub commented Dec 20, 2019

It is better to mark cookie secure flag with request.isSecure() to ensure cookie identity is same

It is better to mark cookie secure flag with request.isSecure() to ensure cookie identity is same
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Dec 20, 2019
@eleftherias eleftherias self-assigned this Dec 27, 2019
Copy link
Contributor

@eleftherias eleftherias 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 @okohub! I have left one comment inline.

assertThat(response.getCookies()).hasSize(1);
assertThat(response.getCookies()[0].getSecure()).isTrue();
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Please add a test checking that when the request is not secure, then the cookie is not secure.

@eleftherias eleftherias 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 Dec 30, 2019
@okohub
Copy link
Contributor Author

okohub commented Jan 12, 2020

@eleftherias I added the test you requested, thanks.

@eleftherias eleftherias changed the title Fix CookieClearingLogoutHandler cookie secure flag: Set secure on cookie when logging out Jan 13, 2020
@eleftherias eleftherias merged commit 1f6381d into spring-projects:master Jan 13, 2020
@eleftherias
Copy link
Contributor

Thanks for the PR @okohub! This is now merged into master.

@eleftherias eleftherias added this to the 5.3.0.RC1 milestone Jan 13, 2020
@okohub okohub deleted the fix-cookie-clear-handler-inconsistency branch January 13, 2020 19:07
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) type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants