10000 bug #15925 Updating behavior to not continue after an authenticator h… · symfony/symfony@06c14a2 · GitHub
[go: up one dir, main page]
Skip to content

Commit 06c14a2

Browse files
fabpotfabpot
committed
bug #15925 Updating behavior to not continue after an authenticator has set the response (weaverryan)
This PR was merged into the 2.8 branch. Discussion ---------- Updating behavior to not continue after an authenticator has set the response | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | https://github.com/symfony/symfony/pull/14673/files#r40492765 | License | MIT | Doc PR | n/a This mirrors the behavior in core: *if* a listener sets a response (on success or failure), then the other listeners are not called. But if a response is *not* set (which is sometimes the case for success, like in BasicAuthenticationListener), then the other listeners are called, and can even fail. It's all a bit of an edge-case, as only one authenticator (like authentication listener) would normally be doing any work on a request, but I think matching the other listeners (since I'm not aware of anyone having issues with its behavior) is best. Commits ------- 5fa2684 Making all "debug" messages use the debug router f403444 Updating behavior to not continue after an authenticator has set the response
2 parents 5dcdc48 + 5fa2684 commit 06c14a2

File tree

2 files changed

+47
-8
lines changed

2 files changed

+47
-8
lines changed

‎src/Symfony/Component/Security/Guard/Firewall/GuardAuthenticationListener.php

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ public function __construct(GuardAuthenticatorHandler $guardHandler, Authenticat
6666
public function handle(GetResponseEvent $event)
6767
{
6868
if (null !== $this->logger) {
69-
$this->logger->info('Checking for guard authentication credentials.', array('firewall_key' => $this->providerKey, 'authenticators' => count($this->guardAuthenticators)));
69+
$this->logger->debug('Checking for guard authentication credentials.', array('firewall_key' => $this->providerKey, 'authenticators' => count($this->guardAuthenticators)));
7070
}
7171

7272
foreach ($this->guardAuthenticators as $key => $guardAuthenticator) {
@@ -75,6 +75,12 @@ public function handle(GetResponseEvent $event)
7575
$uniqueGuardKey = $this->providerKey.'_'.$key;
7676

7777
$this->executeGuardAuthenticator($uniqueGuardKey, $guardAuthenticator, $event);
78+
79+
if ($event->hasResponse()) {
80+
$this->logger->debug(sprintf('The "%s" authenticator set the response. Any later authenticator will not be called', get_class($guardAuthenticator)));
81+
82+
break;
83+
}
7884
}
7985
}
8086

@@ -83,7 +89,7 @@ private function executeGuardAuthenticator($uniqueGuardKey, GuardAuthenticatorIn
8389
$request = $event->getRequest();
8490
try {
8591
if (null !== $this->logger) {
86-
$this->logger->info('Calling getCredentials on guard configurator.', array('firewall_key' => $this->providerKey, 'authenticator' => get_class($guardAuthenticator)));
92+
$this->logger->debug('Calling getCredentials() on guard configurator.', array('firewall_key' => $this->providerKey, 'authenticator' => get_class($guardAuthenticator)));
8793
}
8894

8995
// allow the authenticator to fetch authentication info from the request
@@ -98,7 +104,7 @@ private function executeGuardAuthenticator($uniqueGuardKey, GuardAuthenticatorIn
98104
$token = new PreAuthenticationGuardToken($credentials, $uniqueGuardKey);
99105

100106
if (null !== $this->logger) {
101-
$this->logger->info('Passing guard token information to the GuardAuthenticationProvider', array('firewall_key' => $this->providerKey, 'authenticator' => get_class($guardAuthenticator)));
107+
$this->logger->debug('Passing guard token information to the GuardAuthenticationProvider', array('firewall_key' => $this->providerKey, 'authenticator' => get_class($guardAuthenticator)));
102108
}
103109
// pass the token into the AuthenticationManager system
104110
// this indirectly calls GuardAuthenticationProvider::authenticate()
@@ -130,13 +136,13 @@ private function executeGuardAuthenticator($uniqueGuardKey, GuardAuthenticatorIn
130136
$response = $this->guardHandler->handleAuthenticationSuccess($token, $request, $guardAuthenticator, $this->providerKey);
131137
if ($response instanceof Response) {
132138
if (null !== $this->logger) {
133-
$this->logger->info('Guard authenticator set success response.', array('response' => $response, 'authenticator' => get_class($guardAuthenticator)));
139+
$this->logger->debug('Guard authenticator set success response.', array('response' => $response, 'authenticator' => get_class($guardAuthenticator)));
134140
}
135141

136142
$event->setResponse($response);
137143
} else {
138144
if (null !== $this->logger) {
139-
$this->logger->info('Guard authenticator set no success response: request continues.', array('authenticator' => get_class($guardAuthenticator)));
145+
$this->logger->debug('Guard authenticator set no success response: request continues.', array('authenticator' => get_class($guardAuthenticator)));
140146
}
141147
}
142148

@@ -167,15 +173,15 @@ private function triggerRememberMe(GuardAuthenticatorInterface $guardAuthenticat
167173
{
168174
if (null === $this->rememberMeServices) {
169175
if (null !== $this->logger) {
170-
$this->logger->info('Remember me skipped: it is not configured for the firewall.', array('authenticator' => get_class($guardAuthenticator)));
176+
$this->logger->debug('Remember me skipped: it is not configured for the firewall.', array('authenticator' => get_class($guardAuthenticator)));
171177
}
172178

173179
return;
174180
}
175181

176182
if (!$guardAuthenticator->supportsRememberMe()) {
177183
if (null !== $this->logger) {
178-
$this->logger->info('Remember me skipped: your authenticator does not support it.', array('authenticator' => get_class($guardAuthenticator)));
184+
$this->logger->debug('Remember me skipped: your authenticator does not support it.', array('authenticator' => get_class($guardAuthenticator)));
179185
}
180186

181187
return;

‎src/Symfony/Component/Security/Guard/Tests/Firewall/GuardAuthenticationListenerTest.php

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,36 @@ public function testHandleSuccess()
7979
$listener->handle($this->event);
8080
}
8181

82+
public function testHandleSuccessStopsAfterResponseIsSet()
83+
{
84+
$authenticator1 = $this->getMock('Symfony\Component\Security\Guard\GuardAuthenticatorInterface');
85+
$authenticator2 = $this->getMock('Symfony\Component\Security\Guard\GuardAuthenticatorInterface');
86+
87+
// mock the first authenticator to fail, and set a Response
88+
$authenticator1
89+
->expects($this->once())
90+
->method('getCredentials')
91+
->willThrowException(new AuthenticationException());
92+
$this->guardAuthenticatorHandler
93+
->expects($this->once())
94+
->method('handleAuthenticationFailure')
95+
->willReturn(new Response());
96+
// the second authenticator should *never* be called
97+
$authenticator2
98+
->expects($this->never())
99+
->method('getCredentials');
100+
101+
$listener = new GuardAuthenticationListener(
102+
$this->guardAuthenticatorHandler,
103+
$this->authenticationManager,
104+
'my_firewall',
105+
array($authenticator1, $authenticator2),
106+
$this->logger
107+
);
108+
109+
$listener->handle($this->event);
110+
}
111+
82112
public function testHandleSuccessWithRememberMe()
83113
{
84114
$authenticator = $this->getMock('Symfony\Component\Security\Guard\GuardAuthenticatorInterface');
@@ -201,7 +231,10 @@ protected function setUp()
201231

202232
$this->request = new Request(array(), array(), array(), array(), array(), array());
203233

204-
$this->event = $this->getMock('Symfony\Component\HttpKernel\Event\GetResponseEvent', array(), array(), '', false);
234+
$this->event = $this->getMockBuilder('Symfony\Component\HttpKernel\Event\GetResponseEvent')
235+
->disableOriginalConstructor()
236+
->setMethods(array('getRequest'))
237+
->getMock();
205238
$this->event
206239
->expects($this->any())
207240
->method('getRequest')

0 commit comments

Comments
 (0)
0