Add checks for RP_ID, add a 2FA section in README

README.md

@@ -76,39 +76,45 @@ visit `http://localhost:8000/admin`
 
 ## Configuration
 
-Site configuration is loaded from environment variables. This is
-done in `hc/settings.py`. Additional configuration is loaded
-from `hc/local_settings.py` file, if it exists. You can create this file
-(should be right next to `settings.py` in the filesystem) and override
-settings, or add extra settings as needed.
+Healthchecks prepares its configuration in `hc/settings.py`. It reads configuration
+from two places:
 
-Configurations settings loaded from environment variables:
+ * environment variables (see the variable names in the table below)
+ * it imports configuration for `hc/local_settings.py` file, if it exists
+
+You can use either mechanism, depending on what is more convenient. Using
+`hc/local_settings.py` allows more flexibility: you can set
+each and every [Django setting](https://docs.djangoproject.com/en/3.1/ref/settings/),
+you can run Python code to load configuration from an external source.
+
+Healthchecks reads configuration from the following environment variables:
 
 | Environment variable | Default value | Notes
 | -------------------- | ------------- | ----- |
-| [SECRET_KEY](https://docs.djangoproject.com/en/2.2/ref/settings/#secret-key) | `"---"`
-| [DEBUG](https://docs.djangoproject.com/en/2.2/ref/settings/#debug) | `True` | Set to `False` for production
-| [ALLOWED_HOSTS](https://docs.djangoproject.com/en/2.2/ref/settings/#allowed-hosts) | `*` | Separate multiple hosts with commas
-| [DEFAULT_FROM_EMAIL](https://docs.djangoproject.com/en/2.2/ref/settings/#default-from-email) | `"[email protected]"`
+| [SECRET_KEY](https://docs.djangoproject.com/en/3.1/ref/settings/#secret-key) | `"---"`
+| [DEBUG](https://docs.djangoproject.com/en/3.1/ref/settings/#debug) | `True` | Set to `False` for production
+| [ALLOWED_HOSTS](https://docs.djangoproject.com/en/3.1/ref/settings/#allowed-hosts) | `*` | Separate multiple hosts with commas
+| [DEFAULT_FROM_EMAIL](https://docs.djangoproject.com/en/3.1/ref/settings/#default-from-email) | `"[email protected]"`
 | USE_PAYMENTS | `False`
 | REGISTRATION_OPEN | `True`
 | DB | `"sqlite"` | Set to `"postgres"` or `"mysql"`
-| [DB_HOST](https://docs.djangoproject.com/en/2.2/ref/settings/#host) | `""` *(empty string)*
-| [DB_PORT](https://docs.djangoproject.com/en/2.2/ref/settings/#port) | `""` *(empty string)*
-| [DB_NAME](https://docs.djangoproject.com/en/2.2/ref/settings/#name) | `"hc"` (PostgreSQL, MySQL) or `"/path/to/project/hc.sqlite"` (SQLite) | For SQLite, specify the full path to the database file.
-| [DB_USER](https://docs.djangoproject.com/en/2.2/ref/settings/#user) | `"postgres"` or `"root"`
-| [DB_PASSWORD](https://docs.djangoproject.com/en/2.2/ref/settings/#password) | `""` *(empty string)*
-| [DB_CONN_MAX_AGE](https://docs.djangoproject.com/en/2.2/ref/settings/#conn-max-age) | `0`
+| [DB_HOST](https://docs.djangoproject.com/en/3.1/ref/settings/#host) | `""` *(empty string)*
+| [DB_PORT](https://docs.djangoproject.com/en/3.1/ref/settings/#port) | `""` *(empty string)*
+| [DB_NAME](https://docs.djangoproject.com/en/3.1/ref/settings/#name) | `"hc"` (PostgreSQL, MySQL) or `"/path/to/project/hc.sqlite"` (SQLite) | For SQLite, specify the full path to the database file.
+| [DB_USER](https://docs.djangoproject.com/en/3.1/ref/settings/#user) | `"postgres"` or `"root"`
+| [DB_PASSWORD](https://docs.djangoproject.com/en/3.1/ref/settings/#password) | `""` *(empty string)*
+| [DB_CONN_MAX_AGE](https://docs.djangoproject.com/en/3.1/ref/settings/#conn-max-age) | `0`
 | DB_SSLMODE | `"prefer"` | PostgreSQL-specific, [details](https://blog.github.com/2018-10-21-october21-incident-report/)
 | DB_TARGET_SESSION_ATTRS | `"read-write"` | PostgreSQL-specific, [details](https://www.postgresql.org/docs/10/static/libpq-connect.html#LIBPQ-CONNECT-TARGET-SESSION-ATTRS)
-| EMAIL_HOST | `""` *(empty string)*
-| EMAIL_PORT | `"587"`
-| EMAIL_HOST_USER | `""` *(empty string)*
-| EMAIL_HOST_PASSWORD | `""` *(empty string)*
-| EMAIL_USE_TLS | `"True"`
-| EMAIL_USE_VERIFICATION | `"True"`
+| [EMAIL_HOST](https://docs.djangoproject.com/en/3.1/ref/settings/#email-host) | `""` *(empty string)*
+| [EMAIL_PORT](https://docs.djangoproject.com/en/3.1/ref/settings/#email-port) | `"587"`
+| [EMAIL_HOST_USER](https://docs.djangoproject.com/en/3.1/ref/settings/#email-host-user) | `""` *(empty string)*
+| [EMAIL_HOST_PASSWORD](https://docs.djangoproject.com/en/3.1/ref/settings/#email-host-password) | `""` *(empty string)*
+| [EMAIL_USE_TLS](https://docs.djangoproject.com/en/3.1/ref/settings/#email-use-tls) | `"True"`
+| EMAIL_USE_VERIFICATION | `"True"` | Whether to send confirmation links when adding email integrations
 | SITE_ROOT | `"http://localhost:8000"`
 | SITE_NAME | `"Mychecks"`
+| RP_ID | `None` | Enables WebAuthn support
 | MASTER_BADGE_LABEL | `"Mychecks"`
 | PING_ENDPOINT | `"http://localhost:8000/ping/"`
 | PING_EMAIL_DOMAIN | `"localhost"`
@@ -310,6 +316,19 @@ test them on a copy of your database, not on the live database right away.
 In a production setup, you should also have regular, automated database
 backups set up.
 
+## Two-factor Authentication
+
+Healthchecks optionally supports two-factor authentication using the WebAuthn
+standard. To enable WebAuthn support, set the `RP_ID` (relying party identifier )
+setting to a non-null value. Set its value to your site's domain without scheme
+and without port. For example, if your site runs on `https://my-hc.example.org`,
+set `RP_ID` to `my-hc.example.org`.
+
+Note that WebAuthn requires HTTPS, even if running on localhost. To test WebAuthn
+locally with a self-signed certificate, you can use the `runsslserver` command
+from the `django-sslserver` package.
+
+
 ## Integrations
 
 ### Slack

hc/accounts/tests/test_add_credential.py

@@ -1,9 +1,11 @@
 from unittest.mock import patch
 
+from django.test.utils import override_settings
 from hc.test import BaseTestCase
 from hc.accounts.models import Credential
 
 
+@override_settings(RP_ID="testserver")
 class AddCredentialTestCase(BaseTestCase):
     def setUp(self):
         super().setUp()
@@ -16,6 +18,14 @@ class AddCredentialTestCase(BaseTestCase):
         r = self.client.get(self.url)
         self.assertContains(r, "We have sent a confirmation code")
 
+    @override_settings(RP_ID=None)
+    def test_it_requires_rp_id(self):
+        self.client.login(username="[email protected]", password="password")
+        self.set_sudo_flag()
+
+        r = self.client.get(self.url)
+        self.assertEqual(r.status_code, 404)
+
     def test_it_shows_form(self):
         self.client.login(username="[email protected]", password="password")
         self.set_sudo_flag()

hc/accounts/tests/test_login_webauthn.py

@@ -1,8 +1,10 @@
 from unittest.mock import patch
 
+from django.test.utils import override_settings
 from hc.test import BaseTestCase
 
 
+@override_settings(RP_ID="testserver")
 class LoginWebauthnTestCase(BaseTestCase):
     def setUp(self):
         super().setUp()
@@ -22,6 +24,11 @@ class LoginWebauthnTestCase(BaseTestCase):
         # It should put a "state" key in the session:
         self.assertIn("state", self.client.session)
 
+    @override_settings(RP_ID=None)
+    def test_it_requires_rp_id(self):
+        r = self.client.get(self.url)
+        self.assertEqual(r.status_code, 500)
+
     @patch("hc.accounts.views._check_credential")
     def test_it_logs_in(self, mock_check_credential):
         mock_check_credential.return_value = True

hc/accounts/tests/test_profile.py

@@ -1,7 +1,7 @@
 from datetime import timedelta as td
 from django.core import mail
 
-from django.conf import settings
+from django.test.utils import override_settings
 from django.utils.timezone import now
 from hc.test import BaseTestCase
 from hc.accounts.models import Credential
@@ -9,6 +9,12 @@ from hc.api.models import Check
 
 
 class ProfileTestCase(BaseTestCase):
+    def test_it_shows_profile_page(self):
+        self.client.login(username="[email protected]", password="password")
+
+        r = self.client.get("/accounts/profile/")
+        self.assertContains(r, "Email and Password")
+
     def test_it_sends_report(self):
         check = Check(project=self.project, name="Test Check")
         check.last_ping = now()
@@ -118,6 +124,22 @@ class ProfileTestCase(BaseTestCase):
         r = self.client.get("/accounts/profile/")
         self.assertContains(r, "You do not have any projects. Create one!")
 
+    @override_settings(RP_ID=None)
+    def test_it_hides_2fa_section_if_rp_id_not_set(self):
+        self.client.login(username="[email protected]", password="password")
+
+        r = self.client.get("/accounts/profile/")
+        self.assertNotContains(r, "Two-factor Authentication")
+
+    @override_settings(RP_ID="testserver")
+    def test_it_handles_no_credentials(self):
+        self.client.login(username="[email protected]", password="password")
+
+        r = self.client.get("/accounts/profile/")
+        self.assertContains(r, "Two-factor Authentication")
+        self.assertContains(r, "Your account has no registered security keys")
+
+    @override_settings(RP_ID="testserver")
     def test_it_shows_security_key(self):
         Credential.objects.create(user=self.alice, name="Alices Key")
 

hc/accounts/tests/test_remove_credential.py

@@ -1,7 +1,10 @@
+from django.test.utils import override_settings
+
 from hc.test import BaseTestCase
 from hc.accounts.models import Credential
 
 
+@override_settings(RP_ID="testserver")
 class RemoveCredentialTestCase(BaseTestCase):
     def setUp(self):
         super().setUp()
@@ -15,6 +18,14 @@ class RemoveCredentialTestCase(BaseTestCase):
         r = self.client.get(self.url)
         self.assertContains(r, "We have sent a confirmation code")
 
+    @override_settings(RP_ID=None)
+    def test_it_requires_rp_id(self):
+        self.client.login(username="[email protected]", password="password")
+        self.set_sudo_flag()
+
+        r = self.client.get(self.url)
+        self.assertEqual(r.status_code, 404)
+
     def test_it_shows_form(self):
         self.client.login(username="[email protected]", password="password")
         self.set_sudo_flag()

hc/accounts/tests/test_sudo_mode.py

@@ -11,7 +11,7 @@ class SudoModeTestCase(BaseTestCase):
         super().setUp()
 
         self.c = Credential.objects.create(user=self.alice, name="Alices Key")
-        self.url = f"/accounts/two_factor/{self.c.code}/remove/"
+        self.url = f"/accounts/set_password/"
 
     def test_it_sends_code(self):
         self.client.login(username="[email protected]", password="password")
@@ -60,7 +60,7 @@ class SudoModeTestCase(BaseTestCase):
         session.save()
 
         r = self.client.get(self.url)
-        self.assertContains(r, "Remove Security Key")
+        self.assertContains(r, "Please pick a password")
 
     def test_it_uses_rate_limiting(self):
         self.client.login(username="[email protected]", password="password")

hc/accounts/views.py

@@ -12,7 +12,7 @@ from django.contrib.auth import authenticate, update_session_auth_hash
 from django.contrib.auth.decorators import login_required
 from django.contrib.auth.models import User
 from django.core import signing
-from django.http import HttpResponseForbidden, HttpResponseBadRequest
+from django.http import HttpResponse, HttpResponseForbidden, HttpResponseBadRequest
 from django.shortcuts import get_object_or_404, redirect, render
 from django.utils.timezone import now
 from django.urls import resolve, reverse, Resolver404
@@ -223,6 +223,7 @@ def profile(request):
         "added_credential_name": request.session.pop("added_credential_name", ""),
         "removed_credential_name": request.session.pop("removed_credential_name", ""),
         "credentials": request.user.credentials.order_by("id"),
+        "use_2fa": settings.RP_ID,
     }
 
     if ctx["added_credential_name"]:
@@ -594,6 +595,9 @@ def _get_credential_data(request, form):
 @login_required
 @require_sudo_mode
 def add_credential(request):
+    if not settings.RP_ID:
+        return HttpResponse(status=404)
+
     if request.method == "POST":
         form = forms.AddCredentialForm(request.POST)
         if not form.is_valid():
@@ -630,6 +634,9 @@ def add_credential(request):
 @login_required
 @require_sudo_mode
 def remove_credential(request, code):
+    if not settings.RP_ID:
+        return HttpResponse(status=404)
+
     try:
         credential = Credential.objects.get(user=request.user, code=code)
     except Credential.DoesNotExist:
@@ -669,6 +676,10 @@ def login_webauthn(request):
     if "2fa_user_id" not in request.session:
         return HttpResponseBadRequest()
 
+    # We require RP_ID. Fail predicably if it is not set:
+    if not settings.RP_ID:
+        return HttpResponse(status=500)
+
     user = User.objects.get(id=request.session["2fa_user_id"])
     credentials = [c.unpack() for c in user.credentials.all()]
 

hc/settings.py

@@ -165,7 +165,7 @@ COMPRESS_OFFLINE = True
 COMPRESS_CSS_HASHING_METHOD = "content"
 
 # Webauthn
-RP_ID = "localhost"
+RP_ID = os.getenv("RP_ID")
 
 # Discord integration
 DISCORD_CLIENT_ID = os.getenv("DISCORD_CLIENT_ID")

templates/accounts/profile.html

@@ -62,6 +62,7 @@
             {% endif %}
         </div>
 
+        {% if use_2fa %}
         <div class="panel panel-{{ 2fa_status }}">
             <div class="panel-body settings-block">
                 <form method="post">
@@ -112,7 +113,7 @@
             </div>
             {% endif %}
         </div>
-
+        {% endif %}
 
         <div class="panel panel-{{ my_projects_status }}">
             <div class="panel-body settings-block">