Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

EmailSettings password encryption improvement [Migration] #35492

Merged
merged 1 commit into from
Jan 15, 2025

Conversation

Jtang-1
Copy link
Contributor

@Jtang-1 Jtang-1 commented Dec 9, 2024

Product Description

Technical Summary

The migration related to this PR #35403

Feature Flag

Not specific to a feature flag but the model effected is only relevant for this feature flag https://www.commcarehq.org/hq/flags/edit/custom_email_gateway/

Safety Assurance

Safety story

Locally Tested

Automated test coverage

The coordinated previous PR tests that the function for reencryption from ECB to CBC on the EmailSettings model existing password field results in password being overwritten with the same plaintext password but cbc encrypted.

QA Plan

no QA

Migrations

  • The migrations in this code can be safely applied first independently of the code

PR that supports reading ECB and CBC encrypted password and starts writing new passwords as CBC encrypted

Rollback instructions

  • This PR can be reverted after deploy with no further considerations

Labels & Review

  • Risk label is set correctly
  • The set of people pinged as reviewers is appropriate for the level of risk of the change

@dimagimon dimagimon added the reindex/migration Reindex or migration will be required during or before deploy label Dec 9, 2024
@Jtang-1 Jtang-1 requested review from jingcheng16, gherceg, a team, AddisonDunn, minhaminha and Robert-Costello and removed request for a team December 9, 2024 17:56
@Jtang-1 Jtang-1 changed the base branch from master to jt+jc/encryption_improvement December 9, 2024 18:38
@dimagimon dimagimon removed the reindex/migration Reindex or migration will be required during or before deploy label Dec 9, 2024
Base automatically changed from jt+jc/encryption_improvement to master December 9, 2024 21:28
@Jtang-1 Jtang-1 force-pushed the jt+jc/encryption_improvement_migration branch from bbc58d0 to dbbd31b Compare December 9, 2024 21:31
@dimagimon dimagimon added the reindex/migration Reindex or migration will be required during or before deploy label Dec 9, 2024
@Jtang-1 Jtang-1 marked this pull request as ready for review December 9, 2024 21:31
@Jtang-1 Jtang-1 added the Open for review: do not merge A work in progress label Dec 9, 2024
Copy link
Contributor

@minhaminha minhaminha left a comment

Choose a reason for hiding this comment

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

Looks solid but I'd run this on staging too just to make sure there's no weird edge case errors when applying on a larger database

@Jtang-1 Jtang-1 merged commit 4757ea3 into master Jan 15, 2025
12 of 13 checks passed
@Jtang-1 Jtang-1 deleted the jt+jc/encryption_improvement_migration branch January 15, 2025 07:17
@Jtang-1 Jtang-1 restored the jt+jc/encryption_improvement_migration branch January 15, 2025 07:17
@Jtang-1 Jtang-1 deleted the jt+jc/encryption_improvement_migration branch January 15, 2025 07:17
@Jtang-1 Jtang-1 removed the Open for review: do not merge A work in progress label Jan 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
reindex/migration Reindex or migration will be required during or before deploy
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants