Skip to content

Commit 7765e49

Browse files
kayjoostenjohanib
authored andcommitted
Add feature toggle and tests for stable hash migration
Introduces the `feature_stable_consent_hash_migration` toggle (default: false) to control when the legacy `attribute` column is phased out, and adds edge-case test coverage. Toggle OFF (default): new consent writes both `attribute` and `attribute_stable` so old EB instances still find records during a rolling deploy; hash upgrades leave `attribute` intact. Toggle ON: new consent writes only `attribute_stable` (attribute=NULL) and upgrades null the old column, cleaning it up over time once all instances run the new version. Also: - Add edge-case tests for consent hash versioning - Fix test configuration for consent scenarios - Drop redundant unserialize/serialize deep copies in ConsentHashService (PHP passes arrays by value; a mutation-safety regression test is added to guard against future changes)
1 parent 18e5fa7 commit 7765e49

22 files changed

Lines changed: 649 additions & 124 deletions

File tree

config/packages/engineblock_features.yaml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,3 +16,4 @@ parameters:
1616
eb.stepup.sfo.override_engine_entityid: "%feature_stepup_sfo_override_engine_entityid%"
1717
eb.stepup.send_user_attributes: "%feature_stepup_send_user_attributes%"
1818
eb.feature_enable_sram_interrupt: "%feature_enable_sram_interrupt%"
19+
eb.stable_consent_hash_migration: "%feature_stable_consent_hash_migration%"

config/packages/parameters.yml.dist

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -227,6 +227,7 @@ parameters:
227227
feature_stepup_sfo_override_engine_entityid: false
228228
feature_stepup_send_user_attributes: false
229229
feature_enable_sram_interrupt: false
230+
feature_stable_consent_hash_migration: false
230231

231232
##########################################################################################
232233
## PROFILE SETTINGS

config/services/services.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,7 @@ services:
7979
public: false
8080
arguments:
8181
- '@OpenConext\EngineBlockBundle\Authentication\Repository\DbalConsentRepository'
82+
- '@OpenConext\EngineBlockBundle\Configuration\FeatureConfiguration'
8283

8384
OpenConext\EngineBlock\Service\Consent\ConsentService:
8485
arguments:

library/EngineBlock/Corto/Model/Consent.php

Lines changed: 12 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
@@ -168,6 +169,7 @@ private function _storeConsent(ServiceProvider $serviceProvider, $consentType):
168169
serviceId: $serviceProvider->entityId,
169170
attributeStableHash: $this->_getStableAttributesHash($this->_responseAttributes),
170171
consentType: $consentType,
172+
attributeHash: $this->_getAttributesHash($this->_responseAttributes),
171173
);
172174

173175
return $this->_hashService->storeConsentHash($parameters);

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
@@ -144,10 +144,11 @@ public function serve($serviceName, Request $httpRequest)
144144

145145

146146
if ($this->isConsentDisabled($spMetadataChain, $identityProvider)) {
147-
if (!$consentRepository->implicitConsentWasGivenFor($serviceProviderMetadata)) {
147+
$implicitConsent = $consentRepository->implicitConsentWasGivenFor($serviceProviderMetadata);
148+
if (!$implicitConsent->given()) {
148149
$consentRepository->giveImplicitConsentFor($serviceProviderMetadata);
149150
} else {
150-
$consentRepository->upgradeAttributeHashFor($serviceProviderMetadata, ConsentType::TYPE_IMPLICIT);
151+
$consentRepository->upgradeAttributeHashFor($serviceProviderMetadata, ConsentType::TYPE_IMPLICIT, $implicitConsent);
151152
}
152153

153154
$response->setConsent(Constants::CONSENT_INAPPLICABLE);
@@ -165,8 +166,8 @@ public function serve($serviceName, Request $httpRequest)
165166
}
166167

167168
$priorConsent = $consentRepository->explicitConsentWasGivenFor($serviceProviderMetadata);
168-
if ($priorConsent) {
169-
$consentRepository->upgradeAttributeHashFor($serviceProviderMetadata, ConsentType::TYPE_EXPLICIT);
169+
if ($priorConsent->given()) {
170+
$consentRepository->upgradeAttributeHashFor($serviceProviderMetadata, ConsentType::TYPE_EXPLICIT, $priorConsent);
170171

171172
$response->setConsent(Constants::CONSENT_PRIOR);
172173

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,8 @@ public function up(Schema $schema): void
3030

3131
public function down(Schema $schema): void
3232
{
33-
$this->addSql('ALTER TABLE consent DROP attribute_stable, CHANGE attribute attribute VARCHAR(80) CHARACTER SET utf8 NOT NULL COLLATE `utf8_unicode_ci`');
33+
$this->addSql('UPDATE consent SET attribute = attribute_stable WHERE attribute IS NULL AND attribute_stable IS NOT NULL');
34+
$this->addSql('ALTER TABLE consent CHANGE attribute attribute VARCHAR(80) NOT NULL');
35+
$this->addSql('ALTER TABLE consent DROP attribute_stable');
3436
}
3537
}

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/Authentication/Value/ConsentStoreParameters.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ public function __construct(
2525
public readonly string $serviceId,
2626
public readonly string $attributeStableHash,
2727
public readonly string $consentType,
28+
public readonly ?string $attributeHash = null,
2829
) {
2930
}
3031
}

src/OpenConext/EngineBlock/Authentication/Value/ConsentUpdateParameters.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ public function __construct(
2626
public readonly string $hashedUserId,
2727
public readonly string $serviceId,
2828
public readonly string $consentType,
29+
public readonly bool $clearLegacyHash = false,
2930
) {
3031
}
3132
}

0 commit comments

Comments
 (0)