Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rename core developer (take 2) #4639

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
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
7 changes: 4 additions & 3 deletions DEVELOPERS.md
Original file line number Diff line number Diff line change
Expand Up @@ -232,17 +232,18 @@ In that situation you can follow the steps below to set up your local copy of th
* The other fields don't matter too much for local development.
1. Register a user account on your local version of job-server by clicking Login
1. Set the `SOCIAL_AUTH_GITHUB_KEY` (aka "Client ID") and `SOCIAL_AUTH_GITHUB_SECRET` environment variables with values from that OAuth application.
1. Give your user the `CoreDeveloper` role by running:
1. Give your user the `StaffAreaAdministrator` role by running:

```sh
> python3 manage.py shell_plus
```

and then running in the `shell_plus` shell:

```
>>> from jobserver.authorization import CoreDeveloper
>>> from jobserver.authorization import StaffAreaAdministrator
>>> user = User.objects.get(username="<your_username>")
>>> user.roles.append(CoreDeveloper)
>>> user.roles.append(StaffAreaAdministrator)
>>> user.save()
```
1. Click on your avatar in the top right-hand corner of the site to access the Staff Area.
Expand Down
9 changes: 7 additions & 2 deletions applications/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,12 @@
from django.utils.decorators import method_decorator
from django.views.generic import CreateView, RedirectView, UpdateView, View

from jobserver.authorization import CoreDeveloper, has_permission, has_role, permissions
from jobserver.authorization import (
StaffAreaAdministrator,
has_permission,
has_role,
permissions,
)
from jobserver.hash_utils import unhash_or_404
from jobserver.slacks import notify_application

Expand Down Expand Up @@ -187,7 +192,7 @@ 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, CoreDeveloper):
if application.approved_at and not has_role(request.user, StaffAreaAdministrator):
messages.warning(
request, "This application has been approved and can no longer be edited"
)
Expand Down
4 changes: 2 additions & 2 deletions interactive/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
from django.utils.text import slugify
from ulid import ULID

from jobserver.authorization import CoreDeveloper, has_role
from jobserver.authorization import StaffAreaAdministrator, has_role
from jobserver.models.common import new_ulid_str


Expand Down Expand Up @@ -173,4 +173,4 @@ def ulid(self):
return ULID.from_str(self.id)

def visible_to(self, user):
return self.created_by == user or has_role(user, CoreDeveloper)
return self.created_by == user or has_role(user, StaffAreaAdministrator)
4 changes: 2 additions & 2 deletions jobserver/authorization/__init__.py
Original file line number Diff line number Diff line change
@@ -1,21 +1,21 @@
from .roles import (
CoreDeveloper,
InteractiveReporter,
OutputChecker,
OutputPublisher,
ProjectCollaborator,
ProjectDeveloper,
StaffAreaAdministrator,
)
from .utils import has_permission, has_role, roles_for, strings_to_roles


__all__ = [
"CoreDeveloper",
"InteractiveReporter",
"OutputChecker",
"OutputPublisher",
"ProjectCollaborator",
"ProjectDeveloper",
"StaffAreaAdministrator",
"has_permission",
"has_role",
"roles_for",
Expand Down
20 changes: 17 additions & 3 deletions jobserver/authorization/parsing.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
from django.utils.module_loading import import_string


EXPECTED_PREFIX = "jobserver.authorization.roles"


def _ensure_role_paths(paths):
"""
Paths must be for one of our Role Classes in jobserver.authorization.roles.
"""
EXPECTED_PREFIX = "jobserver.authorization.roles"

invalid_paths = [p for p in paths if not p.startswith(EXPECTED_PREFIX)]

Expand All @@ -20,8 +22,20 @@ def _ensure_role_paths(paths):

def parse_role(path):
_ensure_role_paths([path])

return import_string(path)
try:
return import_string(path)
except ImportError:
# return skeleton permissionless role with provided path
qualname = path.replace(EXPECTED_PREFIX, "").lstrip(".")
return type(
qualname,
(object,),
{
"__module__": EXPECTED_PREFIX,
"__qualname__": qualname,
"permissions": [],
},
)


def parse_roles(paths):
Expand Down
13 changes: 3 additions & 10 deletions jobserver/authorization/roles.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,20 +20,13 @@
)


class CoreDeveloper:
class StaffAreaAdministrator:
"""
Bennett staff member with administrator access to Job Server.
Jongmassey marked this conversation as resolved.
Show resolved Hide resolved

Note the name is misleading here as this does not imply what we generally mean by
"core developer". We plan to rename this role as part of a more general permissions
revamp.
"""

display_name = "Core Developer"
description = (
"Bennett staff member with administrator access to Job Server. "
"(Not necessarily a developer – this role will be renamed eventually.)"
)
display_name = "Staff Area Administrator"
description = "Bennett staff member with administrator access to Job Server."
models = [
"jobserver.models.user.User",
]
Expand Down
4 changes: 2 additions & 2 deletions jobserver/context_processors.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
from django.urls import reverse
from furl import furl

from .authorization import CoreDeveloper, has_role
from .authorization import StaffAreaAdministrator, has_role
from .nav import NavItem, iter_nav


Expand All @@ -17,7 +17,7 @@ def _is_active(request, prefix):


def can_view_staff_area(request):
can_view_staff_area = has_role(request.user, CoreDeveloper)
can_view_staff_area = has_role(request.user, StaffAreaAdministrator)

return {"user_can_view_staff_area": can_view_staff_area}

Expand Down
6 changes: 3 additions & 3 deletions jobserver/management/commands/create_user.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
from django.db import transaction

from jobserver.authorization.roles import (
CoreDeveloper,
OutputChecker,
StaffAreaAdministrator,
)
from jobserver.models import get_or_create_user

Expand All @@ -28,7 +28,7 @@ def add_arguments(self, parser):
parser.add_argument(
"--core-developer",
action="store_true",
help="Make user global CoreDeveloper",
help="Make user global StaffAreaAdministrator",
)

@transaction.atomic()
Expand All @@ -53,7 +53,7 @@ def handle(self, *args, **options):
if options["output_checker"]:
roles.append(OutputChecker)
if options["core_developer"]:
roles.append(CoreDeveloper)
roles.append(StaffAreaAdministrator)

updated = False
for role in roles:
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
# Generated by Django 5.1.1 on 2024-10-02 11:00
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is mostly hand-written and should have a docstring explaining why it exists, what the purpose and rationale are, and the approach taken. If anyone need to come back to where we are I think that will be very non-obvious.


from django.db import migrations


# We have removed the "real" CoreDeveloper role so make a fake one here
class CoreDeveloper:
Jongmassey marked this conversation as resolved.
Show resolved Hide resolved
__module__ = "jobserver.authorization.roles"


# If we ever want to change StaffAreaAdministrator in future, coupling this migration
# to that class would cause us problems, so use a local fake for this one as well
class StaffAreaAdministrator:
__module__ = "jobserver.authorization.roles"


def change_user_role(apps, from_role, to_role):
User = apps.get_model("jobserver", "User")
users = User.objects.filter(roles__contains=from_role)
for user in users:
for role in user.roles:
# we can't rely on vanilla python comparison of our fakes,
# so use RoleField.get_prep_value() for consistency with Django QuerySet
get_prep_value = User.roles.field.base_field.get_prep_value
from_path = get_prep_value(from_role)
if get_prep_value(role) == from_path:
user.roles.remove(role)
user.roles.append(to_role)
User.objects.bulk_update(users, ["roles"])


def coredeveloper_to_staffadministrator(apps, schema_editor):
change_user_role(apps, CoreDeveloper, StaffAreaAdministrator)


def staffadministrator_to_coredeveloper(apps, schema_editor):
change_user_role(apps, StaffAreaAdministrator, CoreDeveloper)


class Migration(migrations.Migration):
dependencies = [
("jobserver", "0007_remove_user_is_staff_remove_user_is_superuser"),
]

operations = [
migrations.RunPython(
coredeveloper_to_staffadministrator,
reverse_code=staffadministrator_to_coredeveloper,
)
]
9 changes: 7 additions & 2 deletions jobserver/views/job_requests.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,12 @@
from pipeline import load_pipeline

from .. import honeycomb
from ..authorization import CoreDeveloper, has_permission, has_role, permissions
from ..authorization import (
StaffAreaAdministrator,
has_permission,
has_role,
permissions,
)
from ..backends import backends_to_choices
from ..forms import JobRequestCreateForm
from ..github import _get_github_api
Expand Down Expand Up @@ -221,7 +226,7 @@ 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, CoreDeveloper)
honeycomb_can_view_links = has_role(self.request.user, StaffAreaAdministrator)

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

from .. import honeycomb
from ..authorization import CoreDeveloper, has_permission, has_role, permissions
from ..authorization import (
StaffAreaAdministrator,
has_permission,
has_role,
permissions,
)
from ..models import Job, JobRequest


Expand Down Expand Up @@ -59,7 +64,7 @@ def get(self, request, *args, **kwargs):
project=job.job_request.workspace.project,
)

honeycomb_can_view_links = has_role(self.request.user, CoreDeveloper)
honeycomb_can_view_links = has_role(self.request.user, StaffAreaAdministrator)

# 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
9 changes: 7 additions & 2 deletions jobserver/views/workspaces.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,12 @@

from interactive.models import AnalysisRequest

from ..authorization import CoreDeveloper, has_permission, has_role, permissions
from ..authorization import (
StaffAreaAdministrator,
has_permission,
has_role,
permissions,
)
from ..forms import (
WorkspaceArchiveToggleForm,
WorkspaceCreateForm,
Expand Down Expand Up @@ -296,7 +301,7 @@ 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, CoreDeveloper)
honeycomb_can_view_links = has_role(self.request.user, StaffAreaAdministrator)

is_interactive_user = has_permission(
request.user, permissions.analysis_request_create, project=workspace.project
Expand Down
8 changes: 4 additions & 4 deletions staff/views/analysis_requests.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,15 @@

from interactive.models import AnalysisRequest
from interactive.submit import resubmit_analysis
from jobserver.authorization import CoreDeveloper
from jobserver.authorization import StaffAreaAdministrator
from jobserver.authorization.decorators import require_role
from jobserver.github import _get_github_api
from jobserver.models import Project, User

from .qwargs_tools import qwargs


@method_decorator(require_role(CoreDeveloper), name="dispatch")
@method_decorator(require_role(StaffAreaAdministrator), name="dispatch")
class AnalysisRequestDetail(DetailView):
context_object_name = "analysis_request"
model = AnalysisRequest
Expand All @@ -32,7 +32,7 @@ def get_queryset(self):
return super().get_queryset().select_related("created_by", "project")


@method_decorator(require_role(CoreDeveloper), name="dispatch")
@method_decorator(require_role(StaffAreaAdministrator), name="dispatch")
class AnalysisRequestResubmit(View):
get_github_api = staticmethod(_get_github_api)

Expand All @@ -42,7 +42,7 @@ def post(self, request, slug, *args, **kwargs):
return redirect(analysis_request.get_staff_url())


@method_decorator(require_role(CoreDeveloper), name="dispatch")
@method_decorator(require_role(StaffAreaAdministrator), name="dispatch")
class AnalysisRequestList(ListView):
context_object_name = "analysis_request"
model = AnalysisRequest
Expand Down
Loading