Merge pull request #459 from cisagov/nmb/rbac

Role-based Access Control

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.
+

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.

src/registrar/config/urls.py

@@ -61,6 +61,7 @@ urlpatterns = [
         lambda r: always_404(r, "We forgot to include this link, sorry."),
         name="todo",
     ),
+    path("domain/<int:pk>", views.DomainView.as_view(), name="domain"),
 ]
 
 

src/registrar/migrations/0015_remove_domain_owners_userdomainrole_user_domains_and_more.py

@@ -0,0 +1,66 @@
+# Generated by Django 4.1.6 on 2023-03-10 15:32
+
+from django.conf import settings
+from django.db import migrations, models
+import django.db.models.deletion
+
+
+class Migration(migrations.Migration):
+    dependencies = [
+        ("registrar", "0014_user_phone_alter_contact_user"),
+    ]
+
+    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"
+            ),
+        ),
+    ]

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 .public_contact import PublicContact
 from .user import User
 from .website import Website
@@ -17,6 +18,7 @@ __all__ = [
     "HostIP",
     "Host",
     "Nameserver",
+    "UserDomainRole",
     "PublicContact",
     "User",
     "Website",
@@ -28,6 +30,7 @@ auditlog.register(Domain)
 auditlog.register(HostIP)
 auditlog.register(Host)
 auditlog.register(Nameserver)
+auditlog.register(UserDomainRole)
 auditlog.register(PublicContact)
 auditlog.register(User)
 auditlog.register(Website)

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
+    # ForeignKey on UserDomainRole creates a "permissions" member for
-    # and the domain application's `creator` and `submitter`
+    # all of the user-roles that are in place for this domain
-    owners = models.ManyToManyField(
+
-        "registrar.User",
+    # ManyToManyField on User creates a "users" member for all of the
-        help_text="",
+    # users who have some role on this domain
-    )

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

src/registrar/models/user.py

@@ -1,4 +1,5 @@
 from django.contrib.auth.models import AbstractUser
+from django.db import models
 
 from phonenumber_field.modelfields import PhoneNumberField  # type: ignore
 
@@ -9,6 +10,12 @@ class User(AbstractUser):
     but can be customized later.
     """
 
+    domains = models.ManyToManyField(
+        "registrar.Domain",
+        through="registrar.UserDomainRole",
+        related_name="users",
+    )
+
     phone = PhoneNumberField(
         null=True,
         blank=True,

src/registrar/models/user_domain_role.py

@@ -0,0 +1,50 @@
+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, permissions are 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"
+            )
+        ]

src/registrar/templates/domain_detail.html

@@ -0,0 +1,14 @@
+{% extends "dashboard_base.html" %}
+
+
+{% block title %}Domain {{ domain.name }}{% endblock %}
+
+{% block content %}
+<main id="main-content" class="grid-container">
+<div class="tablet:grid-offset-1 desktop:grid-offset-2">
+  <h1>{{ domain.name }}</h1>
+
+  <p>Active: {% if domain.is_active %}Yes{% else %}No{% endif %}</p>
+</div>
+</main>
+{% endblock %}  {# content #}

src/registrar/templates/home.html

@@ -16,7 +16,41 @@
 
   <section class="dashboard tablet:grid-col-11 desktop:grid-col-10">
     <h2>Registered domains</h2>
+    {% if domains %}
+    <table class="usa-table usa-table--borderless usa-table--stacked">
+      <caption class="sr-only">Your domain applications</caption>
+      <thead>
+        <tr>
+          <th data-sortable scope="col" role="columnheader">Domain name</th>
+          <th data-sortable scope="col" role="columnheader">Date created</th>
+          <th data-sortable scope="col" role="columnheader">Status</th>
+          <th scope="col" role="columnheader"><span class="usa-sr-only">Action</span></th>
+        </tr>
+      </thead>
+      <tbody>
+        {% for domain in domains %}
+        <tr>
+          <th th scope="row" role="rowheader" data-label="Domain name">
+            {{ domain.name }}
+          </th>
+          <td data-sort-value="{{ domain.created_time|date:"U" }}" data-label="Date created">{{ domain.created_time|date }}</td>
+          <td data-label="Status">{{ domain.application_status|title }}</td>
+          <td>
+            <a href="{% url "domain" pk=domain.pk %}">
+                Edit <span class="usa-sr-only">{{ domain.name }}</span>
+            </a>
+          </td>
+        </tr>
+        {% endfor %}
+      </tbody>
+    </table>
+    <div
+      class="usa-sr-only usa-table__announcement-region"
+      aria-live="polite"
+    ></div>
+    {% else %}
     <p>You don't have any registered domains yet</p>
+    {% endif %}
   </section>
 
   <section class="dashboard tablet:grid-col-11 desktop:grid-col-10">

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
+from registrar.models import (
+    Contact,
+    DomainApplication,
+    User,
+    Website,
+    Domain,
+    UserDomainRole,
+)
 from unittest import skip
 
 import boto3_mocking  # type: ignore
@@ -139,6 +146,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):

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("/")
+        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
+        )
+        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/")
@@ -1003,3 +1016,46 @@ 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")

src/registrar/views/__init__.py

@@ -1,4 +1,5 @@
 from .application import *
+from .domain import *
 from .health import *
 from .index import *
 from .whoami import *

src/registrar/views/domain.py

@@ -0,0 +1,13 @@
+"""View for a single Domain."""
+
+from django.views.generic import DetailView
+
+from registrar.models import Domain
+
+from .utility import DomainPermission
+
+
+class DomainView(DomainPermission, DetailView):
+    model = Domain
+    template_name = "domain_detail.html"
+    context_object_name = "domain"

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,13 @@ 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",
+            pk=F("domain__id"),
+            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)

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

src/registrar/views/utility/mixins.py

@@ -0,0 +1,37 @@
+"""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
+        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.
+        return True