Skip to content

Commit 2ca5bb1

Browse files
OcramiusOcramius
committed
Merge branch 'hotfix/expired-token-auth'
Prevents invalid tokens from being used for authentication.
2 parents 51a7e2b + 03bd5f1 commit 2ca5bb1

File tree

6 files changed

+272
-60
lines changed

6 files changed

+272
-60
lines changed

‎composer.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,9 @@
2727
"zfr/zfr-oauth2-server": "0.1.*"
2828
},
2929
"require-dev": {
30-
"phpunit/phpunit": "~3.7",
30+
"phpunit/phpunit": "~4.0",
3131
"squizlabs/php_codesniffer": "1.4.*",
32+
"zendframework/zend-view": "~2.2",
3233
"satooshi/php-coveralls": "~0.6"
3334
},
3435
"autoload": {

‎src/ZfrOAuth2Module/Server/Authentication/Storage/AccessTokenStorage.php

Lines changed: 31 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,8 @@
1919
namespace ZfrOAuth2Module\Server\Authentication\Storage;
2020

2121
use Zend\Authentication\Storage\NonPersistent;
22-
use Zend\Http\Request as HttpRequest;
22+
use Zend\Http\Request;
23+
use Zend\Mvc\Application;
2324
use ZfrOAuth2\Server\ResourceServer;
2425

2526
/**
@@ -34,42 +35,57 @@ class AccessTokenStorage extends NonPersistent
3435
protected $resourceServer;
3536

3637
/**
37-
* @var HttpRequest
38+
* @var Application
3839
*/
39-
protected $request;
40+
private $application;
4041

4142
/**
4243
* @param ResourceServer $resourceServer
44+
* @param Application $application
4345
*/
44-
public function __construct(ResourceServer $resourceServer)
46+
public function __construct(ResourceServer $resourceServer, Application $application)
4547
{
4648
$this->resourceServer = $resourceServer;
49+
$this->application = $application;
4750
}
4851

4952
/**
50-
* Set the HTTP request
51-
*
52-
* @param HttpRequest $request
53-
* @return void
53+
* {@inheritDoc}
5454
*/
55-
public function setRequest(HttpRequest $request)
55+
public function isEmpty()
5656
{
57-
$this->request = $request;
57+
$request = $this->getCurrentRequest();
58+
59+
return $request ? $this->resourceServer->getAccessToken($request) === null : true;
5860
}
5961

6062
/**
6163
* {@inheritDoc}
6264
*/
63-
public function isEmpty()
65+
public function read()
6466
{
65-
return $this->resourceServer->getAccessToken($this->request) === null;
67+
$request = $this->getCurrentRequest();
68+
69+
if (! $request) {
70+
return null;
71+
}
72+
73+
$accessToken = $this->resourceServer->getAccessToken($request);
74+
75+
return $accessToken ? $accessToken->getOwner() : null;
6676
}
6777

6878
/**
69-
* {@inheritDoc}
79+
* @return Request|null
7080
*/
71-
public function read()
81+
private function getCurrentRequest()
7282
{
73-
return $this->resourceServer->getAccessToken($this->request)->getOwner();
83+
$request = $this->application->getMvcEvent()->getRequest();
84+
85+
if (! $request instanceof Request || ! $this->resourceServer->isRequestValid($request)) {
86+
return null;
87+
}
88+
89+
return $request;
7490
}
7591
}

‎src/ZfrOAuth2Module/Server/Factory/AccessTokenStorageFactory.php

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -34,15 +34,11 @@ class AccessTokenStorageFactory implements FactoryInterface
3434
*/
3535
public function createService(ServiceLocatorInterface $serviceLocator)
3636
{
37-
$accessTokenStorage = new AccessTokenStorage($serviceLocator->get('ZfrOAuth2\Server\ResourceServer'));
37+
/* @var $resourceServer \ZfrOAuth2\Server\ResourceServer */
38+
$resourceServer = $serviceLocator->get('ZfrOAuth2\Server\ResourceServer');
39+
/* @var $application \Zend\Mvc\Application */
40+
$application = $serviceLocator->get('Application');
3841

39-
// It only makes sense to set the request if it is HTTP request
40-
$request = $serviceLocator->get('Application')->getRequest();
41-
42-
if ($request instanceof HttpRequest) {
43-
$accessTokenStorage->setRequest($request);
44-
}
45-
46-
return $accessTokenStorage;
42+
return new AccessTokenStorage($resourceServer, $application);
4743
}
4844
}

‎tests/ZfrOAuth2ModuleTest/Server/Authentication/AuthenticationFunctionalTest.php

Lines changed: 172 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,172 @@
1+
<?php
2+
/*
3+
* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
4+
* "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
5+
* LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
6+
* A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
7+
* OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
8+
* SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
9+
* LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
10+
* DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
11+
* THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
12+
* (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
13+
* OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
14+
*
15+
* This software consists of voluntary contributions made by many individuals
16+
* and is licensed under the MIT license.
17+
*/
18+
19+
namespace ZfrOAuth2ModuleTest\Server\Authentication\Adapter;
20+
21+
use PHPUnit_Framework_TestCase;
22+
use Zend\Authentication\AuthenticationService;
23+
use Zend\Http\Request as HttpRequest;
24+
use ZfrOAuth2\Server\Entity\AccessToken;
25+
use ZfrOAuth2\Server\Exception\OAuth2Exception;
26+
use ZfrOAuth2Module\Server\Authentication\Storage\AccessTokenStorage;
27+
28+
/**
29+
* @author Marco Pivetta <[email protected]>
30+
* @licence MIT
31+
*
32+
* @coversNothing
33+
*/
34+
class AuthenticationFunctionalTest extends PHPUnit_Framework_TestCase
35+
{
36+
/**
37+
* @var \ZfrOAuth2\Server\ResourceServer|\PHPUnit_Framework_MockObject_MockObject
38+
*/
39+
private $resourceServer;
40+
41+
/**
42+
* @var AccessTokenStorage
43+
*/
44+
private $authenticationStorage;
45+
46+
/**
47+
* @var AuthenticationService
48+
*/
49+
private $authenticationService;
50+
51+
/**
52+
* @var \Zend\Mvc\MvcEvent|\PHPUnit_Framework_MockObject_MockObject
53+
*/
54+
private $mvcEvent;
55+
56+
/**
57+
* {@inheritDoc}
58+
*/
59+
protected function setUp()
60+
{
61+
$this->mvcEvent = $this->getMock('Zend\Mvc\MvcEvent');
62+
$application = $this->getMock('Zend\Mvc\Application', [], [], '', false);
63+
$this->resourceServer = $this->getMock('ZfrOAuth2\Server\ResourceServer', [], [], '', false);
64+
$this->authenticationStorage = new AccessTokenStorage($this->resourceServer, $application);
65+
$this->authenticationService = new AuthenticationService($this->authenticationStorage);
66+
67+
$application->expects($this->any())->method('getMvcEvent')->will($this->returnValue($this->mvcEvent));
68+
}
69+
70+
public function testSuccessAuthenticationOnValidToken()
71+
{
72+
$request = new HttpRequest();
73+
74+
$this->mvcEvent->expects($this->any())->method('getRequest')->will($this->returnValue($request));
75+
76+
$token = new AccessToken();
77+
$owner = $this->getMock('ZfrOAuth2\Server\Entity\TokenOwnerInterface');
78+
$token->setOwner($owner);
79+
80+
$this
81+
->resourceServer
82+
->expects($this->atLeastOnce())
83+
->method('isRequestValid')
84+
->with($request)
85+
->will($this->returnValue(true));
86+
87+
$this
88+
->resourceServer
89+
->expects($this->atLeastOnce())
90+
->method('getAccessToken')
91+
->with($request)
92+
->will($this->returnValue($token));
93+
94+
95+
$this->assertTrue($this->authenticationService->hasIdentity());
96+
$this->assertSame($owner, $this->authenticationService->getIdentity());
97+
}
98+
99+
public function testFailAuthenticationOnNoToken()
100+
{
101+
$request = new HttpRequest();
102+
103+
$this->mvcEvent->expects($this->any())->method('getRequest')->will($this->returnValue($request));
104+
105+
$token = new AccessToken();
106+
$owner = $this->getMock('ZfrOAuth2\Server\Entity\TokenOwnerInterface');
107+
$token->setOwner($owner);
108+
109+
$this
110+
->resourceServer
111+
->expects($this->atLeastOnce())
112+
->method('isRequestValid')
113+
->with($request)
114+
->will($this->returnValue(false));
115+
116+
$this->resourceServer->expects($this->never())->method('getAccessToken');
117+
118+
$this->assertFalse($this->authenticationService->hasIdentity());
119+
$this->assertNull($this->authenticationService->getIdentity());
120+
}
121+
122+
public function testFailAuthenticationOnExpiredToken()
123+
{
124+
$request = new HttpRequest();
125+
126+
$this->mvcEvent->expects($this->any())->method('getRequest')->will($this->returnValue($request));
127+
128+
$token = new AccessToken();
129+
$owner = $this->getMock('ZfrOAuth2\Server\Entity\TokenOwnerInterface');
130+
$token->setOwner($owner);
131+
132+
$this
133+
->resourceServer
134+
->expects($this->atLeastOnce())
135+
->method('isRequestValid')
136+
->with($request)
137+
->will($this->returnValue(true));
138+
139+
$this
140+
->resourceServer
141+
->expects($this->atLeastOnce())
142+
->method('getAccessToken')
143+
->with($request)
144+
->will($this->throwException(new OAuth2Exception('Expired token', 123)));
145+
146+
$this->setExpectedException('ZfrOAuth2\Server\Exception\OAuth2Exception', 'Expired token', 123);
147+
148+
$this->authenticationService->getIdentity();
149+
}
150+
151+
public function testFailAuthenticationOnNoRequest()
152+
{
153+
$this->resourceServer->expects($this->never())->method('isRequestValid');
154+
$this->resourceServer->expects($this->never())->method('getAccessToken');
155+
156+
$this->assertFalse($this->authenticationService->hasIdentity());
157+
$this->assertNull($this->authenticationService->getIdentity());
158+
}
159+
160+
public function testFailAuthenticationOnNonHttpRequest()
161+
{
162+
$request = $this->getMock('Zend\Stdlib\RequestInterface');
163+
164+
$this->mvcEvent->expects($this->any())->method('getRequest')->will($this->returnValue($request));
165+
166+
$this->resourceServer->expects($this->never())->method('isRequestValid');
167+
$this->resourceServer->expects($this->never())->method('getAccessToken');
168+
169+
$this->assertFalse($this->authenticationService->hasIdentity());
170+
$this->assertNull($this->authenticationService->getIdentity());
171+
}
172+
}

‎tests/ZfrOAuth2ModuleTest/Server/Authentication/Storage/AccessTokenStorageTest.php

Lines changed: 53 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,8 @@
1818

1919
namespace ZfrOAuth2ModuleTest\Server\Authentication\Storage;
2020

21-
use Zend\Authentication\Result;
2221
use Zend\Http\Request as HttpRequest;
22+
use Zend\Mvc\MvcEvent;
2323
use ZfrOAuth2\Server\Entity\AccessToken;
2424
use ZfrOAuth2Module\Server\Authentication\Storage\AccessTokenStorage;
2525

@@ -31,39 +31,70 @@
3131
*/
3232
class AccessTokenStorageTest extends \PHPUnit_Framework_TestCase
3333
{
34-
public function testIsConsideredAsEmptyIfNoAccessToken()
35-
{
36-
$resourceServer = $this->getMock('ZfrOAuth2\Server\ResourceServer', [], [], '', false);
37-
$request = new HttpRequest();
34+
/**
35+
* @var \ZfrOAuth2\Server\ResourceServer|\PHPUnit_Framework_MockObject_MockObject
36+
*/
37+
private $resourceServer;
38+
39+
/**
40+
* @var HttpRequest
41+
*/
42+
private $request;
3843

39-
$storage = new AccessTokenStorage($resourceServer);
40-
$storage->setRequest($request);
44+
/**
45+
* @var AccessTokenStorage
46+
*/
47+
private $storage;
4148

42-
$resourceServer->expects($this->once())
43-
->method('getAccessToken')
44-
->with($request)
45-
->will($this->returnValue(null));
49+
/**
50+
* {@inheritDoc}
51+
*/
52+
protected function setUp()
53+
{
54+
$application = $this->getMock('Zend\Mvc\Application', [], [], '', false);
55+
$mvcEvent = new MvcEvent();
56+
$this->resourceServer = $this->getMock('ZfrOAuth2\Server\ResourceServer', [], [], '', false);
57+
$this->request = new HttpRequest();
58+
$this->storage = new AccessTokenStorage($this->resourceServer, $application);
4659

47-
$this->isTrue($storage->isEmpty());
60+
$application->expects($this->any())->method('getMvcEvent')->will($this->returnValue($mvcEvent));
61+
$mvcEvent->setRequest($this->request);
4862
}
4963

50-
public function testReadOwnerFromAccessToken()
64+
public function testIsConsideredAsEmptyIfNoAccessToken()
5165
{
52-
$resourceServer = $this->getMock('ZfrOAuth2\Server\ResourceServer', [], [], '', false);
53-
$request = new HttpRequest();
66+
$this->resourceServer
67+
->expects($this->atLeastOnce())
68+
->method('isRequestValid')
69+
->with($this->request)
70+
->will($this->returnValue(false));
5471

55-
$storage = new AccessTokenStorage($resourceServer);
56-
$storage->setRequest($request);
72+
$this->resourceServer->expects($this->never())->method('getAccessToken');
5773

74+
$this->assertTrue($this->storage->isEmpty());
75+
$this->assertNull($this->storage->read());
76+
}
77+
78+
public function testReadOwnerFromAccessToken()
79+
{
5880
$token = new AccessToken();
5981
$owner = $this->getMock('ZfrOAuth2\Server\Entity\TokenOwnerInterface');
82+
6083
$token->setOwner($owner);
6184

62-
$resourceServer->expects($this->once())
63-
->method('getAccessToken')
64-
->with($request)
65-
->will($this->returnValue($token));
85+
$this->resourceServer
86+
->expects($this->atLeastOnce())
87+
->method('isRequestValid')
88+
->with($this->request)
89+
->will($this->returnValue(true));
90+
91+
$this->resourceServer
92+
->expects($this->atLeastOnce())
93+
->method('getAccessToken')
94+
->with($this->request)
95+
->will($this->returnValue($token));
6696

67-
$this->assertSame($owner, $storage->read());
97+
$this->assertFalse($this->storage->isEmpty());
98+
$this->assertSame($owner, $this->storage->read());
6899
}
69100
}

0 commit comments

Comments
 (0)