Fix #245 : handle email as case insensitive while retrieving user in DB (#250)

* Add unit tests to enforce email case insensitivity

* Handle email as case insentitive while retrieving user in DB

* Apply isort/black

* Add migration doc
This commit is contained in:
François Voron
2020-07-09 18:44:25 +02:00
committed by GitHub
parent e63a67ead3
commit b7dbdf6ea6
12 changed files with 70 additions and 25 deletions

View File

@ -0,0 +1,10 @@
# 2.x.x ➡️ 3.x.x
## Emails are now case-insensitive
Before 3.x.x, the local part (before the @) of the email address was case-sensitive. Therefore, `king.arthur@camelot.bt` and `King.Arthur@camelot.bt` were considered as **two different users**. This behaviour was a bit confusing and not consistent with 99% of web services out there.
After 3.x.x, users are fetched from the database with a case-insensitive email search. Bear in mind though that if the user registers with the email `King.Arthur@camelot.bt`, it will be stored exactly like this in the database (with casing) ; but he will be able to login as `king.arthur@camelot.bt`.
!!! danger
It's super important then, before you upgrade to 3.x.x that you **check if there are several users with the same email with different cases** ; and that you **merge or delete those accounts**.

View File

@ -28,7 +28,9 @@ class MongoDBUserDatabase(BaseUserDatabase[UD]):
return self.user_db_model(**user) if user else None
async def get_by_email(self, email: str) -> Optional[UD]:
user = await self.collection.find_one({"email": email})
user = await self.collection.find_one(
{"email": {"$regex": email, "$options": "i"}}
)
return self.user_db_model(**user) if user else None
async def get_by_oauth_account(self, oauth: str, account_id: str) -> Optional[UD]:

View File

@ -3,7 +3,7 @@ from typing import Mapping, Optional, Type
from databases import Database
from pydantic import UUID4
from sqlalchemy import Boolean, Column, ForeignKey, Integer, String, Table, select
from sqlalchemy import Boolean, Column, ForeignKey, Integer, String, Table, func, select
from sqlalchemy.dialects.postgresql import UUID
from sqlalchemy.ext.declarative import declared_attr
from sqlalchemy.types import CHAR, TypeDecorator
@ -120,7 +120,9 @@ class SQLAlchemyUserDatabase(BaseUserDatabase[UD]):
return await self._make_user(user) if user else None
async def get_by_email(self, email: str) -> Optional[UD]:
query = self.users.select().where(self.users.c.email == email)
query = self.users.select().where(
func.lower(self.users.c.email) == func.lower(email)
)
user = await self.database.fetch_one(query)
return await self._make_user(user) if user else None

View File

@ -77,19 +77,19 @@ class TortoiseUserDatabase(BaseUserDatabase[UD]):
return None
async def get_by_email(self, email: str) -> Optional[UD]:
try:
query = self.model.get(email=email)
query = self.model.filter(email__iexact=email).first()
if self.oauth_account_model is not None:
query = query.prefetch_related("oauth_accounts")
if self.oauth_account_model is not None:
query = query.prefetch_related("oauth_accounts")
user = await query
user_dict = await user.to_dict()
user = await query
return self.user_db_model(**user_dict)
except DoesNotExist:
if user is None:
return None
user_dict = await user.to_dict()
return self.user_db_model(**user_dict)
async def get_by_oauth_account(self, oauth: str, account_id: str) -> Optional[UD]:
try:
query = self.model.get(

View File

@ -53,3 +53,4 @@ nav:
- Migration:
- migration/08_to_1x.md
- migration/1x_to_2x.md
- migration/2x_to_3x.md

View File

@ -153,11 +153,12 @@ def mock_user_db(user, inactive_user, superuser) -> BaseUserDatabase:
return None
async def get_by_email(self, email: str) -> Optional[UserDB]:
if email == user.email:
lower_email = email.lower()
if lower_email == user.email.lower():
return user
if email == inactive_user.email:
if lower_email == inactive_user.email.lower():
return inactive_user
if email == superuser.email:
if lower_email == superuser.email.lower():
return superuser
return None
@ -188,11 +189,12 @@ def mock_user_db_oauth(
return None
async def get_by_email(self, email: str) -> Optional[UserDBOAuth]:
if email == user_oauth.email:
lower_email = email.lower()
if lower_email == user_oauth.email.lower():
return user_oauth
if email == inactive_user_oauth.email:
if lower_email == inactive_user_oauth.email.lower():
return inactive_user_oauth
if email == superuser_oauth.email:
if lower_email == superuser_oauth.email.lower():
return superuser_oauth
return None

View File

@ -77,6 +77,11 @@ async def test_queries(mongodb_user_db: MongoDBUserDatabase[UserDB]):
assert email_user is not None
assert email_user.id == user_db.id
# Get by uppercased email
email_user = await mongodb_user_db.get_by_email("Lancelot@camelot.bt")
assert email_user is not None
assert email_user.id == user_db.id
# Exception when inserting existing email
with pytest.raises(pymongo.errors.DuplicateKeyError):
await mongodb_user_db.create(user)

View File

@ -95,6 +95,11 @@ async def test_queries(sqlalchemy_user_db: SQLAlchemyUserDatabase[UserDB]):
assert email_user is not None
assert email_user.id == user_db.id
# Get by uppercased email
email_user = await sqlalchemy_user_db.get_by_email("Lancelot@camelot.bt")
assert email_user is not None
assert email_user.id == user_db.id
# Exception when inserting existing email
with pytest.raises(sqlite3.IntegrityError):
await sqlalchemy_user_db.create(user)

View File

@ -80,6 +80,11 @@ async def test_queries(tortoise_user_db: TortoiseUserDatabase[UserDB]):
assert email_user is not None
assert email_user.id == user_db.id
# Get by uppercased email
email_user = await tortoise_user_db.get_by_email("Lancelot@camelot.bt")
assert email_user is not None
assert email_user.id == user_db.id
# Exception when inserting existing email
with pytest.raises(IntegrityError):
await tortoise_user_db.create(user)

View File

@ -63,10 +63,13 @@ class TestLogin:
data = cast(Dict[str, Any], response.json())
assert data["detail"] == ErrorCode.LOGIN_BAD_CREDENTIALS
@pytest.mark.parametrize(
"email", ["king.arthur@camelot.bt", "King.Arthur@camelot.bt"]
)
async def test_valid_credentials(
self, path, test_app_client: httpx.AsyncClient, user: UserDB
self, path, email, test_app_client: httpx.AsyncClient, user: UserDB
):
data = {"username": "king.arthur@camelot.bt", "password": "guinevere"}
data = {"username": email, "password": "guinevere"}
response = await test_app_client.post(path, data=data)
assert response.status_code == status.HTTP_200_OK
assert response.json() == {"token": str(user.id)}

View File

@ -65,18 +65,24 @@ class TestRegister:
assert response.status_code == status.HTTP_422_UNPROCESSABLE_ENTITY
assert after_register.called is False
@pytest.mark.parametrize(
"email", ["king.arthur@camelot.bt", "King.Arthur@camelot.bt"]
)
async def test_existing_user(
self, test_app_client: httpx.AsyncClient, after_register
self, email, test_app_client: httpx.AsyncClient, after_register
):
json = {"email": "king.arthur@camelot.bt", "password": "guinevere"}
json = {"email": email, "password": "guinevere"}
response = await test_app_client.post("/register", json=json)
assert response.status_code == status.HTTP_400_BAD_REQUEST
data = cast(Dict[str, Any], response.json())
assert data["detail"] == ErrorCode.REGISTER_USER_ALREADY_EXISTS
assert after_register.called is False
async def test_valid_body(self, test_app_client: httpx.AsyncClient, after_register):
json = {"email": "lancelot@camelot.bt", "password": "guinevere"}
@pytest.mark.parametrize("email", ["lancelot@camelot.bt", "Lancelot@camelot.bt"])
async def test_valid_body(
self, email, test_app_client: httpx.AsyncClient, after_register
):
json = {"email": email, "password": "guinevere"}
response = await test_app_client.post("/register", json=json)
assert response.status_code == status.HTTP_201_CREATED
assert after_register.called is True
@ -88,6 +94,7 @@ class TestRegister:
actual_user = after_register.call_args[0][0]
assert str(actual_user.id) == data["id"]
assert str(actual_user.email) == email
request = after_register.call_args[0][1]
assert isinstance(request, Request)

View File

@ -80,10 +80,13 @@ class TestForgotPassword:
assert response.status_code == status.HTTP_202_ACCEPTED
assert after_forgot_password.called is False
@pytest.mark.parametrize(
"email", ["king.arthur@camelot.bt", "King.Arthur@camelot.bt"]
)
async def test_existing_user(
self, test_app_client: httpx.AsyncClient, after_forgot_password, user
self, email, test_app_client: httpx.AsyncClient, after_forgot_password, user
):
json = {"email": "king.arthur@camelot.bt"}
json = {"email": email}
response = await test_app_client.post("/forgot-password", json=json)
assert response.status_code == status.HTTP_202_ACCEPTED
assert after_forgot_password.called is True