From feb6b1f26c08be38e8ad0fc50572f4ecfd94f613 Mon Sep 17 00:00:00 2001 From: Ilya Kreymer Date: Thu, 19 Sep 2024 12:20:34 -0700 Subject: [PATCH] Ensure email comparisons are case-insensitive, emails stored as lowercase (#2084) (#2086) (fixes from 1.11.7) - Add a custom EmailStr type which lowercases the full e-mail, not just the domain. - Ensure EmailStr is used throughout wherever e-mails are used, both for invites and user models - Tests: update to check for lowercase email responses, e-mails returned from APIs are always lowercase - Tests: remove tests where '@' was ur-lencoded, should not be possible since POSTing JSON and no url-decoding is done/expected. E-mails should have '@' present. - Fixes #2083 where invites were rejected due to case differences - CI: pin pymongo dependency due to latest releases update, update python used for CI --- .github/workflows/k3d-ci.yaml | 4 ++-- backend/btrixcloud/invites.py | 8 ++++++-- backend/btrixcloud/models.py | 24 +++++++++++++++++------- backend/btrixcloud/orgs.py | 5 +---- backend/btrixcloud/users.py | 5 ++--- backend/requirements.txt | 1 + backend/test/test_org.py | 6 ++++-- backend/test/test_org_subs.py | 2 +- backend/test/test_users.py | 4 ++-- 9 files changed, 36 insertions(+), 23 deletions(-) diff --git a/.github/workflows/k3d-ci.yaml b/.github/workflows/k3d-ci.yaml index b0a440f2..a2eff4e7 100644 --- a/.github/workflows/k3d-ci.yaml +++ b/.github/workflows/k3d-ci.yaml @@ -81,9 +81,9 @@ jobs: helm upgrade --install -f ./chart/values.yaml -f ./chart/test/test.yaml btrix ./chart/ - name: Install Python - uses: actions/setup-python@v3 + uses: actions/setup-python@v5 with: - python-version: '3.9' + python-version: 3.x - name: Install Python Libs run: pip install -r ./backend/test-requirements.txt diff --git a/backend/btrixcloud/invites.py b/backend/btrixcloud/invites.py index 27870b37..03d90963 100644 --- a/backend/btrixcloud/invites.py +++ b/backend/btrixcloud/invites.py @@ -13,6 +13,7 @@ from fastapi import HTTPException from .pagination import DEFAULT_PAGE_SIZE from .models import ( + EmailStr, UserRole, InvitePending, InviteRequest, @@ -133,7 +134,10 @@ class InviteOps: ) async def get_valid_invite( - self, invite_token: UUID, email: Optional[str], userid: Optional[UUID] = None + self, + invite_token: UUID, + email: Optional[EmailStr], + userid: Optional[UUID] = None, ) -> InvitePending: """Retrieve a valid invite data from db, or throw if invalid""" token_hash = get_hash(invite_token) @@ -156,7 +160,7 @@ class InviteOps: await self.invites.delete_one({"_id": invite_token}) async def remove_invite_by_email( - self, email: str, oid: Optional[UUID] = None + self, email: EmailStr, oid: Optional[UUID] = None ) -> Any: """remove invite from invite list by email""" query: dict[str, object] = {"email": email} diff --git a/backend/btrixcloud/models.py b/backend/btrixcloud/models.py index 2dfad3ed..0ff0347f 100644 --- a/backend/btrixcloud/models.py +++ b/backend/btrixcloud/models.py @@ -15,7 +15,8 @@ from pydantic import ( Field, HttpUrl as HttpUrlNonStr, AnyHttpUrl as AnyHttpUrlNonStr, - EmailStr, + EmailStr as CasedEmailStr, + validate_email, RootModel, BeforeValidator, TypeAdapter, @@ -47,6 +48,15 @@ HttpUrl = Annotated[ ] +# pylint: disable=too-few-public-methods +class EmailStr(CasedEmailStr): + """EmailStr type that lowercases the full email""" + + @classmethod + def _validate(cls, value: CasedEmailStr, /) -> CasedEmailStr: + return validate_email(value)[1].lower() + + # pylint: disable=invalid-name, too-many-lines # ============================================================================ class UserRole(IntEnum): @@ -70,11 +80,11 @@ class InvitePending(BaseMongoModel): id: UUID created: datetime tokenHash: str - inviterEmail: str + inviterEmail: EmailStr fromSuperuser: Optional[bool] = False oid: Optional[UUID] = None role: UserRole = UserRole.VIEWER - email: Optional[str] = "" + email: Optional[EmailStr] = None # set if existing user userid: Optional[UUID] = None @@ -84,13 +94,13 @@ class InviteOut(BaseModel): """Single invite output model""" created: datetime - inviterEmail: str + inviterEmail: EmailStr inviterName: str oid: Optional[UUID] = None orgName: Optional[str] = None orgSlug: Optional[str] = None role: UserRole = UserRole.VIEWER - email: Optional[str] = "" + email: Optional[EmailStr] = None firstOrgAdmin: Optional[bool] = None @@ -98,7 +108,7 @@ class InviteOut(BaseModel): class InviteRequest(BaseModel): """Request to invite another user""" - email: str + email: EmailStr # ============================================================================ @@ -1179,7 +1189,7 @@ class SubscriptionCreate(BaseModel): status: str planId: str - firstAdminInviteEmail: str + firstAdminInviteEmail: EmailStr quotas: Optional[OrgQuotas] = None diff --git a/backend/btrixcloud/orgs.py b/backend/btrixcloud/orgs.py index fec17667..c517bb07 100644 --- a/backend/btrixcloud/orgs.py +++ b/backend/btrixcloud/orgs.py @@ -8,7 +8,6 @@ import json import math import os import time -import urllib.parse from uuid import UUID, uuid4 from tempfile import NamedTemporaryFile @@ -1614,9 +1613,7 @@ def init_orgs_api( async def delete_invite( invite: RemovePendingInvite, org: Organization = Depends(org_owner_dep) ): - # URL decode email just in case - email = urllib.parse.unquote(invite.email) - result = await user_manager.invites.remove_invite_by_email(email, org.id) + result = await user_manager.invites.remove_invite_by_email(invite.email, org.id) if result.deleted_count > 0: return { "removed": True, diff --git a/backend/btrixcloud/users.py b/backend/btrixcloud/users.py index e0d6896b..6519d1cc 100644 --- a/backend/btrixcloud/users.py +++ b/backend/btrixcloud/users.py @@ -8,8 +8,6 @@ import asyncio from typing import Optional, List, TYPE_CHECKING, cast, Callable -from pydantic import EmailStr - from fastapi import ( Request, HTTPException, @@ -22,6 +20,7 @@ from pymongo.errors import DuplicateKeyError from pymongo.collation import Collation from .models import ( + EmailStr, UserCreate, UserUpdateEmailName, UserUpdatePassword, @@ -685,7 +684,7 @@ def init_users_router( return await user_manager.invites.get_invite_out(invite, user_manager, True) @users_router.get("/invite/{token}", tags=["invites"], response_model=InviteOut) - async def get_invite_info(token: UUID, email: str): + async def get_invite_info(token: UUID, email: EmailStr): invite = await user_manager.invites.get_valid_invite(token, email) return await user_manager.invites.get_invite_out(invite, user_manager, True) diff --git a/backend/requirements.txt b/backend/requirements.txt index 3c6b868b..b29d24d4 100644 --- a/backend/requirements.txt +++ b/backend/requirements.txt @@ -2,6 +2,7 @@ gunicorn uvicorn[standard] fastapi==0.103.2 motor==3.3.1 +pymongo==4.8.0 passlib PyJWT==2.8.0 pydantic==2.8.2 diff --git a/backend/test/test_org.py b/backend/test/test_org.py index 34bda297..5c0dddef 100644 --- a/backend/test/test_org.py +++ b/backend/test/test_org.py @@ -360,16 +360,18 @@ def test_get_pending_org_invites( ("user+comment-org@example.com", "user+comment-org@example.com"), # URL encoded email address with comments ( - "user%2Bcomment-encoded-org%40example.com", + "user%2Bcomment-encoded-org@example.com", "user+comment-encoded-org@example.com", ), # User email with diacritic characters ("diacritic-tést-org@example.com", "diacritic-tést-org@example.com"), # User email with encoded diacritic characters ( - "diacritic-t%C3%A9st-encoded-org%40example.com", + "diacritic-t%C3%A9st-encoded-org@example.com", "diacritic-tést-encoded-org@example.com", ), + # User email with upper case characters, stored as all lowercase + ("exampleName@EXAMple.com", "examplename@example.com"), ], ) def test_send_and_accept_org_invite( diff --git a/backend/test/test_org_subs.py b/backend/test/test_org_subs.py index b48c5bb4..88d93d79 100644 --- a/backend/test/test_org_subs.py +++ b/backend/test/test_org_subs.py @@ -12,7 +12,7 @@ existing_user_invite_token = None VALID_PASSWORD = "ValidPassW0rd!" -invite_email = "test-user@example.com" +invite_email = "test-User@EXample.com" def test_create_sub_org_invalid_auth(crawler_auth_headers): diff --git a/backend/test/test_users.py b/backend/test/test_users.py index a7562700..2ad8e0a3 100644 --- a/backend/test/test_users.py +++ b/backend/test/test_users.py @@ -50,7 +50,7 @@ def test_me_with_orgs(crawler_auth_headers, default_org_id): assert r.status_code == 200 data = r.json() - assert data["email"] == CRAWLER_USERNAME + assert data["email"] == CRAWLER_USERNAME_LOWERCASE assert data["id"] # assert data["is_active"] assert data["is_superuser"] is False @@ -102,7 +102,7 @@ def test_login_user_info(admin_auth_headers, crawler_userid, default_org_id): assert user_info["id"] == crawler_userid assert user_info["name"] == "new-crawler" - assert user_info["email"] == CRAWLER_USERNAME + assert user_info["email"] == CRAWLER_USERNAME_LOWERCASE assert user_info["is_superuser"] is False assert user_info["is_verified"]