apigee-edge-drupal
apigee-edge-drupal copied to clipboard
Fix error sending email with capitalization to ApigeeX Org
Closes #730
As ApigeeX Org is not accepting capitalized email address, we are forcing lowercase email addresses for Apigee-X Org to prevent error.
Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 44.85%. Comparing base (
afeb21e
) to head (b1e4fa5
).
:exclamation: Current head b1e4fa5 differs from pull request most recent head ee49b56. Consider uploading reports for the commit ee49b56 to get more accurate results
Additional details and impacted files
@@ Coverage Diff @@
## 2.x #756 +/- ##
=========================================
Coverage 44.85% 44.85%
Complexity 3047 3047
=========================================
Files 345 345
Lines 11178 11178
=========================================
Hits 5014 5014
Misses 6164 6164
Just wanted to check in on this issue and see if there's any further movement on this...?
Looking at this PR, I would not be expecting this module to control how I am storing a user email in my Drupal site. I guess, to me, the fix I would expect is simply that, when sending an email over to Apigee, that it's transformed to lowercase at that time. Leave it stored in whatever case it's stored in on Drupal... Is that not viable?
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.
Hi Apigee team - Is there any direction on this pull request? This has been open for more than a year now, with little discussion. There are some valid discussions that are still open here. What are the next steps to move this issue forward?
Hi @dpagini , Team is reviewing this PR for any changes and approve it.
Hi @shishir-intelli I checked with other cases also, with Okta SAML login also, I checked with camelcase, It works fine and the email is converted to lowercase with message.
Please note that this approach goes against the SMTP standard: https://datatracker.ietf.org/doc/html/rfc5321#section-2.4 clearly says:
The local-part of a mailbox MUST BE treated as case sensitive.
If this gets merged, all kind of breaks are expected, not only in Drupal, but also in receiving emails, etc.
Hi @boobaa
We are planning to add the following to avoid the duplicate developers with same email ids.
During new user creation - Add a duplicate email id check (Includes a check with lists of existing developers in that X org) When a existing user is logged in - Add a duplicate email id check + Add a status report message of system requirements + Restrict users to show the API keys.
If you have any other additional to-dos, we are open for a discussion over here.
Do not forget about email changing in the Drupal UI, that should be also handled... But not even Drupal core handles that (still) https://www.drupal.org/node/85494
Even if think this auto-lowercasing is a bad idea... Let me try to imagine how this could work securely... Before the lowercased email is saved, the ownership must be confirmed via opening a verification link from the mailbox or similar. Developers are going to have plenty ways to shoot themselves on foot if one system uses upperscase/mixed case emails the other silently converts them to lowercase, just imagine that email stored in the idp gets used for loading a suppose to be existing developer on Apigee.
I'm not sure I agree with this approach, either. I don't think we should be changing the case of an email on the Drupal side. This is very heavy handed. Maybe this is not something that should even be fixed in the module, but instead on the Apigee X Org end to accept mixed case email addresses...? Maybe that's a bigger change, but based on some of the links shared above, it seems like this is a problem with the API endpoint, and not with the integration.
This issue has been resolved here https://github.com/apigee/apigee-edge-drupal/pull/1050