Skip to content

Commit f3873c8

Browse files
robfrawleyrobfrawley
committed
cleanup per pr comments and refactored event and tests
1 parent b3d37d2 commit f3873c8

File tree

5 files changed

+178
-98
lines changed

5 files changed

+178
-98
lines changed

‎src/Symfony/Component/Security/Core/Authentication/AuthenticationProviderManager.php

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@
1111

1212
namespace Symfony\Component\Security\Core\Authentication;
1313

14-
use Symfony\Component\EventDispatcher\Event;
1514
use Symfony\Component\Security\Core\Event\AuthenticationFailureEvent;
1615
use Symfony\Component\Security\Core\Event\AuthenticationEvent;
1716
use Symfony\Component\Security\Core\AuthenticationEvents;
@@ -36,7 +35,7 @@ class AuthenticationProviderManager implements AuthenticationManagerInterface
3635
private $eraseCredentials;
3736

3837
/**
39-
* @var EventDispatcherInterface
38+
* @var EventDispatcherInterface|null
4039
*/
4140
private $eventDispatcher;
4241

@@ -97,7 +96,7 @@ public function authenticate(TokenInterface $token)
9796

9897
if (null !== $result) {
9998
if (null !== $this->eventDispatcher) {
100-
$this->eventDispatcher->dispatch(AuthenticationEvents::AUTHENTICATION_SUCCESS_SENSITIVE, new AuthenticationSensitiveEvent($result, $token, $providerClassName));
99+
$this->eventDispatcher->dispatch(AuthenticationEvents::AUTHENTICATION_SUCCESS_SENSITIVE, new AuthenticationSensitiveEvent($token, $result, $providerClassName));
101100
}
102101

103102
if (true === $this->eraseCredentials) {

‎src/Symfony/Component/Security/Core/Event/AuthenticationEvent.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ public function __construct(TokenInterface $token)
2828
$this->authenticationToken = $token;
2929
}
3030

31-
public function getAuthenticationToken(): TokenInterface
31+
public function getAuthenticationToken()
3232
{
3333
return $this->authenticationToken;
3434
}

‎src/Symfony/Component/Security/Core/Event/AuthenticationSensitiveEvent.php

Lines changed: 39 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -25,10 +25,10 @@ class AuthenticationSensitiveEvent extends Event
2525
private $preAuthenticationToken;
2626
private $authenticationProviderClassName;
2727

28-
public function __construct(TokenInterface $authenticationToken, TokenInterface $preAuthenticationToken, string $authenticationProviderClassName = null)
28+
public function __construct(TokenInterface $preAuthenticationToken, TokenInterface $authenticationToken, ?string $authenticationProviderClassName = null)
2929
{
30-
$this->authenticationToken = $authenticationToken;
3130
$this->preAuthenticationToken = $preAuthenticationToken;
31+
$this->authenticationToken = $authenticationToken;
3232
$this->authenticationProviderClassName = $authenticationProviderClassName;
3333
}
3434

@@ -47,28 +47,52 @@ public function getAuthenticationProviderClassName(): ?string
4747
return $this->authenticationProviderClassName;
4848
}
4949

50+
/**
51+
* Attempts to extract the credentials password, first from the authentication token and second from the pre-
52+
* authentication token. It uses either a custom extractor closure (optionally passed as its only argument) or
53+
* the default extractor implementation, which returns the token's credentials string representation if it is
54+
* not an array, or if it is, searches for the index "password", "secret", or "api_key" (in that order) if the
55+
* token's credentials is an array.
56+
*
57+
* @param \Closure|null $extractor An optional custom token credentials password extraction \Closure that is
58+
* provided an auth token (as an instance of TokenInterface) and an auth event
59+
* (as an instance of AuthenticationSensitiveEvent). This closure is called
60+
* first with the final-auth token and second with the pre-auth token, returning
61+
* early if a non-null value (which must be string castable) is returned.
62+
*
63+
* @return null|string Either a credentials password/secret/auth_key is returned or null on extraction failure
64+
*/
5065
public function getAuthenticationTokenPassword(\Closure $extractor = null): ?string
5166
{
52-
return $this->extractAuthenticationTokenPassword($this->preAuthenticationToken, $extractor)
53-
?: $this->extractAuthenticationTokenPassword($this->authenticationToken, $extractor);
67+
$extractor = $extractor ?? function (TokenInterface $token): ?string {
68+
return $this->scalarCredentialsToString($credentials = $token->getCredentials())
69+
?: $this->arrayCredentialsToString($credentials);
70+
};
71+
72+
return $extractor($this->authenticationToken, $this)
73+
?: $extractor($this->preAuthenticationToken, $this);
5474
}
5575

56-
private function extractAuthenticationTokenPassword(TokenInterface $token, \Closure $extractor = null): ?string
76+
private function scalarCredentialsToString($credentials): ?string
5777
{
58-
return ($extractor ?? function (TokenInterface $token): ?string {
59-
$c = $token->getCredentials();
78+
if (is_scalar($credentials)) {
79+
return $credentials ?: null;
80+
}
6081

61-
return is_array($c) && (null !== $p = $this->resolveArrayAuthenticationTokenCredentialsPassword($c))
62-
? $p ?: null
63-
: $c ?: null;
64-
})($token, $this);
82+
if (is_object($credentials) && method_exists($credentials, '__toString')) {
83+
return $credentials->__toString() ?: null;
84+
}
85+
86+
return null;
6587
}
6688

67-
private function resolveArrayAuthenticationTokenCredentialsPassword(array $credentials): ?string
89+
private function arrayCredentialsToString($credentials): ?string
6890
{
69-
foreach (array('password', 'secret', 'api_key') as $index) {
70-
if (isset($credentials[$index]) && !empty($credentials[$index])) {
71-
return $credentials[$index];
91+
if (is_array($credentials)) {
92+
foreach (array('password', 'secret', 'api_key') as $index) {
93+
if (isset($credentials[$index]) && null !== $c = $this->scalarCredentialsToString($credentials[$index])) {
94+
return $c;
95+
}
7296
}
7397
}
7498

‎src/Symfony/Component/Security/Core/Tests/Authentication/AuthenticationProviderManagerTest.php

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
use Symfony\Component\EventDispatcher\EventDispatcher;
1616
use Symfony\Component\EventDispatcher\EventDispatcherInterface;
1717
use Symfony\Component\Security\Core\Authentication\AuthenticationProviderManager;
18+
use Symfony\Component\Security\Core\Authentication\Provider\AuthenticationProviderInterface;
1819
use Symfony\Component\Security\Core\AuthenticationEvents;
1920
use Symfony\Component\Security\Core\Event\AuthenticationEvent;
2021
use Symfony\Component\Security\Core\Event\AuthenticationFailureEvent;
@@ -168,7 +169,7 @@ public function testAuthenticateDispatchesAuthenticationSuccessEvents()
168169
->expects($this->exactly(2))
169170
->method('dispatch')
170171
->withConsecutive(array(
171-
AuthenticationEvents::AUTHENTICATION_SUCCESS_SENSITIVE, $this->equalTo(new AuthenticationSensitiveEvent($finalToken, $priorToken, $providerCN)),
172+
AuthenticationEvents::AUTHENTICATION_SUCCESS_SENSITIVE, $this->equalTo(new AuthenticationSensitiveEvent($priorToken, $finalToken, $providerCN)),
172173
), array(
173174
AuthenticationEvents::AUTHENTICATION_SUCCESS, $this->equalTo(new AuthenticationEvent($finalToken)),
174175
));
@@ -205,7 +206,7 @@ public function testAuthenticateDispatchesAuthenticationSuccessEventsWithCredent
205206

206207
protected function getAuthenticationProvider($supports, $token = null, $exception = null)
207208
{
208-
$provider = $this->getMockBuilder('Symfony\Component\Security\Core\Authentication\Provider\AuthenticationProviderInterface')->getMock();
209+
$provider = $this->getMockBuilder(AuthenticationProviderInterface::class)->getMock();
209210
$provider->expects($this->once())
210211
->method('supports')
211212
->will($this->returnValue($supports))

‎src/Symfony/Component/Security/Core/Tests/Event/AuthenticationSensitiveEventTest.php

Lines changed: 133 additions & 77 deletions
Original file line numberDiff line numberDiff line change
@@ -12,98 +12,154 @@
1212
namespace Symfony\Component\Security\Core\Tests\Authentication;
1313

1414
use PHPUnit\Framework\TestCase;
15-
use Symfony\Component\EventDispatcher\EventDispatcher;
16-
use Symfony\Component\EventDispatcher\EventDispatcherInterface;
17-
use Symfony\Component\Security\Core\Authentication\AuthenticationProviderManager;
15+
use Symfony\Component\Security\Core\Authentication\Provider\AuthenticationProviderInterface;
1816
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
19-
use Symfony\Component\Security\Core\AuthenticationEvents;
20-
use Symfony\Component\Security\Core\Event\AuthenticationEvent;
2117
use Symfony\Component\Security\Core\Event\AuthenticationSensitiveEvent;
22-
use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken;
2318

2419
class AuthenticationSensitiveEventTest extends TestCase
2520
{
26-
public function testAuthenticateDispatchesAuthenticationSuccessEvents()
21+
public static function provideTestAccessorMethodsData(): \Iterator
2722
{
28-
$finalToken = new UsernamePasswordToken('foo', 'bar', 'baz', array('role-01', 'role-02'));
29-
$priorToken = new UsernamePasswordToken('foo', 'bar', 'baz');
30-
31-
$provider = $this->getAuthenticationProvider(true, $finalToken);
32-
$providerCN = get_class($provider);
33-
34-
$dispatcher = $this->getMockBuilder(EventDispatcherInterface::class)->getMock();
35-
$dispatcher
36-
->expects($this->exactly(2))
37-
->method('dispatch')
38-
->withConsecutive(array(
39-
AuthenticationEvents::AUTHENTICATION_SUCCESS_SENSITIVE, $this->equalTo(new AuthenticationSensitiveEvent($finalToken, $priorToken, $providerCN)),
40-
), array(
41-
AuthenticationEvents::AUTHENTICATION_SUCCESS, $this->equalTo(new AuthenticationEvent($finalToken)),
42-
));
43-
44-
$manager = new AuthenticationProviderManager(array($provider));
45-
$manager->setEventDispatcher($dispatcher);
46-
47-
$this->assertSame($finalToken, $manager->authenticate($priorToken));
23+
$getExtractorFunction = function ($default = null) {
24+
return function ($token, $event) use ($default): ?string {
25+
self::assertInstanceOf(TokenInterface::class, $token);
26+
self::assertInstanceOf(AuthenticationSensitiveEvent::class, $event);
27+
28+
if (method_exists($token, 'getCredentials')) {
29+
$c = $token->getCredentials();
30+
31+
if ($c instanceof \Closure) {
32+
return ($c)();
33+
}
34+
35+
if (method_exists($c, 'getInnerCredentials')) {
36+
return $c->getInnerCredentials();
37+
}
38+
}
39+
40+
return $c ?? $default;
41+
};
42+
};
43+
44+
$getHasCastableClass = function ($return = null) {
45+
return new class($return) {
46+
private $inner;
47+
48+
public function __construct(?string $return = null)
49+
{
50+
$this->return = $return;
51+
}
52+
53+
public function __toString(): string
54+
{
55+
return $this->return ?? '';
56+
}
57+
};
58+
};
59+
60+
$getNotCastableClass = function ($return = null) {
61+
return new class($return) {
62+
private $return;
63+
64+
public function __construct(?string $return = null)
65+
{
66+
$this->return = $return;
67+
}
68+
69+
public function getInnerCredentials(): string
70+
{
71+
return $this->return ?? '';
72+
}
73+
};
74+
};
75+
76+
$getTokenCredentialsValue = function ($return = null) {
77+
return function () use ($return) {
78+
return $return;
79+
};
80+
};
81+
82+
// expects credential password of "null" type
83+
yield array(null);
84+
yield array(null, $getHasCastableClass(''));
85+
yield array(null, $getNotCastableClass('foo'));
86+
yield array(null, array('unknown-index-foo' => 'foo'));
87+
yield array(null, null, $getHasCastableClass(''));
88+
yield array(null, null, $getNotCastableClass('foo'));
89+
yield array(null, null, array('unknown-index-bar' => 'bar'));
90+
yield array(null, null, null, $getExtractorFunction(null));
91+
92+
// expects credential password of "foo" value
93+
yield array('foo', 'foo');
94+
yield array('foo', 'foo', 'bar');
95+
yield array('foo', $getHasCastableClass('foo'));
96+
yield array('foo', $getNotCastableClass('foo'), null, $getExtractorFunction());
97+
yield array('foo', $getTokenCredentialsValue('foo'), null, $getExtractorFunction());
98+
99+
// expects credential password of "bar" value
100+
yield array('bar', null, 'bar');
101+
yield array('bar', null, $getHasCastableClass('bar'));
102+
yield array('bar', null, $getNotCastableClass('bar'), $getExtractorFunction());
103+
yield array('bar', null, $getTokenCredentialsValue('bar'), $getExtractorFunction());
104+
105+
// expects credential password of "baz" value
106+
yield array('baz', null, null, $getExtractorFunction('baz'));
107+
108+
// expects array value will be extracted for all supported indexes
109+
foreach (array('password', 'secret', 'api_key') as $index) {
110+
// expects credential password of "null" type
111+
yield array(null, array($index => null));
112+
yield array(null, null, array($index => ''));
113+
yield array(null, array($index => ''), array($index => null));
114+
115+
// expects credential password of "foo" value
116+
yield array('foo', array($index => 'foo'));
117+
yield array('foo', array($index => 'foo'), array($index => null));
118+
yield array('foo', array($index => 'foo'), array($index => ''));
119+
yield array('foo', array($index => 'foo'), array('unknown-index-bar' => 'bar'));
120+
yield array('foo', array($index => 'foo'), array($index => 'bar'));
121+
122+
// expects credential password of "bar" value
123+
yield array('bar', null, array($index => 'bar'));
124+
yield array('bar', array($index => null), array($index => 'bar'));
125+
yield array('bar', array($index => ''), array($index => 'bar'));
126+
yield array('bar', array('unknown-index-foo' => 'foo'), array($index => 'bar'));
127+
yield array('bar', array($index => $getNotCastableClass), array($index => 'bar'));
128+
}
48129
}
49130

50-
public function testAuthenticateDispatchesAuthenticationSuccessEventsWithCredentialsAvailableAndRemovedForSuccessiveDispatches()
131+
/**
132+
* @dataProvider provideTestAccessorMethodsData
133+
*
134+
* @param string $expectedPassword
135+
* @param string|array|null $finalCredentials
136+
* @param string|array|null $priorCredentials
137+
* @param \Closure|null $passwordExtractor
138+
*/
139+
public function testAccessorMethods(string $expectedPassword = null, $finalCredentials = null, $priorCredentials = null, \Closure $passwordExtractor = null)
51140
{
52-
$finalToken = $this->getTokenInterfaceMock();
53-
$priorToken = $this->getTokenInterfaceMock($credentials = array('password' => 'foobar'));
54-
55-
$provider = $this->getAuthenticationProvider(true, $finalToken);
56-
$providerCN = get_class($provider);
57-
58-
$dispatcher = new EventDispatcher();
59-
$dispatcher->addListener(AuthenticationEvents::AUTHENTICATION_SUCCESS_SENSITIVE, function (AuthenticationSensitiveEvent $event) use ($credentials, $providerCN) {
60-
$this->assertSame($providerCN, $event->getAuthenticationProviderClassName());
61-
$this->assertSame('foobar', $event->getAuthenticationTokenPassword());
62-
$this->assertEquals($credentials, $event->getPreAuthenticationToken()->getCredentials());
63-
});
64-
$dispatcher->addListener(AuthenticationEvents::AUTHENTICATION_SUCCESS, function (AuthenticationEvent $event) {
65-
$this->assertEquals('', $event->getAuthenticationToken()->getCredentials());
66-
});
67-
68-
$manager = new AuthenticationProviderManager(array($provider));
69-
$manager->setEventDispatcher($dispatcher);
70-
$manager->authenticate($priorToken);
141+
$event = new AuthenticationSensitiveEvent(
142+
$priorToken = $this->getTokenInterfaceMock($priorCredentials),
143+
$finalToken = $this->getTokenInterfaceMock($finalCredentials),
144+
AuthenticationProviderInterface::class
145+
);
146+
147+
$this->assertSame($priorToken, $event->getPreAuthenticationToken());
148+
$this->assertSame($finalToken, $event->getAuthenticationToken());
149+
$this->assertSame(AuthenticationProviderInterface::class, $event->getAuthenticationProviderClassName());
150+
$this->assertSame($expectedPassword, $event->getAuthenticationTokenPassword($passwordExtractor));
71151
}
72152

73153
private function getTokenInterfaceMock($credentials = null): TokenInterface
74154
{
75-
$token = $this->getMockBuilder(TokenInterface::class)->getMock();
155+
$token = $this
156+
->getMockBuilder(TokenInterface::class)
157+
->getMock();
76158

77-
if (null !== $credentials) {
78-
$token
79-
->expects($this->atLeastOnce())
80-
->method('getCredentials')
81-
->will($this->returnValue($credentials));
82-
}
159+
$token->expects($this->any())
160+
->method('getCredentials')
161+
->will($this->returnValue($credentials));
83162

84163
return $token;
85164
}
86-
87-
private function getAuthenticationProvider($supports, $token = null, $exception = null)
88-
{
89-
$provider = $this->getMockBuilder('Symfony\Component\Security\Core\Authentication\Provider\AuthenticationProviderInterface')->getMock();
90-
$provider->expects($this->once())
91-
->method('supports')
92-
->will($this->returnValue($supports))
93-
;
94-
95-
if (null !== $token) {
96-
$provider->expects($this->once())
97-
->method('authenticate')
98-
->will($this->returnValue($token))
99-
;
100-
} elseif (null !== $exception) {
101-
$provider->expects($this->once())
102-
->method('authenticate')
103-
->will($this->throwException($this->getMockBuilder($exception)->setMethods(null)->getMock()))
104-
;
105-
}
106-
107-
return $provider;
108-
}
109165
}

0 commit comments

Comments
 (0)