Skip to content

Replace the Pii::Cacher implementation with the Pii::ProfileCacher implementation#9754

Merged
jmhooper merged 1 commit intomainfrom
jmhooper-replace-pii-cacher-with-profile-cacher
Dec 15, 2023
Merged

Replace the Pii::Cacher implementation with the Pii::ProfileCacher implementation#9754
jmhooper merged 1 commit intomainfrom
jmhooper-replace-pii-cacher-with-profile-cacher

Conversation

@jmhooper
Copy link
Contributor

The Pii::ProfileCacher was implemented in #9509. As described in that pull request:

The Pii::ProfileCacher class implements the same API as Pii::Cacher and is intended to supersede Pii::Cacher when this work is done.

All of the invocations to Pii::Cacher that require the deprecated profile-unaware functionality have been removed and the new profile-aware functionality is fully implemented and in-use. This commit does the work of moving the Pii::ProfileCacher functionality into Pii::Cacher and removing Pii::ProfileCacher.

This should not be merged until both session_encrypted_profiles_read_enabled and session_encrypted_profiles_write_enabled have been set to true. This commit removes those feature flags and assumes they are set to true.

@jmhooper jmhooper requested a review from a team December 13, 2023 15:24
@jmhooper jmhooper force-pushed the jmhooper-replace-pii-cacher-with-profile-cacher branch 4 times, most recently from 3cea41f to 9b54e12 Compare December 13, 2023 21:31
…` implementation

The `Pii::ProfileCacher` was implemented in #9509. As described in that pull request:

> The Pii::ProfileCacher class implements the same API as Pii::Cacher and is intended to supersede Pii::Cacher when this work is done.

All of the invocations to `Pii::Cacher` that require the deprecated profile-unaware functionality have been removed and the new profile-aware functionality is fully implemented and in-user. This commit does the work of moving the `Pii::ProfileCacher` functionality into `Pii::Cacher` and removing `Pii::ProfileCacher`

This should not be merged until both `session_encrypted_profiles_read_enabled` and `session_encrypted_profiles_write_enabled` have been set to true. This commit removes those feature flags and assumes they are set to true.

changelog: Internal, Profile and session management, The pii cacher implementation was replaced with the profile cacher implementation
@jmhooper jmhooper force-pushed the jmhooper-replace-pii-cacher-with-profile-cacher branch from 9b54e12 to 351ca6a Compare December 13, 2023 21:50
Copy link
Contributor

@soniaconnolly soniaconnolly left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Ran locally and verified by mail without issues. Also tried resetting my password while logged in, and then while logged out & recovering profile with personal key, and that all worked.

@jmhooper jmhooper merged commit 54bacda into main Dec 15, 2023
@jmhooper jmhooper deleted the jmhooper-replace-pii-cacher-with-profile-cacher branch December 15, 2023 15:51
@jmdembe jmdembe mentioned this pull request Dec 19, 2023
jmhooper added a commit that referenced this pull request Dec 19, 2023
In #9754 the code that depended on the passive encryption of `decrypted_pii` in the session was removed. Some of the code removed here was left behind to ensure that PII remained encrypted.

Once the code in #9754 is deployed to production this should be safe to merge.

[skip changelog]
jmhooper added a commit that referenced this pull request Dec 20, 2023
In #9754 the code that depended on the passive encryption of `decrypted_pii` in the session was removed. Some of the code removed here was left behind to ensure that PII remained encrypted.

Once the code in #9754 is deployed to production this should be safe to merge.

[skip changelog]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants