diff --git a/backend/app/routers/questions.py b/backend/app/routers/questions.py index 6c000ba..4977ed4 100644 --- a/backend/app/routers/questions.py +++ b/backend/app/routers/questions.py @@ -36,7 +36,8 @@ from app.services.quiz_builder import (bank_query, bank_question_predicate, cate create_saved_test, exam_scope_predicate, generate_test) from app.utils.auth import get_current_user, require_moderator from app.utils.category_grants import (assert_can_manage_category, assert_user_can_manage, - is_question_manager, manageable_categories, question_in_scope, require_question_manager) + is_question_manager, manageable_categories, question_in_scope, question_scope_predicate, + require_question_manager) router = APIRouter() @@ -434,6 +435,18 @@ def get_question_bank( quiz_cache: dict[int, str] = {} cat_cache: dict[int, str] = {} + # Which of these the caller writes rather than sits. This listing used to + # hand every signed-in learner the correct option and the explanation for + # the whole bank — the answer key, one HTTP request away from the questions + # it answers. Editors still get it; everyone else gets the stem. + scope = question_scope_predicate(db, current_user) + if scope is None: + editable = {qu.id for qu in questions} + else: + editable = {row[0] for row in db.query(Question.id).filter( + Question.id.in_([qu.id for qu in questions]), scope).all()} if questions else set() + editable |= {qu.id for qu in questions + if qu.user_id is not None and qu.user_id == current_user.id} link_rows = db.query(QuestionCategoryLink.question_id, QuestionCategoryLink.category_id).filter( QuestionCategoryLink.question_id.in_([qu.id for qu in questions])).all() if questions else [] extra_map: dict[int, list[int]] = {} @@ -461,13 +474,14 @@ def get_question_bank( "question_text": qu.question_text, "question_type": qu.question_type, "options": qu.options, - "correct_answer": qu.correct_answer, - "explanation": qu.explanation, + # The answer side, for the people who write it. + "correct_answer": qu.correct_answer if qu.id in editable else None, + "explanation": qu.explanation if qu.id in editable else None, "image_path": qu.image_path, - "explanation_image_path": qu.explanation_image_path, - "option_explanations": qu.option_explanations, - "key_points": qu.key_points, - "attending_tip": qu.attending_tip, + "explanation_image_path": qu.explanation_image_path if qu.id in editable else None, + "option_explanations": qu.option_explanations if qu.id in editable else None, + "key_points": qu.key_points if qu.id in editable else None, + "attending_tip": qu.attending_tip if qu.id in editable else None, "difficulty": qu.difficulty, "user_id": qu.user_id, "match_source": "semantic" if qu.id in semantic_ids else "keyword", diff --git a/backend/app/utils/quiz_access.py b/backend/app/utils/quiz_access.py index 9dd73f4..50c8325 100644 --- a/backend/app/utils/quiz_access.py +++ b/backend/app/utils/quiz_access.py @@ -51,8 +51,31 @@ def set_quiz_shared(db, quiz, user, shared): return {"id": quiz.id, "is_shared": quiz.is_shared, "is_published": quiz.is_published} +def may_edit_question(db, question, user) -> bool: + """Whether this person writes this question, rather than sits it. + + Three ways in: moderation, authorship, or an editorial grant that covers + where the question is filed. It is what separates reading an answer because + it is your job from reading it because you found the URL. + """ + if user.is_moderator: + return True + if question.user_id is not None and question.user_id == user.id: + return True + from app.utils.category_grants import question_scope_predicate + predicate = question_scope_predicate(db, user) + if predicate is None: + return True + return db.query(Question.id).filter(Question.id == question.id, predicate).first() is not None + + def require_question_access(db, question, user, attempt_id=None, review=False): - """Authorize tutor/answer content or a stem.""" + """Authorize tutor/answer content or a stem. + + `review=True` means answer-side content: the correct option, the + explanation, the picture of the explanation, and the tutor — which is given + the answer and told it may explain it, so it is answer-side too. + """ if question is None: raise HTTPException(404, "Question not found") if attempt_id is not None: @@ -67,5 +90,11 @@ def require_question_access(db, question, user, attempt_id=None, review=False): if user.is_moderator: return if db.query(Question.id).filter(Question.id == question.id, bank_question_predicate(user)).first(): - return + # In the bank, so the stem is theirs to read. The answer beside it is + # not: without an attempt, the only people who see it are the people + # who write it. Being able to reach a question was never the same as + # having earned its answer, and the tutor is an answer read aloud. + if not review or may_edit_question(db, question, user): + return + raise HTTPException(403, "Sit this question to see its answer") raise HTTPException(403, "Question is private or requires an authorized study/review attempt") diff --git a/backend/tests/test_category_grants.py b/backend/tests/test_category_grants.py index 15e78d6..497969e 100644 --- a/backend/tests/test_category_grants.py +++ b/backend/tests/test_category_grants.py @@ -179,6 +179,38 @@ class CategoryGrantTests(unittest.TestCase): self.assertFalse(moderator_summary["scoped"]) self.assertEqual(moderator_summary["total"], 4) + def test_the_bank_listing_hands_the_answer_only_to_whoever_writes_it(self): + """Any signed-in learner could read the whole answer key from /bank. + + The listing is the question manager's, but nothing stopped a learner + calling it, and it returned the correct option, the explanation, the + option explanations and the attending tip for every question in the + bank. The stem is bank content; the answer beside it is not. + """ + self.user = self.outsider + rows = self.client.get("/questions/bank").json()["questions"] + self.assertEqual(len(rows), 4) # every stem is still listed + for row in rows: + self.assertTrue(row["question_text"]) + self.assertIsNone(row["correct_answer"]) + self.assertIsNone(row["explanation"]) + self.assertIsNone(row["attending_tip"]) + self.assertIsNone(row["explanation_image_path"]) + + # An educator sees the answer where their grant reaches, and only there. + self.grant(category_id=1, user_id=3) + self.user = self.outsider + answers = {row["id"]: row["correct_answer"] + for row in self.client.get("/questions/bank").json()["questions"]} + self.assertEqual(answers[1], "yes") # Cardiology + self.assertEqual(answers[2], "yes") # a descendant of it + self.assertIsNone(answers[3]) # Neurology + self.assertIsNone(answers[4]) # filed nowhere + + self.user = self.mod + self.assertTrue(all(row["correct_answer"] == "yes" + for row in self.client.get("/questions/bank").json()["questions"])) + def test_ungranted_user_is_locked_out_of_question_management(self): self.user = self.outsider self.assertEqual(self.client.get("/questions/manage/summary").status_code, 403) diff --git a/backend/tests/test_related_privacy.py b/backend/tests/test_related_privacy.py index f1baab5..45384ab 100644 --- a/backend/tests/test_related_privacy.py +++ b/backend/tests/test_related_privacy.py @@ -82,10 +82,20 @@ class PrivacyTests(unittest.TestCase): self.assertIn(self.chat(999).status_code, (403, 404)) for boundary in (self.quota, self.model, self.similar, self.ai): boundary.assert_not_called() - for qid in (1, 3, 4, 5): - res = self.chat(qid) - self.assertEqual(res.status_code, 200, res.text) - self.assertEqual(self.similar.call_args.args[2].id, 1) + # The tutor is handed the correct answer and told it may explain it, so + # outside a session it is an answer key. Being able to reach a question + # in the bank is not the same as having earned its answer: without an + # attempt it opens for the people who write the question, and nobody + # else — and it refuses before the quota, the model or the AI is asked. + for qid in (1, 4, 5): + self.assertEqual(self.chat(qid).status_code, 403) + for boundary in (self.quota, self.model, self.similar, self.ai): + boundary.assert_not_called() + res = self.chat(3) # the owner's own question + self.assertEqual(res.status_code, 200, res.text) + # The context search is run as the caller, not as whoever wrote it. + self.assertEqual(self.similar.call_args.args[2].id, self.owner.id) + self.assertEqual(self.similar.call_args.args[1].id, 3) self.login(self.mod) self.assertEqual(self.chat(4).status_code, 200) @@ -149,11 +159,15 @@ class PrivacyTests(unittest.TestCase): self.client.headers.clear() self.assertEqual(self.client.get('/uploads/questions/stem-4.png').status_code, 401) self.login(self.owner, cookie=True) - for path in ('stem-1', 'stem-3', 'stem-4', 'answer-1', 'answer-3'): + # An answer image is answer-side: 'answer-3' opens because question 3 is + # this learner's own, and 'answer-1' is checked below because it is not. + for path in ('stem-1', 'stem-3', 'stem-4', 'answer-3'): res = self.client.get(f'/uploads/questions/{path}.png') self.assertEqual(res.status_code, 200, res.text) self.assertEqual(res.headers['cache-control'], 'private, no-store') self.assertEqual(res.headers['vary'], 'Cookie, Authorization') + # Somebody else's answer, with no attempt behind it: refused. + self.assertEqual(self.client.get('/uploads/questions/answer-1.png').status_code, 404) # A derivative may be kept by the browser that asked for it — never by # a shared cache, which in front of access-controlled images is how one # learner is served another's private figure. The original still says diff --git a/backend/tests/test_vision_fallback.py b/backend/tests/test_vision_fallback.py index bab2b52..ff367ea 100644 --- a/backend/tests/test_vision_fallback.py +++ b/backend/tests/test_vision_fallback.py @@ -227,6 +227,10 @@ class TutorFigureTests(unittest.TestCase): # against it", which is the case being tested. patch.object(vision_service, "catalogue", return_value={}).start() vision_service._catalogue, vision_service._catalogue_at = {}, 0.0 + # The tutor is answer-side content, so outside a session it opens only + # for whoever writes the question. What is under test here is what a + # model is shown once it is open, so the caller is an educator. + self.privacy.login(self.privacy.mod) def tearDown(self): self.privacy.tearDown() diff --git a/frontend/src/components/QuestionReadingLinks.jsx b/frontend/src/components/QuestionReadingLinks.jsx index 18b05fc..d2d2f5f 100644 --- a/frontend/src/components/QuestionReadingLinks.jsx +++ b/frontend/src/components/QuestionReadingLinks.jsx @@ -55,17 +55,31 @@ export default function QuestionReadingLinks({ questionId, variant = 'list' }) { ) } + // One deck, however many of its cards are tied here: three lines naming the + // same deck are three doors to the same room. + const decks = [] + for (const card of cards) { + if (!decks.some(deck => deck.deck_id === card.deck_id)) decks.push(card) + } return (
Topic reading
) diff --git a/frontend/src/pages/QuestionManagerPage.jsx b/frontend/src/pages/QuestionManagerPage.jsx index 92c3221..07ba80a 100644 --- a/frontend/src/pages/QuestionManagerPage.jsx +++ b/frontend/src/pages/QuestionManagerPage.jsx @@ -137,6 +137,26 @@ export default function QuestionManagerPage() { // Where the editor sends you back to, filters and page intact. const location = useLocation() + // Nothing to manage. The page is not moderator-only — an educator with a + // grant belongs here — so it cannot be gated in the router; it is gated on + // what the grant tree says. Without one it used to load the whole bank as a + // list of stems with every answer field blanked, which reads as a broken + // page rather than a door that is not yours. + if (scope && !scope.is_moderator && !scope.can_manage_questions) { + return ( +
+

Questions

+
+

Editing the question bank needs an editorial grant.

+

+ Ask an administrator for one, naming the topics you write about. +

+ Back to the bank +
+
+ ) + } + return (
{previewing && ( diff --git a/frontend/src/pages/ResultsPage.jsx b/frontend/src/pages/ResultsPage.jsx index f0cf2eb..2569b52 100644 --- a/frontend/src/pages/ResultsPage.jsx +++ b/frontend/src/pages/ResultsPage.jsx @@ -6,6 +6,7 @@ import { useClaimSessionChrome } from '../context/SessionChrome' import useMediaQuery from '../hooks/useMediaQuery' import QuizTools, { LabValues } from '../components/QuizTools' import FigureStrip from '../components/FigureStrip' +import QuestionReadingLinks from '../components/QuestionReadingLinks' import { optionLetter } from '../utils/options' import { uploadUrl } from '../utils/uploads' import './QuizPlayer.css' @@ -256,6 +257,12 @@ export default function ResultsPage() { )}
)} + + {/* Where to read it up, and the cards written against it. The + player has offered these under the answer for a while; the + review did not, which is the one place a learner is going + through everything they got wrong. */} + )}