Skip to content

Conversation

@dracarys18
Copy link
Contributor

Type of Change

  • New feature

Description

This PR adds support for encrypting PII fields in following tables

  • MerchantAccount
  • MerchantConnectorAccount
  • Address
  • Customer

Here's how the basic flow goes, the main encryption key master_key will be encrypted with KMS and configured in development.toml. For every merchant we will create seperate encryption key and encrypt that key with master_key and push it to the database and we will use that key to encrypt the PII fields.

This PR also has migration script that will migrate currently decrypted database to encrypted database.

Additional Changes

  • This PR modifies the database schema
  • This PR modifies application configuration/environment variables

Motivation and Context

This is for GDPR compliance for saving PII data in an encrypted format.

How did you test it?

Manual

Checklist

  • I formatted the code cargo +nightly fmt --all
  • I addressed lints thrown by cargo clippy
  • I reviewed submitted code

NishantJoshi00 and others added 30 commits March 31, 2023 14:24
@dracarys18 dracarys18 removed the S-needs-conflict-resolution Status: This PR needs conflicts to be resolved by the author label May 29, 2023
.into_inner()
.expose();

assert!(dummy_data_returned == dummy_data)
Copy link
Member

Choose a reason for hiding this comment

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

Use assert_eq!() instead.

use error_stack::{report, FutureExt, ResultExt};
use masking::Secret;
use storage_models::{enums, merchant_account};
use masking::Secret; //PeekInterface
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
use masking::Secret; //PeekInterface
use masking::Secret;

@SanchithHegde SanchithHegde added S-waiting-on-author Status: This PR is incomplete or needs to address review comments and removed S-blocked Status: Blocked on something else or other implementation work R-L1-completed labels May 30, 2023
@jarnura jarnura enabled auto-merge May 30, 2023 08:11
@jarnura jarnura added this to the May 2023 Release milestone May 30, 2023
@jarnura jarnura added this pull request to the merge queue May 30, 2023
@jarnura jarnura added S-ready-for-merge and removed S-waiting-on-author Status: This PR is incomplete or needs to address review comments labels May 30, 2023
Merged via the queue into main with commit fa392c4 May 30, 2023
@jarnura jarnura deleted the pii-script branch May 30, 2023 08:49
merchant_id: merchant_id.to_string(),
..Default::default()
})
.change_context(errors::ApiErrorResponse::AddressNotFound)?;
Copy link
Member

Choose a reason for hiding this comment

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

This change raises an incorrect error message that the address was not found, when in reality, the deseriliazation of the address failed. Could you please fix this, and ensure that "resource not found" errors are not being raised incorrectly elsewhere?

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

Labels

A-core Area: Core flows M-configuration-changes Metadata: This PR involves configuration changes M-database-changes Metadata: This PR involves database schema changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants