Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Two-Factor Authentication #2497

Merged
merged 23 commits into from
Jun 21, 2022
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions app/config/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -405,6 +405,8 @@
"django.middleware.common.CommonMiddleware",
"django.middleware.csrf.CsrfViewMiddleware",
"django.contrib.auth.middleware.AuthenticationMiddleware",
# Django-otp middleware must be after the AuthenticationMiddleware.
"django_otp.middleware.OTPMiddleware",
"django.contrib.messages.middleware.MessageMiddleware",
"django.contrib.sites.middleware.CurrentSiteMiddleware",
"django.middleware.clickjacking.XFrameOptionsMiddleware",
Expand All @@ -419,6 +421,13 @@
"django.contrib.flatpages.middleware.FlatpageFallbackMiddleware",
# Redirects last as they're a last resort
"django.contrib.redirects.middleware.RedirectFallbackMiddleware",
# 2FA middleware, needs to be after subdomina, flatpage and redirect middleware
# TwoFactorMiddleware resets the login flow if another page is loaded
# between login and successfully entering two-factor credentials. We're subsetting
# the original allauth_2fa middleware to pass the correct urlconf.
"grandchallenge.core.middleware.TwoFactorMiddleware",
# Force 2AF for staff users
"grandchallenge.core.middleware.RequireStaffAndSuperuser2FAMiddleware",
amickan marked this conversation as resolved.
Show resolved Hide resolved
)

# Python dotted path to the WSGI application used by Django's runserver.
Expand Down Expand Up @@ -484,6 +493,12 @@
# Overridden apps
"grandchallenge.forum_conversation",
"grandchallenge.forum_member",
# Configure the django-otp package
"django_otp",
"django_otp.plugins.otp_totp",
"django_otp.plugins.otp_static",
# Enable two-factor auth
"allauth_2fa",
]

LOCAL_APPS = [
Expand Down
1 change: 1 addition & 0 deletions app/config/urls/root.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ def handler500(request):
name="django.contrib.sitemaps.views.sitemap",
),
path(settings.ADMIN_URL, admin.site.urls),
path("accounts/", include("allauth_2fa.urls")),
path("accounts/", include("allauth.urls")),
path(
"stats/",
Expand Down
29 changes: 29 additions & 0 deletions app/grandchallenge/core/middleware.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
from allauth_2fa.middleware import BaseRequire2FAMiddleware
from django.urls import get_resolver
from django.utils.deprecation import MiddlewareMixin


class RequireStaffAndSuperuser2FAMiddleware(BaseRequire2FAMiddleware):
def require_2fa(self, request):
# Staff users and superusers are required to have 2FA.
return request.user.is_staff or request.user.is_superuser


class TwoFactorMiddleware(MiddlewareMixin):
"""
Reset the login flow if another page is loaded halfway through the login.
(I.e. if the user has logged in with a username/password, but not yet
entered their two-factor credentials.) This makes sure a user does not stay
half logged in by mistake.

"""

def process_request(self, request):
match = get_resolver(request.urlconf).resolve(request.path)
if not match.url_name or not match.url_name.startswith(
"two-factor-authenticate"
):
try:
del request.session["allauth_2fa_user_id"]
except KeyError:
pass
amickan marked this conversation as resolved.
Show resolved Hide resolved
28 changes: 26 additions & 2 deletions app/grandchallenge/profiles/adapters.py
Original file line number Diff line number Diff line change
@@ -1,14 +1,18 @@
from allauth.account.adapter import DefaultAccountAdapter
from allauth.account.utils import user_email, user_username
from allauth.exceptions import ImmediateHttpResponse
from allauth.socialaccount.adapter import DefaultSocialAccountAdapter
from allauth_2fa.adapter import OTPAdapter
from allauth_2fa.utils import user_has_valid_totp_device
from django import forms
from django.conf import settings
from django.http import HttpResponseRedirect
from django.utils.http import url_has_allowed_host_and_scheme

from grandchallenge.challenges.models import Challenge
from grandchallenge.subdomains.utils import reverse


class AccountAdapter(DefaultAccountAdapter):
class AccountAdapter(OTPAdapter):
def is_safe_url(self, url):
challenge_domains = {
f"{c.short_name.lower()}{settings.SESSION_COOKIE_DOMAIN}"
Expand Down Expand Up @@ -50,3 +54,23 @@ def populate_user(self, *args, **kwargs):
user_username(user, user_email(user).split("@")[0])

return user

def pre_social_login(self, request, sociallogin):
if user_has_valid_totp_device(sociallogin.user):
# Cast to string for the case when this is not a JSON serializable
# object, e.g. a UUID.
request.session["allauth_2fa_user_id"] = str(sociallogin.user.id)
redirect_url = reverse("two-factor-authenticate")
redirect_url += "?next=" + request.get_full_path()
raise ImmediateHttpResponse(
response=HttpResponseRedirect(redirect_url)
)
elif sociallogin.user.is_staff and not user_has_valid_totp_device(
sociallogin.user
):
redirect_url = reverse("two-factor-setup")
redirect_url += "?next=" + request.get_full_path()
raise ImmediateHttpResponse(
response=HttpResponseRedirect(redirect_url)
)
return super().pre_social_login(request, sociallogin)
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
{% extends "account/base.html" %}
{% load i18n %}

{% block content %}
<h1>
{% trans "Two-Factor Authentication" %}
</h1>
<p>{% trans "Enter the token from your authenticator app below." %}</p>
<form method="post" class="mt-3">
{% csrf_token %}
{{ form.non_field_errors }}
{{ form.otp_token.label }}:
{{ form.otp_token }}
<br>
<button class="btn btn-primary mt-3" type="submit">
{% trans 'Authenticate' %}
</button>
</form>
{% endblock %}
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
{% extends "base.html" %}
{% load i18n %}

{% block title %}Two-Factor Authentication{% endblock %}

{% block breadcrumbs %}
<ol class="breadcrumb">
<li class="breadcrumb-item text-light">Users</li>
<li class="breadcrumb-item"><a
href="{% url 'profile-detail' username=request.user.username %}">{{ request.user.username }}</a></li>
<li class="breadcrumb-item active" aria-current="page">Two Factor Authentication Settings</li>
</ol>
{% endblock %}

{% block content %}
<h1 class="mb-3">
{% trans "Two-Factor Authentication" %}
</h1>
<p><i class="fas fa-check-circle text-success mr-1"></i>{% trans 'Two-Factor Authentication is enabled for your account.' %}</p>

<h3 class="mt-4">
{% trans "Back-Up Tokens" %}
</h3>
<p>{% trans "If you don't have your authentication device with you, you can access your account using backup tokens." %}</p>

{% if backup_tokens %}
{% if reveal_tokens %}
<ul>
{% for token in backup_tokens %}
<li>{{ token.token }}</li>
{% endfor %}
</ul>
{% else %}
{% trans 'Backup tokens have been generated, but are not revealed here for security reasons. Press the button below to generate new ones.' %}
{% endif %}
{% else %}
{% trans 'No tokens. Press the button below to generate some.' %}
{% endif %}

<form method="post">
{% csrf_token %}
<button class="btn btn-primary" type="submit">
{% trans 'Generate backup tokens' %}
</button>
</form>

<h3 class="mt-4">
{% trans "Disable Two-Factor Authentication" %}
</h3>
<p>{% trans 'You can disable two-factor authentication for your account at any time.' %}</p>
<a class="btn btn-primary" href="{% url 'two-factor-remove' %}">Disable Two Factor Authentication</a>

{% endblock %}
28 changes: 28 additions & 0 deletions app/grandchallenge/profiles/templates/allauth_2fa/remove.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
{% extends "base.html" %}
{% load i18n %}

{% block title %}Disable Two-Factor Authentication{% endblock %}

{% block breadcrumbs %}
<ol class="breadcrumb">
<li class="breadcrumb-item text-light">Users</li>
<li class="breadcrumb-item"><a
href="{% url 'profile-detail' username=request.user.username %}">{{ request.user.username }}</a></li>
<li class="breadcrumb-item active" aria-current="page">Disable Two Factor Authentication</li>
</ol>
{% endblock %}

{% block content %}
<h1>
{% trans "Disable Two-Factor Authentication" %}
</h1>

<p>{% trans "Are you sure you want to disable Two-Factor Authentication from your account?" %}</p>

<form method="post">
{% csrf_token %}
<button class="btn btn-primary" type="submit">
{% trans 'Yes, disable 2FA' %}
</button>
</form>
{% endblock %}
47 changes: 47 additions & 0 deletions app/grandchallenge/profiles/templates/allauth_2fa/setup.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
{% extends "base.html" %}
{% load i18n %}

{% block title %}Enable Two-Factor Authentication{% endblock %}

{% block breadcrumbs %}
<ol class="breadcrumb">
<li class="breadcrumb-item text-light">Users</li>
<li class="breadcrumb-item"><a
href="{% url 'profile-detail' username=request.user.username %}">{{ request.user.username }}</a></li>
<li class="breadcrumb-item active" aria-current="page">Set-up Two Factor Authentication</li>
</ol>
{% endblock %}

{% block content %}
<h1>
{% trans "Setup Two-Factor Authentication" %}
</h1>

<h4>
{% trans 'Step 1' %}:
</h4>

<p>
{% trans 'Scan the QR code below with a token generator of your choice (e.g., Google Authenticator, Microsoft Authenticator).' %}
</p>

<img src="{{ qr_code_url }}" />
amickan marked this conversation as resolved.
Show resolved Hide resolved

<h4>
{% trans 'Step 2' %}:
</h4>

<p>
{% trans 'Input the token generated by the app:' %}
</p>

<form method="post">
{% csrf_token %}
{{ form.non_field_errors }}
{{ form.token.label }}: {{ form.token }}

<button class="btn btn-primary btn-sm" type="submit">
{% trans 'Verify' %}
</button>
</form>
{% endblock %}
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
{% load meta_attr %}
{% load evaluation_extras %}
{% load humanize %}
{% load profiles %}

{% block title %}{% blocktrans with profile.user.username as username %}
{{ username }}'s profile
Expand Down Expand Up @@ -77,6 +78,13 @@ <h2 class="mb-3">{{ profile.user.username }}</h2>
<a class="list-group-item list-group-item-action"
href='{% url 'api-tokens:list' %}'><i
class="fas fa-unlock fa-fw"></i>&nbsp;{% trans "Manage API Tokens" %}</a>
{% if request.user|has_2fa_enabled %}
<a class="list-group-item list-group-item-action" href="{% url 'two-factor-backup-tokens' %}"><i
class="fas fa-qrcode fa-fw"></i>{% trans "Two-Factor Authentication Settings" %}</a>
{% else %}
<a class="list-group-item list-group-item-action" href="{% url 'two-factor-setup' %}"><i
class="fas fa-qrcode fa-fw"></i>{% trans "Enable Two-Factor Authentication" %}</a>
{% endif %}
</ul>
{% endif %}

Expand Down
6 changes: 6 additions & 0 deletions app/grandchallenge/profiles/templatetags/profiles.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
from typing import Union

from allauth_2fa.utils import user_has_valid_totp_device
from django import template
from django.contrib.auth import get_user_model
from django.contrib.auth.models import AbstractUser
Expand Down Expand Up @@ -64,3 +65,8 @@ def user_profile_link(user: Union[AbstractUser, None]) -> str:
def user_profile_link_username(username: str) -> str:
User = get_user_model() # noqa: N806
return user_profile_link(User.objects.get(username=username))


@register.filter
def has_2fa_enabled(user):
return user_has_valid_totp_device(user)
6 changes: 4 additions & 2 deletions app/tests/cases_tests/test_forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,9 @@


@pytest.mark.django_db
def test_upload_some_images(client: Client, challenge_set, settings):
def test_upload_some_images(
client: Client, challenge_set, settings, authenticated_staff_user
):
# Override the celery settings
settings.task_eager_propagates = (True,)
settings.task_always_eager = (True,)
Expand Down Expand Up @@ -64,6 +66,6 @@ def test_upload_some_images(client: Client, challenge_set, settings):
response = get_view_for_user(
url=sessions[0].get_absolute_url(),
client=client,
user=UserFactory(is_staff=True),
user=authenticated_staff_user,
)
assert response.status_code == 403
41 changes: 41 additions & 0 deletions app/tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +9,20 @@
import docker
import pytest
from django.conf import settings
from django.contrib.auth import authenticate
from django.contrib.auth.models import Group
from django.contrib.sites.models import Site
from django.utils.timezone import now
from django_otp.oath import TOTP
from django_otp.plugins.otp_totp.models import TOTPDevice
from guardian.shortcuts import assign_perm

from grandchallenge.cases.models import Image
from grandchallenge.challenges.utils import ChallengeTypeChoices
from grandchallenge.components.models import ComponentInterface
from grandchallenge.core.fixtures import create_uploaded_image
from grandchallenge.reader_studies.models import Question
from grandchallenge.subdomains.utils import reverse_lazy
from tests.annotations_tests.factories import (
ImagePathologyAnnotationFactory,
ImageQualityAnnotationFactory,
Expand All @@ -36,6 +40,7 @@
)
from tests.evaluation_tests.factories import MethodFactory
from tests.factories import (
SUPER_SECURE_TEST_PASSWORD,
ChallengeFactory,
ChallengeRequestFactory,
ImageFactory,
Expand Down Expand Up @@ -676,3 +681,39 @@ def challenge_reviewer():
assign_perm("challenges.change_challengerequest", user)
assign_perm("challenges.view_challengerequest", user)
return user


AUTH_URL = reverse_lazy("two-factor-authenticate")


def get_token_from_totp_device(totp_model) -> str:
return TOTP(
key=totp_model.bin_key,
step=totp_model.step,
t0=totp_model.t0,
digits=totp_model.digits,
).token()


def do_totp_authentication(
client,
totp_device: TOTPDevice,
*,
auth_url: str = AUTH_URL,
):
token = get_token_from_totp_device(totp_device)
client.post(auth_url, {"otp_token": token})


@pytest.fixture
def authenticated_staff_user(client):
user = UserFactory(username="john", is_staff=True)
totp_device = user.totpdevice_set.create()
user = authenticate(
username=user.username, password=SUPER_SECURE_TEST_PASSWORD
)
do_totp_authentication(
client=client,
totp_device=totp_device,
)
return user
Loading