Fix error sending email with capitalization to ApigeeX Org #756
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1267,6 +1267,36 @@ function template_preprocess_app_credential(array &$variables) { | |
| } | ||
| } | ||
|
|
||
| /** | ||
| * Forcing user email address to lowercase for ApigeeX/Hybrid Org. | ||
| * @see https://github.com/apigee/apigee-edge-drupal/issues/730 | ||
| */ | ||
| function apigee_edge_user_email_check(&$account) { | ||
| $email = $account->getEmail(); | ||
| try { | ||
| /** @var \Drupal\apigee_edge\SDKConnectorInterface $sdk_connector */ | ||
| $sdk_connector = \Drupal::service('apigee_edge.sdk_connector'); | ||
| $org_controller = \Drupal::service('apigee_edge.controller.organization'); | ||
| /* @var \Apigee\Edge\Api\Management\Entity\Organization $organization */ | ||
| $organization = $org_controller->load($sdk_connector->getOrganization()); | ||
|
|
||
| // Check if org is ApigeeX. | ||
| if ($organization && ('CLOUD' === $organization->getRuntimeType() || 'HYBRID' === $organization->getRuntimeType())) { | ||
| // Apigee X Org is only accepting lower case email address, | ||
| // therefore preventing the form from creating account | ||
| // with uppercase email address. | ||
| if (preg_match('/[A-Z]/', $email)) { | ||
| $account->set('mail', strtolower($email)); | ||
|
There was a problem hiding this comment. I have security concerns regarding this approach because I am quite sure that somewhere in the user create/update process it is checked whether the new/changed email address of Drupal user is already belongs to a developer account on Apigee, BUT if that runs before this code then somebody could easily take over a developer account of somebody else by using a uppercase version of the email address. There was a problem hiding this comment. Thank you @mxr576 for reviewing this PR. Do you mean to say, that instead of forcefully converting the email address to lowercase we should put a constraint on the email field so the user gets an error message while user create/update if capitalized email address found for Apigee-X Org. There was a problem hiding this comment. Yes, exactly - And developer sync is also going to fail (it already works like this) when entity validation fails. |
||
| \Drupal::messenger()->addWarning(t('The site has converted your email address to lowercase.')); | ||
| } | ||
| } | ||
| } | ||
| catch (\Exception $exception) { | ||
| // If not able to connect to Apigee Edge. | ||
| Drupal::logger('apigee_edge')->error($exception->getMessage()); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Implements hook_user_presave(). | ||
| * | ||
|
|
@@ -1303,6 +1333,11 @@ function apigee_edge_user_presave(UserInterface $account) { | |
| return; | ||
| } | ||
|
|
||
| // Force email address to lowercase for ApigeeX org, | ||
| // Apigee X Org is only accepting lowercase email address. | ||
| // @see https://github.com/apigee/apigee-edge-drupal/issues/730 | ||
| apigee_edge_user_email_check($account); | ||
|
|
||
| /** @var \Drupal\apigee_edge\UserDeveloperConverterInterface $user_developer */ | ||
| $user_developer = \Drupal::service('apigee_edge.converter.user_developer'); | ||
| /** @var \Drupal\apigee_edge\FieldAttributeConverterInterface $field_to_attribute */ | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, but I believe this should be a new field level constraint which would run whenever and wherever
$user->validate()or$userl->mail->validate()is called. This would be the standard Drupal/Symfony way or ensuring data consistency.As a last result, a sanity check could still be executed in
apigee_edge_user_presave()to ensure the above described field level validation was actually validated before entity save - because we all know that some IDP integration modules loves to bypass validating the user entity before they save it - and if an email still contains uppercase characters aLogicException(or a subclass of it) could be thrown to prevent user creation. (Which is still better than spending hours on figuring our why they is a Drupal user in the system without a developer account on the Apigee side.)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know of a case where the client's LDP was using upper-case characters in every user's email address, which apparently was getting synced with Apigee. The client was reluctant to change the LDP configurations so wrote a hook to convert the new user's email address to lowercase and also an update hook to convert the existing user's email address to lowercase which in turn syncs the developer account Apigee as well.