fix(auth): resolve timezone-naive datetime comparisons in QR challenge functions and update docs
Co-authored-by: christianlouis <361235+christianlouis@users.noreply.github.com>
This commit is contained in:
@@ -336,7 +336,12 @@ def validate_qr_challenge(db: Session, challenge_token: str) -> QRLoginChallenge
|
|||||||
return None
|
return None
|
||||||
if challenge.is_claimed or challenge.is_cancelled:
|
if challenge.is_claimed or challenge.is_cancelled:
|
||||||
return None
|
return None
|
||||||
if challenge.expires_at < now:
|
|
||||||
|
# Ensure timezone-aware comparison (SQLite returns naive datetimes)
|
||||||
|
expires = challenge.expires_at
|
||||||
|
if expires and expires.tzinfo is None:
|
||||||
|
expires = expires.replace(tzinfo=timezone.utc)
|
||||||
|
if expires and expires < now:
|
||||||
return None
|
return None
|
||||||
|
|
||||||
return challenge
|
return challenge
|
||||||
@@ -432,11 +437,16 @@ def get_challenge_status(db: Session, challenge_id: int, user_id: str) -> dict |
|
|||||||
return None
|
return None
|
||||||
|
|
||||||
now = datetime.now(timezone.utc)
|
now = datetime.now(timezone.utc)
|
||||||
|
# Ensure timezone-aware comparison (SQLite returns naive datetimes)
|
||||||
|
expires = challenge.expires_at
|
||||||
|
if expires and expires.tzinfo is None:
|
||||||
|
expires = expires.replace(tzinfo=timezone.utc)
|
||||||
|
|
||||||
if challenge.is_claimed:
|
if challenge.is_claimed:
|
||||||
status = "claimed"
|
status = "claimed"
|
||||||
elif challenge.is_cancelled:
|
elif challenge.is_cancelled:
|
||||||
status = "cancelled"
|
status = "cancelled"
|
||||||
elif challenge.expires_at < now:
|
elif expires and expires < now:
|
||||||
status = "expired"
|
status = "expired"
|
||||||
else:
|
else:
|
||||||
status = "pending"
|
status = "pending"
|
||||||
|
|||||||
@@ -154,6 +154,59 @@ DocuElevate can work with any OpenID Connect-compliant provider, not just Authen
|
|||||||
OAUTH_PROVIDER_NAME=Auth0
|
OAUTH_PROVIDER_NAME=Auth0
|
||||||
```
|
```
|
||||||
|
|
||||||
|
## Server-Side Session Management
|
||||||
|
|
||||||
|
DocuElevate supports server-side session tracking. Every login creates a `UserSession` record that can be listed and revoked individually or all at once ("log off everywhere").
|
||||||
|
|
||||||
|
### Configuration
|
||||||
|
|
||||||
|
| Variable | Description | Default |
|
||||||
|
|----------|-------------|---------|
|
||||||
|
| `SESSION_LIFETIME_DAYS` | Number of days before a session expires | `30` |
|
||||||
|
| `SESSION_LIFETIME_CUSTOM_DAYS` | Override for `SESSION_LIFETIME_DAYS` when set | — |
|
||||||
|
|
||||||
|
### Managing Sessions
|
||||||
|
|
||||||
|
Users can manage their active sessions from the **Profile → Security** section:
|
||||||
|
|
||||||
|
- **View active sessions** — see browser, device, IP address, and last activity for each session.
|
||||||
|
- **Revoke a single session** — immediately invalidate one session.
|
||||||
|
- **Log off everywhere** — revoke all sessions (optionally keeping the current one) and all API tokens at once.
|
||||||
|
|
||||||
|
Expired sessions are automatically cleaned up by a periodic background task.
|
||||||
|
|
||||||
|
### API Endpoints
|
||||||
|
|
||||||
|
| Method | Path | Description |
|
||||||
|
|--------|------|-------------|
|
||||||
|
| `GET` | `/api/sessions` | List the current user's active sessions |
|
||||||
|
| `DELETE` | `/api/sessions/{id}` | Revoke a single session |
|
||||||
|
| `POST` | `/api/sessions/revoke-all` | Revoke all sessions for the current user |
|
||||||
|
|
||||||
|
## QR Code Login
|
||||||
|
|
||||||
|
QR code login allows users to authenticate a mobile device by scanning a QR code displayed in the web UI, without manually entering credentials on the phone.
|
||||||
|
|
||||||
|
### How It Works
|
||||||
|
|
||||||
|
1. The authenticated web user opens the **QR Login** page and a challenge QR code is displayed.
|
||||||
|
2. The mobile app scans the QR code and calls the claim endpoint.
|
||||||
|
3. An API token is issued for the mobile device and the web UI is notified via polling.
|
||||||
|
|
||||||
|
### Configuration
|
||||||
|
|
||||||
|
| Variable | Description | Default |
|
||||||
|
|----------|-------------|---------|
|
||||||
|
| `QR_LOGIN_CHALLENGE_TTL_SECONDS` | How long a QR challenge is valid | `120` |
|
||||||
|
|
||||||
|
### API Endpoints
|
||||||
|
|
||||||
|
| Method | Path | Description |
|
||||||
|
|--------|------|-------------|
|
||||||
|
| `POST` | `/api/qr-auth/challenge` | Create a new QR login challenge |
|
||||||
|
| `GET` | `/api/qr-auth/challenge/{id}/status` | Poll the status of a challenge |
|
||||||
|
| `POST` | `/api/qr-auth/claim` | Claim a challenge from a mobile device |
|
||||||
|
|
||||||
## Security Considerations
|
## Security Considerations
|
||||||
|
|
||||||
1. **Always use HTTPS** in production to protect authentication tokens and passwords
|
1. **Always use HTTPS** in production to protect authentication tokens and passwords
|
||||||
|
|||||||
@@ -364,6 +364,9 @@ Credentials are encrypted at rest using Fernet encryption.
|
|||||||
|-------------------------|---------------------------------------------------------------|
|
|-------------------------|---------------------------------------------------------------|
|
||||||
| `AUTH_ENABLED` | Enable or disable authentication (`true`/`false`). |
|
| `AUTH_ENABLED` | Enable or disable authentication (`true`/`false`). |
|
||||||
| `SESSION_SECRET` | Secret key used to encrypt sessions and cookies (at least 32 chars). |
|
| `SESSION_SECRET` | Secret key used to encrypt sessions and cookies (at least 32 chars). |
|
||||||
|
| `SESSION_LIFETIME_DAYS` | Number of days before a server-side session expires. Default: `30`. |
|
||||||
|
| `SESSION_LIFETIME_CUSTOM_DAYS` | Override for `SESSION_LIFETIME_DAYS` when set. |
|
||||||
|
| `QR_LOGIN_CHALLENGE_TTL_SECONDS` | How long a QR login challenge is valid (seconds). Default: `120`. |
|
||||||
| `ADMIN_USERNAME` | Username for basic authentication (when not using OIDC). |
|
| `ADMIN_USERNAME` | Username for basic authentication (when not using OIDC). |
|
||||||
| `ADMIN_PASSWORD` | Password for basic authentication (when not using OIDC). |
|
| `ADMIN_PASSWORD` | Password for basic authentication (when not using OIDC). |
|
||||||
| `ADMIN_GROUP_NAME` | Group name in OIDC claims that grants admin access. Default: `admin`. |
|
| `ADMIN_GROUP_NAME` | Group name in OIDC claims that grants admin access. Default: `admin`. |
|
||||||
|
|||||||
@@ -13,7 +13,7 @@ from __future__ import annotations
|
|||||||
|
|
||||||
import secrets
|
import secrets
|
||||||
from datetime import datetime, timedelta, timezone
|
from datetime import datetime, timedelta, timezone
|
||||||
from unittest.mock import MagicMock, patch
|
from unittest.mock import patch
|
||||||
|
|
||||||
import pytest
|
import pytest
|
||||||
from sqlalchemy import create_engine
|
from sqlalchemy import create_engine
|
||||||
@@ -22,11 +22,11 @@ from sqlalchemy.orm import Session, sessionmaker
|
|||||||
from app.database import Base
|
from app.database import Base
|
||||||
from app.models import ApiToken, QRLoginChallenge, UserSession
|
from app.models import ApiToken, QRLoginChallenge, UserSession
|
||||||
|
|
||||||
|
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
# Fixtures
|
# Fixtures
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
|
||||||
@pytest.fixture()
|
@pytest.fixture()
|
||||||
def db_session():
|
def db_session():
|
||||||
"""Provide an in-memory SQLite session with all tables created."""
|
"""Provide an in-memory SQLite session with all tables created."""
|
||||||
@@ -48,6 +48,7 @@ def sample_user_id():
|
|||||||
# Model Tests
|
# Model Tests
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.unit
|
@pytest.mark.unit
|
||||||
class TestUserSessionModel:
|
class TestUserSessionModel:
|
||||||
"""Tests for the UserSession ORM model."""
|
"""Tests for the UserSession ORM model."""
|
||||||
@@ -124,6 +125,7 @@ class TestQRLoginChallengeModel:
|
|||||||
# Session Manager Tests
|
# Session Manager Tests
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.unit
|
@pytest.mark.unit
|
||||||
class TestSessionManager:
|
class TestSessionManager:
|
||||||
"""Tests for app/utils/session_manager.py functions."""
|
"""Tests for app/utils/session_manager.py functions."""
|
||||||
@@ -301,7 +303,10 @@ class TestSessionManager:
|
|||||||
s3 = create_session(db_session, user_id=sample_user_id)
|
s3 = create_session(db_session, user_id=sample_user_id)
|
||||||
|
|
||||||
count = revoke_all_sessions(
|
count = revoke_all_sessions(
|
||||||
db_session, sample_user_id, except_session_id=s1.id, revoke_api_tokens=False,
|
db_session,
|
||||||
|
sample_user_id,
|
||||||
|
except_session_id=s1.id,
|
||||||
|
revoke_api_tokens=False,
|
||||||
)
|
)
|
||||||
assert count == 2
|
assert count == 2
|
||||||
|
|
||||||
@@ -370,6 +375,7 @@ class TestSessionManager:
|
|||||||
# QR Login Tests
|
# QR Login Tests
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.unit
|
@pytest.mark.unit
|
||||||
class TestQRLogin:
|
class TestQRLogin:
|
||||||
"""Tests for QR login challenge/claim flow."""
|
"""Tests for QR login challenge/claim flow."""
|
||||||
@@ -389,7 +395,11 @@ class TestQRLogin:
|
|||||||
assert len(challenge.challenge_token) > 32
|
assert len(challenge.challenge_token) > 32
|
||||||
assert challenge.is_claimed is False
|
assert challenge.is_claimed is False
|
||||||
assert challenge.created_by_ip == "10.0.0.1"
|
assert challenge.created_by_ip == "10.0.0.1"
|
||||||
assert challenge.expires_at > datetime.now(timezone.utc)
|
# SQLite returns naive datetimes; normalise before comparison
|
||||||
|
expires = challenge.expires_at
|
||||||
|
if expires.tzinfo is None:
|
||||||
|
expires = expires.replace(tzinfo=timezone.utc)
|
||||||
|
assert expires > datetime.now(timezone.utc)
|
||||||
|
|
||||||
@patch("app.utils.session_manager.settings")
|
@patch("app.utils.session_manager.settings")
|
||||||
def test_validate_qr_challenge_valid(self, mock_settings, db_session: Session, sample_user_id: str):
|
def test_validate_qr_challenge_valid(self, mock_settings, db_session: Session, sample_user_id: str):
|
||||||
@@ -575,6 +585,7 @@ class TestQRLogin:
|
|||||||
# Device Info Parsing Tests
|
# Device Info Parsing Tests
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.unit
|
@pytest.mark.unit
|
||||||
class TestDeviceInfoParsing:
|
class TestDeviceInfoParsing:
|
||||||
"""Tests for User-Agent parsing."""
|
"""Tests for User-Agent parsing."""
|
||||||
@@ -634,6 +645,7 @@ class TestDeviceInfoParsing:
|
|||||||
# Config Tests
|
# Config Tests
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.unit
|
@pytest.mark.unit
|
||||||
class TestSessionConfig:
|
class TestSessionConfig:
|
||||||
"""Tests for session-related configuration fields."""
|
"""Tests for session-related configuration fields."""
|
||||||
|
|||||||
Reference in New Issue
Block a user