diff --git a/.github/workflows/security-check.yaml b/.github/workflows/security-check.yaml index be29dbf05..06c717e0a 100644 --- a/.github/workflows/security-check.yaml +++ b/.github/workflows/security-check.yaml @@ -35,6 +35,17 @@ jobs: name: security-check-output path: ./src/output.txt + backdoor-check: + name: Ensure custom mods are contained + runs-on: ubuntu-latest + + steps: + - name: Check out + uses: actions/checkout@v3 + - name: MockUserLogin should not be in settings.MIDDLEWARE + run: "! grep -rwn * --exclude-dir=node_modules -e registrar.tests.common.MockUserLogin" + working-directory: ./src + owasp-scan: name: OWASP security scan runs-on: ubuntu-latest @@ -42,6 +53,15 @@ jobs: steps: - name: Check out uses: actions/checkout@v3 + + - name: Disable Login + # by adding MockUserLogin to settings.MIDDLEWARE + run: | + perl -pi \ + -e 's/"csp.middleware.CSPMiddleware",/$&"registrar.tests.common.MockUserLogin",/' \ + src/registrar/config/settings.py + working-directory: ./src + - name: OWASP scan run: docker compose run owasp working-directory: ./src diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index 9dd0eaf14..b6ca7d804 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -35,6 +35,14 @@ jobs: steps: - uses: actions/checkout@v3 + - name: Disable Login + working-directory: ./src + # by adding MockUserLogin to settings.MIDDLEWARE + run: | + perl -pi \ + -e 's/"csp.middleware.CSPMiddleware",/$&"registrar.tests.common.MockUserLogin",/' \ + src/registrar/config/settings.py + - name: Accessibility Scan working-directory: ./src # leverage the docker compose setup that we already have for local development diff --git a/docs/developer/README.md b/docs/developer/README.md index b04d27810..58fb25739 100644 --- a/docs/developer/README.md +++ b/docs/developer/README.md @@ -68,7 +68,17 @@ Linters: docker-compose exec app ./manage.py lint ``` -## Accessibility Scanning +### Testing behind logged in pages + +To test behind logged in pages with external tools, like `pa11y-ci` or `OWASP Zap`, add + +``` +"registrar.tests.common.MockUserLogin" +``` + +to MIDDLEWARE in settings.py. **Remove it when you are finished testing.** + +### Accessibility Scanning The tool `pa11y-ci` is used to scan pages for compliance with a set of accessibility rules. The scan runs as part of our CI setup (see @@ -82,6 +92,17 @@ docker-compose run pa11y npm run pa11y-ci The URLs that `pa11y-ci` will scan are configured in `src/.pa11yci`. When new views and pages are added, their URLs should also be added to that file. +### Security Scanning + +The tool OWASP Zap is used for scanning the codebase for compliance with +security rules. The scan runs as part of our CI setup (see +`.github/workflows/test.yaml`) but it can also be run locally. To run locally, +type + +```shell +docker-compose run owasp +``` + ## USWDS and styling We use the U.S. Web Design System (USWDS) for building and styling our applications. Additionally, we utilize the [uswds-compile tool](https://designsystem.digital.gov/documentation/getting-started/developers/phase-two-compile/) from USWDS to compile and package the static assets. When you run `docker-compose up` the `node` service in the container will begin to watch for changes in the `registrar/assets` folder, and will recompile once any changes are made. diff --git a/src/.pa11yci b/src/.pa11yci index 0f0aa5174..5dcd9e2f0 100644 --- a/src/.pa11yci +++ b/src/.pa11yci @@ -1,6 +1,7 @@ { "urls": [ "http://app:8080/", - "http://app:8080/health/" + "http://app:8080/health/", + "http://app:8080/whoami/" ] } diff --git a/src/djangooidc/tests/test_views.py b/src/djangooidc/tests/test_views.py index 0d28ae355..2895a6099 100644 --- a/src/djangooidc/tests/test_views.py +++ b/src/djangooidc/tests/test_views.py @@ -33,7 +33,7 @@ class ViewsTest(TestCase): # mock mock_client.create_authn_request.side_effect = self.say_hi # test - response = self.client.get(reverse("openid"), {"next": callback_url}) + response = self.client.get(reverse("login"), {"next": callback_url}) # assert session = mock_client.create_authn_request.call_args[0][0] self.assertEqual(session["next"], callback_url) @@ -45,7 +45,7 @@ class ViewsTest(TestCase): mock_client.create_authn_request.side_effect = Exception("Test") # test with less_console_noise(): - response = self.client.get(reverse("openid")) + response = self.client.get(reverse("login")) # assert self.assertEqual(response.status_code, 500) self.assertTemplateUsed(response, "500.html") diff --git a/src/djangooidc/urls.py b/src/djangooidc/urls.py index ecd3a81a2..53b1356cb 100644 --- a/src/djangooidc/urls.py +++ b/src/djangooidc/urls.py @@ -5,7 +5,7 @@ from django.urls import path from . import views urlpatterns = [ - path("login/", views.openid, name="openid"), + path("login/", views.openid, name="login"), path("callback/login/", views.login_callback, name="openid_login_callback"), path("logout/", views.logout, name="logout"), path("callback/logout/", views.logout_callback, name="openid_logout_callback"), diff --git a/src/registrar/config/settings.py b/src/registrar/config/settings.py index c4523ed7b..6c8b34c27 100644 --- a/src/registrar/config/settings.py +++ b/src/registrar/config/settings.py @@ -403,7 +403,7 @@ AUTHENTICATION_BACKENDS = [ # this is where unauthenticated requests are redirected when using # the login_required() decorator, LoginRequiredMixin, or AccessMixin -LOGIN_URL = "openid/login" +LOGIN_URL = "/openid/login" # where to go after logging out LOGOUT_REDIRECT_URL = "home" diff --git a/src/registrar/config/urls.py b/src/registrar/config/urls.py index 0a9ff6da9..8468b4208 100644 --- a/src/registrar/config/urls.py +++ b/src/registrar/config/urls.py @@ -4,20 +4,35 @@ For more information see: https://docs.djangoproject.com/en/4.0/topics/http/urls/ """ +from django.conf import settings from django.contrib import admin from django.urls import include, path +from django.views.generic import RedirectView from registrar.views import health, index, profile, whoami urlpatterns = [ path("", index.index, name="home"), - path("whoami", whoami.whoami, name="whoami"), + path("whoami/", whoami.whoami, name="whoami"), path("admin/", admin.site.urls), path("health/", health.health), path("edit_profile/", profile.edit_profile, name="edit-profile"), path("openid/", include("djangooidc.urls")), ] +if not settings.DEBUG: + urlpatterns += [ + # redirect to login.gov + path( + "admin/login/", RedirectView.as_view(pattern_name="login", permanent=False) + ), + # redirect to login.gov + path( + "admin/logout/", + RedirectView.as_view(pattern_name="logout", permanent=False), + ), + ] + # we normally would guard these with `if settings.DEBUG` but tests run with # DEBUG = False even when these apps have been loaded because settings.DEBUG # was actually True. Instead, let's add these URLs any time we are able to diff --git a/src/registrar/templates/401.html b/src/registrar/templates/401.html index 23a48b8bd..0785da4c5 100644 --- a/src/registrar/templates/401.html +++ b/src/registrar/templates/401.html @@ -12,7 +12,7 @@

{% translate "Authorization failed." %}

{% endif %} -

+

{% translate "Would you like to try logging in again?" %}

diff --git a/src/registrar/tests/common.py b/src/registrar/tests/common.py index e69de29bb..2d6a89fef 100644 --- a/src/registrar/tests/common.py +++ b/src/registrar/tests/common.py @@ -0,0 +1,25 @@ +from django.conf import settings +from django.contrib.auth import get_user_model, login + + +class MockUserLogin: + def __init__(self, get_response): + self.get_response = get_response + + def __call__(self, request): + if request.user.is_anonymous: + user = None + UserModel = get_user_model() + username = "Testy" + args = { + UserModel.USERNAME_FIELD: username, + } + user = UserModel.objects.get_or_create(**args) + user.is_staff = True + user.is_superuser = True + user.save() + backend = settings.AUTHENTICATION_BACKENDS[-1] + login(request, user, backend=backend) + + response = self.get_response(request) + return response diff --git a/src/registrar/tests/test_views.py b/src/registrar/tests/test_views.py index 7209c12b4..b124cad5a 100644 --- a/src/registrar/tests/test_views.py +++ b/src/registrar/tests/test_views.py @@ -18,9 +18,9 @@ class TestViews(TestCase): def test_whoami_page_no_user(self): """Whoami page not accessible without a logged-in user.""" - response = self.client.get("/whoami") + response = self.client.get("/whoami/") self.assertEqual(response.status_code, 302) - self.assertIn("?next=/whoami", response.headers["Location"]) + self.assertIn("?next=/whoami/", response.headers["Location"]) class LoggedInTests(TestCase): @@ -36,7 +36,7 @@ class LoggedInTests(TestCase): def test_whoami_page(self): """User information appears on the whoami page.""" - response = self.client.get("/whoami") + response = self.client.get("/whoami/") self.assertContains(response, self.user.first_name) self.assertContains(response, self.user.last_name) self.assertContains(response, self.user.email)