Skip to content

Commit

Permalink
Ensure email comparisons are case-insensitive, emails stored as lower…
Browse files Browse the repository at this point in the history
…case (#2084)

- 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
- bump to 1.11.7
  • Loading branch information
ikreymer authored Sep 19, 2024
1 parent 1f919de commit 7a61568
Show file tree
Hide file tree
Showing 15 changed files with 46 additions and 33 deletions.
4 changes: 2 additions & 2 deletions .github/workflows/k3d-ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 4 additions & 4 deletions .github/workflows/ui-tests-playwright.yml
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,11 @@ jobs:
working-directory: ./frontend
steps:
- name: Checkout
uses: actions/checkout@v3
uses: actions/checkout@v4
- name: Setup Node
uses: actions/setup-node@v3
uses: actions/setup-node@v4
with:
node-version: '18'
node-version: '20'
cache: 'yarn'
cache-dependency-path: frontend/yarn.lock
- name: Install dependencies
Expand All @@ -43,7 +43,7 @@ jobs:
id: build-frontend
- name: Run Playwright tests
run: cd frontend && yarn playwright test
- uses: actions/upload-artifact@v2
- uses: actions/upload-artifact@v4
if: always()
with:
name: playwright-report
Expand Down
8 changes: 6 additions & 2 deletions backend/btrixcloud/invites.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@

from .pagination import DEFAULT_PAGE_SIZE
from .models import (
EmailStr,
UserRole,
InvitePending,
InviteRequest,
Expand Down Expand Up @@ -133,7 +134,10 @@ async def add_existing_user_invite(
)

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)
Expand All @@ -156,7 +160,7 @@ async def remove_invite(self, invite_token: UUID) -> None:
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}
Expand Down
24 changes: 17 additions & 7 deletions backend/btrixcloud/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@
Field,
HttpUrl as HttpUrlNonStr,
AnyHttpUrl as AnyHttpUrlNonStr,
EmailStr,
EmailStr as CasedEmailStr,
validate_email,
RootModel,
BeforeValidator,
TypeAdapter,
Expand Down Expand Up @@ -47,6 +48,15 @@
]


# 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):
Expand All @@ -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

Expand All @@ -84,21 +94,21 @@ 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


# ============================================================================
class InviteRequest(BaseModel):
"""Request to invite another user"""

email: str
email: EmailStr


# ============================================================================
Expand Down Expand Up @@ -1179,7 +1189,7 @@ class SubscriptionCreate(BaseModel):
status: str
planId: str

firstAdminInviteEmail: str
firstAdminInviteEmail: EmailStr
quotas: Optional[OrgQuotas] = None


Expand Down
5 changes: 1 addition & 4 deletions backend/btrixcloud/orgs.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
import math
import os
import time
import urllib.parse

from uuid import UUID, uuid4
from tempfile import NamedTemporaryFile
Expand Down Expand Up @@ -1614,9 +1613,7 @@ async def get_pending_org_invites(
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,
Expand Down
5 changes: 2 additions & 3 deletions backend/btrixcloud/users.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@

from typing import Optional, List, TYPE_CHECKING, cast, Callable

from pydantic import EmailStr

from fastapi import (
Request,
HTTPException,
Expand All @@ -22,6 +20,7 @@
from pymongo.collation import Collation

from .models import (
EmailStr,
UserCreate,
UserUpdateEmailName,
UserUpdatePassword,
Expand Down Expand Up @@ -685,7 +684,7 @@ async def get_existing_user_invite_info(
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)
Expand Down
2 changes: 1 addition & 1 deletion backend/btrixcloud/version.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
""" current version """

__version__ = "1.11.6"
__version__ = "1.11.7"
1 change: 1 addition & 0 deletions backend/requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 4 additions & 2 deletions backend/test/test_org.py
Original file line number Diff line number Diff line change
Expand Up @@ -360,16 +360,18 @@ def test_get_pending_org_invites(
("[email protected]", "[email protected]"),
# URL encoded email address with comments
(
"user%2Bcomment-encoded-org%40example.com",
"user%2Bcomment-encoded-org@example.com",
"[email protected]",
),
# User email with diacritic characters
("diacritic-té[email protected]", "diacritic-té[email protected]"),
# User email with encoded diacritic characters
(
"diacritic-t%C3%A9st-encoded-org%40example.com",
"diacritic-t%C3%A9st-encoded-org@example.com",
"diacritic-té[email protected]",
),
# User email with upper case characters, stored as all lowercase
("[email protected]", "[email protected]"),
],
)
def test_send_and_accept_org_invite(
Expand Down
2 changes: 1 addition & 1 deletion backend/test/test_org_subs.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@

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):
Expand Down
4 changes: 2 additions & 2 deletions backend/test/test_users.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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"]

Expand Down
2 changes: 1 addition & 1 deletion chart/Chart.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ type: application
icon: https://webrecorder.net/assets/icon.png

# Browsertrix and Chart Version
version: v1.11.6
version: v1.11.7

dependencies:
- name: btrix-admin-logging
Expand Down
4 changes: 2 additions & 2 deletions chart/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ default_org: "My Organization"

# API Image
# =========================================
backend_image: "docker.io/webrecorder/browsertrix-backend:1.11.6"
backend_image: "docker.io/webrecorder/browsertrix-backend:1.11.7"
backend_pull_policy: "Always"

backend_password_secret: "PASSWORD!"
Expand Down Expand Up @@ -141,7 +141,7 @@ backend_avg_memory_threshold: 95

# Nginx Image
# =========================================
frontend_image: "docker.io/webrecorder/browsertrix-frontend:1.11.6"
frontend_image: "docker.io/webrecorder/browsertrix-frontend:1.11.7"
frontend_pull_policy: "Always"

frontend_cpu: "10m"
Expand Down
2 changes: 1 addition & 1 deletion frontend/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "browsertrix-frontend",
"version": "1.11.6",
"version": "1.11.7",
"main": "index.ts",
"license": "AGPL-3.0-or-later",
"dependencies": {
Expand Down
2 changes: 1 addition & 1 deletion version.txt
Original file line number Diff line number Diff line change
@@ -1 +1 @@
1.11.6
1.11.7

0 comments on commit 7a61568

Please sign in to comment.