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.

Sign up for GitHub

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

Jump to bottom

Improve handling of sales_order_payment cc_number_enc and reencrypt-column #9

Open
convenient opened this issue Jul 16, 2024 · 3 comments · May be fixed by #36
Open

Improve handling of sales_order_payment cc_number_enc and reencrypt-column #9

convenient opened this issue Jul 16, 2024 · 3 comments · May be fixed by #36
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@convenient
Copy link
Contributor

convenient commented Jul 16, 2024 •
edited
Loading

Courtesy of @mpchadwick

i'm also wondering there should be pagination used here https://github.com/genecommerce/module-encryption-key-manager/blob/master/Service/ChangeEncryptionKey.php#L56-L60 . I don't think we've seen it yet, but I think fetchPairs wlll attempt to load the entire dataset into memory, so I do think this can run out of memory at a certain point

The custom reencrypt-column command also may need a review.
@convenient convenient added help wanted Extra attention is needed enhancement New feature or request labels Jul 16, 2024
@convenient convenient changed the title Improve handling of sales_order_payment cc_number_enc Improve handling of sales_order_payment cc_number_enc and reencrypt-column Jul 18, 2024
@braders
Copy link

braders commented Jul 22, 2024

I am working on a site with around 2.5 million records in the sales_order_payment table, and am wary to try loading that into memory.

We don't support saved card numbers, and so cc_number_enc is always empty. It'd be nice to have the possibility to skip running the _reEncryptCreditCardNumbers function.

@convenient
Copy link
Contributor Author

@braders I would happily take a PR for that, it's a good addition.

  1. Add a new flag like --skip-sales-order-payment-cc-number (or something with a better name)

module-encryption-key-manager/Console/GenerateEncryptionKey.php

Lines 51 to 56 in 2174b19

new InputOption(
self::INPUT_KEY_FORCE,
null,
InputOption::VALUE_NONE,
'Whether to force this action to take effect'
),

  1. Pass information down to ChangeEncryptionKey

It's not super elegant, but similar to how I pass down the console output

module-encryption-key-manager/Console/GenerateEncryptionKey.php

Line 104 in 2174b19

$this->changeEncryptionKey->setOutput($output);

We can set configuration on the object to alter its behaviour prior to running the changeEncryptionKey command

  1. Update ChangeEncryptionKey

Update the code below to check for the config and skip if not required.

module-encryption-key-manager/Service/ChangeEncryptionKey.php

Lines 52 to 54 in 2174b19

protected function _reEncryptCreditCardNumbers()
{
$this->writeOutput('_reEncryptCreditCardNumbers - start');

@convenient convenient mentioned this issue Jul 22, 2024
Merged
@convenient
Copy link
Contributor Author

@braders can you please look at #26 ?

@convenient convenient linked a pull request Jul 24, 2024 that will close this issue
Draft
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement New feature or request help wanted Extra attention is needed
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Improve performance with large data sets genecommerce/module-encryption-key-manager
2 participants
@convenient @braders