apigee-edge-drupal icon indicating copy to clipboard operation
apigee-edge-drupal copied to clipboard

Fix error sending email with capitalization to ApigeeX Org

Open shishir-intelli opened this issue 2 years ago • 10 comments

Closes #730

As ApigeeX Org is not accepting capitalized email address, we are forcing lowercase email addresses for Apigee-X Org to prevent error.

shishir-intelli avatar Nov 07 '22 07:11 shishir-intelli

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

Impacted file tree graph

@@            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           

codecov[bot] avatar Nov 14 '22 05:11 codecov[bot]

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?

dpagini avatar Apr 05 '23 18:04 dpagini

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.

github-actions[bot] avatar Jun 05 '23 01:06 github-actions[bot]

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?

dpagini avatar Feb 23 '24 18:02 dpagini

Hi @dpagini , Team is reviewing this PR for any changes and approve it.

shishir-intelli avatar Mar 07 '24 04:03 shishir-intelli

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.

kedarkhaire avatar Mar 14 '24 07:03 kedarkhaire

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.

boobaa avatar Mar 14 '24 07:03 boobaa

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.

kedarkhaire avatar Mar 15 '24 07:03 kedarkhaire

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.

mxr576 avatar Mar 15 '24 11:03 mxr576

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.

dpagini avatar Mar 20 '24 19:03 dpagini

This issue has been resolved here https://github.com/apigee/apigee-edge-drupal/pull/1050

shishir-intelli avatar May 09 '24 05:05 shishir-intelli