Skip to content

Commit fc2f619

Browse files
perajovicperajovic
committed
Improve Authentication class
It has better guards and it is simplified a bit.
1 parent e69dd57 commit fc2f619

File tree

5 files changed

+50
-127
lines changed

5 files changed

+50
-127
lines changed

‎src/AuthenticatedParameters.php

-43
This file was deleted.

‎src/Authentication.php

+20-36
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
use ActiveCollab\Authentication\Adapter\AdapterInterface;
1212
use ActiveCollab\Authentication\Authorizer\AuthorizerInterface;
1313
use ActiveCollab\Authentication\Exception\InvalidAuthenticationRequestException;
14-
use ActiveCollab\Authentication\Exception\InvalidCredentialsException;
14+
use Exception;
1515
use Psr\Http\Message\RequestInterface;
1616
use RuntimeException;
1717

@@ -23,15 +23,9 @@ class Authentication implements AuthenticationInterface
2323
private $adapters;
2424

2525
/**
26-
* @var AuthorizerInterface
26+
* @param array $adapters
2727
*/
28-
private $authorizer;
29-
30-
/**
31-
* @param array $adapters
32-
* @param AuthorizerInterface|null $authorizer
33-
*/
34-
public function __construct(array $adapters, AuthorizerInterface $authorizer = null)
28+
public function __construct(array $adapters)
3529
{
3630
foreach ($adapters as $adapter) {
3731
if (!($adapter instanceof AdapterInterface)) {
@@ -40,15 +34,6 @@ public function __construct(array $adapters, AuthorizerInterface $authorizer = n
4034
}
4135

4236
$this->adapters = $adapters;
43-
$this->authorizer = $authorizer;
44-
}
45-
46-
/**
47-
* {@inheritdoc}
48-
*/
49-
public function setAuthorizer(AuthorizerInterface $authorizer)
50-
{
51-
$this->authorizer = $authorizer;
5237
}
5338

5439
/**
@@ -57,45 +42,44 @@ public function setAuthorizer(AuthorizerInterface $authorizer)
5742
public function initialize(RequestInterface $request)
5843
{
5944
$exception = null;
60-
$results = ['authenticated_parameters' => []];
45+
$results = ['authenticated_user' => [], 'authentication_result' => []];
6146

6247
foreach ($this->adapters as $adapter) {
6348
try {
6449
$result = $adapter->initialize($request);
65-
if ($result instanceof AuthenticatedParameters) {
66-
$results['authenticated_parameters'][] = $result;
50+
if ($result) {
51+
$results['authenticated_user'][] = $result['authenticated_user'];
52+
$results['authentication_result'][] = $result['authentication_result'];
6753
}
6854
} catch (Exception $e) {
6955
$exception = $e;
7056
}
7157
}
7258

73-
if (empty($results['authenticated_parameters']) && $exception) {
74-
throw $exception;
59+
if (empty($results['authenticated_user'])) {
60+
if ($exception) {
61+
throw $exception;
62+
}
63+
64+
return $request;
7565
}
7666

77-
if (count($results['authenticated_parameters']) > 1) {
67+
if (count($results['authenticated_user']) > 1) {
7868
throw new InvalidAuthenticationRequestException('You can not be authenticated with more than one authentication method');
7969
}
8070

81-
return $request->withAttribute('authenticated_parameters', $results['authenticated_parameters'][0]);
71+
return $request
72+
->withAttribute('authenticated_user', $results['authenticated_user'][0])
73+
->withAttribute('authentication_result', $results['authentication_result'][0]);
8274
}
8375

8476
/**
8577
* {@inheritdoc}
8678
*/
87-
public function authorize(RequestInterface $request, array $credentials = [])
79+
public function authorize(AuthorizerInterface $authorizer, AdapterInterface $adapter, array $credentials)
8880
{
89-
if (!$this->authorizer) {
90-
throw new RuntimeException('Authorizer object is not configured');
91-
}
92-
93-
if (!$this->authorizer->verifyCredentials($credentials)) {
94-
throw new InvalidCredentialsException();
95-
}
96-
97-
$authenticated_parameters = $request->getAttribute('authenticated_parameters');
81+
$user = $authorizer->verifyCredentials($credentials);
9882

99-
return $authenticated_parameters->adapter->authenticate($authenticated_parameters->authenticated_user);
83+
return $adapter->authenticate($user);
10084
}
10185
}

‎src/AuthenticationInterface.php

+5-10
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88

99
namespace ActiveCollab\Authentication;
1010

11+
use ActiveCollab\Authentication\Adapter\AdapterInterface;
1112
use ActiveCollab\Authentication\AuthenticatedUser\AuthenticatedUserInterface;
1213
use ActiveCollab\Authentication\Authorizer\AuthorizerInterface;
1314
use Psr\Http\Message\RequestInterface;
@@ -26,18 +27,12 @@ interface AuthenticationInterface
2627
public function initialize(RequestInterface $request);
2728

2829
/**
29-
* Set Authorizer object.
30+
* Authorize and authenticate with given credentials against authorization/authentication source.
3031
*
31-
* @param AuthorizerInterface $authorizer
32-
*/
33-
public function setAuthorizer(AuthorizerInterface $authorizer);
34-
35-
/**
36-
* Authenticate with given credential agains authentication source.
37-
*
38-
* @param RequestInterface $request
32+
* @param AuthorizerInterface $authorizer
33+
* @param AdapterInterface $adapter
3934
* @param array $credentials
4035
* @return AuthenticatedUserInterface
4136
*/
42-
public function authorize(RequestInterface $request, array $credentials = []);
37+
public function authorize(AuthorizerInterface $authorizer, AdapterInterface $adapter, array $credentials);
4338
}

‎test/src/AuthenticationTest.php

+13-35
Original file line numberDiff line numberDiff line change
@@ -55,14 +55,13 @@ public function setUp()
5555
{
5656
parent::setUp();
5757

58-
$this->authorizer = new Authorizer();
59-
$this->user_repository = new UserRepository([
60-
'[email protected]' => new AuthenticatedUser(1, '[email protected]', 'John Doe', '123'),
61-
]);
58+
$authenticated_user = new AuthenticatedUser(1, '[email protected]', 'John Doe', '123');
59+
$this->authorizer = new Authorizer($authenticated_user);
60+
$this->user_repository = new UserRepository(['[email protected]' => $authenticated_user]);
6261
$this->empty_user_repository = new UserRepository();
6362
$this->token_repository = new TokenRepository([
64-
'123' => new Token(123, '[email protected]'), ]
65-
);
63+
'123' => new Token(123, '[email protected]'),
64+
]);
6665
$this->empty_token_repository = new TokenRepository();
6766
$this->authenticated_user = new AuthenticatedUser(1, '[email protected]', 'John Doe', '123');
6867
$this->request = $this->request->withHeader('Authorization', 'Bearer 123');
@@ -74,21 +73,14 @@ public function setUp()
7473
*/
7574
public function testForInvalidAdapterExceptionIsThrown()
7675
{
77-
new Authentication([new stdClass()], $this->authorizer);
76+
new Authentication([new stdClass()]);
7877
}
7978

80-
/**
81-
* @expectedException ActiveCollab\Authentication\Exception\InvalidCredentialsException
82-
* @expectedExceptionMessage Invalid credentials provided
83-
*/
84-
public function testForInvalidCredentialsExceptionIsThrown()
79+
public function testAdaptersNotInitializedReturnsRequest()
8580
{
86-
$authentication = new Authentication(
87-
[new TokenBearer($this->user_repository, $this->token_repository)],
88-
$this->authorizer
89-
);
81+
$request = (new Authentication([]))->initialize($this->request);
9082

91-
$authentication->authorize($this->request, ['username' => '[email protected]']);
83+
$this->assertSame($request, $this->request);
9284
}
9385

9486
/**
@@ -99,7 +91,7 @@ public function testFailedAdapterInitializationThrowsException()
9991
{
10092
$token_bearer = new TokenBearer($this->empty_user_repository, $this->empty_token_repository);
10193

102-
(new Authentication([$token_bearer], $this->authorizer))->initialize($this->request);
94+
(new Authentication([$token_bearer]))->initialize($this->request);
10395
}
10496

10597
/**
@@ -116,27 +108,13 @@ public function testMultipleAdapterSuccessfullyInitializedThrowsException()
116108
$authentication->initialize($this->request);
117109
}
118110

119-
/**
120-
* @expectedException RuntimeException
121-
* @expectedExceptionMessage Authorizer object is not configured
122-
*/
123-
public function testForNotConfiguredAuthorizerExceptionIsThrown()
124-
{
125-
$authentication = new Authentication([new TokenBearer($this->user_repository, $this->token_repository)], null);
126-
127-
$authentication->authorize($this->request, ['username' => '[email protected]']);
128-
}
129-
130111
public function testUserIsAuthorized()
131112
{
132-
$authentication = new Authentication(
133-
[new TokenBearer($this->user_repository, $this->token_repository)],
134-
$this->authorizer
135-
);
113+
$token_bearer = new TokenBearer($this->user_repository, $this->token_repository);
136114

115+
$authentication = new Authentication([$token_bearer]);
137116
$request = $authentication->initialize($this->request);
138-
139-
$authentication_result = $authentication->authorize($request, ['username' => '[email protected]']);
117+
$authentication_result = $authentication->authorize($this->authorizer, $token_bearer, ['username' => '[email protected]']);
140118

141119
$this->assertInstanceOf(AuthenticationResultInterface::class, $authentication_result);
142120
}

‎test/src/Authorizer/Authorizer.php

+12-3
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
namespace ActiveCollab\Authentication\Test\Authorizer;
1010

1111
use ActiveCollab\Authentication\Authorizer\AuthorizerInterface;
12+
use ActiveCollab\Authentication\Test\AuthenticatedUser\AuthenticatedUser;
1213

1314
/**
1415
* @package ActiveCollab\Authentication\Test\Authorizer
@@ -17,11 +18,19 @@ class Authorizer implements AuthorizerInterface
1718
{
1819
private $username = '[email protected]';
1920

21+
/**
22+
* @param AuthenticatedUser $authenticated_user
23+
*/
24+
public function __construct(AuthenticatedUser $authenticated_user)
25+
{
26+
$this->authenticated_user = $authenticated_user;
27+
}
28+
2029
public function verifyCredentials(array $payload)
2130
{
22-
return isset($payload['username']) && $payload['username'] === $this->username
23-
? true
24-
: false;
31+
if (isset($payload['username']) && $payload['username'] === $this->username) {
32+
return $this->authenticated_user;
33+
}
2534
}
2635

2736
public function onLogin(array $payload)

0 commit comments

Comments
 (0)