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

TableauConnectedApp, TransifexOrganization, and OpenmrsImporter models: AES CBC encryption read and write #35665

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

Jtang-1
Copy link
Contributor

@Jtang-1 Jtang-1 commented Jan 23, 2025

Product Description

N/A

Technical Summary

Continuation of the work initiated by this PR
Jira Ticket

Introduces a solution for these code scan alerts:
https://github.com/dimagi/commcare-hq/security/code-scanning/295
https://github.com/dimagi/commcare-hq/security/code-scanning/296

This PR updates TableauConnectedApp 's secret_value, TransifexOrganization's api_token, and OpenmrsImporter's password. The model continues supporting reads for both AES ECB and AES CBC encryption while writes only with CBC encryption.

Feature Flag

The OpenmrsImporter document is only relevant to openmrs_integration FF

The TableauConnected model is only relevant to embedded_tableau and tableau_user_syncing FF

Safety Assurance

Safety story

locally tested. This change will not affect any existing data and existing data will be read the same way.

Automated test coverage

There exists tests that the encryption and decryption with CBC results in the expected plaintext. Also test that TableauConnectedApp secret value can be encrypted and decrypted.

QA Plan

no QA

Migrations

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

There is no migration in this PR. But there is a follow up PR that does a migration related to the changes in this PR.

Rollback instructions

This PR will start writing data with AES CBC encryption while also adding support to read AES CBC encrypted data. If this PR is reverted, the data encrypted with CBC will fail to be decrypted. To rollback this PR, data written with AES CBC encryption will need to be reencrypted with EBC.

Labels & Review

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

@Jtang-1 Jtang-1 changed the title TableauConnec and API settings models: AES CBC encryption read and write TableauConnectedApp, TransifexOrganization, and OpenmrsImporter models: AES CBC encryption read and write Jan 23, 2025
@Jtang-1 Jtang-1 requested review from a team, AddisonDunn, minhaminha, Robert-Costello and jingcheng16 and removed request for a team and AddisonDunn January 23, 2025 00:22
@Jtang-1 Jtang-1 marked this pull request as ready for review January 23, 2025 00:23
@Jtang-1 Jtang-1 requested a review from millerdev January 23, 2025 00:28
Copy link
Contributor

@jingcheng16 jingcheng16 left a comment

Choose a reason for hiding this comment

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

Looks good overall, just one question.

corehq/apps/translations/models.py Show resolved Hide resolved
@jingcheng16
Copy link
Contributor

Can you link the follow up PR in this PR's Migration section for convenience?

Copy link
Contributor

@jingcheng16 jingcheng16 left a comment

Choose a reason for hiding this comment

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

Thank you for making the change!

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