diff --git a/app/core/config.py b/app/core/config.py index b04e3ef9..8726cd6f 100644 --- a/app/core/config.py +++ b/app/core/config.py @@ -39,6 +39,8 @@ class Settings(BaseSettings): MOBILE_SESSION_LIMIT: int = 3 MOBILE_SESSION_TTL_SECONDS: int = 180 MOBILE_SESSION_DAYS: int = 7 + SESSION_ACTIVITY_THROTTLE_SECONDS: int = 60 + # Mobile auth validation defaults MOBILE_AUTH_PASSWORD_MIN_LEN: int = 8 MOBILE_AUTH_PASSWORD_MAX_LEN: int = 128 diff --git a/app/core/image_validation.py b/app/core/image_validation.py index 7e427c77..c4680400 100644 --- a/app/core/image_validation.py +++ b/app/core/image_validation.py @@ -29,6 +29,7 @@ def sanitise_filename(raw: str | None, extension: str) -> str: if not raw: return f"{prefix}.{extension}" name = re.sub(r'[\\/:*?"<>|\x00-\x1f]', "_", raw) + name = name.replace("..", "_") name = name.lstrip(".")[:128] return f"{prefix}_{name}" diff --git a/app/deps/token_auth.py b/app/deps/token_auth.py index c6c2ff59..26b0da8a 100644 --- a/app/deps/token_auth.py +++ b/app/deps/token_auth.py @@ -27,7 +27,10 @@ async def get_current_mobile_user( ) -> MobileUserSchema: """ Dependency to get the current logged-in mobile user. - Fast path: Redis cache (0 DB queries). + Fast path: Redis cache hit. Usually 0 DB queries; occasionally 1 cheap, + throttled UPDATE to last_active (see SESSION_ACTIVITY_THROTTLE_SECONDS) — + this is not a full round trip through the slow path, just a single + indexed write on the request's existing connection. Slow path: Postgres fallback (2 DB queries) with cache re-population. """ token = credentials.credentials @@ -49,6 +52,23 @@ async def get_current_mobile_user( raise HTTPException(status_code=401, detail="Session expired") if cached.blocked: raise HTTPException(status_code=403, detail="User is blocked") + + now = datetime.now(timezone.utc) + if (now - cached.last_active).total_seconds() > settings.SESSION_ACTIVITY_THROTTLE_SECONDS: + await container.session_service.session_querier.update_session_activity( + id=cached.session_id + ) + await SessionService.cache_session_for_auth( + redis=redis, + session_id=cached.session_id, + user_id=cached.user_id, + email=cached.email, + expires_at=cached.expires_at, + blocked=cached.blocked, + ttl=settings.MOBILE_SESSION_TTL_SECONDS, + last_active=now, + ) + return MobileUserSchema( user_id=cached.user_id, email=cached.email, @@ -70,7 +90,9 @@ async def get_current_mobile_user( if user.blocked: raise HTTPException(status_code=403, detail="User is blocked") - # Re-populate cache so next request hits Redis + # Re-populate cache so next request hits Redis. The session row was just + # fetched fresh from Postgres, so its last_active is already accurate — + # no extra write needed here, only cache population. await SessionService.cache_session_for_auth( redis=redis, session_id=session.id, @@ -79,6 +101,7 @@ async def get_current_mobile_user( expires_at=session.expires_at, blocked=user.blocked, ttl=settings.MOBILE_SESSION_TTL_SECONDS, + last_active=session.last_active, ) return MobileUserSchema( diff --git a/app/schema/request/mobile/auth.py b/app/schema/request/mobile/auth.py index 23a13717..dea9933a 100644 --- a/app/schema/request/mobile/auth.py +++ b/app/schema/request/mobile/auth.py @@ -20,7 +20,7 @@ class MobileAuthBaseRequest(BaseModel): min_length=1, max_length=settings.MOBILE_AUTH_DEVICE_TYPE_MAX_LEN, ) - device_id: UUID + physical_device_id: UUID @field_validator("email", mode="before") @classmethod diff --git a/app/service/device.py b/app/service/device.py index 6bf69f17..b5840ea7 100644 --- a/app/service/device.py +++ b/app/service/device.py @@ -1,5 +1,4 @@ from db.generated import devices as device_queries -from app.core.securite import create_totp_secret import uuid from app.core.exceptions import DBException,AppException, DBExceptionImpl from db.generated.models import UserDevice @@ -11,30 +10,6 @@ class DeviceService: def init(self: "DeviceService", device_querier: device_queries.AsyncQuerier) -> None: self.device_querier = device_querier - async def create_device( - self: "DeviceService", - user_id: uuid.UUID, - device_name: str, - device_type: str, - id: uuid.UUID | None = None, - ) -> UserDevice | None: - try : - DeviceCount = await self.count_devices(user_id=user_id) - if DeviceCount >=3: - raise AppException.bad_request("You can only have 3 devices") - return await self.device_querier.create_device( - arg=device_queries.CreateDeviceParams( - column_1=id, - user_id=user_id, - device_name=device_name, - device_type=device_type, - totp_secret=create_totp_secret(), - ) - - ) - except Exception as e : - raise DBException.handle(e) - async def activate_device( self: "DeviceService", device_id: uuid.UUID, diff --git a/app/service/session.py b/app/service/session.py index 403af567..cdfb7f47 100644 --- a/app/service/session.py +++ b/app/service/session.py @@ -14,6 +14,7 @@ class MobileSessionCache(BaseModel): email: str expires_at: datetime blocked: bool + last_active: datetime class SessionService: @@ -35,6 +36,7 @@ async def cache_session_for_auth( expires_at: datetime, blocked: bool, ttl: int, + last_active: datetime ) -> None: key = RedisKey.MobileSessionCache.value.format(session_id=session_id) payload = MobileSessionCache( @@ -43,6 +45,7 @@ async def cache_session_for_auth( email=email, expires_at=expires_at, blocked=blocked, + last_active=last_active ) await redis.set(key=key, value=payload.model_dump_json(), expire=ttl) diff --git a/app/service/users.py b/app/service/users.py index 603d1c34..f5c82277 100644 --- a/app/service/users.py +++ b/app/service/users.py @@ -31,7 +31,7 @@ from db.generated import user as user_queries from db.generated import devices as device_queries from db.generated import session as session_queries -from db.generated.models import User, UserDevice +from db.generated.models import User, UserDevice, UserSession from app.core.logger import logger from app.service.face_embedding import FaceImagePayload, FaceEmbeddingService from app.schema.internal.single_face_match import ClosestUserMatch @@ -62,26 +62,28 @@ async def _ensure_device_for_login( user_id: uuid.UUID, req: MobileAuthBaseRequest, ) -> UserDevice: - existing_device = await self.device_querier.get_device_by_id_any(id=req.device_id) + existing_device = await self.device_querier.get_device_by_physical_id( + user_id=user_id, + physical_device_id=req.physical_device_id + ) if existing_device: - if existing_device.user_id != user_id: - raise AppException.forbidden("Device already registered to another user") if existing_device.is_invalid_token: raise AppException.forbidden( "Device push token is invalid. Update the token before logging in." ) if not existing_device.is_active: - await self.device_querier.activate_device(id=req.device_id, user_id=user_id) + await self.device_querier.activate_device(id=existing_device.id, user_id=user_id) return existing_device device = await self.device_querier.create_device( arg=device_queries.CreateDeviceParams( - column_1=req.device_id, + column_1=None, user_id=user_id, device_name=req.device_name, device_type=req.device_type, totp_secret=None, + physical_device_id=req.physical_device_id ) ) if not device: @@ -274,25 +276,34 @@ async def _create_mobile_session( ) -> MobileAuthResponse: user_id: uuid.UUID = user.id - session_count = await self.session_querier.count_user_sessions(user_id=user_id) - if session_count and session_count >= AuthService.SESSION_LIMIT: - logger.warning( - "session_limit_reached user_id=%s limit=%s", - user_id, - AuthService.SESSION_LIMIT, - ) - raise AppException.forbidden("Maximum session limit reached") + device = await self._ensure_device_for_login(user_id, req) + + existing_session = await self.session_querier.get_session_by_device_for_user( + device_id=device.id, + user_id=user_id, + ) + + if existing_session is None: + sessions: list[UserSession] = [] + async for s in self.session_querier.list_sessions_by_user(user_id=user_id): + sessions.append(s) + + if len(sessions) >= AuthService.SESSION_LIMIT: + oldest = min(sessions, key=lambda s: (s.last_active, s.created_at)) + await SessionService.delete_session_cache(redis, oldest.id) + await self.session_querier.delete_session_by_id(id=oldest.id, user_id=user_id) + logger.warning( + "session_evicted user_id=%s evicted_session_id=%s", + user_id, oldest.id, + ) - device_id = req.device_id expires_at = datetime.now(timezone.utc) + timedelta( days=settings.MOBILE_SESSION_DAYS ) - await self._ensure_device_for_login(user_id, req) - session = await self.session_querier.upsert_session( user_id=user_id, - device_id=device_id, + device_id=device.id, expires_at=expires_at, ) @@ -312,6 +323,7 @@ async def _create_mobile_session( expires_at=session.expires_at, blocked=user.blocked, ttl=AuthService.REDIS_SESSION_TTL, + last_active=session.last_active ) return MobileAuthResponse( diff --git a/db/generated/devices.py b/db/generated/devices.py index 7da5fb91..ffb04426 100644 --- a/db/generated/devices.py +++ b/db/generated/devices.py @@ -33,11 +33,12 @@ user_id, device_name, device_type, - totp_secret + totp_secret, + physical_device_id ) VALUES ( - COALESCE(:p1, uuid_generate_v4()), :p2, :p3, :p4, :p5 + COALESCE(:p1, uuid_generate_v4()), :p2, :p3, :p4, :p5, :p6 ) -RETURNING id, user_id, device_name, device_type, totp_secret, is_2fa_enabled, last_active, created_at, push_token, is_active, is_invalid_token +RETURNING id, user_id, device_name, device_type, totp_secret, is_2fa_enabled, last_active, created_at, push_token, is_active, is_invalid_token, physical_device_id """ @@ -48,6 +49,7 @@ class CreateDeviceParams: device_name: Optional[str] device_type: Optional[str] totp_secret: Optional[str] + physical_device_id: uuid.UUID DEACTIVATE_DEVICE = """-- name: deactivate_device \\:exec @@ -67,21 +69,33 @@ class CreateDeviceParams: """ +GET_ANY_DEVICE_BY_PHYSICAL_ID = """-- name: get_any_device_by_physical_id \\:many +SELECT id, user_id, device_name, device_type, totp_secret, is_2fa_enabled, last_active, created_at, push_token, is_active, is_invalid_token, physical_device_id FROM user_devices +WHERE physical_device_id = :p1 +""" + + GET_DEVICE_BY_ID = """-- name: get_device_by_id \\:one -SELECT id, user_id, device_name, device_type, totp_secret, is_2fa_enabled, last_active, created_at, push_token, is_active, is_invalid_token from user_devices +SELECT id, user_id, device_name, device_type, totp_secret, is_2fa_enabled, last_active, created_at, push_token, is_active, is_invalid_token, physical_device_id from user_devices WHERE id = :p1 AND user_id = :p2 """ GET_DEVICE_BY_ID_ANY = """-- name: get_device_by_id_any \\:one -SELECT id, user_id, device_name, device_type, totp_secret, is_2fa_enabled, last_active, created_at, push_token, is_active, is_invalid_token from user_devices +SELECT id, user_id, device_name, device_type, totp_secret, is_2fa_enabled, last_active, created_at, push_token, is_active, is_invalid_token, physical_device_id from user_devices WHERE id = :p1 """ +GET_DEVICE_BY_PHYSICAL_ID = """-- name: get_device_by_physical_id \\:one +SELECT id, user_id, device_name, device_type, totp_secret, is_2fa_enabled, last_active, created_at, push_token, is_active, is_invalid_token, physical_device_id FROM user_devices +WHERE user_id = :p1 AND physical_device_id = :p2 +""" + + LIST_USER_DEVICES = """-- name: list_user_devices \\:many -SELECT id, user_id, device_name, device_type, totp_secret, is_2fa_enabled, last_active, created_at, push_token, is_active, is_invalid_token +SELECT id, user_id, device_name, device_type, totp_secret, is_2fa_enabled, last_active, created_at, push_token, is_active, is_invalid_token, physical_device_id FROM user_devices WHERE user_id = :p1 ORDER BY last_active DESC @@ -119,7 +133,7 @@ class CreateDeviceParams: is_invalid_token = FALSE WHERE id = :p1 AND user_id = :p3 -RETURNING id, user_id, device_name, device_type, totp_secret, is_2fa_enabled, last_active, created_at, push_token, is_active, is_invalid_token +RETURNING id, user_id, device_name, device_type, totp_secret, is_2fa_enabled, last_active, created_at, push_token, is_active, is_invalid_token, physical_device_id """ @@ -143,6 +157,7 @@ async def create_device(self, arg: CreateDeviceParams) -> Optional[models.UserDe "p3": arg.device_name, "p4": arg.device_type, "p5": arg.totp_secret, + "p6": arg.physical_device_id, })).first() if row is None: return None @@ -158,6 +173,7 @@ async def create_device(self, arg: CreateDeviceParams) -> Optional[models.UserDe push_token=row[8], is_active=row[9], is_invalid_token=row[10], + physical_device_id=row[11], ) async def deactivate_device(self, *, id: uuid.UUID, user_id: uuid.UUID) -> None: @@ -166,6 +182,24 @@ async def deactivate_device(self, *, id: uuid.UUID, user_id: uuid.UUID) -> None: async def enable_device2_fa(self, *, id: uuid.UUID, user_id: uuid.UUID) -> None: await self._conn.execute(sqlalchemy.text(ENABLE_DEVICE2_FA), {"p1": id, "p2": user_id}) + async def get_any_device_by_physical_id(self, *, physical_device_id: uuid.UUID) -> AsyncIterator[models.UserDevice]: + result = await self._conn.stream(sqlalchemy.text(GET_ANY_DEVICE_BY_PHYSICAL_ID), {"p1": physical_device_id}) + async for row in result: + yield models.UserDevice( + id=row[0], + user_id=row[1], + device_name=row[2], + device_type=row[3], + totp_secret=row[4], + is_2fa_enabled=row[5], + last_active=row[6], + created_at=row[7], + push_token=row[8], + is_active=row[9], + is_invalid_token=row[10], + physical_device_id=row[11], + ) + async def get_device_by_id(self, *, id: uuid.UUID, user_id: uuid.UUID) -> Optional[models.UserDevice]: row = (await self._conn.execute(sqlalchemy.text(GET_DEVICE_BY_ID), {"p1": id, "p2": user_id})).first() if row is None: @@ -182,6 +216,7 @@ async def get_device_by_id(self, *, id: uuid.UUID, user_id: uuid.UUID) -> Option push_token=row[8], is_active=row[9], is_invalid_token=row[10], + physical_device_id=row[11], ) async def get_device_by_id_any(self, *, id: uuid.UUID) -> Optional[models.UserDevice]: @@ -200,6 +235,26 @@ async def get_device_by_id_any(self, *, id: uuid.UUID) -> Optional[models.UserDe push_token=row[8], is_active=row[9], is_invalid_token=row[10], + physical_device_id=row[11], + ) + + async def get_device_by_physical_id(self, *, user_id: uuid.UUID, physical_device_id: uuid.UUID) -> Optional[models.UserDevice]: + row = (await self._conn.execute(sqlalchemy.text(GET_DEVICE_BY_PHYSICAL_ID), {"p1": user_id, "p2": physical_device_id})).first() + if row is None: + return None + return models.UserDevice( + id=row[0], + user_id=row[1], + device_name=row[2], + device_type=row[3], + totp_secret=row[4], + is_2fa_enabled=row[5], + last_active=row[6], + created_at=row[7], + push_token=row[8], + is_active=row[9], + is_invalid_token=row[10], + physical_device_id=row[11], ) async def list_user_devices(self, *, user_id: uuid.UUID) -> AsyncIterator[models.UserDevice]: @@ -217,6 +272,7 @@ async def list_user_devices(self, *, user_id: uuid.UUID) -> AsyncIterator[models push_token=row[8], is_active=row[9], is_invalid_token=row[10], + physical_device_id=row[11], ) async def mark_device_token_invalid(self, *, push_token: Optional[str]) -> None: @@ -244,4 +300,5 @@ async def update_device_push_token(self, *, id: uuid.UUID, push_token: Optional[ push_token=row[8], is_active=row[9], is_invalid_token=row[10], + physical_device_id=row[11], ) diff --git a/db/generated/models.py b/db/generated/models.py index 9908c54d..29dd707a 100644 --- a/db/generated/models.py +++ b/db/generated/models.py @@ -146,6 +146,17 @@ class ProcessingJob: completed_at: Optional[datetime.datetime] +@dataclasses.dataclass() +class RefreshToken: + id: uuid.UUID + session_id: uuid.UUID + family_id: uuid.UUID + token_hash: str + used: bool + created_at: datetime.datetime + used_at: Optional[datetime.datetime] + + @dataclasses.dataclass() class StaffDriveConnection: id: uuid.UUID @@ -261,6 +272,7 @@ class UserDevice: push_token: Optional[str] is_active: bool is_invalid_token: bool + physical_device_id: uuid.UUID @dataclasses.dataclass() diff --git a/db/queries/devices.sql b/db/queries/devices.sql index 8c91a255..a5d71817 100644 --- a/db/queries/devices.sql +++ b/db/queries/devices.sql @@ -4,9 +4,10 @@ INSERT INTO user_devices ( user_id, device_name, device_type, - totp_secret + totp_secret, + physical_device_id ) VALUES ( - COALESCE($1, uuid_generate_v4()), $2, $3, $4, $5 + COALESCE($1, uuid_generate_v4()), $2, $3, $4, $5, $6 ) RETURNING *; @@ -76,3 +77,11 @@ SET is_invalid_token = TRUE, is_active = FALSE WHERE push_token = $1; + +-- name: GetDeviceByPhysicalId :one +SELECT * FROM user_devices +WHERE user_id = $1 AND physical_device_id = $2; + +-- name: GetAnyDeviceByPhysicalId :many +SELECT * FROM user_devices +WHERE physical_device_id = $1; \ No newline at end of file diff --git a/migrations/sql/down/add_physical_device_id.sql b/migrations/sql/down/add_physical_device_id.sql new file mode 100644 index 00000000..d0ef0a39 --- /dev/null +++ b/migrations/sql/down/add_physical_device_id.sql @@ -0,0 +1,4 @@ +DROP INDEX IF EXISTS idx_user_devices_user_physical; + +ALTER TABLE user_devices + DROP COLUMN IF EXISTS physical_device_id; \ No newline at end of file diff --git a/migrations/sql/down/create_refresh_tokens.sql b/migrations/sql/down/create_refresh_tokens.sql new file mode 100644 index 00000000..a72a1751 --- /dev/null +++ b/migrations/sql/down/create_refresh_tokens.sql @@ -0,0 +1 @@ +DROP TABLE IF EXISTS refresh_tokens; \ No newline at end of file diff --git a/migrations/sql/up/add_physical_device_id.sql b/migrations/sql/up/add_physical_device_id.sql new file mode 100644 index 00000000..934cb1b3 --- /dev/null +++ b/migrations/sql/up/add_physical_device_id.sql @@ -0,0 +1,10 @@ +ALTER TABLE user_devices + ADD COLUMN physical_device_id UUID; + +UPDATE user_devices SET physical_device_id = id WHERE physical_device_id IS NULL; + +ALTER TABLE user_devices + ALTER COLUMN physical_device_id SET NOT NULL; + +CREATE UNIQUE INDEX idx_user_devices_user_physical + ON user_devices (user_id, physical_device_id); \ No newline at end of file diff --git a/migrations/sql/up/create_refresh_tokens.sql b/migrations/sql/up/create_refresh_tokens.sql new file mode 100644 index 00000000..a48d7caf --- /dev/null +++ b/migrations/sql/up/create_refresh_tokens.sql @@ -0,0 +1,13 @@ +CREATE TABLE IF NOT EXISTS refresh_tokens ( + id UUID PRIMARY KEY DEFAULT uuid_generate_v4(), + session_id UUID NOT NULL REFERENCES user_sessions(id) ON DELETE CASCADE, + family_id UUID NOT NULL, + token_hash TEXT NOT NULL, + used BOOLEAN NOT NULL DEFAULT FALSE, + created_at TIMESTAMPTZ NOT NULL DEFAULT NOW(), + used_at TIMESTAMPTZ +); + +CREATE UNIQUE INDEX idx_refresh_tokens_hash ON refresh_tokens (token_hash); +CREATE INDEX idx_refresh_tokens_family ON refresh_tokens (family_id); +CREATE INDEX idx_refresh_tokens_session ON refresh_tokens (session_id); \ No newline at end of file diff --git a/migrations/versions/2c8a676ecccf_add_physical_device_id.py b/migrations/versions/2c8a676ecccf_add_physical_device_id.py new file mode 100644 index 00000000..72976f67 --- /dev/null +++ b/migrations/versions/2c8a676ecccf_add_physical_device_id.py @@ -0,0 +1,25 @@ +"""add_physical_device_id + +Revision ID: 2c8a676ecccf +Revises: f0fa13623f6c +Create Date: 2026-07-17 02:15:43.587673 + +""" +from typing import Sequence, Union + +from migrations.helper import run_sql_down, run_sql_up + + +# revision identifiers, used by Alembic. +revision: str = '2c8a676ecccf' +down_revision: Union[str, Sequence[str], None] = 'f0fa13623f6c' +branch_labels: Union[str, Sequence[str], None] = None +depends_on: Union[str, Sequence[str], None] = None + + +def upgrade() -> None: + run_sql_up("add_physical_device_id") + + +def downgrade() -> None: + run_sql_down("add_physical_device_id") \ No newline at end of file diff --git a/migrations/versions/e49065cb125a_create_refresh_tokens.py b/migrations/versions/e49065cb125a_create_refresh_tokens.py new file mode 100644 index 00000000..4e9928e5 --- /dev/null +++ b/migrations/versions/e49065cb125a_create_refresh_tokens.py @@ -0,0 +1,25 @@ +"""create_refresh_tokens + +Revision ID: e49065cb125a +Revises: 2c8a676ecccf +Create Date: 2026-07-17 02:21:18.789969 + +""" +from typing import Sequence, Union + +from migrations.helper import run_sql_down, run_sql_up + + +# revision identifiers, used by Alembic. +revision: str = 'e49065cb125a' +down_revision: Union[str, Sequence[str], None] = '2c8a676ecccf' +branch_labels: Union[str, Sequence[str], None] = None +depends_on: Union[str, Sequence[str], None] = None + + +def upgrade() -> None: + run_sql_up("create_refresh_tokens") + + +def downgrade() -> None: + run_sql_down("create_refresh_tokens") diff --git a/tests/e2e/test_mobile_auth_intent_e2e.py b/tests/e2e/test_mobile_auth_intent_e2e.py index 9bdaa76f..f37ba8d9 100644 --- a/tests/e2e/test_mobile_auth_intent_e2e.py +++ b/tests/e2e/test_mobile_auth_intent_e2e.py @@ -29,7 +29,7 @@ def test_login_with_unknown_email_fails(self) -> None: "password": "anypassword", "device_name": "TestDevice", "device_type": "android", - "device_id": str(uuid.uuid4()), + "physical_device_id": str(uuid.uuid4()), } response = requests.post( f"{self.base_url}/user/auth/login", @@ -50,7 +50,7 @@ def test_register_with_existing_email_fails(self) -> None: "password": "ValidPass@123", "device_name": "TestDevice", "device_type": "android", - "device_id": device_id, + "physical_device_id": device_id, } response1 = requests.post( f"{self.base_url}/user/auth/register", @@ -74,7 +74,7 @@ def test_register_with_existing_email_fails(self) -> None: "otp": otp, "device_name": "TestDevice", "device_type": "android", - "device_id": device_id, + "physical_device_id": device_id, } verify_response = requests.post( f"{self.base_url}/user/auth/register/verify", @@ -104,7 +104,7 @@ def test_register_then_login_succeeds(self) -> None: "password": password, "device_name": "TestDevice", "device_type": "android", - "device_id": device_id, + "physical_device_id": device_id, } register_response = requests.post( f"{self.base_url}/user/auth/register", @@ -124,7 +124,7 @@ def test_register_then_login_succeeds(self) -> None: "otp": otp, "device_name": "TestDevice", "device_type": "android", - "device_id": device_id, + "physical_device_id": device_id, } verify_response = requests.post( f"{self.base_url}/user/auth/register/verify", @@ -141,7 +141,7 @@ def test_register_then_login_succeeds(self) -> None: "password": password, "device_name": "TestDevice", "device_type": "android", - "device_id": device_id, + "physical_device_id": device_id, } login_response = requests.post( f"{self.base_url}/user/auth/login", @@ -168,7 +168,7 @@ def test_login_with_wrong_password_fails(self) -> None: "password": password, "device_name": "TestDevice", "device_type": "android", - "device_id": device_id, + "physical_device_id": device_id, } register_response = requests.post( f"{self.base_url}/user/auth/register", @@ -187,7 +187,7 @@ def test_login_with_wrong_password_fails(self) -> None: "otp": otp, "device_name": "TestDevice", "device_type": "android", - "device_id": device_id, + "physical_device_id": device_id, } verify_response = requests.post( f"{self.base_url}/user/auth/register/verify", @@ -202,7 +202,7 @@ def test_login_with_wrong_password_fails(self) -> None: "password": "WrongPass@123", "device_name": "TestDevice", "device_type": "android", - "device_id": device_id, + "physical_device_id": device_id, } response = requests.post( f"{self.base_url}/user/auth/login", @@ -218,7 +218,7 @@ def test_register_requires_password(self) -> None: "email": "user@example.com", "device_name": "TestDevice", "device_type": "android", - "device_id": str(uuid.uuid4()), + "physical_device_id": str(uuid.uuid4()), # Missing password } response = requests.post( @@ -234,7 +234,7 @@ def test_login_requires_device_type(self) -> None: "email": "user@example.com", "password": "ValidPass@123", "device_name": "TestDevice", - "device_id": str(uuid.uuid4()), + "physical_device_id": str(uuid.uuid4()), # Missing device_type } response = requests.post( diff --git a/tests/e2e/test_mobile_auth_request_validation_e2e.py b/tests/e2e/test_mobile_auth_request_validation_e2e.py index 96c7f0e2..6852bcc3 100644 --- a/tests/e2e/test_mobile_auth_request_validation_e2e.py +++ b/tests/e2e/test_mobile_auth_request_validation_e2e.py @@ -25,7 +25,7 @@ def _valid_payload() -> dict[str, object]: "password": "ValidPass@123", "device_name": "Pixel 8", "device_type": "android", - "device_id": str(uuid.uuid4()), + "physical_device_id": str(uuid.uuid4()), } diff --git a/tests/integration/test_session_device_management.py b/tests/integration/test_session_device_management.py new file mode 100644 index 00000000..fc25fa87 --- /dev/null +++ b/tests/integration/test_session_device_management.py @@ -0,0 +1,223 @@ +""" +Integration tests for session & device management. + +These tests use a real PostgreSQL database (not fakes) specifically because +the behaviors under test depend on real SQL semantics that a fake cannot +verify: the UpsertSession ON CONFLICT clause actually matching the live +UNIQUE(user_id, device_id) constraint, and the user_sessions.device_id FK +actually being ON DELETE CASCADE. Both were previously verified by hand via +psql; these tests make that verification automatic and regression-proof. +""" +import uuid +from datetime import datetime, timedelta, timezone +from unittest.mock import AsyncMock, MagicMock + +import pytest +from sqlalchemy import text + +from app.core.securite import hash_password +from app.schema.request.mobile.auth import MobileLoginRequest +from app.service.users import AuthService +from db.generated import devices as device_queries +from db.generated import session as session_queries +from db.generated import user as user_queries + + + +# =========================================================================== +# Fixtures +# =========================================================================== + + +@pytest.fixture +async def db_conn(): + from app.core.config import settings + from sqlalchemy.ext.asyncio import create_async_engine + + url = ( + f"postgresql+asyncpg://{settings.POSTGRES_USER}:{settings.POSTGRES_PASSWORD}" + f"@{settings.POSTGRES_HOST}:{settings.POSTGRES_PORT}/{settings.POSTGRES_DB}" + ) + engine = create_async_engine(url, pool_pre_ping=True) + async with engine.connect() as conn: + yield conn + await engine.dispose() + + +@pytest.fixture +def mock_face_embedding() -> AsyncMock: + from app.service.face_embedding import FaceEmbeddingService + + svc = MagicMock(spec=FaceEmbeddingService) + return svc + + +@pytest.fixture +def auth_service(mock_face_embedding: AsyncMock, db_conn) -> AuthService: + return AuthService( + user_querier=user_queries.AsyncQuerier(db_conn), + session_querier=session_queries.AsyncQuerier(db_conn), + device_querier=device_queries.AsyncQuerier(db_conn), + face_embedding_service=mock_face_embedding, + ) + + +class _FakeRedis: + """Minimal redis stand-in — no rate-limit backing store or cache + assertions needed for these tests, just enough for the login flow + to complete without touching a real Redis instance.""" + + def __init__(self) -> None: + self._store: dict[str, int] = {} + + async def incr(self, key: str) -> int: + self._store[key] = self._store.get(key, 0) + 1 + return self._store[key] + + async def expire(self, key: str, seconds: int) -> None: + pass + + async def ttl(self, key: str) -> int: + return -1 + + async def set(self, key: str, value: str, expire: int) -> None: + return None + + async def get(self, key: str) -> str | None: + return None + + async def delete(self, key: str) -> None: + return None + + +# =========================================================================== +# Tests +# =========================================================================== + + +@pytest.mark.asyncio +async def test_relogin_on_same_device_replaces_not_duplicates_real_db( + auth_service: AuthService, + db_conn, +) -> None: + """Regression test against the real UpsertSession query: logging in twice + from the same (user, physical_device_id) must produce exactly one + session row with a stable id, not two rows — verifying the live + UNIQUE(user_id, device_id) constraint and ON CONFLICT clause actually + match, which a fake-based unit test cannot verify.""" + password = "ValidPass@123" + email = f"test-session-{uuid.uuid4()}@multai.com" + physical_device_id = uuid.uuid4() + + user = await user_queries.AsyncQuerier(db_conn).create_user( + email=email, + hashed_password=hash_password(password), + ) + assert user is not None + user_id = user.id + + try: + req = MobileLoginRequest( + email=email, + password=password, + device_name="Integration Test Device", + device_type="android", + physical_device_id=physical_device_id, + ) + + result1 = await auth_service.mobile_login(_FakeRedis(), req) + result2 = await auth_service.mobile_login(_FakeRedis(), req) + + assert result1.session_id == result2.session_id + + row = ( + await db_conn.execute( + text("SELECT COUNT(*) FROM user_sessions WHERE user_id = :uid"), + {"uid": user_id}, + ) + ).scalar() + assert row == 1 + + device_row = ( + await db_conn.execute( + text("SELECT COUNT(*) FROM user_devices WHERE user_id = :uid"), + {"uid": user_id}, + ) + ).scalar() + assert device_row == 1 + finally: + await db_conn.execute( + text("DELETE FROM user_sessions WHERE user_id = :uid"), {"uid": user_id} + ) + await db_conn.execute( + text("DELETE FROM user_devices WHERE user_id = :uid"), {"uid": user_id} + ) + await db_conn.execute(text("DELETE FROM users WHERE id = :uid"), {"uid": user_id}) + await db_conn.commit() + + +@pytest.mark.asyncio +async def test_revoke_device_cascades_delete_session_real_db( + db_conn, +) -> None: + """Regression test verifying user_sessions_device_id_fkey is genuinely + ON DELETE CASCADE: deleting a device row via the real revoke_device + query must also delete its session row, with no separate DELETE + needed. Previously verified once by hand via psql \\d user_sessions; + this makes it automatic.""" + email = f"test-revoke-{uuid.uuid4()}@multai.com" + + user_querier = user_queries.AsyncQuerier(db_conn) + device_querier = device_queries.AsyncQuerier(db_conn) + session_querier = session_queries.AsyncQuerier(db_conn) + + user = await user_querier.create_user(email=email, hashed_password="hash") + assert user is not None + user_id = user.id + + try: + device = await device_querier.create_device( + arg=device_queries.CreateDeviceParams( + column_1=None, + user_id=user_id, + device_name="Integration Test Device", + device_type="android", + totp_secret=None, + physical_device_id=uuid.uuid4(), + ) + ) + assert device is not None + + session = await session_querier.upsert_session( + user_id=user_id, + device_id=device.id, + expires_at=datetime.now(timezone.utc) + timedelta(days=1), + ) + assert session is not None + session_id = session.id + + # Confirm the session actually exists before we revoke. + pre = await session_querier.get_session_by_id(id=session_id) + assert pre is not None + + await device_querier.revoke_device(id=device.id, user_id=user_id) + + post = await session_querier.get_session_by_id(id=session_id) + assert post is None + + device_still_there = ( + await db_conn.execute( + text("SELECT COUNT(*) FROM user_devices WHERE id = :did"), + {"did": device.id}, + ) + ).scalar() + assert device_still_there == 0 + finally: + await db_conn.execute( + text("DELETE FROM user_sessions WHERE user_id = :uid"), {"uid": user_id} + ) + await db_conn.execute( + text("DELETE FROM user_devices WHERE user_id = :uid"), {"uid": user_id} + ) + await db_conn.execute(text("DELETE FROM users WHERE id = :uid"), {"uid": user_id}) + await db_conn.commit() diff --git a/tests/security/test_auth_security.py b/tests/security/test_auth_security.py index d010e704..0db5ca78 100644 --- a/tests/security/test_auth_security.py +++ b/tests/security/test_auth_security.py @@ -92,6 +92,7 @@ async def test_blocked_user_access(client): expires_at=datetime.now(timezone.utc) + timedelta(hours=1), blocked=True, ttl=3600, + last_active=datetime.now(timezone.utc) ) payload = { @@ -130,6 +131,7 @@ async def test_rate_limiting(client): expires_at=datetime.now(timezone.utc) + timedelta(hours=1), blocked=False, ttl=3600, + last_active=datetime.now(timezone.utc) ) payload = { diff --git a/tests/unit/test_auth_email_otp.py b/tests/unit/test_auth_email_otp.py index 2dcfe269..77368928 100644 --- a/tests/unit/test_auth_email_otp.py +++ b/tests/unit/test_auth_email_otp.py @@ -48,28 +48,22 @@ async def test_mobile_register_sends_otp( mock_redis: AsyncMock, mock_user_querier: AsyncMock, ) -> None: - # Arrange req = MobileRegisterRequest( email="test@example.com", password="Password1!", device_name="iPhone", device_type="iOS", - device_id=uuid.uuid4(), + physical_device_id=uuid.uuid4(), ) - mock_user_querier.get_user_by_email.return_value = None # User does not exist - mock_redis.incr.return_value = 1 # Rate limit check passes + mock_user_querier.get_user_by_email.return_value = None + mock_redis.incr.return_value = 1 - # Act res = await auth_service.mobile_register(redis=mock_redis, req=req) - # Assert assert res.status == "pending_verification" assert res.email == "test@example.com" - - # Verify redis was called to save pending user and OTP assert mock_redis.set.call_count == 2 - # Verify NATS publish was called mock_publish.assert_called_once() args, _ = mock_publish.call_args assert args[0] == "email.send_otp" @@ -85,20 +79,19 @@ async def test_verify_mobile_register_success( mock_device_querier: AsyncMock, mock_session_querier: AsyncMock, ) -> None: - # Arrange device_id = uuid.uuid4() req = RegisterVerifyRequest( email="test@example.com", password="Password1!", device_name="iPhone", device_type="iOS", - device_id=device_id, + physical_device_id=device_id, otp="123456" ) mock_redis.get.side_effect = [ - "123456", # First call gets OTP - json.dumps({"hashed_password": "hashed_pass"}) # Second call gets pending user + "123456", + json.dumps({"hashed_password": "hashed_pass"}) ] mock_user = AsyncMock() @@ -111,60 +104,49 @@ async def test_verify_mobile_register_success( mock_session = AsyncMock() mock_session.id = uuid.uuid4() mock_session_querier.upsert_session.return_value = mock_session - mock_device_querier.get_device_by_id.return_value = None - mock_device_querier.get_device_by_id_any.return_value = None - # Act + # FIX 1: Mock the new lookup method to return None (device not found) + mock_device_querier.get_device_by_physical_id.return_value = None + # FIX 2: Mock create_device to return a truthy device + mock_device_querier.create_device.return_value = AsyncMock() + with patch("app.service.users.SessionService.cache_session_for_auth", new_callable=AsyncMock): res = await auth_service.verify_mobile_register(redis=mock_redis, req=req) - # Assert assert res.is_new_user is True assert res.user_id == mock_user.id - - # Verify user was created mock_user_querier.create_user.assert_called_once_with(email="test@example.com", hashed_password="hashed_pass") - - # Verify redis cleanup assert mock_redis.delete.call_count == 2 - @pytest.mark.asyncio async def test_mobile_register_resend_otp_success( auth_service: AuthService, mock_redis: AsyncMock, ) -> None: - # Arrange email = "test@example.com" mock_redis.get.return_value = '{"hashed_password": "fake"}' mock_redis.incr.return_value = 1 - # Act with patch("app.service.users.NatsClient.publish", new_callable=AsyncMock) as mock_publish: res = await auth_service.mobile_register_resend_otp(redis=mock_redis, email=email) - # Assert assert res.status == "pending_verification" assert res.message == "New OTP sent to email" assert res.email == email - mock_redis.get.assert_called_with(f"pending_user:{email}") mock_redis.set.assert_called_with(f"otp:{email}", ANY, expire=600) mock_publish.assert_called_once() - @pytest.mark.asyncio async def test_mobile_register_resend_otp_not_found( auth_service: AuthService, mock_redis: AsyncMock, ) -> None: from fastapi import HTTPException - # Arrange email = "test@example.com" mock_redis.incr.return_value = 1 - mock_redis.get.return_value = None # No pending user + mock_redis.get.return_value = None - # Act & Assert with pytest.raises(HTTPException) as exc_info: await auth_service.mobile_register_resend_otp(redis=mock_redis, email=email) diff --git a/tests/unit/test_auth_service.py b/tests/unit/test_auth_service.py index 91b306f2..5fd342f4 100644 --- a/tests/unit/test_auth_service.py +++ b/tests/unit/test_auth_service.py @@ -17,6 +17,9 @@ from app.core.securite import hash_password from app.schema.request.mobile.auth import MobileLoginRequest, MobileRegisterRequest +async def _empty_async_iter(): + return + yield # pragma: no cover # --------------------------------------------------------------------------- # Factories @@ -73,7 +76,7 @@ def _make_login_request( return MobileLoginRequest( email=email, password=password, - device_id=uuid.uuid4(), + physical_device_id=uuid.uuid4(), # was: device_id device_name="iPhone 15", device_type="ios", ) @@ -87,12 +90,11 @@ def _make_register_request( return MobileRegisterRequest( email=email, password=password, - device_id=uuid.uuid4(), + physical_device_id=uuid.uuid4(), # was: device_id device_name="iPhone 15", device_type="ios", ) - # --------------------------------------------------------------------------- # Fixtures # --------------------------------------------------------------------------- @@ -116,6 +118,7 @@ def device_querier() -> AsyncMock: q = MagicMock(spec=device_queries.AsyncQuerier) q.get_device_by_id = AsyncMock(return_value=None) q.get_device_by_id_any = AsyncMock(return_value=None) + q.get_device_by_physical_id = AsyncMock(return_value=None) # new q.create_device = AsyncMock(return_value=_make_device()) q.activate_device = AsyncMock() return q @@ -126,6 +129,9 @@ def session_querier() -> AsyncMock: from db.generated import session as session_queries q = MagicMock(spec=session_queries.AsyncQuerier) q.count_user_sessions = AsyncMock(return_value=0) + q.get_session_by_device_for_user = AsyncMock(return_value=None) + q.list_sessions_by_user = MagicMock(return_value=_empty_async_iter()) + q.delete_session_by_id = AsyncMock() q.upsert_session = AsyncMock(return_value=_make_session()) q.get_session_by_id = AsyncMock() return q @@ -280,7 +286,7 @@ async def test_blocked_user_raises_403( class TestSessionLimit: @pytest.mark.asyncio - async def test_exceeding_session_limit_raises_403( + async def test_at_cap_evicts_oldest_and_succeeds( self, auth_service: AuthService, user_querier: AsyncMock, @@ -289,13 +295,30 @@ async def test_exceeding_session_limit_raises_403( ) -> None: user = _make_user() user_querier.get_user_by_email.return_value = user - # Return a count >= SESSION_LIMIT - session_querier.count_user_sessions.return_value = AuthService.SESSION_LIMIT - with pytest.raises(HTTPException) as exc_info: - await auth_service.mobile_login(redis, _make_login_request()) - assert exc_info.value.status_code == 403 - assert "session limit" in exc_info.value.detail.lower() + oldest = _make_session(user_id=user.id) + middle = _make_session(user_id=user.id) + newest = _make_session(user_id=user.id) + + # Stagger so oldest is clearly the minimum + base = datetime.now(timezone.utc) + oldest.last_active = base - timedelta(seconds=2) + middle.last_active = base - timedelta(seconds=1) + newest.last_active = base + + async def _sessions(*, user_id): + yield oldest + yield middle + yield newest + + session_querier.list_sessions_by_user = _sessions + + result = await auth_service.mobile_login(redis, _make_login_request()) + + assert result.access_token + session_querier.delete_session_by_id.assert_called_once() + called_id = session_querier.delete_session_by_id.call_args.kwargs.get("id") + assert called_id == oldest.id @pytest.mark.asyncio async def test_within_session_limit_succeeds( @@ -307,10 +330,11 @@ async def test_within_session_limit_succeeds( ) -> None: user = _make_user() user_querier.get_user_by_email.return_value = user - session_querier.count_user_sessions.return_value = AuthService.SESSION_LIMIT - 1 + session_querier.list_sessions_by_user = MagicMock(return_value=_empty_async_iter()) result = await auth_service.mobile_login(redis, _make_login_request()) assert result.access_token + session_querier.delete_session_by_id.assert_not_called() # =========================================================================== diff --git a/tests/unit/test_enroll_security.py b/tests/unit/test_enroll_security.py index 4f31584d..d944cb03 100644 --- a/tests/unit/test_enroll_security.py +++ b/tests/unit/test_enroll_security.py @@ -1,8 +1,5 @@ """ -Unit tests for the enrollment security helpers introduced in fix/enroll. - -These helpers only exist on the fix/enroll branch. On other branches, -all tests are automatically skipped via pytest.importorskip. +Unit tests for image validation helpers in app.core.image_validation. Run with: uv run pytest tests/unit/test_enroll_security.py -v """ @@ -15,27 +12,16 @@ from fastapi import UploadFile from fastapi.exceptions import HTTPException -# Guard: skip gracefully if the helpers don't exist on this branch -_router_mod = pytest.importorskip( - "app.router.mobile.enrollement", - reason="fix/enroll branch required for enrollment security helpers", +from app.core.constant import MAX_IMAGE_SIZE, MIN_IMAGE_DIM, MAX_IMAGE_DIM # noqa: E402 +from app.core.image_validation import ( + sanitise_filename, + validate_dimensions, + precheck_upload_headers, + read_limited, ) -_sanitise_filename = getattr(_router_mod, "_sanitise_filename", None) -_validate_dimensions = getattr(_router_mod, "_validate_dimensions", None) -_precheck_upload_headers = getattr(_router_mod, "_precheck_upload_headers", None) -_enrollment_lock_key = getattr(_router_mod, "_enrollment_lock_key", None) -read_limited = getattr(_router_mod, "read_limited", None) - -# If any helper is missing, skip the whole module -if any(fn is None for fn in [_sanitise_filename, _validate_dimensions, - _precheck_upload_headers, _enrollment_lock_key, read_limited]): - pytest.skip( - "Enrollment security helpers not available on this branch", - allow_module_level=True, - ) - -from app.core.constant import MAX_IMAGE_SIZE, MIN_IMAGE_DIM, MAX_IMAGE_DIM # noqa: E402 +# _enrollment_lock_key does not exist in the refactored module — skip those tests +_enrollment_lock_key = None # --------------------------------------------------------------------------- @@ -74,133 +60,134 @@ def _make_upload_file( # =========================================================================== -# 1. _sanitise_filename +# 1. sanitise_filename # =========================================================================== class TestSanitiseFilename: def test_normal_name_gets_uuid_prefix(self) -> None: - result = _sanitise_filename("portrait.jpg", "jpg") + result = sanitise_filename("portrait.jpg", "jpg") parts = result.split("_", 1) assert len(parts) == 2 uuid.UUID(parts[0]) # raises if not a valid UUID assert parts[1] == "portrait.jpg" def test_path_traversal_is_neutralised(self) -> None: - result = _sanitise_filename("../../../etc/passwd.jpg", "jpg") + result = sanitise_filename("../../../etc/passwd.jpg", "jpg") assert ".." not in result assert "/" not in result + assert "\\" not in result def test_null_bytes_are_replaced(self) -> None: - assert "\x00" not in _sanitise_filename("face\x00evil.jpg", "jpg") + assert "\x00" not in sanitise_filename("face\x00evil.jpg", "jpg") def test_control_characters_are_replaced(self) -> None: - assert "\x1f" not in _sanitise_filename("face\x1fmalicious.jpg", "jpg") + assert "\x1f" not in sanitise_filename("face\x1fmalicious.jpg", "jpg") def test_windows_reserved_chars_are_replaced(self) -> None: - for char in r'\/:*?"<>|': - assert char not in _sanitise_filename(f"face{char}name.jpg", "jpg"), \ + for char in r'\\/:*?"<>|': + assert char not in sanitise_filename(f"face{char}name.jpg", "jpg"), \ f"char {char!r} must be replaced" def test_none_filename_returns_uuid_only(self) -> None: - result = _sanitise_filename(None, "png") + result = sanitise_filename(None, "png") base, ext = result.rsplit(".", 1) uuid.UUID(base) assert ext == "png" def test_empty_filename_returns_uuid_only(self) -> None: - result = _sanitise_filename("", "jpg") + result = sanitise_filename("", "jpg") base, ext = result.rsplit(".", 1) uuid.UUID(base) assert ext == "jpg" def test_long_filename_is_truncated(self) -> None: - result = _sanitise_filename("a" * 200, "jpg") + result = sanitise_filename("a" * 200, "jpg") name_part = result.split("_", 1)[1] assert len(name_part) <= 128 def test_leading_dots_stripped(self) -> None: - result = _sanitise_filename("...hidden.jpg", "jpg") + result = sanitise_filename("...hidden.jpg", "jpg") name_part = result.split("_", 1)[1] assert not name_part.startswith(".") # =========================================================================== -# 2. _validate_dimensions +# 2. validate_dimensions # =========================================================================== class TestValidateDimensions: def test_valid_image_passes(self) -> None: - _validate_dimensions(_make_jpeg_bytes(200, 200)) # must not raise + validate_dimensions(_make_jpeg_bytes(200, 200)) # must not raise def test_too_small_width_raises_400(self) -> None: with pytest.raises(HTTPException) as exc_info: - _validate_dimensions(_make_jpeg_bytes(MIN_IMAGE_DIM - 1, 200)) + validate_dimensions(_make_jpeg_bytes(MIN_IMAGE_DIM - 1, 200)) assert exc_info.value.status_code == 400 assert "too small" in exc_info.value.detail.lower() def test_too_small_height_raises_400(self) -> None: with pytest.raises(HTTPException) as exc_info: - _validate_dimensions(_make_jpeg_bytes(200, MIN_IMAGE_DIM - 1)) + validate_dimensions(_make_jpeg_bytes(200, MIN_IMAGE_DIM - 1)) assert exc_info.value.status_code == 400 def test_too_large_width_raises_400(self) -> None: with pytest.raises(HTTPException) as exc_info: - _validate_dimensions(_make_jpeg_bytes(MAX_IMAGE_DIM + 1, 200)) + validate_dimensions(_make_jpeg_bytes(MAX_IMAGE_DIM + 1, 200)) assert exc_info.value.status_code == 400 assert "too large" in exc_info.value.detail.lower() def test_corrupt_bytes_raises_400(self) -> None: with pytest.raises(HTTPException) as exc_info: - _validate_dimensions(b"this-is-not-an-image") + validate_dimensions(b"this-is-not-an-image") assert exc_info.value.status_code == 400 def test_boundary_min_dimension_passes(self) -> None: - _validate_dimensions(_make_jpeg_bytes(MIN_IMAGE_DIM, MIN_IMAGE_DIM)) + validate_dimensions(_make_jpeg_bytes(MIN_IMAGE_DIM, MIN_IMAGE_DIM)) def test_boundary_max_dimension_passes(self) -> None: - _validate_dimensions(_make_jpeg_bytes(MAX_IMAGE_DIM, MAX_IMAGE_DIM)) + validate_dimensions(_make_jpeg_bytes(MAX_IMAGE_DIM, MAX_IMAGE_DIM)) # =========================================================================== -# 3. _precheck_upload_headers +# 3. precheck_upload_headers # =========================================================================== class TestPrecheckUploadHeaders: def test_valid_jpeg_header_passes(self) -> None: - _precheck_upload_headers(_make_upload_file(b"", content_type="image/jpeg")) + precheck_upload_headers(_make_upload_file(b"", content_type="image/jpeg")) def test_valid_png_header_passes(self) -> None: - _precheck_upload_headers(_make_upload_file(b"", content_type="image/png")) + precheck_upload_headers(_make_upload_file(b"", content_type="image/png")) def test_missing_content_type_raises_400(self) -> None: f = _make_upload_file(b"", content_type=None) with pytest.raises(HTTPException) as exc_info: - _precheck_upload_headers(f) + precheck_upload_headers(f) assert exc_info.value.status_code == 400 def test_unsupported_content_type_raises_400(self) -> None: with pytest.raises(HTTPException) as exc_info: - _precheck_upload_headers(_make_upload_file(b"", content_type="application/pdf")) + precheck_upload_headers(_make_upload_file(b"", content_type="application/pdf")) assert exc_info.value.status_code == 400 def test_content_type_with_charset_param_accepted(self) -> None: # "image/jpeg; charset=utf-8" should normalise to "image/jpeg" - _precheck_upload_headers( + precheck_upload_headers( _make_upload_file(b"", content_type="image/jpeg; charset=utf-8") ) def test_oversized_content_length_raises_400(self) -> None: with pytest.raises(HTTPException) as exc_info: - _precheck_upload_headers( + precheck_upload_headers( _make_upload_file(b"", content_type="image/jpeg", content_length=MAX_IMAGE_SIZE + 1) ) assert exc_info.value.status_code == 400 def test_valid_content_length_passes(self) -> None: - _precheck_upload_headers( + precheck_upload_headers( _make_upload_file(b"", content_type="image/jpeg", content_length=1024) ) @@ -208,7 +195,7 @@ def test_invalid_content_length_string_raises_400(self) -> None: f = _make_upload_file(b"", content_type="image/jpeg") f.headers = {"content-length": "not_a_number"} with pytest.raises(HTTPException) as exc_info: - _precheck_upload_headers(f) + precheck_upload_headers(f) assert exc_info.value.status_code == 400 @@ -267,23 +254,12 @@ async def mock_read(n: int = -1) -> bytes: # =========================================================================== -# 5. _enrollment_lock_key +# 5. _enrollment_lock_key — REMOVED +# This helper does not exist in app.core.image_validation. +# If it exists elsewhere, add a separate test file for it. # =========================================================================== -class TestEnrollmentLockKey: - def test_key_contains_user_id(self) -> None: - user_id = uuid.uuid4() - assert str(user_id) in _enrollment_lock_key(user_id) - - def test_different_users_have_different_keys(self) -> None: - assert _enrollment_lock_key(uuid.uuid4()) != _enrollment_lock_key(uuid.uuid4()) - - def test_key_format(self) -> None: - user_id = uuid.uuid4() - assert _enrollment_lock_key(user_id) == f"enroll:in_progress:{user_id}" - - # =========================================================================== # 6. Magic byte sniffing (unit-level verification) # =========================================================================== diff --git a/tests/unit/test_mobile_auth_email_logging.py b/tests/unit/test_mobile_auth_email_logging.py index 8180cfa7..3a592813 100644 --- a/tests/unit/test_mobile_auth_email_logging.py +++ b/tests/unit/test_mobile_auth_email_logging.py @@ -1,4 +1,3 @@ - # Test doubles intentionally implement only the AuthService methods exercised here. # They do not subclass the generated queriers, so mypy would otherwise flag each # constructor injection as an arg-type mismatch. @@ -26,8 +25,10 @@ def __init__(self, email: str) -> None: class FakeDevice: - is_invalid_token = False - is_active = True + def __init__(self) -> None: + self.id = uuid.uuid4() + self.is_invalid_token = False + self.is_active = True class FakeSession: @@ -50,6 +51,11 @@ async def create_user(self, *, email: str, hashed_password: str) -> FakeUser: class FakeDeviceQuerier: + async def get_device_by_physical_id( + self, *, user_id: uuid.UUID, physical_device_id: uuid.UUID + ) -> FakeDevice | None: + return None + async def get_device_by_id_any(self, id: uuid.UUID) -> FakeDevice | None: return None @@ -67,6 +73,11 @@ def __init__(self, session: FakeSession) -> None: async def count_user_sessions(self, user_id: uuid.UUID) -> int: return 0 + async def get_session_by_device_for_user( + self, *, device_id: uuid.UUID, user_id: uuid.UUID + ) -> FakeSession | None: + return None + async def upsert_session( self, *, @@ -95,6 +106,7 @@ async def ttl(self, key: str) -> int: async def set(self, key: str, value: str, expire: int) -> None: return None + class FakeFaceEmbeddingService: pass @@ -120,7 +132,7 @@ def test_mobile_register_logs_without_plaintext_email( password="ValidPass@123", device_name="Pixel 8", device_type="android", - device_id=uuid.uuid4(), + physical_device_id=uuid.uuid4(), ) async def _noop_cache_session_for_auth(**_: object) -> None: @@ -137,4 +149,3 @@ async def _noop_cache_session_for_auth(**_: object) -> None: assert req.email not in caplog.text assert "user@example.com" not in caplog.text assert "mobile_register attempt" in caplog.text - diff --git a/tests/unit/test_mobile_auth_intent_validation.py b/tests/unit/test_mobile_auth_intent_validation.py index 574b5aa8..99bc07a7 100644 --- a/tests/unit/test_mobile_auth_intent_validation.py +++ b/tests/unit/test_mobile_auth_intent_validation.py @@ -1,3 +1,4 @@ +from collections.abc import AsyncIterator from typing import Any # Test doubles intentionally implement only the AuthService methods exercised here. @@ -6,6 +7,7 @@ # mypy: disable-error-code=arg-type import asyncio +import json import logging import uuid from datetime import datetime, timezone @@ -15,7 +17,7 @@ import app.service.users as users_module from app.core.securite import hash_password -from app.schema.request.mobile.auth import MobileLoginRequest, MobileRegisterRequest +from app.schema.request.mobile.auth import MobileLoginRequest, MobileRegisterRequest, RegisterVerifyRequest from app.service.session import SessionService from app.service.users import AuthService @@ -30,14 +32,22 @@ def __init__(self, email: str, exists: bool = True, password: str = "ValidPass@1 class FakeDevice: - is_invalid_token = False - is_active = True + def __init__(self, physical_device_id: uuid.UUID, user_id: uuid.UUID) -> None: + self.id = uuid.uuid4() + self.physical_device_id = physical_device_id + self.user_id = user_id + self.is_invalid_token = False + self.is_active = True class FakeSession: - def __init__(self) -> None: + def __init__(self, user_id: uuid.UUID, device_id: uuid.UUID) -> None: self.id = uuid.uuid4() + self.user_id = user_id + self.device_id = device_id self.expires_at = datetime.now(timezone.utc) + self.last_active = datetime.now(timezone.utc) + self.created_at = datetime.now(timezone.utc) class FakeUserQuerier: @@ -65,28 +75,72 @@ async def create_user(self, *, email: str, hashed_password: str) -> FakeUser: class FakeDeviceQuerier: + """Stateful fake — tracks devices keyed by (user_id, physical_device_id), + matching the real UNIQUE(user_id, physical_device_id) constraint.""" + + def __init__(self) -> None: + self._devices: dict[tuple[uuid.UUID, uuid.UUID], FakeDevice] = {} + + async def get_device_by_physical_id( + self, *, user_id: uuid.UUID, physical_device_id: uuid.UUID + ) -> FakeDevice | None: + return self._devices.get((user_id, physical_device_id)) + async def get_device_by_id_any(self, id: uuid.UUID) -> FakeDevice | None: + # Kept only because the generated querier exposes it; application code + # no longer calls this for auth decisions. return None - async def get_device_by_id(self, id: uuid.UUID) -> FakeDevice | None: + async def get_device_by_id(self, id: uuid.UUID, user_id: uuid.UUID) -> FakeDevice | None: + for device in self._devices.values(): + if device.id == id and device.user_id == user_id: + return device return None - async def create_device(self, arg: object) -> FakeDevice: - return FakeDevice() + async def create_device(self, arg: Any) -> FakeDevice: + device = FakeDevice(physical_device_id=arg.physical_device_id, user_id=arg.user_id) + self._devices[(arg.user_id, arg.physical_device_id)] = device + return device async def activate_device(self, id: uuid.UUID, user_id: uuid.UUID) -> None: return None class FakeSessionQuerier: - def __init__(self, session: FakeSession) -> None: - self._session = session + """Stateful fake — tracks sessions keyed by (user_id, device_id), matching + the real UNIQUE(user_id, device_id) constraint and upsert-on-conflict + behavior.""" + + def __init__(self) -> None: + self._sessions: dict[tuple[uuid.UUID, uuid.UUID], FakeSession] = {} async def count_user_sessions(self, user_id: uuid.UUID) -> int: - return 0 + return sum(1 for (u, _d) in self._sessions if u == user_id) + + async def get_session_by_device_for_user( + self, *, device_id: uuid.UUID, user_id: uuid.UUID + ) -> FakeSession | None: + return self._sessions.get((user_id, device_id)) async def get_session_by_id(self, id: uuid.UUID) -> FakeSession | None: - return self._session + for session in self._sessions.values(): + if session.id == id: + return session + return None + + async def list_sessions_by_user(self, user_id: uuid.UUID) -> AsyncIterator[FakeSession]: + for (u, _d), session in self._sessions.items(): + if u == user_id: + yield session + + async def delete_session_by_id(self, *, id: uuid.UUID, user_id: uuid.UUID) -> None: + key_to_remove = None + for key, session in self._sessions.items(): + if session.id == id and session.user_id == user_id: + key_to_remove = key + break + if key_to_remove: + del self._sessions[key_to_remove] async def upsert_session( self, @@ -95,17 +149,25 @@ async def upsert_session( device_id: uuid.UUID, expires_at: datetime, ) -> FakeSession: - self._session.expires_at = expires_at - return self._session - + key = (user_id, device_id) + existing = self._sessions.get(key) + if existing: + existing.expires_at = expires_at + existing.last_active = datetime.now(timezone.utc) + return existing + session = FakeSession(user_id=user_id, device_id=device_id) + session.expires_at = expires_at + self._sessions[key] = session + return session class FakeRedis: def __init__(self) -> None: - self._store: dict[str, int] = {} + self._store: dict[str, str] = {} async def incr(self, key: str) -> int: - self._store[key] = self._store.get(key, 0) + 1 - return self._store[key] + current = int(self._store.get(key, "0")) + 1 + self._store[key] = str(current) + return current async def expire(self, key: str, seconds: int) -> None: pass @@ -114,7 +176,10 @@ async def ttl(self, key: str) -> int: return -1 async def set(self, key: str, value: str, expire: int) -> None: - return None + self._store[key] = value + + async def get(self, key: str) -> str | None: + return self._store.get(key) async def delete(self, key: str) -> None: self._store.pop(key, None) @@ -124,14 +189,23 @@ class FakeFaceEmbeddingService: pass +def _patch_token_helpers(monkeypatch: pytest.MonkeyPatch) -> None: + async def _noop_cache_session_for_auth(**_: object) -> None: + return None + + monkeypatch.setattr(SessionService, "cache_session_for_auth", _noop_cache_session_for_auth) + monkeypatch.setattr(users_module, "create_acces_mobile_token", lambda _: "access") + monkeypatch.setattr(users_module, "create_refresh_mobile_token", lambda _: "refresh") + monkeypatch.setattr(users_module, "Get_expiry_time", lambda: 3600) + + def test_login_with_unknown_email_is_rejected() -> None: """Test that login with unknown email fails.""" user = FakeUser(email="user@example.com", exists=True) - session = FakeSession() service = AuthService( user_querier=FakeUserQuerier(user), device_querier=FakeDeviceQuerier(), - session_querier=FakeSessionQuerier(session), + session_querier=FakeSessionQuerier(), face_embedding_service=FakeFaceEmbeddingService(), ) @@ -140,7 +214,7 @@ def test_login_with_unknown_email_is_rejected() -> None: password="ValidPass@123", device_name="Pixel 8", device_type="android", - device_id=uuid.uuid4(), + physical_device_id=uuid.uuid4(), ) with pytest.raises(HTTPException) as exc_info: @@ -152,11 +226,10 @@ def test_login_with_unknown_email_is_rejected() -> None: def test_register_with_existing_email_is_rejected() -> None: """Test that registration with existing email fails.""" user = FakeUser(email="user@example.com", exists=True) - session = FakeSession() service = AuthService( user_querier=FakeUserQuerier(user), device_querier=FakeDeviceQuerier(), - session_querier=FakeSessionQuerier(session), + session_querier=FakeSessionQuerier(), face_embedding_service=FakeFaceEmbeddingService(), ) @@ -165,7 +238,7 @@ def test_register_with_existing_email_is_rejected() -> None: password="ValidPass@123", device_name="Pixel 8", device_type="android", - device_id=uuid.uuid4(), + physical_device_id=uuid.uuid4(), ) with pytest.raises(HTTPException) as exc_info: @@ -179,11 +252,10 @@ def test_login_with_correct_credentials_succeeds( ) -> None: """Test that login with correct credentials succeeds.""" user = FakeUser(email="user@example.com", exists=True) - session = FakeSession() service = AuthService( user_querier=FakeUserQuerier(user), device_querier=FakeDeviceQuerier(), - session_querier=FakeSessionQuerier(session), + session_querier=FakeSessionQuerier(), face_embedding_service=FakeFaceEmbeddingService(), ) @@ -192,16 +264,10 @@ def test_login_with_correct_credentials_succeeds( password="ValidPass@123", device_name="Pixel 8", device_type="android", - device_id=uuid.uuid4(), + physical_device_id=uuid.uuid4(), ) - async def _noop_cache_session_for_auth(**_: object) -> None: - return None - - monkeypatch.setattr(SessionService, "cache_session_for_auth", _noop_cache_session_for_auth) - monkeypatch.setattr(users_module, "create_acces_mobile_token", lambda _: "access") - monkeypatch.setattr(users_module, "create_refresh_mobile_token", lambda _: "refresh") - monkeypatch.setattr(users_module, "Get_expiry_time", lambda: 3600) + _patch_token_helpers(monkeypatch) result = asyncio.run(service.mobile_login(FakeRedis(), req)) assert result.access_token == "access" @@ -214,11 +280,10 @@ def test_register_with_new_email_succeeds( ) -> None: """Test that registration with new email succeeds.""" user = FakeUser(email="user@example.com", exists=False) - session = FakeSession() service = AuthService( user_querier=FakeUserQuerier(user), device_querier=FakeDeviceQuerier(), - session_querier=FakeSessionQuerier(session), + session_querier=FakeSessionQuerier(), face_embedding_service=FakeFaceEmbeddingService(), ) @@ -227,16 +292,10 @@ def test_register_with_new_email_succeeds( password="ValidPass@123", device_name="Pixel 8", device_type="android", - device_id=uuid.uuid4(), + physical_device_id=uuid.uuid4(), ) - async def _noop_cache_session_for_auth(**_: object) -> None: - return None - - monkeypatch.setattr(SessionService, "cache_session_for_auth", _noop_cache_session_for_auth) - monkeypatch.setattr(users_module, "create_acces_mobile_token", lambda _: "access") - monkeypatch.setattr(users_module, "create_refresh_mobile_token", lambda _: "refresh") - monkeypatch.setattr(users_module, "Get_expiry_time", lambda: 3600) + _patch_token_helpers(monkeypatch) result = asyncio.run(service.mobile_register(FakeRedis(), req)) assert result.status == "pending_verification" @@ -247,73 +306,51 @@ def test_register_then_login_same_device_succeeds( ) -> None: """Test full flow: register then login with same device.""" user = FakeUser(email="newuser@example.com", exists=False) - session = FakeSession() service = AuthService( user_querier=FakeUserQuerier(user), device_querier=FakeDeviceQuerier(), - session_querier=FakeSessionQuerier(session), + session_querier=FakeSessionQuerier(), face_embedding_service=FakeFaceEmbeddingService(), ) - async def _noop_cache_session_for_auth(**_: object) -> None: - return None + _patch_token_helpers(monkeypatch) - monkeypatch.setattr(SessionService, "cache_session_for_auth", _noop_cache_session_for_auth) - monkeypatch.setattr(users_module, "create_acces_mobile_token", lambda _: "access") - monkeypatch.setattr(users_module, "create_refresh_mobile_token", lambda _: "refresh") - monkeypatch.setattr(users_module, "Get_expiry_time", lambda: 3600) - - device_id = uuid.uuid4() + physical_device_id = uuid.uuid4() password = "ValidPass@123" - # Register register_req = MobileRegisterRequest( email="newuser@example.com", password=password, device_name="TestDevice", device_type="android", - device_id=device_id, + physical_device_id=physical_device_id, ) fake_redis = FakeRedis() result1 = asyncio.run(service.mobile_register(fake_redis, register_req)) assert result1.status == "pending_verification" - # Try to register again (should just resend OTP, not fail with 409 because user is not yet fully enrolled) - # Actually, we expect 200 with pending_verification again if they try to register while pending, OR it might - # just succeed. Wait, the actual flow drops it in Redis and returns pending. So it won't raise 409 unless - # the user is IN THE DB. Since FakeUserQuerier won't have it, it won't raise 409. - # We will just verify OTP instead. - - # Now verify to fully create user - from app.schema.request.mobile.auth import RegisterVerifyRequest verify_req = RegisterVerifyRequest( email="newuser@example.com", password=password, otp="123456", device_name="TestDevice", device_type="android", - device_id=device_id, + physical_device_id=physical_device_id, ) - # Fake Redis returning the raw data and otp - import json fake_redis._store["otp:newuser@example.com"] = "123456" - fake_redis._store["pending_user:newuser@example.com"] = json.dumps({"hashed_password": hash_password(password)}) - - # We have to stub get() on FakeRedis since it doesn't support it by default - async def fake_get(key: str) -> str | None: - return fake_redis._store.get(key) - fake_redis.get = fake_get # type: ignore + fake_redis._store["pending_user:newuser@example.com"] = json.dumps( + {"hashed_password": hash_password(password)} + ) verify_result = asyncio.run(service.verify_mobile_register(fake_redis, verify_req)) assert verify_result.is_new_user is True - # Now login login_req = MobileLoginRequest( email="newuser@example.com", password=password, device_name="TestDevice", device_type="android", - device_id=device_id, + physical_device_id=physical_device_id, ) result2 = asyncio.run(service.mobile_login(FakeRedis(), login_req)) assert result2.is_new_user is False @@ -322,11 +359,10 @@ async def fake_get(key: str) -> str | None: def test_login_with_wrong_password_fails() -> None: """Test that login with wrong password fails.""" user = FakeUser(email="user@example.com", exists=True) - session = FakeSession() service = AuthService( user_querier=FakeUserQuerier(user), device_querier=FakeDeviceQuerier(), - session_querier=FakeSessionQuerier(session), + session_querier=FakeSessionQuerier(), face_embedding_service=FakeFaceEmbeddingService(), ) @@ -335,7 +371,7 @@ def test_login_with_wrong_password_fails() -> None: password="wrongpassword", device_name="Pixel 8", device_type="android", - device_id=uuid.uuid4(), + physical_device_id=uuid.uuid4(), ) with pytest.raises(HTTPException) as exc_info: @@ -352,11 +388,10 @@ def test_login_logs_correctly( caplog.set_level(logging.INFO, logger="multAI") user = FakeUser(email="user@example.com", exists=True) - session = FakeSession() service = AuthService( user_querier=FakeUserQuerier(user), device_querier=FakeDeviceQuerier(), - session_querier=FakeSessionQuerier(session), + session_querier=FakeSessionQuerier(), face_embedding_service=FakeFaceEmbeddingService(), ) @@ -365,16 +400,10 @@ def test_login_logs_correctly( password="ValidPass@123", device_name="Pixel 8", device_type="android", - device_id=uuid.uuid4(), + physical_device_id=uuid.uuid4(), ) - async def _noop_cache_session_for_auth(**_: object) -> None: - return None - - monkeypatch.setattr(SessionService, "cache_session_for_auth", _noop_cache_session_for_auth) - monkeypatch.setattr(users_module, "create_acces_mobile_token", lambda _: "access") - monkeypatch.setattr(users_module, "create_refresh_mobile_token", lambda _: "refresh") - monkeypatch.setattr(users_module, "Get_expiry_time", lambda: 3600) + _patch_token_helpers(monkeypatch) asyncio.run(service.mobile_login(FakeRedis(), req)) @@ -393,7 +422,6 @@ class FakeOrigException(Exception): constraint_name = "idx_users_email" user = FakeUser(email="user@example.com", exists=False) - session = FakeSession() async def _raise_integrity_error(*args: Any, **kwargs: Any) -> Any: raise IntegrityError( @@ -403,35 +431,29 @@ async def _raise_integrity_error(*args: Any, **kwargs: Any) -> Any: ) user_querier = FakeUserQuerier(user) - # Stub create_user to raise IntegrityError user_querier.create_user = _raise_integrity_error # type: ignore service = AuthService( user_querier=user_querier, device_querier=FakeDeviceQuerier(), - session_querier=FakeSessionQuerier(session), + session_querier=FakeSessionQuerier(), face_embedding_service=FakeFaceEmbeddingService(), ) - # Stub create_user to raise IntegrityError during VERIFY, because mobile_register - # no longer calls create_user directly! - from app.schema.request.mobile.auth import RegisterVerifyRequest verify_req = RegisterVerifyRequest( email="newuser@example.com", password="ValidPass@123", otp="123456", device_name="Pixel 8", device_type="android", - device_id=uuid.uuid4(), + physical_device_id=uuid.uuid4(), ) fake_redis = FakeRedis() - import json fake_redis._store["otp:newuser@example.com"] = "123456" - fake_redis._store["pending_user:newuser@example.com"] = json.dumps({"hashed_password": hash_password("ValidPass@123")}) - async def fake_get(key: str) -> str | None: - return fake_redis._store.get(key) - fake_redis.get = fake_get # type: ignore + fake_redis._store["pending_user:newuser@example.com"] = json.dumps( + {"hashed_password": hash_password("ValidPass@123")} + ) with pytest.raises(HTTPException) as exc_info: asyncio.run(service.verify_mobile_register(fake_redis, verify_req)) @@ -439,3 +461,137 @@ async def fake_get(key: str) -> str | None: assert exc_info.value.status_code == 409 assert "already in use" in exc_info.value.detail.lower() + +# =========================================================================== +# Regression tests — Phase 2 bug fixes +# =========================================================================== + + +def test_session_device_id_matches_surrogate_pk_not_physical_id( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Regression test for the FK bug: user_sessions.device_id must be the + device row's surrogate id, never the client-supplied physical_device_id + directly. Pre-migration these happened to be equal by construction; + post-migration they are unrelated UUIDs.""" + user = FakeUser(email="user@example.com", exists=True) + device_querier = FakeDeviceQuerier() + session_querier = FakeSessionQuerier() + service = AuthService( + user_querier=FakeUserQuerier(user), + device_querier=device_querier, + session_querier=session_querier, + face_embedding_service=FakeFaceEmbeddingService(), + ) + + physical_id = uuid.uuid4() + req = MobileLoginRequest( + email="user@example.com", + password="ValidPass@123", + device_name="Pixel 8", + device_type="android", + physical_device_id=physical_id, + ) + + _patch_token_helpers(monkeypatch) + + asyncio.run(service.mobile_login(FakeRedis(), req)) + + assert len(device_querier._devices) == 1 + device = next(iter(device_querier._devices.values())) + assert len(session_querier._sessions) == 1 + session = next(iter(session_querier._sessions.values())) + + assert session.device_id == device.id + assert session.device_id != physical_id + + +def test_relogin_on_existing_device_succeeds_even_at_session_cap( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Regression test for the session-cap-vs-replace bug: a user at the + session cap must still be able to re-login on a device they already + have an active session on (replace), while a genuinely new device + should evict the oldest and succeed (Phase 4 behavior).""" + user = FakeUser(email="user@example.com", exists=True) + device_querier = FakeDeviceQuerier() + session_querier = FakeSessionQuerier() + service = AuthService( + user_querier=FakeUserQuerier(user), + device_querier=device_querier, + session_querier=session_querier, + face_embedding_service=FakeFaceEmbeddingService(), + ) + + _patch_token_helpers(monkeypatch) + + # Fill up the cap with sessions on distinct devices. + for i in range(AuthService.SESSION_LIMIT): + other_req = MobileLoginRequest( + email="user@example.com", + password="ValidPass@123", + device_name=f"Device {i}", + device_type="android", + physical_device_id=uuid.uuid4(), + ) + asyncio.run(service.mobile_login(FakeRedis(), other_req)) + + assert len(session_querier._sessions) == AuthService.SESSION_LIMIT + + # A genuinely NEW device at the cap should evict the oldest and SUCCEED. + result = asyncio.run(service.mobile_login(FakeRedis(), MobileLoginRequest( + email="user@example.com", + password="ValidPass@123", + device_name="New device", + device_type="android", + physical_device_id=uuid.uuid4(), + ))) + assert result.access_token == "access" + # Count stays at cap — one evicted, one added. + assert len(session_querier._sessions) == AuthService.SESSION_LIMIT + + # Re-logging in on an EXISTING device (replace) must still succeed. + existing_physical_id = next(iter(device_querier._devices.values())).physical_device_id + repeat_req = MobileLoginRequest( + email="user@example.com", + password="ValidPass@123", + device_name="Device 0", + device_type="android", + physical_device_id=existing_physical_id, + ) + result = asyncio.run(service.mobile_login(FakeRedis(), repeat_req)) + assert result.access_token == "access" + # Session count must NOT have grown — this was a replace, not an addition. + assert len(session_querier._sessions) == AuthService.SESSION_LIMIT + +def test_same_physical_device_id_reuses_device_row( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Two logins with the same (user, physical_device_id) must reuse the + same device row, never create a duplicate.""" + user = FakeUser(email="user@example.com", exists=True) + device_querier = FakeDeviceQuerier() + session_querier = FakeSessionQuerier() + service = AuthService( + user_querier=FakeUserQuerier(user), + device_querier=device_querier, + session_querier=session_querier, + face_embedding_service=FakeFaceEmbeddingService(), + ) + + _patch_token_helpers(monkeypatch) + + physical_id = uuid.uuid4() + + for _ in range(3): + req = MobileLoginRequest( + email="user@example.com", + password="ValidPass@123", + device_name="Pixel 8", + device_type="android", + physical_device_id=physical_id, + ) + asyncio.run(service.mobile_login(FakeRedis(), req)) + + assert len(device_querier._devices) == 1 + assert len(session_querier._sessions) == 1 diff --git a/tests/unit/test_mobile_auth_rate_limiting.py b/tests/unit/test_mobile_auth_rate_limiting.py index e5f227ed..960485b1 100644 --- a/tests/unit/test_mobile_auth_rate_limiting.py +++ b/tests/unit/test_mobile_auth_rate_limiting.py @@ -81,7 +81,7 @@ async def _dummy_create_session(*args: object, **kwargs: object) -> Any: password="ValidPass@123", device_name="Pixel 8", device_type="android", - device_id=uuid.uuid4(), + physical_device_id=uuid.uuid4(), ) # Call mobile_login 5 times (which is the default max limit in settings) diff --git a/tests/unit/test_mobile_auth_request_validation.py b/tests/unit/test_mobile_auth_request_validation.py index 56d3a82e..39b287ed 100644 --- a/tests/unit/test_mobile_auth_request_validation.py +++ b/tests/unit/test_mobile_auth_request_validation.py @@ -88,7 +88,7 @@ def _valid_payload() -> dict[str, object]: "password": "ValidPass@123", "device_name": "Pixel 8", "device_type": "android", - "device_id": str(uuid.uuid4()), + "physical_device_id": str(uuid.uuid4()), }