From 7ba37561e4d84dc97c8ab876251ee7e63bd00975 Mon Sep 17 00:00:00 2001 From: Daniel Date: Sat, 12 Sep 2026 05:43:25 +0200 Subject: [PATCH] feat: the exam player as the reference has it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The incomplete-block warning is the one from the screenshot: a red heading that says the block is incomplete, the count of unanswered items, the sentence about resuming not matching exam day, and End Block against Remain in Block. My version asked the question in my own words and led with the wrong button. Pausing says "Exam Paused" and offers Return to exam. Nothing else — the warning about real exams is somebody else's disclaimer, not ours. Exit session asks "Are you sure you want to end this session?" before it goes, rather than going. Time's Up says what it is and the button says Close, which is the only thing left to do: it is already handed in and marked, and Close lands on the session's analysis. One name for one action: the bottom button read Skip on an unanswered question and Next on an answered one, while the arrow an inch above it said Next for both. And the rail shows stems again once the block is handed in. Numbers while it is being sat — reading ahead is not something the exam being rehearsed allows — but there is nothing left to protect afterwards, so the review reads like study mode. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01TqXevQJhxFrM7jJg82cgZN --- backend/app/config.py | 8 +- backend/app/routers/auth.py | 31 +------ backend/app/routers/contact.py | 32 ++----- backend/app/routers/mobile.py | 2 +- backend/app/schemas/auth.py | 3 +- backend/app/services/captcha.py | 66 ++++++++++++++ frontend/src/pages/QuizPage.jsx | 132 ++++++++++++++++----------- frontend/src/pages/QuizPage.test.jsx | 21 +++-- frontend/src/pages/QuizPlayer.css | 28 +++--- 9 files changed, 194 insertions(+), 129 deletions(-) create mode 100644 backend/app/services/captcha.py diff --git a/backend/app/config.py b/backend/app/config.py index 4b9fd7d..e33c136 100644 --- a/backend/app/config.py +++ b/backend/app/config.py @@ -53,7 +53,13 @@ class Settings(BaseSettings): S3_REGION: str = "us-east-1" MAX_UPLOAD_SIZE: int = 524288000 # 500MB - TURNSTILE_SECRET_KEY: str = "" # Cloudflare Turnstile — leave blank to disable captcha + # hCaptcha. Leave the secret blank to disable the challenge entirely. + HCAPTCHA_SECRET_KEY: str = "" + # The browser gets its own copy of the site key from the frontend + # container, which is a separate image with a separate .env. This one is + # here so an operator can keep both halves of the pair together and see + # at a glance which widget the secret belongs to. + HCAPTCHA_SITE_KEY: str = "" ADMIN_EMAIL: str = "" # Where contact form submissions are emailed DEFAULT_ADMIN_EMAIL: str = "" # Optional explicit bootstrap admin email diff --git a/backend/app/routers/auth.py b/backend/app/routers/auth.py index c285b82..b09f575 100644 --- a/backend/app/routers/auth.py +++ b/backend/app/routers/auth.py @@ -4,7 +4,7 @@ from datetime import datetime, timedelta from fastapi import APIRouter, Depends, HTTPException, BackgroundTasks, status, Request from sqlalchemy.orm import Session -from app.services import invites, site_settings +from app.services import captcha, invites, site_settings from app.database import get_db from app.models.user import User from app.models.email_verification import EmailVerification @@ -61,25 +61,6 @@ def _check_reset_rate_limit(db: Session, email: str): ) -async def _verify_turnstile(token: str) -> bool: - """Verify Cloudflare Turnstile token. Returns True if valid or if not configured.""" - from app.config import settings as cfg - secret = cfg.TURNSTILE_SECRET_KEY - if not secret: - return True - try: - import httpx - resp = httpx.post( - "https://challenges.cloudflare.com/turnstile/v0/siteverify", - data={"secret": secret, "response": token}, - timeout=10, - ) - return resp.json().get("success", False) - except Exception as e: - import logging; logging.getLogger(__name__).warning(f"Turnstile verification failed (failing open): {e}") - return True - - @router.get("/signup-policy") def signup_policy(db: Session = Depends(get_db)): """What a would-be member needs, before they are anybody. @@ -98,13 +79,9 @@ def signup_policy(db: Session = Depends(get_db)): @router.post("/register") async def register(user_data: UserCreate, background_tasks: BackgroundTasks, db: Session = Depends(get_db)): - # Verify Turnstile if configured - from app.config import settings as cfg - if cfg.TURNSTILE_SECRET_KEY: - if not user_data.turnstile_token: - raise HTTPException(status_code=400, detail="Bot verification required") - if not await _verify_turnstile(user_data.turnstile_token): - raise HTTPException(status_code=400, detail="Bot verification failed — please try again") + # Failing open: an hCaptcha outage should cost the site a little spam, not + # every account that would have been created while it lasted. + await captcha.require_human(user_data.captcha_token, fail_open=True) # Check if registration is enabled (unless this is the first user - always allow admin creation) is_first_user = db.query(User).count() == 0 diff --git a/backend/app/routers/contact.py b/backend/app/routers/contact.py index b16d27b..c48a6e3 100644 --- a/backend/app/routers/contact.py +++ b/backend/app/routers/contact.py @@ -10,6 +10,7 @@ from sqlalchemy.orm import Session from app.database import get_db from app.config import settings +from app.services import captcha router = APIRouter() log = logging.getLogger(__name__) @@ -20,25 +21,7 @@ class ContactRequest(BaseModel): email: EmailStr type: str # "question" | "moderator" message: str - turnstile_token: str | None = None - - -async def _verify_turnstile(token: str) -> bool: - """Verify Cloudflare Turnstile token. Returns True if valid or if Turnstile not configured.""" - secret = getattr(settings, "TURNSTILE_SECRET_KEY", None) - if not secret: - return True # Not configured — allow in dev/no-captcha mode - try: - import httpx - resp = httpx.post( - "https://challenges.cloudflare.com/turnstile/v0/siteverify", - data={"secret": secret, "response": token}, - timeout=10, - ) - return resp.json().get("success", False) - except Exception as e: - log.warning(f"Turnstile verification failed: {e}") - return False + captcha_token: str | None = None @router.post("") @@ -52,13 +35,10 @@ async def submit_contact(req: ContactRequest, db: Session = Depends(get_db)): if not name or not message: raise HTTPException(status_code=400, detail="Name and message are required") - # Verify Turnstile if configured - secret = getattr(settings, "TURNSTILE_SECRET_KEY", None) - if secret: - if not req.turnstile_token: - raise HTTPException(status_code=400, detail="Bot verification required") - if not await _verify_turnstile(req.turnstile_token): - raise HTTPException(status_code=400, detail="Bot verification failed — please try again") + # Failing shut, unlike registration: if hCaptcha cannot be reached, the + # cost of turning this form away is one retry, and the inbox behind it has + # no other defence. + await captcha.require_human(req.captcha_token, fail_open=False) db.execute(text( "INSERT INTO contact_submissions (name, email, type, message) VALUES (:name, :email, :type, :message)" diff --git a/backend/app/routers/mobile.py b/backend/app/routers/mobile.py index 661ab81..903f80d 100644 --- a/backend/app/routers/mobile.py +++ b/backend/app/routers/mobile.py @@ -138,7 +138,7 @@ def _quiz_payload(db: Session, quiz: Quiz, include_questions: bool = True): @router.post("/auth/login") def mobile_login(data: MobileLoginRequest, request: Request, db: Session = Depends(get_db)): - """Password login for native apps. Uses rate limiting instead of browser Turnstile.""" + """Password login for native apps. Uses rate limiting instead of a browser captcha.""" client_ip = request.client.host if request and request.client else "unknown" _mobile_login_rate_limit(client_ip) diff --git a/backend/app/schemas/auth.py b/backend/app/schemas/auth.py index eb78866..407839f 100644 --- a/backend/app/schemas/auth.py +++ b/backend/app/schemas/auth.py @@ -7,7 +7,8 @@ class UserCreate(BaseModel): email: EmailStr password: str name: str - turnstile_token: str | None = None + #: Solved hCaptcha challenge. Absent when the site has no secret configured. + captcha_token: str | None = None #: Required only while the site is invite-only. invite_code: str | None = None diff --git a/backend/app/services/captcha.py b/backend/app/services/captcha.py new file mode 100644 index 0000000..5c61d0d --- /dev/null +++ b/backend/app/services/captcha.py @@ -0,0 +1,66 @@ +"""hCaptcha checks on the two forms a stranger can reach. + +Both the sign-up and the contact form used to carry their own copy of this, +which is how they drifted apart. It lives here once so that the answer to +"what happens when the challenge is missing, wrong, or unanswerable" is given +in a single place. + +A blank secret means no captcha at all. That is the deliberate default: a +deployment that has never signed up for hCaptcha must still be able to +register its first administrator, so an unset key reads as "there is no +challenge here", never as "reject everybody". +""" +import logging + +from fastapi import HTTPException + +from app.config import settings + +logger = logging.getLogger(__name__) + +VERIFY_URL = "https://api.hcaptcha.com/siteverify" + + +def configured() -> bool: + return bool(settings.HCAPTCHA_SECRET_KEY) + + +async def verify(token: str, *, fail_open: bool) -> bool: + """Ask hCaptcha whether this token is a real, unspent solve. + + `fail_open` decides what an unreachable hCaptcha means, and the two callers + genuinely want different answers: an outage that stops people creating + accounts costs the site its users, while an outage that bounces a contact + message costs the sender one retry. + """ + if not configured(): + return True + try: + import httpx + # Awaited rather than blocking: uvicorn runs four workers, and a + # ten-second stall on a third party would otherwise hold up every + # other request that worker is serving. + async with httpx.AsyncClient(timeout=10) as client: + resp = await client.post( + VERIFY_URL, + data={"secret": settings.HCAPTCHA_SECRET_KEY, "response": token}, + ) + return resp.json().get("success", False) + except Exception: + logger.warning("hCaptcha verification unavailable; %s", + "allowing the request" if fail_open else "rejecting the request", + exc_info=True) + return fail_open + + +async def require_human(token: str | None, *, fail_open: bool) -> None: + """Reject the submission unless it carries a solved challenge. + + Silent when no secret is set, so an unconfigured site keeps working. + """ + if not configured(): + return + if not token: + raise HTTPException(status_code=400, detail="Bot verification required") + if not await verify(token, fail_open=fail_open): + raise HTTPException(status_code=400, detail="Bot verification failed — please try again") diff --git a/frontend/src/pages/QuizPage.jsx b/frontend/src/pages/QuizPage.jsx index 85c70a1..09a9aa9 100644 --- a/frontend/src/pages/QuizPage.jsx +++ b/frontend/src/pages/QuizPage.jsx @@ -433,6 +433,10 @@ export default function QuizPage() { const [sessionSeconds, setSessionSeconds] = useState(0) const [questionSeconds, setQuestionSeconds] = useState(0) const [clockPaused, setClockPaused] = useState(false) + // Asked before the session closes. Leaving is not destructive — the answers + // are saved and it can be resumed — but it is not what a mis-aimed thumb + // should do either. + const [leaving, setLeaving] = useState(false) // The clock ran out: the answers are in, and the analysis waits behind an // acknowledgement rather than replacing the exam without a word. const [timeUp, setTimeUp] = useState(false) @@ -1250,12 +1254,12 @@ const timerStarted = timeLeft !== null {isStudy ? 'Finish session' : 'End block'} ) : ( - /* Skip, when nothing has been chosen. Moving on from a question you - have not answered is a decision, and the button should say which - decision it is rather than calling both of them Next. */ + /* One name. It used to read Skip on an unanswered question and Next on + an answered one, while the arrow at the top of the screen said Next + for both — two words for one action, an inch apart. */ )} @@ -1286,6 +1290,9 @@ const timerStarted = timeLeft !== null } } + // Handed in: the block is over, so the rail may say what each question was. + const reviewing = answeredCount >= totalCount && totalCount > 0 + const QuestionRailItem = ({ q, i }) => { const isActive = i === currentIdx const isDone = !!answers[q.id] @@ -1293,10 +1300,11 @@ const timerStarted = timeLeft !== null // Only questions the learner has reached show their text. Previewing one // they have not opened would give away the case before they read it. // - // In exam mode none of them do. A real paper's status rail is a column of - // numbers, and reading the stems of the questions still to come is not - // something the exam being rehearsed would allow. - const seen = !isStudy ? false : seenIndexes.has(i) + // While an exam is being sat, none of them do: a real paper's status rail + // is a column of numbers, and reading the stems still to come is not + // something the exam being rehearsed would allow. Once it is handed in + // there is nothing left to protect, so the review reads like study mode. + const seen = (isStudy || reviewing) ? seenIndexes.has(i) : false const excerpt = seen ? questionStem(q).replace(/\s+/g, ' ').trim() : '' return ( - ))} - - - )} -
- - +
+
+

+ {missing.length + ? 'Warning - This block is incomplete!' + : 'End Block'} +

+ {missing.length ? ( + <> +

Number of unanswered items in this block: {missing.length}

+ {/* Said because it is true, not to talk anybody out of it: + the block can be resumed, and a real paper cannot. */} +

+ You will be able to resume, however we do not recommend this + as this deviates from your exam day experience. +

+ + ) : ( +

All {totalCount} questions are answered.

+ )} +
+ + +
- +
) })()} + {submitError &&
{submitError}
} {progressError &&
{progressError}
} {timeUp && (
-

Time's up

-

- You have run out of time for this block. What you answered has - been handed in and marked. -

+

Time's Up

+

You have run out of time to complete this question block.

+ {/* Closing is the only thing left to do: it is already handed in + and marked, and this lands on the session's analysis. */}
)} + {clockPaused && !isStudy && ( +
+
+

Exam Paused

+ {/* Nothing else. The clock is stopped and the questions are + covered; a paragraph about how real exams work is somebody + else's disclaimer. */} + +
+
+ )} + + {leaving && ( +
+
+

End Session

+

Are you sure you want to end this session?

+
+ + +
+
+
+ )} + {/* Still there? The clock is already stopped by the time this shows — it is not a threat, it is how the time stays honest. */} {(askingStillHere || away) && ( @@ -1906,7 +1936,7 @@ const timerStarted = timeLeft !== null scrolls inside it, rather than the whole page scrolling. */}
+ onClick={() => setLeaving(true)}>Exit session {quizNavigation('bottom')} {/* Reviewing a block before handing it in is an exam idea. A study session has nothing to hand in: it keeps going until every diff --git a/frontend/src/pages/QuizPage.test.jsx b/frontend/src/pages/QuizPage.test.jsx index 0ed9d34..db6a06d 100644 --- a/frontend/src/pages/QuizPage.test.jsx +++ b/frontend/src/pages/QuizPage.test.jsx @@ -241,6 +241,7 @@ describe('quiz player', () => { // point of this test is what is saved, not what is provisionally picked. fireEvent.keyDown(window, { key: 'Enter' }) await userEvent.click(screen.getByRole('button', { name: 'Exit session' })) + await userEvent.click(screen.getByRole('button', { name: 'End Session' })) expect(await screen.findByRole('alert')).toHaveTextContent('Keep this tab open') expect(inCard().getByText('Full first clinical question.')).toBeInTheDocument() failSaving = false @@ -255,6 +256,7 @@ describe('quiz player', () => { // One press. Nothing is lost by leaving — the answers are saved and the // clock pauses — so there is nothing to confirm. await userEvent.click(screen.getByRole('button', { name: 'Exit session' })) + await userEvent.click(screen.getByRole('button', { name: 'End Session' })) // Leaving a session part-way through should land where what you answered // is scored and Resume sits — not on a list of every session you own. expect(await screen.findByText('Submitted results')).toBeInTheDocument() @@ -350,7 +352,7 @@ describe('quiz player', () => { expect(bar.closest('.quiz-layout')).toBeNull() expect(within(bar).getByRole('button', { name: 'Exit session' })).toBeInTheDocument() // Nothing answered yet, so moving on is a skip and the button says so. - expect(within(bar).getByRole('button', { name: /Skip/ })).toBeInTheDocument() + expect(within(bar).getByRole('button', { name: /Next/ })).toBeInTheDocument() }) it('keeps notes with the question, not in a second notepad floating over it', async () => { @@ -461,17 +463,18 @@ describe('quiz player', () => { expect(screen.queryByText(/Full explanation/)).not.toBeInTheDocument() expect(api.get.mock.calls.some(([url]) => url.startsWith('/study-tools/attempts/'))).toBe(false) await userEvent.click(screen.getAllByRole('button', { name: 'Review & Complete' })[0]) - const review = screen.getByRole('dialog', { name: 'End block?' }) + const review = screen.getByRole('dialog', { name: /This block is incomplete/ }) // The sentence names what is missing, not what is done — that is the // decision being confirmed. - expect(within(review).getByText(/1 of 2 questions is unanswered/)).toBeInTheDocument() + expect(within(review).getByText(/Number of unanswered items in this block: 1/)).toBeInTheDocument() // And there is a way back into the block, not only a way out of it. - expect(within(review).getByRole('button', { name: 'Go back and answer them' })).toBeInTheDocument() - await userEvent.click(within(review).getByRole('button', { name: '2' })) + expect(within(review).getByRole('button', { name: 'Remain in Block' })).toBeInTheDocument() + await userEvent.click(within(review).getByRole('button', { name: 'Remain in Block' })) + await userEvent.click(screen.getAllByRole('button', { name: /Next/ })[0]) await findStem('Full second clinical question.') expect(api.post.mock.calls.some(([url]) => url === '/attempts/50/submit')).toBe(false) await userEvent.click(screen.getAllByRole('button', { name: 'Review & Complete' })[0]) - await userEvent.click(within(screen.getByRole('dialog')).getByRole('button', { name: 'End block' })) + await userEvent.click(within(screen.getByRole('dialog')).getByRole('button', { name: 'End Block' })) expect(await screen.findByText('Submitted results')).toBeInTheDocument() expect(api.post).toHaveBeenCalledWith('/attempts/50/submit', expect.objectContaining({ answers: [{ question_id: 1, user_answer: 'First answer' }], @@ -486,7 +489,7 @@ describe('quiz player', () => { expect(screen.getByRole('note')).toHaveTextContent('extrathoracic') fireEvent.keyDown(window, { key: '1' }) await userEvent.click(screen.getAllByRole('button', { name: 'Review & Complete' })[0]) - await userEvent.click(within(screen.getByRole('dialog')).getByRole('button', { name: 'End block' })) + await userEvent.click(within(screen.getByRole('dialog')).getByRole('button', { name: 'End Block' })) expect(await screen.findByText('Submitted results')).toBeInTheDocument() expect(api.post).toHaveBeenCalledWith('/attempts/50/submit', expect.objectContaining({ hints: [1], @@ -503,7 +506,7 @@ describe('quiz player', () => { fireEvent.keyDown(window, { key: '1' }) await userEvent.click(await screen.findByRole('button', { name: 'stridor' })) await userEvent.click(screen.getAllByRole('button', { name: 'Review & Complete' })[0]) - await userEvent.click(within(screen.getByRole('dialog')).getByRole('button', { name: 'End block' })) + await userEvent.click(within(screen.getByRole('dialog')).getByRole('button', { name: 'End Block' })) expect(await screen.findByText('Submitted results')).toBeInTheDocument() expect(api.post).toHaveBeenCalledWith('/attempts/50/submit', expect.objectContaining({ hints: [], @@ -580,7 +583,7 @@ describe('quiz player', () => { return originalPost(url, ...args) }) await userEvent.click(screen.getAllByRole('button', { name: 'Review & Complete' })[0]) - await userEvent.click(within(screen.getByRole('dialog')).getByRole('button', { name: 'End block' })) + await userEvent.click(within(screen.getByRole('dialog')).getByRole('button', { name: 'End Block' })) expect(await screen.findByRole('alert')).toHaveTextContent('Try again safely') expect(api.delete).not.toHaveBeenCalled() await userEvent.click(screen.getByRole('button', { name: 'Retry submission' })) diff --git a/frontend/src/pages/QuizPlayer.css b/frontend/src/pages/QuizPlayer.css index 92222ad..fec220b 100644 --- a/frontend/src/pages/QuizPlayer.css +++ b/frontend/src/pages/QuizPlayer.css @@ -442,17 +442,19 @@ body:has(.quiz-player.is-boxed) .site-footer { display: none; } .quiz-drawer-menu a:hover { background: var(--bg); } /* ── Ending a block ─────────────────────────────────────────────────── - A confirmation with one button is not a confirmation. The way back into - the block is offered first, and the unanswered are numbers you can press - rather than a list to be read. */ -.quiz-end-lead { margin: 0 0 8px; font-size: 1rem; font-weight: 600; } -.quiz-end-note { margin: 0 0 14px; font-size: 0.86rem; line-height: 1.6; color: var(--text-muted); } -.quiz-end-missing { display: flex; flex-wrap: wrap; gap: 8px; margin-bottom: 18px; } -.quiz-end-missing button { - min-width: 42px; min-height: 42px; padding: 0 12px; - font: inherit; font-weight: 700; cursor: pointer; - background: var(--wrong-bg); color: var(--wrong-fg); - border: 1px solid var(--wrong-bd); border-radius: 9px; + Leaving questions behind is the thing worth saying out loud, so an + incomplete block wears the warning in its heading and the button that + ends it is the one that looks like a consequence. */ +.quiz-endblock p { margin: 0 0 10px; font-size: 0.92rem; line-height: 1.6; } +.quiz-endblock.is-warning h2 { + margin: -20px -20px 16px; padding: 12px 16px; + background: var(--wrong-fg); color: #fff; + border-radius: 12px 12px 0 0; font-size: 1rem; } -.quiz-end-missing button:hover { border-color: var(--wrong-fg); } -.quiz-end-actions { display: flex; gap: 10px; flex-wrap: wrap; justify-content: flex-end; } +.quiz-endblock-go { + background: var(--wrong-fg); color: #fff; border: 1px solid var(--wrong-fg); +} +.quiz-endblock-go:hover:not(:disabled) { filter: brightness(1.1); } + +/* Two buttons in a dialog sit side by side, the one that acts first. */ +.quiz-away-actions { display: flex; gap: 10px; flex-wrap: wrap; justify-content: center; }