From 2ac0f87560c8b4ba0f951de63ae468672c642b7a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C4=93teris=20Caune?= Date: Sat, 14 Nov 2020 11:45:09 +0200 Subject: [PATCH] Implement a "Remove Security Key" feature --- hc/accounts/forms.py | 2 +- hc/accounts/migrations/0034_credential.py | 4 +-- hc/accounts/models.py | 2 +- hc/accounts/urls.py | 5 +++ hc/accounts/views.py | 34 +++++++++++++++++- static/js/add_credential.js | 9 ++++- templates/accounts/add_credential.html | 7 +++- templates/accounts/profile.html | 25 +++++++++++--- templates/accounts/remove_credential.html | 42 +++++++++++++++++++++++ 9 files changed, 118 insertions(+), 12 deletions(-) create mode 100644 templates/accounts/remove_credential.html diff --git a/hc/accounts/forms.py b/hc/accounts/forms.py index 9b1f5dc2..ad8f11f0 100644 --- a/hc/accounts/forms.py +++ b/hc/accounts/forms.py @@ -119,7 +119,7 @@ class TransferForm(forms.Form): class AddCredentialForm(forms.Form): - name = forms.CharField(max_length=100, required=False) + name = forms.CharField(max_length=100) client_data_json = forms.CharField(required=True) attestation_object = forms.CharField(required=True) diff --git a/hc/accounts/migrations/0034_credential.py b/hc/accounts/migrations/0034_credential.py index 2c67619e..37ea1704 100644 --- a/hc/accounts/migrations/0034_credential.py +++ b/hc/accounts/migrations/0034_credential.py @@ -1,4 +1,4 @@ -# Generated by Django 3.1.2 on 2020-11-12 15:29 +# Generated by Django 3.1.2 on 2020-11-14 09:29 from django.conf import settings from django.db import migrations, models @@ -19,7 +19,7 @@ class Migration(migrations.Migration): fields=[ ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), ('code', models.UUIDField(default=uuid.uuid4, unique=True)), - ('name', models.CharField(blank=True, max_length=200)), + ('name', models.CharField(max_length=100)), ('created', models.DateTimeField(auto_now_add=True)), ('data', models.BinaryField()), ('user', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='credentials', to=settings.AUTH_USER_MODEL)), diff --git a/hc/accounts/models.py b/hc/accounts/models.py index 4ae69de9..3ded1d75 100644 --- a/hc/accounts/models.py +++ b/hc/accounts/models.py @@ -395,7 +395,7 @@ class Member(models.Model): class Credential(models.Model): code = models.UUIDField(default=uuid.uuid4, unique=True) - name = models.CharField(max_length=200, blank=True) + name = models.CharField(max_length=100) user = models.ForeignKey(User, models.CASCADE, related_name="credentials") created = models.DateTimeField(auto_now_add=True) data = models.BinaryField() diff --git a/hc/accounts/urls.py b/hc/accounts/urls.py index b5e20be0..d139e2b2 100644 --- a/hc/accounts/urls.py +++ b/hc/accounts/urls.py @@ -24,4 +24,9 @@ urlpatterns = [ path("change_email/done/", views.change_email_done, name="hc-change-email-done"), path("change_email//", views.change_email, name="hc-change-email"), path("two_factor/add/", views.add_credential, name="hc-add-credential"), + path( + "two_factor//remove/", + views.remove_credential, + name="hc-remove-credential", + ), ] diff --git a/hc/accounts/views.py b/hc/accounts/views.py index ecb5a0aa..942636db 100644 --- a/hc/accounts/views.py +++ b/hc/accounts/views.py @@ -203,7 +203,21 @@ def check_token(request, username, token): def profile(request): profile = request.profile - ctx = {"page": "profile", "profile": profile, "my_projects_status": "default"} + ctx = { + "page": "profile", + "profile": profile, + "my_projects_status": "default", + "tfa_status": "default", + "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"), + } + + if ctx["added_credential_name"]: + ctx["tfa_status"] = "success" + + if ctx["removed_credential_name"]: + ctx["tfa_status"] = "info" if request.method == "POST": if "change_email" in request.POST: @@ -575,6 +589,7 @@ def add_credential(request): c.data = auth_data.credential_data c.save() + request.session["added_credential_name"] = c.name return redirect("hc-profile") credentials = [c.unpack() for c in request.user.credentials.all()] @@ -591,3 +606,20 @@ def add_credential(request): ctx = {"options": base64.b64encode(cbor.encode(options)).decode()} return render(request, "accounts/add_credential.html", ctx) + + +@login_required +@require_sudo_mode +def remove_credential(request, code): + try: + credential = Credential.objects.get(user=request.user, code=code) + except Credential.DoesNotExist: + return HttpResponseBadRequest() + + if request.method == "POST" and "remove_credential" in request.POST: + request.session["removed_credential_name"] = credential.name + credential.delete() + return redirect("hc-profile") + + ctx = {"credential": credential} + return render(request, "accounts/remove_credential.html", ctx) diff --git a/static/js/add_credential.js b/static/js/add_credential.js index c8b2e0e1..7b427df9 100644 --- a/static/js/add_credential.js +++ b/static/js/add_credential.js @@ -30,7 +30,14 @@ $(function() { }); } + $("#name").on('keypress',function(e) { + if (e.which == 13) { + e.preventDefault(); + requestCredentials(); + } + }); + $("#name-next").click(requestCredentials); $("#retry").click(requestCredentials); -}); \ No newline at end of file +}); diff --git a/templates/accounts/add_credential.html b/templates/accounts/add_credential.html index 534807bb..65003b21 100644 --- a/templates/accounts/add_credential.html +++ b/templates/accounts/add_credential.html @@ -19,7 +19,12 @@
- +
Give this credential a descriptive name. Example: "My primary Yubikey"
diff --git a/templates/accounts/profile.html b/templates/accounts/profile.html index 91e0b0c4..ae5336a6 100644 --- a/templates/accounts/profile.html +++ b/templates/accounts/profile.html @@ -59,30 +59,33 @@
-
+
{% csrf_token %}

Two-factor Authentication

- {% if profile.user.credentials.exists %} + {% if credentials.exists %} + - {% for credential in profile.user.credentials.all %} + {% for credential in credentials %} - + {% endfor %}
Security keys
{{ credential.name|default:"unnamed" }} – registered on {{ credential.created|date:"M j, Y" }} Remove + Remove +
{% else %}

- Your account has no registered security keys. + Your account has no registered security keys.
Two-factor authentication is disabled.

{% endif %} @@ -93,6 +96,18 @@
+ + {% if added_credential_name %} +
+ Added security key {{ added_credential_name }}. +
+ {% endif %} + + {% if removed_credential_name %} +
+ Removed security key {{ removed_credential_name }}. +
+ {% endif %}
diff --git a/templates/accounts/remove_credential.html b/templates/accounts/remove_credential.html new file mode 100644 index 00000000..0ede5b9e --- /dev/null +++ b/templates/accounts/remove_credential.html @@ -0,0 +1,42 @@ +{% extends "base.html" %} +{% load compress static hc_extras %} + +{% block content %} + +{{ registration_dict|json_script:"registration" }} +
+
+ {% csrf_token %} +
+
+

Remove Security Key?

+

+

You are about to remove + the security key {{ credential.name|default:'unnamed' }} + from your two-factor authentication methods. Are you sure? +

+
+ Cancel + +
+
+
+ + +
+
+{% endblock %} + +{% block scripts %} +{% compress js %} + + + + +{% endcompress %} +{% endblock %}