Skip to content

Commit dae614b

Browse files
MKoddeMKodde
committed
Integrate stable attribute hash requirements
The requirements are stated in the story: https://www.pivotaltracker.com/story/show/176513931 and specifically these points: A challenge is that we do not want to invalidate all given consents with the current algorithm. So we implement it as follows: * We do not touch the existing consent hashing method at all * We create a new hashing method that is more stable. * We cover this new method with an abundance of unit tests to verify the stability given all sorts of inputs. * We change the consent query from (pseudocode): SELECT * FROM consent WHERE user = me AND consenthash = hashfromoldmethod OR consenthash = hashfromnewmethod * Newly given consent will be stored with the new hash. * When old consent matched, still generate new consent hash (without showing consent screen
1 parent 167fb9e commit dae614b

File tree

11 files changed

+445
-21
lines changed

11 files changed

+445
-21
lines changed

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

Lines changed: 47 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
* limitations under the License.
1717
*/
1818

19+
use OpenConext\EngineBlock\Authentication\Value\ConsentVersion;
1920
use OpenConext\EngineBlock\Metadata\Entity\ServiceProvider;
2021
use OpenConext\EngineBlock\Authentication\Value\ConsentType;
2122
use OpenConext\EngineBlock\Service\Consent\ConsentHashServiceInterface;
@@ -83,14 +84,27 @@ public function __construct(
8384

8485
public function explicitConsentWasGivenFor(ServiceProvider $serviceProvider): bool
8586
{
86-
return !$this->_consentEnabled ||
87-
$this->_hasStoredConsent($serviceProvider, ConsentType::TYPE_EXPLICIT);
87+
$consent = $this->_hasStoredConsent($serviceProvider, ConsentType::TYPE_EXPLICIT);
88+
return !$this->_consentEnabled || $consent->given();
89+
}
90+
91+
/**
92+
* Although the user has given consent previously we want to upgrade the deprecated unstable consent
93+
* to the new stable consent type.
94+
* https://www.pivotaltracker.com/story/show/176513931
95+
*/
96+
public function upgradeAttributeHashFor(ServiceProvider $serviceProvider, string $consentType): void
97+
{
98+
$consentVersion = $this->_hasStoredConsent($serviceProvider, $consentType);
99+
if ($consentVersion->isUnstable()) {
100+
$this->_updateConsent($serviceProvider, $consentType);
101+
}
88102
}
89103

90104
public function implicitConsentWasGivenFor(ServiceProvider $serviceProvider): bool
91105
{
92-
return !$this->_consentEnabled ||
93-
$this->_hasStoredConsent($serviceProvider, ConsentType::TYPE_IMPLICIT);
106+
$consent = $this->_hasStoredConsent($serviceProvider, ConsentType::TYPE_IMPLICIT);
107+
return !$this->_consentEnabled || $consent->given();
94108
}
95109

96110
public function giveExplicitConsentFor(ServiceProvider $serviceProvider): bool
@@ -142,28 +156,47 @@ private function _storeConsent(ServiceProvider $serviceProvider, $consentType):
142156
return $this->_hashService->storeConsentHash($parameters);
143157
}
144158

145-
private function _hasStoredConsent(ServiceProvider $serviceProvider, $consentType): bool
159+
private function _updateConsent(ServiceProvider $serviceProvider, $consentType): bool
146160
{
147161
$parameters = array(
162+
$this->_getStableAttributesHash($this->_responseAttributes),
163+
$this->_getAttributesHash($this->_responseAttributes),
148164
sha1($this->_getConsentUid()),
149165
$serviceProvider->entityId,
150-
$this->_getAttributesHash($this->_responseAttributes),
151166
$consentType,
152167
);
153168

154-
$hasUnstableConsentHash = $this->_hashService->retrieveConsentHash($parameters);
169+
return $this->_hashService->updateConsentHash($parameters);
170+
}
155171

156-
if ($hasUnstableConsentHash) {
157-
return true;
172+
private function _hasStoredConsent(ServiceProvider $serviceProvider, $consentType): ConsentVersion
173+
{
174+
$consentUuid = sha1($this->_getConsentUid());
175+
$parameters = [
176+
$consentUuid,
177+
$serviceProvider->entityId,
178+
$this->_getStableAttributesHash($this->_responseAttributes),
179+
$consentType,
180+
];
181+
$hasStableConsentHash = $this->_hashService->retrieveStableConsentHash($parameters);
182+
183+
if ($hasStableConsentHash) {
184+
return ConsentVersion::stable();
158185
}
159186

160-
$parameters[2] = array(
161-
sha1($this->_getConsentUid()),
187+
$parameters = [
188+
$consentUuid,
162189
$serviceProvider->entityId,
163-
$this->_getStableAttributesHash($this->_responseAttributes),
190+
$this->_getAttributesHash($this->_responseAttributes),
164191
$consentType,
165-
);
192+
];
193+
194+
$hasUnstableConsentHash = $this->_hashService->retrieveConsentHash($parameters);
195+
196+
if ($hasUnstableConsentHash) {
197+
return ConsentVersion::unstable();
198+
}
166199

167-
return $this->_hashService->retrieveConsentHash($parameters);
200+
return ConsentVersion::notGiven();
168201
}
169202
}

‎library/EngineBlock/Corto/Module/Service/ProcessConsent.php‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
* limitations under the License.
1717
*/
1818

19+
use OpenConext\EngineBlock\Authentication\Value\ConsentType;
1920
use OpenConext\EngineBlock\Service\AuthenticationStateHelperInterface;
2021
use OpenConext\EngineBlock\Service\ProcessingStateHelperInterface;
2122
use SAML2\Constants;
@@ -102,6 +103,7 @@ public function serve($serviceName, Request $httpRequest)
102103
if (!$consentRepository->explicitConsentWasGivenFor($serviceProvider)) {
103104
$consentRepository->giveExplicitConsentFor($destinationMetadata);
104105
}
106+
$consentRepository->upgradeAttributeHashFor($destinationMetadata, ConsentType::TYPE_EXPLICIT);
105107

106108
$response->setConsent(Constants::CONSENT_OBTAINED);
107109
$response->setDestination($response->getReturn());

‎library/EngineBlock/Corto/Module/Service/ProvideConsent.php‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
* limitations under the License.
1717
*/
1818

19+
use OpenConext\EngineBlock\Authentication\Value\ConsentType;
1920
use OpenConext\EngineBlock\Metadata\Entity\IdentityProvider;
2021
use OpenConext\EngineBlock\Metadata\Entity\ServiceProvider;
2122
use OpenConext\EngineBlock\Service\AuthenticationStateHelperInterface;
@@ -136,6 +137,7 @@ public function serve($serviceName, Request $httpRequest)
136137
if (!$consentRepository->implicitConsentWasGivenFor($serviceProviderMetadata)) {
137138
$consentRepository->giveImplicitConsentFor($serviceProviderMetadata);
138139
}
140+
$consentRepository->upgradeAttributeHashFor($serviceProviderMetadata, ConsentType::TYPE_IMPLICIT);
139141

140142
$response->setConsent(Constants::CONSENT_INAPPLICABLE);
141143
$response->setDestination($response->getReturn());
@@ -153,6 +155,8 @@ public function serve($serviceName, Request $httpRequest)
153155

154156
$priorConsent = $consentRepository->explicitConsentWasGivenFor($serviceProviderMetadata);
155157
if ($priorConsent) {
158+
$consentRepository->upgradeAttributeHashFor($serviceProviderMetadata, ConsentType::TYPE_EXPLICIT);
159+
156160
$response->setConsent(Constants::CONSENT_PRIOR);
157161

158162
$response->setDestination($response->getReturn());

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

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,9 +38,33 @@ public function deleteAllFor($userId);
3838

3939
public function deleteOneFor(string $userId, string $serviceProviderEntityId): bool;
4040

41+
/**
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.
47+
*/
4148
public function hasConsentHash(array $parameters): bool;
4249

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;
57+
58+
/**
59+
* By default stores the stable consent hash. The legacy consent hash is left.
60+
*/
4361
public function storeConsentHash(array $parameters): bool;
4462

63+
/**
64+
* When a deprecated unstable consent hash is encoutered, we upgrade it to the new format using this
65+
* update consent hash method.
66+
*/
67+
public function updateConsentHash(array $parameters): bool;
68+
4569
public function countTotalConsent($consentUid): int;
4670
}

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,11 +50,21 @@ public function retrieveConsentHash(array $parameters): bool
5050
return $this->consentRepository->hasConsentHash($parameters);
5151
}
5252

53+
public function retrieveStableConsentHash(array $parameters): bool
54+
{
55+
return $this->consentRepository->hasStableConsentHash($parameters);
56+
}
57+
5358
public function storeConsentHash(array $parameters): bool
5459
{
5560
return $this->consentRepository->storeConsentHash($parameters);
5661
}
5762

63+
public function updateConsentHash(array $parameters): bool
64+
{
65+
return $this->consentRepository->updateConsentHash($parameters);
66+
}
67+
5868
public function countTotalConsent($consentUid): int
5969
{
6070
return $this->consentRepository->countTotalConsent($consentUid);

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,20 @@
2020

2121
interface ConsentHashServiceInterface
2222
{
23+
/**
24+
* Retrieve the old-style (deprecated) unstable consent hash
25+
*/
2326
public function retrieveConsentHash(array $parameters): bool;
2427

28+
/**
29+
* Retrieve the stable consent hash
30+
*/
31+
public function retrieveStableConsentHash(array $parameters): bool;
32+
2533
public function storeConsentHash(array $parameters): bool;
2634

35+
public function updateConsentHash(array $parameters): bool;
36+
2737
public function countTotalConsent($consentUid): int;
2838

2939
public function getUnstableAttributesHash(array $attributes, bool $mustStoreValues): string;

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

Lines changed: 68 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -200,15 +200,46 @@ public function hasConsentHash(array $parameters): bool
200200
throw new RuntimeException(sprintf('Consent retrieval failed! Error: "%s"', $e->getMessage()));
201201
}
202202
}
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+
}
203234

204235
/**
205236
* @throws RuntimeException
206237
*/
207238
public function storeConsentHash(array $parameters): bool
208239
{
209-
$query = "INSERT INTO consent (hashed_user_id, service_id, attribute, consent_type, consent_date, deleted_at)
240+
$query = "INSERT INTO consent (hashed_user_id, service_id, attribute_stable, consent_type, consent_date, deleted_at)
210241
VALUES (?, ?, ?, ?, NOW(), '0000-00-00 00:00:00')
211-
ON DUPLICATE KEY UPDATE attribute=VALUES(attribute), consent_type=VALUES(consent_type), consent_date=NOW()";
242+
ON DUPLICATE KEY UPDATE attribute_stable=VALUES(attribute_stable), consent_type=VALUES(consent_type), consent_date=NOW()";
212243
$statement = $this->connection->prepare($query);
213244
if (!$statement) {
214245
throw new RuntimeException("Unable to create a prepared statement to insert consent?!");
@@ -223,6 +254,41 @@ public function storeConsentHash(array $parameters): bool
223254
return true;
224255
}
225256

257+
/**
258+
* @throws RuntimeException
259+
*/
260+
public function updateConsentHash(array $parameters): bool
261+
{
262+
$query = "
263+
UPDATE
264+
consent
265+
SET
266+
attribute_stable = ?
267+
WHERE
268+
attribute = ?
269+
AND
270+
hashed_user_id = ?
271+
AND
272+
service_id = ?
273+
AND
274+
consent_type = ?
275+
AND
276+
deleted_at IS NULL
277+
";
278+
$statement = $this->connection->prepare($query);
279+
if (!$statement) {
280+
throw new RuntimeException("Unable to create a prepared statement to update consent?!");
281+
}
282+
283+
if (!$statement->execute($parameters)) {
284+
throw new RuntimeException(
285+
sprintf('Error storing updated consent: "%s"', var_export($statement->errorInfo(), true))
286+
);
287+
}
288+
289+
return true;
290+
}
291+
226292
/**
227293
* @throws RuntimeException
228294
*/

‎tests/functional/OpenConext/EngineBlockBundle/Controller/Api/ConsentControllerTest.php‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -507,23 +507,31 @@ private function disableConsentApiFeatureFor(Client $client)
507507
$container->set('engineblock.features', $featureToggles);
508508
}
509509

510-
private function addConsentFixture($userId, $serviceId, $attributeHash, $consentType, $consentDate, $deletedAt)
511-
{
510+
private function addConsentFixture(
511+
$userId,
512+
$serviceId,
513+
$attributeHash,
514+
$consentType,
515+
$consentDate,
516+
$deletedAt
517+
) {
512518
$queryBuilder = $this->getContainer()->get('doctrine')->getConnection()->createQueryBuilder();
513519
$queryBuilder
514520
->insert('consent')
515521
->values([
516522
'hashed_user_id' => ':user_id',
517523
'service_id' => ':service_id',
518524
'attribute' => ':attribute',
525+
'attribute_stable' => ':attribute_stable',
519526
'consent_type' => ':consent_type',
520527
'consent_date' => ':consent_date',
521528
'deleted_at' => ':deleted_at',
522529
])
523530
->setParameters([
524531
':user_id' => sha1($userId),
525532
':service_id' => $serviceId,
526-
':attribute' => $attributeHash,
533+
':attribute' => '',
534+
':attribute_stable' => $attributeHash,
527535
':consent_type' => $consentType,
528536
':consent_date' => $consentDate,
529537
':deleted_at' => $deletedAt,

‎tests/functional/OpenConext/EngineBlockBundle/Controller/Api/DeprovisionControllerTest.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -397,6 +397,7 @@ private function addConsentFixture($userId, $serviceId, $attributeHash, $consent
397397
'hashed_user_id' => ':user_id',
398398
'service_id' => ':service_id',
399399
'attribute' => ':attribute',
400+
'attribute_stable' => ':attribute',
400401
'consent_type' => ':consent_type',
401402
'consent_date' => ':consent_date',
402403
'deleted_at' => '"0000-00-00 00:00:00"',

0 commit comments

Comments
 (0)