Skip to content

Commit 025f511

Browse files
authored
Revert feature flag to enable/disable content hash cleanup (#1966)
* Revert feature flag to enable/disable content hash cleanup The `feature_stable_consent_hash_migration` feature flag was confusing, and setting the old hash to null provided no real value
1 parent 33ba9a9 commit 025f511

12 files changed

Lines changed: 42 additions & 202 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,9 +30,7 @@ Changes:
3030

3131
* Stabilized consent checks
3232
* In order to make the consent hashes more robust, a more consistent way of hashing the user attributes has been introduced
33-
* This feature automatically migrates from the old hashes to the new hashes, cleaning up the old hash.
34-
* However, if blue/green deployments are used or if you want to keep the option open to roll back the EB release, keep the `feature_stable_consent_hash_migration` set to false in order to preserve the old consent hashes.
35-
* Once the new release is fully rolled out, set `feature_stable_consent_hash_migration` to true. This will clean up the old consent hashes upon login. In the next EB release, the old consent hash column will be deleted.
33+
* This feature automatically migrates from the old hashes to the new hashes upon login.
3634

3735
## 7.1.0
3836
[SBS](https://github.com/SURFscz/SBS) integration

config/packages/engineblock_features.yaml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,4 +16,3 @@ 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: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -227,7 +227,6 @@ 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
231230

232231
##########################################################################################
233232
## PROFILE SETTINGS

config/services/services.yml

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

8483
OpenConext\EngineBlock\Service\Consent\ConsentService:
8584
arguments:

migrations/DoctrineMigrations/Version20260315000001.php

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -25,13 +25,12 @@
2525
/**
2626
* Change to the consent schema
2727
* 1. Added the `attribute_stable` column, string(80), nullable
28-
* 2. Changed the `attribute` column, has been made nullable
2928
*/
3029
final class Version20260315000001 extends AbstractEngineBlockMigration
3130
{
3231
public function getDescription(): string
3332
{
34-
return 'Add attribute_stable column to consent table and make attribute nullable';
33+
return 'Add attribute_stable column to consent table';
3534
}
3635

3736
public function preUp(Schema $schema): void
@@ -41,14 +40,11 @@ public function preUp(Schema $schema): void
4140

4241
public function up(Schema $schema): void
4342
{
44-
$this->addSql('ALTER TABLE consent ADD attribute_stable VARCHAR(80) DEFAULT NULL, CHANGE attribute attribute VARCHAR(80) DEFAULT NULL');
43+
$this->addSql('ALTER TABLE consent ADD attribute_stable VARCHAR(80) DEFAULT NULL');
4544
}
4645

4746
public function down(Schema $schema): void
4847
{
49-
$this->warnIf(true, 'This migration is not reversible without deleting all migrated consent hashes.' .
50-
' Manually run `DELETE FROM consent WHERE attribute IS NOT NULL` to delete all migrated consent hashes.');
51-
$this->addSql('ALTER TABLE consent CHANGE attribute attribute VARCHAR(80) NOT NULL');
5248
$this->addSql('ALTER TABLE consent DROP attribute_stable');
5349
}
5450
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ public function __construct(
2626
public readonly string $attributeStableHash,
2727
public readonly string $consentType,
2828
/** @deprecated Remove after stable consent hash is running in production */
29-
public readonly ?string $attributeHash = null,
29+
public readonly string $attributeHash,
3030
) {
3131
}
3232
}

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

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,6 @@ public function __construct(
2727
public readonly string $hashedUserId,
2828
public readonly string $serviceId,
2929
public readonly string $consentType,
30-
/** @deprecated Remove after stable consent hash is running in production */
31-
public readonly bool $clearLegacyHash = false,
3230
) {
3331
}
3432
}

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

Lines changed: 1 addition & 36 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\EngineBlockBundle\Configuration\FeatureConfigurationInterface;
2726
use function array_keys;
2827
use function implode;
2928
use function ksort;
@@ -32,23 +31,14 @@
3231

3332
final class ConsentHashService implements ConsentHashServiceInterface
3433
{
35-
/** @deprecated Remove after stable consent hash is running in production */
36-
private const FEATURE_MIGRATION = 'eb.stable_consent_hash_migration';
37-
3834
/**
3935
* @var ConsentRepository
4036
*/
4137
private $consentRepository;
4238

43-
/**
44-
* @var FeatureConfigurationInterface
45-
*/
46-
private $featureConfiguration;
47-
48-
public function __construct(ConsentRepository $consentHashRepository, FeatureConfigurationInterface $featureConfiguration)
39+
public function __construct(ConsentRepository $consentHashRepository)
4940
{
5041
$this->consentRepository = $consentHashRepository;
51-
$this->featureConfiguration = $featureConfiguration;
5242
}
5343

5444
public function retrieveConsentHash(ConsentHashQuery $query): ConsentVersion
@@ -58,36 +48,11 @@ public function retrieveConsentHash(ConsentHashQuery $query): ConsentVersion
5848

5949
public function storeConsentHash(ConsentStoreParameters $parameters): bool
6050
{
61-
$migrationEnabled = $this->featureConfiguration->isEnabled(self::FEATURE_MIGRATION);
62-
63-
if ($migrationEnabled) {
64-
$parameters = new ConsentStoreParameters(
65-
hashedUserId: $parameters->hashedUserId,
66-
serviceId: $parameters->serviceId,
67-
attributeStableHash: $parameters->attributeStableHash,
68-
consentType: $parameters->consentType,
69-
attributeHash: null,
70-
);
71-
}
72-
7351
return $this->consentRepository->storeConsentHash($parameters);
7452
}
7553

7654
public function updateConsentHash(ConsentUpdateParameters $parameters): bool
7755
{
78-
$migrationEnabled = $this->featureConfiguration->isEnabled(self::FEATURE_MIGRATION);
79-
80-
if ($migrationEnabled) {
81-
$parameters = new ConsentUpdateParameters(
82-
attributeStableHash: $parameters->attributeStableHash,
83-
attributeHash: $parameters->attributeHash,
84-
hashedUserId: $parameters->hashedUserId,
85-
serviceId: $parameters->serviceId,
86-
consentType: $parameters->consentType,
87-
clearLegacyHash: true,
88-
);
89-
}
90-
9156
return $this->consentRepository->updateConsentHash($parameters);
9257
}
9358

src/OpenConext/EngineBlockBundle/Authentication/Entity/Consent.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ class Consent
5656
/**
5757
* @var string
5858
*/
59-
#[ORM\Column(type: Types::STRING, length: 80, nullable: true)]
59+
#[ORM\Column(type: Types::STRING, length: 80, nullable: false)]
6060
public ?string $attribute = null;
6161

6262
/**

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

Lines changed: 27 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -226,30 +226,17 @@ public function hasConsentHash(ConsentHashQuery $query): ConsentVersion
226226
*/
227227
public function storeConsentHash(ConsentStoreParameters $parameters): bool
228228
{
229-
if ($parameters->attributeHash !== null) {
230-
$query = "INSERT INTO consent (hashed_user_id, service_id, attribute, attribute_stable, consent_type, consent_date, deleted_at)
231-
VALUES (?, ?, ?, ?, ?, NOW(), '0000-00-00 00:00:00')
232-
ON DUPLICATE KEY UPDATE attribute=VALUES(attribute), attribute_stable=VALUES(attribute_stable),
233-
consent_type=VALUES(consent_type), consent_date=NOW(), deleted_at='0000-00-00 00:00:00'";
234-
$bindings = [
235-
$parameters->hashedUserId,
236-
$parameters->serviceId,
237-
$parameters->attributeHash,
238-
$parameters->attributeStableHash,
239-
$parameters->consentType,
240-
];
241-
} else {
242-
$query = "INSERT INTO consent (hashed_user_id, service_id, attribute_stable, consent_type, consent_date, deleted_at)
243-
VALUES (?, ?, ?, ?, NOW(), '0000-00-00 00:00:00')
244-
ON DUPLICATE KEY UPDATE attribute_stable=VALUES(attribute_stable),
245-
consent_type=VALUES(consent_type), consent_date=NOW(), deleted_at='0000-00-00 00:00:00'";
246-
$bindings = [
247-
$parameters->hashedUserId,
248-
$parameters->serviceId,
249-
$parameters->attributeStableHash,
250-
$parameters->consentType,
251-
];
252-
}
229+
$query = "INSERT INTO consent (hashed_user_id, service_id, attribute, attribute_stable, consent_type, consent_date, deleted_at)
230+
VALUES (?, ?, ?, ?, ?, NOW(), '0000-00-00 00:00:00')
231+
ON DUPLICATE KEY UPDATE attribute=VALUES(attribute), attribute_stable=VALUES(attribute_stable),
232+
consent_type=VALUES(consent_type), consent_date=NOW(), deleted_at='0000-00-00 00:00:00'";
233+
$bindings = [
234+
$parameters->hashedUserId,
235+
$parameters->serviceId,
236+
$parameters->attributeHash,
237+
$parameters->attributeStableHash,
238+
$parameters->consentType,
239+
];
253240

254241
try {
255242
$this->connection->executeStatement($query, $bindings);
@@ -267,42 +254,22 @@ public function storeConsentHash(ConsentStoreParameters $parameters): bool
267254
*/
268255
public function updateConsentHash(ConsentUpdateParameters $parameters): bool
269256
{
270-
if ($parameters->clearLegacyHash) {
271-
$query = "
272-
UPDATE
273-
consent
274-
SET
275-
attribute_stable = ?,
276-
attribute = NULL
277-
WHERE
278-
attribute = ?
279-
AND
280-
hashed_user_id = ?
281-
AND
282-
service_id = ?
283-
AND
284-
consent_type = ?
285-
AND
286-
deleted_at IS NULL
287-
";
288-
} else {
289-
$query = "
290-
UPDATE
291-
consent
292-
SET
293-
attribute_stable = ?
294-
WHERE
295-
attribute = ?
296-
AND
297-
hashed_user_id = ?
298-
AND
299-
service_id = ?
300-
AND
301-
consent_type = ?
302-
AND
303-
deleted_at IS NULL
304-
";
305-
}
257+
$query = "
258+
UPDATE
259+
consent
260+
SET
261+
attribute_stable = ?
262+
WHERE
263+
attribute = ?
264+
AND
265+
hashed_user_id = ?
266+
AND
267+
service_id = ?
268+
AND
269+
consent_type = ?
270+
AND
271+
deleted_at IS NULL
272+
";
306273

307274
try {
308275
$affected = $this->connection->executeStatement($query, [

0 commit comments

Comments
 (0)