Skip to content

Commit f05b277

Browse files
kayjoostenkayjoosten
committed
fix(consent): polish follow-up fixes and test configuration
1 parent 069c5db commit f05b277

File tree

16 files changed

+142
-136
lines changed

16 files changed

+142
-136
lines changed

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

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -85,38 +85,39 @@ public function __construct(
8585
$this->_consentEnabled = $consentEnabled;
8686
}
8787

88-
public function explicitConsentWasGivenFor(ServiceProvider $serviceProvider): bool
88+
public function explicitConsentWasGivenFor(ServiceProvider $serviceProvider): ConsentVersion
8989
{
9090
if (!$this->_consentEnabled) {
91-
return true;
91+
// Consent disabled: treat as already given (stable — no upgrade needed)
92+
return ConsentVersion::stable();
9293
}
93-
$consent = $this->_hasStoredConsent($serviceProvider, ConsentType::TYPE_EXPLICIT);
94-
return $consent->given();
94+
return $this->_hasStoredConsent($serviceProvider, ConsentType::TYPE_EXPLICIT);
9595
}
9696

9797
/**
9898
* Although the user has given consent previously we want to upgrade the deprecated unstable consent
9999
* to the new stable consent type.
100100
* https://www.pivotaltracker.com/story/show/176513931
101+
*
102+
* The caller must pass the ConsentVersion already retrieved by explicitConsentWasGivenFor or
103+
* implicitConsentWasGivenFor to avoid a second identical DB query.
101104
*/
102-
public function upgradeAttributeHashFor(ServiceProvider $serviceProvider, string $consentType): void
105+
public function upgradeAttributeHashFor(ServiceProvider $serviceProvider, string $consentType, ConsentVersion $consentVersion): void
103106
{
104107
if (!$this->_consentEnabled) {
105108
return;
106109
}
107-
$consentVersion = $this->_hasStoredConsent($serviceProvider, $consentType);
108110
if ($consentVersion->isUnstable()) {
109111
$this->_updateConsent($serviceProvider, $consentType);
110112
}
111113
}
112114

113-
public function implicitConsentWasGivenFor(ServiceProvider $serviceProvider): bool
115+
public function implicitConsentWasGivenFor(ServiceProvider $serviceProvider): ConsentVersion
114116
{
115117
if (!$this->_consentEnabled) {
116-
return true;
118+
return ConsentVersion::stable();
117119
}
118-
$consent = $this->_hasStoredConsent($serviceProvider, ConsentType::TYPE_IMPLICIT);
119-
return $consent->given();
120+
return $this->_hasStoredConsent($serviceProvider, ConsentType::TYPE_IMPLICIT);
120121
}
121122

122123
public function giveExplicitConsentFor(ServiceProvider $serviceProvider): bool

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -100,10 +100,11 @@ public function serve($serviceName, Request $httpRequest)
100100
$attributes = $response->getAssertion()->getAttributes();
101101
$consentRepository = $this->_consentFactory->create($this->_server, $response, $attributes);
102102

103-
if (!$consentRepository->explicitConsentWasGivenFor($serviceProvider)) {
103+
$explicitConsent = $consentRepository->explicitConsentWasGivenFor($serviceProvider);
104+
if (!$explicitConsent->given()) {
104105
$consentRepository->giveExplicitConsentFor($destinationMetadata);
105106
} else {
106-
$consentRepository->upgradeAttributeHashFor($destinationMetadata, ConsentType::TYPE_EXPLICIT);
107+
$consentRepository->upgradeAttributeHashFor($destinationMetadata, ConsentType::TYPE_EXPLICIT, $explicitConsent);
107108
}
108109

109110
$response->setConsent(Constants::CONSENT_OBTAINED);

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

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -147,10 +147,11 @@ public function serve($serviceName, Request $httpRequest)
147147

148148

149149
if ($this->isConsentDisabled($spMetadataChain, $identityProvider)) {
150-
if (!$consentRepository->implicitConsentWasGivenFor($serviceProviderMetadata)) {
150+
$implicitConsent = $consentRepository->implicitConsentWasGivenFor($serviceProviderMetadata);
151+
if (!$implicitConsent->given()) {
151152
$consentRepository->giveImplicitConsentFor($serviceProviderMetadata);
152153
} else {
153-
$consentRepository->upgradeAttributeHashFor($serviceProviderMetadata, ConsentType::TYPE_IMPLICIT);
154+
$consentRepository->upgradeAttributeHashFor($serviceProviderMetadata, ConsentType::TYPE_IMPLICIT, $implicitConsent);
154155
}
155156

156157
$response->setConsent(Constants::CONSENT_INAPPLICABLE);
@@ -168,8 +169,8 @@ public function serve($serviceName, Request $httpRequest)
168169
}
169170

170171
$priorConsent = $consentRepository->explicitConsentWasGivenFor($serviceProviderMetadata);
171-
if ($priorConsent) {
172-
$consentRepository->upgradeAttributeHashFor($serviceProviderMetadata, ConsentType::TYPE_EXPLICIT);
172+
if ($priorConsent->given()) {
173+
$consentRepository->upgradeAttributeHashFor($serviceProviderMetadata, ConsentType::TYPE_EXPLICIT, $priorConsent);
173174

174175
$response->setConsent(Constants::CONSENT_PRIOR);
175176

migrations/DoctrineMigrations/Version20220503092351.php renamed to migrations/DoctrineMigrations/Version20260315000001.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
* 1. Added the `attribute_stable` column, string(80), nullable
1313
* 2. Changed the `attribute` column, has been made nullable
1414
*/
15-
final class Version20220503092351 extends AbstractMigration
15+
final class Version20260315000001 extends AbstractMigration
1616
{
1717
public function getDescription(): string
1818
{

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,5 +58,5 @@ public function storeConsentHash(ConsentStoreParameters $parameters): bool;
5858
*/
5959
public function updateConsentHash(ConsentUpdateParameters $parameters): bool;
6060

61-
public function countTotalConsent($consentUid): int;
61+
public function countTotalConsent(string $consentUid): int;
6262
}

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

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@
2323
use OpenConext\EngineBlock\Authentication\Value\ConsentStoreParameters;
2424
use OpenConext\EngineBlock\Authentication\Value\ConsentUpdateParameters;
2525
use OpenConext\EngineBlock\Authentication\Value\ConsentVersion;
26-
use OpenConext\UserLifecycle\Domain\ValueObject\Client\Name;
2726
use SAML2\XML\saml\NameID;
2827
use function array_filter;
2928
use function array_keys;
@@ -33,11 +32,10 @@
3332
use function is_array;
3433
use function is_numeric;
3534
use function ksort;
35+
use function mb_strtolower;
3636
use function serialize;
3737
use function sha1;
3838
use function sort;
39-
use function str_replace;
40-
use function strtolower;
4139
use function unserialize;
4240

4341
final class ConsentHashService implements ConsentHashServiceInterface
@@ -67,7 +65,7 @@ public function updateConsentHash(ConsentUpdateParameters $parameters): bool
6765
return $this->consentRepository->updateConsentHash($parameters);
6866
}
6967

70-
public function countTotalConsent($consentUid): int
68+
public function countTotalConsent(string $consentUid): int
7169
{
7270
return $this->consentRepository->countTotalConsent($consentUid);
7371
}
@@ -114,14 +112,27 @@ private function createHashBaseWithoutValues(array $lowerCasedAttributes): strin
114112
}
115113

116114
/**
117-
* Lowercases all array keys and values.
115+
* Lowercases all array keys and string values recursively using mb_strtolower
116+
* to handle multi-byte UTF-8 characters (e.g. Ü→ü, Arabic, Chinese — common in SAML).
117+
*
118+
* The previous implementation used serialize/strtolower/unserialize which corrupted
119+
* PHP's s:N: byte-length markers for multi-byte values, causing unserialize() to silently
120+
* return false and producing wrong hashes for any user with a non-ASCII attribute value.
118121
*/
119122
private function caseNormalizeStringArray(array $original): array
120123
{
121-
$serialized = serialize($original);
122-
$lowerCased = strtolower($serialized);
123-
$unserialized = unserialize($lowerCased);
124-
return $unserialized;
124+
$result = [];
125+
foreach ($original as $key => $value) {
126+
$normalizedKey = is_string($key) ? mb_strtolower($key) : $key;
127+
if (is_array($value)) {
128+
$result[$normalizedKey] = $this->caseNormalizeStringArray($value);
129+
} elseif (is_string($value)) {
130+
$result[$normalizedKey] = mb_strtolower($value);
131+
} else {
132+
$result[$normalizedKey] = $value;
133+
}
134+
}
135+
return $result;
125136
}
126137

127138
/**

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ public function storeConsentHash(ConsentStoreParameters $parameters): bool;
3434

3535
public function updateConsentHash(ConsentUpdateParameters $parameters): bool;
3636

37-
public function countTotalConsent($consentUid): int;
37+
public function countTotalConsent(string $consentUid): int;
3838

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

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -228,7 +228,8 @@ public function storeConsentHash(ConsentStoreParameters $parameters): bool
228228
{
229229
$query = "INSERT INTO consent (hashed_user_id, service_id, attribute_stable, consent_type, consent_date, deleted_at)
230230
VALUES (?, ?, ?, ?, NOW(), '0000-00-00 00:00:00')
231-
ON DUPLICATE KEY UPDATE attribute_stable=VALUES(attribute_stable), consent_type=VALUES(consent_type), consent_date=NOW()";
231+
ON DUPLICATE KEY UPDATE attribute_stable=VALUES(attribute_stable),
232+
consent_type=VALUES(consent_type), consent_date=NOW(), deleted_at='0000-00-00 00:00:00'";
232233

233234
try {
234235
$this->connection->executeStatement($query, [
@@ -301,7 +302,7 @@ public function updateConsentHash(ConsentUpdateParameters $parameters): bool
301302
/**
302303
* @throws RuntimeException
303304
*/
304-
public function countTotalConsent($consentUid): int
305+
public function countTotalConsent(string $consentUid): int
305306
{
306307
$query = "SELECT COUNT(*) FROM consent where hashed_user_id = ? AND deleted_at IS NULL";
307308
$parameters = [sha1($consentUid)];

0 commit comments

Comments
 (0)