From dbd7c834a6713d329f09538a55de62cd96d1523f Mon Sep 17 00:00:00 2001 From: dpi Date: Sun, 8 Nov 2020 20:02:06 +0800 Subject: [PATCH 1/3] Switch to fully using simulated time where possible --- src/Token/AccessToken.php | 16 +++++++++-- test/src/Provider/AbstractProviderTest.php | 12 ++++++-- test/src/Token/AccessTokenTest.php | 33 ++++++++++++++-------- 3 files changed, 45 insertions(+), 16 deletions(-) diff --git a/src/Token/AccessToken.php b/src/Token/AccessToken.php index 81533c30..7fbb3a07 100644 --- a/src/Token/AccessToken.php +++ b/src/Token/AccessToken.php @@ -15,6 +15,7 @@ namespace League\OAuth2\Client\Token; use InvalidArgumentException; +use League\OAuth2\Client\Provider\ProviderClock; use RuntimeException; /** @@ -50,12 +51,17 @@ class AccessToken implements AccessTokenInterface, ResourceOwnerAccessTokenInter protected $values = []; /** - * @var int + * The current time, or NULL to get the true current time via PHP. + * + * @var int|null */ private static $timeNow; /** * Set the time now. This should only be used for testing purposes. + + /** + * Sets the current time. * * @param int $timeNow the time in seconds since epoch * @return void @@ -66,7 +72,7 @@ public static function setTimeNow($timeNow) } /** - * Reset the time now if it was set for test purposes. + * Reset the current time so the true current time via PHP is used. * * @return void */ @@ -76,6 +82,10 @@ public static function resetTimeNow() } /** + + /** + * Get the current time, whether true or simulated. + * * @return int */ public function getTimeNow() @@ -196,7 +206,7 @@ public function hasExpired() throw new RuntimeException('"expires" is not set on the token'); } - return $expires < time(); + return $expires < $this->getTimeNow(); } /** diff --git a/test/src/Provider/AbstractProviderTest.php b/test/src/Provider/AbstractProviderTest.php index bf24ba38..463af186 100644 --- a/test/src/Provider/AbstractProviderTest.php +++ b/test/src/Provider/AbstractProviderTest.php @@ -24,6 +24,14 @@ class AbstractProviderTest extends TestCase { + + /** + * The current simulated time. + * + * @var int + */ + const NOW = 1359504000; + protected function getMockProvider() { return new MockProvider([ @@ -504,7 +512,7 @@ public function testGetAccessToken($method) $provider->setAccessTokenMethod($method); - $raw_response = ['access_token' => 'okay', 'expires' => time() + 3600, 'resource_owner_id' => 3]; + $raw_response = ['access_token' => 'okay', 'expires' => static::NOW + 3600, 'resource_owner_id' => 3]; $grant = Mockery::mock(AbstractGrant::class); $grant @@ -786,7 +794,7 @@ public function testExtendedProviderDoesNotErrorWhenUsingAccessTokenAsTheTypeHin $token = new AccessToken([ 'access_token' => 'mock_access_token', 'refresh_token' => 'mock_refresh_token', - 'expires' => time(), + 'expires' => 123, 'resource_owner_id' => 'mock_resource_owner_id', ]); diff --git a/test/src/Token/AccessTokenTest.php b/test/src/Token/AccessTokenTest.php index 23ad105f..fbe369ac 100644 --- a/test/src/Token/AccessTokenTest.php +++ b/test/src/Token/AccessTokenTest.php @@ -10,6 +10,14 @@ class AccessTokenTest extends TestCase { + + /** + * The current simulated time. + * + * @var int + */ + const NOW = 1359504000; + /** * BC teardown. * @@ -40,14 +48,15 @@ protected function getAccessToken($options = []) public function testExpiresInCorrection() { + // Correction happens in constructor so time needs to be set before + // object is created. + AccessToken::setTimeNow(static::NOW); $options = ['access_token' => 'access_token', 'expires_in' => 100]; $token = $this->getAccessToken($options); $expires = $token->getExpires(); - $this->assertNotNull($expires); - $this->assertGreaterThan(time(), $expires); - $this->assertLessThan(time() + 200, $expires); + $this->assertEquals(static::NOW + 100, $expires); self::tearDownForBackwardsCompatibility(); } @@ -79,13 +88,13 @@ public function testSetTimeNow() public function testResetTimeNow() { - AccessToken::setTimeNow(1577836800); + AccessToken::setTimeNow(static::NOW); $token = $this->getAccessToken(['access_token' => 'asdf']); - $this->assertEquals(1577836800, $token->getTimeNow()); + $this->assertEquals(static::NOW, $token->getTimeNow()); AccessToken::resetTimeNow(); - $this->assertNotEquals(1577836800, $token->getTimeNow()); + $this->assertNotEquals(static::NOW, $token->getTimeNow()); $timeBeforeAssertion = time(); $this->assertGreaterThanOrEqual($timeBeforeAssertion, $token->getTimeNow()); @@ -95,8 +104,9 @@ public function testResetTimeNow() public function testExpiresPastTimestamp() { - $options = ['access_token' => 'access_token', 'expires' => strtotime('5 days ago')]; + $options = ['access_token' => 'access_token', 'expires' => static::NOW - 1]; $token = $this->getAccessToken($options); + AccessToken::setTimeNow(static::NOW); $this->assertTrue($token->hasExpired()); @@ -129,8 +139,9 @@ public function testHasNotExpiredWhenPropertySetInFuture() 'access_token' => 'access_token' ]; - $expectedExpires = strtotime('+1 day'); + $expectedExpires = static::NOW + 1; + AccessToken::setTimeNow(static::NOW); $token = Mockery::mock(AccessToken::class, [$options])->makePartial(); $token ->shouldReceive('getExpires') @@ -148,7 +159,7 @@ public function testHasExpiredWhenPropertySetInPast() 'access_token' => 'access_token' ]; - $expectedExpires = strtotime('-1 day'); + $expectedExpires = static::NOW - 1; $token = Mockery::mock(AccessToken::class, [$options])->makePartial(); $token @@ -195,7 +206,7 @@ public function testJsonSerializable() $options = [ 'access_token' => 'mock_access_token', 'refresh_token' => 'mock_refresh_token', - 'expires' => time(), + 'expires' => static::NOW + 3600, 'resource_owner_id' => 'mock_resource_owner_id', ]; @@ -212,7 +223,7 @@ public function testValues() $options = [ 'access_token' => 'mock_access_token', 'refresh_token' => 'mock_refresh_token', - 'expires' => time(), + 'expires' => static::NOW + 3600, 'resource_owner_id' => 'mock_resource_owner_id', 'custom_thing' => 'i am a test!', ]; From 04ccbe45ae4f024e881e2e43f9622793206314c2 Mon Sep 17 00:00:00 2001 From: dpi Date: Sun, 8 Nov 2020 21:01:38 +0800 Subject: [PATCH 2/3] Allow time to be fetched from a universal clock --- src/Provider/AbstractProvider.php | 36 ++++++++++++++++++++++ src/Provider/Clock.php | 20 ++++++++++++ src/Token/AccessToken.php | 30 ++++++++++++++---- src/Token/AccessTokenInterface.php | 17 ++++++++++ src/Tool/MacAuthorizationTrait.php | 9 +++++- test/src/Provider/AbstractProviderTest.php | 9 ++++++ test/src/Provider/FrozenClock.php | 29 +++++++++++++++++ test/src/Token/AccessTokenTest.php | 27 +++++++++++++++- 8 files changed, 169 insertions(+), 8 deletions(-) create mode 100644 src/Provider/Clock.php create mode 100644 test/src/Provider/FrozenClock.php diff --git a/src/Provider/AbstractProvider.php b/src/Provider/AbstractProvider.php index d1679998..a3fd9e94 100644 --- a/src/Provider/AbstractProvider.php +++ b/src/Provider/AbstractProvider.php @@ -98,6 +98,11 @@ abstract class AbstractProvider */ protected $optionProvider; + /** + * @var Clock + */ + protected $clock; + /** * Constructs an OAuth 2.0 service provider. * @@ -138,6 +143,11 @@ public function __construct(array $options = [], array $collaborators = []) $collaborators['optionProvider'] = new PostAuthOptionProvider(); } $this->setOptionProvider($collaborators['optionProvider']); + + if (empty($collaborators['clock'])) { + $collaborators['clock'] = new Clock(); + } + $this->setClock($collaborators['clock']); } /** @@ -252,6 +262,31 @@ public function getOptionProvider() return $this->optionProvider; } + /** + * Sets the clock. + * + * @param Clock $clock + * + * @return self + */ + public function setClock(Clock $clock) + { + $this->clock = $clock; + + return $this; + } + + + /** + * Returns the clock. + * + * @return Clock + */ + public function getClock() + { + return $this->clock; + } + /** * Returns the current value of the state parameter. * @@ -541,6 +576,7 @@ public function getAccessToken($grant, array $options = []) ); } $prepared = $this->prepareAccessTokenResponse($response); + $prepared['clock'] = $this->clock; $token = $this->createAccessToken($prepared, $grant); return $token; diff --git a/src/Provider/Clock.php b/src/Provider/Clock.php new file mode 100644 index 00000000..bb5a2d50 --- /dev/null +++ b/src/Provider/Clock.php @@ -0,0 +1,20 @@ +clock = $clock; + } /** - * Get the current time, whether true or simulated. - * - * @return int + * @inheritdoc */ public function getTimeNow() { - return self::$timeNow ? self::$timeNow : time(); + if (self::$timeNow) { + return self::$timeNow; + } elseif (isset($this->clock)) { + return $this->clock->now()->getTimestamp(); + } else { + return time(); + } } /** @@ -116,6 +130,10 @@ public function __construct(array $options = []) $this->refreshToken = $options['refresh_token']; } + if (!empty($options['clock'])) { + $this->clock = $options['clock']; + } + // We need to know when the token expires. Show preference to // 'expires_in' since it is defined in RFC6749 Section 5.1. // Defer to 'expires' if it is provided instead. diff --git a/src/Token/AccessTokenInterface.php b/src/Token/AccessTokenInterface.php index c5f13350..132397db 100644 --- a/src/Token/AccessTokenInterface.php +++ b/src/Token/AccessTokenInterface.php @@ -15,6 +15,7 @@ namespace League\OAuth2\Client\Token; use JsonSerializable; +use League\OAuth2\Client\Provider\Clock; use RuntimeException; interface AccessTokenInterface extends JsonSerializable @@ -69,4 +70,20 @@ public function __toString(); * @return array */ public function jsonSerialize(); + + /** + * Sets the clock. + * + * @param Clock $clock a clock. + * + * @return void + */ + public function setClock(Clock $clock); + + /** + * Get the current time, whether real or simulated. + * + * @return int + */ + public function getTimeNow(); } diff --git a/src/Tool/MacAuthorizationTrait.php b/src/Tool/MacAuthorizationTrait.php index f8dcd77c..7de6f3d5 100644 --- a/src/Tool/MacAuthorizationTrait.php +++ b/src/Tool/MacAuthorizationTrait.php @@ -14,6 +14,7 @@ namespace League\OAuth2\Client\Tool; +use League\OAuth2\Client\Provider\Clock; use League\OAuth2\Client\Token\AccessToken; use League\OAuth2\Client\Token\AccessTokenInterface; @@ -24,6 +25,12 @@ */ trait MacAuthorizationTrait { + + /** + * @var Clock + */ + protected $clock; + /** * Returns the id of this token for MAC generation. * @@ -68,7 +75,7 @@ protected function getAuthorizationHeaders($token = null) return []; } - $ts = time(); + $ts = $this->clock->now()->getTimestamp(); $id = $this->getTokenId($token); $nonce = $this->getRandomState(16); $mac = $this->getMacSignature($id, $ts, $nonce); diff --git a/test/src/Provider/AbstractProviderTest.php b/test/src/Provider/AbstractProviderTest.php index 463af186..cf7c83f8 100644 --- a/test/src/Provider/AbstractProviderTest.php +++ b/test/src/Provider/AbstractProviderTest.php @@ -3,6 +3,7 @@ namespace League\OAuth2\Client\Test\Provider; use League\OAuth2\Client\OptionProvider\PostAuthOptionProvider; +use League\OAuth2\Client\Provider\Clock; use Mockery; use ReflectionClass; use UnexpectedValueException; @@ -49,6 +50,14 @@ public function testGetOptionProvider() ); } + public function testGetClock() + { + $this->assertInstanceOf( + Clock::class, + $this->getMockProvider()->getClock() + ); + } + public function testInvalidGrantString() { $this->expectException(InvalidGrantException::class); diff --git a/test/src/Provider/FrozenClock.php b/test/src/Provider/FrozenClock.php new file mode 100644 index 00000000..ff2b0983 --- /dev/null +++ b/test/src/Provider/FrozenClock.php @@ -0,0 +1,29 @@ +getAccessToken(['access_token' => 'asdf', 'clock' => $clock]); + $this->assertEquals(FrozenClock::NOW, $token->getTimeNow()); + } + + public function testSetClockMethod() + { + $clock = new FrozenClock(); + $token = $this->getAccessToken(['access_token' => 'asdf']); + $token->setClock($clock); + $this->assertEquals(FrozenClock::NOW, $token->getTimeNow()); + } + public function testSetTimeNow() { AccessToken::setTimeNow(1577836800); @@ -86,6 +102,15 @@ public function testSetTimeNow() self::tearDownForBackwardsCompatibility(); } + public function testSetClockAndSetTime() + { + // When both a clock and time set, time wins over clock. + $clock = new FrozenClock(); + AccessToken::setTimeNow(static::NOW); + $token = $this->getAccessToken(['access_token' => 'asdf', 'clock' => $clock]); + $this->assertEquals(static::NOW, $token->getTimeNow()); + } + public function testResetTimeNow() { AccessToken::setTimeNow(static::NOW); @@ -197,7 +222,7 @@ public function testInvalidExpiresIn() $token = $this->getAccessToken($options); - self::tearDownForBackwardsCompatibility(); + self::tearDownForBackwardsCompatibility(); } From d286c6a2b7b3427f09cad70d4915cdc66f2bc26f Mon Sep 17 00:00:00 2001 From: dpi Date: Sun, 8 Nov 2020 22:31:07 +0800 Subject: [PATCH 3/3] Tests an access token has the same clock as a provider. --- test/src/Provider/AbstractProviderTest.php | 10 ++++++ test/src/Provider/ProgrammableClock.php | 41 ++++++++++++++++++++++ 2 files changed, 51 insertions(+) create mode 100644 test/src/Provider/ProgrammableClock.php diff --git a/test/src/Provider/AbstractProviderTest.php b/test/src/Provider/AbstractProviderTest.php index cf7c83f8..39b081c1 100644 --- a/test/src/Provider/AbstractProviderTest.php +++ b/test/src/Provider/AbstractProviderTest.php @@ -555,6 +555,9 @@ public function testGetAccessToken($method) ]); $provider->setHttpClient($client); + $clock = (new ProgrammableClock()) + ->setTime(new \DateTimeImmutable('1st February 2013 1pm')); + $provider->setClock($clock); $token = $provider->getAccessToken($grant, ['code' => 'mock_authorization_code']); $this->assertInstanceOf(AccessTokenInterface::class, $token); @@ -563,6 +566,13 @@ public function testGetAccessToken($method) $this->assertSame($raw_response['access_token'], $token->getToken()); $this->assertSame($raw_response['expires'], $token->getExpires()); + // Set the time to a different value so we know references to the + // original clock object in provider was not lost. + $newTime = new \DateTimeImmutable('2nd February 2013 1pm'); + $clock->setTime($newTime); + + $this->assertEquals($newTime->getTimestamp(), $token->getTimeNow()); + $client ->shouldHaveReceived('send') ->once() diff --git a/test/src/Provider/ProgrammableClock.php b/test/src/Provider/ProgrammableClock.php new file mode 100644 index 00000000..7231b8bd --- /dev/null +++ b/test/src/Provider/ProgrammableClock.php @@ -0,0 +1,41 @@ +time)) { + throw new \LogicException('Time must be set explicitly'); + } + return $this->time; + } + + /** + * Sets the current time. + * + * @param \DateTimeImmutable|null the current time. + * @return self + */ + public function setTime($time) + { + $this->time = $time; + + return $this; + } +}