From 2a3e26624e8889798f20ad3e77c244650c8fcbc5 Mon Sep 17 00:00:00 2001
From: Andy Byers <a.byers@bbk.ac.uk>
Date: Thu, 13 Aug 2026 12:41:57 +0100
Subject: [PATCH] fix: #2970 explain why access was denied instead of a bare
 Permission Denied

Task links opened with the wrong account signed in gave no clue what was
wrong. Each denial now names the account making the request and what it
would take to get in, without ever naming the account a task belongs to.

Assignment lookups are scoped to the account and the journal in the URL,
so a task belonging to somebody else, one on another journal and one that
does not exist are answered identically and cannot be told apart.

Staff no longer pass reviewer_user_for_assignment_required. The views
behind it fetch the assignment again filtered by reviewer=request.user,
so admitting staff only moved the failure into the view, where it was
reported as the review not belonging to them. Staff read reviews through
the editor pages. Two tests that asserted the old behaviour are updated.
---
 src/core/middleware.py                 |   4 +-
 src/security/const.py                  |  45 ++++
 src/security/decorators.py             | 199 +++++++++++-----
 src/security/logic.py                  |  65 ++++++
 src/security/test_security.py          | 312 +++++++++++++++++++++++--
 src/themes/OLH/templates/403.html      |   8 +-
 src/themes/material/templates/403.html |  22 ++
 src/utils/testing/helpers.py           |  32 +++
 8 files changed, 604 insertions(+), 83 deletions(-)
 create mode 100644 src/security/const.py
 create mode 100644 src/themes/material/templates/403.html

diff --git a/src/core/middleware.py b/src/core/middleware.py
index fde5b9be51..061564aa5d 100755
--- a/src/core/middleware.py
+++ b/src/core/middleware.py
@@ -167,7 +167,9 @@ def process_request(request):
                     "general", "maintenance_message", request.journal
                 )
                 request.META["maintenance_mode"] = maintenance_mode_message
-                raise PermissionDenied(request, maintenance_mode_message)
+                # The message is rendered as the 403 page's explanation, so
+                # pass it alone: adding the request made str(exception) a tuple.
+                raise PermissionDenied(maintenance_mode_message.value)
 
 
 class CounterCookieMiddleware(BaseMiddleware):
diff --git a/src/security/const.py b/src/security/const.py
new file mode 100644
index 0000000000..b12b31d41c
--- /dev/null
+++ b/src/security/const.py
@@ -0,0 +1,45 @@
+from django.utils.translation import gettext_lazy as _
+
+from utils.const import EnumContains
+
+
+class AccessDeniedMessages(EnumContains):
+    """Explanations shown to a reader who has been denied access."""
+
+    SIGN_IN_REQUIRED = _("You need to sign in to view this page.")
+
+    # Shown when a page needed a role the account does not hold.
+    WRONG_ACCOUNT_ADVICE = _(
+        "If you have more than one account, sign out and sign in again with "
+        "the correct account."
+    )
+
+    # Shown when a task link was opened by the wrong account.
+    TASK_INVITATION_ADVICE = _(
+        "If you have more than one account, sign out and sign in again using "
+        "the email address that received the task invitation."
+    )
+
+    # These describe the account making the request and never the account the
+    # task belongs to. The same message answers a task that belongs to
+    # somebody else, one on another journal, and one that does not exist, so
+    # that the page cannot be used to discover which ids are real.
+    REVIEW_NOT_ASSIGNED = _("This review is not assigned to %(email)s.")
+    COPYEDIT_NOT_ASSIGNED = _("This copyediting task is not assigned to %(email)s.")
+    PROOFING_NOT_ASSIGNED = _("This proofing task is not assigned to %(email)s.")
+    TYPESETTING_NOT_ASSIGNED = _("This typesetting task is not assigned to %(email)s.")
+    ARTICLE_NOT_ASSOCIATED = _(
+        "This article is not associated with the account %(email)s."
+    )
+
+    REVIEW_LINK_INVALID = _(
+        "This review link is no longer valid. Ask the editor who invited you "
+        "to send a new invitation."
+    )
+    REVIEW_STAGE_PASSED = _(
+        "This article has moved past the review stage, so the review is now "
+        "closed. You do not need to do anything further. Contact the editor "
+        "if you were expecting to complete this review."
+    )
+    TASK_CANCELLED = _("This task was cancelled, so there is nothing left to do.")
+    TASK_COMPLETED = _("You have already completed this task.")
diff --git a/src/security/decorators.py b/src/security/decorators.py
index 36df526d21..f4ab985d65 100755
--- a/src/security/decorators.py
+++ b/src/security/decorators.py
@@ -19,6 +19,8 @@
 from submission import models
 from copyediting import models as copyediting_models
 from proofing import models as proofing_models
+from security import logic as security_logic
+from security.const import AccessDeniedMessages as ADM
 from security.logic import (
     can_edit_file,
     can_see_pii,
@@ -33,6 +35,36 @@
 logger = get_logger(__name__)
 
 
+def review_not_assigned(request):
+    return security_logic.task_not_assigned_message(
+        request.user, ADM.REVIEW_NOT_ASSIGNED
+    )
+
+
+def copyedit_not_assigned(request):
+    return security_logic.task_not_assigned_message(
+        request.user, ADM.COPYEDIT_NOT_ASSIGNED
+    )
+
+
+def proofing_not_assigned(request):
+    return security_logic.task_not_assigned_message(
+        request.user, ADM.PROOFING_NOT_ASSIGNED
+    )
+
+
+def typesetting_not_assigned(request):
+    return security_logic.task_not_assigned_message(
+        request.user, ADM.TYPESETTING_NOT_ASSIGNED
+    )
+
+
+def article_not_yours(request):
+    return security_logic.task_not_assigned_message(
+        request.user, ADM.ARTICLE_NOT_ASSOCIATED
+    )
+
+
 # General role-based security decorators
 
 
@@ -131,7 +163,7 @@ def wrapper(request, *args, **kwargs):
             return func(request, *args, **kwargs)
 
         else:
-            deny_access(request)
+            deny_access(request, required_roles=["editor"])
 
     return wrapper
 
@@ -177,7 +209,10 @@ def wrapper(request, *args, **kwargs):
             return func(request, *args, **kwargs)
 
         else:
-            deny_access(request)
+            deny_access(
+                request,
+                required_roles=["editor", "production manager", "section editor"],
+            )
 
     return wrapper
 
@@ -199,7 +234,9 @@ def wrapper(request, *args, **kwargs):
             return func(request, *args, **kwargs)
 
         else:
-            deny_access(request)
+            deny_access(
+                request, required_roles=["editor", "proofing manager", "section editor"]
+            )
 
     return wrapper
 
@@ -280,7 +317,7 @@ def wrapper(request, *args, **kwargs):
             request.journal
         ):
             return func(request, *args, **kwargs)
-        deny_access(request)
+        deny_access(request, required_roles=["editor", "journal manager"])
 
     return wrapper
 
@@ -316,7 +353,7 @@ def wrapper(request, *args, **kwargs):
                 deny_access(request, "You are not a section editor for this article")
 
         else:
-            deny_access(request)
+            deny_access(request, required_roles=["editor", "journal manager"])
 
     return wrapper
 
@@ -354,7 +391,7 @@ def wrapper(request, *args, **kwargs):
         if request.user.has_an_editor_role(request) or request.user.is_staff:
             return func(request, *args, **kwargs)
         else:
-            deny_access(request)
+            deny_access(request, required_roles=["editor", "section editor"])
 
     return wrapper
 
@@ -397,7 +434,7 @@ def wrapper(request, *args, **kwargs):
         if request.user.is_reviewer(request) or request.user.is_staff:
             return func(request, *args, **kwargs)
         else:
-            deny_access(request)
+            deny_access(request, required_roles=["reviewer"])
 
     return wrapper
 
@@ -414,7 +451,7 @@ def wrapper(request, *args, **kwargs):
         if request.user.is_author(request) or request.user.is_staff:
             return func(request, *args, **kwargs)
         else:
-            deny_access(request)
+            deny_access(request, required_roles=["author"])
 
     return wrapper
 
@@ -431,10 +468,15 @@ def wrapper(request, *args, **kwargs):
         article_id = kwargs["article_id"]
         article = models.Article.get_article(request.journal, "id", article_id)
 
-        if request.user.is_author(request) and article.user_is_author(request.user):
-            return func(request, *args, **kwargs)
-        else:
-            deny_access(request)
+        # A missing or cross-journal id is answered like an article the
+        # reader did not write, so ids cannot be enumerated.
+        if not article or not article.user_is_author(request.user):
+            deny_access(request, article_not_yours(request))
+
+        if not request.user.is_author(request):
+            deny_access(request, required_roles=["author"])
+
+        return func(request, *args, **kwargs)
 
     return wrapper
 
@@ -451,7 +493,7 @@ def wrapper(request, *args, **kwargs):
         if request.user.is_proofreader(request) or request.user.is_proofreader(request):
             return func(request, *args, **kwargs)
         else:
-            deny_access(request)
+            deny_access(request, required_roles=["proofreader"])
 
     return wrapper
 
@@ -468,7 +510,7 @@ def wrapper(request, *args, **kwargs):
         if request.user.is_copyeditor(request) or request.user.is_copyeditor(request):
             return func(request, *args, **kwargs)
         else:
-            deny_access(request)
+            deny_access(request, required_roles=["copyeditor"])
 
     return wrapper
 
@@ -483,18 +525,31 @@ def copyeditor_for_copyedit_required(func):
     @base_check_required
     def wrapper(request, *args, **kwargs):
         copyedit_id = kwargs["copyedit_id"]
-        copyedit = get_object_or_404(
-            copyediting_models.CopyeditAssignment, pk=copyedit_id
-        )
 
-        if (
-            request.user == copyedit.copyeditor
-            and request.user.is_copyeditor(request)
-            or request.user.is_staff
-        ):
-            return func(request, *args, **kwargs)
-        else:
-            deny_access(request)
+        # Staff keep access: the views here do not narrow the assignment
+        # to its copyeditor.
+        if request.user.is_staff:
+            if copyediting_models.CopyeditAssignment.objects.filter(
+                pk=copyedit_id,
+            ).exists():
+                return func(request, *args, **kwargs)
+
+        # Scoped to the user and the journal so that somebody else's
+        # assignment, another journal's, and a missing one cannot be
+        # told apart.
+        copyedit = copyediting_models.CopyeditAssignment.objects.filter(
+            pk=copyedit_id,
+            copyeditor=request.user,
+            article__journal=request.journal,
+        ).first()
+
+        if not copyedit:
+            deny_access(request, copyedit_not_assigned(request))
+
+        if not request.user.is_copyeditor(request):
+            deny_access(request, required_roles=["copyeditor"])
+
+        return func(request, *args, **kwargs)
 
     return wrapper
 
@@ -605,47 +660,40 @@ def wrapper(request, *args, **kwargs):
         if access_code is not None:
             try:
                 assignment = review_models.ReviewAssignment.objects.get(
-                    pk=assignment_id, access_code=access_code
+                    pk=assignment_id,
+                    access_code=access_code,
+                    article__journal=request.journal,
                 )
 
                 if assignment:
                     return func(request, *args, **kwargs)
                 else:
-                    deny_access(request)
+                    deny_access(request, ADM.REVIEW_LINK_INVALID.value)
 
             except review_models.ReviewAssignment.DoesNotExist:
-                deny_access(request)
+                deny_access(request, ADM.REVIEW_LINK_INVALID.value)
 
         if request.user.is_anonymous or not request.user.is_active:
             deny_access(request)
 
         if not request.user.is_reviewer(request):
-            deny_access(request)
+            deny_access(request, required_roles=["reviewer"])
 
         try:
-            if request.user.is_staff:
-                assignment = review_models.ReviewAssignment.objects.get(
-                    pk=assignment_id
-                )
-
-                if assignment:
-                    return func(request, *args, **kwargs)
-                else:
-                    deny_access(request)
-
             assignment = review_models.ReviewAssignment.objects.get(
-                pk=assignment_id, reviewer=request.user
+                pk=assignment_id,
+                reviewer=request.user,
+                article__journal=request.journal,
             )
-
-            if assignment:
-                if assignment.article.stage not in models.REVIEW_ACCESSIBLE_STAGES:
-                    deny_access(request)
-                else:
-                    return func(request, *args, **kwargs)
-            else:
-                deny_access(request)
+        # Somebody else's assignment and a missing one are reported the
+        # same way, so ids cannot be enumerated.
         except review_models.ReviewAssignment.DoesNotExist:
-            deny_access(request)
+            deny_access(request, review_not_assigned(request))
+
+        if assignment.article.stage not in models.REVIEW_ACCESSIBLE_STAGES:
+            deny_access(request, ADM.REVIEW_STAGE_PASSED.value)
+
+        return func(request, *args, **kwargs)
 
     return wrapper
 
@@ -729,7 +777,7 @@ def wrapper(request, *args, **kwargs):
                 return func(request, *args, **kwargs)
 
         else:
-            deny_access(request)
+            deny_access(request, required_roles=["editor", "production manager"])
 
     return wrapper
 
@@ -1114,7 +1162,7 @@ def wrapper(request, *args, **kwargs):
         if request.user.is_typesetter(request) or request.user.is_staff:
             return func(request, *args, **kwargs)
         else:
-            deny_access(request)
+            deny_access(request, required_roles=["typesetter"])
 
     return wrapper
 
@@ -1189,7 +1237,7 @@ def wrapper(request, *args, **kwargs):
             if request.user in article.section_editors():
                 return func(request, *args, **kwargs)
         else:
-            deny_access(request)
+            deny_access(request, required_roles=["editor", "proofing manager"])
 
     return wrapper
 
@@ -1284,7 +1332,18 @@ def wrapper(request, *args, **kwargs):
             return func(request, *args, **kwargs)
 
         else:
-            deny_access(request)
+            task = proofing_models.ProofingTask.objects.filter(
+                pk=kwargs["proofing_task_id"],
+                proofreader=request.user,
+                round__assignment__article__journal=request.journal,
+            ).first()
+
+            if task and task.cancelled:
+                deny_access(request, ADM.TASK_CANCELLED.value)
+            elif task and task.completed:
+                deny_access(request, ADM.TASK_COMPLETED.value)
+            else:
+                deny_access(request, proofing_not_assigned(request))
 
     return wrapper
 
@@ -1311,7 +1370,18 @@ def wrapper(request, *args, **kwargs):
             )
             return func(request, *args, **kwargs)
         except proofing_models.TypesetterProofingTask.DoesNotExist:
-            deny_access(request)
+            task = proofing_models.TypesetterProofingTask.objects.filter(
+                pk=kwargs["typeset_task_id"],
+                typesetter=request.user,
+                proofing_task__round__assignment__article__journal=request.journal,
+            ).first()
+
+            if task and task.cancelled:
+                deny_access(request, ADM.TASK_CANCELLED.value)
+            elif task and task.completed:
+                deny_access(request, ADM.TASK_COMPLETED.value)
+            else:
+                deny_access(request, typesetting_not_assigned(request))
 
     return wrapper
 
@@ -1416,11 +1486,16 @@ def preprint_manager_wrapper(request, *args, **kwargs):
     return preprint_manager_wrapper
 
 
-def deny_access(request, *args, **kwargs):
+def deny_access(request, *args, required_roles=None, **kwargs):
     """Wrapper for raising a PermissionDenied exception
 
-    *args and **kwargs are passed to the PermissionDenied constructor
+    *args and **kwargs are passed to the PermissionDenied constructor.
+
+    Callers that pass neither a message nor required_roles keep the bare
+    denial they have always raised.
+
     :param request: A django HttpRequest
+    :param required_roles: names of the roles this page asks for, if known
     """
     try:
         ident = request.user.email
@@ -1435,6 +1510,16 @@ def deny_access(request, *args, **kwargs):
         ),
     )
 
+    if not args and required_roles:
+        args = (
+            security_logic.access_denied_message(
+                request.user,
+                roles,
+                required_roles=required_roles,
+                journal=getattr(request, "journal", None),
+            ),
+        )
+
     raise PermissionDenied(*args, **kwargs)
 
 
@@ -1453,7 +1538,7 @@ def review_required_wrapper(request, article_id=None, *args, **kwargs):
         article = get_object_or_404(models.Article, pk=article_id)
 
         if article.stage not in models.REVIEW_STAGES:
-            deny_access(request)
+            deny_access(request, ADM.REVIEW_STAGE_PASSED.value)
         else:
             return func(request, article_id, *args, **kwargs)
 
diff --git a/src/security/logic.py b/src/security/logic.py
index 6a490bac56..d0beb3c2dc 100755
--- a/src/security/logic.py
+++ b/src/security/logic.py
@@ -2,12 +2,77 @@
 __author__ = "Martin Paul Eve & Andy Byers"
 __license__ = "AGPL v3"
 __maintainer__ = "Birkbeck Centre for Technology and Publishing"
+from django.utils.translation import gettext as _
+
 from production import models as production_models
 from proofing import models as proofing_models
+from security.const import AccessDeniedMessages as ADM
 from submission import models as submission_models
 from utils import setting_handler
 
 
+def task_not_assigned_message(user, message):
+    """Explains that a task belongs to a different account.
+
+    Deliberately says nothing about who the task does belong to, whether it
+    sits on another journal, or whether it exists at all, so that the message
+    cannot be used to discover any of those.
+
+    :param user: the Account making the request
+    :param message: an AccessDeniedMessages member accepting an email
+    """
+    if not getattr(user, "is_authenticated", False):
+        return str(ADM.SIGN_IN_REQUIRED.value)
+
+    return " ".join(
+        [
+            str(message.value) % {"email": user.email},
+            str(ADM.TASK_INVITATION_ADVICE.value),
+        ]
+    )
+
+
+def access_denied_message(user, roles, required_roles=None, journal=None):
+    """Builds the explanation shown to a user who has been denied access.
+
+    Only ever describes the account making the request, never the account a
+    task belongs to, so that the page cannot be used to discover who is
+    working on what.
+
+    The roles a reader holds are recorded per journal, so that part of the
+    message is left out entirely on press and repository pages rather than
+    describing a journal the reader is not looking at.
+
+    :param user: the Account making the request, or an anonymous user
+    :param roles: the AccountRoles the account holds on this journal
+    :param required_roles: names of the roles the page asks for, if known
+    :param journal: the journal in scope, if the request has one
+    """
+    if not getattr(user, "is_authenticated", False):
+        return str(ADM.SIGN_IN_REQUIRED.value)
+
+    parts = [_("You are signed in as %(email)s.") % {"email": user.email}]
+
+    if journal:
+        role_names = sorted({role.role.name for role in roles})
+        if role_names:
+            parts.append(
+                _("On this journal your account has these roles: %(roles)s.")
+                % {"roles": ", ".join(role_names)}
+            )
+        else:
+            parts.append(_("Your account has no roles on this journal."))
+
+    if required_roles:
+        parts.append(
+            _("This page is for users with the %(required)s role.")
+            % {"required": " or ".join(sorted(str(role) for role in required_roles))}
+        )
+
+    parts.append(ADM.WRONG_ACCOUNT_ADVICE.value)
+    return " ".join(str(part) for part in parts)
+
+
 def can_edit_file(request, user, file_object, article):
     if user.is_anonymous:
         return False
diff --git a/src/security/test_security.py b/src/security/test_security.py
index d24698a0b9..b369d1f573 100644
--- a/src/security/test_security.py
+++ b/src/security/test_security.py
@@ -1430,10 +1430,15 @@ def test_reviewer_user_for_assignment_required_decorator_handles_null_user(self)
             "reviewer_user_for_assignment_required decorator incorrectly handles request.user=None",
         )
 
-    def test_reviewer_user_for_assignment_required_allows_staff(self):
+    def test_reviewer_user_for_assignment_required_blocks_staff(self):
         """
-        Tests that reviewer_user_for_assignment_required allows staff to view the article.
-        :return: None or raises an assertion
+        Tests that reviewer_user_for_assignment_required holds staff to the
+        same check as anyone else.
+
+        The views behind this decorator fetch the assignment again filtered by
+        reviewer=request.user, so admitting staff here only moved the failure
+        into the view, where it was reported as the review not belonging to
+        them. Staff read reviews through the editor pages instead.
         """
         func = Mock()
         decorated_func = decorators.reviewer_user_for_assignment_required(func)
@@ -1441,36 +1446,27 @@ def test_reviewer_user_for_assignment_required_allows_staff(self):
         request = self.prepare_request_with_user(self.admin_user, self.journal_one)
         kwargs = {"assignment_id": self.review_assignment.id}
 
-        decorated_func(request, **kwargs)
+        with self.assertRaises(PermissionDenied):
+            decorated_func(request, **kwargs)
 
-        # test that the callback was called
-        self.assertTrue(
+        self.assertFalse(
             func.called,
-            "reviewer_user_for_assignment_required decorator wrongly prohibits staff from "
-            "accessing an article in production",
+            "reviewer_user_for_assignment_required decorator wrongly allows staff "
+            "to open a review assigned to somebody else",
         )
 
-    def test_reviewer_user_for_assignment_required_allows_staff_regardless_of_stage(
-        self,
-    ):
-        """
-        Tests that reviewer_user_for_assignment_required allows staff to article in review, regardless of stage.
-        :return: None or raises an assertion
-        """
+    def test_reviewer_user_for_assignment_required_tells_staff_it_is_not_theirs(self):
+        """The message must describe the real reason, not the stage."""
         func = Mock()
         decorated_func = decorators.reviewer_user_for_assignment_required(func)
 
         request = self.prepare_request_with_user(self.admin_user, self.journal_one)
         kwargs = {"assignment_id": self.review_assignment.id}
 
-        decorated_func(request, **kwargs)
+        with self.assertRaises(PermissionDenied) as denial:
+            decorated_func(request, **kwargs)
 
-        # test that the callback was called
-        self.assertTrue(
-            func.called,
-            "reviewer_user_for_assignment_required decorator wrongly prohibits staff from "
-            "accessing an article that has been published's production stage",
-        )
+        self.assertIn("not assigned to", str(denial.exception))
 
     def test_reviewer_user_for_assignment_required_blocks_editor(self):
         """
@@ -5522,3 +5518,275 @@ def prepare_request_with_user(user, journal=None, press=None, repository=None):
         request.repository = repository
 
         return request
+
+
+class AccessDeniedMessageTests(TestCase):
+    """Covers the explanations shown on a 403.
+
+    A reader who follows a task link while signed in with the wrong account
+    used to be told only "Permission denied", with nothing to act on.
+    """
+
+    @classmethod
+    def setUpTestData(cls):
+        cls.press = helpers.create_press()
+        cls.journal_one, cls.journal_two = helpers.create_journals()
+        helpers.create_roles(["editor", "author", "reviewer", "proofreader"])
+
+        cls.reviewer = helpers.create_user(
+            "assigned_reviewer@example.org",
+            roles=["reviewer"],
+            journal=cls.journal_one,
+        )
+        cls.reviewer.is_active = True
+        cls.reviewer.save()
+
+        cls.other_reviewer = helpers.create_user(
+            "other_reviewer@example.org",
+            roles=["reviewer"],
+            journal=cls.journal_one,
+        )
+        cls.other_reviewer.is_active = True
+        cls.other_reviewer.save()
+
+        cls.roleless_user = helpers.create_user("no_roles@example.org")
+        cls.roleless_user.is_active = True
+        cls.roleless_user.save()
+
+        cls.review_assignment = helpers.create_review_assignment(
+            journal=cls.journal_one,
+            reviewer=cls.reviewer,
+        )
+        cls.article = cls.review_assignment.article
+
+        cls.task_owner = helpers.create_user(
+            "task_owner@example.org",
+            roles=["author", "reviewer", "proofreader"],
+            journal=cls.journal_one,
+        )
+        cls.task_owner.is_active = True
+        cls.task_owner.save()
+
+        cls.owned_article = helpers.create_submission(
+            owner=cls.task_owner,
+            journal_id=cls.journal_one.pk,
+        )
+        cls.copyedit = helpers.create_copyedit_assignment(
+            article=cls.owned_article,
+            copyeditor=cls.task_owner,
+        )
+        cls.proofing_task = helpers.create_proofing_task(
+            article=cls.owned_article,
+            proofreader=cls.task_owner,
+        )
+        cls.typeset_task = helpers.create_typesetter_proofing_task(
+            proofing_task=cls.proofing_task,
+            typesetter=cls.task_owner,
+        )
+
+        # The same objects again on the other journal, to prove that a valid
+        # id on a journal the reader is not looking at is answered the same
+        # way as one that does not exist.
+        cls.other_journal_article = helpers.create_submission(
+            owner=cls.task_owner,
+            journal_id=cls.journal_two.pk,
+        )
+        cls.other_journal_copyedit = helpers.create_copyedit_assignment(
+            article=cls.other_journal_article,
+            copyeditor=cls.other_reviewer,
+        )
+        cls.other_journal_review = helpers.create_review_assignment(
+            journal=cls.journal_two,
+            article=cls.other_journal_article,
+            reviewer=cls.other_reviewer,
+        )
+
+    MISSING_ID = 99999
+
+    def get_page(self, user, url_name, kwargs):
+        self.client.force_login(user)
+        return self.client.get(
+            reverse(url_name, kwargs=kwargs),
+            SERVER_NAME=self.journal_one.domain,
+        )
+
+    def assert_indistinguishable(self, user, url_name, key, real_id, other_id):
+        """A wrong owner, another journal and a missing id must look alike.
+
+        Any difference between them, including 404 against 403, lets a reader
+        step through the ids to learn which ones exist.
+        """
+        responses = [
+            self.get_page(user, url_name, {key: real_id}),
+            self.get_page(user, url_name, {key: other_id}),
+            self.get_page(user, url_name, {key: self.MISSING_ID}),
+        ]
+
+        statuses = {response.status_code for response in responses}
+        self.assertEqual(
+            statuses,
+            {403},
+            "{} told the reader which ids exist: {}".format(url_name, statuses),
+        )
+
+        bodies = {self.denial_message(response) for response in responses}
+        self.assertEqual(
+            len(bodies),
+            1,
+            "{} worded its denials differently: {}".format(url_name, bodies),
+        )
+
+    def denial_message(self, response):
+        content = response.content.decode()
+        for line in content.split("<p>"):
+            if "is not" in line:
+                return line.split("</p>")[0].strip()
+        return ""
+
+    def test_copyedit_does_not_reveal_which_ids_exist(self):
+        self.assert_indistinguishable(
+            self.other_reviewer,
+            "do_copyedit",
+            "copyedit_id",
+            self.copyedit.pk,
+            self.other_journal_copyedit.pk,
+        )
+
+    def test_review_does_not_reveal_which_ids_exist(self):
+        self.assert_indistinguishable(
+            self.other_reviewer,
+            "do_review",
+            "assignment_id",
+            self.review_assignment.pk,
+            self.other_journal_review.pk,
+        )
+
+    def test_copyedit_denial_does_not_name_the_copyeditor(self):
+        response = self.get_page(
+            self.other_reviewer,
+            "do_copyedit",
+            {"copyedit_id": self.copyedit.pk},
+        )
+        self.assertNotContains(response, self.task_owner.email, status_code=403)
+
+    def test_proofing_denial_does_not_name_the_proofreader(self):
+        response = self.get_page(
+            self.other_reviewer,
+            "do_proofing",
+            {"proofing_task_id": self.proofing_task.pk},
+        )
+        self.assertEqual(response.status_code, 403)
+        self.assertNotContains(response, self.task_owner.email, status_code=403)
+
+    def test_proofing_hides_completion_from_everyone_but_the_owner(self):
+        """Whether a task is finished is the owner's business alone."""
+        self.proofing_task.completed = timezone.now()
+        self.proofing_task.save()
+
+        response = self.get_page(
+            self.other_reviewer,
+            "do_proofing",
+            {"proofing_task_id": self.proofing_task.pk},
+        )
+        self.assertNotContains(response, "already completed", status_code=403)
+
+        response = self.get_page(
+            self.task_owner,
+            "do_proofing",
+            {"proofing_task_id": self.proofing_task.pk},
+        )
+        self.assertContains(response, "already completed", status_code=403)
+
+        self.proofing_task.completed = None
+        self.proofing_task.save()
+
+    def test_author_task_does_not_reveal_which_articles_exist(self):
+        self.assert_indistinguishable(
+            self.other_reviewer,
+            "review_author_view",
+            "article_id",
+            self.owned_article.pk,
+            self.other_journal_article.pk,
+        )
+
+    def get_review_page(self, user):
+        self.client.force_login(user)
+        return self.client.get(
+            reverse(
+                "do_review",
+                kwargs={"assignment_id": self.review_assignment.pk},
+            ),
+            SERVER_NAME=self.journal_one.domain,
+        )
+
+    def test_message_names_the_account_and_its_roles(self):
+        message = decorators.security_logic.access_denied_message(
+            self.reviewer,
+            list(self.reviewer.accountrole_set.filter(journal=self.journal_one)),
+            journal=self.journal_one,
+        )
+        self.assertIn(self.reviewer.email, message)
+        self.assertIn("reviewer", message)
+
+    def test_message_says_when_the_account_has_no_roles(self):
+        message = decorators.security_logic.access_denied_message(
+            self.roleless_user,
+            [],
+            journal=self.journal_one,
+        )
+        self.assertIn("no roles on this journal", message)
+
+    def test_message_omits_journal_roles_when_there_is_no_journal(self):
+        """Press and repository denials must not describe journal roles."""
+        message = decorators.security_logic.access_denied_message(
+            self.roleless_user,
+            [],
+            required_roles=["editor"],
+            journal=None,
+        )
+        self.assertNotIn("journal", message)
+        self.assertIn(self.roleless_user.email, message)
+
+    def test_message_names_the_role_the_page_requires(self):
+        message = decorators.security_logic.access_denied_message(
+            self.roleless_user,
+            [],
+            required_roles=["editor"],
+        )
+        self.assertIn("editor", message)
+
+    def test_message_asks_anonymous_users_to_sign_in(self):
+        message = decorators.security_logic.access_denied_message(
+            AnonymousUser(),
+            [],
+        )
+        self.assertIn("sign in", message)
+
+    @override_settings(URL_CONFIG="domain")
+    def test_review_for_another_account_explains_the_mismatch(self):
+        response = self.get_review_page(self.other_reviewer)
+        self.assertEqual(response.status_code, 403)
+        self.assertContains(
+            response,
+            "not assigned to {}".format(self.other_reviewer.email),
+            status_code=403,
+        )
+
+    @override_settings(URL_CONFIG="domain")
+    def test_review_for_another_account_does_not_name_the_assignee(self):
+        """The 403 must not disclose who is working on the article."""
+        response = self.get_review_page(self.other_reviewer)
+        self.assertNotContains(
+            response,
+            self.reviewer.email,
+            status_code=403,
+        )
+
+    @override_settings(URL_CONFIG="domain")
+    def test_article_past_review_says_the_review_is_closed(self):
+        self.article.stage = submission_models.STAGE_EDITOR_COPYEDITING
+        self.article.save()
+        response = self.get_review_page(self.reviewer)
+        self.assertContains(response, "moved past the review stage", status_code=403)
+        self.article.stage = submission_models.STAGE_UNDER_REVIEW
+        self.article.save()
diff --git a/src/themes/OLH/templates/403.html b/src/themes/OLH/templates/403.html
index 446aa4527f..9b9aba12a4 100644
--- a/src/themes/OLH/templates/403.html
+++ b/src/themes/OLH/templates/403.html
@@ -1,7 +1,7 @@
 {% extends "core/base.html" %}
 {% load i18n %}
 
-{% block page_title %}{% trans "Issue" %} {{ issue }}{% endblock %}
+{% block page_title %}{% trans "Permission Denied" %}{% endblock %}
 
 {% block css %}
 {% endblock %}
@@ -14,8 +14,10 @@ <h1>{% trans 'Maintenance Mode' %}</h1>
             <p>{{ request.META.maintenance_mode.value|safe }}</p>
             {% else %}
             <h1>{% trans "Permission Denied" %}</h1>
-             {% if exception %}
-              <p>{{ exception }}</p>
+            {% if exception %}
+                <p>{{ exception }}</p>
+            {% else %}
+                <p>{% trans "You do not have permission to view this page." %}</p>
             {% endif %}
             {% endif %}
         </div>
diff --git a/src/themes/material/templates/403.html b/src/themes/material/templates/403.html
new file mode 100644
index 0000000000..dc6ee12c0c
--- /dev/null
+++ b/src/themes/material/templates/403.html
@@ -0,0 +1,22 @@
+{% extends "core/base.html" %}
+{% load i18n %}
+
+{% block page_title %}{% trans "Permission Denied" %}{% endblock %}
+
+{% block body %}
+    <section id="content">
+        <div class="row column">
+            {% if request.META.maintenance_mode %}
+                <h1>{% trans "Maintenance Mode" %}</h1>
+                <p>{{ request.META.maintenance_mode.value|safe }}</p>
+            {% else %}
+                <h1>{% trans "Permission Denied" %}</h1>
+                {% if exception %}
+                    <p>{{ exception }}</p>
+                {% else %}
+                    <p>{% trans "You do not have permission to view this page." %}</p>
+                {% endif %}
+            {% endif %}
+        </div>
+    </section>
+{% endblock body %}
diff --git a/src/utils/testing/helpers.py b/src/utils/testing/helpers.py
index 755cb1ce89..0dbc120daf 100755
--- a/src/utils/testing/helpers.py
+++ b/src/utils/testing/helpers.py
@@ -32,6 +32,7 @@
 from submission import models as sm_models
 from review import models as review_models
 from copyediting import models as copyediting_models
+from proofing import models as proofing_models
 from comms import models as comms_models
 from cms import models as cms_models
 from utils import setting_handler, models as utils_models
@@ -898,3 +899,34 @@ def send_contact_message(
         if contact_form.is_valid():
             core_logic.send_contact_message(contact_form, request)
             return utils_models.LogEntry.objects.order_by("-date").first()
+
+
+def create_proofing_task(article, proofreader, manager=None, **kwargs):
+    """Builds the assignment, round and task a proofreader is given."""
+    if not manager:
+        manager = create_editor(article.journal)
+
+    assignment, _created = proofing_models.ProofingAssignment.objects.get_or_create(
+        article=article,
+        defaults={"proofing_manager": manager, "editor": manager},
+    )
+    proofing_round = proofing_models.ProofingRound.objects.create(
+        assignment=assignment,
+        number=assignment.current_proofing_round_number + 1,
+    )
+    return proofing_models.ProofingTask.objects.create(
+        round=proofing_round,
+        proofreader=proofreader,
+        due=timezone.now() + datetime.timedelta(days=3),
+        **kwargs,
+    )
+
+
+def create_typesetter_proofing_task(proofing_task, typesetter, **kwargs):
+    """Builds the corrections task a typesetter is given after proofing."""
+    return proofing_models.TypesetterProofingTask.objects.create(
+        proofing_task=proofing_task,
+        typesetter=typesetter,
+        due=timezone.now() + datetime.timedelta(days=3),
+        **kwargs,
+    )