From 49b4f078e80880a6aea89d287b6abef047c5a1db Mon Sep 17 00:00:00 2001 From: Neil Martinsen-Burrell Date: Mon, 6 Mar 2023 12:03:29 -0600 Subject: [PATCH 1/7] Add UserDomainRole table and helpers --- ...rs_userdomainrole_user_domains_and_more.py | 66 +++++++++++++++++++ src/registrar/models/__init__.py | 3 + src/registrar/models/domain.py | 11 ++-- src/registrar/models/domain_application.py | 19 ++++++ src/registrar/models/user.py | 7 ++ src/registrar/models/user_domain_role.py | 49 ++++++++++++++ src/registrar/templates/home.html | 34 ++++++++++ src/registrar/tests/test_models.py | 20 +++++- src/registrar/views/index.py | 9 +++ 9 files changed, 211 insertions(+), 7 deletions(-) create mode 100644 src/registrar/migrations/0012_remove_domain_owners_userdomainrole_user_domains_and_more.py create mode 100644 src/registrar/models/user_domain_role.py diff --git a/src/registrar/migrations/0012_remove_domain_owners_userdomainrole_user_domains_and_more.py b/src/registrar/migrations/0012_remove_domain_owners_userdomainrole_user_domains_and_more.py new file mode 100644 index 000000000..48f6b7b66 --- /dev/null +++ b/src/registrar/migrations/0012_remove_domain_owners_userdomainrole_user_domains_and_more.py @@ -0,0 +1,66 @@ +# Generated by Django 4.1.6 on 2023-03-06 17:26 + +from django.conf import settings +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + dependencies = [ + ("registrar", "0011_remove_domainapplication_security_email"), + ] + + operations = [ + migrations.RemoveField( + model_name="domain", + name="owners", + ), + migrations.CreateModel( + name="UserDomainRole", + fields=[ + ( + "id", + models.BigAutoField( + auto_created=True, + primary_key=True, + serialize=False, + verbose_name="ID", + ), + ), + ("created_at", models.DateTimeField(auto_now_add=True)), + ("updated_at", models.DateTimeField(auto_now=True)), + ("role", models.TextField(choices=[("admin", "Admin")])), + ( + "domain", + models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, + related_name="permissions", + to="registrar.domain", + ), + ), + ( + "user", + models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, + related_name="permissions", + to=settings.AUTH_USER_MODEL, + ), + ), + ], + ), + migrations.AddField( + model_name="user", + name="domains", + field=models.ManyToManyField( + related_name="users", + through="registrar.UserDomainRole", + to="registrar.domain", + ), + ), + migrations.AddConstraint( + model_name="userdomainrole", + constraint=models.UniqueConstraint( + fields=("user", "domain"), name="unique_user_domain_role" + ), + ), + ] diff --git a/src/registrar/models/__init__.py b/src/registrar/models/__init__.py index 1bb9dde84..6ff2fe4e1 100644 --- a/src/registrar/models/__init__.py +++ b/src/registrar/models/__init__.py @@ -6,6 +6,7 @@ from .domain import Domain from .host_ip import HostIP from .host import Host from .nameserver import Nameserver +from .user_domain_role import UserDomainRole from .user_profile import UserProfile from .user import User from .website import Website @@ -17,6 +18,7 @@ __all__ = [ "HostIP", "Host", "Nameserver", + "UserDomainRole", "UserProfile", "User", "Website", @@ -28,6 +30,7 @@ auditlog.register(Domain) auditlog.register(HostIP) auditlog.register(Host) auditlog.register(Nameserver) +auditlog.register(UserDomainRole) auditlog.register(UserProfile) auditlog.register(User) auditlog.register(Website) diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index eb89cd387..6697e2e64 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -276,9 +276,8 @@ class Domain(TimeStampedModel): help_text="Domain is live in the registry", ) - # TODO: determine the relationship between this field - # and the domain application's `creator` and `submitter` - owners = models.ManyToManyField( - "registrar.User", - help_text="", - ) + # ForeignKey on UserDomainRole creates a "permissions" member for + # all of the user-roles that are in place for this domain + + # ManyToManyField on User creates a "users" member for all of the + # users who have some role on this domain diff --git a/src/registrar/models/domain_application.py b/src/registrar/models/domain_application.py index d836e36db..1d3052fe3 100644 --- a/src/registrar/models/domain_application.py +++ b/src/registrar/models/domain_application.py @@ -502,6 +502,25 @@ class DomainApplication(TimeStampedModel): # This is a side-effect of the state transition self._send_confirmation_email() + @transition(field="status", source=[SUBMITTED, INVESTIGATING], target=APPROVED) + def approve(self): + """Approve an application that has been submitted. + + This has substantial side-effects because it creates another database + object for the approved Domain and makes the user who created the + application into an admin on that domain. + """ + + # create the domain if it doesn't exist + Domain = apps.get_model("registrar.Domain") + created_domain, _ = Domain.objects.get_or_create(name=self.requested_domain) + + # create the permission for the user + UserDomainRole = apps.get_model("registrar.UserDomainRole") + UserDomainRole.objects.get_or_create( + user=self.creator, domain=created_domain, role=UserDomainRole.Roles.ADMIN + ) + # ## Form policies ### # # These methods control what questions need to be answered by applicants diff --git a/src/registrar/models/user.py b/src/registrar/models/user.py index acfd1769e..ed8a015a5 100644 --- a/src/registrar/models/user.py +++ b/src/registrar/models/user.py @@ -1,4 +1,5 @@ from django.contrib.auth.models import AbstractUser +from django.db import models class User(AbstractUser): @@ -7,6 +8,12 @@ class User(AbstractUser): but can be customized later. """ + domains = models.ManyToManyField( + "registrar.Domain", + through="registrar.UserDomainRole", + related_name="users", + ) + def __str__(self): # this info is pulled from Login.gov if self.first_name or self.last_name: diff --git a/src/registrar/models/user_domain_role.py b/src/registrar/models/user_domain_role.py new file mode 100644 index 000000000..617b04518 --- /dev/null +++ b/src/registrar/models/user_domain_role.py @@ -0,0 +1,49 @@ +from django.db import models + +from .utility.time_stamped_model import TimeStampedModel + +class UserDomainRole(TimeStampedModel): + + """This is a linking table that connects a user with a role on a domain.""" + + class Roles(models.TextChoices): + + """The possible roles are listed here. + + Implementation of the named roles for allowing particular operations happens + elsewhere. + """ + + ADMIN = "admin" + + user = models.ForeignKey( + "registrar.User", + null=False, + on_delete=models.CASCADE, # when a user is deleted, their permissions will be too + related_name="permissions", + ) + + domain = models.ForeignKey( + "registrar.Domain", + null=False, + on_delete=models.CASCADE, # when a domain is deleted, permissions are too + related_name="permissions" + ) + + role = models.TextField( + choices=Roles.choices, + null=False, + blank=False, + ) + + def __str__(self): + return "User {} is {} on domain {}".format(self.user, self.role, self.domain) + + class Meta: + constraints = [ + # a user can have only one role on a given domain, that is, there can + # be only a single row with a certain (user, domain) pair. + models.UniqueConstraint( + fields=['user', 'domain'], name='unique_user_domain_role' + ) + ] diff --git a/src/registrar/templates/home.html b/src/registrar/templates/home.html index 0a753d25c..091a36e8d 100644 --- a/src/registrar/templates/home.html +++ b/src/registrar/templates/home.html @@ -16,7 +16,41 @@

Registered domains

+ {% if domains %} + + + + + + + + + + + + {% for domain in domains %} + + + + + + + {% endfor %} + +
Your domain applications
Domain nameDate createdStatusAction
+ {{ domain.name }} + {{ domain.created_time|date }}{{ domain.application_status|title }} + + Edit {{ domain.name }} + +
+
+ {% else %}

You don't have any registered domains yet

+ {% endif %}
diff --git a/src/registrar/tests/test_models.py b/src/registrar/tests/test_models.py index 911b73086..ec609d70f 100644 --- a/src/registrar/tests/test_models.py +++ b/src/registrar/tests/test_models.py @@ -1,7 +1,7 @@ from django.test import TestCase from django.db.utils import IntegrityError -from registrar.models import Contact, DomainApplication, User, Website, Domain +from registrar.models import Contact, DomainApplication, User, Website, Domain, UserDomainRole from unittest import skip import boto3_mocking # type: ignore @@ -139,6 +139,24 @@ class TestDomain(TestCase): d1.activate() +class TestPermissions(TestCase): + + """Test the User-Domain-Role connection.""" + + def test_approval_creates_role(self): + domain, _ = Domain.objects.get_or_create(name="igorville.gov") + user, _ = User.objects.get_or_create() + application = DomainApplication.objects.create(creator=user, + requested_domain=domain) + # skip using the submit method + application.status = DomainApplication.SUBMITTED + application.approve() + + # should be a role for this user + self.assertTrue(UserDomainRole.objects.get(user=user, domain=domain)) + + + @skip("Not implemented yet.") class TestDomainApplicationLifeCycle(TestCase): def test_application_approval(self): diff --git a/src/registrar/views/index.py b/src/registrar/views/index.py index 6d50b3948..c0e2d1ffb 100644 --- a/src/registrar/views/index.py +++ b/src/registrar/views/index.py @@ -1,3 +1,4 @@ +from django.db.models import F from django.shortcuts import render from registrar.models import DomainApplication @@ -9,4 +10,12 @@ def index(request): if request.user.is_authenticated: applications = DomainApplication.objects.filter(creator=request.user) context["domain_applications"] = applications + + domains = request.user.permissions.values( + "role", + name=F("domain__name"), + created_time=F("domain__created_at"), + application_status=F("domain__domain_application__status"), + ) + context["domains"] = domains return render(request, "home.html", context) From b8f3d9bc5ddae689a1558446acf4ccc802d2b51b Mon Sep 17 00:00:00 2001 From: Neil Martinsen-Burrell Date: Thu, 9 Mar 2023 14:53:54 -0600 Subject: [PATCH 2/7] Domain detail view --- src/registrar/config/urls.py | 1 + src/registrar/templates/domain_detail.html | 14 ++++++++++++++ src/registrar/templates/home.html | 4 ++-- src/registrar/views/__init__.py | 1 + src/registrar/views/domain.py | 11 +++++++++++ src/registrar/views/index.py | 1 + 6 files changed, 30 insertions(+), 2 deletions(-) create mode 100644 src/registrar/templates/domain_detail.html create mode 100644 src/registrar/views/domain.py diff --git a/src/registrar/config/urls.py b/src/registrar/config/urls.py index 19bd176e5..c31987fc6 100644 --- a/src/registrar/config/urls.py +++ b/src/registrar/config/urls.py @@ -62,6 +62,7 @@ urlpatterns = [ lambda r: always_404(r, "We forgot to include this link, sorry."), name="todo", ), + path("domain/", views.DomainView.as_view(), name="domain"), ] diff --git a/src/registrar/templates/domain_detail.html b/src/registrar/templates/domain_detail.html new file mode 100644 index 000000000..5d0624209 --- /dev/null +++ b/src/registrar/templates/domain_detail.html @@ -0,0 +1,14 @@ +{% extends "dashboard_base.html" %} + + +{% block title %}Domain {{ domain.name }}{% endblock %} + +{% block content %} +
+
+

{{ domain.name }}

+ +

Active: {% if domain.is_active %}Yes{% else %}No{% endif %}

+
+
+{% endblock %} {# content #} diff --git a/src/registrar/templates/home.html b/src/registrar/templates/home.html index 091a36e8d..f15f1d1a8 100644 --- a/src/registrar/templates/home.html +++ b/src/registrar/templates/home.html @@ -36,8 +36,8 @@ {{ domain.created_time|date }} {{ domain.application_status|title }} - - Edit {{ domain.name }} + + Edit {{ domain.name }} diff --git a/src/registrar/views/__init__.py b/src/registrar/views/__init__.py index 2952d1185..7b6498e3f 100644 --- a/src/registrar/views/__init__.py +++ b/src/registrar/views/__init__.py @@ -1,4 +1,5 @@ from .application import * +from .domain import * from .health import * from .index import * from .profile import * diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py new file mode 100644 index 000000000..1cbc0a603 --- /dev/null +++ b/src/registrar/views/domain.py @@ -0,0 +1,11 @@ +"""View for a single Domain.""" + +from django.views.generic import DetailView + +from registrar.models import Domain + +class DomainView(DetailView): + + model = Domain + template_name = "domain_detail.html" + context_object_name = "domain" diff --git a/src/registrar/views/index.py b/src/registrar/views/index.py index c0e2d1ffb..35a67bceb 100644 --- a/src/registrar/views/index.py +++ b/src/registrar/views/index.py @@ -13,6 +13,7 @@ def index(request): domains = request.user.permissions.values( "role", + pk=F("domain__id"), name=F("domain__name"), created_time=F("domain__created_at"), application_status=F("domain__domain_application__status"), From 36eb7064c2692e8c968a330945fb47364fb4d885 Mon Sep 17 00:00:00 2001 From: Neil Martinsen-Burrell Date: Thu, 9 Mar 2023 15:16:04 -0600 Subject: [PATCH 3/7] Permissions mixin and tests --- src/registrar/tests/test_views.py | 59 ++++++++++++++++++++++++- src/registrar/views/domain.py | 4 +- src/registrar/views/utility/__init__.py | 1 + src/registrar/views/utility/mixins.py | 40 +++++++++++++++++ 4 files changed, 102 insertions(+), 2 deletions(-) create mode 100644 src/registrar/views/utility/mixins.py diff --git a/src/registrar/tests/test_views.py b/src/registrar/tests/test_views.py index b4fc3c1cb..2d77e2a1b 100644 --- a/src/registrar/tests/test_views.py +++ b/src/registrar/tests/test_views.py @@ -9,7 +9,7 @@ from django_webtest import WebTest # type: ignore import boto3_mocking # type: ignore -from registrar.models import DomainApplication, Domain, Contact, Website +from registrar.models import DomainApplication, Domain, Contact, Website, UserDomainRole from registrar.views.application import ApplicationWizard, Step from .common import less_console_noise @@ -76,6 +76,19 @@ class LoggedInTests(TestWithUser): # clean up application.delete() + def test_home_lists_domains(self): + response = self.client.get("/") + self.assertNotContains(response, "igorville.gov") + domain, _ = Domain.objects.get_or_create(name="igorville.gov") + role, _ = UserDomainRole.objects.get_or_create( + user=self.user, domain=domain, role=UserDomainRole.Roles.ADMIN + ) + response = self.client.get("/") + # count = 2 because it is also in screenreader content + self.assertContains(response, "igorville.gov", count=2) + # clean up + role.delete() + def test_whoami_page(self): """User information appears on the whoami page.""" response = self.client.get("/whoami/") @@ -1070,3 +1083,47 @@ class DomainApplicationTests(TestWithUser, WebTest): # self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) # page = self.app.get(url) # self.assertNotContains(page, "VALUE") + + +class TestDomainPermissions(TestWithUser): + def setUp(self): + super().setUp() + self.domain, _ = Domain.objects.get_or_create(name="igorville.gov") + self.role, _ = UserDomainRole.objects.get_or_create( + user=self.user, domain=self.domain, role=UserDomainRole.Roles.ADMIN + ) + + def tearDown(self): + try: + self.domain.delete() + self.role.delete() + except ValueError: # pass if already deleted + pass + super().tearDown() + + def test_not_logged_in(self): + """Not logged in gets a redirect to Login.""" + response = self.client.get(reverse("domain", kwargs={"pk": self.domain.id})) + self.assertEqual(response.status_code, 302) + + def test_no_domain_role(self): + """Logged in but no role gets 403 Forbidden.""" + self.client.force_login(self.user) + self.role.delete() # user no longer has a role on this domain + with less_console_noise(): + response = self.client.get(reverse("domain", kwargs={"pk": self.domain.id})) + self.assertEqual(response.status_code, 403) + + +class TestDomainDetail(TestDomainPermissions, WebTest): + + def setUp(self): + super().setUp() + self.app.set_user(self.user.username) + + def test_domain_detail_link_works(self): + home_page = self.app.get("/") + self.assertContains(home_page, "igorville.gov") + # click the "Edit" link + detail_page = home_page.click("Edit") + self.assertContains(detail_page, "igorville.gov") diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index 1cbc0a603..e2553ce44 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -4,8 +4,10 @@ from django.views.generic import DetailView from registrar.models import Domain -class DomainView(DetailView): +from .utility import DomainPermission + +class DomainView(DomainPermission, DetailView): model = Domain template_name = "domain_detail.html" context_object_name = "domain" diff --git a/src/registrar/views/utility/__init__.py b/src/registrar/views/utility/__init__.py index cf6ac377a..a492b5cb3 100644 --- a/src/registrar/views/utility/__init__.py +++ b/src/registrar/views/utility/__init__.py @@ -1,2 +1,3 @@ from .steps_helper import StepsHelper from .always_404 import always_404 +from .mixins import DomainPermission diff --git a/src/registrar/views/utility/mixins.py b/src/registrar/views/utility/mixins.py new file mode 100644 index 000000000..7600c4447 --- /dev/null +++ b/src/registrar/views/utility/mixins.py @@ -0,0 +1,40 @@ +"""Permissions-related mixin classes.""" + +from django.contrib.auth.mixins import PermissionRequiredMixin + +from registrar.models import UserDomainRole + + +class PermissionsLoginMixin(PermissionRequiredMixin): + + """Mixin that redirects to login page if not logged in, otherwise 403.""" + + def handle_no_permission(self): + self.raise_exception = self.request.user.is_authenticated + return super().handle_no_permission() + + +class DomainPermission(PermissionsLoginMixin): + + """Does the logged-in user have access to this domain?""" + + def has_permission(self): + """Check if this user has access to this domain. + + The user is in self.request.user and the domain needs to be looked + up from the domain's primary key in self.kwargs["pk"] + """ + if not self.request.user.is_authenticated: + return False + + # user needs to have a role on the domain + try: + role = UserDomainRole.objects.get( + user=self.request.user, domain__id=self.kwargs["pk"] + ) + except UserDomainRole.DoesNotExist: + # can't find the role + return False + + # if we need to check more about the nature of role, do it here. + return True From 99425ebc4f341a312bacfe1a838993d368921bdc Mon Sep 17 00:00:00 2001 From: Neil Martinsen-Burrell Date: Thu, 9 Mar 2023 15:17:30 -0600 Subject: [PATCH 4/7] Lint fixes --- src/registrar/models/user_domain_role.py | 7 ++++--- src/registrar/tests/test_models.py | 15 +++++++++++---- src/registrar/tests/test_views.py | 1 - src/registrar/views/utility/mixins.py | 2 +- 4 files changed, 16 insertions(+), 9 deletions(-) diff --git a/src/registrar/models/user_domain_role.py b/src/registrar/models/user_domain_role.py index 617b04518..5a5219543 100644 --- a/src/registrar/models/user_domain_role.py +++ b/src/registrar/models/user_domain_role.py @@ -2,6 +2,7 @@ from django.db import models from .utility.time_stamped_model import TimeStampedModel + class UserDomainRole(TimeStampedModel): """This is a linking table that connects a user with a role on a domain.""" @@ -19,7 +20,7 @@ class UserDomainRole(TimeStampedModel): user = models.ForeignKey( "registrar.User", null=False, - on_delete=models.CASCADE, # when a user is deleted, their permissions will be too + on_delete=models.CASCADE, # when a user is deleted, permissions are too related_name="permissions", ) @@ -27,7 +28,7 @@ class UserDomainRole(TimeStampedModel): "registrar.Domain", null=False, on_delete=models.CASCADE, # when a domain is deleted, permissions are too - related_name="permissions" + related_name="permissions", ) role = models.TextField( @@ -44,6 +45,6 @@ class UserDomainRole(TimeStampedModel): # a user can have only one role on a given domain, that is, there can # be only a single row with a certain (user, domain) pair. models.UniqueConstraint( - fields=['user', 'domain'], name='unique_user_domain_role' + fields=["user", "domain"], name="unique_user_domain_role" ) ] diff --git a/src/registrar/tests/test_models.py b/src/registrar/tests/test_models.py index ec609d70f..ef9659cd6 100644 --- a/src/registrar/tests/test_models.py +++ b/src/registrar/tests/test_models.py @@ -1,7 +1,14 @@ from django.test import TestCase from django.db.utils import IntegrityError -from registrar.models import Contact, DomainApplication, User, Website, Domain, UserDomainRole +from registrar.models import ( + Contact, + DomainApplication, + User, + Website, + Domain, + UserDomainRole, +) from unittest import skip import boto3_mocking # type: ignore @@ -146,8 +153,9 @@ class TestPermissions(TestCase): def test_approval_creates_role(self): domain, _ = Domain.objects.get_or_create(name="igorville.gov") user, _ = User.objects.get_or_create() - application = DomainApplication.objects.create(creator=user, - requested_domain=domain) + application = DomainApplication.objects.create( + creator=user, requested_domain=domain + ) # skip using the submit method application.status = DomainApplication.SUBMITTED application.approve() @@ -156,7 +164,6 @@ class TestPermissions(TestCase): self.assertTrue(UserDomainRole.objects.get(user=user, domain=domain)) - @skip("Not implemented yet.") class TestDomainApplicationLifeCycle(TestCase): def test_application_approval(self): diff --git a/src/registrar/tests/test_views.py b/src/registrar/tests/test_views.py index 2d77e2a1b..637044e8b 100644 --- a/src/registrar/tests/test_views.py +++ b/src/registrar/tests/test_views.py @@ -1116,7 +1116,6 @@ class TestDomainPermissions(TestWithUser): class TestDomainDetail(TestDomainPermissions, WebTest): - def setUp(self): super().setUp() self.app.set_user(self.user.username) diff --git a/src/registrar/views/utility/mixins.py b/src/registrar/views/utility/mixins.py index 7600c4447..a8b347181 100644 --- a/src/registrar/views/utility/mixins.py +++ b/src/registrar/views/utility/mixins.py @@ -29,7 +29,7 @@ class DomainPermission(PermissionsLoginMixin): # user needs to have a role on the domain try: - role = UserDomainRole.objects.get( + UserDomainRole.objects.get( user=self.request.user, domain__id=self.kwargs["pk"] ) except UserDomainRole.DoesNotExist: From 78500ef74cadaf2e8a2929c9e4684e51e4cf77cc Mon Sep 17 00:00:00 2001 From: Neil Martinsen-Burrell Date: Fri, 17 Mar 2023 13:44:13 -0500 Subject: [PATCH 5/7] Review feedback: exists and better test Signed-off-by: Neil Martinsen-Burrell --- src/registrar/tests/test_views.py | 2 +- src/registrar/views/utility/mixins.py | 9 +++------ 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/src/registrar/tests/test_views.py b/src/registrar/tests/test_views.py index dac35b194..22f2e2ac9 100644 --- a/src/registrar/tests/test_views.py +++ b/src/registrar/tests/test_views.py @@ -78,8 +78,8 @@ class LoggedInTests(TestWithUser): def test_home_lists_domains(self): response = self.client.get("/") - self.assertNotContains(response, "igorville.gov") domain, _ = Domain.objects.get_or_create(name="igorville.gov") + self.assertNotContains(response, "igorville.gov") role, _ = UserDomainRole.objects.get_or_create( user=self.user, domain=domain, role=UserDomainRole.Roles.ADMIN ) diff --git a/src/registrar/views/utility/mixins.py b/src/registrar/views/utility/mixins.py index a8b347181..71129a1f6 100644 --- a/src/registrar/views/utility/mixins.py +++ b/src/registrar/views/utility/mixins.py @@ -28,12 +28,9 @@ class DomainPermission(PermissionsLoginMixin): return False # user needs to have a role on the domain - try: - UserDomainRole.objects.get( - user=self.request.user, domain__id=self.kwargs["pk"] - ) - except UserDomainRole.DoesNotExist: - # can't find the role + if not UserDomainRole.objects.filter( + user=self.request.user, domain__id=self.kwargs["pk"] + ).exists(): return False # if we need to check more about the nature of role, do it here. From 23eb9d448b27c37b29fd6d09ba1730ef2afc929d Mon Sep 17 00:00:00 2001 From: Neil Martinsen-Burrell Date: Fri, 17 Mar 2023 13:47:25 -0500 Subject: [PATCH 6/7] Add RBAC ADR Signed-off-by: Neil Martinsen-Burrell --- .../0019-role-based-access-control.md | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) create mode 100644 docs/architecture/decisions/0019-role-based-access-control.md diff --git a/docs/architecture/decisions/0019-role-based-access-control.md b/docs/architecture/decisions/0019-role-based-access-control.md new file mode 100644 index 000000000..d5bc5ddb3 --- /dev/null +++ b/docs/architecture/decisions/0019-role-based-access-control.md @@ -0,0 +1,29 @@ +# 19. Role-based Access Control + +Date: 2023-03-17 + +## Status + +Approved + +## Context + +In the registrar application, a single user might be associated with many +domains, and they might have different levels of access to view or change +those domains. + +## Decision + +To use a role-based access control system where we have a model of different +roles and an association that links a user to a specific role with a specified +role. Each role would have some associated permissions in the application and +we can enforce those permissions by using decorators on our Django views. + +## Consequences + +There is no enterprise model here of users belonging to an “organization” with +a role on all of its associated domain names. Instead, the association is +per-domain and a user would have to be granted the role on each domain +individually. There is also no process designed yet for how and whether users +can grant other users roles on a domain. + From 7423ba30239dc51124bde203829da3e99e534b8f Mon Sep 17 00:00:00 2001 From: Neil Martinsen-Burrell Date: Fri, 17 Mar 2023 14:35:21 -0500 Subject: [PATCH 7/7] Developer documentation on roles and permissions Signed-off-by: Neil Martinsen-Burrell --- docs/developer/user-permissions.md | 40 ++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) create mode 100644 docs/developer/user-permissions.md diff --git a/docs/developer/user-permissions.md b/docs/developer/user-permissions.md new file mode 100644 index 000000000..281a58c1b --- /dev/null +++ b/docs/developer/user-permissions.md @@ -0,0 +1,40 @@ +# User Permissions + +In our registrar application, we need authenticated users (via Login.gov) to +be able to access domains that they are authorized to and not to access +domains that they are not authorized to. In our initial MVP design, this +access is controlled at the domain level, there is no "enterprise" or +"organization" layer for assigning permissions in bulk. (See [this +ADR](../architecture/decisions/0019-role-based-access-control.md) for more on +that decision.) + +## Data modeling + +We need a way to associate a particular user with a particular domain and the +role or set of permissions that they have. We use a `UserDomainRole` +[model](../../src/registrar/models/user_domain_role.py) with `ForeignKey`s to +`User` and `Domain` and a `role` field. There are reverse relationships called +`permissions` for a user and for a domain to get a list of all of the +`UserDomainRole`s that involve the user or the domain. In addition, there is a +`User.domains` many-to-many relationship that works through the +`UserDomainRole` link table. + +## Permission decorator + +The Django objects that need to be permission controlled are various views. +For that purpose, we add a very simple permission mixin +[`DomainPermission`](../../src/registrar/views/utility/mixins.py) that can be +added to a view to require that (a) there is a logged-in user and (b) that the +logged in user has a role that permits access to that view. This mixin is the +place where the details of the permissions are enforced. It can allow a view +to load, or deny access with various status codes, e.g. "403 Forbidden". + +## Adding roles + +The current MVP design uses only a single role called +`UserDomainRole.Roles.ADMIN` that has all access on a domain. As such, the +permission mixin doesn't need to examine the `role` field carefully. In the +future, as we add additional roles that our product vision calls for +(read-only? editing only some information?), we need to add conditional +behavior in the permission mixin, or additional mixins that more clearly +express what is allowed for those new roles.