Skip to content

Commit

Permalink
Make role parsing tolerant to unknown roles
Browse files Browse the repository at this point in the history
Where a role path in the db is not found in the current codebase,
create a skeleton permissionless role instead of blowing up.

This means we can remove role names from the python code and legacy entries
in the db will no longer cause adverse effects.

This supports finding of the old roles in (for example) a migration, but leaves
these old unknown roles untouched unless they are explicitly acted on.
  • Loading branch information
Jongmassey committed Oct 4, 2024
1 parent 551c7a8 commit ec33b2d
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 3 deletions.
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: 13 additions & 0 deletions tests/unit/jobserver/authorization/test_parsing.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,3 +34,16 @@ def test_parse_roles_success():
roles = parse_roles(paths)

assert roles == [OutputChecker, ProjectCollaborator]


def test_parse_role_with_legacy_role():
paths = [
"jobserver.authorization.roles.TestRole",
]

roles = parse_roles(paths)

assert len(roles) == 1
assert roles[0].__module__ == "jobserver.authorization.roles"
assert roles[0].__qualname__ == "TestRole"
assert roles[0].permissions == []
19 changes: 19 additions & 0 deletions tests/unit/jobserver/models/test_user.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,15 @@
from django.urls import reverse
from django.utils import timezone

from jobserver.authorization.parsing import parse_roles
from jobserver.authorization.roles import (
InteractiveReporter,
OutputChecker,
ProjectCollaborator,
ProjectDeveloper,
)
from jobserver.models.user import User, get_or_create_user
from jobserver.utils import dotted_path

from ....factories import (
OrgFactory,
Expand Down Expand Up @@ -351,3 +353,20 @@ def test_create_user():
assert user.email == "[email protected]"
# Password is not stored in plaintext.
assert user.password != password


def test_user_legacy_roles_round_trip():
roles = parse_roles(["jobserver.authorization.roles.LegacyRole"]) + [OutputChecker]
user = UserFactory(roles=roles)
user.save()
user.refresh_from_db()

assert {dotted_path(r) for r in user.roles} == {dotted_path(r) for r in roles}

user.roles.append(ProjectCollaborator)
user.save()
user.refresh_from_db()

assert {dotted_path(r) for r in user.roles} == {
dotted_path(r) for r in roles + [ProjectCollaborator]
}

0 comments on commit ec33b2d

Please sign in to comment.