feat: handle Garmin MFA and activity retries
This commit is contained in:
@@ -168,6 +168,15 @@ class ActivityRepository:
|
|||||||
self.session.commit()
|
self.session.commit()
|
||||||
return activity
|
return activity
|
||||||
|
|
||||||
|
def reset_retryable_failure(self, activity_id: int) -> Activity:
|
||||||
|
activity = self._require(activity_id)
|
||||||
|
if activity.status != ActivityStatus.FAILED or not activity.retryable:
|
||||||
|
raise ValueError("activity is not retryable")
|
||||||
|
activity.status = activity.last_completed_stage
|
||||||
|
activity.last_error = None
|
||||||
|
self.session.commit()
|
||||||
|
return activity
|
||||||
|
|
||||||
def list_pending_for_user(self, user_id: int) -> list[Activity]:
|
def list_pending_for_user(self, user_id: int) -> list[Activity]:
|
||||||
return list(
|
return list(
|
||||||
self.session.scalars(
|
self.session.scalars(
|
||||||
|
|||||||
@@ -254,6 +254,11 @@ class SyncManager:
|
|||||||
|
|
||||||
session.commit()
|
session.commit()
|
||||||
|
|
||||||
|
if not stop_user_run:
|
||||||
|
user.action_reason = None
|
||||||
|
user.health_state = HealthState.DEGRADED if failed_count > 0 else HealthState.HEALTHY
|
||||||
|
session.commit()
|
||||||
|
|
||||||
status = (
|
status = (
|
||||||
SyncRunStatus.SUCCESS
|
SyncRunStatus.SUCCESS
|
||||||
if failed_count == 0 and not stop_user_run
|
if failed_count == 0 and not stop_user_run
|
||||||
|
|||||||
@@ -1,11 +1,11 @@
|
|||||||
from fastapi import APIRouter, Form, Request
|
from fastapi import APIRouter, Form, HTTPException, Request
|
||||||
from fastapi.responses import HTMLResponse
|
from fastapi.responses import HTMLResponse
|
||||||
from sqlalchemy import func, select
|
from sqlalchemy import func, select
|
||||||
|
|
||||||
from app.auth.admin import require_admin
|
from app.auth.admin import require_admin
|
||||||
from app.auth.csrf import ensure_csrf_token, validate_csrf
|
from app.auth.csrf import ensure_csrf_token, validate_csrf
|
||||||
from app.db.models import Activity
|
from app.db.models import Activity
|
||||||
from app.db.repositories import UserRepository
|
from app.db.repositories import ActivityRepository, UserRepository
|
||||||
from app.sync.manager import SyncAlreadyRunning
|
from app.sync.manager import SyncAlreadyRunning
|
||||||
from app.web.routes import templates
|
from app.web.routes import templates
|
||||||
|
|
||||||
@@ -59,6 +59,42 @@ async def manual_sync_all(request: Request, csrf_token: str = Form(...)):
|
|||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
@router.post("/users/{user_id}/garmin-mfa", response_class=HTMLResponse)
|
||||||
|
async def garmin_mfa(request: Request, user_id: int, csrf_token: str = Form(...), code: str = Form(...)):
|
||||||
|
require_admin(request)
|
||||||
|
validate_csrf(request, csrf_token)
|
||||||
|
stripped = code.strip()
|
||||||
|
if not stripped or len(stripped) > 20:
|
||||||
|
raise HTTPException(status_code=400, detail="Invalid MFA code")
|
||||||
|
try:
|
||||||
|
outcome = await request.app.state.sync_manager.sync_user(user_id, mfa_code=stripped)
|
||||||
|
except SyncAlreadyRunning:
|
||||||
|
return HTMLResponse("Sync already running for this user", status_code=409)
|
||||||
|
return templates.TemplateResponse(
|
||||||
|
request, "fragments/sync_result.html", {"outcomes": [_normalize_outcome(outcome)]}
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
@router.post("/activities/{activity_id}/retry", response_class=HTMLResponse)
|
||||||
|
async def retry_activity(request: Request, activity_id: int, csrf_token: str = Form(...)):
|
||||||
|
require_admin(request)
|
||||||
|
validate_csrf(request, csrf_token)
|
||||||
|
with request.app.state.session_factory() as session:
|
||||||
|
activity_repo = ActivityRepository(session)
|
||||||
|
try:
|
||||||
|
activity = activity_repo.reset_retryable_failure(activity_id)
|
||||||
|
except ValueError:
|
||||||
|
return HTMLResponse("Activity is not retryable", status_code=409)
|
||||||
|
user_id = activity.user_id
|
||||||
|
try:
|
||||||
|
outcome = await request.app.state.sync_manager.sync_user(user_id)
|
||||||
|
except SyncAlreadyRunning:
|
||||||
|
return HTMLResponse("Sync already running for this user", status_code=409)
|
||||||
|
return templates.TemplateResponse(
|
||||||
|
request, "fragments/sync_result.html", {"outcomes": [_normalize_outcome(outcome)]}
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
@router.get("/system", response_class=HTMLResponse)
|
@router.get("/system", response_class=HTMLResponse)
|
||||||
def system_page(request: Request):
|
def system_page(request: Request):
|
||||||
require_admin(request)
|
require_admin(request)
|
||||||
|
|||||||
@@ -7,7 +7,7 @@ from fastapi.templating import Jinja2Templates
|
|||||||
from app.auth.admin import password_matches, require_admin
|
from app.auth.admin import password_matches, require_admin
|
||||||
from app.auth.csrf import ensure_csrf_token, validate_csrf
|
from app.auth.csrf import ensure_csrf_token, validate_csrf
|
||||||
from app.db.models import SyncUser
|
from app.db.models import SyncUser
|
||||||
from app.db.repositories import UserRepository
|
from app.db.repositories import ActivityRepository, UserRepository
|
||||||
from app.security.credentials import CredentialCipher
|
from app.security.credentials import CredentialCipher
|
||||||
from app.web.forms import UserFormData
|
from app.web.forms import UserFormData
|
||||||
|
|
||||||
@@ -131,12 +131,14 @@ def user_detail(request: Request, user_id: int):
|
|||||||
require_admin(request)
|
require_admin(request)
|
||||||
with request.app.state.session_factory() as session:
|
with request.app.state.session_factory() as session:
|
||||||
user = _get_user_or_404(UserRepository(session), user_id)
|
user = _get_user_or_404(UserRepository(session), user_id)
|
||||||
|
activities = ActivityRepository(session).list_pending_for_user(user_id)
|
||||||
return templates.TemplateResponse(
|
return templates.TemplateResponse(
|
||||||
request,
|
request,
|
||||||
"users/detail.html",
|
"users/detail.html",
|
||||||
{
|
{
|
||||||
"csrf_token": ensure_csrf_token(request),
|
"csrf_token": ensure_csrf_token(request),
|
||||||
"user": user,
|
"user": user,
|
||||||
|
"activities": activities,
|
||||||
},
|
},
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|||||||
5
app/web/templates/fragments/mfa_form.html
Normal file
5
app/web/templates/fragments/mfa_form.html
Normal file
@@ -0,0 +1,5 @@
|
|||||||
|
<form method="post" action="/users/{{ user.id }}/garmin-mfa">
|
||||||
|
<input type="hidden" name="csrf_token" value="{{ csrf_token }}">
|
||||||
|
<label>Garmin MFA code: <input type="text" name="code" maxlength="20" required></label>
|
||||||
|
<button type="submit">Submit code</button>
|
||||||
|
</form>
|
||||||
@@ -28,4 +28,26 @@
|
|||||||
<dt>Updated at</dt>
|
<dt>Updated at</dt>
|
||||||
<dd>{{ user.updated_at }}</dd>
|
<dd>{{ user.updated_at }}</dd>
|
||||||
</dl>
|
</dl>
|
||||||
|
|
||||||
|
{% if user.action_reason == "garmin_mfa_required" %}
|
||||||
|
<h2>Garmin MFA required</h2>
|
||||||
|
{% include "fragments/mfa_form.html" %}
|
||||||
|
{% endif %}
|
||||||
|
|
||||||
|
<h2>Activities</h2>
|
||||||
|
<ul>
|
||||||
|
{% for activity in activities %}
|
||||||
|
<li>
|
||||||
|
{{ activity.activity_name }} — {{ activity.status.value }}
|
||||||
|
{% if activity.status.value == "failed" and activity.retryable %}
|
||||||
|
<form method="post" action="/activities/{{ activity.id }}/retry" style="display:inline">
|
||||||
|
<input type="hidden" name="csrf_token" value="{{ csrf_token }}">
|
||||||
|
<button type="submit">Retry</button>
|
||||||
|
</form>
|
||||||
|
{% endif %}
|
||||||
|
</li>
|
||||||
|
{% else %}
|
||||||
|
<li>No pending activities.</li>
|
||||||
|
{% endfor %}
|
||||||
|
</ul>
|
||||||
{% endblock %}
|
{% endblock %}
|
||||||
|
|||||||
139
tests/web/test_mfa.py
Normal file
139
tests/web/test_mfa.py
Normal file
@@ -0,0 +1,139 @@
|
|||||||
|
import logging
|
||||||
|
|
||||||
|
from sqlalchemy import text
|
||||||
|
|
||||||
|
from app.db.models import Activity, ActivityStatus
|
||||||
|
|
||||||
|
|
||||||
|
def test_mfa_code_is_used_once_and_not_persisted(authenticated_client, fake_sync_manager, app, caplog) -> None:
|
||||||
|
with caplog.at_level(logging.DEBUG):
|
||||||
|
response = authenticated_client.post(
|
||||||
|
"/users/1/garmin-mfa",
|
||||||
|
data={"csrf_token": authenticated_client.csrf_token, "code": "123456"},
|
||||||
|
)
|
||||||
|
assert response.status_code == 200
|
||||||
|
assert fake_sync_manager.mfa_calls == [(1, "123456")]
|
||||||
|
|
||||||
|
# Defense-in-depth: the MFA code must never end up written to the real
|
||||||
|
# app database (the one authenticated_client's requests actually hit),
|
||||||
|
# not some unrelated in-memory db.
|
||||||
|
with app.state.session_factory() as session:
|
||||||
|
persisted_text = " ".join(str(row) for row in session.execute(text("select * from sync_runs")).all())
|
||||||
|
assert "123456" not in persisted_text
|
||||||
|
|
||||||
|
# The check that actually matters: the code must never be logged,
|
||||||
|
# regardless of which SyncManager implementation is in play.
|
||||||
|
assert "123456" not in caplog.text
|
||||||
|
|
||||||
|
|
||||||
|
def test_mfa_code_rejects_empty_code(authenticated_client, fake_sync_manager) -> None:
|
||||||
|
response = authenticated_client.post(
|
||||||
|
"/users/1/garmin-mfa",
|
||||||
|
data={"csrf_token": authenticated_client.csrf_token, "code": " "},
|
||||||
|
)
|
||||||
|
assert response.status_code == 400
|
||||||
|
assert fake_sync_manager.mfa_calls == []
|
||||||
|
|
||||||
|
|
||||||
|
def test_mfa_code_rejects_overlong_code(authenticated_client, fake_sync_manager) -> None:
|
||||||
|
response = authenticated_client.post(
|
||||||
|
"/users/1/garmin-mfa",
|
||||||
|
data={"csrf_token": authenticated_client.csrf_token, "code": "1" * 21},
|
||||||
|
)
|
||||||
|
assert response.status_code == 400
|
||||||
|
assert fake_sync_manager.mfa_calls == []
|
||||||
|
|
||||||
|
|
||||||
|
def _seed_activity(app, *, status: ActivityStatus, last_completed_stage: ActivityStatus, retryable: bool, last_error: str | None = None) -> tuple[int, int]:
|
||||||
|
"""Seed a real Activity (and its owning user) in the app fixture's actual
|
||||||
|
database -- the same database authenticated_client's HTTP requests hit --
|
||||||
|
and return (user_id, activity_id)."""
|
||||||
|
from app.db.repositories import ActivityRepository, UserRepository
|
||||||
|
|
||||||
|
with app.state.session_factory() as session:
|
||||||
|
user = UserRepository(session).create(
|
||||||
|
name="MFA Test User",
|
||||||
|
enabled=True,
|
||||||
|
mywhoosh_email_enc="mw@example.com",
|
||||||
|
mywhoosh_password_enc="mw-pass",
|
||||||
|
garmin_email_enc="garmin@example.com",
|
||||||
|
garmin_password_enc="garmin-pass",
|
||||||
|
)
|
||||||
|
activity_repo = ActivityRepository(session)
|
||||||
|
activity, _ = activity_repo.get_or_create_discovered(
|
||||||
|
user_id=user.id,
|
||||||
|
mywhoosh_activity_id="mw-activity-1",
|
||||||
|
activity_name="Test Activity",
|
||||||
|
activity_timestamp=None,
|
||||||
|
)
|
||||||
|
activity.status = status
|
||||||
|
activity.last_completed_stage = last_completed_stage
|
||||||
|
activity.retryable = retryable
|
||||||
|
if last_error is not None:
|
||||||
|
activity.last_error = last_error
|
||||||
|
session.commit()
|
||||||
|
return user.id, activity.id
|
||||||
|
|
||||||
|
|
||||||
|
def test_retry_resets_and_calls_sync_for_retryable_failed_activity(authenticated_client, fake_sync_manager, app) -> None:
|
||||||
|
user_id, activity_id = _seed_activity(
|
||||||
|
app,
|
||||||
|
status=ActivityStatus.FAILED,
|
||||||
|
last_completed_stage=ActivityStatus.CONVERTED,
|
||||||
|
retryable=True,
|
||||||
|
last_error="some transient error",
|
||||||
|
)
|
||||||
|
|
||||||
|
response = authenticated_client.post(
|
||||||
|
f"/activities/{activity_id}/retry",
|
||||||
|
data={"csrf_token": authenticated_client.csrf_token},
|
||||||
|
)
|
||||||
|
|
||||||
|
assert response.status_code == 200
|
||||||
|
assert fake_sync_manager.user_calls == [user_id]
|
||||||
|
|
||||||
|
with app.state.session_factory() as session:
|
||||||
|
reloaded = session.get(Activity, activity_id)
|
||||||
|
assert reloaded.status == ActivityStatus.CONVERTED
|
||||||
|
assert reloaded.last_error is None
|
||||||
|
|
||||||
|
|
||||||
|
def test_retry_rejects_non_retryable_activity(authenticated_client, fake_sync_manager, app) -> None:
|
||||||
|
user_id, activity_id = _seed_activity(
|
||||||
|
app,
|
||||||
|
status=ActivityStatus.FAILED,
|
||||||
|
last_completed_stage=ActivityStatus.CONVERTED,
|
||||||
|
retryable=False,
|
||||||
|
last_error="permanent failure",
|
||||||
|
)
|
||||||
|
|
||||||
|
response = authenticated_client.post(
|
||||||
|
f"/activities/{activity_id}/retry",
|
||||||
|
data={"csrf_token": authenticated_client.csrf_token},
|
||||||
|
)
|
||||||
|
|
||||||
|
assert response.status_code == 409
|
||||||
|
assert fake_sync_manager.user_calls == []
|
||||||
|
|
||||||
|
with app.state.session_factory() as session:
|
||||||
|
reloaded = session.get(Activity, activity_id)
|
||||||
|
assert reloaded.status == ActivityStatus.FAILED
|
||||||
|
assert reloaded.retryable is False
|
||||||
|
assert reloaded.last_error == "permanent failure"
|
||||||
|
|
||||||
|
|
||||||
|
def test_retry_rejects_non_failed_activity(authenticated_client, fake_sync_manager, app) -> None:
|
||||||
|
user_id, activity_id = _seed_activity(
|
||||||
|
app,
|
||||||
|
status=ActivityStatus.DISCOVERED,
|
||||||
|
last_completed_stage=ActivityStatus.DISCOVERED,
|
||||||
|
retryable=True,
|
||||||
|
)
|
||||||
|
|
||||||
|
response = authenticated_client.post(
|
||||||
|
f"/activities/{activity_id}/retry",
|
||||||
|
data={"csrf_token": authenticated_client.csrf_token},
|
||||||
|
)
|
||||||
|
|
||||||
|
assert response.status_code == 409
|
||||||
|
assert fake_sync_manager.user_calls == []
|
||||||
Reference in New Issue
Block a user