Merge pull request #3016 from modoboa/tfa-setup-improvement

Tfa setup improvement
This commit is contained in:
Antoine Nguyen
2023-07-07 11:32:45 +02:00
committed by GitHub
14 changed files with 325 additions and 83 deletions
+1 -1
View File
@@ -7,7 +7,7 @@ ENV PATH="$VIRTUAL_ENV/bin:$PATH"
FROM base as build
RUN apk add --update openssl librrd python3-dev libffi-dev gcc g++ musl-dev libxml2-dev libxslt-dev \
libressl-dev jpeg-dev rrdtool-dev file make gettext freetype-dev cairo-dev \
libressl-dev jpeg-dev rrdtool-dev file make gettext freetype-dev cairo-dev cargo \
&& rm -rf /var/cache/apk/*
RUN python3 -m venv $VIRTUAL_ENV
WORKDIR /tmp
+2 -1
View File
@@ -23,7 +23,8 @@
"vue-gettext": "^2.1.12",
"vue-router": "^3.2.0",
"vuetify": "^2.6.10",
"vuex": "^3.4.0"
"vuex": "^3.4.0",
"qrcode.vue": "~1.7.0"
},
"devDependencies": {
"@babel/core": "^7.12.16",
+6 -6
View File
@@ -16,18 +16,18 @@ export default {
startTFASetup () {
return repository.post(`${resource}/tfa/setup/`)
},
getQRCodeForTFASetup () {
return repository.get(`${resource}/tfa/setup/qr_code/`)
getKeyForTFASetup () {
return repository.get(`${resource}/tfa/setup/key/`)
},
finalizeTFASetup (pinCode) {
const payload = { pin_code: pinCode }
return repository.post(`${resource}/tfa/setup/check/`, payload)
},
disableTFA () {
return repository.post(`${resource}/tfa/disable/`)
disableTFA (data) {
return repository.post(`${resource}/tfa/disable/`, data)
},
resetRecoveryCodes () {
return repository.post(`${resource}/tfa/reset_codes/`)
resetRecoveryCodes (data) {
return repository.post(`${resource}/tfa/reset_codes/`, data)
},
getForward () {
return repository.get(`${resource}/forward/`)
@@ -5,10 +5,31 @@
<span class="text-subtitle-1"><translate>Two factor authentication</translate></span>
</v-card-title>
<v-card-text>
<template v-if="qrCode">
<template v-if="qrURL">
<v-row>
<v-col cols="4">
<div v-html="qrCode"></div>
<v-row rows="2">
<qrcode-vue
:value="qrURL"
:size="250"
render-as="svg"
level="H"
class= "qrcode"
/>
</v-row>
<v-row>
<v-btn color="primary" @click="copyKey" class="key">
<translate>
Click here to copy the key
</translate>
<v-icon
v-if="clicked"
color="success"
>
mdi-check-all
</v-icon>
</v-btn>
</v-row>
</v-col>
<v-col cols="6">
<v-alert type="info">
@@ -22,7 +43,8 @@
<validation-observer ref="observer">
<validation-provider
v-slot="{ errors }"
rules="required"
name="pin_code"
rules="required|numeric|minmax:6"
>
<v-text-field
v-model="pinCode"
@@ -59,12 +81,29 @@
</template>
<template v-else-if="account.tfa_enabled">
<v-alert type="info">
Two-Factor Authentication is enabled for your account.
<translate> Two-Factor Authentication is enabled for your account.</translate>
</v-alert>
<v-btn color="error" @click="disableTFA" class="mr-2">
<validation-observer ref="observer">
<validation-provider
vid="password"
v-slot="{ errors }"
rules="required"
>
<label class="m-label"><translate>Password</translate></label>
<v-text-field
ref="password"
type="password"
v-model="password"
:error-messages="errors"
outlined
autocomplete="new-password"
/>
</validation-provider>
</validation-observer>
<v-btn color="error" @click="disableTFA" class="mr-2" :loading="loadingDisable">
<translate>Disable 2FA</translate>
</v-btn>
<v-btn @click="resetRecoveryCodes">
<v-btn @click="resetRecoveryCodes" :loading="loadingReset">
<translate>Reset recovery codes</translate>
</v-btn>
</template>
@@ -95,10 +134,13 @@
import account from '@/api/account'
import Cookies from 'js-cookie'
import RecoveryCodesResetDialog from './RecoveryCodesResetDialog'
import QrcodeVue from 'qrcode.vue'
import { bus } from '@/main'
export default {
components: {
RecoveryCodesResetDialog
RecoveryCodesResetDialog,
QrcodeVue
},
props: {
account: Object
@@ -107,20 +149,31 @@ export default {
return {
newTokens: [],
pinCode: null,
qrCode: null,
key: null,
qrURL: null,
clicked: false,
showCodesResetDialog: false,
tokens: []
loadingDisable: false,
loadingReset: false,
password: '',
tokens: [],
errors: {}
}
},
methods: {
getQRCode () {
account.getQRCodeForTFASetup().then(resp => {
this.qrCode = resp.data
getKey () {
account.getKeyForTFASetup().then(resp => {
this.key = resp.data.key
this.qrURL = resp.data.url
})
},
copyKey () {
navigator.clipboard.writeText(this.key)
this.clicked = true
},
startTFASetup () {
account.startTFASetup().then(resp => {
this.getQRCode()
this.getKey()
})
},
async finalizeTFASetup () {
@@ -128,22 +181,66 @@ export default {
if (!valid) {
return
}
const resp = await account.finalizeTFASetup(this.pinCode)
this.qrCode = null
this.tokens = resp.data.tokens
Cookies.set('token', resp.data.access, { sameSite: 'strict' })
Cookies.set('refreshToken', resp.data.refresh, { sameSite: 'strict' })
this.$store.dispatch('auth/initialize')
try {
const resp = await account.finalizeTFASetup(this.pinCode)
this.key = null
this.qrURL = null
this.tokens = resp.data.tokens
Cookies.set('token', resp.data.access, { sameSite: 'strict' })
Cookies.set('refreshToken', resp.data.refresh, { sameSite: 'strict' })
this.$store.dispatch('auth/initialize')
} catch (err) {
if (err.response.status === 400) {
this.$refs.observer.setErrors(err.response.data)
}
}
},
disableTFA () {
account.disableTFA().then(resp => {
async disableTFA () {
const isValid = await this.$refs.observer.validate()
if (!isValid) {
return
}
const payload = {
password: this.password
}
this.loadingDisable = true
account.disableTFA(payload).then(resp => {
this.loadingDisable = false
this.$store.dispatch('auth/fetchUser')
this.$refs.password.reset()
this.$refs.observer.reset()
}).catch(err => {
this.loadingDisable = false
if (err.response.status === 400) {
this.$refs.observer.setErrors(err.response.data)
} else {
bus.$emit('notification', { msg: this.$gettext(err.response.data), type: 'error' })
}
})
},
resetRecoveryCodes () {
account.resetRecoveryCodes().then(resp => {
async resetRecoveryCodes () {
const isValid = await this.$refs.observer.validate()
if (!isValid) {
return
}
const payload = {
password: this.password
}
this.loadingReset = true
account.resetRecoveryCodes(payload).then(resp => {
this.loadingReset = false
this.newTokens = resp.data.tokens
this.showCodesResetDialog = true
this.$refs.password.reset()
this.$refs.observer.reset()
}).catch(err => {
this.loadingReset = false
if (err.response.status === 400) {
this.$refs.observer.setErrors(err.response.data)
} else {
bus.$emit('notification', { msg: this.$gettext(err.response.data), type: 'error' })
}
})
},
closeRecoveryCodesResetDialog () {
@@ -153,8 +250,19 @@ export default {
},
created () {
if (!this.account.tfa_enabled) {
this.getQRCode()
this.getKey()
}
}
}
</script>
<style>
.qrcode {
margin-left:20px;
margin-top: 2.0em;
}
.key {
margin-left:10px;
margin-top:1.75em;
}
</style>
+7
View File
@@ -15,6 +15,13 @@ extend('numeric', {
...numeric,
message: () => $gettext('This field must be a valid number')
})
extend('minmax', {
validate: async (value, args) => {
return value.length >= args.length && value.length <= args.length
},
params: ['length'],
message: $gettext('This field must have a length of {length} characters')
})
extend('required', {
...required,
message: () => $gettext('This field is required')
+4
View File
@@ -20,6 +20,7 @@
<v-col cols="6" offset="3">
<label class="m-label"><translate>Two-factor authentication code</translate></label>
<v-otp-input
ref="otpInput"
v-model="code"
length="6"
:error-messages="errors"
@@ -52,6 +53,9 @@ export default {
loading: false
}
},
mounted: function () {
this.$refs.otpInput.focus()
},
methods: {
async verifyCode () {
const isValid = await this.$refs.observer.validate()
+5
View File
@@ -6350,6 +6350,11 @@ punycode@^2.1.0, punycode@^2.1.1:
resolved "https://registry.yarnpkg.com/punycode/-/punycode-2.1.1.tgz#b58b010ac40c22c5657616c8d2c2c02c7bf479ec"
integrity sha512-XRsRjdf+j5ml+y/6GKHPZbrF/8p2Yga0JPtdqTIY2Xe5ohJPD9saDJJLPvp9+NSBprVvevdXZybnj2cv8OEd0A==
qrcode.vue@~1.7.0:
version "1.7.0"
resolved "https://registry.yarnpkg.com/qrcode.vue/-/qrcode.vue-1.7.0.tgz#c54b2934f0650c10d92785d08aaad36c55e0fc56"
integrity sha512-R7t6Y3fDDtcU7L4rtqwGUDP9xD64gJhIwpfjhRCTKmBoYF6SS49PIJHRJ048cse6OI7iwTwgyy2C46N9Ygoc6g==
qs@6.10.3:
version "6.10.3"
resolved "https://registry.yarnpkg.com/qs/-/qs-6.10.3.tgz#d6cde1b2ffca87b5aa57889816c5f81535e22e8e"
+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,9 +57,18 @@ 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:
# We include it as "password" to display the error
return response.Response({"error": _("2FA is not enabled")},
status=400)
status=403)
request.user.totpdevice_set.all().delete()
request.user.staticdevice_set.all().delete()
request.user.tfa_enabled = False
@@ -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({"error": _("2FA is not enabled")},
status=403)
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")
+5 -13
View File
@@ -1,10 +1,5 @@
"""Core API v2 viewsets."""
import io
import qrcode
import qrcode.image.svg
from django_otp.plugins.otp_static.models import StaticDevice, StaticToken
from drf_spectacular.utils import extend_schema
from rest_framework import filters, permissions, response, viewsets
@@ -102,19 +97,16 @@ class AccountViewSet(core_v1_viewsets.AccountViewSet):
"access": str(refresh.access_token)
})
@action(methods=["get"], detail=False, url_path="tfa/setup/qr_code")
def tfa_setup_get_qr_code(self, request):
"""Get a QR code to finalize the setup process."""
@action(methods=["get"], detail=False, url_path="tfa/setup/key")
def tfa_setup_get_key(self, request):
"""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()
if not device:
return response.Response(status=404)
factory = qrcode.image.svg.SvgPathImage
img = qrcode.make(device.config_url, image_factory=factory)
buf = io.BytesIO()
img.save(buf)
return response.Response(buf.getvalue(), content_type="application/xml")
return response.Response({"key": device.key, "url":device.config_url},
content_type="application/json")
@extend_schema(
request=core_v1_serializers.CheckTFASetupSerializer
+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>
+7 -5
View File
@@ -19,7 +19,7 @@ class AccountViewSetTestCase(ModoAPITestCase):
admin_factories.populate_database()
@mock.patch("django_otp.plugins.otp_totp.models.TOTPDevice.verify_token")
def test_tfa_setup_process(self, verify_mock):
def test_tfa_setup_process(self, verify_mock, password_ok="password"):
admin = models.User.objects.get(username="admin")
self.client.force_login(admin)
@@ -58,18 +58,20 @@ class AccountViewSetTestCase(ModoAPITestCase):
# Try to disable TFA auth
url = reverse("api:account-tfa-disable")
response = self.client.post(url)
data = {"password": password_ok}
response = self.client.post(url, data, format="json")
self.assertEqual(response.status_code, 200)
self.assertFalse(admin.totpdevice_set.exists())
def test_tfa_reset_codes(self):
def test_tfa_reset_codes(self, password_ok="password"):
admin = models.User.objects.get(username="admin")
self.client.force_login(admin)
url = reverse("api:account-tfa-reset-codes")
response = self.client.post(url)
data = {"password": password_ok}
response = self.client.post(url, data, format="json")
self.assertEqual(response.status_code, 403)
device = admin.staticdevice_set.create(name="Device")
response = self.client.post(url)
response = self.client.post(url, data, format="json")
self.assertEqual(response.status_code, 200)
self.assertEqual(device.token_set.count(), 10)
-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();
});
}
};