[Security] Added a REMOTE_USER based listener to security firewalls #10698
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
zefrog
changed the title
| $serverVars = array(); | ||
| if ('' !== $user) { | ||
| $serverVars['REMOTE_USER'] = $user; | ||
| } |
There was a problem hiding this comment.
There should be no conditions in tests so they're easy to follow. In this particular case $user will never be empty.
|
Thank you for your comments. I think I have fixed these issues. |
|
The Travis error does not seem to be related to my PR... Can someone from the core tell me if this has any chance to be accepted? Do I need to prepare a documentation PR? |
|
@zefrog Looks good to me. Can you create a doc PR? |
|
I've added the firewall entry in the configuration reference and made a cookbook page describing how to use pre authenticated listeners. Is that ok? |
|
@fabpot will this feature be integrated in the next 2.6 release ? Thx. |
|
No thoughts on this PR ? It's been around since april... |
|
👍 |
|
Thanks for your work on this new feature! |
fabpot
added a commit
that referenced
this pull request
Sep 23, 2014
…ity firewalls (Maxime Douailin) This PR was squashed before being merged into the 2.6-dev branch (closes #10698). Discussion ---------- [Security] Added a REMOTE_USER based listener to security firewalls | Q | A | ------------- | --- | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | / | License | MIT | Doc PR | symfony/symfony-docs#3912 TODO - [x] submit changes to the documentation I've seen myself implementing a few times a REMOTE_USER based authentication listener, as a large part of security modules for Apache (Kerberos, CAS, and more) are providing the username via an environment variable. So I thought this could benefit the whole community if directly included in the framework. It is very similar to the X509AuthenticationListener, and basing the RemoteUserAuthenticationListener on the AbstractPreAuthenticatedListener is relevant and very convenient. Using the X509AuthenticationListener could be possible, but it is confusing to use it directly when your authentication is not certificate based. Please let me know if I need to update anything. Regards Commits ------- a2872f2 [Security] Added a REMOTE_USER based listener to security firewalls
|
thanks @zefrog 💖, very nice feature ! |
weaverryan
added a commit
to symfony/symfony-docs
that referenced
this pull request
Oct 10, 2014
…ation for pre authenticated firewalls (Maxime Douailin, mdouailin) This PR was merged into the master branch. Discussion ---------- [Security] Added remote_user firewall info and documentation for pre authenticated firewalls | Q | A | ------------- | --- | Doc fix? | no | New docs? | yes (symfony/symfony#10698) | Applies to | 2.6+ | Fixed tickets | no Some documentation for pre authenticated firewalls, and added remote_user configuration reference for this new firewall. Commits ------- f36c45e uppercase title e6aa733 swapped comment and opening in xml configuration example b8a0eb2 fixes missing backtick be0d866 fix missing backtick, rephrased bottom note 86ba188 rebased using x509 pr, added remote_user pre authenticated part 8465d46 [Reference][Configuration] Removed version added for remote_user 34ad1b5 [Security] Added remote_user firewall info and documentation for pre authenticated firewalls
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
TODO
I've seen myself implementing a few times a REMOTE_USER based authentication listener, as a large part of security modules for Apache (Kerberos, CAS, and more) are providing the username via an environment variable.
So I thought this could benefit the whole community if directly included in the framework. It is very similar to the X509AuthenticationListener, and basing the RemoteUserAuthenticationListener on the AbstractPreAuthenticatedListener is relevant and very convenient.
Using the X509AuthenticationListener could be possible, but it is confusing to use it directly when your authentication is not certificate based.
Please let me know if I need to update anything.
Regards