Skip to content
This repository was archived by the owner on Jan 29, 2020. It is now read-only.

Fix PHP Warning if no colon is present in decoded basic-auth payload #9

Merged
merged 19 commits into from Jul 25, 2018
Merged
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -2,6 +2,32 @@

All notable changes to this project will be documented in this file, in reverse chronological order by release.

## 0.3.1 - TBD

### Added

- Nothing.

### Changed

- Nothing.

### Deprecated

- Nothing.

### Removed

- Nothing

### Fixed

- [#9](https://github.com/zendframework/zend-expressive-authentication-basic/pull/9)
If the decoded authentication string did contain a colon, a PHP Warning was thrown in PHP 7.2

- [#9](https://github.com/zendframework/zend-expressive-authentication-basic/pull/9)
It was not possible to use passwords that contained colons

## 0.3.0 - 2018-03-15

### Added
27 changes: 23 additions & 4 deletions src/BasicAccess.php
Original file line number Diff line number Diff line change
@@ -52,16 +52,35 @@ public function __construct(

public function authenticate(ServerRequestInterface $request) : ?UserInterface
{
$authHeader = $request->getHeader('Authorization');
if (empty($authHeader)) {
$authHeaders = $request->getHeader('Authorization');

if (1 !== count($authHeaders)) {
return null;
}

$authHeader = array_shift($authHeaders);

if (! preg_match('/Basic (?P<credentials>.+)/', $authHeader, $match)) {
return null;
}

$decodedCredentials = base64_decode($match['credentials'], true);

if (false === $decodedCredentials) {
return null;
}

$credentialParts = explode(':', $decodedCredentials, 2);

if (false === $credentialParts) {
return null;
}

if (! preg_match('/Basic (?P<credentials>[a-zA-Z0-9\+\/\=]+)/', $authHeader[0], $match)) {
if (2 !== count($credentialParts)) {
return null;
}

[$username, $password] = explode(':', base64_decode($match['credentials']));
[$username, $password] = $credentialParts;

return $this->repository->authenticate($username, $password);
}
86 changes: 64 additions & 22 deletions test/BasicAccessTest.php
Original file line number Diff line number Diff line change
@@ -56,49 +56,47 @@ public function testConstructor()
$this->assertInstanceOf(AuthenticationInterface::class, $basicAccess);
}

public function testIsAuthenticatedWithoutHeader()
{
$this->request
->getHeader('Authorization')
->willReturn([]);

$basicAccess = new BasicAccess(
$this->userRepository->reveal(),
'test',
$this->responseFactory
);
$this->assertNull($basicAccess->authenticate($this->request->reveal()));
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a note for anybody else reviewing: this test becomes redundant, as it is covered by the empty set of the provideInvalidAuthenticationHeader provider used by the new testIsAuthenticatedWithInvalidData test case.


public function testIsAuthenticatedWithoutBasic()
/**
* @param array $authHeader
* @dataProvider provideInvalidAuthenticationHeader
*/
public function testIsAuthenticatedWithInvalidData(array $authHeader)
{
$this->request
->getHeader('Authorization')
->willReturn(['foo']);
->willReturn($authHeader);

$this->userRepository->authenticate(Argument::any(), Argument::any())->shouldNotBeCalled();

$basicAccess = new BasicAccess(
$this->userRepository->reveal(),
'test',
$this->responseFactory
);

$this->assertNull($basicAccess->authenticate($this->request->reveal()));
}

public function testIsAuthenticatedWithValidCredential()
/**
* @param string $username
* @param string $password
* @param array $authHeader
* @dataProvider provideValidAuthentication
*/
public function testIsAuthenticatedWithValidCredential(string $username, string $password, array $authHeader)
{
$this->request
->getHeader('Authorization')
->willReturn(['Basic QWxhZGRpbjpPcGVuU2VzYW1l']);
->willReturn($authHeader);
$this->request
->withAttribute(UserInterface::class, Argument::type(UserInterface::class))
->willReturn($this->request->reveal());

$this->authenticatedUser
->getIdentity()
->willReturn('Aladdin');
->willReturn($username);
$this->userRepository
->authenticate('Aladdin', 'OpenSesame')
->authenticate($username, $password)
->willReturn($this->authenticatedUser->reveal());

$basicAccess = new BasicAccess(
@@ -109,7 +107,6 @@ public function testIsAuthenticatedWithValidCredential()

$user = $basicAccess->authenticate($this->request->reveal());
$this->assertInstanceOf(UserInterface::class, $user);
$this->assertEquals('Aladdin', $user->getIdentity());
}

public function testIsAuthenticatedWithNoCredential()
@@ -151,7 +148,52 @@ public function testGetUnauthenticatedResponse()

$response = $basicAccess->unauthorizedResponse($this->request->reveal());

$this->assertInstanceOf(ResponseInterface::class, $response);
$this->assertEquals(['Basic realm="test"'], $response->getHeader('WWW-Authenticate'));
}

public function provideInvalidAuthenticationHeader(): array
{
return [
'empty-header' => [[]],
'missing-basic-prefix' => [['foo']],
'only-username-without-colon' => [['Basic ' . base64_encode('Aladdin')]],
'base64-encoded-pile-of-poo-emoji' => [['Basic ' . base64_encode('💩')]],
'pile-of-poo-emoji' => [['Basic 💩']],
'only-pile-of-poo-emoji' => [['💩']],
'basic-prefix-without-content' => [['Basic ']],
'only-basic' => [['Basic']],
'multiple-auth-headers' => [
[
['Basic ' . base64_encode('Aladdin:OpenSesame')],
['Basic ' . base64_encode('Aladdin:OpenSesame')],
],
],
];
}

public function provideValidAuthentication(): array
{
return [
'aladdin' => ['Aladdin', 'OpenSesame', ['Basic ' . base64_encode('Aladdin:OpenSesame')]],
'aladdin-with-nonzero-array-index' => [
'Aladdin',
'OpenSesame',
[-200 => 'Basic ' . base64_encode('Aladdin:OpenSesame')]
],
'passwords-with-colon' => ['Aladdin', 'Open:Sesame', ['Basic ' . base64_encode('Aladdin:Open:Sesame')]],
'username-without-password' => ['Aladdin', '', ['Basic ' . base64_encode('Aladdin:')]],
'password-without-username' => ['', 'OpenSesame', ['Basic ' . base64_encode(':OpenSesame')]],
'passwords-with-multiple-colons' => [
'Aladdin',
'::Open:::Sesame::',
['Basic ' . base64_encode('Aladdin:::Open:::Sesame::')]
],
'no-username-or-password' => ['', '', ['Basic ' . base64_encode(':')]],
'no-username-password-only-colons' => ['', '::::::', ['Basic ' . base64_encode(':::::::')]],
'unicode-username-and-password' => [
'thumbsup-emoji-👍',
'thumbsdown-emoji-👎',
['Basic ' . base64_encode('thumbsup-emoji-👍:thumbsdown-emoji-👎')]],
];
}
}