Skip to content

Commit 26fbcb2

Browse files
committed
feat: add session secret warning and improve device auth token consumption logic
1 parent 6afbf76 commit 26fbcb2

5 files changed

Lines changed: 46 additions & 39 deletions

File tree

app.py

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,14 @@ async def lifespan(app: FastAPI) -> AsyncIterator[None]:
124124

125125
await ensure_indexes(app.state.db)
126126

127+
# Warn if session secret is missing when auth is enabled
128+
if settings.jwt and not settings.secret_key:
129+
log.warning(
130+
"secret_key_empty",
131+
detail="SECRET_KEY is empty — session cookies are unsigned. "
132+
"Set a strong SECRET_KEY when auth/OAuth is enabled.",
133+
)
134+
127135
# Warn if CORS private origins not configured in production
128136
if settings.is_production and not settings.cors_private_origins:
129137
log.warning(

repositories/token_repository.py

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,34 @@ async def find_by_hash_and_type(
5959
)
6060
raise
6161

62+
async def consume_by_hash(
63+
self, token_hash: str, token_type: str
64+
) -> VerificationTokenDoc | None:
65+
"""Atomically find an unused, non-expired token and mark it as used.
66+
67+
Returns the pre-update document, or None if no matching token exists.
68+
"""
69+
now = datetime.now(timezone.utc)
70+
try:
71+
doc = await self._col.find_one_and_update(
72+
{
73+
"token_hash": token_hash,
74+
"token_type": token_type,
75+
"used_at": None,
76+
"expires_at": {"$gt": now},
77+
},
78+
{"$set": {"used_at": now}},
79+
)
80+
return VerificationTokenDoc.from_mongo(doc)
81+
except PyMongoError as exc:
82+
log.error(
83+
"token_repo_consume_by_hash_failed",
84+
token_type=token_type,
85+
error=str(exc),
86+
error_type=type(exc).__name__,
87+
)
88+
raise
89+
6290
async def mark_as_used(self, token_id: ObjectId) -> bool:
6391
"""
6492
Mark a token as consumed by setting ``used_at`` to now.

schemas/dto/requests/auth.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,5 +122,7 @@ class DeviceTokenRequest(BaseModel):
122122
model_config = ConfigDict(populate_by_name=True)
123123

124124
code: str = Field(
125+
min_length=1,
126+
max_length=128,
125127
description="One-time auth code from the device callback page",
126128
)

services/auth_service.py

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -627,23 +627,12 @@ async def exchange_device_code(self, code: str) -> tuple[UserDoc, str, str]:
627627
AuthenticationError: Code invalid, expired, or already used.
628628
"""
629629
token_hash = hash_token(code)
630-
token_doc = await self._token_repo.find_by_hash_and_type(
630+
token_doc = await self._token_repo.consume_by_hash(
631631
token_hash, TOKEN_TYPE_DEVICE_AUTH
632632
)
633633
if not token_doc:
634634
raise AuthenticationError("invalid or expired device auth code")
635635

636-
expires_at = token_doc.expires_at
637-
if not expires_at.tzinfo:
638-
expires_at = expires_at.replace(tzinfo=timezone.utc)
639-
640-
if expires_at <= datetime.now(timezone.utc):
641-
raise AuthenticationError("device auth code has expired")
642-
643-
marked = await self._token_repo.mark_as_used(token_doc.id)
644-
if not marked:
645-
raise AppError("failed to consume device auth code")
646-
647636
user = await self._user_repo.find_by_id(token_doc.user_id)
648637
if not user or user.status != UserStatus.ACTIVE:
649638
raise AuthenticationError("user not found or inactive")

tests/unit/services/test_auth_service.py

Lines changed: 7 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -884,47 +884,27 @@ async def test_exchange_device_code_success(self):
884884
"attempts": 0,
885885
}
886886
)
887-
svc._token_repo.find_by_hash_and_type.return_value = token_doc
888-
svc._token_repo.mark_as_used.return_value = True
887+
svc._token_repo.consume_by_hash.return_value = token_doc
889888
svc._user_repo.find_by_id.return_value = make_user_doc(email_verified=True)
890889

891890
_user, access, refresh = await svc.exchange_device_code(raw_code)
892891
assert isinstance(access, str)
893892
assert isinstance(refresh, str)
894-
svc._token_repo.mark_as_used.assert_awaited_once()
893+
svc._token_repo.consume_by_hash.assert_awaited_once()
895894

896895
@pytest.mark.asyncio
897896
async def test_exchange_device_code_invalid(self):
898897
svc = make_auth_service()
899-
svc._token_repo.find_by_hash_and_type.return_value = None
898+
svc._token_repo.consume_by_hash.return_value = None
900899

901900
with pytest.raises(AuthenticationError, match="invalid or expired"):
902901
await svc.exchange_device_code("bad-code")
903902

904903
@pytest.mark.asyncio
905904
async def test_exchange_device_code_expired(self):
906-
from datetime import timedelta
907-
908-
from schemas.models.token import TOKEN_TYPE_DEVICE_AUTH, VerificationTokenDoc
909-
from shared.crypto import hash_token
910-
905+
"""Expired codes are filtered out by consume_by_hash (expires_at in query)."""
911906
svc = make_auth_service()
912-
raw_code = "expired-code"
913-
past = datetime(2020, 1, 1, tzinfo=timezone.utc)
914-
token_doc = VerificationTokenDoc.from_mongo(
915-
{
916-
"_id": ObjectId(),
917-
"user_id": USER_OID,
918-
"email": "test@example.com",
919-
"token_hash": hash_token(raw_code),
920-
"token_type": TOKEN_TYPE_DEVICE_AUTH,
921-
"expires_at": past,
922-
"created_at": past - timedelta(minutes=5),
923-
"used_at": None,
924-
"attempts": 0,
925-
}
926-
)
927-
svc._token_repo.find_by_hash_and_type.return_value = token_doc
907+
svc._token_repo.consume_by_hash.return_value = None
928908

929-
with pytest.raises(AuthenticationError, match="expired"):
930-
await svc.exchange_device_code(raw_code)
909+
with pytest.raises(AuthenticationError, match="invalid or expired"):
910+
await svc.exchange_device_code("expired-code")

0 commit comments

Comments
 (0)