Repository: saleor/saleor
Pull request: Make email authentication case insensitive
https://github.com/saleor/saleor/issues/11277

I want to merge this change, because I want to make email authentication case insensitive. It should be possible to login under the same account using [test@test.com](mailto:test@test.com) and [Test@test.com](mailto:Test@test.com).

This PR also force emails to be lower case only.

<!-- Please mention all relevant issue numbers. -->

# Impact

* [ ] New migrations
* [ ] New/Updated API fields or mutations
* [ ] Deprecated API fields or mutations
* [ ] Removed API types, fields, or mutations
* [ ] Documentation needs to be updated

# Pull Request Checklist

<!-- Please keep this section. It will make the maintainer's life easier. -->

* [ ] Privileged queries and mutations are guarded by proper permission checks
* [ ] Database queries are optimized and the number of queries is constant
* [ ] Database migration files are up to date
* [ ] The changes are tested
* [ ] GraphQL schema and type definitions are up to date
* [ ] Changes are mentioned in the changelog


Problem statement:
Make email case insensitive 
### What are you trying to achieve?

Make emails case insensitive to prevent login issues.

### Steps to reproduce the problem

Create account Test@test.com
Login with test@test.com

### What did you expect to happen?

It should be possible to login under the same account using test@test.com and Test@test.com. Essentially any provided email should be read in lower case. With exception if the system has two accounts under test@test.com and Test@test.com then login should happen to the corresponding account provided. 

### Logs

_No response_

### Environment

Saleor version: 3.9

Hints from issue discussion:
@timuric  Should we also take into account the case where we have email provided with upper case? Like [test@test.com](mailto:test@test.com), [Test@test.com](mailto:Test@test.com) and TEST@TEST.COM?

Commit message: correct changelog

Patch to review:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 9b87b2ac2eb..b17580bd83a 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -38,7 +38,7 @@ All notable, unreleased changes to this project will be documented in this file.
 - Include fully qualified API URL `Saleor-Api-Url` in communication with Apps. #11223 by @przlada
 - Add `attribute` field to `AttributeValueTranslatableContent` type. #11028 by @zedzior
 - Allow assigning attribute value using its ID. Add to `AttributeValueInput` dedicated field for each input type. #11206 by @zedzior
-...
+- Make email authentication case-insensitive. #11284 by @zedzior
 
 # 3.8.0
 
diff --git a/saleor/graphql/account/mutations/account.py b/saleor/graphql/account/mutations/account.py
index a0e7364cc37..22d515e2323 100644
--- a/saleor/graphql/account/mutations/account.py
+++ b/saleor/graphql/account/mutations/account.py
@@ -121,6 +121,8 @@ def clean_input(cls, info, instance, data, input_cls=None):
             data.get("channel"), error_class=AccountErrorCode
         ).slug
 
+        data["email"] = data["email"].lower()
+
         password = data["password"]
         try:
             password_validation.validate_password(password, instance)
@@ -429,7 +431,7 @@ class Meta:
     def perform_mutation(cls, _root, info, **data):
         user = info.context.user
         password = data["password"]
-        new_email = data["new_email"]
+        new_email = data["new_email"].lower()
         redirect_url = data["redirect_url"]
 
         if not user.check_password(password):
@@ -519,7 +521,7 @@ def perform_mutation(cls, _root, info, **data):
         token = data["token"]
 
         payload = cls.get_token_payload(token)
-        new_email = payload["new_email"]
+        new_email = payload["new_email"].lower()
         old_email = payload["old_email"]
 
         if models.User.objects.filter(email=new_email).exists():
diff --git a/saleor/graphql/account/mutations/authentication.py b/saleor/graphql/account/mutations/authentication.py
index b6c7bb8df07..894f0b30e94 100644
--- a/saleor/graphql/account/mutations/authentication.py
+++ b/saleor/graphql/account/mutations/authentication.py
@@ -30,6 +30,7 @@
 from ...core.types import AccountError
 from ...plugins.dataloaders import load_plugin_manager
 from ..types import User
+from ..utils import retrieve_user_by_email
 
 
 def get_payload(token):
@@ -106,7 +107,8 @@ class Meta:
 
     @classmethod
     def _retrieve_user_from_credentials(cls, email, password) -> Optional[models.User]:
-        user = models.User.objects.filter(email=email).first()
+        user = retrieve_user_by_email(email)
+
         if user and user.check_password(password):
             return user
         return None
diff --git a/saleor/graphql/account/mutations/base.py b/saleor/graphql/account/mutations/base.py
index 421dad1bc2f..f6be462d17c 100644
--- a/saleor/graphql/account/mutations/base.py
+++ b/saleor/graphql/account/mutations/base.py
@@ -33,6 +33,7 @@
 )
 from ...core.types import AccountError
 from ...plugins.dataloaders import load_plugin_manager
+from ..utils import retrieve_user_by_email
 from .authentication import CreateToken
 
 BILLING_ADDRESS_FIELD = "default_billing_address"
@@ -158,9 +159,8 @@ def clean_user(cls, email, redirect_url):
                 {"redirect_url": error}, code=AccountErrorCode.INVALID
             )
 
-        try:
-            user = models.User.objects.get(email=email)
-        except ObjectDoesNotExist:
+        user = retrieve_user_by_email(email)
+        if not user:
             raise ValidationError(
                 {
                     "email": ValidationError(
diff --git a/saleor/graphql/account/tests/mutations/test_token_create.py b/saleor/graphql/account/tests/mutations/test_token_create.py
index 784a560aa7c..afbb84a893d 100644
--- a/saleor/graphql/account/tests/mutations/test_token_create.py
+++ b/saleor/graphql/account/tests/mutations/test_token_create.py
@@ -1,10 +1,13 @@
 from datetime import datetime, timedelta, timezone
+from typing import List
 
 import graphene
+import pytest
 from django.urls import reverse
 from freezegun import freeze_time
 
 from .....account.error_codes import AccountErrorCode
+from .....account.models import User
 from .....core.jwt import (
     JWT_ACCESS_TYPE,
     JWT_REFRESH_TYPE,
@@ -13,7 +16,7 @@
 )
 from .....core.utils import build_absolute_uri
 from ....tests.utils import get_graphql_content
-from ...mutations.authentication import _get_new_csrf_token
+from ...mutations.authentication import CreateToken, _get_new_csrf_token
 
 MUTATION_CREATE_TOKEN = """
     mutation tokenCreate($email: String!, $password: String!, $audience: String){
@@ -221,3 +224,42 @@ def test_create_token_active_user_logged_before(api_client, customer_user, setti
     assert payload["type"] == JWT_REFRESH_TYPE
     assert payload["token"] == customer_user.jwt_token_key
     assert payload["iss"] == build_absolute_uri(reverse("api"))
+
+
+@pytest.fixture
+def users_with_similar_emails():
+    users = [
+        User.objects.create_user("andrew@example.com", "password"),
+        User.objects.create_user("Andrew@example.com", "password"),
+        User.objects.create_user("john@example.com", "password"),
+        User.objects.create_user("Susan@example.com", "password"),
+        User.objects.create_user("Cindy@example.com", "password"),
+        User.objects.create_user("CINDY@example.com", "password"),
+    ]
+    return users
+
+
+@freeze_time("2020-03-18 12:00:00")
+@pytest.mark.parametrize(
+    "email,expected_user",
+    [
+        ("andrew@example.com", 0),
+        ("Andrew@example.com", 1),
+        ("ANDREW@example.com", 0),
+        ("john@example.com", 2),
+        ("John@example.com", 2),
+        ("Susan@example.com", 3),
+        ("susan@example.com", 3),
+        ("Cindy@example.com", 4),
+        ("cindy@example.com", None),
+        ("CiNdY@example.com", None),
+        ("non_existing_email@example.com", None),
+    ],
+)
+def test_email_case_sensitivity(email, expected_user, users_with_similar_emails):
+    # given
+    users: List[User] = users_with_similar_emails
+    # when
+    user = CreateToken._retrieve_user_from_credentials(email=email, password="password")
+    # then
+    assert user == users[expected_user] if expected_user is not None else user is None
diff --git a/saleor/graphql/account/tests/test_account.py b/saleor/graphql/account/tests/test_account.py
index 6a0bdcbc3fc..ef552b01d4e 100644
--- a/saleor/graphql/account/tests/test_account.py
+++ b/saleor/graphql/account/tests/test_account.py
@@ -9,6 +9,7 @@
 
 import graphene
 import pytest
+from django.conf import settings
 from django.contrib.auth.models import Group
 from django.contrib.auth.tokens import default_token_generator
 from django.core.exceptions import ValidationError
@@ -1384,6 +1385,7 @@ def test_user_with_cancelled_fulfillments(
             }
             user {
                 id
+                email
             }
         }
     }
@@ -1488,6 +1490,22 @@ def test_customer_register_no_redirect_url(mocked_notify, api_client):
     mocked_notify.assert_not_called()
 
 
+@override_settings(ENABLE_ACCOUNT_CONFIRMATION_BY_EMAIL=False)
+def test_customer_register_upper_case_email(api_client):
+    # given
+    email = "CUSTOMER@example.com"
+    variables = {"email": email, "password": "Password"}
+
+    # when
+    response = api_client.post_graphql(ACCOUNT_REGISTER_MUTATION, variables)
+    content = get_graphql_content(response)
+
+    # then
+    data = content["data"]["accountRegister"]
+    assert not data["errors"]
+    assert data["user"]["email"].lower()
+
+
 CUSTOMER_CREATE_MUTATION = """
     mutation CreateCustomer(
         $email: String, $firstName: String, $lastName: String, $channel: String
@@ -4891,6 +4909,45 @@ def test_account_reset_password(
     )
 
 
+@freeze_time("2018-05-31 12:00:01")
+@patch("saleor.plugins.manager.PluginsManager.notify")
+def test_account_reset_password_with_upper_case_email(
+    mocked_notify,
+    user_api_client,
+    customer_user,
+    channel_PLN,
+    channel_USD,
+    site_settings,
+):
+    redirect_url = "https://www.example.com"
+    variables = {
+        "email": customer_user.email.upper(),
+        "redirectUrl": redirect_url,
+        "channel": channel_PLN.slug,
+    }
+    response = user_api_client.post_graphql(REQUEST_PASSWORD_RESET_MUTATION, variables)
+    content = get_graphql_content(response)
+    data = content["data"]["requestPasswordReset"]
+    assert not data["errors"]
+    token = default_token_generator.make_token(customer_user)
+    params = urlencode({"email": customer_user.email, "token": token})
+    reset_url = prepare_url(params, redirect_url)
+    expected_payload = {
+        "user": get_default_user_payload(customer_user),
+        "reset_url": reset_url,
+        "token": token,
+        "recipient_email": customer_user.email,
+        "channel_slug": channel_PLN.slug,
+        **get_site_context_payload(site_settings.site),
+    }
+
+    mocked_notify.assert_called_once_with(
+        NotifyEventType.ACCOUNT_PASSWORD_RESET,
+        payload=expected_payload,
+        channel_slug=channel_PLN.slug,
+    )
+
+
 @freeze_time("2018-05-31 12:00:01")
 @patch("saleor.graphql.account.mutations.base.assign_user_gift_cards")
 @patch("saleor.graphql.account.mutations.base.match_orders_with_new_user")
@@ -6359,6 +6416,58 @@ def test_address_query_with_invalid_object_type(
 """
 
 
+@freeze_time("2018-05-31 12:00:01")
+@patch("saleor.plugins.manager.PluginsManager.notify")
+def test_account_request_email_change_with_upper_case_email(
+    mocked_notify,
+    user_api_client,
+    customer_user,
+    site_settings,
+    channel_PLN,
+):
+    # given
+    new_email = "NEW_EMAIL@example.com"
+    redirect_url = "https://www.example.com"
+    variables = {
+        "new_email": new_email,
+        "redirect_url": redirect_url,
+        "password": "password",
+    }
+    token_payload = {
+        "old_email": customer_user.email,
+        "new_email": new_email.lower(),
+        "user_pk": customer_user.pk,
+    }
+    token = create_token(token_payload, settings.JWT_TTL_REQUEST_EMAIL_CHANGE)
+
+    # when
+    response = user_api_client.post_graphql(REQUEST_EMAIL_CHANGE_QUERY, variables)
+    content = get_graphql_content(response)
+
+    # then
+    data = content["data"]["requestEmailChange"]
+    assert not data["errors"]
+
+    params = urlencode({"token": token})
+    redirect_url = prepare_url(params, redirect_url)
+    expected_payload = {
+        "user": get_default_user_payload(customer_user),
+        "recipient_email": new_email.lower(),
+        "token": token,
+        "redirect_url": redirect_url,
+        "old_email": customer_user.email,
+        "new_email": new_email.lower(),
+        "channel_slug": channel_PLN.slug,
+        **get_site_context_payload(site_settings.site),
+    }
+
+    mocked_notify.assert_called_once_with(
+        NotifyEventType.ACCOUNT_CHANGE_EMAIL_REQUEST,
+        payload=expected_payload,
+        channel_slug=channel_PLN.slug,
+    )
+
+
 def test_request_email_change(user_api_client, customer_user, channel_PLN):
     variables = {
         "password": "password",
diff --git a/saleor/graphql/account/utils.py b/saleor/graphql/account/utils.py
index defd04c8992..903d3455b74 100644
--- a/saleor/graphql/account/utils.py
+++ b/saleor/graphql/account/utils.py
@@ -11,6 +11,7 @@
 
 from ...account import events as account_events
 from ...account.error_codes import AccountErrorCode
+from ...account.models import User
 from ...core.exceptions import PermissionDenied
 from ...core.permissions import (
     AccountPermissions,
@@ -22,7 +23,6 @@
 if TYPE_CHECKING:
     from django.db.models import QuerySet
 
-    from ...account.models import User
     from ...app.models import App
 
 
@@ -510,3 +510,21 @@ def check_is_owner_or_has_one_of_perms(
     """
     if not is_owner_or_has_one_of_perms(requestor, owner, *perms):
         raise PermissionDenied(permissions=list(perms) + [AuthorizationFilters.OWNER])
+
+
+def retrieve_user_by_email(email):
+    """Retrieve user by email.
+
+    Email lookup is case-insensitive, unless the query returns more than one user. In
+    such a case, function return case-sensitive result.
+    """
+    users = User.objects.filter(email__iexact=email)
+
+    if len(users) > 1:
+        users_exact = [user for user in users if user.email == email]
+        users_iexact = [user for user in users if user.email == email.lower()]
+        users = users_exact or users_iexact  # type: ignore
+
+    if users:
+        return users[0]
+    return None


Code review comments:
diff_hunk: @@ -510,3 +510,21 @@ def check_is_owner_or_has_one_of_perms(
     """
     if not is_owner_or_has_one_of_perms(requestor, owner, *perms):
         raise PermissionDenied(permissions=list(perms) + [AuthorizationFilters.OWNER])
+
+
+def retrieve_user_by_email(email):
path: saleor/graphql/account/utils.py
text: @user1:
IMO it's shouldn't be in `graphql` path. Maybe it's should be in `/saleor/account/utils.py`. 
We should add some unit tests for this function. 

@author:
agree, fits better, thanks