From 0f8981746353723f7a9bf055876abd953d6ee495 Mon Sep 17 00:00:00 2001
From: Julian Rother <julianr@fsmpi.rwth-aachen.de>
Date: Sun, 4 Oct 2020 01:50:46 +0200
Subject: [PATCH] mfa integration into selfservice, admin pages and login
process
---
uffd/mfa/templates/auth.html | 2 +-
uffd/mfa/templates/setup.html | 2 +-
uffd/mfa/templates/setup_totp.html | 2 +-
uffd/mfa/views.py | 48 +++++++++++++++++++++-------
uffd/selfservice/templates/self.html | 5 +++
uffd/session/views.py | 26 +++++++++------
uffd/user/templates/user.html | 1 +
7 files changed, 63 insertions(+), 23 deletions(-)
diff --git a/uffd/mfa/templates/auth.html b/uffd/mfa/templates/auth.html
index 8e359995..57f5d588 100644
--- a/uffd/mfa/templates/auth.html
+++ b/uffd/mfa/templates/auth.html
@@ -57,7 +57,7 @@ function begin_webauthn() {
return navigator.credentials.get(options);
}).then(function(assertion) {
$('#webauthn-btn-text').text('Verifing response');
- return fetch({{ url_for('mfa.auth_webauthn_begin')|tojson }}, {
+ return fetch({{ url_for('mfa.auth_webauthn_complete')|tojson }}, {
method: 'POST',
headers: {'Content-Type': 'application/cbor'},
body: CBOR.encode({
diff --git a/uffd/mfa/templates/setup.html b/uffd/mfa/templates/setup.html
index 80cb5d58..48388149 100644
--- a/uffd/mfa/templates/setup.html
+++ b/uffd/mfa/templates/setup.html
@@ -27,7 +27,7 @@ You need to setup at least one authentication method to enable two-factor authen
<button type="submit" class="btn btn-danger mb-2">Disable two-factor authentication</button>
</form>
{% else %}
- <form class="form float-right" action="{{ url_for('mfa.disable') }}" method="POST">
+ <form class="form float-right" action="{{ url_for('mfa.disable_confirm') }}" method="POST">
<button type="submit" class="btn btn-light mb-2">Reset two-factor configuration</button>
</form>
{% endif %}
diff --git a/uffd/mfa/templates/setup_totp.html b/uffd/mfa/templates/setup_totp.html
index fef0c439..f0850914 100644
--- a/uffd/mfa/templates/setup_totp.html
+++ b/uffd/mfa/templates/setup_totp.html
@@ -25,7 +25,7 @@
</div>
</div>
-<form action="{{ url_for('mfa.setup_totp', name=name) }}" method="POST" class="form">
+<form action="{{ url_for('mfa.setup_totp_finish', name=name) }}" method="POST" class="form">
<div class="row m-0">
<input type="text" name="code" class="form-control mb-2 mr-2 col-auto col-md" id="code" placeholder="Code" required autofocus>
<button type="submit" class="btn btn-primary mb-2 col col-md-auto">Verify and complete setup</button>
diff --git a/uffd/mfa/views.py b/uffd/mfa/views.py
index 170653bb..2237db2d 100644
--- a/uffd/mfa/views.py
+++ b/uffd/mfa/views.py
@@ -1,4 +1,4 @@
-from flask import Blueprint, render_template, session, request, redirect, url_for, flash
+from flask import Blueprint, render_template, session, request, redirect, url_for, flash, current_app
import urllib.parse
from fido2.webauthn import PublicKeyCredentialRpEntity, UserVerificationRequirement
@@ -9,7 +9,10 @@ from fido2 import cbor
from uffd.database import db
from uffd.mfa.models import MFAMethod, TOTPMethod, WebauthnMethod, RecoveryCodeMethod
-from uffd.session.views import get_current_user, login_required
+from uffd.session.views import get_current_user, login_required, is_valid_session
+from uffd.ldap import uid_to_dn
+from uffd.user.models import User
+from uffd.csrf import csrf_protect
bp = Blueprint('mfa', __name__, template_folder='templates', url_prefix='/mfa/')
@@ -29,14 +32,31 @@ def disable():
@bp.route('/setup/disable', methods=['POST'])
@login_required()
+@csrf_protect(blueprint=bp)
def disable_confirm():
user = get_current_user()
MFAMethod.query.filter_by(dn=user.dn).delete()
db.session.commit()
return redirect(url_for('mfa.setup'))
+@bp.route('/admin/<int:uid>/disable')
+@login_required()
+@csrf_protect(blueprint=bp)
+def admin_disable(uid):
+ # Group cannot be checked with login_required kwarg, because the config
+ # variable is not available when the decorator is processed
+ if not get_current_user().is_in_group(current_app.config['ACL_ADMIN_GROUP']):
+ flash('Access denied')
+ return redirect(url_for('index'))
+ user = User.from_ldap_dn(uid_to_dn(uid))
+ MFAMethod.query.filter_by(dn=user.dn).delete()
+ db.session.commit()
+ flash('Two-factor authentication was reset')
+ return redirect(url_for('user.show', uid=uid))
+
@bp.route('/setup/recovery', methods=['POST'])
@login_required()
+@csrf_protect(blueprint=bp)
def setup_recovery():
user = get_current_user()
for method in RecoveryCodeMethod.query.filter_by(dn=user.dn).all():
@@ -59,6 +79,7 @@ def setup_totp():
@bp.route('/setup/totp', methods=['POST'])
@login_required()
+@csrf_protect(blueprint=bp)
def setup_totp_finish():
user = get_current_user()
if not RecoveryCodeMethod.query.filter_by(dn=user.dn).all():
@@ -74,6 +95,7 @@ def setup_totp_finish():
@bp.route('/setup/totp/<int:id>/delete')
@login_required()
+@csrf_protect(blueprint=bp)
def delete_totp(id):
user = get_current_user()
method = TOTPMethod.query.filter_by(dn=user.dn, id=id).first_or_404()
@@ -86,6 +108,7 @@ def get_webauthn_server():
@bp.route('/setup/webauthn/begin', methods=['POST'])
@login_required()
+@csrf_protect(blueprint=bp)
def setup_webauthn_begin():
user = get_current_user()
if not RecoveryCodeMethod.query.filter_by(dn=user.dn).all():
@@ -108,6 +131,7 @@ def setup_webauthn_begin():
@bp.route('/setup/webauthn/complete', methods=['POST'])
@login_required()
+@csrf_protect(blueprint=bp)
def setup_webauthn_complete():
user = get_current_user()
server = get_webauthn_server()
@@ -123,6 +147,7 @@ def setup_webauthn_complete():
@bp.route('/setup/webauthn/<int:id>/delete')
@login_required()
+@csrf_protect(blueprint=bp)
def delete_webauthn(id):
user = get_current_user()
method = WebauthnMethod.query.filter_by(dn=user.dn, id=id).first_or_404()
@@ -136,7 +161,6 @@ def auth_webauthn_begin():
server = get_webauthn_server()
methods = WebauthnMethod.query.filter_by(dn=user.dn).all()
creds = [method.cred_data.credential_data for method in methods]
- print(creds)
if not creds:
abort(404)
auth_data, state = server.authenticate_begin(creds, user_verification=UserVerificationRequirement.DISCOURAGED)
@@ -156,44 +180,46 @@ def auth_webauthn_complete():
client_data = ClientData(data["clientDataJSON"])
auth_data = AuthenticatorData(data["authenticatorData"])
signature = data["signature"]
- print("clientData", client_data)
- print("AuthenticatorData", auth_data)
server.authenticate_complete(
- session.pop("state"),
+ session.pop("webauthn-state"),
creds,
credential_id,
client_data,
auth_data,
signature,
)
- print("ASSERTION OK")
+ session['user_mfa'] = True
return cbor.encode({"status": "OK"})
@bp.route('/auth', methods=['GET'])
-@login_required()
+@login_required(skip_mfa=True)
def auth():
user = get_current_user()
recovery_methods = RecoveryCodeMethod.query.filter_by(dn=user.dn).all()
totp_methods = TOTPMethod.query.filter_by(dn=user.dn).all()
webauthn_methods = WebauthnMethod.query.filter_by(dn=user.dn).all()
+ if not totp_methods and not webauthn_methods:
+ session['user_mfa'] = True
+ if session.get('user_mfa'):
+ return redirect(request.values.get('ref', url_for('index')))
return render_template('auth.html', ref=request.values.get('ref'), totp_methods=totp_methods,
webauthn_methods=webauthn_methods, recovery_methods=recovery_methods)
@bp.route('/auth', methods=['POST'])
-@login_required()
+@login_required(skip_mfa=True)
def auth_finish():
user = get_current_user()
recovery_methods = RecoveryCodeMethod.query.filter_by(dn=user.dn).all()
totp_methods = TOTPMethod.query.filter_by(dn=user.dn).all()
for method in totp_methods:
if method.verify(request.form['code']):
- session['mfa_verifed'] = True
+ session['user_mfa'] = True
return redirect(request.values.get('ref', url_for('index')))
for method in recovery_methods:
if method.verify(request.form['code']):
db.session.delete(method)
db.session.commit()
- session['mfa_verifed'] = True
+ session['user_mfa'] = True
if len(recovery_methods) <= 1:
flash('You have exhausted your recovery codes. Please generate new ones now!')
return redirect(url_for('mfa.setup'))
diff --git a/uffd/selfservice/templates/self.html b/uffd/selfservice/templates/self.html
index 874fac7d..026aaf61 100644
--- a/uffd/selfservice/templates/self.html
+++ b/uffd/selfservice/templates/self.html
@@ -1,6 +1,11 @@
{% extends 'base.html' %}
{% block body %}
+
+<div class="btn-toolbar">
+ <a class="ml-auto mb-3 btn btn-primary" href="{{ url_for('mfa.setup') }}">Manage two-factor authentication</a>
+</div>
+
<form action="{{ url_for("selfservice.update") }}" method="POST" onInput="
password2.setCustomValidity(password1.value != password2.value ? 'Passwords do not match.' : '');
password1.setCustomValidity((password1.value.length < 8 || password1.value.length == 0) ? 'Password is too short' : '') ">
diff --git a/uffd/session/views.py b/uffd/session/views.py
index 5cb275bb..561d1909 100644
--- a/uffd/session/views.py
+++ b/uffd/session/views.py
@@ -22,11 +22,9 @@ def login():
username = request.form['loginname']
password = request.form['password']
conn = user_conn(username, password)
- if not conn:
- flash('Login name or password is wrong')
- return redirect(url_for('.login'))
- conn.search(conn.user, '(objectClass=person)')
- if not len(conn.entries) == 1:
+ if conn:
+ conn.search(conn.user, '(objectClass=person)')
+ if not conn or len(conn.entries) != 1:
flash('Login name or password is wrong')
return render_template('login.html', ref=request.values.get('ref'))
user = User.from_ldap(conn.entries[0])
@@ -36,7 +34,7 @@ def login():
session['user_uid'] = user.uid
session['logintime'] = datetime.datetime.now().timestamp()
session['_csrf_token'] = secrets.token_hex(128)
- return redirect(request.values.get('ref', url_for('index')))
+ return redirect(url_for('mfa.auth', ref=request.values.get('ref', url_for('index'))))
def get_current_user():
if not session.get('user_uid'):
@@ -45,22 +43,32 @@ def get_current_user():
request.current_user = User.from_ldap_dn(uid_to_dn(session['user_uid']))
return request.current_user
-def is_valid_session():
+def login_valid():
user = get_current_user()
if not user:
return False
if datetime.datetime.now().timestamp() > session['logintime'] + current_app.config['SESSION_LIFETIME_SECONDS']:
return False
return True
+
+def is_valid_session():
+ if not login_valid():
+ return False
+ if not session.get('user_mfa'):
+ return False
+ return True
bp.add_app_template_global(is_valid_session)
-def login_required(group=None):
+def login_required(group=None, skip_mfa=False):
def wrapper(func):
@functools.wraps(func)
def decorator(*args, **kwargs):
- if not is_valid_session():
+ if not login_valid():
flash('You need to login first')
return redirect(url_for('session.login', ref=request.url))
+ if not skip_mfa and not session.get('user_mfa'):
+ print('redirecting login_required', skip_mfa, session.get('user_mfa'))
+ return redirect(url_for('mfa.auth', ref=request.url))
if not get_current_user().is_in_group(group):
flash('Access denied')
return redirect(url_for('index'))
diff --git a/uffd/user/templates/user.html b/uffd/user/templates/user.html
index 68dcb7ea..bb9c1e6a 100644
--- a/uffd/user/templates/user.html
+++ b/uffd/user/templates/user.html
@@ -6,6 +6,7 @@
<div class="float-sm-right pb-2">
<button type="submit" class="btn btn-primary"><i class="fa fa-save" aria-hidden="true"></i> Save</button>
<a href="{{ url_for("user.index") }}" class="btn btn-secondary">Cancel</a>
+ <a href="{{ url_for("mfa.admin_disable", uid=user.uid) }}" class="btn btn-secondary">Reset 2FA</a>
{% if user.uid %}
<a href="{{ url_for("user.delete", uid=user.uid) }}" class="btn btn-danger"><i class="fa fa-trash" aria-hidden="true"></i> Delete</a>
{% else %}
--
GitLab