Skip to content

Commit c5b1f5c

Browse files
committed
Remove OAuth client secret from Authorization records
Remove the client secret from the Authorization table – for now at least. We don't need to store the client secret, because if OAuth is used with client credentials flow, the client secret is likely available somewhere lese (for example via a setting or an application specific storage) and when authorization code flow is used, the secret is only needed when authorization is started and does not have to be stored. We may need the client secret for refreshing a token, but this is not correctly implemented at this time, so we may need to solved this at a later point in time.
1 parent 4b6c280 commit c5b1f5c

File tree

4 files changed

+54
-30
lines changed

4 files changed

+54
-30
lines changed

Classes/Authorization.php

Lines changed: 8 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -48,12 +48,6 @@ class Authorization
4848
*/
4949
protected $clientId;
5050

51-
/**
52-
* @var string
53-
* @ORM\Column(nullable = true, length=5000)
54-
*/
55-
protected $clientSecret;
56-
5751
/**
5852
* @var string
5953
*/
@@ -144,22 +138,6 @@ public function getClientId(): string
144138
return $this->clientId;
145139
}
146140

147-
/**
148-
* @param string $clientSecret
149-
*/
150-
public function setClientSecret(string $clientSecret): void
151-
{
152-
$this->clientSecret = $clientSecret;
153-
}
154-
155-
/**
156-
* @return string
157-
*/
158-
public function getClientSecret(): string
159-
{
160-
return $this->clientSecret;
161-
}
162-
163141
/**
164142
* @return string
165143
*/
@@ -204,11 +182,17 @@ public function setSerializedAccessToken(string $serializedAccessToken): void
204182
/**
205183
* @param AccessTokenInterface $accessToken
206184
* @return void
207-
* @throws JsonException
185+
* @throws \InvalidArgumentException
208186
*/
209187
public function setAccessToken(AccessTokenInterface $accessToken): void
210188
{
211-
$this->serializedAccessToken = json_encode($accessToken, JSON_THROW_ON_ERROR, 512);
189+
try {
190+
$this->serializedAccessToken = json_encode($accessToken, JSON_THROW_ON_ERROR, 512);
191+
// @codeCoverageIgnoreStart
192+
} catch (JsonException $e) {
193+
throw new \InvalidArgumentException('Failed serializing the given access token', 1602515717);
194+
// @codeCoverageIgnoreEnd
195+
}
212196
}
213197

214198
/**

Classes/OAuthClient.php

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,8 @@
88
use GuzzleHttp\Exception\GuzzleException;
99
use GuzzleHttp\Psr7\Response;
1010
use GuzzleHttp\Psr7\Uri;
11-
use InvalidArgumentException;
1211
use League\OAuth2\Client\Provider\Exception\IdentityProviderException;
1312
use League\OAuth2\Client\Provider\GenericProvider;
14-
use League\OAuth2\Client\Token\AccessTokenInterface;
1513
use League\OAuth2\Client\Tool\RequestFactory;
1614
use Neos\Cache\Exception;
1715
use Neos\Cache\Frontend\VariableFrontend;
Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
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+
* Drop the client secret field in Authorization
11+
*/
12+
class Version20201012151008 extends AbstractMigration
13+
{
14+
15+
/**
16+
* @return string
17+
*/
18+
public function getDescription(): string
19+
{
20+
return 'Drop the client secret field in Authorization';
21+
}
22+
23+
/**
24+
* @param Schema $schema
25+
* @return void
26+
* @throws AbortMigrationException|Exception
27+
*/
28+
public function up(Schema $schema): void
29+
{
30+
$this->abortIf($this->connection->getDatabasePlatform()->getName() !== 'mysql', 'Migration can only be executed safely on "mysql".');
31+
32+
$this->addSql('ALTER TABLE flownative_oauth2_client_authorization DROP clientsecret');
33+
}
34+
35+
/**
36+
* @param Schema $schema
37+
* @return void
38+
* @throws AbortMigrationException|Exception
39+
*/
40+
public function down(Schema $schema): void
41+
{
42+
$this->abortIf($this->connection->getDatabasePlatform()->getName() !== 'mysql', 'Migration can only be executed safely on "mysql".');
43+
44+
$this->addSql('ALTER TABLE flownative_oauth2_client_authorization ADD clientsecret VARCHAR(5000) CHARACTER SET utf8mb4 DEFAULT NULL COLLATE `utf8mb4_unicode_ci`');
45+
}
46+
}

Tests/Unit/AuthorizationTest.php

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@
1414
*/
1515

1616
use Exception;
17-
use JsonException;
1817
use League\OAuth2\Client\Token\AccessToken;
1918
use Neos\Flow\Tests\UnitTestCase;
2019
use Neos\Flow\Utility\Algorithms;
@@ -58,7 +57,6 @@ public function constructSetsAuthorizationParameters(string $authorizationId, st
5857

5958
/**
6059
* @test
61-
* @throws JsonException
6260
* @throws Exception
6361
*/
6462
public function getAccessTokenReturnsClonedObject(): void
@@ -75,7 +73,6 @@ public function getAccessTokenReturnsClonedObject(): void
7573

7674
/**
7775
* @test
78-
* @throws JsonException
7976
* @throws Exception
8077
*/
8178
public function getSerializedAccessTokenReturnsCorrectJsonString(): void
@@ -91,7 +88,6 @@ public function getSerializedAccessTokenReturnsCorrectJsonString(): void
9188

9289
/**
9390
* @test
94-
* @throws JsonException
9591
* @throws Exception
9692
*/
9793
public function getAccessTokenReturnsPreviouslySetSerializedToken(): void

0 commit comments

Comments
 (0)