Skip to content

Commit f998ae5

Browse files
MKoddekayjoosten
MKodde
authored and
kayjoosten
committed
Retrieve old/new style attribute hash in one go
Previously when testing if previous consent was given was based on testing for old-style attribute hash calculation. And if that one did not exist, the new (stable) attribute hash was tested. Having that in two queries made little sense as it can easily be performed in one action. The interface of the repo and service changed slightly, as now a value object is returned instead of a boolean value. The value object reflects the version (type of attribute hash) of consent that was given. That can either be: unstable, stable or not given at all.
1 parent 37805e8 commit f998ae5

File tree

7 files changed

+34
-116
lines changed

7 files changed

+34
-116
lines changed

‎library/EngineBlock/Corto/Model/Consent.php‎

Lines changed: 2 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -183,31 +183,13 @@ private function _updateConsent(ServiceProvider $serviceProvider, $consentType):
183183
private function _hasStoredConsent(ServiceProvider $serviceProvider, $consentType): ConsentVersion
184184
{
185185
$consentUuid = sha1($this->_getConsentUid());
186-
$parameters = [
187-
$consentUuid,
188-
$serviceProvider->entityId,
189-
$this->_getStableAttributesHash($this->_responseAttributes),
190-
$consentType,
191-
];
192-
$hasStableConsentHash = $this->_hashService->retrieveStableConsentHash($parameters);
193-
194-
if ($hasStableConsentHash) {
195-
return ConsentVersion::stable();
196-
}
197-
198186
$parameters = [
199187
$consentUuid,
200188
$serviceProvider->entityId,
201189
$this->_getAttributesHash($this->_responseAttributes),
190+
$this->_getStableAttributesHash($this->_responseAttributes),
202191
$consentType,
203192
];
204-
205-
$hasUnstableConsentHash = $this->_hashService->retrieveConsentHash($parameters);
206-
207-
if ($hasUnstableConsentHash) {
208-
return ConsentVersion::unstable();
209-
}
210-
211-
return ConsentVersion::notGiven();
193+
return $this->_hashService->retrieveConsentHash($parameters);
212194
}
213195
}

‎src/OpenConext/EngineBlock/Authentication/Repository/ConsentRepository.php‎

Lines changed: 3 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
namespace OpenConext\EngineBlock\Authentication\Repository;
2020

2121
use OpenConext\EngineBlock\Authentication\Model\Consent;
22+
use OpenConext\EngineBlock\Authentication\Value\ConsentVersion;
2223

2324
interface ConsentRepository
2425
{
@@ -39,21 +40,9 @@ public function deleteAllFor($userId);
3940
public function deleteOneFor(string $userId, string $serviceProviderEntityId): bool;
4041

4142
/**
42-
* Test if the consent row is set with the legacy (unstable) consent hash
43-
* This is the consent hash that was originally created by EB. It can change
44-
* based on factors that should not result in a hash change per se. Think of the
45-
* change of the attribute ordering, case change or the existence of empty
46-
* attribute values.
43+
* Test if the consent row is set with an attribute hash either stable or unstable
4744
*/
48-
public function hasConsentHash(array $parameters): bool;
49-
50-
/**
51-
* Tests the presence of the stable consent hash
52-
*
53-
* The stable consent hash is used by default, it is not affected by attribute order, case change
54-
* or other irrelevant factors that could result in a changed hash calculation.
55-
*/
56-
public function hasStableConsentHash(array $parameters): bool;
45+
public function hasConsentHash(array $parameters): ConsentVersion;
5746

5847
/**
5948
* By default stores the stable consent hash. The legacy consent hash is left.

‎src/OpenConext/EngineBlock/Service/Consent/ConsentHashService.php‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
namespace OpenConext\EngineBlock\Service\Consent;
2020

2121
use OpenConext\EngineBlock\Authentication\Repository\ConsentRepository;
22+
use OpenConext\EngineBlock\Authentication\Value\ConsentVersion;
2223
use OpenConext\UserLifecycle\Domain\ValueObject\Client\Name;
2324
use SAML2\XML\saml\NameID;
2425
use function array_filter;
@@ -48,16 +49,11 @@ public function __construct(ConsentRepository $consentHashRepository)
4849
$this->consentRepository = $consentHashRepository;
4950
}
5051

51-
public function retrieveConsentHash(array $parameters): bool
52+
public function retrieveConsentHash(array $parameters): ConsentVersion
5253
{
5354
return $this->consentRepository->hasConsentHash($parameters);
5455
}
5556

56-
public function retrieveStableConsentHash(array $parameters): bool
57-
{
58-
return $this->consentRepository->hasStableConsentHash($parameters);
59-
}
60-
6157
public function storeConsentHash(array $parameters): bool
6258
{
6359
return $this->consentRepository->storeConsentHash($parameters);

‎src/OpenConext/EngineBlock/Service/Consent/ConsentHashServiceInterface.php‎

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -18,17 +18,14 @@
1818

1919
namespace OpenConext\EngineBlock\Service\Consent;
2020

21+
use OpenConext\EngineBlock\Authentication\Value\ConsentVersion;
22+
2123
interface ConsentHashServiceInterface
2224
{
2325
/**
24-
* Retrieve the old-style (deprecated) unstable consent hash
25-
*/
26-
public function retrieveConsentHash(array $parameters): bool;
27-
28-
/**
29-
* Retrieve the stable consent hash
26+
* Retrieve the consent hash
3027
*/
31-
public function retrieveStableConsentHash(array $parameters): bool;
28+
public function retrieveConsentHash(array $parameters): ConsentVersion;
3229

3330
public function storeConsentHash(array $parameters): bool;
3431

‎src/OpenConext/EngineBlockBundle/Authentication/Repository/DbalConsentRepository.php‎

Lines changed: 8 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
use OpenConext\EngineBlock\Authentication\Model\Consent;
2727
use OpenConext\EngineBlock\Authentication\Repository\ConsentRepository;
2828
use OpenConext\EngineBlock\Authentication\Value\ConsentType;
29+
use OpenConext\EngineBlock\Authentication\Value\ConsentVersion;
2930
use OpenConext\EngineBlock\Exception\RuntimeException;
3031
use PDO;
3132
use PDOException;
@@ -177,7 +178,7 @@ public function deleteOneFor(string $userId, string $serviceProviderEntityId): b
177178
/**
178179
* @throws RuntimeException
179180
*/
180-
public function hasConsentHash(array $parameters): bool
181+
public function hasConsentHash(array $parameters): ConsentVersion
181182
{
182183
try {
183184
$query = " SELECT
@@ -189,7 +190,7 @@ public function hasConsentHash(array $parameters): bool
189190
AND
190191
service_id = ?
191192
AND
192-
attribute = ?
193+
(attribute = ? OR attribute_stable = ?)
193194
AND
194195
consent_type = ?
195196
AND
@@ -202,45 +203,17 @@ public function hasConsentHash(array $parameters): bool
202203

203204
if (count($rows) < 1) {
204205
// No stored consent found
205-
return false;
206+
return ConsentVersion::notGiven();
206207
}
207208

208-
return true;
209+
if ($rows[0]['attribute_stable'] !== '') {
210+
return ConsentVersion::stable();
211+
}
212+
return ConsentVersion::unstable();
209213
} catch (PDOException $e) {
210214
throw new RuntimeException(sprintf('Consent retrieval failed! Error: "%s"', $e->getMessage()));
211215
}
212216
}
213-
/**
214-
* @throws RuntimeException
215-
*/
216-
public function hasStableConsentHash(array $parameters): bool
217-
{
218-
try {
219-
$query = " SELECT
220-
*
221-
FROM
222-
consent
223-
WHERE
224-
hashed_user_id = ?
225-
AND
226-
service_id = ?
227-
AND
228-
attribute_stable = ?
229-
AND
230-
consent_type = ?
231-
AND
232-
deleted_at IS NULL
233-
";
234-
235-
$statement = $this->connection->prepare($query);
236-
$statement->execute($parameters);
237-
$rows = $statement->fetchAll();
238-
239-
return count($rows) >= 1;
240-
} catch (PDOException $e) {
241-
throw new RuntimeException(sprintf('Consent retrieval on stable consent hash failed! Error: "%s"', $e->getMessage()));
242-
}
243-
}
244217

245218
/**
246219
* @throws RuntimeException

‎tests/library/EngineBlock/Test/Corto/Model/ConsentIntegrationTest.php‎

Lines changed: 13 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
use Mockery\MockInterface;
2121
use OpenConext\EngineBlock\Authentication\Repository\ConsentRepository;
2222
use OpenConext\EngineBlock\Authentication\Value\ConsentType;
23+
use OpenConext\EngineBlock\Authentication\Value\ConsentVersion;
2324
use OpenConext\EngineBlock\Metadata\Entity\ServiceProvider;
2425
use OpenConext\EngineBlock\Service\Consent\ConsentHashService;
2526
use OpenConext\EngineBlockBundle\Authentication\Repository\DbalConsentRepository;
@@ -73,11 +74,7 @@ public function test_no_previous_consent_given($consentType)
7374
$this->consentRepository
7475
->shouldReceive('hasConsentHash')
7576
->once()
76-
->andReturn(false);
77-
$this->consentRepository
78-
->shouldReceive('hasStableConsentHash')
79-
->once()
80-
->andReturn(false);
77+
->andReturn(ConsentVersion::notGiven());
8178
switch ($consentType) {
8279
case ConsentType::TYPE_EXPLICIT:
8380
$this->assertFalse($this->consent->explicitConsentWasGivenFor($serviceProvider));
@@ -98,18 +95,11 @@ public function test_unstable_previous_consent_given($consentType)
9895
->once()
9996
->andReturn('collab:person:id:org-a:joe-a');
10097
// Stable consent is not yet stored
101-
$this->consentRepository
102-
->shouldReceive('hasStableConsentHash')
103-
->with(['0e54805079c56c2b1c1197a760af86ac337b7bac', 'service-provider-entity-id', '8739602554c7f3241958e3cc9b57fdecb474d508', $consentType])
104-
->once()
105-
->andReturn(false);
106-
// Old-style (unstable) consent was given previously
10798
$this->consentRepository
10899
->shouldReceive('hasConsentHash')
109-
->with(['0e54805079c56c2b1c1197a760af86ac337b7bac', 'service-provider-entity-id', '8739602554c7f3241958e3cc9b57fdecb474d508', $consentType])
100+
->with(['0e54805079c56c2b1c1197a760af86ac337b7bac', 'service-provider-entity-id', '8739602554c7f3241958e3cc9b57fdecb474d508', '8739602554c7f3241958e3cc9b57fdecb474d508', $consentType])
110101
->once()
111-
->andReturn(true);
112-
102+
->andReturn(ConsentVersion::unstable());
113103

114104
switch ($consentType) {
115105
case ConsentType::TYPE_EXPLICIT:
@@ -132,13 +122,10 @@ public function test_stable_consent_given($consentType)
132122
->andReturn('collab:person:id:org-a:joe-a');
133123
// Stable consent is not yet stored
134124
$this->consentRepository
135-
->shouldReceive('hasStableConsentHash')
136-
->with(['0e54805079c56c2b1c1197a760af86ac337b7bac', 'service-provider-entity-id', '8739602554c7f3241958e3cc9b57fdecb474d508', $consentType])
125+
->shouldReceive('hasConsentHash')
126+
->with(['0e54805079c56c2b1c1197a760af86ac337b7bac', 'service-provider-entity-id', '8739602554c7f3241958e3cc9b57fdecb474d508', '8739602554c7f3241958e3cc9b57fdecb474d508', $consentType])
137127
->once()
138-
->andReturn(true);
139-
// Old-style (unstable) consent was given previously
140-
$this->consentRepository
141-
->shouldNotReceive('hasConsentHash');
128+
->andReturn(ConsentVersion::stable());
142129

143130
switch ($consentType) {
144131
case ConsentType::TYPE_EXPLICIT:
@@ -211,18 +198,12 @@ public function test_upgrade_to_stable_consent($consentType)
211198
$this->response->shouldReceive('getNameIdValue')
212199
->twice()
213200
->andReturn('collab:person:id:org-a:joe-a');
214-
// Stable consent is not yet stored
215-
$this->consentRepository
216-
->shouldReceive('hasStableConsentHash')
217-
->with(['0e54805079c56c2b1c1197a760af86ac337b7bac', 'service-provider-entity-id', '8739602554c7f3241958e3cc9b57fdecb474d508', $consentType])
218-
->once()
219-
->andReturn(false);
220201
// Old-style (unstable) consent was given previously
221202
$this->consentRepository
222203
->shouldReceive('hasConsentHash')
223-
->with(['0e54805079c56c2b1c1197a760af86ac337b7bac', 'service-provider-entity-id', '8739602554c7f3241958e3cc9b57fdecb474d508', $consentType])
204+
->with(['0e54805079c56c2b1c1197a760af86ac337b7bac', 'service-provider-entity-id', '8739602554c7f3241958e3cc9b57fdecb474d508', '8739602554c7f3241958e3cc9b57fdecb474d508', $consentType])
224205
->once()
225-
->andReturn(true);
206+
->andReturn(ConsentVersion::unstable());
226207
// Now assert that the new stable consent hash is going to be set
227208
$this->consentRepository
228209
->shouldReceive('updateConsentHash')
@@ -242,12 +223,12 @@ public function test_upgrade_to_stable_consent_not_applied_when_stable($consentT
242223
$this->response->shouldReceive('getNameIdValue')
243224
->once()
244225
->andReturn('collab:person:id:org-a:joe-a');
245-
// Stable consent is not yet stored
226+
// Stable consent is stored
246227
$this->consentRepository
247-
->shouldReceive('hasStableConsentHash')
248-
->with(['0e54805079c56c2b1c1197a760af86ac337b7bac', 'service-provider-entity-id', '8739602554c7f3241958e3cc9b57fdecb474d508', $consentType])
228+
->shouldReceive('hasConsentHash')
229+
->with(['0e54805079c56c2b1c1197a760af86ac337b7bac', 'service-provider-entity-id', '8739602554c7f3241958e3cc9b57fdecb474d508', '8739602554c7f3241958e3cc9b57fdecb474d508', $consentType])
249230
->once()
250-
->andReturn(true);
231+
->andReturn(ConsentVersion::stable());
251232
// Now assert that the new stable consent hash is NOT going to be set
252233
$this->consentRepository
253234
->shouldNotReceive('storeConsentHash');

‎tests/library/EngineBlock/Test/Corto/Model/ConsentTest.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
*/
1818

1919
use Mockery\Adapter\Phpunit\MockeryPHPUnitIntegration;
20+
use OpenConext\EngineBlock\Authentication\Value\ConsentVersion;
2021
use OpenConext\EngineBlock\Metadata\Entity\ServiceProvider;
2122
use OpenConext\EngineBlock\Service\Consent\ConsentHashServiceInterface;
2223
use PHPUnit\Framework\TestCase;
@@ -76,8 +77,7 @@ public function testConsentWriteToDatabase()
7677
$serviceProvider = new ServiceProvider("service-provider-entity-id");
7778
$this->consentService->shouldReceive('getStableAttributesHash');
7879
$this->consentService->shouldReceive('getUnstableAttributesHash')->andReturn(sha1('unstable'));
79-
$this->consentService->shouldReceive('retrieveConsentHash')->andReturn(sha1('unstable'));
80-
$this->consentService->shouldReceive('retrieveStableConsentHash');
80+
$this->consentService->shouldReceive('retrieveConsentHash')->andReturn(ConsentVersion::stable());
8181
$this->consent->explicitConsentWasGivenFor($serviceProvider);
8282

8383
$this->consentService->shouldReceive('getStableAttributesHash')->andReturn(sha1('stable'));

0 commit comments

Comments
 (0)