Skip to content

Commit 4b6c280

Browse files
robertlemkerobertlemke
committed
Store serialized access token as string
This change modifies Doctrine's column schema for the serialized access token so it's stored as a string, not as an "array". This doesn't change any actual data in the database, but changes the set- and getSerializedAccesstoken() methods which now expect a string instead of an array. This change also improves test coverage of the Authorization class.
1 parent 87d0794 commit 4b6c280

File tree

3 files changed

+150
-27
lines changed

3 files changed

+150
-27
lines changed

‎Classes/Authorization.php‎

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515

1616
use Doctrine\ORM\Mapping as ORM;
1717
use Exception;
18+
use JsonException;
1819
use League\OAuth2\Client\Token\AccessToken;
1920
use League\OAuth2\Client\Token\AccessTokenInterface;
2021
use Neos\Flow\Annotations as Flow;
@@ -64,8 +65,8 @@ class Authorization
6465
protected $scope;
6566

6667
/**
67-
* @var array
68-
* @ORM\Column(type="json_array", nullable = true)
68+
* @var string
69+
* @ORM\Column(nullable = true, type = "text")
6970
*/
7071
protected $serializedAccessToken;
7172

@@ -98,9 +99,11 @@ public static function generateAuthorizationIdForAuthorizationCodeGrant(string $
9899
{
99100
try {
100101
return $serviceType . '-' . $serviceName . '-' . Uuid::uuid4()->toString();
102+
// @codeCoverageIgnoreStart
101103
} catch (Exception $e) {
102104
throw new OAuthClientException(sprintf('Failed generating authorization id for %s %s', $serviceName, $clientId), 1597311416, $e);
103105
}
106+
// @codeCoverageIgnoreEnd
104107
}
105108

106109
/**
@@ -183,35 +186,43 @@ public function setScope(string $scope): void
183186
}
184187

185188
/**
186-
* @return array
189+
* @return string
187190
*/
188-
public function getSerializedAccessToken(): array
191+
public function getSerializedAccessToken(): string
189192
{
190-
return $this->serializedAccessToken ?? [];
193+
return $this->serializedAccessToken ?? '';
191194
}
192195

193196
/**
194-
* @param array $serializedAccessToken
197+
* @param string $serializedAccessToken
195198
*/
196-
public function setSerializedAccessToken(array $serializedAccessToken): void
199+
public function setSerializedAccessToken(string $serializedAccessToken): void
197200
{
198201
$this->serializedAccessToken = $serializedAccessToken;
199202
}
200203

201204
/**
202205
* @param AccessTokenInterface $accessToken
203206
* @return void
207+
* @throws JsonException
204208
*/
205209
public function setAccessToken(AccessTokenInterface $accessToken): void
206210
{
207-
$this->serializedAccessToken = $accessToken->jsonSerialize();
211+
$this->serializedAccessToken = json_encode($accessToken, JSON_THROW_ON_ERROR, 512);
208212
}
209213

210214
/**
211215
* @return AccessToken
212216
*/
213217
public function getAccessToken(): ?AccessToken
214218
{
215-
return !empty($this->serializedAccessToken) ? new AccessToken($this->serializedAccessToken) : null;
219+
try {
220+
if (!empty($this->serializedAccessToken)) {
221+
$unserializedAccessToken = json_decode($this->serializedAccessToken, true, 512, JSON_THROW_ON_ERROR);
222+
return new AccessToken($unserializedAccessToken);
223+
}
224+
} catch (JsonException $e) {
225+
}
226+
return null;
216227
}
217228
}

‎Migrations/Mysql/Version20201012142627.php‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
<?php
2+
namespace Neos\Flow\Persistence\Doctrine\Migrations;
3+
4+
use Doctrine\DBAL\Exception;
5+
use Doctrine\Migrations\AbstractMigration;
6+
use Doctrine\DBAL\Schema\Schema;
7+
use Doctrine\DBAL\Migrations\AbortMigrationException;
8+
9+
/**
10+
* Adjust column type for serialized access token
11+
*/
12+
class Version20201012142627 extends AbstractMigration
13+
{
14+
15+
/**
16+
* @return string
17+
*/
18+
public function getDescription(): string
19+
{
20+
return 'Adjust column type for serialized access token';
21+
}
22+
23+
/**
24+
* @param Schema $schema
25+
* @return void
26+
* @throws AbortMigrationException
27+
* @throws Exception
28+
*/
29+
public function up(Schema $schema): void
30+
{
31+
$this->abortIf($this->connection->getDatabasePlatform()->getName() !== 'mysql', 'Migration can only be executed safely on "mysql".');
32+
33+
$this->addSql('ALTER TABLE flownative_oauth2_client_authorization CHANGE serializedaccesstoken serializedaccesstoken LONGTEXT DEFAULT NULL');
34+
}
35+
36+
/**
37+
* @param Schema $schema
38+
* @return void
39+
* @throws AbortMigrationException
40+
* @throws Exception
41+
*/
42+
public function down(Schema $schema): void
43+
{
44+
$this->abortIf($this->connection->getDatabasePlatform()->getName() !== 'mysql', 'Migration can only be executed safely on "mysql".');
45+
46+
$this->addSql('ALTER TABLE flownative_oauth2_client_authorization CHANGE serializedaccesstoken serializedaccesstoken LONGTEXT CHARACTER SET utf8mb4 DEFAULT NULL COLLATE `utf8mb4_unicode_ci` COMMENT \'(DC2Type:json_array)\'');
47+
}
48+
}

‎Tests/Unit/AuthorizationTest.php‎

Lines changed: 82 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@
1313
* source code.
1414
*/
1515

16+
use Exception;
17+
use JsonException;
1618
use League\OAuth2\Client\Token\AccessToken;
1719
use Neos\Flow\Tests\UnitTestCase;
1820
use Neos\Flow\Utility\Algorithms;
@@ -29,37 +31,41 @@ public function correctConstructorArguments(): array
2931
'3d47f0eafd6a8b49e32b55103d817b6e4ef489e7',
3032
'myService',
3133
'ac36cGG4d2Cef1DeuevA7T1u7V4WOUI14',
32-
'CMc4EHfyMPLw}Tua%rnyxCnrTWMuX3',
3334
'authorization_code',
3435
'profile oidc'
3536
]
3637
];
3738
}
3839

3940
/**
40-
* @param string $expectedAuthorizationId
41+
* @param string $authorizationId
4142
* @param string $serviceName
4243
* @param string $clientId
43-
* @param string $clientSecret
4444
* @param string $grantType
4545
* @param string $scope
4646
* @test
4747
* @dataProvider correctConstructorArguments
4848
*/
49-
public function constructSetsAuthorizationIdentifier(string $expectedAuthorizationId, string $serviceName, string $clientId, string $clientSecret, string $grantType, string $scope): void
49+
public function constructSetsAuthorizationParameters(string $authorizationId, string $serviceName, string $clientId, string $grantType, string $scope): void
5050
{
51-
$authorization = new Authorization($serviceName, $clientId, $grantType, $scope);
52-
self::assertSame($expectedAuthorizationId, $authorization->getAuthorizationId());
51+
$authorization = new Authorization($authorizationId, $serviceName, $clientId, $grantType, $scope);
52+
self::assertSame($authorizationId, $authorization->getAuthorizationId());
53+
self::assertSame($serviceName, $authorization->getServiceName());
54+
self::assertSame($clientId, $authorization->getClientId());
55+
self::assertSame($grantType, $authorization->getGrantType());
56+
self::assertSame($scope, $authorization->getScope());
5357
}
5458

5559
/**
5660
* @test
61+
* @throws JsonException
62+
* @throws Exception
5763
*/
5864
public function getAccessTokenReturnsClonedObject(): void
5965
{
6066
$accessToken = $this->createValidAccessToken();
6167

62-
$authorization = new Authorization('service', 'clientId',Authorization::GRANT_AUTHORIZATION_CODE, 'profile');
68+
$authorization = new Authorization('3d47f0eafd6a8b49e32b55103d817b6e4ef489e7', 'service', 'clientId',Authorization::GRANT_AUTHORIZATION_CODE, 'profile');
6369
$authorization->setAccessToken($accessToken);
6470
$retrievedAccessToken = $authorization->getAccessToken();
6571

@@ -69,52 +75,110 @@ public function getAccessTokenReturnsClonedObject(): void
6975

7076
/**
7177
* @test
78+
* @throws JsonException
79+
* @throws Exception
7280
*/
7381
public function getSerializedAccessTokenReturnsCorrectJsonString(): void
7482
{
7583
$accessToken = $this->createValidAccessToken();
7684

77-
$authorization = new Authorization('service', 'clientId', Authorization::GRANT_AUTHORIZATION_CODE, '');
85+
$authorization = new Authorization('3d47f0eafd6a8b49e32b55103d817b6e4ef489e7', 'service', 'clientId', Authorization::GRANT_AUTHORIZATION_CODE, '');
7886
$authorization->setAccessToken($accessToken);
7987

80-
$secondAccessToken = new AccessToken($authorization->getSerializedAccessToken());
88+
$secondAccessToken = new AccessToken(json_decode($authorization->getSerializedAccessToken(), true, 512, JSON_THROW_ON_ERROR));
8189
$this->assertEquals($accessToken, $secondAccessToken);
8290
}
8391

8492
/**
8593
* @test
94+
* @throws JsonException
95+
* @throws Exception
8696
*/
8797
public function getAccessTokenReturnsPreviouslySetSerializedToken(): void
8898
{
8999
$accessToken = $this->createValidAccessToken();
90100

91-
$authorization = new Authorization('service', 'clientId', Authorization::GRANT_AUTHORIZATION_CODE, '');
92-
$authorization->setSerializedAccessToken($accessToken->jsonSerialize());
101+
$authorization = new Authorization('3d47f0eafd6a8b49e32b55103d817b6e4ef489e7', 'service', 'clientId', Authorization::GRANT_AUTHORIZATION_CODE, '');
102+
$authorization->setSerializedAccessToken(json_encode($accessToken, JSON_THROW_ON_ERROR, 512));
93103

94-
$secondAccessToken = new AccessToken($authorization->getSerializedAccessToken());
104+
$secondAccessToken = new AccessToken(json_decode($authorization->getSerializedAccessToken(), true, 512, JSON_THROW_ON_ERROR));
95105
$this->assertEquals($accessToken, $secondAccessToken);
96106
}
97107

98108
/**
99109
* @test
100110
*/
101-
public function calculateAuthorizationIdReturnsSha1(): void
111+
public function generateAuthorizationIdForClientCredentialsGrantReturnsSha1(): void
102112
{
103-
$authorizationId = Authorization::calculateAuthorizationId(
104-
'oidc_test', 'ac36cGG4d2Cef1DeuevA7T1u7V4WOUI14', 'oidc profile', 'authorization_code'
113+
$authorizationId = Authorization::generateAuthorizationIdForClientCredentialsGrant(
114+
'oidc_test', 'ac36cGG4d2Cef1DeuevA7T1u7V4WOUI14', 'CMc4EHfyMPLw}Tua%rnyxCnrTWMuX3', 'oidc profile'
105115
);
106-
self::assertSame('21de19f789834af6edff3346e8d9449fcd0d4dae', $authorizationId);
116+
self::assertSame('bd55b7bc1b40d6342789c74fcc1900877b3966f4656c5d6a1c0a9111a1da02365ba9f00fcb1d058629446f7ec83d02166b0a8c271cbf1374467e7f294bb4b784', $authorizationId);
117+
}
118+
119+
/**
120+
* @test
121+
* @throws OAuthClientException
122+
*
123+
* @see https://github.com/flownative/flow-oauth2-client/issues/13
124+
*/
125+
public function generateAuthorizationIdForAuthorizationCodeGrantReturnsRandomIdentifiers(): void
126+
{
127+
$firstAuthorizationId = Authorization::generateAuthorizationIdForAuthorizationCodeGrant(
128+
'oidc_test', 'test', 'ac36cGG4d2Cef1DeuevA7T1u7V4WOUI14'
129+
);
130+
131+
self::assertStringStartsWith('oidc_test-test-', $firstAuthorizationId);
132+
self::assertStringMatchesFormat('oidc_test-test-%x%x%x%x%x%x%x%x-%x%x%x%x-%x%x%x%x-%x%x%x%x-%x%x%x%x%x%x%x%x%x%x%x%x', $firstAuthorizationId);
133+
134+
$secondAuthorizationId = Authorization::generateAuthorizationIdForAuthorizationCodeGrant(
135+
'oidc_test', 'test', 'ac36cGG4d2Cef1DeuevA7T1u7V4WOUI14'
136+
);
137+
138+
self::assertStringStartsWith('oidc_test-test-', $secondAuthorizationId);
139+
self::assertStringMatchesFormat('oidc_test-test-%x%x%x%x%x%x%x%x-%x%x%x%x-%x%x%x%x-%x%x%x%x-%x%x%x%x%x%x%x%x%x%x%x%x', $secondAuthorizationId);
140+
141+
self::assertNotSame($firstAuthorizationId, $secondAuthorizationId);
142+
}
143+
144+
/**
145+
* @test
146+
*/
147+
public function getAccessTokenReturnsNullIfNoTokenWasSet(): void
148+
{
149+
$authorization = new Authorization('3d47f0eafd6a8b49e32b55103d817b6e4ef489e7', 'service', 'clientId', Authorization::GRANT_AUTHORIZATION_CODE, '');
150+
self::assertNull($authorization->getAccessToken());
151+
}
152+
153+
/**
154+
* @test
155+
*/
156+
public function getAccessTokenReturnsNullIfTokenCouldNotBeUnserialized(): void
157+
{
158+
$authorization = new Authorization('3d47f0eafd6a8b49e32b55103d817b6e4ef489e7', 'service', 'clientId', Authorization::GRANT_AUTHORIZATION_CODE, '');
159+
$authorization->setSerializedAccessToken('invalid json syntax');
160+
self::assertNull($authorization->getAccessToken());
161+
}
162+
163+
/**
164+
* @test
165+
*/
166+
public function getScopeReturnsScope(): void
167+
{
168+
$authorization = new Authorization('3d47f0eafd6a8b49e32b55103d817b6e4ef489e7', 'service', 'clientId', Authorization::GRANT_AUTHORIZATION_CODE, '');
169+
$authorization->setScope('some-custom-scope');
170+
self::assertSame('some-custom-scope', $authorization->getScope());
107171
}
108172

109173
/**
110174
* @return AccessToken
175+
* @throws Exception
111176
*/
112177
private function createValidAccessToken(): AccessToken
113178
{
114-
$accessToken = new AccessToken([
179+
return new AccessToken([
115180
'access_token' => Algorithms::generateRandomToken(500),
116181
'expires' => time() + 3600
117182
]);
118-
return $accessToken;
119183
}
120184
}

0 commit comments

Comments
 (0)