Added password check to modify TFA setup

This commit is contained in:
Spitap
2023-06-30 16:28:10 +02:00
parent 399eaf5709
commit b03af24338
9 changed files with 243 additions and 52 deletions
+22
View File
@@ -1,9 +1,14 @@
"""Core API serializers."""
import logging
from django.utils.html import escape
from django.utils.translation import gettext as _
from rest_framework import serializers
logger = logging.getLogger("modoboa.auth")
class CheckTFASetupSerializer(serializers.Serializer):
"""Serializer used to finalize 2FA setup."""
@@ -21,3 +26,20 @@ class CheckTFASetupSerializer(serializers.Serializer):
if user.tfa_enabled or user.staticdevice_set.exists():
raise serializers.ValidationError(_("2FA is already enabled"))
return data
class CheckPasswordTFASerializer(serializers.Serializer):
"""Serializer used to check user password when modifying TFA."""
password = serializers.CharField()
def validate_password(self, value):
user = self.context["user"]
if not user.check_password(value):
logger.warning(
_("Failed TFA settings editing attempt from '%s' as user '%s'"),
self.context["remote_addr"],
escape(user.username)
)
raise serializers.ValidationError(_("Invalid password"))
return value
+20 -2
View File
@@ -57,8 +57,17 @@ class AccountViewSet(GetThrottleViewsetMixin, viewsets.ViewSet):
@action(methods=["post"], detail=False, url_path="tfa/disable")
def tfa_disable(self, request):
"""Disable TFA."""
serializer = serializers.CheckPasswordTFASerializer(
data=request.data,
context={
"user": request.user,
"remote_addr": request.META["REMOTE_ADDR"]
}
)
serializer.is_valid(raise_exception=True)
if not request.user.tfa_enabled:
return response.Response({"error": _("2FA is not enabled")},
# We include it as "password" to display the error
return response.Response({"password": _("2FA is not enabled")},
status=400)
request.user.totpdevice_set.all().delete()
request.user.staticdevice_set.all().delete()
@@ -70,9 +79,18 @@ class AccountViewSet(GetThrottleViewsetMixin, viewsets.ViewSet):
@action(methods=["post"], detail=False, url_path="tfa/reset_codes")
def tfa_reset_codes(self, request, *args, **kwargs):
"""Reset recovery codes."""
serializer = serializers.CheckPasswordTFASerializer(
data=request.data,
context={
"user": request.user,
"remote_addr": request.META["REMOTE_ADDR"]
}
)
serializer.is_valid(raise_exception=True)
device = request.user.staticdevice_set.first()
if device is None:
return response.Response(status=403)
return response.Response({"password": _("2FA is not enabled")},
status=400)
device.token_set.all().delete()
for cpt in range(10):
token = StaticToken.random_token()
+33 -5
View File
@@ -146,8 +146,9 @@ class AccountViewSetTestCase(ModoAPITestCase):
self.assertEqual(resp.status_code, 200)
self.assertIn("access", resp.json())
def test_tfa_setup_get_qr_code(self):
url = reverse("v2:account-tfa-setup-get-qr-code")
def test_tfa_setup(self):
# Setup TFA
url = reverse("v2:account-tfa-setup-get-key")
resp = self.client.get(url)
self.assertEqual(resp.status_code, 404)
@@ -156,7 +157,7 @@ class AccountViewSetTestCase(ModoAPITestCase):
user.totpdevice_set.create(name="Device")
resp = self.client.get(url)
self.assertEqual(resp.status_code, 200)
self.assertEqual(resp.content_type, "application/xml")
self.assertEqual(resp.content_type, "application/json")
user.tfa_enabled = True
user.save()
@@ -164,7 +165,11 @@ class AccountViewSetTestCase(ModoAPITestCase):
self.assertEqual(resp.status_code, 404)
@mock.patch("django_otp.plugins.otp_totp.models.TOTPDevice.verify_token")
def test_tfa_setup_check(self, verify_mock):
def test_tfa_setup_modify(self,
verify_mock,
password_ko="Toto1234",
password_ok="password"
):
user = models.User.objects.get(username="admin")
user.totpdevice_set.create(name="Device")
@@ -183,6 +188,30 @@ class AccountViewSetTestCase(ModoAPITestCase):
resp = self.client.post(url, data, format="json")
self.assertEqual(resp.status_code, 400)
url_reset = reverse("v2:account-tfa-reset-codes")
url_disable = reverse("v2:account-tfa-disable")
data = {"password": password_ko}
# Try regenerate TFA backup code with wrong password
resp = self.client.post(url_reset, data, format="json")
self.assertEqual(resp.status_code, 400)
# Try disable with wrong password
resp = self.client.post(url_disable, data, format="json")
self.assertEqual(resp.status_code, 400)
# Test with password ok
data = {"password": password_ok}
# Try regenerate TFA backup codes with good password
resp = self.client.post(url_reset, data, format="json")
self.assertEqual(resp.status_code, 200)
self.assertIn("tokens", resp.json())
# Try disable with good password
resp = self.client.post(url_disable, data, format="json")
self.assertEqual(resp.status_code, 200)
user.refresh_from_db()
self.assertEqual(user.tfa_enabled, False)
def test_api_token(self):
# 1. Obtain a JWT token so we can safely play with basic token
url = reverse("v2:token_obtain_pair")
@@ -415,7 +444,6 @@ class PasswordResetTestCase(AccountViewSetTestCase):
# TODO: See why user doesn't update it's password --> self.test_me_password(password_ok="MyHardenedPass1!")
class AuthenticationTestCase(ModoAPITestCase):
@mock.patch("django_otp.match_token")
+1 -1
View File
@@ -99,7 +99,7 @@ class AccountViewSet(core_v1_viewsets.AccountViewSet):
@action(methods=["get"], detail=False, url_path="tfa/setup/key")
def tfa_setup_get_key(self, request):
"""Get a QR code to finalize the setup process."""
"""Get a key and url to finalize the setup process."""
if request.user.tfa_enabled:
return response.Response(status=404)
device = request.user.totpdevice_set.first()
+81 -2
View File
@@ -54,10 +54,10 @@
Two-Factor Authentication is enabled for your account.
{% endblocktrans %}
</div>
<a id="deactivate-tfa" class="btn btn-danger" href="{% url 'api:account-tfa-disable' %}">
<a id="deactivate-tfa" class="btn btn-danger" data-href="{% url 'api:account-tfa-disable' %}" data-toggle="modal" data-target="#disable-modal" data-content="{% trans 'This will invalidate your registered application.' %}">
{% trans "Disable 2FA" %}
</a>
<a id="tfa-refresh-codes" class="btn btn-default" href="{% url 'api:account-tfa-reset-codes' %}">
<a id="tfa-refresh-codes" class="btn btn-default" data-href="{% url 'api:account-tfa-reset-codes' %}" data-toggle="modal" data-target="#disable-modal" data-content="{% trans 'This will regenerate your tokens.' %}">
{% trans "Reset recovery codes" %}
</a>
{% else %}
@@ -101,3 +101,82 @@
</div>
</div>
</div>
<div id="disable-modal" class="modal fade" tabindex="-1" role="dialog" aria-labelledby="passwordConfirmModal" aria-hidden="true">
<div class="modal-dialog" role="document">
<div class="modal-content">
<form id="password-confirm">
<div class="modal-header">
<h3 class="modal-title">Are you sure?</h3>
</div>
<div class="modal-body">
<div class="alert alert-danger">
<h4>Warning</h4>
<div id="password-confirm-content">
</div>
</div>
<div class="form-group">
<label for="password">{% trans "Confirm your password" %}</label>
<input name="password" id="password" class="form-control" type="password" placeholder="{% trans 'password' %}" required>
<div class="col-sm-3">
<small id="password-error" class="text-danger"></small>
</div>
</div>
</div>
<div class="modal-footer">
<button type="submit" class="btn btn-primary">Ok</button>
<a data-dismiss="modal" class="btn btn-default">Cancel</a>
</div>
</form>
</div>
</div>
</div>
<script type="text/javascript">
const passwordError = document.getElementById("password-error")
var buttonClicked;
$('#disable-modal').on('shown.bs.modal', function (event) {
buttonClicked = $(event.relatedTarget); // Button that triggered the modal
document.getElementById("password-confirm-content").innerHTML = buttonClicked.attr("data-content")
});
const form = document.getElementById('password-confirm')
form.addEventListener("submit", async function(event) {
event.preventDefault()
passwordError.innerHTML = ""
const url = buttonClicked.attr("data-href")
const formData = new FormData(event.target);
const plainFormData = Object.fromEntries(formData.entries());
const formDataJsonString = JSON.stringify(plainFormData);
const fetchOptions = {
method: "POST",
credentials: 'same-origin',
headers: {
"Content-Type": "application/json",
"Accept": "application/json",
"X-CSRFToken": getCookie("csrftoken")
},
body: formDataJsonString,
};
const response = await fetch(url, fetchOptions);
if (!response.ok && response.status === 400) {
const text = await response.json();
passwordError.innerHTML = text.password[0];
return;
} else {
if (buttonClicked.attr("id") === "tfa-refresh-codes") {
var $container = $('#tfa-codes-area');
var $ul = $('<ul>');
const resp = await response.json()
$.each(resp.tokens, function (index, value) {
var $li = $('<li />', { html: value });
$ul.append($li);
});
$container.append($ul);
$('#codes-modal').modal();
$('#disable-modal').modal('hide')
return;
}
}
window.location.reload();
})
</script>
-24
View File
@@ -166,12 +166,6 @@ TwocolsNav.prototype = {
this.update_content(data);
$('#start-tfa-setup').click(this.startTFASetup);
$('#finalize-tfa-setup').click(this.finalizeTFASetup);
$('#deactivate-tfa').confirm({
question: gettext('Are you sure?'),
warning: gettext('This will invalidate your registered application.'),
method: 'POST'
});
$('#tfa-refresh-codes').click(this.resetTFARecoveryCodes);
},
startTFASetup: function (evt) {
@@ -202,24 +196,6 @@ TwocolsNav.prototype = {
var errors = JSON.parse(jqxhr.responseText);
display_field_error($('#pin-code'), errors.pin_code.join(' '));
});
},
resetTFARecoveryCodes: function (evt) {
evt.preventDefault();
$.ajax({
url: $(this).attr('href'),
type: 'post',
contentType: 'application/json'
}).done(function (resp) {
var $container = $('#tfa-codes-area');
var $ul = $('<ul>');
$.each(resp.tokens, function (index, value) {
var $li = $('<li />', { html: value });
$ul.append($li);
});
$container.append($ul);
$('#codes-modal').modal();
});
}
};