[SecurityBundle] Authentication entry point is only registered with firewall exception listener, not with authentication listeners #12296
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
|
@fabpot Can you look into this? Is there no dedicated merger/decider for Security? |
|
@schmittjoh, @fabpot Any chance either of you could tell us what to do to get this handled or (preferably 😉 )merged in? Or is our expectation of the Thanks in advance! |
|
This looks like a valid fix. Without it, a custom entry point will indeed never be injected into the security listener. However, it would be good to include tests for this. It's probably the best to add some functional tests (see Tests/Functaional in the SecurityBundle). |
|
Cheers, we'll look into adding the tests. |
added 2 commits
November 5, 2014 15:02
I had configured a different firewall entry point for one firewall. However, when authentication had to be performed, it still called BasicAuthenticationEntryPoint::start() instead of my service's start(). My service was instantiated, yet never used. The issue appeared to be that the entry point is registered with the firewall's exception listener, but not with the BasicAuthenticationListener. This means that when the BasicAuthenticationListener determines the user has provided wrong credentials, BasicAuthenticationEntryPoint is still used. Only in case of an exception would my entry point service be used. See #12261.
|
@jakzal Failing test is unrelated. Can this be merged? |
|
@rjkip I'll review this weekend. |
|
@jakzal Did you manage to take a look at this? Thanks in advance! |
|
@stof, @webmozart, @Tobion, @romainneutron, @nicolas-grekas, @fabpot As far as I can see in http://symfony.com/doc/current/contributing/code/core_team.html#active-core-members there is no "merger" assigned for the SecurityBundle, any chance either of you can take a peek at this? Edit: spelling |
|
Thank you @rjkip. |
fabpot
added a commit
that referenced
this pull request
Nov 20, 2014
…ered with firewall exception listener, not with authentication listeners (rjkip) This PR was submitted for the master branch but it was merged into the 2.3 branch instead (closes #12296). Discussion ---------- [SecurityBundle] Authentication entry point is only registered with firewall exception listener, not with authentication listeners | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | BC breaks? | when relying on this configuration behaviour | Deprecations? | no | Tests pass? | yes | Fixed tickets | #12261 | License | MIT | Doc PR | — See #12261. I configured a different firewall entry point for one firewall. However, when authentication had to be performed, it still called BasicAuthenticationEntryPoint::start() instead of my service's start(). My service was instantiated, yet never used. The issue appears to be that the entry point is registered with the firewall's exception listener, but not with the BasicAuthenticationListener. This means that when the BasicAuthenticationListener determines the user has provided wrong credentials, BasicAuthenticationEntryPoint is still used. Only in case of an exception would my entry point service be used. In my opinion, this is not correct behaviour. Can someone confirm this? Are there currently tests that pertain to the `entry_point` configuration on which I can base a test? --- Test setup: ```yaml # security.yml security: firewalls: api: pattern: ^/api/ http_basic: ~ entry_point: my.service default: anonymous: ~ ``` Commits ------- 92c8dfb [SecurityBundle] Authentication entry point is only registered with firewall exception listener, not with authentication listeners
fabpot
added a commit
that referenced
this pull request
Dec 2, 2014
…ured entry point or a default entry point (rjkip) This PR was squashed before being merged into the 2.3 branch (closes #12811). Discussion ---------- Configure firewall's kernel exception listener with configured entry point or a default entry point | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | BC breaks? | when relying on buggy behaviour | Deprecations? | no | Tests pass? | yes | Fixed tickets | #12581 #12801 | License | MIT | Doc PR | — The #12296 PR introduced a bug where the firewall's exception listener was sometimes configured with the default entry point returned by `SecurityExtension#createAuthenticationListeners()`. These changes add a regression test and make the configured entry point (if any) always take precedence over a default entry point. If someone can confirm this fix, it would have my preference merging this over reverting #12296 for Symfony 2.6.1. Commits ------- b122262 Configure firewall's kernel exception listener with configured entry point or a default entry point
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.
See #12261.
I configured a different firewall entry point for one firewall. However, when authentication had to be performed, it still called BasicAuthenticationEntryPoint::start() instead of my service's start(). My service was instantiated, yet never used.
The issue appears to be that the entry point is registered with the firewall's exception listener, but not with the BasicAuthenticationListener. This means that when the BasicAuthenticationListener determines the user has provided wrong credentials, BasicAuthenticationEntryPoint is still used. Only in case of an exception would my entry point service be used.
In my opinion, this is not correct behaviour. Can someone confirm this? Are there currently tests that pertain to the
entry_pointconfiguration on which I can base a test?Test setup: