From ae3ba84f1adf25d712ac84bfda55eb208c871038 Mon Sep 17 00:00:00 2001 From: Neil Martinsen-Burrell Date: Thu, 1 Jun 2023 16:03:31 -0500 Subject: [PATCH 01/45] Add LoginRequiredMiddleware to make (almost) every URL login required by default --- src/Pipfile | 1 + src/Pipfile.lock | 45 +++++++++++++++++------------- src/api/tests/test_available.py | 3 ++ src/registrar/config/settings.py | 8 ++++++ src/registrar/tests/test_views.py | 5 ++-- src/registrar/views/application.py | 3 +- src/registrar/views/health.py | 4 +++ src/requirements.txt | 5 ++-- 8 files changed, 48 insertions(+), 26 deletions(-) diff --git a/src/Pipfile b/src/Pipfile index 433f41632..3d92c36ab 100644 --- a/src/Pipfile +++ b/src/Pipfile @@ -24,6 +24,7 @@ django-fsm = "*" django-phonenumber-field = {extras = ["phonenumberslite"], version = "*"} boto3 = "*" typing-extensions ='*' +django-login-required-middleware = "*" [dev-packages] django-debug-toolbar = "*" diff --git a/src/Pipfile.lock b/src/Pipfile.lock index e485a9b1a..cba61498c 100644 --- a/src/Pipfile.lock +++ b/src/Pipfile.lock @@ -1,7 +1,7 @@ { "_meta": { "hash": { - "sha256": "6a6f3722d88e1f0059e38f23c104708d7922d41b42be1f17287cd4d84dcf9b05" + "sha256": "187278933085d8c5448015e2d28c23934c2098f4aadd82157379174629d8cf6b" }, "pipfile-spec": 6, "requires": {}, @@ -24,19 +24,19 @@ }, "boto3": { "hashes": [ - "sha256:2eb9e688aa86bf1fadcec0b6995b42ec9788e7cd5f1a9c8ac1b66a2506aa209f", - "sha256:5b7e9f2674fe8aa99e2d168744023a3f66da12d9c51e0624489dd0db7aafe30d" + "sha256:30f8ab1cf89d5864a80ba2d5eb5316dbd2a63c9469877e0cffb522630438aa85", + "sha256:77e8fa7c257f9ed8bfe0c3ffc2ccc47b1cfa27058f99415b6003699d1202e0c0" ], "index": "pypi", - "version": "==1.26.144" + "version": "==1.26.145" }, "botocore": { "hashes": [ - "sha256:c60b9158cbc7447411abdec77b87a71d86d9404064702e92d317dca6a1ec9a5b", - "sha256:e2b970e68643cf4752cad4e45ba3319fc35707f1bff7f150f7ffcac1b1427b47" + "sha256:264a3f19ed280d80711b7e278be09acff7ed379a96432fdf179b4e6e3a687e6a", + "sha256:65e2a2b1cc70583225f87d6d63736215f93c6234721967bdab872270ba7a1f45" ], "markers": "python_version >= '3.7'", - "version": "==1.29.144" + "version": "==1.29.145" }, "cachetools": { "hashes": [ @@ -306,6 +306,13 @@ "index": "pypi", "version": "==2.8.1" }, + "django-login-required-middleware": { + "hashes": [ + "sha256:847ae9a69fd7a07618ed53192b3c06946af70a0caf6d0f4eb40a8f37593cd970" + ], + "index": "pypi", + "version": "==0.9.0" + }, "django-phonenumber-field": { "extras": [ "phonenumberslite" @@ -793,11 +800,11 @@ }, "boto3": { "hashes": [ - "sha256:2eb9e688aa86bf1fadcec0b6995b42ec9788e7cd5f1a9c8ac1b66a2506aa209f", - "sha256:5b7e9f2674fe8aa99e2d168744023a3f66da12d9c51e0624489dd0db7aafe30d" + "sha256:30f8ab1cf89d5864a80ba2d5eb5316dbd2a63c9469877e0cffb522630438aa85", + "sha256:77e8fa7c257f9ed8bfe0c3ffc2ccc47b1cfa27058f99415b6003699d1202e0c0" ], "index": "pypi", - "version": "==1.26.144" + "version": "==1.26.145" }, "boto3-mocking": { "hashes": [ @@ -809,27 +816,27 @@ }, "boto3-stubs": { "hashes": [ - "sha256:1062612f63f154f47a4f5b7b40c2cb15debe5f44774587110da76fd292f528f0", - "sha256:bc0cc5067f55b2da628db8a73119ecccd74b27cf424af83e56526cd90beaf9f8" + "sha256:9413cb395c803d5b85e9ec7b16fba855a613ecd78b2e0011e2f6b62cf0b4fc1e", + "sha256:be2007f92138781288c7a22eba30b7d60742466fc28edd04637b31fabee854a5" ], "index": "pypi", - "version": "==1.26.144" + "version": "==1.26.145" }, "botocore": { "hashes": [ - "sha256:c60b9158cbc7447411abdec77b87a71d86d9404064702e92d317dca6a1ec9a5b", - "sha256:e2b970e68643cf4752cad4e45ba3319fc35707f1bff7f150f7ffcac1b1427b47" + "sha256:264a3f19ed280d80711b7e278be09acff7ed379a96432fdf179b4e6e3a687e6a", + "sha256:65e2a2b1cc70583225f87d6d63736215f93c6234721967bdab872270ba7a1f45" ], "markers": "python_version >= '3.7'", - "version": "==1.29.144" + "version": "==1.29.145" }, "botocore-stubs": { "hashes": [ - "sha256:b9db32981b4deefb01784d9b196afeaca7df6f6f185d8ba7f96c02b1c3bc0d90", - "sha256:d456543af79fbdd23df76a2d7a7525cd672b4bb5b057d7e060bc117d9af71694" + "sha256:80ffab72ad428d20cb1cf538ee55fcd94f7d81315b77d84fec99e218c3974e8b", + "sha256:928c58a434dd83bef956e3b5bb1e96278fff5eee9f8b8ab08d916cef1e9a2014" ], "markers": "python_version >= '3.7' and python_version < '4.0'", - "version": "==1.29.144" + "version": "==1.29.145" }, "click": { "hashes": [ diff --git a/src/api/tests/test_available.py b/src/api/tests/test_available.py index 061b6274c..39ddba071 100644 --- a/src/api/tests/test_available.py +++ b/src/api/tests/test_available.py @@ -104,6 +104,9 @@ class AvailableAPITest(TestCase): def test_available_post(self): """Cannot post to the /available/ API endpoint.""" + # have to log in to test the correct thing now that we require login + # for all URLs by default + self.client.force_login(self.user) with less_console_noise(): response = self.client.post(API_BASE_PATH + "nonsense") self.assertEqual(response.status_code, 405) diff --git a/src/registrar/config/settings.py b/src/registrar/config/settings.py index 54eb35d9f..5020fe382 100644 --- a/src/registrar/config/settings.py +++ b/src/registrar/config/settings.py @@ -134,6 +134,8 @@ MIDDLEWARE = [ "django.middleware.csrf.CsrfViewMiddleware", # add `user` (the currently-logged-in user) to incoming HttpRequest objects "django.contrib.auth.middleware.AuthenticationMiddleware", + # Require login for every single request by default + "login_required.middleware.LoginRequiredMiddleware", # provide framework for displaying messages to the user, see documentation "django.contrib.messages.middleware.MessageMiddleware", # provide clickjacking protection via the X-Frame-Options header @@ -461,6 +463,12 @@ AUTHENTICATION_BACKENDS = [ # the login_required() decorator, LoginRequiredMixin, or AccessMixin LOGIN_URL = "/openid/login" +# We don't want the OIDC app to be login-required because then it can't handle +# the initial login requests without erroring. +LOGIN_REQUIRED_IGNORE_PATHS = [ + r"/openid/(.+)$", +] + # where to go after logging out LOGOUT_REDIRECT_URL = "home" diff --git a/src/registrar/tests/test_views.py b/src/registrar/tests/test_views.py index bf1cef2f2..500912952 100644 --- a/src/registrar/tests/test_views.py +++ b/src/registrar/tests/test_views.py @@ -35,10 +35,9 @@ class TestViews(TestCase): self.assertContains(response, "OK", status_code=200) def test_home_page(self): - """Home page should be available without a login.""" + """Home page should NOT be available without a login.""" response = self.client.get("/") - self.assertContains(response, "registrar", status_code=200) - self.assertContains(response, "Sign in") + self.assertEqual(response.status_code, 302) def test_whoami_page_no_user(self): """Whoami page not accessible without a logged-in user.""" diff --git a/src/registrar/views/application.py b/src/registrar/views/application.py index b8454bf78..d4368e868 100644 --- a/src/registrar/views/application.py +++ b/src/registrar/views/application.py @@ -1,6 +1,5 @@ import logging -from django.contrib.auth.mixins import LoginRequiredMixin from django.http import Http404, HttpResponse, HttpResponseRedirect from django.shortcuts import redirect, render from django.urls import resolve, reverse @@ -44,7 +43,7 @@ class Step(StrEnum): REVIEW = "review" -class ApplicationWizard(LoginRequiredMixin, TemplateView): +class ApplicationWizard(TemplateView): """ A common set of methods and configuration. diff --git a/src/registrar/views/health.py b/src/registrar/views/health.py index 53179382a..40f7330a0 100644 --- a/src/registrar/views/health.py +++ b/src/registrar/views/health.py @@ -1,6 +1,10 @@ from django.http import HttpResponse +from login_required import login_not_required +# the health check endpoint needs to be globally available so that the +# PaaS orchestrator can make sure the app has come up properly +@login_not_required def health(request): return HttpResponse( 'OK - Get.govOK' diff --git a/src/requirements.txt b/src/requirements.txt index 9d17f87f7..077cfed2a 100644 --- a/src/requirements.txt +++ b/src/requirements.txt @@ -1,7 +1,7 @@ -i https://pypi.python.org/simple asgiref==3.7.2 ; python_version >= '3.7' -boto3==1.26.144 -botocore==1.29.144 ; python_version >= '3.7' +boto3==1.26.145 +botocore==1.29.145 ; python_version >= '3.7' cachetools==5.3.1 certifi==2023.5.7 ; python_version >= '3.6' cfenv==0.5.3 @@ -17,6 +17,7 @@ django-auditlog==2.3.0 django-cache-url==3.4.4 django-csp==3.7 django-fsm==2.8.1 +django-login-required-middleware==0.9.0 django-phonenumber-field[phonenumberslite]==7.1.0 django-widget-tweaks==1.4.12 environs[django]==9.5.0 From 2e7644bdc3fc8b477c54e64a2832f30079cb319d Mon Sep 17 00:00:00 2001 From: Neil Martinsen-Burrell Date: Thu, 1 Jun 2023 16:05:15 -0500 Subject: [PATCH 02/45] fix linting errors --- src/registrar/views/health.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/registrar/views/health.py b/src/registrar/views/health.py index 40f7330a0..276dc4b09 100644 --- a/src/registrar/views/health.py +++ b/src/registrar/views/health.py @@ -2,6 +2,7 @@ from django.http import HttpResponse from login_required import login_not_required + # the health check endpoint needs to be globally available so that the # PaaS orchestrator can make sure the app has come up properly @login_not_required From 85a8b108ded61637db64948715b71d3ae72d345c Mon Sep 17 00:00:00 2001 From: Neil Martinsen-Burrell Date: Thu, 1 Jun 2023 16:35:40 -0500 Subject: [PATCH 03/45] Add test that login is required for almost every URL --- src/registrar/tests/test_url_auth.py | 158 +++++++++++++++++++++++++++ 1 file changed, 158 insertions(+) create mode 100644 src/registrar/tests/test_url_auth.py diff --git a/src/registrar/tests/test_url_auth.py b/src/registrar/tests/test_url_auth.py new file mode 100644 index 000000000..8547e0bba --- /dev/null +++ b/src/registrar/tests/test_url_auth.py @@ -0,0 +1,158 @@ +"""Test that almost all URLs require authentication. + +This uses deep Django URLConf pattern magic and was shamelessly lifted from +https://github.com/18F/tock/blob/main/tock/tock/tests/test_url_auth.py +""" + +from django.test import TestCase +from django.urls import reverse, URLPattern +from django.urls.resolvers import URLResolver + +import registrar.config.urls + +from .common import less_console_noise + +# When a URLconf pattern contains named capture groups, we'll use this +# dictionary to retrieve a sample value for it, which will be included +# in the sample URLs we generate, when attempting to perform a GET +# request on the view. +SAMPLE_KWARGS = { + "app_label": "registrar", + "pk": "1", + "id": "1", + "content_type_id": "2", + "object_id": "3", + "domain": "whitehouse.gov", +} + +# Our test suite will ignore some namespaces. +IGNORE_NAMESPACES = [ + # The Django Debug Toolbar (DJDT) ends up in the URL config but it's always + # disabled in production, so don't worry about it. + "djdt" +] + +# In general, we don't want to have any unnamed views, because that makes it +# impossible to generate sample URLs that point at them. We'll make exceptions +# for some namespaces that we don't have control over, though. +NAMESPACES_WITH_UNNAMED_VIEWS = ["admin", None] + + +def iter_patterns(urlconf, patterns=None, namespace=None): + """ + Iterate through all patterns in the given Django URLconf. Yields + `(viewname, route)` tuples, where `viewname` is the fully-qualified view name + (including its namespace, if any), and `route` is a regular expression that + corresponds to the part of the pattern that contains any capturing groups. + """ + if patterns is None: + patterns = urlconf.urlpatterns + for pattern in patterns: + # Resolve if it's a route or an include + if isinstance(pattern, URLPattern): + viewname = pattern.name + if viewname is None and namespace not in NAMESPACES_WITH_UNNAMED_VIEWS: + raise AssertionError( + f"namespace {namespace} cannot contain unnamed views" + ) + if namespace and viewname is not None: + viewname = f"{namespace}:{viewname}" + yield (viewname, pattern.pattern) + elif isinstance(pattern, URLResolver): + if len(pattern.default_kwargs.keys()) > 0: + raise AssertionError("resolvers are not expected to have kwargs") + if pattern.namespace and namespace is not None: + raise AssertionError("nested namespaces are not currently supported") + if pattern.namespace in IGNORE_NAMESPACES: + continue + yield from iter_patterns( + urlconf, pattern.url_patterns, namespace or pattern.namespace + ) + else: + raise AssertionError("unknown pattern class") + + +def iter_sample_urls(urlconf): + """ + Yields sample URLs for all entries in the given Django URLconf. + This gets pretty deep into the muck of RoutePattern + https://docs.djangoproject.com/en/2.1/_modules/django/urls/resolvers/ + """ + + for viewname, route in iter_patterns(urlconf): + if not viewname: + continue + if viewname == "auth_user_password_change": + print(route) + break + named_groups = route.regex.groupindex.keys() + kwargs = {} + args = () + + for kwarg in named_groups: + if kwarg not in SAMPLE_KWARGS: + raise AssertionError( + f'Sample value for {kwarg} in pattern "{route}" not found' + ) + kwargs[kwarg] = SAMPLE_KWARGS[kwarg] + + url = reverse(viewname, args=args, kwargs=kwargs) + yield (viewname, url) + + +class TestURLAuth(TestCase): + """ + Tests to ensure that most URLs in a Django URLconf are protected by + authentication. + """ + + # We won't test that the following URLs are protected by auth. + # Note that the trailing slash is wobbly depending on how the URL was defined. + IGNORE_URLS = [ + # These are the OIDC auth endpoints that always need + # to be public. + "/openid/login/", + "/openid/logout/", + "/openid/callback", + "/openid/callback/logout/", + ] + + def assertURLIsProtectedByAuth(self, url): + """ + Make a GET request to the given URL, and ensure that it either redirects + to login or denies access outright. + """ + + try: + with less_console_noise(): + response = self.client.get(url) + except Exception as e: + # It'll be helpful to provide information on what URL was being + # accessed at the time the exception occurred. Python 3 will + # also include a full traceback of the original exception, so + # we don't need to worry about hiding the original cause. + raise AssertionError(f'Accessing {url} raised "{e}"', e) + + code = response.status_code + if code == 302: + redirect = response["location"] + self.assertRegex( + redirect, + r"^\/openid\/login", + f"GET {url} should redirect to login or deny access, but instead " + f"it redirects to {redirect}", + ) + elif code == 401 or code == 403: + pass + else: + raise AssertionError( + f"GET {url} returned HTTP {code}, but should redirect to login or " + "deny access", + ) + + def test_login_required_all_urls(self): + """All URLs redirect to the login view.""" + for viewname, url in iter_sample_urls(registrar.config.urls): + if url not in self.IGNORE_URLS: + with self.subTest(viewname=viewname): + self.assertURLIsProtectedByAuth(url) From 84b13bd178ccb95c8bd59e4ecb1ff61bf95b72b5 Mon Sep 17 00:00:00 2001 From: Neil Martinsen-Burrell Date: Thu, 1 Jun 2023 18:23:06 -0500 Subject: [PATCH 04/45] Fix test failure from CI --- src/registrar/tests/test_url_auth.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/registrar/tests/test_url_auth.py b/src/registrar/tests/test_url_auth.py index 8547e0bba..17ad2c329 100644 --- a/src/registrar/tests/test_url_auth.py +++ b/src/registrar/tests/test_url_auth.py @@ -114,6 +114,7 @@ class TestURLAuth(TestCase): "/openid/login/", "/openid/logout/", "/openid/callback", + "/openid/callback/login/", "/openid/callback/logout/", ] From 593a589636dd906c9fafc3830805d7195d69310e Mon Sep 17 00:00:00 2001 From: Seamus Johnston Date: Mon, 5 Jun 2023 08:54:48 -0500 Subject: [PATCH 05/45] Add developer documentation for EPP --- docs/developer/registry-access.md | 101 ++++++++++++++++++++++++++++++ 1 file changed, 101 insertions(+) create mode 100644 docs/developer/registry-access.md diff --git a/docs/developer/registry-access.md b/docs/developer/registry-access.md new file mode 100644 index 000000000..3f5d06c09 --- /dev/null +++ b/docs/developer/registry-access.md @@ -0,0 +1,101 @@ +# Working with the registry via EPP + +## Overview of parts + +**EPP** is the protocol which describes how a registry and registrar communicate with XML over a TCP socket connection. + +**epplib** is a Python library implementation of the TCP socket connection. It has helper functions and dataclasses which can be used to send and receive the XML messages. + +**epplibwrapper** is a module in this repository which abstracts away the details of authenticating with the registry. It assists with error handling by providing error code constants and an error class with some helper methods. + +**Domain** is a Python class. It inherits from `django.db.models.Model` and is therefore part of Django's ORM and has a corresponding table in the local registrar database. Its purpose is to provide a developer-friendly interface to the registry based on *what a registrant or analyst wants to do*, not on the technical details of EPP. + +## Debugging in a Python shell + +You'll first need access to a Django shell in an environment with valid registry credentials. For example: + +```shell +cf ssh getgov-ENVIRONMENT +/tmp/lifecycle/shell # this configures your environment +./manage.py shell +``` + +You'll next need to import some code. + +``` +from epplibwrapper import CLIENT as registry, commands +from epplib.models import common +``` + +Finally, you'll need to craft a request and send it. + +``` +request = ... +response = registry.send(request) +``` + +See below for some example commands to send. Replace example data with data which makes sense for your debugging scenario. Other commands are available; see the source code of epplib for more options. + + +### Get info about a contact + +``` +request = commands.InfoContact(id='sh8013') +``` + +### Create a new contact + +``` +DF = common.DiscloseField +di = common.Disclose(flag=False, fields={DF.FAX, DF.VOICE, DF.ADDR}, types={DF.ADDR: "loc"}) +addr = common.ContactAddr(street=['123 Example Dr.'], city='Dulles', pc='20166-6503', cc='US', sp='VA') +pi = common.PostalInfo(name='John Doe', addr=addr, org="Example Inc.", type="loc") +ai = common.ContactAuthInfo(pw='feedabee') + +request = commands.CreateContact(id='sh8013', postal_info=pi, email='jdoe@example.com', voice='+1.7035555555', fax='+1.7035555556', auth_info=ai, disclose=di, vat=None, ident=None, notify_email=None) +``` + +### Create a new domain + +``` +ai = common.DomainAuthInfo(pw='feedabee') +request = commands.CreateDomain(name="okay.gov", registrant="sh8013", auth_info=ai) +``` + +### Create a host object + +``` +request = commands.CreateHost(name="ns1.okay.gov", addrs=[common.Ip(addr="127.0.01"), common.Ip(addr="0:0:0:0:0:0:0:1", ip="v6")]) +``` + +### Check if a host is available + +``` +request = commands.CheckHost(["ns2.okay.gov"]) +``` + +### Update a domain + +``` +request = commands.UpdateDomain(name="okay.gov", add=[common.HostObjSet(["ns1.okay.gov"])]) +``` + +``` +request = commands.UpdateDomain(name="okay.gov", add=[common.DomainContact(contact="sh8014", type="technical")]) +``` + +### How to see the raw XML + +To see the XML of a command before the request is sent, call `request.xml()`. + +To see the XML of the response, you must send the command using a different method. + +``` +registry._client.connect() +registry._client.send(registry._login) + +request = commands.InfoDomain(name="ok.gov") + +registry._client.transport.send(request.xml()) +response = registry._client.transport.receive() +``` From 0254d879e45ed13932ba72aa35427b4aa64fcb30 Mon Sep 17 00:00:00 2001 From: Neil Martinsen-Burrell Date: Mon, 5 Jun 2023 14:42:11 -0500 Subject: [PATCH 06/45] Tweak auto-login configuration --- .github/workflows/security-check.yaml | 2 +- .github/workflows/test.yaml | 2 +- src/registrar/tests/test_url_auth.py | 1 - src/zap.conf | 1 + 4 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/security-check.yaml b/.github/workflows/security-check.yaml index 39f8a80a3..b2d840c84 100644 --- a/.github/workflows/security-check.yaml +++ b/.github/workflows/security-check.yaml @@ -72,7 +72,7 @@ jobs: # by adding MockUserLogin to settings.MIDDLEWARE run: | perl -pi \ - -e 's/"csp.middleware.CSPMiddleware",/$&"registrar.tests.common.MockUserLogin",/' \ + -e 's/"django.contrib.auth.middleware.AuthenticationMiddleware",/$&"registrar.tests.common.MockUserLogin",/' \ src/registrar/config/settings.py working-directory: ./src diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index a45b9fa72..6d34f80b5 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -59,7 +59,7 @@ jobs: # by adding MockUserLogin to settings.MIDDLEWARE run: | perl -pi \ - -e 's/"csp.middleware.CSPMiddleware",/$&"registrar.tests.common.MockUserLogin",/' \ + -e 's/"django.contrib.auth.middleware.AuthenticationMiddleware",/$&"registrar.tests.common.MockUserLogin",/' \ registrar/config/settings.py - name: Start container diff --git a/src/registrar/tests/test_url_auth.py b/src/registrar/tests/test_url_auth.py index 17ad2c329..a81e0a4c3 100644 --- a/src/registrar/tests/test_url_auth.py +++ b/src/registrar/tests/test_url_auth.py @@ -112,7 +112,6 @@ class TestURLAuth(TestCase): # These are the OIDC auth endpoints that always need # to be public. "/openid/login/", - "/openid/logout/", "/openid/callback", "/openid/callback/login/", "/openid/callback/logout/", diff --git a/src/zap.conf b/src/zap.conf index 6a5e9bf77..1aa905458 100644 --- a/src/zap.conf +++ b/src/zap.conf @@ -53,6 +53,7 @@ 10038 OUTOFSCOPE http://app:8080/users/add 10038 OUTOFSCOPE http://app:8080/nameservers 10038 OUTOFSCOPE http://app:8080/your-contact-information +10038 OUTOFSCOPE http://app:8080/authorizing-official 10038 OUTOFSCOPE http://app:8080/security-email 10038 OUTOFSCOPE http://app:8080/delete 10038 OUTOFSCOPE http://app:8080/withdraw From 816c6c8c90b48e339541598c211f4cd376ade07e Mon Sep 17 00:00:00 2001 From: Neil Martinsen-Burrell Date: Mon, 5 Jun 2023 14:51:21 -0500 Subject: [PATCH 07/45] try to make python tests pass --- src/registrar/tests/test_url_auth.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/registrar/tests/test_url_auth.py b/src/registrar/tests/test_url_auth.py index a81e0a4c3..17ad2c329 100644 --- a/src/registrar/tests/test_url_auth.py +++ b/src/registrar/tests/test_url_auth.py @@ -112,6 +112,7 @@ class TestURLAuth(TestCase): # These are the OIDC auth endpoints that always need # to be public. "/openid/login/", + "/openid/logout/", "/openid/callback", "/openid/callback/login/", "/openid/callback/logout/", From 75be6d6eb8ccd2275e31e7baaf1f2d30c2970012 Mon Sep 17 00:00:00 2001 From: Neil Martinsen-Burrell Date: Wed, 7 Jun 2023 13:00:55 -0500 Subject: [PATCH 08/45] Add content team members to fixtures --- src/registrar/fixtures.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/registrar/fixtures.py b/src/registrar/fixtures.py index 438f6723b..02a6915aa 100644 --- a/src/registrar/fixtures.py +++ b/src/registrar/fixtures.py @@ -59,6 +59,21 @@ class UserFixture: "first_name": "Alysia", "last_name": "Broddrick", }, + { + "username": "55a3bc26-cd1d-4a5c-a8c0-7e1f561ef7f4", + "first_name": "Michelle", + "last_name": "Rago", + }, + { + "username": "8f8e7293-17f7-4716-889b-1990241cbd39", + "first_name": "Katherine", + "last_name": "Osos", + }, + { + "username": "70488e0a-e937-4894-a28c-16f5949effd4", + "first_name": "Gaby", + "last_name": "DiSarli", + }, ] @classmethod From 162d42e6478c9e5ddcb48312ab8b3617fc7f38d5 Mon Sep 17 00:00:00 2001 From: igorkorenfeld Date: Wed, 7 Jun 2023 14:43:17 -0400 Subject: [PATCH 09/45] Setup org name address page --- src/registrar/config/urls.py | 5 ++ src/registrar/forms/__init__.py | 1 + src/registrar/forms/domain.py | 57 ++++++++++++++++++- .../templates/domain_org_name_address.html | 47 +++++++++++++++ src/registrar/views/__init__.py | 1 + src/registrar/views/domain.py | 47 ++++++++++++++- 6 files changed, 154 insertions(+), 4 deletions(-) create mode 100644 src/registrar/templates/domain_org_name_address.html diff --git a/src/registrar/config/urls.py b/src/registrar/config/urls.py index a19da4791..0ef9fbe36 100644 --- a/src/registrar/config/urls.py +++ b/src/registrar/config/urls.py @@ -88,6 +88,11 @@ urlpatterns = [ views.DomainYourContactInformationView.as_view(), name="domain-your-contact-information", ), + path( + "domain//org-name-address", + views.DomainOrgNameAddressView.as_view(), + name="domain-org-name-address", + ), path( "domain//authorizing-official", views.DomainAuthorizingOfficialView.as_view(), diff --git a/src/registrar/forms/__init__.py b/src/registrar/forms/__init__.py index 364740211..13f75563f 100644 --- a/src/registrar/forms/__init__.py +++ b/src/registrar/forms/__init__.py @@ -3,5 +3,6 @@ from .domain import ( DomainAddUserForm, NameserverFormset, DomainSecurityEmailForm, + DomainOrgNameAddressForm, ContactForm, ) diff --git a/src/registrar/forms/domain.py b/src/registrar/forms/domain.py index e6fbc8fee..a86993cfb 100644 --- a/src/registrar/forms/domain.py +++ b/src/registrar/forms/domain.py @@ -5,7 +5,7 @@ from django.forms import formset_factory from phonenumber_field.widgets import RegionalPhoneNumberWidget -from ..models import Contact +from ..models import Contact, DomainInformation class DomainAddUserForm(forms.Form): @@ -64,3 +64,58 @@ class DomainSecurityEmailForm(forms.Form): """Form for adding or editing a security email to a domain.""" security_email = forms.EmailField(label="Security email") + + +class DomainOrgNameAddressForm(forms.ModelForm): + + """Form for updating the organization name and mailing address.""" + + class Meta: + model = DomainInformation + fields = ["federal_agency", "organization_name", "address_line1", "address_line2", "city", "state_territory", "zipcode", "urbanization"] + labels = { + "address_line1": "Street address", + "address_line2": "Street address line 2", + "state_territory": "State, territory, or military post", + "urbanization": "Urbanization (Puerto Rico only)", + } + error_messages = { + "federal_agency": {"required": "Select the federal agency for your organization." }, + "organization_name": {"required": "Enter the name of your organization." }, + "address_line1": {"required": "Enter the street address of your organization." }, + "city": {"required": "Enter the city where your organization is located."}, + "state_territory": {"required": "Select the state, territory, or military post where your organization is located."}, + "zipcode": {"required": "Enter a zip code in the form of 12345 or 12345-6789."}, + } + widgets = { + "federal_agency": forms.Select(attrs={"required": True}, choices=DomainInformation.AGENCY_CHOICES), + "organization_name": forms.TextInput, + "address_line1": forms.TextInput, + "address_line2": forms.TextInput, + "city": forms.TextInput, + "state_territory": forms.Select(attrs={"required": True,}, choices=DomainInformation.StateTerritoryChoices.choices), + "zipcode": forms.TextInput, + "urbanization" : forms.TextInput, + } + + # the database fields have blank=True so ModelForm doesn't create + # required fields by default. Use this list in __init__ to mark each + # of these fields as required + required = ["organization_name", "address_line1", "city", "zipcode"] + + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + for field_name in self.required: + self.fields[field_name].required = True + self.fields["state_territory"].widget.attrs.pop("maxlength", None) + self.fields["zipcode"].widget.attrs.pop("maxlength", None) + + + + + + + + + + diff --git a/src/registrar/templates/domain_org_name_address.html b/src/registrar/templates/domain_org_name_address.html new file mode 100644 index 000000000..587ba4782 --- /dev/null +++ b/src/registrar/templates/domain_org_name_address.html @@ -0,0 +1,47 @@ +{% extends "domain_base.html" %} +{% load static field_helpers%} + +{% block title %}Organization name and mailing address | {{ domain.name }} | {% endblock %} + +{% block domain_content %} + {# this is right after the messages block in the parent template #} + {% include "includes/form_errors.html" with form=form %} + +

Organization name and mailing address

+ +

The name of your organization will be publicly listed as the domain registrant.

+ + {% include "includes/required_fields.html" %} + +
+ {% csrf_token %} + + {% if domain.domain_info.organization_type == 'federal' %} + {% input_with_errors form.federal_agency %} + {% endif %} + + {% input_with_errors form.organization_name %} + + {% input_with_errors form.address_line1 %} + + {% input_with_errors form.address_line2 %} + + {% input_with_errors form.city %} + + {% input_with_errors form.state_territory %} + + {% with add_class="usa-input--small" %} + {% input_with_errors form.zipcode %} + {% endwith %} + + {% input_with_errors form.urbanization %} + + +
+ +{% endblock %} {# domain_content #} diff --git a/src/registrar/views/__init__.py b/src/registrar/views/__init__.py index 8212d140d..ed1d8ba39 100644 --- a/src/registrar/views/__init__.py +++ b/src/registrar/views/__init__.py @@ -2,6 +2,7 @@ from .application import * from .domain import ( DomainView, DomainAuthorizingOfficialView, + DomainOrgNameAddressView, DomainNameserversView, DomainYourContactInformationView, DomainSecurityEmailView, diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index 9771a09c6..9c410a788 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -22,10 +22,11 @@ from registrar.models import ( ) from ..forms import ( - DomainAddUserForm, - NameserverFormset, - DomainSecurityEmailForm, ContactForm, + DomainOrgNameAddressForm, + DomainAddUserForm, + DomainSecurityEmailForm, + NameserverFormset, ) from ..utility.email import send_templated_email, EmailSendingError from .utility import DomainPermissionView, DomainInvitationPermissionDeleteView @@ -40,6 +41,46 @@ class DomainView(DomainPermissionView): template_name = "domain_detail.html" +class DomainOrgNameAddressView(DomainPermissionView, FormMixin): + """Organization name and mailing address view """ + model = Domain + template_name = "domain_org_name_address.html" + context_object_name = "domain" + form_class = DomainOrgNameAddressForm + + def get_form_kwargs(self, *args, **kwargs): + """Add domain_info.organization_name instance to make a bound form.""" + form_kwargs = super().get_form_kwargs(*args, **kwargs) + form_kwargs["instance"] = self.get_object().domain_info + return form_kwargs + + def get_success_url(self): + """Redirect to the overview page for the domain.""" + return reverse("domain-org-name-address", kwargs={"pk": self.object.pk}) + + def post(self, request, *args, **kwargs): + """Form submission posts to this view. + + This post method harmonizes using DetailView and FormMixin together. + """ + self.object = self.get_object() + form = self.get_form() + if form.is_valid(): + return self.form_valid(form) + else: + return self.form_invalid(form) + + def form_valid(self, form): + """The form is valid, save the organization name and mailing address.""" + form.save() + + messages.success( + self.request, "The organization name and mailing address has been updated." + ) + # superclass has the redirect + return super().form_valid(form) + + class DomainAuthorizingOfficialView(DomainPermissionView, FormMixin): From 8816b06c66cc0331211e6e8a8d0aeecc61eeb08d Mon Sep 17 00:00:00 2001 From: igorkorenfeld Date: Wed, 7 Jun 2023 14:43:58 -0400 Subject: [PATCH 10/45] Add org name address page to sidebar --- src/registrar/templates/domain_sidebar.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/templates/domain_sidebar.html b/src/registrar/templates/domain_sidebar.html index 5b0457e55..1e4cd1882 100644 --- a/src/registrar/templates/domain_sidebar.html +++ b/src/registrar/templates/domain_sidebar.html @@ -22,7 +22,7 @@
  • - {% url 'todo' as url %} + {% url 'domain-org-name-address' pk=domain.id as url %} From e93f680746ba7cfb9dbe0550a7f2edd568680054 Mon Sep 17 00:00:00 2001 From: igorkorenfeld Date: Wed, 7 Jun 2023 14:49:16 -0400 Subject: [PATCH 11/45] Adjust py code formatting --- src/registrar/forms/domain.py | 62 +++++++++++++++++++++-------------- src/registrar/views/domain.py | 7 ++-- 2 files changed, 42 insertions(+), 27 deletions(-) diff --git a/src/registrar/forms/domain.py b/src/registrar/forms/domain.py index a86993cfb..e90da3b35 100644 --- a/src/registrar/forms/domain.py +++ b/src/registrar/forms/domain.py @@ -72,30 +72,54 @@ class DomainOrgNameAddressForm(forms.ModelForm): class Meta: model = DomainInformation - fields = ["federal_agency", "organization_name", "address_line1", "address_line2", "city", "state_territory", "zipcode", "urbanization"] + fields = [ + "federal_agency", + "organization_name", + "address_line1", + "address_line2", + "city", + "state_territory", + "zipcode", + "urbanization", + ] labels = { - "address_line1": "Street address", - "address_line2": "Street address line 2", - "state_territory": "State, territory, or military post", - "urbanization": "Urbanization (Puerto Rico only)", - } + "address_line1": "Street address", + "address_line2": "Street address line 2", + "state_territory": "State, territory, or military post", + "urbanization": "Urbanization (Puerto Rico only)", + } error_messages = { - "federal_agency": {"required": "Select the federal agency for your organization." }, - "organization_name": {"required": "Enter the name of your organization." }, - "address_line1": {"required": "Enter the street address of your organization." }, + "federal_agency": { + "required": "Select the federal agency for your organization." + }, + "organization_name": {"required": "Enter the name of your organization."}, + "address_line1": { + "required": "Enter the street address of your organization." + }, "city": {"required": "Enter the city where your organization is located."}, - "state_territory": {"required": "Select the state, territory, or military post where your organization is located."}, - "zipcode": {"required": "Enter a zip code in the form of 12345 or 12345-6789."}, + "state_territory": { + "required": "Select the state, territory, or military post where your organization is located." + }, + "zipcode": { + "required": "Enter a zip code in the form of 12345 or 12345-6789." + }, } widgets = { - "federal_agency": forms.Select(attrs={"required": True}, choices=DomainInformation.AGENCY_CHOICES), + "federal_agency": forms.Select( + attrs={"required": True}, choices=DomainInformation.AGENCY_CHOICES + ), "organization_name": forms.TextInput, "address_line1": forms.TextInput, "address_line2": forms.TextInput, "city": forms.TextInput, - "state_territory": forms.Select(attrs={"required": True,}, choices=DomainInformation.StateTerritoryChoices.choices), + "state_territory": forms.Select( + attrs={ + "required": True, + }, + choices=DomainInformation.StateTerritoryChoices.choices, + ), "zipcode": forms.TextInput, - "urbanization" : forms.TextInput, + "urbanization": forms.TextInput, } # the database fields have blank=True so ModelForm doesn't create @@ -109,13 +133,3 @@ class DomainOrgNameAddressForm(forms.ModelForm): self.fields[field_name].required = True self.fields["state_territory"].widget.attrs.pop("maxlength", None) self.fields["zipcode"].widget.attrs.pop("maxlength", None) - - - - - - - - - - diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index 9c410a788..6a33ec994 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -41,8 +41,10 @@ class DomainView(DomainPermissionView): template_name = "domain_detail.html" + class DomainOrgNameAddressView(DomainPermissionView, FormMixin): - """Organization name and mailing address view """ + """Organization name and mailing address view""" + model = Domain template_name = "domain_org_name_address.html" context_object_name = "domain" @@ -52,7 +54,7 @@ class DomainOrgNameAddressView(DomainPermissionView, FormMixin): """Add domain_info.organization_name instance to make a bound form.""" form_kwargs = super().get_form_kwargs(*args, **kwargs) form_kwargs["instance"] = self.get_object().domain_info - return form_kwargs + return form_kwargs def get_success_url(self): """Redirect to the overview page for the domain.""" @@ -81,7 +83,6 @@ class DomainOrgNameAddressView(DomainPermissionView, FormMixin): return super().form_valid(form) - class DomainAuthorizingOfficialView(DomainPermissionView, FormMixin): """Domain authorizing official editing view.""" From b03f280b0eabf0c6d231d7af92ce3efc7977ab3b Mon Sep 17 00:00:00 2001 From: igorkorenfeld Date: Wed, 7 Jun 2023 14:52:00 -0400 Subject: [PATCH 12/45] Make line shorter per linter --- src/registrar/forms/domain.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/registrar/forms/domain.py b/src/registrar/forms/domain.py index e90da3b35..3a77fa2e4 100644 --- a/src/registrar/forms/domain.py +++ b/src/registrar/forms/domain.py @@ -98,7 +98,8 @@ class DomainOrgNameAddressForm(forms.ModelForm): }, "city": {"required": "Enter the city where your organization is located."}, "state_territory": { - "required": "Select the state, territory, or military post where your organization is located." + "required": "Select the state, territory, or military post where your" + "organization is located." }, "zipcode": { "required": "Enter a zip code in the form of 12345 or 12345-6789." From d97ad474da33337a3b0f8145b2157ebb9befefb1 Mon Sep 17 00:00:00 2001 From: Neil Martinsen-Burrell Date: Wed, 7 Jun 2023 14:06:29 -0500 Subject: [PATCH 13/45] Get environment variable from a better location --- src/registrar/config/settings.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/registrar/config/settings.py b/src/registrar/config/settings.py index 54eb35d9f..8f3307d80 100644 --- a/src/registrar/config/settings.py +++ b/src/registrar/config/settings.py @@ -48,6 +48,7 @@ env_db_url = env.dj_db_url("DATABASE_URL") env_debug = env.bool("DJANGO_DEBUG", default=False) env_log_level = env.str("DJANGO_LOG_LEVEL", "DEBUG") env_base_url = env.str("DJANGO_BASE_URL") +env_getgov_public_site_url = env.str("GETGOV_PUBLIC_SITE_URL", "") secret_login_key = b64decode(secret("DJANGO_SECRET_LOGIN_KEY", "")) secret_key = secret("DJANGO_SECRET_KEY") @@ -62,8 +63,6 @@ secret_registry_key = b64decode(secret("REGISTRY_KEY", "")) secret_registry_key_passphrase = secret("REGISTRY_KEY_PASSPHRASE", "") secret_registry_hostname = secret("REGISTRY_HOSTNAME") -secret_getgov_public_site_url = secret("GETGOV_PUBLIC_SITE_URL", "") - # region: Basic Django Config-----------------------------------------------### # Build paths inside the project like this: BASE_DIR / "subdir". @@ -509,7 +508,7 @@ STATIC_URL = "public/" # Base URL of our separate static public website. Used by the # {% public_site_url subdir/path %} template tag -GETGOV_PUBLIC_SITE_URL = secret_getgov_public_site_url +GETGOV_PUBLIC_SITE_URL = env_getgov_public_site_url # endregion # region: Registry----------------------------------------------------------### From 28fde240104c55d924314f29b04c4e37377163cd Mon Sep 17 00:00:00 2001 From: Seamus Johnston Date: Wed, 7 Jun 2023 15:40:25 -0500 Subject: [PATCH 14/45] Update docs/developer/registry-access.md Co-authored-by: Alysia Broddrick <109625347+abroddrick@users.noreply.github.com> --- docs/developer/registry-access.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/developer/registry-access.md b/docs/developer/registry-access.md index 3f5d06c09..a438d565d 100644 --- a/docs/developer/registry-access.md +++ b/docs/developer/registry-access.md @@ -81,7 +81,7 @@ request = commands.UpdateDomain(name="okay.gov", add=[common.HostObjSet(["ns1.ok ``` ``` -request = commands.UpdateDomain(name="okay.gov", add=[common.DomainContact(contact="sh8014", type="technical")]) +request = commands.UpdateDomain(name="okay.gov", add=[common.DomainContact(contact="sh8014", type="tech")]) ``` ### How to see the raw XML From 0bd07bb8ed2ab1982379ff9c6d1a93c90f0d1e14 Mon Sep 17 00:00:00 2001 From: Seamus Johnston Date: Wed, 7 Jun 2023 15:40:40 -0500 Subject: [PATCH 15/45] Update docs/developer/registry-access.md Co-authored-by: Alysia Broddrick <109625347+abroddrick@users.noreply.github.com> --- docs/developer/registry-access.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/developer/registry-access.md b/docs/developer/registry-access.md index a438d565d..4ec5440dd 100644 --- a/docs/developer/registry-access.md +++ b/docs/developer/registry-access.md @@ -65,7 +65,7 @@ request = commands.CreateDomain(name="okay.gov", registrant="sh8013", auth_info= ### Create a host object ``` -request = commands.CreateHost(name="ns1.okay.gov", addrs=[common.Ip(addr="127.0.01"), common.Ip(addr="0:0:0:0:0:0:0:1", ip="v6")]) +request = commands.CreateHost(name="ns1.okay.gov", addrs=[common.Ip(addr="127.0.0.1"), common.Ip(addr="0:0:0:0:0:0:0:1", ip="v6")]) ``` ### Check if a host is available From dc95ff49322d38b4078c491f90a70a1c2356b7e1 Mon Sep 17 00:00:00 2001 From: brandonlenz Date: Wed, 7 Jun 2023 17:37:34 -0400 Subject: [PATCH 16/45] Add issue links section to bug template --- .github/ISSUE_TEMPLATE/bug.yml | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/.github/ISSUE_TEMPLATE/bug.yml b/.github/ISSUE_TEMPLATE/bug.yml index e3e6a6f23..eac435061 100644 --- a/.github/ISSUE_TEMPLATE/bug.yml +++ b/.github/ISSUE_TEMPLATE/bug.yml @@ -57,3 +57,13 @@ body: attributes: label: Additional Context (optional) description: "Please include additional references, screenshots, documentation, etc. that are relevant" + - type: textarea + id: issue-links + attributes: + label: Issue Links (optional) + description: | + What other issues does this story relate to and how? + + Example: + - 🚧 Blocked by: #123 + - 🔄 Relates to: #234 From 6489218d82ed4d514f8866ff348ca74997d36570 Mon Sep 17 00:00:00 2001 From: igorkorenfeld Date: Wed, 7 Jun 2023 17:43:22 -0400 Subject: [PATCH 17/45] Add validation for zipcode --- src/registrar/forms/domain.py | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/src/registrar/forms/domain.py b/src/registrar/forms/domain.py index 3a77fa2e4..62c50add5 100644 --- a/src/registrar/forms/domain.py +++ b/src/registrar/forms/domain.py @@ -1,6 +1,7 @@ """Forms for domain management.""" from django import forms +from django.core.validators import RegexValidator from django.forms import formset_factory from phonenumber_field.widgets import RegionalPhoneNumberWidget @@ -70,6 +71,16 @@ class DomainOrgNameAddressForm(forms.ModelForm): """Form for updating the organization name and mailing address.""" + zipcode = forms.CharField( + label="Zip code", + validators=[ + RegexValidator( + "^[0-9]{5}(?:-[0-9]{4})?$|^$", + message="Enter a zip code in the form of 12345 or 12345-6789.", + ) + ], + ) + class Meta: model = DomainInformation fields = [ @@ -101,9 +112,6 @@ class DomainOrgNameAddressForm(forms.ModelForm): "required": "Select the state, territory, or military post where your" "organization is located." }, - "zipcode": { - "required": "Enter a zip code in the form of 12345 or 12345-6789." - }, } widgets = { "federal_agency": forms.Select( @@ -119,7 +127,6 @@ class DomainOrgNameAddressForm(forms.ModelForm): }, choices=DomainInformation.StateTerritoryChoices.choices, ), - "zipcode": forms.TextInput, "urbanization": forms.TextInput, } @@ -134,3 +141,10 @@ class DomainOrgNameAddressForm(forms.ModelForm): self.fields[field_name].required = True self.fields["state_territory"].widget.attrs.pop("maxlength", None) self.fields["zipcode"].widget.attrs.pop("maxlength", None) +""" + def clean(self): + data = super().clean() + if not re.match(r"^[0-9]{5}(?:-[0-9]{4})?$|^$", data["zipcode"]): + raise forms.ValidationError("Enter a zip code in the form of 12345 or 12345-6789.", code="invalid") + return self.cleaned_data + """ From ea1b89fdb26a09d12c59a092742c215b4de81ed9 Mon Sep 17 00:00:00 2001 From: igorkorenfeld Date: Wed, 7 Jun 2023 17:43:46 -0400 Subject: [PATCH 18/45] Remove commented out code --- src/registrar/forms/domain.py | 7 ------- 1 file changed, 7 deletions(-) diff --git a/src/registrar/forms/domain.py b/src/registrar/forms/domain.py index 62c50add5..6a1077672 100644 --- a/src/registrar/forms/domain.py +++ b/src/registrar/forms/domain.py @@ -141,10 +141,3 @@ class DomainOrgNameAddressForm(forms.ModelForm): self.fields[field_name].required = True self.fields["state_territory"].widget.attrs.pop("maxlength", None) self.fields["zipcode"].widget.attrs.pop("maxlength", None) -""" - def clean(self): - data = super().clean() - if not re.match(r"^[0-9]{5}(?:-[0-9]{4})?$|^$", data["zipcode"]): - raise forms.ValidationError("Enter a zip code in the form of 12345 or 12345-6789.", code="invalid") - return self.cleaned_data - """ From 4c04876ea0728286d578ac7ef0e457e5a7e76dae Mon Sep 17 00:00:00 2001 From: brandonlenz Date: Wed, 7 Jun 2023 17:56:24 -0400 Subject: [PATCH 19/45] Loosen "given, when, then" requirement to optional recommendation --- .github/ISSUE_TEMPLATE/user-story.yml | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/.github/ISSUE_TEMPLATE/user-story.yml b/.github/ISSUE_TEMPLATE/user-story.yml index 0ad2f2853..3f66e3def 100644 --- a/.github/ISSUE_TEMPLATE/user-story.yml +++ b/.github/ISSUE_TEMPLATE/user-story.yml @@ -19,7 +19,7 @@ body: If more than one "as a, I want, so that" describes the story, add multiple. Example: - As an administrator + As an analyst I want the ability to approve a domain application so that a request can be fulfilled and a new .gov domain can be provisioned value: | @@ -33,16 +33,16 @@ body: attributes: label: Acceptance Criteria description: | - Please add the acceptance criteria using one or more "given, when, then" formulae + Please add the acceptance criteria that best describe the desired outcomes when this work is completed Example: - Given that I am an administrator who has finished reviewing a domain application + - Application sends an email when analysts approve domain requests + - Domain application status is "approved" + + Example ("given, when, then" format): + Given that I am an analyst who has finished reviewing a domain application When I click to approve a domain application Then the domain provisioning process should be initiated, and the applicant should receive an email update. - value: | - Given - When - Then validations: required: true - type: textarea From b37caceefbdd130f0b01fb871bb29ede2ab4dbac Mon Sep 17 00:00:00 2001 From: brandonlenz Date: Wed, 7 Jun 2023 18:01:27 -0400 Subject: [PATCH 20/45] Explicitly note that design links are additional context --- .github/ISSUE_TEMPLATE/bug.yml | 2 +- .github/ISSUE_TEMPLATE/user-story.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/ISSUE_TEMPLATE/bug.yml b/.github/ISSUE_TEMPLATE/bug.yml index eac435061..d6701d9e5 100644 --- a/.github/ISSUE_TEMPLATE/bug.yml +++ b/.github/ISSUE_TEMPLATE/bug.yml @@ -56,7 +56,7 @@ body: id: additional-context attributes: label: Additional Context (optional) - description: "Please include additional references, screenshots, documentation, etc. that are relevant" + description: "Please include additional references (screenshots, designs, documentation, etc.) that are relevant" - type: textarea id: issue-links attributes: diff --git a/.github/ISSUE_TEMPLATE/user-story.yml b/.github/ISSUE_TEMPLATE/user-story.yml index 3f66e3def..0aef510a8 100644 --- a/.github/ISSUE_TEMPLATE/user-story.yml +++ b/.github/ISSUE_TEMPLATE/user-story.yml @@ -49,7 +49,7 @@ body: id: additional-context attributes: label: Additional Context (optional) - description: "Please include additional references, screenshots, documentation, etc. that are relevant" + description: "Please include additional references (screenshots, designs, documentation, etc.) that are relevant" - type: textarea id: issue-links attributes: From c91e98081223d76609b11b74a2e91e2b38490ab4 Mon Sep 17 00:00:00 2001 From: brandonlenz Date: Wed, 7 Jun 2023 18:03:42 -0400 Subject: [PATCH 21/45] Based on previous PR feedback, no need to prefix issues --- .github/ISSUE_TEMPLATE/bug.yml | 2 +- .github/ISSUE_TEMPLATE/user-story.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/ISSUE_TEMPLATE/bug.yml b/.github/ISSUE_TEMPLATE/bug.yml index d6701d9e5..ed3dd736a 100644 --- a/.github/ISSUE_TEMPLATE/bug.yml +++ b/.github/ISSUE_TEMPLATE/bug.yml @@ -1,6 +1,6 @@ name: Bug description: Report a bug -title: "[Bug]: " +title: "" labels: ["bug"] body: diff --git a/.github/ISSUE_TEMPLATE/user-story.yml b/.github/ISSUE_TEMPLATE/user-story.yml index 0aef510a8..e71b68e0f 100644 --- a/.github/ISSUE_TEMPLATE/user-story.yml +++ b/.github/ISSUE_TEMPLATE/user-story.yml @@ -1,6 +1,6 @@ name: User Story description: Capture actionable sprint work -title: "[Story]: " +title: "" labels: ["story"] body: From ee42264174a936322ebd94014fa16f6d923e44c1 Mon Sep 17 00:00:00 2001 From: brandonlenz Date: Wed, 7 Jun 2023 18:05:08 -0400 Subject: [PATCH 22/45] Lower the barrier for users finding the right template if they aren't familiar with the term "bug" or how we use it --- .github/ISSUE_TEMPLATE/bug.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/ISSUE_TEMPLATE/bug.yml b/.github/ISSUE_TEMPLATE/bug.yml index ed3dd736a..7452e02be 100644 --- a/.github/ISSUE_TEMPLATE/bug.yml +++ b/.github/ISSUE_TEMPLATE/bug.yml @@ -1,5 +1,5 @@ name: Bug -description: Report a bug +description: Report a problem with the application title: "" labels: ["bug"] From b60e48689a1cf66b0b889310f7c25aa7a6facec0 Mon Sep 17 00:00:00 2001 From: brandonlenz Date: Thu, 8 Jun 2023 09:14:25 -0400 Subject: [PATCH 23/45] Update bug template description --- .github/ISSUE_TEMPLATE/bug.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/ISSUE_TEMPLATE/bug.yml b/.github/ISSUE_TEMPLATE/bug.yml index 7452e02be..44229a366 100644 --- a/.github/ISSUE_TEMPLATE/bug.yml +++ b/.github/ISSUE_TEMPLATE/bug.yml @@ -1,5 +1,5 @@ name: Bug -description: Report a problem with the application +description: Report a bug or problem with the application title: "" labels: ["bug"] From 360ebab27f1e33ca4eb8a6c8f368a58afb56438b Mon Sep 17 00:00:00 2001 From: brandonlenz Date: Thu, 8 Jun 2023 09:15:24 -0400 Subject: [PATCH 24/45] Say "design links" instead of "designs" --- .github/ISSUE_TEMPLATE/bug.yml | 2 +- .github/ISSUE_TEMPLATE/user-story.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/ISSUE_TEMPLATE/bug.yml b/.github/ISSUE_TEMPLATE/bug.yml index 44229a366..a7d70057c 100644 --- a/.github/ISSUE_TEMPLATE/bug.yml +++ b/.github/ISSUE_TEMPLATE/bug.yml @@ -56,7 +56,7 @@ body: id: additional-context attributes: label: Additional Context (optional) - description: "Please include additional references (screenshots, designs, documentation, etc.) that are relevant" + description: "Please include additional references (screenshots, design links, documentation, etc.) that are relevant" - type: textarea id: issue-links attributes: diff --git a/.github/ISSUE_TEMPLATE/user-story.yml b/.github/ISSUE_TEMPLATE/user-story.yml index e71b68e0f..6e2442b64 100644 --- a/.github/ISSUE_TEMPLATE/user-story.yml +++ b/.github/ISSUE_TEMPLATE/user-story.yml @@ -49,7 +49,7 @@ body: id: additional-context attributes: label: Additional Context (optional) - description: "Please include additional references (screenshots, designs, documentation, etc.) that are relevant" + description: "Please include additional references (screenshots, design links, documentation, etc.) that are relevant" - type: textarea id: issue-links attributes: From 4ac2cd5236b5fa8de74a11dd7a3d2af341eba0f1 Mon Sep 17 00:00:00 2001 From: brandonlenz Date: Thu, 8 Jun 2023 10:00:40 -0400 Subject: [PATCH 25/45] Just use "story" instead of "user story" --- .github/ISSUE_TEMPLATE/user-story.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/ISSUE_TEMPLATE/user-story.yml b/.github/ISSUE_TEMPLATE/user-story.yml index 6e2442b64..93bbc00bc 100644 --- a/.github/ISSUE_TEMPLATE/user-story.yml +++ b/.github/ISSUE_TEMPLATE/user-story.yml @@ -1,4 +1,4 @@ -name: User Story +name: Story description: Capture actionable sprint work title: "" labels: ["story"] @@ -13,9 +13,9 @@ body: - type: textarea id: story attributes: - label: User Story + label: Story description: | - Please add the "as a, I want, so that" details that describe the user story. + Please add the "as a, I want, so that" details that describe the story. If more than one "as a, I want, so that" describes the story, add multiple. Example: From 4cab4be98bbad151311551c4d62a3f9af7a7307a Mon Sep 17 00:00:00 2001 From: igorkorenfeld Date: Thu, 8 Jun 2023 10:00:23 -0400 Subject: [PATCH 26/45] Add tests for org name and address --- src/registrar/tests/test_views.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/src/registrar/tests/test_views.py b/src/registrar/tests/test_views.py index bf1cef2f2..4b5371c90 100644 --- a/src/registrar/tests/test_views.py +++ b/src/registrar/tests/test_views.py @@ -1058,6 +1058,7 @@ class TestDomainPermissions(TestWithDomainPermissions): "domain-users", "domain-users-add", "domain-nameservers", + "domain-org-name-address", "domain-authorizing-official", "domain-your-contact-information", "domain-security-email", @@ -1078,6 +1079,7 @@ class TestDomainPermissions(TestWithDomainPermissions): "domain-users", "domain-users-add", "domain-nameservers", + "domain-org-name-address", "domain-authorizing-official", "domain-your-contact-information", "domain-security-email", @@ -1315,6 +1317,23 @@ class TestDomainDetail(TestWithDomainPermissions, WebTest): ) self.assertContains(page, "Testy") + def test_domain_org_name_address(self): + """Can load domain's org name and mailing address page.""" + page = self.client.get( + reverse("domain-org-name-address", kwargs={"pk": self.domain.id}) + ) + # once on the sidebar, once in the page title, once as H1 + self.assertContains(page, "Organization name and mailing address", count=3) + + def test_domain_org_name_address_content(self): + """Org name and address information appears on the page.""" + self.domain_information.organization_name = "Town of Igorville" + self.domain_information.save() + page = self.app.get( + reverse("domain-org-name-address", kwargs={"pk": self.domain.id}) + ) + self.assertContains(page, "Town of Igorville") + def test_domain_your_contact_information(self): """Can load domain's your contact information page.""" page = self.client.get( From 881b0cca8c29856ed24f96e9f1028a9cfcd6c28c Mon Sep 17 00:00:00 2001 From: brandonlenz Date: Thu, 8 Jun 2023 10:01:08 -0400 Subject: [PATCH 27/45] Rename "user story" template to just "story" --- .github/ISSUE_TEMPLATE/{user-story.yml => story.yml} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename .github/ISSUE_TEMPLATE/{user-story.yml => story.yml} (100%) diff --git a/.github/ISSUE_TEMPLATE/user-story.yml b/.github/ISSUE_TEMPLATE/story.yml similarity index 100% rename from .github/ISSUE_TEMPLATE/user-story.yml rename to .github/ISSUE_TEMPLATE/story.yml From 97f8a710487ed0e32231eb3e01b7841bf227e1d8 Mon Sep 17 00:00:00 2001 From: Seamus Johnston Date: Thu, 8 Jun 2023 11:16:37 -0500 Subject: [PATCH 28/45] Try adding a rule for uswds-init --- src/zap.conf | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/zap.conf b/src/zap.conf index 6a5e9bf77..b79fac71e 100644 --- a/src/zap.conf +++ b/src/zap.conf @@ -27,6 +27,8 @@ 10027 OUTOFSCOPE http://app:8080/public/debug_toolbar/js/toolbar.js # USWDS.min.js contains suspicious words "query", "select", "from" in ordinary usage 10027 OUTOFSCOPE http://app:8080/public/js/uswds.min.js +# UNCLEAR WHY THIS ONE IS FAILING. Giving 404 error. +10027 OUTOFSCOPE http://app:8080/public/js/uswds-init.min.js # get-gov.js contains suspicious word "from" as in `Array.from()` 10027 OUTOFSCOPE http://app:8080/public/js/get-gov.js 10028 FAIL (Open Redirect - Passive/beta) From f4c0cbb549ad484ceda6586df8f0f05f1c807413 Mon Sep 17 00:00:00 2001 From: brandonlenz Date: Thu, 8 Jun 2023 15:30:33 -0400 Subject: [PATCH 29/45] Remove optional title attribute, since we didn't want it prefilled or prefixed --- .github/ISSUE_TEMPLATE/bug.yml | 1 - .github/ISSUE_TEMPLATE/story.yml | 1 - 2 files changed, 2 deletions(-) diff --git a/.github/ISSUE_TEMPLATE/bug.yml b/.github/ISSUE_TEMPLATE/bug.yml index a7d70057c..53533a951 100644 --- a/.github/ISSUE_TEMPLATE/bug.yml +++ b/.github/ISSUE_TEMPLATE/bug.yml @@ -1,6 +1,5 @@ name: Bug description: Report a bug or problem with the application -title: "" labels: ["bug"] body: diff --git a/.github/ISSUE_TEMPLATE/story.yml b/.github/ISSUE_TEMPLATE/story.yml index 93bbc00bc..6173dd9e1 100644 --- a/.github/ISSUE_TEMPLATE/story.yml +++ b/.github/ISSUE_TEMPLATE/story.yml @@ -1,6 +1,5 @@ name: Story description: Capture actionable sprint work -title: "" labels: ["story"] body: From 6212c625642c0ccf54d4e7aa3dde2ee949c219e4 Mon Sep 17 00:00:00 2001 From: igorkorenfeld Date: Thu, 8 Jun 2023 15:45:57 -0400 Subject: [PATCH 30/45] Update developer docs with note about custom images --- docs/developer/README.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/developer/README.md b/docs/developer/README.md index d96fa7253..bc102a64b 100644 --- a/docs/developer/README.md +++ b/docs/developer/README.md @@ -199,6 +199,8 @@ Static files (images, CSS stylesheets, JavaScripts, etc) are known as "assets". Assets are stored in `registrar/assets` during development and served from `registrar/public`. During deployment, assets are copied from `registrar/assets` into `registrar/public`. Any assets which need processing, such as USWDS Sass files, are processed before copying. +**Note:** Custom images are added to `/registrar/assets/img/registrar`, keeping them separate from the images copied over by USWDS. However, because the `/img/` directory is listed in `.gitignore`, any files added to `/registrar/assets/img/registrar` will need to be force added (i.e. `git add --force `) before they can be deployed. + We utilize the [uswds-compile tool](https://designsystem.digital.gov/documentation/getting-started/developers/phase-two-compile/) from USWDS to compile and package USWDS assets. ## Making and view style changes From 72fe97717883827b20a8e8948c9b3132b58103e7 Mon Sep 17 00:00:00 2001 From: Seamus Johnston Date: Thu, 8 Jun 2023 11:12:07 -0500 Subject: [PATCH 31/45] Respond to PR feedback --- docs/developer/registry-access.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/developer/registry-access.md b/docs/developer/registry-access.md index 4ec5440dd..d2df556e9 100644 --- a/docs/developer/registry-access.md +++ b/docs/developer/registry-access.md @@ -12,7 +12,7 @@ ## Debugging in a Python shell -You'll first need access to a Django shell in an environment with valid registry credentials. For example: +You'll first need access to a Django shell in an environment with valid registry credentials. Only some environments are allowed access: your laptop is probably not one of them. For example: ```shell cf ssh getgov-ENVIRONMENT @@ -99,3 +99,5 @@ request = commands.InfoDomain(name="ok.gov") registry._client.transport.send(request.xml()) response = registry._client.transport.receive() ``` + +This is helpful for debugging situations where epplib is not correctly or fully parsing the XML returned from the registry. From 3d6fe6a6290f8b6752c9509fb20b88c82279687d Mon Sep 17 00:00:00 2001 From: Seamus Johnston Date: Fri, 9 Jun 2023 07:37:15 -0500 Subject: [PATCH 32/45] Add note about cleaned flag --- docs/developer/registry-access.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/developer/registry-access.md b/docs/developer/registry-access.md index d2df556e9..a59c8b8b7 100644 --- a/docs/developer/registry-access.md +++ b/docs/developer/registry-access.md @@ -34,6 +34,8 @@ request = ... response = registry.send(request) ``` +Note that you'll need to attest that the data you are sending has been sanitized to remove malicious or invalid strings. Use `send(..., cleaned=True)` to do that. + See below for some example commands to send. Replace example data with data which makes sense for your debugging scenario. Other commands are available; see the source code of epplib for more options. From 8d8c2a258c864a99147b206df56c52207b9b349c Mon Sep 17 00:00:00 2001 From: igorkorenfeld Date: Fri, 9 Jun 2023 11:46:33 -0400 Subject: [PATCH 33/45] PR edit: remove lables, replace with verbose_name, comment required attributes --- src/registrar/forms/domain.py | 10 ++++------ src/registrar/models/domain_information.py | 4 ++++ 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/src/registrar/forms/domain.py b/src/registrar/forms/domain.py index 6a1077672..986085d69 100644 --- a/src/registrar/forms/domain.py +++ b/src/registrar/forms/domain.py @@ -93,12 +93,6 @@ class DomainOrgNameAddressForm(forms.ModelForm): "zipcode", "urbanization", ] - labels = { - "address_line1": "Street address", - "address_line2": "Street address line 2", - "state_territory": "State, territory, or military post", - "urbanization": "Urbanization (Puerto Rico only)", - } error_messages = { "federal_agency": { "required": "Select the federal agency for your organization." @@ -114,6 +108,10 @@ class DomainOrgNameAddressForm(forms.ModelForm): }, } widgets = { + # We need to set the required attributed for federal_agency and + # state/territory because for these fields we are creating an individual + # instance of the Select. For the other fields we use the for loop to set + # the class's required attribute to true. "federal_agency": forms.Select( attrs={"required": True}, choices=DomainInformation.AGENCY_CHOICES ), diff --git a/src/registrar/models/domain_information.py b/src/registrar/models/domain_information.py index 78b68a3fa..b12039e73 100644 --- a/src/registrar/models/domain_information.py +++ b/src/registrar/models/domain_information.py @@ -100,11 +100,13 @@ class DomainInformation(TimeStampedModel): null=True, blank=True, help_text="Street address", + verbose_name="Street address", ) address_line2 = models.TextField( null=True, blank=True, help_text="Street address line 2", + verbose_name="Street address line 2", ) city = models.TextField( null=True, @@ -116,6 +118,7 @@ class DomainInformation(TimeStampedModel): null=True, blank=True, help_text="State, territory, or military post", + verbose_name="State, territory, or military post", ) zipcode = models.CharField( max_length=10, @@ -128,6 +131,7 @@ class DomainInformation(TimeStampedModel): null=True, blank=True, help_text="Urbanization (Puerto Rico only)", + verbose_name="Urbanization (Puerto Rico only)", ) type_of_work = models.TextField( From 045ddbdd92d9d4467071f45e5ce1d4753936b83c Mon Sep 17 00:00:00 2001 From: igorkorenfeld Date: Fri, 9 Jun 2023 11:46:53 -0400 Subject: [PATCH 34/45] PR edit: additional test of the form --- src/registrar/tests/test_views.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/src/registrar/tests/test_views.py b/src/registrar/tests/test_views.py index 4b5371c90..17274aef2 100644 --- a/src/registrar/tests/test_views.py +++ b/src/registrar/tests/test_views.py @@ -1334,6 +1334,25 @@ class TestDomainDetail(TestWithDomainPermissions, WebTest): ) self.assertContains(page, "Town of Igorville") + def test_domain_org_name_address_form(self): + """Submitting changes works on the org name address page.""" + self.domain_information.organization_name = "Town of Igorville" + self.domain_information.save() + org_name_page = self.app.get( + reverse("domain-org-name-address", kwargs={"pk": self.domain.id}) + ) + session_id = self.app.cookies[settings.SESSION_COOKIE_NAME] + + org_name_page.form["organization_name"] = "Not igorville" + org_name_page.form["city"] = "Faketown" + + self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + success_result_page = org_name_page.form.submit() + self.assertEqual(success_result_page.status_code, 200) + + self.assertContains(success_result_page, "Not igorville") + self.assertContains(success_result_page, "Faketown") + def test_domain_your_contact_information(self): """Can load domain's your contact information page.""" page = self.client.get( From 2f20dbb70f0c5c82c17a47b9a4ccbc85a4c6ddfc Mon Sep 17 00:00:00 2001 From: igorkorenfeld Date: Fri, 9 Jun 2023 11:47:59 -0400 Subject: [PATCH 35/45] Replace TODO link on domain overview for org name address --- src/registrar/templates/domain_detail.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/templates/domain_detail.html b/src/registrar/templates/domain_detail.html index ef3484dfc..dd176c862 100644 --- a/src/registrar/templates/domain_detail.html +++ b/src/registrar/templates/domain_detail.html @@ -14,7 +14,7 @@ Add DNS name servers {% endif %} - {% url 'todo' as url %} + {% url 'domain-org-name-address' pk=domain.id as url %} {% include "includes/summary_item.html" with title='Organization name and mailing address' value=domain.domain_info address='true' edit_link=url %} {% url 'domain-authorizing-official' pk=domain.id as url %} From ace6b338ee3c5610eb7aeb22d541e16d6bce933a Mon Sep 17 00:00:00 2001 From: igorkorenfeld Date: Fri, 9 Jun 2023 12:05:03 -0400 Subject: [PATCH 36/45] Linting: remove trailing space --- src/registrar/forms/domain.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/registrar/forms/domain.py b/src/registrar/forms/domain.py index 986085d69..f14448bcf 100644 --- a/src/registrar/forms/domain.py +++ b/src/registrar/forms/domain.py @@ -110,8 +110,8 @@ class DomainOrgNameAddressForm(forms.ModelForm): widgets = { # We need to set the required attributed for federal_agency and # state/territory because for these fields we are creating an individual - # instance of the Select. For the other fields we use the for loop to set - # the class's required attribute to true. + # instance of the Select. For the other fields we use the for loop to set + # the class's required attribute to true. "federal_agency": forms.Select( attrs={"required": True}, choices=DomainInformation.AGENCY_CHOICES ), From 020e6763e4dadd62adda1e3e8c4fa6bbe318067a Mon Sep 17 00:00:00 2001 From: igorkorenfeld Date: Fri, 9 Jun 2023 12:39:15 -0400 Subject: [PATCH 37/45] Generate migrations file --- ...omaininformation_address_line1_and_more.py | 53 +++++++++++++++++++ 1 file changed, 53 insertions(+) create mode 100644 src/registrar/migrations/0027_alter_domaininformation_address_line1_and_more.py diff --git a/src/registrar/migrations/0027_alter_domaininformation_address_line1_and_more.py b/src/registrar/migrations/0027_alter_domaininformation_address_line1_and_more.py new file mode 100644 index 000000000..9f362c956 --- /dev/null +++ b/src/registrar/migrations/0027_alter_domaininformation_address_line1_and_more.py @@ -0,0 +1,53 @@ +# Generated by Django 4.2.1 on 2023-06-09 16:38 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("registrar", "0026_alter_domainapplication_address_line2_and_more"), + ] + + operations = [ + migrations.AlterField( + model_name="domaininformation", + name="address_line1", + field=models.TextField( + blank=True, + help_text="Street address", + null=True, + verbose_name="Street address", + ), + ), + migrations.AlterField( + model_name="domaininformation", + name="address_line2", + field=models.TextField( + blank=True, + help_text="Street address line 2", + null=True, + verbose_name="Street address line 2", + ), + ), + migrations.AlterField( + model_name="domaininformation", + name="state_territory", + field=models.CharField( + blank=True, + help_text="State, territory, or military post", + max_length=2, + null=True, + verbose_name="State, territory, or military post", + ), + ), + migrations.AlterField( + model_name="domaininformation", + name="urbanization", + field=models.TextField( + blank=True, + help_text="Urbanization (Puerto Rico only)", + null=True, + verbose_name="Urbanization (Puerto Rico only)", + ), + ), + ] From eaf98a050949bfb5202d6a57efffe2011864d6c9 Mon Sep 17 00:00:00 2001 From: Jon Roberts Date: Fri, 9 Jun 2023 11:53:22 -0600 Subject: [PATCH 38/45] added withdraw transition --- src/registrar/models/domain_application.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/registrar/models/domain_application.py b/src/registrar/models/domain_application.py index fa3a2973c..4caf26924 100644 --- a/src/registrar/models/domain_application.py +++ b/src/registrar/models/domain_application.py @@ -541,6 +541,10 @@ class DomainApplication(TimeStampedModel): user=self.creator, domain=created_domain, role=UserDomainRole.Roles.ADMIN ) + @transition(field="status", source=[SUBMITTED, INVESTIGATING], target=WITHDRAWN) + def withdraw(self): + """Withdraw an application that has been submitted.""" + # ## Form policies ### # # These methods control what questions need to be answered by applicants From 0ca6889ee3bdeba07d3c3a9ecbc12cec84b8002a Mon Sep 17 00:00:00 2001 From: Neil Martinsen-Burrell Date: Fri, 9 Jun 2023 13:16:24 -0500 Subject: [PATCH 39/45] New OWASP false positives with MockLogin --- src/zap.conf | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/zap.conf b/src/zap.conf index 0a219ae06..b2234250e 100644 --- a/src/zap.conf +++ b/src/zap.conf @@ -64,6 +64,7 @@ 10038 OUTOFSCOPE http://app:8080/todo # OIDC isn't configured in the test environment and DEBUG=True so this gives a 500 without CSP headers 10038 OUTOFSCOPE http://app:8080/openid/login/ +10038 OUTOFSCOPE http://app:8080/openid/logout/ 10039 FAIL (X-Backend-Server Header Information Leak - Passive/beta) 10040 FAIL (Secure Pages Include Mixed Content - Passive/release) 10041 FAIL (HTTP to HTTPS Insecure Transition in Form Post - Passive/beta) @@ -150,6 +151,7 @@ # OIDC isn't configured in the test environment and DEBUG=True so these error pages # trigger this rule in a way that they won't in production 90022 OUTOFSCOPE http://app:8080/openid/login/ +90022 OUTOFSCOPE http://app:8080/openid/logout/ 90023 FAIL (XML External Entity Attack - Active/beta) 90024 FAIL (Generic Padding Oracle - Active/beta) 90025 FAIL (Expression Language Injection - Active/beta) From 3b232a334637723195598ca1c9dcceb31fca4c53 Mon Sep 17 00:00:00 2001 From: Jon Roberts Date: Fri, 9 Jun 2023 12:26:34 -0600 Subject: [PATCH 40/45] edit withdraw to use .withdraw() instead of manually setting it --- src/registrar/views/application.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/views/application.py b/src/registrar/views/application.py index b8454bf78..fb1cdff21 100644 --- a/src/registrar/views/application.py +++ b/src/registrar/views/application.py @@ -502,6 +502,6 @@ class ApplicationWithdrawn(DomainApplicationPermissionView): to withdraw and send back to homepage. """ application = DomainApplication.objects.get(id=self.kwargs["pk"]) - application.status = "withdrawn" + application.withdraw() application.save() return HttpResponseRedirect(reverse("home")) From 45f317d11334ac4aafda527a4fce3dc07fbf1e58 Mon Sep 17 00:00:00 2001 From: Neil Martinsen-Burrell Date: Fri, 9 Jun 2023 13:51:26 -0500 Subject: [PATCH 41/45] Fix Disable Login step --- .github/workflows/security-check.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/security-check.yaml b/.github/workflows/security-check.yaml index b2d840c84..dd75d5c98 100644 --- a/.github/workflows/security-check.yaml +++ b/.github/workflows/security-check.yaml @@ -73,7 +73,7 @@ jobs: run: | perl -pi \ -e 's/"django.contrib.auth.middleware.AuthenticationMiddleware",/$&"registrar.tests.common.MockUserLogin",/' \ - src/registrar/config/settings.py + registrar/config/settings.py working-directory: ./src - name: OWASP scan From 2fc070e6c5e41c12f03882a0426a11f0983f76fb Mon Sep 17 00:00:00 2001 From: Neil Martinsen-Burrell Date: Fri, 9 Jun 2023 14:17:55 -0500 Subject: [PATCH 42/45] Review feedback: updated developer documentation --- docs/developer/user-permissions.md | 22 ++++++++++++++++------ src/zap.conf | 4 ++++ 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/docs/developer/user-permissions.md b/docs/developer/user-permissions.md index 281a58c1b..af5aa1259 100644 --- a/docs/developer/user-permissions.md +++ b/docs/developer/user-permissions.md @@ -22,12 +22,22 @@ role or set of permissions that they have. We use a `UserDomainRole` ## 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". +For that purpose, we have a View subclass to enforce user permissions on a +domain called +[`DomainPermissionView`](../../src/registrar/views/utility/permission_views.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". + +In addition, we now require all of our application views to have a logged-in +user by using a Django middleware that makes every request "login required". +This is slightly belt-and-suspenders because our permissions view also checks +that the request includes a logged in user, but it avoids accidentally creating +content that is publicly available by accident. We can specifically mark a view +as "not login required" if we do need to have publicly accessible content (such +as health checks used by our platform). ## Adding roles diff --git a/src/zap.conf b/src/zap.conf index b2234250e..e5e7b4d04 100644 --- a/src/zap.conf +++ b/src/zap.conf @@ -87,6 +87,10 @@ 10062 FAIL (PII Disclosure - Passive/beta) 10095 FAIL (Backup File Disclosure - Active/beta) 10096 FAIL (Timestamp Disclosure - Passive/release) +# Our sortable table of domains uses timestamps as sort keys so this appears as +# a false-positive to the OWASP scanner +10096 OUTOFSCOPE http://app:8080 +10096 OUTOFSCOPE http://app:8080/ 10097 FAIL (Hash Disclosure - Passive/beta) 10098 FAIL (Cross-Domain Misconfiguration - Passive/release) 10104 FAIL (User Agent Fuzzer - Active/beta) From 995250e4dde94a583f4f371ce7ecb4582fadfd01 Mon Sep 17 00:00:00 2001 From: Michelle Rago <60157596+michelle-rago@users.noreply.github.com> Date: Fri, 9 Jun 2023 16:16:02 -0400 Subject: [PATCH 43/45] Link and text updates (#700) * Update application_dotgov_domain.html * Update footer.html * Update application_authorizing_official.html * Update application_dotgov_domain.html * Update 403.html * Update 403.html * Update 500.html * Update 500.html * Update 403.html * Update 404.html * Update domain_security_email.html * Update 401.html * Update domain_authorizing_official.html * Update src/registrar/templates/includes/footer.html Co-authored-by: Neil MartinsenBurrell * Remove todo styling from public site links * Update 401.html * Update footer.html --------- Co-authored-by: Neil MartinsenBurrell Co-authored-by: igorkorenfeld --- .../_theme/_uswds-theme-custom-styles.scss | 1 - src/registrar/templates/401.html | 3 ++- src/registrar/templates/403.html | 7 ++++--- src/registrar/templates/404.html | 3 ++- src/registrar/templates/500.html | 3 ++- .../application_authorizing_official.html | 11 ++++------ .../templates/application_dotgov_domain.html | 6 +++--- .../domain_authorizing_official.html | 6 ++---- .../templates/domain_security_email.html | 4 ++-- src/registrar/templates/includes/footer.html | 21 +++++++------------ 10 files changed, 29 insertions(+), 36 deletions(-) diff --git a/src/registrar/assets/sass/_theme/_uswds-theme-custom-styles.scss b/src/registrar/assets/sass/_theme/_uswds-theme-custom-styles.scss index 8c06730bf..905ab872c 100644 --- a/src/registrar/assets/sass/_theme/_uswds-theme-custom-styles.scss +++ b/src/registrar/assets/sass/_theme/_uswds-theme-custom-styles.scss @@ -124,7 +124,6 @@ h2 { } /* Make "placeholder" links visually obvious */ -a[href^="https://federalist-"]::after, a[href$="todo"]::after { background-color: yellow; color: color(blue-80v); diff --git a/src/registrar/templates/401.html b/src/registrar/templates/401.html index 9fe0194ed..a419dab53 100644 --- a/src/registrar/templates/401.html +++ b/src/registrar/templates/401.html @@ -1,5 +1,6 @@ {% extends "base.html" %} {% load i18n static %} +{% load url_helpers %} {% block title %}{% translate "Unauthorized | " %}{% endblock %} @@ -25,7 +26,7 @@ Would you like to try logging in again?

    - If you would like help with this error contact us + If you'd like help with this error contact us .

    {% if log_identifier %} diff --git a/src/registrar/templates/403.html b/src/registrar/templates/403.html index cc8c98656..93d32f65a 100644 --- a/src/registrar/templates/403.html +++ b/src/registrar/templates/403.html @@ -1,5 +1,6 @@ {% extends "base.html" %} {% load i18n static %} +{% load url_helpers %} {% block title %}{% translate "Forbidden | " %}{% endblock %} @@ -22,12 +23,12 @@ {% endif %}

    You must be an authorized user and need to be signed in to view this page. - Would you like to try logging in again? + Would you like to try logging in again?

    - If you would like help with this error contact us + If you'd like help with this error contact us .

    - + {% if log_identifier %}

    Here's a unique identifier for this error.

    {{ log_identifier }}

    diff --git a/src/registrar/templates/404.html b/src/registrar/templates/404.html index 76a301187..deec71ce2 100644 --- a/src/registrar/templates/404.html +++ b/src/registrar/templates/404.html @@ -1,5 +1,6 @@ {% extends "base.html" %} {% load i18n static %} +{% load url_helpers %} {% block title %}{% translate "Page not found | " %}{% endblock %} @@ -14,7 +15,7 @@ {% translate "Status 404" %} -

    Try going to the homepage. If you can’t find what you’re looking for, contact us. +

    Try going to the homepage. If you can’t find what you’re looking for, contact us .

    diff --git a/src/registrar/templates/500.html b/src/registrar/templates/500.html index 7b7e1dfed..5749c0ad3 100644 --- a/src/registrar/templates/500.html +++ b/src/registrar/templates/500.html @@ -1,5 +1,6 @@ {% extends "base.html" %} {% load i18n static %} +{% load url_helpers %} {% block title %}{% translate "Server error | " %}{% endblock %} @@ -18,7 +19,7 @@ {% else %}

    Sorry! Try waiting a few minutes and then reloading the page. - Contact us if you need help. + contact us if you need help.

    {% endif %} diff --git a/src/registrar/templates/application_authorizing_official.html b/src/registrar/templates/application_authorizing_official.html index 6648816ce..dead1974f 100644 --- a/src/registrar/templates/application_authorizing_official.html +++ b/src/registrar/templates/application_authorizing_official.html @@ -1,22 +1,19 @@ {% extends 'application_form.html' %} -{% load field_helpers %} +{% load field_helpers url_helpers %} {% block form_instructions %}

    Who is the authorizing official for your organization?

    -

    Your authorizing official is the person within your organization who can authorize - your domain request. This is generally the highest-ranking or highest-elected official - in your organization. Read more about who can serve as an - authorizing official.

    +

    Your authorizing official is the person within your organization who can authorize your domain request. This is generally the highest-ranking or highest-elected official in your organization.

    {% include "includes/ao_example.html" %}
    -

    We’ll contact your authorizing official to let them know that you made this request - and to double check that they approve it.

    +

    We might contact your authorizing official, or their office, to double check that they approve this request. Read more about who can serve as an authorizing official.

    + {% endblock %} diff --git a/src/registrar/templates/application_dotgov_domain.html b/src/registrar/templates/application_dotgov_domain.html index a6b9d1037..8b53d5695 100644 --- a/src/registrar/templates/application_dotgov_domain.html +++ b/src/registrar/templates/application_dotgov_domain.html @@ -1,9 +1,9 @@ {% extends 'application_form.html' %} -{% load static field_helpers %} +{% load static field_helpers url_helpers %} {% block form_instructions %} -

    Before requesting a .gov domain, please make sure it - meets our naming requirements. Your domain name must: +

    Before requesting a .gov domain, please make sure it + meets our naming requirements. Your domain name must: