From a8e20a97755219cbd78ff743ef1976348ea17cc4 Mon Sep 17 00:00:00 2001 From: ErickSkrauch Date: Fri, 13 Dec 2019 13:55:09 +0300 Subject: [PATCH] Replace custom aud and ely-scopes JWT claims with its public analogues --- api/components/Tokens/TokenReader.php | 17 ++--- api/components/Tokens/TokensFactory.php | 15 ++-- .../OAuth2/Entities/AccessTokenEntityTest.php | 2 +- .../components/Tokens/TokenReaderTest.php | 76 +++++++++++++++++++ .../components/Tokens/TokensFactoryTest.php | 10 +-- .../unit/components/User/JwtIdentityTest.php | 2 +- .../authentication/RefreshTokenFormTest.php | 2 +- .../authentication/RegistrationFormTest.php | 13 ++-- 8 files changed, 104 insertions(+), 33 deletions(-) create mode 100644 api/tests/unit/components/Tokens/TokenReaderTest.php diff --git a/api/components/Tokens/TokenReader.php b/api/components/Tokens/TokenReader.php index 95fb934..4075c84 100644 --- a/api/components/Tokens/TokenReader.php +++ b/api/components/Tokens/TokenReader.php @@ -31,19 +31,16 @@ class TokenReader { } public function getClientId(): ?string { - $aud = $this->token->getClaim('aud', false); - if ($aud === false) { - return null; - } - - if (mb_strpos((string)$aud, TokensFactory::AUD_CLIENT_PREFIX) !== 0) { - return null; - } - - return mb_substr($aud, mb_strlen(TokensFactory::AUD_CLIENT_PREFIX)); + return $this->token->getClaim('client_id', false) ?: null; } public function getScopes(): ?array { + $scopes = $this->token->getClaim('scope', false); + if ($scopes !== false) { + return explode(' ', $scopes); + } + + // Handle legacy tokens, which used "ely-scopes" claim and was delimited with comma $scopes = $this->token->getClaim('ely-scopes', false); if ($scopes === false) { return null; diff --git a/api/components/Tokens/TokensFactory.php b/api/components/Tokens/TokensFactory.php index 0b7837c..3e5c3da 100644 --- a/api/components/Tokens/TokensFactory.php +++ b/api/components/Tokens/TokensFactory.php @@ -18,13 +18,12 @@ use yii\base\Component; class TokensFactory extends Component { public const SUB_ACCOUNT_PREFIX = 'ely|'; - public const AUD_CLIENT_PREFIX = 'client|'; public function createForWebAccount(Account $account, AccountSession $session = null): Token { $payloads = [ - 'ely-scopes' => $this->prepareScopes([R::ACCOUNTS_WEB_USER]), 'sub' => $this->buildSub($account->id), 'exp' => Carbon::now()->addHour()->getTimestamp(), + 'scope' => $this->prepareScopes([R::ACCOUNTS_WEB_USER]), ]; if ($session === null) { // If we don't remember a session, the token should live longer @@ -39,8 +38,8 @@ class TokensFactory extends Component { public function createForOAuthClient(AccessTokenEntityInterface $accessToken): Token { $payloads = [ - 'aud' => $this->buildAud($accessToken->getClient()->getIdentifier()), - 'ely-scopes' => $this->prepareScopes($accessToken->getScopes()), + 'client_id' => $accessToken->getClient()->getIdentifier(), + 'scope' => $this->prepareScopes($accessToken->getScopes()), ]; if ($accessToken->getExpiryDateTime() > new DateTime()) { $payloads['exp'] = $accessToken->getExpiryDateTime()->getTimestamp(); @@ -55,7 +54,7 @@ class TokensFactory extends Component { public function createForMinecraftAccount(Account $account, string $clientToken): Token { return Yii::$app->tokens->create([ - 'ely-scopes' => $this->prepareScopes([P::MINECRAFT_SERVER_SESSION]), + 'scope' => $this->prepareScopes([P::MINECRAFT_SERVER_SESSION]), 'ely-client-token' => new EncryptedValue($clientToken), 'sub' => $this->buildSub($account->id), 'exp' => Carbon::now()->addDays(2)->getTimestamp(), @@ -68,7 +67,7 @@ class TokensFactory extends Component { * @return string */ private function prepareScopes(array $scopes): string { - return implode(',', array_map(function($scope): string { // TODO: replace to the space if it's possible + return implode(' ', array_map(function($scope): string { if ($scope instanceof ScopeEntityInterface) { return $scope->getIdentifier(); } @@ -81,8 +80,4 @@ class TokensFactory extends Component { return self::SUB_ACCOUNT_PREFIX . $accountId; } - private function buildAud(string $clientId): string { - return self::AUD_CLIENT_PREFIX . $clientId; - } - } diff --git a/api/tests/unit/components/OAuth2/Entities/AccessTokenEntityTest.php b/api/tests/unit/components/OAuth2/Entities/AccessTokenEntityTest.php index 83a8af7..8a22146 100644 --- a/api/tests/unit/components/OAuth2/Entities/AccessTokenEntityTest.php +++ b/api/tests/unit/components/OAuth2/Entities/AccessTokenEntityTest.php @@ -24,7 +24,7 @@ class AccessTokenEntityTest extends TestCase { $token = (string)$entity; $payloads = json_decode(base64_decode(explode('.', $token)[1]), true); - $this->assertSame('first,second', $payloads['ely-scopes']); + $this->assertSame('first second', $payloads['scope']); } private function createScopeEntity(string $id): ScopeEntityInterface { diff --git a/api/tests/unit/components/Tokens/TokenReaderTest.php b/api/tests/unit/components/Tokens/TokenReaderTest.php new file mode 100644 index 0000000..95d5426 --- /dev/null +++ b/api/tests/unit/components/Tokens/TokenReaderTest.php @@ -0,0 +1,76 @@ +assertSame($expectedResult, $this->createReader($claims)->getAccountId()); + } + + public function getAccountIdTestCases() { + yield [['sub' => 'ely|1'], 1]; + yield [['sub' => '1'], null]; + yield [['sub' => 'ely-login|1'], null]; + yield [[], null]; + } + + /** + * @dataProvider getClientIdTestCases + */ + public function testGetClientId(array $claims, $expectedResult) { + $this->assertSame($expectedResult, $this->createReader($claims)->getClientId()); + } + + public function getClientIdTestCases() { + yield [['client_id' => 'find-me'], 'find-me']; + yield [[], null]; + } + + /** + * @dataProvider getScopesTestCases + */ + public function testGetScopes(array $claims, $expectedResult) { + $this->assertSame($expectedResult, $this->createReader($claims)->getScopes()); + } + + public function getScopesTestCases() { + yield [['scope' => 'scope1 scope2'], ['scope1', 'scope2']]; + yield [['ely-scopes' => 'scope1,scope2'], ['scope1', 'scope2']]; + yield [[], null]; + } + + /** + * @dataProvider getMinecraftClientTokenTestCases + */ + public function testGetMinecraftClientToken(array $claims, $expectedResult) { + $this->assertSame($expectedResult, $this->createReader($claims)->getMinecraftClientToken()); + } + + public function getMinecraftClientTokenTestCases() { + yield [['ely-client-token' => 'GPZiBFlJld30KfGTe-E2yITKbfJYmWFA6Ky5CsllnIsVdmswMu_PXNdYnQGexF_CkXiuOQd1smrO3S4'], 'aaaaa-aaa-aaa-aaaaa']; + yield [[], null]; + } + + private function createReader(array $claims): TokenReader { + $claimsObjects = []; + foreach ($claims as $key => $value) { + $claim = $this->createMock(Claim::class); + $claim->method('getName')->willReturn($key); + $claim->method('getValue')->willReturn($value); + $claimsObjects[$key] = $claim; + } + + return new TokenReader(new Token([], $claimsObjects)); + } + +} diff --git a/api/tests/unit/components/Tokens/TokensFactoryTest.php b/api/tests/unit/components/Tokens/TokensFactoryTest.php index 36e536a..8932613 100644 --- a/api/tests/unit/components/Tokens/TokensFactoryTest.php +++ b/api/tests/unit/components/Tokens/TokensFactoryTest.php @@ -26,7 +26,7 @@ class TokensFactoryTest extends TestCase { $this->assertEqualsWithDelta(time(), $token->getClaim('iat'), 1); $this->assertEqualsWithDelta(time() + 60 * 60 * 24 * 7, $token->getClaim('exp'), 2); $this->assertSame('ely|1', $token->getClaim('sub')); - $this->assertSame('accounts_web_user', $token->getClaim('ely-scopes')); + $this->assertSame('accounts_web_user', $token->getClaim('scope')); $this->assertArrayNotHasKey('jti', $token->getClaims()); $session = new AccountSession(); @@ -38,7 +38,7 @@ class TokensFactoryTest extends TestCase { $this->assertEqualsWithDelta(time(), $token->getClaim('iat'), 1); $this->assertEqualsWithDelta(time() + 3600, $token->getClaim('exp'), 2); $this->assertSame('ely|1', $token->getClaim('sub')); - $this->assertSame('accounts_web_user', $token->getClaim('ely-scopes')); + $this->assertSame('accounts_web_user', $token->getClaim('scope')); $this->assertSame(2, $token->getClaim('jti')); } @@ -67,8 +67,8 @@ class TokensFactoryTest extends TestCase { $this->assertEqualsWithDelta(time(), $token->getClaim('iat'), 1); $this->assertEqualsWithDelta($expiryDateTime->getTimestamp(), $token->getClaim('exp'), 2); $this->assertSame('ely|1', $token->getClaim('sub')); - $this->assertSame('client|clientId', $token->getClaim('aud')); - $this->assertSame('scope1,scope2', $token->getClaim('ely-scopes')); + $this->assertSame('clientId', $token->getClaim('client_id')); + $this->assertSame('scope1 scope2', $token->getClaim('scope')); // Create for client credentials grant @@ -93,7 +93,7 @@ class TokensFactoryTest extends TestCase { $token = $factory->createForMinecraftAccount($account, $clientToken); $this->assertEqualsWithDelta(time(), $token->getClaim('iat'), 5); $this->assertEqualsWithDelta(time() + 60 * 60 * 24 * 2, $token->getClaim('exp'), 5); - $this->assertSame('minecraft_server_session', $token->getClaim('ely-scopes')); + $this->assertSame('minecraft_server_session', $token->getClaim('scope')); $this->assertNotSame('e44fae79-f80e-4975-952e-47e8a9ed9472', $token->getClaim('ely-client-token')); $this->assertSame('ely|1', $token->getClaim('sub')); } diff --git a/api/tests/unit/components/User/JwtIdentityTest.php b/api/tests/unit/components/User/JwtIdentityTest.php index 2488f69..202c579 100644 --- a/api/tests/unit/components/User/JwtIdentityTest.php +++ b/api/tests/unit/components/User/JwtIdentityTest.php @@ -51,7 +51,7 @@ class JwtIdentityTest extends TestCase { 'Incorrect token', ]; yield 'revoked by oauth client' => [ - 'eyJ0eXAiOiJKV1QiLCJhbGciOiJFUzI1NiJ9.eyJlbHktc2NvcGVzIjoiYWNjb3VudF9pbmZvLG1pbmVjcmFmdF9zZXJ2ZXJfc2Vzc2lvbiIsImlhdCI6MTU2NDYxMDUwMCwic3ViIjoiZWx5fDEiLCJhdWQiOiJjbGllbnR8dGxhdW5jaGVyIn0.YzUzvnREEoQPu8CvU6WLdysUU0bC_xzigQPs2LK1su38uysSYgSbPzNOZYkQnvcmVLehHY-ON44x-oA8Os-9ZA', + 'eyJ0eXAiOiJKV1QiLCJhbGciOiJFUzI1NiJ9.eyJlbHktc2NvcGVzIjoiYWNjb3VudF9pbmZvLG1pbmVjcmFmdF9zZXJ2ZXJfc2Vzc2lvbiIsImlhdCI6MTU2NDYxMDUwMCwic3ViIjoiZWx5fDEiLCJjbGllbnRfaWQiOiJ0bGF1bmNoZXIifQ.qmiPOjI8jGAQdP5LoAVHO8L75Ly7fRcrTB_iYsUgQ4azgsPnLEhvG7dUnQ9utEd3RK5swDpaZ0bXf90vRbvnmg', 'Token has been revoked', ]; yield 'revoked by unauthorized minecraft launcher' => [ diff --git a/api/tests/unit/models/authentication/RefreshTokenFormTest.php b/api/tests/unit/models/authentication/RefreshTokenFormTest.php index 4c04a05..8bf6383 100644 --- a/api/tests/unit/models/authentication/RefreshTokenFormTest.php +++ b/api/tests/unit/models/authentication/RefreshTokenFormTest.php @@ -31,7 +31,7 @@ class RefreshTokenFormTest extends TestCase { $token = $result->getToken(); $this->assertSame('ely|1', $token->getClaim('sub')); - $this->assertSame('accounts_web_user', $token->getClaim('ely-scopes')); + $this->assertSame('accounts_web_user', $token->getClaim('scope')); $this->assertEqualsWithDelta(time(), $token->getClaim('iat'), 5); $this->assertEqualsWithDelta(time() + 3600, $token->getClaim('exp'), 5); $this->assertSame(1, $token->getClaim('jti')); diff --git a/api/tests/unit/models/authentication/RegistrationFormTest.php b/api/tests/unit/models/authentication/RegistrationFormTest.php index eb4e472..63624dc 100644 --- a/api/tests/unit/models/authentication/RegistrationFormTest.php +++ b/api/tests/unit/models/authentication/RegistrationFormTest.php @@ -112,11 +112,14 @@ class RegistrationFormTest extends TestCase { ]) ->one(); $this->assertInstanceOf(EmailActivation::class, $activation, 'email activation code exists in database'); - $this->assertTrue(UsernameHistory::find()->andWhere([ - 'username' => $account->username, - 'account_id' => $account->id, - 'applied_in' => $account->created_at, - ])->exists(), 'username history record exists in database'); + $this->assertTrue( + UsernameHistory::find() + ->andWhere(['username' => $account->username]) + ->andWhere(['account_id' => $account->id]) + ->andWhere(['>=', 'applied_in', $account->created_at]) + ->exists(), + 'username history record exists in database' + ); /** @var SendRegistrationEmail $job */ $job = $this->tester->grabLastQueuedJob();