From 7052db1d38c17980e4815ffaa2cf086cd6b5a9a5 Mon Sep 17 00:00:00 2001 From: Adrian D'Alessandro Date: Fri, 16 Oct 2026 16:15:51 +0100 Subject: [PATCH 1/5] Add django-guardian to requirements --- dev-requirements.txt | 95 ++++++++++++++++++++----------------------- doc-requirements.txt | 56 ++++++++++++------------- prod-requirements.txt | 43 +++++++++----------- pyproject.toml | 3 +- requirements.txt | 41 +++++++++---------- 5 files changed, 112 insertions(+), 126 deletions(-) diff --git a/dev-requirements.txt b/dev-requirements.txt index 35e1b3f7..d26f29dc 100644 --- a/dev-requirements.txt +++ b/dev-requirements.txt @@ -1,13 +1,11 @@ -# -# This file is autogenerated by pip-compile with Python 3.12 -# by the following command: -# -# pip-compile --extra=dev --output-file=dev-requirements.txt -# +# This file was autogenerated by uv via the following command: +# uv pip compile pyproject.toml --extra=dev --output-file=dev-requirements.txt asgiref==3.13.0 # via django +ast-serialize==0.13.1 + # via mypy beautifulsoup4==4.15.0 - # via direct_webapp (pyproject.toml) + # via direct-webapp (pyproject.toml) build==1.6.1 # via pip-tools certifi==2026.10.23 @@ -20,14 +18,12 @@ click==8.5.0 # via # djlint # pip-tools -colorama==0.4.6 - # via djlint confusable-homoglyphs==3.3.1 # via django-registration -coverage[toml]==7.17.0 +coverage==7.17.0 # via pytest-cov crispy-bootstrap5==2026.9 - # via direct_webapp (pyproject.toml) + # via direct-webapp (pyproject.toml) cssbeautifier==2.0.3 # via djlint diff-match-patch==20241021 @@ -36,10 +32,11 @@ distlib==0.4.4 # via virtualenv django==6.1.2 # via + # direct-webapp (pyproject.toml) # crispy-bootstrap5 - # direct_webapp (pyproject.toml) # django-bootstrap5 # django-crispy-forms + # django-guardian # django-import-export # django-multiselectfield # django-registration @@ -47,29 +44,31 @@ django==6.1.2 # django-stubs-ext # django-tables2 django-bootstrap5==26.4 - # via direct_webapp (pyproject.toml) + # via direct-webapp (pyproject.toml) django-crispy-forms==2.7 # via + # direct-webapp (pyproject.toml) # crispy-bootstrap5 - # direct_webapp (pyproject.toml) +django-guardian==3.5.2 + # via direct-webapp (pyproject.toml) django-import-export==4.4.4 - # via direct_webapp (pyproject.toml) + # via direct-webapp (pyproject.toml) django-multiselectfield==1.0.1 - # via direct_webapp (pyproject.toml) + # via direct-webapp (pyproject.toml) django-registration==5.2.1 - # via direct_webapp (pyproject.toml) -django-stubs[compatible-mypy]==6.1.2 + # via direct-webapp (pyproject.toml) +django-stubs==6.1.2 # via - # direct_webapp (pyproject.toml) + # direct-webapp (pyproject.toml) # types-django-import-export django-stubs-ext==6.1.2 # via - # direct_webapp (pyproject.toml) + # direct-webapp (pyproject.toml) # django-stubs django-tables2==3.0.1 - # via direct_webapp (pyproject.toml) + # via direct-webapp (pyproject.toml) djlint==1.47.1 - # via direct_webapp (pyproject.toml) + # via direct-webapp (pyproject.toml) editorconfig==0.17.1 # via # cssbeautifier @@ -79,7 +78,7 @@ filelock==4.2.4 # python-discovery # virtualenv gunicorn==26.2.0 - # via direct_webapp (pyproject.toml) + # via direct-webapp (pyproject.toml) identify==2.6.20 # via pre-commit idna==3.20 @@ -95,39 +94,39 @@ json5==0.17.3 librt==0.16.0 # via mypy markdown==3.11.2 - # via direct_webapp (pyproject.toml) + # via direct-webapp (pyproject.toml) mypy==2.4.1 # via - # direct_webapp (pyproject.toml) + # direct-webapp (pyproject.toml) # django-stubs mypy-extensions==1.1.0 # via mypy nh3==0.3.7 - # via direct_webapp (pyproject.toml) + # via direct-webapp (pyproject.toml) nodeenv==1.11.0 # via pre-commit packaging==26.3 # via # build - # gunicorn # pytest + # virtualenv # wheel pathspec==1.1.1 # via # djlint # mypy +pip==26.2.2 + # via pip-tools pip-tools==7.6.2 - # via direct_webapp (pyproject.toml) + # via direct-webapp (pyproject.toml) platformdirs==4.14.0 - # via - # python-discovery - # virtualenv + # via virtualenv pluggy==1.7.0 # via # pytest # pytest-cov pre-commit==4.7.0 - # via direct_webapp (pyproject.toml) + # via direct-webapp (pyproject.toml) pygments==2.21.0 # via pytest pyproject-hooks==1.3.3 @@ -136,16 +135,16 @@ pyproject-hooks==1.3.3 # pip-tools pytest==9.2.0 # via - # direct_webapp (pyproject.toml) + # direct-webapp (pyproject.toml) # pytest-cov # pytest-django # pytest-mock pytest-cov==7.1.0 - # via direct_webapp (pyproject.toml) + # via direct-webapp (pyproject.toml) pytest-django==4.14.0 - # via direct_webapp (pyproject.toml) + # via direct-webapp (pyproject.toml) pytest-mock==3.17.0 - # via direct_webapp (pyproject.toml) + # via direct-webapp (pyproject.toml) python-discovery==1.6.7 # via virtualenv pyyaml==6.0.3 @@ -155,29 +154,25 @@ pyyaml==6.0.3 regex==2026.9.29 # via djlint requests==2.34.2 - # via direct_webapp (pyproject.toml) + # via direct-webapp (pyproject.toml) ruff==0.17.3 - # via direct_webapp (pyproject.toml) -six==1.17.0 - # via - # cssbeautifier - # jsbeautifier + # via direct-webapp (pyproject.toml) +setuptools==84.2.1 + # via pip-tools soupsieve==3.0.4 # via beautifulsoup4 sqlparse==0.6.0 # via django tablib==3.10.0 # via django-import-export -tqdm==4.70.1 - # via djlint types-django-import-export==4.4.0.20260813 - # via direct_webapp (pyproject.toml) + # via direct-webapp (pyproject.toml) types-markdown==3.11.2.20261029 - # via direct_webapp (pyproject.toml) + # via direct-webapp (pyproject.toml) types-pyyaml==6.0.12.20260906 # via django-stubs types-requests==2.33.0.20261006 - # via direct_webapp (pyproject.toml) + # via direct-webapp (pyproject.toml) typing-extensions==4.16.0 # via # beautifulsoup4 @@ -193,8 +188,4 @@ virtualenv==21.15.1 wheel==0.48.0 # via pip-tools whitenoise==6.12.0 - # via direct_webapp (pyproject.toml) - -# The following packages are considered to be unsafe in a requirements file: -# pip -# setuptools + # via direct-webapp (pyproject.toml) diff --git a/doc-requirements.txt b/doc-requirements.txt index c91b3187..b64748b2 100644 --- a/doc-requirements.txt +++ b/doc-requirements.txt @@ -1,9 +1,5 @@ -# -# This file is autogenerated by pip-compile with Python 3.12 -# by the following command: -# -# pip-compile --extra=doc --output-file=doc-requirements.txt -# +# This file was autogenerated by uv via the following command: +# uv pip compile pyproject.toml --extra=doc --output-file=doc-requirements.txt asgiref==3.13.0 # via django babel==2.18.0 @@ -23,36 +19,39 @@ colorama==0.4.6 confusable-homoglyphs==3.3.1 # via django-registration crispy-bootstrap5==2026.9 - # via direct_webapp (pyproject.toml) + # via direct-webapp (pyproject.toml) diff-match-patch==20241021 # via django-import-export django==6.1.2 # via + # direct-webapp (pyproject.toml) # crispy-bootstrap5 - # direct_webapp (pyproject.toml) # django-bootstrap5 # django-crispy-forms + # django-guardian # django-import-export # django-multiselectfield # django-registration # django-stubs-ext # django-tables2 django-bootstrap5==26.4 - # via direct_webapp (pyproject.toml) + # via direct-webapp (pyproject.toml) django-crispy-forms==2.7 # via + # direct-webapp (pyproject.toml) # crispy-bootstrap5 - # direct_webapp (pyproject.toml) +django-guardian==3.5.2 + # via direct-webapp (pyproject.toml) django-import-export==4.4.4 - # via direct_webapp (pyproject.toml) + # via direct-webapp (pyproject.toml) django-multiselectfield==1.0.1 - # via direct_webapp (pyproject.toml) + # via direct-webapp (pyproject.toml) django-registration==5.2.1 - # via direct_webapp (pyproject.toml) + # via direct-webapp (pyproject.toml) django-stubs-ext==6.1.2 - # via direct_webapp (pyproject.toml) + # via direct-webapp (pyproject.toml) django-tables2==3.0.1 - # via direct_webapp (pyproject.toml) + # via direct-webapp (pyproject.toml) ghp-import==2.1.0 # via # mkdocs @@ -60,7 +59,7 @@ ghp-import==2.1.0 griffelib==2.4.1 # via mkdocstrings-python gunicorn==26.2.0 - # via direct_webapp (pyproject.toml) + # via direct-webapp (pyproject.toml) idna==3.20 # via requests jinja2==3.1.6 @@ -71,7 +70,7 @@ jinja2==3.1.6 # properdocs markdown==3.11.2 # via - # direct_webapp (pyproject.toml) + # direct-webapp (pyproject.toml) # mkdocs # mkdocs-autorefs # mkdocs-material @@ -91,7 +90,7 @@ mergedeep==1.3.4 # mkdocs-get-deps mkdocs==1.6.1 # via - # direct_webapp (pyproject.toml) + # direct-webapp (pyproject.toml) # mkdocs-autorefs # mkdocs-gen-files # mkdocs-literate-nav @@ -103,28 +102,27 @@ mkdocs-autorefs==1.4.5 # mkdocstrings # mkdocstrings-python mkdocs-gen-files==0.6.1 - # via direct_webapp (pyproject.toml) + # via direct-webapp (pyproject.toml) mkdocs-get-deps==0.2.2 # via mkdocs mkdocs-literate-nav==0.6.3 - # via direct_webapp (pyproject.toml) + # via direct-webapp (pyproject.toml) mkdocs-material==9.8.0 - # via direct_webapp (pyproject.toml) + # via direct-webapp (pyproject.toml) mkdocs-material-extensions==1.3.1 # via mkdocs-material mkdocs-section-index==0.3.12 - # via direct_webapp (pyproject.toml) + # via direct-webapp (pyproject.toml) mkdocstrings==1.0.8 # via - # direct_webapp (pyproject.toml) + # direct-webapp (pyproject.toml) # mkdocstrings-python mkdocstrings-python==2.0.10 - # via direct_webapp (pyproject.toml) + # via direct-webapp (pyproject.toml) nh3==0.3.7 - # via direct_webapp (pyproject.toml) + # via direct-webapp (pyproject.toml) packaging==26.3 # via - # gunicorn # mkdocs # properdocs paginate==0.5.7 @@ -163,7 +161,7 @@ pyyaml-env-tag==1.1 # properdocs requests==2.34.2 # via - # direct_webapp (pyproject.toml) + # direct-webapp (pyproject.toml) # mkdocs-material six==1.17.0 # via python-dateutil @@ -173,6 +171,8 @@ tablib==3.10.0 # via django-import-export typing-extensions==4.16.0 # via django-stubs-ext +tzdata==2026.6 + # via python-dateutil urllib3==2.8.0 # via requests watchdog==6.0.0 @@ -180,4 +180,4 @@ watchdog==6.0.0 # mkdocs # properdocs whitenoise==6.12.0 - # via direct_webapp (pyproject.toml) + # via direct-webapp (pyproject.toml) diff --git a/prod-requirements.txt b/prod-requirements.txt index 2efded42..2d0f104a 100644 --- a/prod-requirements.txt +++ b/prod-requirements.txt @@ -1,9 +1,5 @@ -# -# This file is autogenerated by pip-compile with Python 3.12 -# by the following command: -# -# pip-compile --extra=prod --output-file=prod-requirements.txt -# +# This file was autogenerated by uv via the following command: +# uv pip compile pyproject.toml --extra=prod --output-file=prod-requirements.txt asgiref==3.13.0 # via django certifi==2026.10.23 @@ -13,50 +9,51 @@ charset-normalizer==3.5.2 confusable-homoglyphs==3.3.1 # via django-registration crispy-bootstrap5==2026.9 - # via direct_webapp (pyproject.toml) + # via direct-webapp (pyproject.toml) diff-match-patch==20241021 # via django-import-export django==6.1.2 # via + # direct-webapp (pyproject.toml) # crispy-bootstrap5 - # direct_webapp (pyproject.toml) # django-bootstrap5 # django-crispy-forms + # django-guardian # django-import-export # django-multiselectfield # django-registration # django-stubs-ext # django-tables2 django-bootstrap5==26.4 - # via direct_webapp (pyproject.toml) + # via direct-webapp (pyproject.toml) django-crispy-forms==2.7 # via + # direct-webapp (pyproject.toml) # crispy-bootstrap5 - # direct_webapp (pyproject.toml) +django-guardian==3.5.2 + # via direct-webapp (pyproject.toml) django-import-export==4.4.4 - # via direct_webapp (pyproject.toml) + # via direct-webapp (pyproject.toml) django-multiselectfield==1.0.1 - # via direct_webapp (pyproject.toml) + # via direct-webapp (pyproject.toml) django-registration==5.2.1 - # via direct_webapp (pyproject.toml) + # via direct-webapp (pyproject.toml) django-stubs-ext==6.1.2 - # via direct_webapp (pyproject.toml) + # via direct-webapp (pyproject.toml) django-tables2==3.0.1 - # via direct_webapp (pyproject.toml) + # via direct-webapp (pyproject.toml) gunicorn==26.2.0 - # via direct_webapp (pyproject.toml) + # via direct-webapp (pyproject.toml) idna==3.20 # via requests markdown==3.11.2 - # via direct_webapp (pyproject.toml) + # via direct-webapp (pyproject.toml) mysqlclient==2.3.0 - # via direct_webapp (pyproject.toml) + # via direct-webapp (pyproject.toml) nh3==0.3.7 - # via direct_webapp (pyproject.toml) -packaging==26.3 - # via gunicorn + # via direct-webapp (pyproject.toml) requests==2.34.2 - # via direct_webapp (pyproject.toml) + # via direct-webapp (pyproject.toml) sqlparse==0.6.0 # via django tablib==3.10.0 @@ -66,4 +63,4 @@ typing-extensions==4.16.0 urllib3==2.8.0 # via requests whitenoise==6.12.0 - # via direct_webapp (pyproject.toml) + # via direct-webapp (pyproject.toml) diff --git a/pyproject.toml b/pyproject.toml index 0d94b749..0c746893 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -20,7 +20,8 @@ dependencies = [ "django-tables2", "markdown", "nh3", - "requests" + "requests", + "django-guardian", ] [project.optional-dependencies] diff --git a/requirements.txt b/requirements.txt index 1c838794..b8de7e31 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,9 +1,5 @@ -# -# This file is autogenerated by pip-compile with Python 3.12 -# by the following command: -# -# pip-compile -# +# This file was autogenerated by uv via the following command: +# uv pip compile pyproject.toml --output-file=requirements.txt asgiref==3.13.0 # via django certifi==2026.10.23 @@ -13,48 +9,49 @@ charset-normalizer==3.5.2 confusable-homoglyphs==3.3.1 # via django-registration crispy-bootstrap5==2026.9 - # via direct_webapp (pyproject.toml) + # via direct-webapp (pyproject.toml) diff-match-patch==20241021 # via django-import-export django==6.1.2 # via + # direct-webapp (pyproject.toml) # crispy-bootstrap5 - # direct_webapp (pyproject.toml) # django-bootstrap5 # django-crispy-forms + # django-guardian # django-import-export # django-multiselectfield # django-registration # django-stubs-ext # django-tables2 django-bootstrap5==26.4 - # via direct_webapp (pyproject.toml) + # via direct-webapp (pyproject.toml) django-crispy-forms==2.7 # via + # direct-webapp (pyproject.toml) # crispy-bootstrap5 - # direct_webapp (pyproject.toml) +django-guardian==3.5.2 + # via direct-webapp (pyproject.toml) django-import-export==4.4.4 - # via direct_webapp (pyproject.toml) + # via direct-webapp (pyproject.toml) django-multiselectfield==1.0.1 - # via direct_webapp (pyproject.toml) + # via direct-webapp (pyproject.toml) django-registration==5.2.1 - # via direct_webapp (pyproject.toml) + # via direct-webapp (pyproject.toml) django-stubs-ext==6.1.2 - # via direct_webapp (pyproject.toml) + # via direct-webapp (pyproject.toml) django-tables2==3.0.1 - # via direct_webapp (pyproject.toml) + # via direct-webapp (pyproject.toml) gunicorn==26.2.0 - # via direct_webapp (pyproject.toml) + # via direct-webapp (pyproject.toml) idna==3.20 # via requests markdown==3.11.2 - # via direct_webapp (pyproject.toml) + # via direct-webapp (pyproject.toml) nh3==0.3.7 - # via direct_webapp (pyproject.toml) -packaging==26.3 - # via gunicorn + # via direct-webapp (pyproject.toml) requests==2.34.2 - # via direct_webapp (pyproject.toml) + # via direct-webapp (pyproject.toml) sqlparse==0.6.0 # via django tablib==3.10.0 @@ -64,4 +61,4 @@ typing-extensions==4.16.0 urllib3==2.8.0 # via requests whitenoise==6.12.0 - # via direct_webapp (pyproject.toml) + # via direct-webapp (pyproject.toml) From 0f374d45cc3d908ffc7b7af3e29dfc181a44e794 Mon Sep 17 00:00:00 2001 From: Adrian D'Alessandro Date: Fri, 16 Oct 2026 16:49:24 +0100 Subject: [PATCH 2/5] Configure guardian in settings --- .pre-commit-config.yaml | 1 + direct_webapp/settings/settings.py | 5 +++++ 2 files changed, 6 insertions(+) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index bbdffc03..83f0f83e 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -45,6 +45,7 @@ repos: - types-django-import-export - django-multiselectfield - django-tables2 + - django-guardian - repo: https://github.com/igorshubovych/markdownlint-cli rev: v0.50.0 hooks: diff --git a/direct_webapp/settings/settings.py b/direct_webapp/settings/settings.py index b1604956..89afb589 100644 --- a/direct_webapp/settings/settings.py +++ b/direct_webapp/settings/settings.py @@ -128,7 +128,12 @@ "crispy_bootstrap5", "import_export", "django_tables2", + "guardian", ] +AUTHENTICATION_BACKENDS = ( + "django.contrib.auth.backends.ModelBackend", # this is default + "guardian.backends.ObjectPermissionBackend", +) MIDDLEWARE.insert(1, "whitenoise.middleware.WhiteNoiseMiddleware") STATIC_ROOT = BASE_DIR / "staticfiles" AUTH_USER_MODEL = "main.User" From a8508ad8fe4158dfad96816ab704d0034dff17ab Mon Sep 17 00:00:00 2001 From: Adrian D'Alessandro Date: Tue, 20 Oct 2026 13:03:42 +0100 Subject: [PATCH 3/5] First pass at adding guardian perms. Generated-by: GPT-6 Luna --- GLOSSARY.md | 31 +++++++ ...eam-permissions-derived-from-membership.md | 38 ++++++++ main/admin.py | 3 +- main/migrations/0032_team_permissions.py | 87 +++++++++++++++++++ main/models/team_models.py | 14 ++- main/signals.py | 64 +++++++++++++- tests/main/test_team_models.py | 73 ++++++++++++++++ tests/main/test_team_permission_migrations.py | 63 ++++++++++++++ 8 files changed, 369 insertions(+), 4 deletions(-) create mode 100644 docs/adr/0001-team-permissions-derived-from-membership.md create mode 100644 main/migrations/0032_team_permissions.py create mode 100644 tests/main/test_team_permission_migrations.py diff --git a/GLOSSARY.md b/GLOSSARY.md index de30d592..7cfb308a 100644 --- a/GLOSSARY.md +++ b/GLOSSARY.md @@ -56,6 +56,37 @@ A person's collection of skill assessments. A visualisation of a skill profile. _Alias_: Skill wheel. +## Teams + +**Team**: +A group of people who share their skill profiles for collaboration. A team is +visible only to its members. + +**Team member**: +A person who has accepted an invitation to a team. Members can see the team, its +aggregate skill profile, the names of the other members and who the managers +are, but not other members' skill profiles. A member may leave +a team at any time, which ends any access others had to their skill profile +through that team. + +**Team manager**: +A team member who administers the team. Managers can see every member's whole +skill profile, edit or delete the team, invite and remove members, and promote or +demote managers. A team always has at least one manager; whoever creates a team is its +first manager. +_Avoid_: Owner, admin. + +**Team invitation**: +An offer from a team manager for a registered person to join a team. The person +becomes a team member only after accepting it; a pending invitation grants no +access. The invitee may decline it and any manager may withdraw it. A person has +at most one pending invitation per team. + +**Aggregate skill profile**: +A combined view of the skill profiles of a team's members. It is only shown when +enough members contribute that no individual's skill profile can be deduced +from it. + ## Sources Definitions are summarised locally so external access is optional: diff --git a/docs/adr/0001-team-permissions-derived-from-membership.md b/docs/adr/0001-team-permissions-derived-from-membership.md new file mode 100644 index 00000000..a41395bd --- /dev/null +++ b/docs/adr/0001-team-permissions-derived-from-membership.md @@ -0,0 +1,38 @@ +--- +status: accepted +--- + +# Team permissions are derived from membership + +Team membership, including the manager flag, is the single source of truth for +who belongs to a team and who manages it. django-guardian object permissions on +`Team` are a derived projection of membership. They are recomputed whenever a +membership is saved or deleted, and are never edited directly. We still use +guardian because it gives us the standard `has_perm(perm, team)` API, +`get_objects_for_user` querying and admin tooling for future views. Keeping +membership as the authority means a person's role is recorded in only one place. + +## Considered Options + +- **Guardian as the source of truth (drop the manager flag):** rejected. Roles + would become implicit in permission rows, so the question "is Alice a + manager?" turns into a permissions query. It also makes the + at-least-one-manager rule awkward to enforce. +- **Custom auth backend computing permissions from membership:** rejected. There + would be nothing to sync, but we would lose guardian's object querying and + admin integration, and have to rebuild them ourselves. +- **Per-team guardian groups (members and managers):** rejected. Group + membership would duplicate team membership and could drift from it. + +## Consequences + +- Permissions are assigned to individual users and stored in guardian's + direct-foreign-key tables for `Team`. These rows cascade when a team or user is + deleted. The generic-foreign-key tables would leave orphaned rows that could + attach to a later team that reuses the same ID. +- Sync relies on model signals, so code must not change memberships with + queryset `update()`, `bulk_create` or bulk delete, because those bypass the + signals. +- Existing memberships are backfilled by a data migration. +- Views should check `has_perm` on the team rather than reading the manager flag, + so access rules live in one place. diff --git a/main/admin.py b/main/admin.py index 30bad4a4..6177d8a8 100644 --- a/main/admin.py +++ b/main/admin.py @@ -5,6 +5,7 @@ from django.contrib import admin from django.contrib.auth.admin import UserAdmin from django.http import HttpRequest +from guardian.admin import GuardedModelAdmin from import_export.admin import ImportExportModelAdmin from .io_resources import ( @@ -257,7 +258,7 @@ class TeamMembershipInline(admin.TabularInline[TeamMembership, Team]): @admin.register(Team) -class TeamAdmin(admin.ModelAdmin[Team]): +class TeamAdmin(GuardedModelAdmin): """Admin class for the Team model.""" list_display = ("name", "description") diff --git a/main/migrations/0032_team_permissions.py b/main/migrations/0032_team_permissions.py new file mode 100644 index 00000000..4b95b59b --- /dev/null +++ b/main/migrations/0032_team_permissions.py @@ -0,0 +1,87 @@ +"""Add team permissions and backfill existing memberships.""" + +from django.apps.registry import Apps +from django.db import migrations +from django.db.backends.base.schema import BaseDatabaseSchemaEditor + +TEAM_PERMISSION_NAMES = { + "view_team": "Can view team", + "change_team": "Can change team", + "delete_team": "Can delete team", + "view_team_member_profiles": "Can view team member profiles", + "manage_team_members": "Can manage team members", +} + + +def backfill_team_permissions( + apps: Apps, schema_editor: BaseDatabaseSchemaEditor +) -> None: + """Assign existing memberships their derived Guardian permissions.""" + TeamMembership = apps.get_model("main", "TeamMembership") + UserObjectPermission = apps.get_model("guardian", "UserObjectPermission") + ContentType = apps.get_model("contenttypes", "ContentType") + Permission = apps.get_model("auth", "Permission") + database = schema_editor.connection.alias + + content_type, _ = ContentType.objects.using(database).get_or_create( + app_label="main", model="team" + ) + permissions = {} + for codename, name in TEAM_PERMISSION_NAMES.items(): + permissions[codename], _ = Permission.objects.using(database).get_or_create( + content_type=content_type, codename=codename, defaults={"name": name} + ) + + for membership in TeamMembership.objects.using(database).all(): + codenames = ( + TEAM_PERMISSION_NAMES.keys() + if membership.manager + else ("view_team",) + ) + for codename in codenames: + UserObjectPermission.objects.using(database).get_or_create( + user_id=membership.user_id, + permission_id=permissions[codename].pk, + content_type_id=content_type.pk, + object_pk=str(membership.team_id), + ) + + +def remove_team_permissions( + apps: Apps, schema_editor: BaseDatabaseSchemaEditor +) -> None: + """Remove derived team permission assignments when reversing the migration.""" + UserObjectPermission = apps.get_model("guardian", "UserObjectPermission") + database = schema_editor.connection.alias + UserObjectPermission.objects.using(database).filter( + content_type__app_label="main", + content_type__model="team", + permission__codename__in=TEAM_PERMISSION_NAMES, + ).delete() + + +class Migration(migrations.Migration): + dependencies = [ + ( + "guardian", + "0003_remove_groupobjectpermission_guardian_gr_content_ae6aec_idx_and_more", + ), + ("auth", "0012_alter_user_first_name_max_length"), + ("main", "0031_team_teammembership"), + ] + + operations = [ + migrations.AlterModelOptions( + name="team", + options={ + "permissions": ( + ( + "view_team_member_profiles", + "Can view team member profiles", + ), + ("manage_team_members", "Can manage team members"), + ) + }, + ), + migrations.RunPython(backfill_team_permissions, remove_team_permissions), + ] diff --git a/main/models/team_models.py b/main/models/team_models.py index 1c0fca05..0ac93f8c 100644 --- a/main/models/team_models.py +++ b/main/models/team_models.py @@ -9,9 +9,21 @@ class Team(SluggedModel): """Model representing a team of users working together.""" + class Meta: + """Meta options for the Team model.""" + + permissions = ( + ("view_team_member_profiles", "Can view team member profiles"), + ("manage_team_members", "Can manage team members"), + ) + class TeamMembership(models.Model): - """Model for mapping Users to Teams and including a manager flag.""" + """Map users to teams and keep their team permissions synchronized. + + Membership changes must use model save/delete operations; queryset updates + and bulk operations bypass permission synchronization. + """ user = models.ForeignKey( settings.AUTH_USER_MODEL, related_name="teams", on_delete=models.CASCADE diff --git a/main/signals.py b/main/signals.py index 740dca45..3e7abc66 100644 --- a/main/signals.py +++ b/main/signals.py @@ -3,10 +3,13 @@ from typing import Any from django.conf import settings -from django.db.models.signals import post_save +from django.contrib.contenttypes.models import ContentType +from django.db.models.signals import post_delete, post_save, pre_delete from django.dispatch import receiver +from guardian.models import UserObjectPermission +from guardian.shortcuts import assign_perm, remove_perm -from .models import Profile, User +from .models import Profile, Team, TeamMembership, User @receiver(post_save, sender=settings.AUTH_USER_MODEL) @@ -20,3 +23,60 @@ def create_user_profile( """ if created and not raw: Profile.objects.create(user=instance) + + +TEAM_PERMISSIONS = ( + "view_team", + "change_team", + "delete_team", + "view_team_member_profiles", + "manage_team_members", +) +MANAGER_PERMISSIONS = TEAM_PERMISSIONS[1:] + + +def synchronize_team_membership_permissions( + membership: TeamMembership, *, manager: bool +) -> None: + """Recompute a member's permissions for a team from their manager role.""" + for codename in TEAM_PERMISSIONS: + remove_perm(f"main.{codename}", membership.user, membership.team) + + codenames = ("view_team", *MANAGER_PERMISSIONS) if manager else ("view_team",) + for codename in codenames: + assign_perm(f"main.{codename}", membership.user, membership.team) + + +def remove_team_membership_permissions(membership: TeamMembership) -> None: + """Remove every permission derived from a deleted membership.""" + for codename in TEAM_PERMISSIONS: + remove_perm(f"main.{codename}", membership.user, membership.team) + + +@receiver(post_save, sender=TeamMembership) +def team_membership_saved( + sender: type[TeamMembership], + instance: TeamMembership, + raw: bool = False, + **kwargs: Any, +) -> None: + """Synchronize permissions when a membership is saved.""" + if not raw: + synchronize_team_membership_permissions(instance, manager=instance.manager) + + +@receiver(post_delete, sender=TeamMembership) +def team_membership_deleted( + sender: type[TeamMembership], instance: TeamMembership, **kwargs: Any +) -> None: + """Remove team permissions when a membership is deleted.""" + remove_team_membership_permissions(instance) + + +@receiver(pre_delete, sender=Team) +def team_deleted(sender: type[Team], instance: Team, **kwargs: Any) -> None: + """Remove generic Guardian permissions before the team is deleted.""" + content_type = ContentType.objects.get_for_model(instance) + UserObjectPermission.objects.filter( + content_type=content_type, object_pk=str(instance.pk) + ).delete() diff --git a/tests/main/test_team_models.py b/tests/main/test_team_models.py index 6af5c1a1..5567b450 100644 --- a/tests/main/test_team_models.py +++ b/tests/main/test_team_models.py @@ -2,9 +2,17 @@ import pytest from django.core.exceptions import ValidationError +from guardian.shortcuts import get_objects_for_user from main.models import Team, TeamMembership +TEAM_MANAGER_PERMISSIONS = ( + "change_team", + "delete_team", + "view_team_member_profiles", + "manage_team_members", +) + @pytest.mark.django_db def test_team_membership_unique(user) -> None: @@ -23,3 +31,68 @@ def test_team_membership_unique(user) -> None: match=r"Team membership with this User and Team already exists.", ): new_team_membership.full_clean() + + +@pytest.mark.django_db +def test_membership_permissions_follow_manager_role(user, django_user_model) -> None: + """Membership saves and deletes synchronize permissions visible via Django.""" + team = Team.objects.create(name="Team 2", description="The second team") + other_user = django_user_model.objects.create_user(username="other") + membership = TeamMembership.objects.create(user=user, team=team) + + assert user.has_perm("main.view_team", team) + assert not any( + user.has_perm(f"main.{perm}", team) for perm in TEAM_MANAGER_PERMISSIONS + ) + assert not other_user.has_perm("main.view_team", team) + assert team in get_objects_for_user(user, "main.view_team", klass=Team) + + membership.manager = True + membership.save() + user = type(user).objects.get(pk=user.pk) + assert all(user.has_perm(f"main.{perm}", team) for perm in TEAM_MANAGER_PERMISSIONS) + assert team in get_objects_for_user( + user, "main.view_team_member_profiles", klass=Team + ) + + membership.manager = False + membership.save() + user = type(user).objects.get(pk=user.pk) + assert user.has_perm("main.view_team", team) + assert not any( + user.has_perm(f"main.{perm}", team) for perm in TEAM_MANAGER_PERMISSIONS + ) + + membership.delete() + user = type(user).objects.get(pk=user.pk) + assert not user.has_perm("main.view_team", team) + assert team not in get_objects_for_user(user, "main.view_team", klass=Team) + + +@pytest.mark.django_db +def test_team_deletion_does_not_grant_permissions_to_later_team(user) -> None: + """Deleting a team removes its permissions from future teams.""" + team = Team.objects.create(name="Team 3", description="The third team") + TeamMembership.objects.create(user=user, team=team, manager=True) + team.delete() + + later_team = Team.objects.create(name="Team 3 again", description="A new team") + assert not user.has_perm("main.view_team", later_team) + assert not user.has_perm("main.manage_team_members", later_team) + + +@pytest.mark.django_db +def test_user_deletion_does_not_grant_permissions_to_recreated_user(user) -> None: + """Deleting a user removes their team permissions.""" + team = Team.objects.create(name="Team 4", description="The fourth team") + user_model = type(user) + username = user.username + TeamMembership.objects.create(user=user, team=team, manager=True) + user.delete() + + recreated_user = user_model.objects.create_user(username=username) + assert not recreated_user.has_perm("main.view_team", team) + assert not recreated_user.has_perm("main.manage_team_members", team) + assert team not in get_objects_for_user( + recreated_user, "main.view_team", klass=Team + ) diff --git a/tests/main/test_team_permission_migrations.py b/tests/main/test_team_permission_migrations.py new file mode 100644 index 00000000..d220cdbd --- /dev/null +++ b/tests/main/test_team_permission_migrations.py @@ -0,0 +1,63 @@ +"""Tests for team permission migrations.""" + +import pytest +from django.db import connection +from django.db.migrations.executor import MigrationExecutor +from guardian.shortcuts import get_objects_for_user + +from main.models import Team, User + + +@pytest.mark.django_db(transaction=True) +def test_migration_backfills_existing_team_memberships() -> None: + """Backfill member and manager permissions for existing teams.""" + before_permissions = [("main", "0031_team_teammembership")] + after_permissions = [("main", "0032_team_permissions")] + executor = MigrationExecutor(connection) + executor.migrate(before_permissions) + + old_apps = executor.loader.project_state(before_permissions).apps + OldTeam = old_apps.get_model("main", "Team") + OldTeamMembership = old_apps.get_model("main", "TeamMembership") + OldUser = old_apps.get_model("main", "User") + + member = OldUser.objects.create(username="backfill-member") + manager = OldUser.objects.create(username="backfill-manager") + team = OldTeam.objects.create( + name="Backfill team", description="Existing team", slug="backfill-team" + ) + OldTeamMembership.objects.create(user=member, team=team, manager=False) + OldTeamMembership.objects.create(user=manager, team=team, manager=True) + + executor = MigrationExecutor(connection) + executor.migrate(after_permissions) + + member = User.objects.get(username="backfill-member") + manager = User.objects.get(username="backfill-manager") + team = Team.objects.get(slug="backfill-team") + + assert member.has_perm("main.view_team", team) + assert not member.has_perm("main.change_team", team) + assert not member.has_perm("main.view_team_member_profiles", team) + assert team in get_objects_for_user(member, "main.view_team", klass=Team) + assert manager.has_perm("main.view_team", team) + assert manager.has_perm("main.change_team", team) + assert manager.has_perm("main.delete_team", team) + assert manager.has_perm("main.view_team_member_profiles", team) + assert manager.has_perm("main.manage_team_members", team) + + MigrationExecutor(connection).migrate(before_permissions) + + member = User.objects.get(username="backfill-member") + manager = User.objects.get(username="backfill-manager") + team = Team.objects.get(slug="backfill-team") + assert not member.has_perm("main.view_team", team) + assert not manager.has_perm("main.view_team", team) + + MigrationExecutor(connection).migrate(after_permissions) + + member = User.objects.get(username="backfill-member") + manager = User.objects.get(username="backfill-manager") + team = Team.objects.get(slug="backfill-team") + assert member.has_perm("main.view_team", team) + assert manager.has_perm("main.view_team", team) From d2f94bf3ba5d22136d0db8e56ce662518e1c9296 Mon Sep 17 00:00:00 2001 From: Adrian D'Alessandro Date: Mon, 2 Nov 2026 17:31:43 +0000 Subject: [PATCH 4/5] Adjust migration to fit within rebase --- .../{0032_team_permissions.py => 0036_team_permissions.py} | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) rename main/migrations/{0032_team_permissions.py => 0036_team_permissions.py} (96%) diff --git a/main/migrations/0032_team_permissions.py b/main/migrations/0036_team_permissions.py similarity index 96% rename from main/migrations/0032_team_permissions.py rename to main/migrations/0036_team_permissions.py index 4b95b59b..796704e6 100644 --- a/main/migrations/0032_team_permissions.py +++ b/main/migrations/0036_team_permissions.py @@ -66,8 +66,7 @@ class Migration(migrations.Migration): "guardian", "0003_remove_groupobjectpermission_guardian_gr_content_ae6aec_idx_and_more", ), - ("auth", "0012_alter_user_first_name_max_length"), - ("main", "0031_team_teammembership"), + ("main", "0035_rename_userskill_skillprofile"), ] operations = [ From ce09e5224f51e64988322b278ac36221f4bde016 Mon Sep 17 00:00:00 2001 From: Adrian D'Alessandro Date: Wed, 4 Nov 2026 17:12:19 +0000 Subject: [PATCH 5/5] Update migration tests and improve reversal. Generated-by: GPT-6 Luna --- main/migrations/0036_team_permissions.py | 17 +++++---- tests/main/test_team_models.py | 35 +++++++++++++++++-- tests/main/test_team_permission_migrations.py | 27 ++++++++++++-- 3 files changed, 67 insertions(+), 12 deletions(-) diff --git a/main/migrations/0036_team_permissions.py b/main/migrations/0036_team_permissions.py index 796704e6..eeb918e6 100644 --- a/main/migrations/0036_team_permissions.py +++ b/main/migrations/0036_team_permissions.py @@ -50,14 +50,19 @@ def backfill_team_permissions( def remove_team_permissions( apps: Apps, schema_editor: BaseDatabaseSchemaEditor ) -> None: - """Remove derived team permission assignments when reversing the migration.""" + """Remove backfilled permissions for memberships when reversing the migration.""" + TeamMembership = apps.get_model("main", "TeamMembership") UserObjectPermission = apps.get_model("guardian", "UserObjectPermission") database = schema_editor.connection.alias - UserObjectPermission.objects.using(database).filter( - content_type__app_label="main", - content_type__model="team", - permission__codename__in=TEAM_PERMISSION_NAMES, - ).delete() + + for membership in TeamMembership.objects.using(database).all(): + UserObjectPermission.objects.using(database).filter( + user_id=membership.user_id, + content_type__app_label="main", + content_type__model="team", + permission__codename__in=TEAM_PERMISSION_NAMES, + object_pk=str(membership.team_id), + ).delete() class Migration(migrations.Migration): diff --git a/tests/main/test_team_models.py b/tests/main/test_team_models.py index 5567b450..3ed77b86 100644 --- a/tests/main/test_team_models.py +++ b/tests/main/test_team_models.py @@ -51,9 +51,8 @@ def test_membership_permissions_follow_manager_role(user, django_user_model) -> membership.save() user = type(user).objects.get(pk=user.pk) assert all(user.has_perm(f"main.{perm}", team) for perm in TEAM_MANAGER_PERMISSIONS) - assert team in get_objects_for_user( - user, "main.view_team_member_profiles", klass=Team - ) + for permission in TEAM_MANAGER_PERMISSIONS: + assert team in get_objects_for_user(user, f"main.{permission}", klass=Team) membership.manager = False membership.save() @@ -96,3 +95,33 @@ def test_user_deletion_does_not_grant_permissions_to_recreated_user(user) -> Non assert team not in get_objects_for_user( recreated_user, "main.view_team", klass=Team ) + + +@pytest.mark.django_db +def test_admin_membership_role_change_synchronizes_permissions( + admin_client, user +) -> None: + """Changing a membership role in the admin updates team permissions.""" + team = Team.objects.create(name="Admin team", description="Admin test team") + membership = TeamMembership.objects.create(user=user, team=team) + + response = admin_client.post( + f"/admin/main/team/{team.pk}/change/", + { + "name": team.name, + "description": team.description, + "slug": team.slug, + "members-TOTAL_FORMS": "1", + "members-INITIAL_FORMS": "1", + "members-MIN_NUM_FORMS": "0", + "members-MAX_NUM_FORMS": "1000", + "members-0-id": str(membership.pk), + "members-0-user": str(user.pk), + "members-0-manager": "on", + }, + ) + + assert response.status_code == 302 + user.refresh_from_db() + assert user.has_perm("main.change_team", team) + assert user.has_perm("main.manage_team_members", team) diff --git a/tests/main/test_team_permission_migrations.py b/tests/main/test_team_permission_migrations.py index d220cdbd..254d44fd 100644 --- a/tests/main/test_team_permission_migrations.py +++ b/tests/main/test_team_permission_migrations.py @@ -3,7 +3,7 @@ import pytest from django.db import connection from django.db.migrations.executor import MigrationExecutor -from guardian.shortcuts import get_objects_for_user +from guardian.shortcuts import assign_perm, get_objects_for_user from main.models import Team, User @@ -11,11 +11,18 @@ @pytest.mark.django_db(transaction=True) def test_migration_backfills_existing_team_memberships() -> None: """Backfill member and manager permissions for existing teams.""" - before_permissions = [("main", "0031_team_teammembership")] - after_permissions = [("main", "0032_team_permissions")] + before_permissions = [("main", "0035_rename_userskill_skillprofile")] + after_permissions = [("main", "0036_team_permissions")] executor = MigrationExecutor(connection) executor.migrate(before_permissions) + MigrationExecutor(connection).migrate(after_permissions) + empty_apps = ( + MigrationExecutor(connection).loader.project_state(after_permissions).apps + ) + assert empty_apps.get_model("main", "TeamMembership").objects.count() == 0 + MigrationExecutor(connection).migrate(before_permissions) + old_apps = executor.loader.project_state(before_permissions).apps OldTeam = old_apps.get_model("main", "Team") OldTeamMembership = old_apps.get_model("main", "TeamMembership") @@ -36,6 +43,12 @@ def test_migration_backfills_existing_team_memberships() -> None: manager = User.objects.get(username="backfill-manager") team = Team.objects.get(slug="backfill-team") + unrelated = User.objects.create_user(username="unrelated-permission") + unrelated_team = Team.objects.create( + name="Unrelated team", description="Manual team permission" + ) + assign_perm("main.change_team", unrelated, unrelated_team) + assert member.has_perm("main.view_team", team) assert not member.has_perm("main.change_team", team) assert not member.has_perm("main.view_team_member_profiles", team) @@ -45,6 +58,7 @@ def test_migration_backfills_existing_team_memberships() -> None: assert manager.has_perm("main.delete_team", team) assert manager.has_perm("main.view_team_member_profiles", team) assert manager.has_perm("main.manage_team_members", team) + assert unrelated.has_perm("main.change_team", unrelated_team) MigrationExecutor(connection).migrate(before_permissions) @@ -53,6 +67,13 @@ def test_migration_backfills_existing_team_memberships() -> None: team = Team.objects.get(slug="backfill-team") assert not member.has_perm("main.view_team", team) assert not manager.has_perm("main.view_team", team) + assert not manager.has_perm("main.change_team", team) + assert not manager.has_perm("main.delete_team", team) + assert not manager.has_perm("main.view_team_member_profiles", team) + assert not manager.has_perm("main.manage_team_members", team) + unrelated = User.objects.get(username="unrelated-permission") + unrelated_team = Team.objects.get(name="Unrelated team") + assert unrelated.has_perm("main.change_team", unrelated_team) MigrationExecutor(connection).migrate(after_permissions)