- 🇯🇵Japan ptmkenny
So I started looking at the module code and found that the mail column should be converted from VARCHAR to TEXT automatically upon module installation in
dbee.install
.However, in my case this does not happen.
Steps to reproduce
1. Create a Drupal 10 site and install dbee. Export all config.
2. Wipe the database.
3. Set up a new db with the existing code:drush site:install --account-name=admin --account-pass=admin -y
4. Import config. This will install the dbee module, but for some reason the install hook isn't run, and mail + init remain VARCHAR (see screenshot).Workaround: Uninstall dbee and re-enable it.
How I found this bug: My CI tests start with a blank DB install, so if I import config and then run my tests, those with long email addresses fail.
- Status changed to Postponed: needs info
almost 2 years ago 3:01pm 23 January 2023 - 🇵🇹Portugal jcnventura
And "Uninstall dbee and re-enable it.", worked?
There's no way the module knows it is the 2nd time that it is being installed, so something went wrong in your first install. If you can figure out what that was, it would be good to test for those conditions, and handle them correctly.
Otherwise, and also by the fact that no one else reported this problem before, I'm leaning towards a "Can't reproduce" status here.
- Status changed to Active
almost 2 years ago 4:58pm 23 January 2023 - 🇯🇵Japan ptmkenny
Thank you for the response.
I've been trying to debug this and I came up with a reproducible example, which can be seen here:
https://github.com/ptmkenny/d10/tree/dbeeThe issue can be reproduced by checking out the repo and then entering the codes listed in the README.
Alternately, here are the exact steps I took to reproduce this issue with ddev:
# Do a standard ddev install of Drupal 10. ddev config --project-type=drupal10 --docroot=web --create-docroot ddev start ddev composer create drupal/recommended-project ddev composer require drush/drush ddev drush site:install minimal --account-name=admin --account-pass=admin -y # Add dbee. ddev composer require 'drupal/dbee:^3.0@RC' ddev drush pm-enable dbee ddev drush cex -y # Copy down the site's UUID. Then drop the db and reinstall Drupal. ddev drush sql:drop -y ddev drush site:install minimal --account-name=admin --account-pass=admin -y # Need to set the site UUID because the new db will have a different UUID. ddev drush cset system.site uuid 1463ec05-e4f4-457c-8f9a-e3c3cf2376b4 -y ddev drush cr ddev drush cim -y
The import will complete successfully, but when you check the structure of the database (in my case, using
ddev sequelace
), then the mail + init will still be VARCHAR.So there's something about enabling ddev through a config import that is causing this issue.
- 🇯🇵Japan ptmkenny
More info:
It seems the problem is:
In dbee.install, before adjusting the field length, it checks:
if (_dbee_create_encryption_key(DBEE_DEFAULT_KEY_NAME, DBEE_DEFAULT_KEY_BYTES, DBEE_DEFAULT_KEY_FILENAME, $is_syncing) && _dbee_create_encryt_profile(DBEE_ENCRYPT_NAME, DBEE_DEFAULT_KEY_NAME, DBEE_DEFAULT_ENCRYPTION_METHOD, $is_syncing)) {
And _dbee_create_encryt_profile returns FALSE if the module is being installed by config import:
/** * Set a dbee encryption profile entity. * * The encryption profile entity is provided by the Encrypt contrib module. * Look for an existing entity, and if none exists, create one. * Steps and errors are logged. * * @param string $encrypt_profile_id * The encryption profile entity machine name. * @param string $key_id * The key entity machine name. * @param string $encryption_method * The encryption method. Defaults to 'real_aes', which corresponds to * the real_aes contrib module. * @param bool $is_syncing * TRUE if the module is being installed as part of a configuration import. * * @return bool * TRUE if the Encryption Profile entity already exists or has been * successfully created, or FALSE if an error has occurred. */ function _dbee_create_encryt_profile($encrypt_profile_id, $key_id, $encryption_method = 'real_aes', $is_syncing = FALSE) { if ($is_syncing) { return FALSE; }
What is the reason for this different behavior due to config import?
- 🇵🇹Portugal jcnventura
The reason for that is that the module should not start the very time consuming operation of encrypting all the user emails during the config import. But thanks a lot for this last gem. It does indicate that the _dbee_create_encryt_profile() function might need to only be responsible for actually encrypting the profiles, and not for changing the database type.
- 🇯🇵Japan ptmkenny
So I moved the lengthening logic outside the profile check and now installing by config sync lengthens them correctly.
However, the email addresses aren't encrypted during config import, and I tried running cron and they didn't get encrypted, either.
If the module is installed by config sync, what additional steps need to be taken to encrypt the existing email addresses? Is there a way to automate this, too?
- 🇵🇹Portugal jcnventura
Can you confirm that new users created will get their emails encrypted? This should only mean that the existing users will need to be updated. This should clearly be a batch operation that can be triggered when this situation occurs. That should definitively be a new issue, as the problem in this issue should get fixed with the code split you just did in #8
- Status changed to Needs review
almost 2 years ago 11:34am 24 January 2023 - 🇯🇵Japan ptmkenny
Yes, new user emails are encrypted with this MR. I tested with the standard dbee config and also my own custom config (uses a custom encryption profile/key). New user emails were encrypted in both cases.
For existing users (in my setup now, just a single user), I realized that all I have to do is re-save that user account to encrypt the email (but we should open a new issue about that).
Hello, it seems this fix is still not merged ? I encountered the same issue with a too long mail. This bug can affect a lot of users on some countries...
Sincerely- 🇵🇹Portugal jcnventura
No one ever bothered with reviewing the patch, and marked it "Reviewed & tested by the community".
Nothing will change until that happens.
- Status changed to RTBC
11 months ago 4:43pm 20 January 2024 - 🇦🇺Australia elc
Seems to work for me.
Now that everyone can just include patches in composer, I've found participation in issues queues only happens to re-roll a patch when a significant enough change happens that the last patch made no longer cleanly applies. You'll also find people creating patches to the last release instead of HEAD, just so it can be used in composer. People just grab the patch and move on.
- 🇵🇹Portugal jcnventura
jcnventura → changed the visibility of the branch fix-install-hook to hidden.
-
jcnventura →
authored 350846d6 on 3.x
Issue #3334840 by ptmkenny, jcnventura, ELC: SQL error 1406 Data too...
-
jcnventura →
authored 350846d6 on 3.x
- Status changed to Fixed
11 months ago 7:39pm 21 January 2024 - 🇵🇹Portugal jcnventura
Added a further check around the section in
dbee_install()
where all the emails are encrypted to avoid that section being activated in case the immediate FALSE return from_dbee_create_encrypt_profile()
is ever modified..With these changes, the code should still be the same as before, and there is probably still a problem that after a config import, the emails are not encrypted in the database, which as I referred in #10 should be a new issue.
Automatically closed - issue fixed for 2 weeks with no activity.