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; }