Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions applications/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,7 @@
from django.views.generic import CreateView, RedirectView, UpdateView, View

from jobserver.authorization import (
StaffAreaAdministrator,
has_permission,
has_role,
permissions,
)
from jobserver.hash_utils import unhash_or_404
Expand Down Expand Up @@ -192,7 +190,9 @@ def page(request, pk_hash, key):
# check the user can access this application
validate_application_access(request.user, application)

if application.approved_at and not has_role(request.user, StaffAreaAdministrator):
if application.approved_at and not has_permission(
request.user, permissions.staff_area_access
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thought: I've tried to keep the diffs minimal, and easy to understand, because there are a large number of changes across multiple files.

So:

  • sometimes we're using permissions as an import, because we already had that imported, and then accessing staff_area_access on that
  • other times, we're importing staff_area_access directly and using that, which is an exchange for StaffAreaAdministrator

If it's thought preferable to make all the imports using the permissions consistent, perhaps to explicitly mark them as permissions.staff_area_access, that's also fine.

Though, it may be that the permissions get refactored into a class in #5459 subsequently, so I'm not sure it's worth worrying about too much now.

Copy link
Contributor

@mikerkelly mikerkelly Dec 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think either from is okay, personally I find has_permission(user, staff_area_access) or require_permission(staff_area_access) more readable but permissions.staff_area_access has advantages for some IDEs. Consistency is nice but I am not worried about it here.

I would not bother changing here as you say #5459 will probably change again, and maybe we will make future changes in this area.

):
messages.warning(
request, "This application has been approved and can no longer be edited"
)
Expand Down
18 changes: 1 addition & 17 deletions jobserver/authorization/decorators.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
from django.core.exceptions import PermissionDenied

from .permissions import backend_manage
from .utils import has_permission, has_role
from .utils import has_permission


def require_permission(permission):
Expand All @@ -26,20 +26,4 @@ def wrapper_require_role(request, *args, **kwargs):
return decorator_require_permission


def require_role(role):
"""Decorator for views which require a given Role"""

def decorator_require_role(f): # sigh
@wraps(f)
def wrapper_require_role(request, *args, **kwargs):
if not has_role(request.user, role):
raise PermissionDenied

return f(request, *args, **kwargs)

return wrapper_require_role

return decorator_require_role


require_manage_backends = require_permission(backend_manage)
1 change: 1 addition & 0 deletions jobserver/authorization/permissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
repo_sign_off_with_outputs = "repo_sign_off_with_outputs"
snapshot_create = "snapshot_create"
snapshot_publish = "snapshot_publish"
staff_area_access = "staff_area_access"
unreleased_outputs_view = "unreleased_outputs_view"
user_manage = "user_manage"
workspace_archive = "workspace_archive"
Expand Down
2 changes: 2 additions & 0 deletions jobserver/authorization/roles.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
repo_sign_off_with_outputs,
snapshot_create,
snapshot_publish,
staff_area_access,
unreleased_outputs_view,
user_manage,
workspace_archive,
Expand All @@ -33,6 +34,7 @@ class StaffAreaAdministrator:
backend_manage,
org_create,
user_manage,
staff_area_access,
]


Expand Down
7 changes: 5 additions & 2 deletions jobserver/context_processors.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@
from django.urls import reverse
from furl import furl

from .authorization import StaffAreaAdministrator, has_role
from jobserver.authorization import has_permission, permissions

from .models import Backend, SiteAlert
from .nav import NavItem, iter_nav

Expand All @@ -31,7 +32,9 @@ def in_production(request):

def can_view_staff_area(request):
user = getattr(request, "user", None) or AnonymousUser()
return {"user_can_view_staff_area": has_role(user, StaffAreaAdministrator)}
return {
"user_can_view_staff_area": has_permission(user, permissions.staff_area_access)
}


def disable_creating_jobs(request):
Expand Down
6 changes: 3 additions & 3 deletions jobserver/views/job_requests.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,7 @@

from .. import honeycomb
from ..authorization import (
StaffAreaAdministrator,
has_permission,
has_role,
permissions,
)
from ..backends import backends_to_choices
Expand Down Expand Up @@ -321,7 +319,9 @@ def get(self, request, *args, **kwargs):
can_cancel_jobs = job_request.created_by == request.user or has_permission(
request.user, permissions.job_cancel, project=job_request.workspace.project
)
honeycomb_can_view_links = has_role(self.request.user, StaffAreaAdministrator)
honeycomb_can_view_links = has_permission(
self.request.user, permissions.staff_area_access
)

# build up is_missing_updates to define if we've not seen the backend
# running this JobRequest for a while.
Expand Down
6 changes: 3 additions & 3 deletions jobserver/views/jobs.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,7 @@

from .. import honeycomb
from ..authorization import (
StaffAreaAdministrator,
has_permission,
has_role,
permissions,
)
from ..models import Job, JobRequest
Expand Down Expand Up @@ -62,7 +60,9 @@ def get(self, request, *args, **kwargs):
project=job.job_request.workspace.project,
)

honeycomb_can_view_links = has_role(self.request.user, StaffAreaAdministrator)
honeycomb_can_view_links = has_permission(
self.request.user, permissions.staff_area_access
)

# we need all HTML to be in HTML files, so we built this here and make
# use of it in the template rather than looking it up with a templatetag
Expand Down
6 changes: 3 additions & 3 deletions jobserver/views/workspaces.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,7 @@
from django.views.generic import CreateView, FormView, ListView, View

from ..authorization import (
StaffAreaAdministrator,
has_permission,
has_role,
permissions,
)
from ..forms import (
Expand Down Expand Up @@ -201,7 +199,9 @@ def get(self, request, *args, **kwargs):
# Should we show the admin section in the UI?
show_admin = can_archive_workspace or can_toggle_notifications

honeycomb_can_view_links = has_role(self.request.user, StaffAreaAdministrator)
honeycomb_can_view_links = has_permission(
self.request.user, permissions.staff_area_access
)

outputs = self.get_output_permissions(workspace)

Expand Down
16 changes: 8 additions & 8 deletions staff/views/applications.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,15 @@
from applications.models import Application
from applications.wizard import Wizard
from jobserver.actions import projects
from jobserver.authorization import StaffAreaAdministrator
from jobserver.authorization.decorators import require_role
from jobserver.authorization.decorators import require_permission
from jobserver.authorization.permissions import staff_area_access
from jobserver.hash_utils import unhash, unhash_or_404
from jobserver.models import Org, Project, User

from ..forms import ApplicationApproveForm


@method_decorator(require_role(StaffAreaAdministrator), name="dispatch")
@method_decorator(require_permission(staff_area_access), name="dispatch")
class ApplicationApprove(FormView):
form_class = ApplicationApproveForm
model = Application
Expand Down Expand Up @@ -97,7 +97,7 @@ def get_initial(self):
return initial


@method_decorator(require_role(StaffAreaAdministrator), name="dispatch")
@method_decorator(require_permission(staff_area_access), name="dispatch")
class ApplicationDetail(View):
def dispatch(self, request, *args, **kwargs):
self.application = get_object_or_404(
Expand Down Expand Up @@ -144,7 +144,7 @@ def post(self, request, *args, **kwargs):
return redirect("staff:application-detail", self.application.pk_hash)


@method_decorator(require_role(StaffAreaAdministrator), name="dispatch")
@method_decorator(require_permission(staff_area_access), name="dispatch")
class ApplicationEdit(UpdateView):
fields = [
"status",
Expand Down Expand Up @@ -176,7 +176,7 @@ def get_success_url(self):
return self.object.get_staff_url()


@method_decorator(require_role(StaffAreaAdministrator), name="dispatch")
@method_decorator(require_permission(staff_area_access), name="dispatch")
class ApplicationList(ListView):
response_class = TemplateResponse
ordering = "-created_at"
Expand Down Expand Up @@ -232,7 +232,7 @@ def get_queryset(self):
return qs.distinct()


@method_decorator(require_role(StaffAreaAdministrator), name="dispatch")
@method_decorator(require_permission(staff_area_access), name="dispatch")
class ApplicationRemove(View):
def post(self, request, *args, **kwargs):
application = get_object_or_404(
Expand All @@ -255,7 +255,7 @@ def post(self, request, *args, **kwargs):
return redirect("staff:application-list")


@method_decorator(require_role(StaffAreaAdministrator), name="dispatch")
@method_decorator(require_permission(staff_area_access), name="dispatch")
class ApplicationRestore(View):
def post(self, request, *args, **kwargs):
application = get_object_or_404(
Expand Down
6 changes: 3 additions & 3 deletions staff/views/dashboards/copiloting.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@
from django.utils.decorators import method_decorator
from django.views.generic import TemplateView

from jobserver.authorization import StaffAreaAdministrator
from jobserver.authorization.decorators import require_role
from jobserver.authorization.decorators import require_permission
from jobserver.authorization.permissions import staff_area_access
from jobserver.github import GitHubError, _get_github_api
from jobserver.models import Project, ReleaseFile, Repo

Expand Down Expand Up @@ -78,7 +78,7 @@ def build_repos_by_project(projects, get_github_api=_get_github_api):
return {p.pk: [r for r in repos if r["pk"] in p.repo_ids] for p in projects}


@method_decorator(require_role(StaffAreaAdministrator), name="dispatch")
@method_decorator(require_permission(staff_area_access), name="dispatch")
@method_decorator(csp_exempt(), name="dispatch")
class Copiloting(TemplateView):
get_github_api = staticmethod(_get_github_api)
Expand Down
6 changes: 3 additions & 3 deletions staff/views/dashboards/index.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
from django.utils.decorators import method_decorator
from django.views.generic import TemplateView

from jobserver.authorization import StaffAreaAdministrator
from jobserver.authorization.decorators import require_role
from jobserver.authorization.decorators import require_permission
from jobserver.authorization.permissions import staff_area_access


@method_decorator(require_role(StaffAreaAdministrator), name="dispatch")
@method_decorator(require_permission(staff_area_access), name="dispatch")
class DashboardIndex(TemplateView):
template_name = "staff/dashboards/index.html"
6 changes: 3 additions & 3 deletions staff/views/dashboards/projects.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,12 @@
from django.utils.decorators import method_decorator
from django.views.generic import TemplateView

from jobserver.authorization import StaffAreaAdministrator
from jobserver.authorization.decorators import require_role
from jobserver.authorization.decorators import require_permission
from jobserver.authorization.permissions import staff_area_access
from jobserver.models import Org, Project, ReleaseFile


@method_decorator(require_role(StaffAreaAdministrator), name="dispatch")
@method_decorator(require_permission(staff_area_access), name="dispatch")
@method_decorator(csp_exempt(), name="dispatch")
class ProjectsDashboard(TemplateView):
template_name = "staff/dashboards/projects.html"
Expand Down
8 changes: 4 additions & 4 deletions staff/views/dashboards/repos.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,16 +10,16 @@
from django.utils.decorators import method_decorator
from django.views.generic import View

from jobserver.authorization import StaffAreaAdministrator
from jobserver.authorization.decorators import require_role
from jobserver.authorization.decorators import require_permission
from jobserver.authorization.permissions import staff_area_access
from jobserver.github import _get_github_api
from jobserver.models import Project, Repo, Workspace


logger = structlog.get_logger(__name__)


@method_decorator(require_role(StaffAreaAdministrator), name="dispatch")
@method_decorator(require_permission(staff_area_access), name="dispatch")
class PrivateReposDashboard(View):
get_github_api = staticmethod(_get_github_api)

Expand Down Expand Up @@ -147,7 +147,7 @@ def select(repo):
)


@method_decorator(require_role(StaffAreaAdministrator), name="dispatch")
@method_decorator(require_permission(staff_area_access), name="dispatch")
class ReposWithMultipleProjects(View):
@csp_exempt()
def get(self, request, *args, **kwargs):
Expand Down
6 changes: 3 additions & 3 deletions staff/views/index.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@
from django.views.generic import View

from applications.models import Application
from jobserver.authorization import StaffAreaAdministrator
from jobserver.authorization.decorators import require_role
from jobserver.authorization.decorators import require_permission
from jobserver.authorization.permissions import staff_area_access
from jobserver.models import Backend, Org, Project, User, Workspace


Expand Down Expand Up @@ -95,7 +95,7 @@ def get_results(q):
return list(itertools.chain.from_iterable(queries))


@method_decorator(require_role(StaffAreaAdministrator), name="dispatch")
@method_decorator(require_permission(staff_area_access), name="dispatch")
class Index(View):
def get(self, request, *args, **kwargs):
q = self.request.GET.get("q")
Expand Down
16 changes: 9 additions & 7 deletions staff/views/job_requests.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,32 +3,34 @@
from django.utils.decorators import method_decorator
from django.views.generic import DetailView, ListView

from jobserver.authorization import StaffAreaAdministrator
from jobserver.authorization.decorators import require_role
from jobserver.authorization.utils import has_role
from jobserver.authorization.decorators import (
has_permission,
require_permission,
)
from jobserver.authorization.permissions import staff_area_access
from jobserver.models import Backend, JobRequest, Org, Project, User, Workspace
from jobserver.views.job_requests import JobRequestCancel as BaseJobRequestCancel

from .qwargs_tools import qwargs


@method_decorator(require_role(StaffAreaAdministrator), name="dispatch")
@method_decorator(require_permission(staff_area_access), name="dispatch")
class JobRequestCancel(BaseJobRequestCancel):
def user_has_permission_to_cancel(self, request):
return has_role(request.user, StaffAreaAdministrator)
return has_permission(request.user, staff_area_access)

def redirect(self):
return redirect(self.job_request.get_staff_url())


@method_decorator(require_role(StaffAreaAdministrator), name="dispatch")
@method_decorator(require_permission(staff_area_access), name="dispatch")
class JobRequestDetail(DetailView):
context_object_name = "job_request"
model = JobRequest
template_name = "staff/job_request/detail.html"


@method_decorator(require_role(StaffAreaAdministrator), name="dispatch")
@method_decorator(require_permission(staff_area_access), name="dispatch")
class JobRequestList(ListView):
ordering = "-created_at"
paginate_by = 25
Expand Down
Loading