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

Caution about impersonation not compatible with pre authenticated #6673

Merged
merged 2 commits into from
Jul 2, 2016
Merged

Caution about impersonation not compatible with pre authenticated #6673

merged 2 commits into from
Jul 2, 2016

Conversation

pasdeloup
Copy link
Contributor

Caution added to close Symfony issue #2172 according to @fabpot comment in the Symfony #19059 PR.
I also improved the similar caution about REMOTE_USER to make it global.

User impersonation is not compatible with
:doc:`pre Authenticated firewalls</cookbook/security/pre_authenticated>`. The
reason is that impersonation requires the authentication state to be maintained
server-side but pre Authenticated information (``SSL_CLIENT_S_DN_Email``,
Copy link
Member

Choose a reason for hiding this comment

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

pre Authenticated information -> preauthenticated information or pre-authenticated information

@javiereguiluz
Copy link
Member

👍

Status: reviewed

@xabbuh
Copy link
Member

xabbuh commented Jun 30, 2016

👍

@wouterj wouterj merged commit b5354e5 into symfony:2.7 Jul 2, 2016
wouterj added a commit that referenced this pull request Jul 2, 2016
…henticated (pasdeloup)

This PR was merged into the 2.7 branch.

Discussion
----------

Caution about impersonation not compatible with pre authenticated

Caution added to close Symfony issue #2172 according to @fabpot comment in the Symfony #19059 PR.
I also improved the similar caution about REMOTE_USER to make it global.

Commits
-------

b5354e5 fix pre Authenticated -> pre-authenticated
f4249f3 improve caution about impersonation not compatible with pre authenticated firewalls
wouterj added a commit that referenced this pull request Jul 2, 2016
@wouterj
Copy link
Member

wouterj commented Jul 2, 2016

Thank you @pasdeloup! This is a very nice caution, explaining just enough to know why this aren't supported without being too verbose.

I've made some very minor rewordings in dd3f08f after merging this PR. If you don't agree, feel free to comment. Thanks again!

@pasdeloup
Copy link
Contributor Author

Thanks, it's better like that.

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

Successfully merging this pull request may close these issues.

5 participants