Skip to content

Commit 145ebd9

Browse files
MKoddeMKodde
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 54433bf commit 145ebd9

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
@@ -178,31 +178,13 @@ private function _updateConsent(ServiceProvider $serviceProvider, $consentType):
178178
private function _hasStoredConsent(ServiceProvider $serviceProvider, $consentType): ConsentVersion
179179
{
180180
$consentUuid = sha1($this->_getConsentUid());
181-
$parameters = [
182-
$consentUuid,
183-
$serviceProvider->entityId,
184-
$this->_getStableAttributesHash($this->_responseAttributes),
185-
$consentType,
186-
];
187-
$hasStableConsentHash = $this->_hashService->retrieveStableConsentHash($parameters);
188-
189-
if ($hasStableConsentHash) {
190-
return ConsentVersion::stable();
191-
}
192-
193181
$parameters = [
194182
$consentUuid,
195183
$serviceProvider->entityId,
196184
$this->_getAttributesHash($this->_responseAttributes),
185+
$this->_getStableAttributesHash($this->_responseAttributes),
197186
$consentType,
198187
];
199-
200-
$hasUnstableConsentHash = $this->_hashService->retrieveConsentHash($parameters);
201-
202-
if ($hasUnstableConsentHash) {
203-
return ConsentVersion::unstable();
204-
}
205-
206-
return ConsentVersion::notGiven();
188+
return $this->_hashService->retrieveConsentHash($parameters);
207189
}
208190
}

‎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
@@ -24,6 +24,7 @@
2424
use OpenConext\EngineBlock\Authentication\Model\Consent;
2525
use OpenConext\EngineBlock\Authentication\Repository\ConsentRepository;
2626
use OpenConext\EngineBlock\Authentication\Value\ConsentType;
27+
use OpenConext\EngineBlock\Authentication\Value\ConsentVersion;
2728
use OpenConext\EngineBlock\Exception\RuntimeException;
2829
use PDO;
2930
use PDOException;
@@ -167,7 +168,7 @@ public function deleteOneFor(string $userId, string $serviceProviderEntityId): b
167168
/**
168169
* @throws RuntimeException
169170
*/
170-
public function hasConsentHash(array $parameters): bool
171+
public function hasConsentHash(array $parameters): ConsentVersion
171172
{
172173
try {
173174
$query = " SELECT
@@ -179,7 +180,7 @@ public function hasConsentHash(array $parameters): bool
179180
AND
180181
service_id = ?
181182
AND
182-
attribute = ?
183+
(attribute = ? OR attribute_stable = ?)
183184
AND
184185
consent_type = ?
185186
AND
@@ -192,45 +193,17 @@ public function hasConsentHash(array $parameters): bool
192193

193194
if (count($rows) < 1) {
194195
// No stored consent found
195-
return false;
196+
return ConsentVersion::notGiven();
196197
}
197198

198-
return true;
199+
if ($rows[0]['attribute_stable'] !== '') {
200+
return ConsentVersion::stable();
201+
}
202+
return ConsentVersion::unstable();
199203
} catch (PDOException $e) {
200204
throw new RuntimeException(sprintf('Consent retrieval failed! Error: "%s"', $e->getMessage()));
201205
}
202206
}
203-
/**
204-
* @throws RuntimeException
205-
*/
206-
public function hasStableConsentHash(array $parameters): bool
207-
{
208-
try {
209-
$query = " SELECT
210-
*
211-
FROM
212-
consent
213-
WHERE
214-
hashed_user_id = ?
215-
AND
216-
service_id = ?
217-
AND
218-
attribute_stable = ?
219-
AND
220-
consent_type = ?
221-
AND
222-
deleted_at IS NULL
223-
";
224-
225-
$statement = $this->connection->prepare($query);
226-
$statement->execute($parameters);
227-
$rows = $statement->fetchAll();
228-
229-
return count($rows) >= 1;
230-
} catch (PDOException $e) {
231-
throw new RuntimeException(sprintf('Consent retrieval on stable consent hash failed! Error: "%s"', $e->getMessage()));
232-
}
233-
}
234207

235208
/**
236209
* @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)