From 661703c43d0f3966cd99864c77cd881b4f3779e2 Mon Sep 17 00:00:00 2001
From: Julian Rother <julianr@fsmpi.rwth-aachen.de>
Date: Sun, 25 Jul 2021 11:26:17 +0200
Subject: [PATCH] Selfservice redesign and button to leave roles

---
 tests/test_selfservice.py                     |  71 +++++++-
 tests/utils.py                                |   1 +
 uffd/__init__.py                              |   7 +-
 uffd/default_config.cfg                       |   1 +
 .../templates/selfservice/self.html           | 165 +++++++++++++-----
 uffd/selfservice/views.py                     |  47 +++--
 6 files changed, 221 insertions(+), 71 deletions(-)

diff --git a/tests/test_selfservice.py b/tests/test_selfservice.py
index c6018f2d..f36cfe93 100644
--- a/tests/test_selfservice.py
+++ b/tests/test_selfservice.py
@@ -8,6 +8,7 @@ from uffd import ldap, user
 
 from uffd.selfservice.models import MailToken, PasswordToken
 from uffd.user.models import User
+from uffd.role.models import Role
 from uffd import create_app, db
 
 from utils import dump, UffdTestCase
@@ -27,8 +28,8 @@ class TestSelfservice(UffdTestCase):
 	def test_update_displayname(self):
 		self.login_as('user')
 		user = request.user
-		r = self.client.post(path=url_for('selfservice.update'),
-			data={'displayname': 'New Display Name', 'mail': user.mail, 'password': '', 'password1': ''},
+		r = self.client.post(path=url_for('selfservice.update_profile'),
+			data={'displayname': 'New Display Name', 'mail': user.mail},
 			follow_redirects=True)
 		dump('update_displayname', r)
 		self.assertEqual(r.status_code, 200)
@@ -38,8 +39,8 @@ class TestSelfservice(UffdTestCase):
 	def test_update_displayname_invalid(self):
 		self.login_as('user')
 		user = request.user
-		r = self.client.post(path=url_for('selfservice.update'),
-			data={'displayname': '', 'mail': user.mail, 'password': '', 'password1': ''},
+		r = self.client.post(path=url_for('selfservice.update_profile'),
+			data={'displayname': '', 'mail': user.mail},
 			follow_redirects=True)
 		dump('update_displayname_invalid', r)
 		self.assertEqual(r.status_code, 200)
@@ -49,8 +50,8 @@ class TestSelfservice(UffdTestCase):
 	def test_update_mail(self):
 		self.login_as('user')
 		user = request.user
-		r = self.client.post(path=url_for('selfservice.update'),
-			data={'displayname': user.displayname, 'mail': 'newemail@example.com', 'password': '', 'password1': ''},
+		r = self.client.post(path=url_for('selfservice.update_profile'),
+			data={'displayname': user.displayname, 'mail': 'newemail@example.com'},
 			follow_redirects=True)
 		dump('update_mail', r)
 		self.assertEqual(r.status_code, 200)
@@ -68,8 +69,8 @@ class TestSelfservice(UffdTestCase):
 		self.app.config['MAIL_SKIP_SEND'] = 'fail'
 		self.login_as('user')
 		user = request.user
-		r = self.client.post(path=url_for('selfservice.update'),
-			data={'displayname': user.displayname, 'mail': 'newemail@example.com', 'password': '', 'password1': ''},
+		r = self.client.post(path=url_for('selfservice.update_profile'),
+			data={'displayname': user.displayname, 'mail': 'newemail@example.com'},
 			follow_redirects=True)
 		dump('update_mail_sendfailure', r)
 		self.assertEqual(r.status_code, 200)
@@ -77,6 +78,60 @@ class TestSelfservice(UffdTestCase):
 		self.assertNotEqual(_user.mail, 'newemail@example.com')
 		# Maybe also check that there is no new token in the db
 
+	def test_change_password(self):
+		self.login_as('user')
+		user = request.user
+		r = self.client.post(path=url_for('selfservice.change_password'),
+			data={'password1': 'newpassword', 'password2': 'newpassword'},
+			follow_redirects=True)
+		dump('change_password', r)
+		self.assertEqual(r.status_code, 200)
+		_user = request.user
+		self.assertTrue(ldap.test_user_bind(_user.dn, 'newpassword'))
+
+	def test_change_password_invalid(self):
+		self.login_as('user')
+		user = request.user
+		r = self.client.post(path=url_for('selfservice.change_password'),
+			data={'password1': 'shortpw', 'password2': 'shortpw'},
+			follow_redirects=True)
+		dump('change_password_invalid', r)
+		self.assertEqual(r.status_code, 200)
+		_user = request.user
+		self.assertFalse(ldap.test_user_bind(_user.dn, 'shortpw'))
+		self.assertTrue(ldap.test_user_bind(_user.dn, 'userpassword'))
+
+	def test_change_password_mismatch(self):
+		self.login_as('user')
+		user = request.user
+		r = self.client.post(path=url_for('selfservice.change_password'),
+			data={'password1': 'newpassword1', 'password2': 'newpassword2'},
+			follow_redirects=True)
+		dump('change_password_mismatch', r)
+		self.assertEqual(r.status_code, 200)
+		_user = request.user
+		self.assertFalse(ldap.test_user_bind(_user.dn, 'newpassword1'))
+		self.assertFalse(ldap.test_user_bind(_user.dn, 'newpassword2'))
+		self.assertTrue(ldap.test_user_bind(_user.dn, 'userpassword'))
+
+	def test_leave_role(self):
+		if self.use_userconnection:
+			self.skipTest('Leaving roles is not possible in user mode')
+		role1 = Role(name='testrole1')
+		role2 = Role(name='testrole2')
+		db.session.add(role1)
+		db.session.add(role2)
+		self.get_user().roles = [role1, role2]
+		db.session.commit()
+		roleid = role1.id
+		self.login_as('user')
+		r = self.client.post(path=url_for('selfservice.leave_role', roleid=roleid), follow_redirects=True)
+		dump('leave_role', r)
+		self.assertEqual(r.status_code, 200)
+		_user = self.get_user()
+		self.assertEqual(len(_user.roles), 1)
+		self.assertEqual(list(_user.roles)[0].name, 'testrole2')
+
 	def test_token_mail_emptydb(self):
 		self.login_as('user')
 		user = request.user
diff --git a/tests/utils.py b/tests/utils.py
index ee4b5762..7404f272 100644
--- a/tests/utils.py
+++ b/tests/utils.py
@@ -73,6 +73,7 @@ class UffdTestCase(unittest.TestCase):
 				config['SELF_SIGNUP'] = False
 				config['ENABLE_INVITE'] = False
 				config['ENABLE_PASSWORDRESET'] = False
+				config['ENABLE_ROLESELFSERVICE'] = False
 			else:
 				config['LDAP_SERVICE_BIND_DN'] = 'cn=uffd,ou=system,dc=example,dc=com'
 			config['LDAP_SERVICE_BIND_PASSWORD'] = 'uffd-ldap-password'
diff --git a/uffd/__init__.py b/uffd/__init__.py
index 8bd5e263..989ddab8 100644
--- a/uffd/__init__.py
+++ b/uffd/__init__.py
@@ -74,9 +74,10 @@ def create_app(test_config=None): # pylint: disable=too-many-locals
 		app.register_blueprint(i)
 
 	if app.config['LDAP_SERVICE_USER_BIND'] and (app.config['ENABLE_INVITE'] or
-												app.config['SELF_SIGNUP'] or
-												app.config['ENABLE_PASSWORDRESET']):
-		raise InternalServerError(description="You cannot use INVITES, SIGNUP or PASSWORDRESET when using a USER_BIND!")
+	                                             app.config['SELF_SIGNUP'] or
+	                                             app.config['ENABLE_PASSWORDRESET'] or
+	                                             app.config['ENABLE_ROLESELFSERVICE']):
+		raise InternalServerError(description="You cannot use INVITES, SIGNUP, PASSWORDRESET or ROLESELFSERVICE when using a USER_BIND!")
 
 	if app.config['ENABLE_INVITE'] or app.config['SELF_SIGNUP']:
 		for i in signup.bp:
diff --git a/uffd/default_config.cfg b/uffd/default_config.cfg
index 00e39f7b..23b755b5 100644
--- a/uffd/default_config.cfg
+++ b/uffd/default_config.cfg
@@ -74,6 +74,7 @@ MAIL_FROM_ADDRESS='foo@bar.com'
 # The following settings are not available when using a user connection
 ENABLE_INVITE=True
 ENABLE_PASSWORDRESET=True
+ENABLE_ROLESELFSERVICE=True
 # Do not enable this on a public service! There is no spam protection implemented at the moment.
 SELF_SIGNUP=False
 
diff --git a/uffd/selfservice/templates/selfservice/self.html b/uffd/selfservice/templates/selfservice/self.html
index e872748a..66c9d677 100644
--- a/uffd/selfservice/templates/selfservice/self.html
+++ b/uffd/selfservice/templates/selfservice/self.html
@@ -9,63 +9,132 @@
 </div>
 {% endif %}
 
-<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' : '') ">
-<div class="align-self-center row">
-	<div class="form-group col-md-6">
-		<label for="user-uid">{{_("Uid")}}</label>
-		<input type="number" class="form-control" id="user-uid" name="uid" value="{{ user.uid }}" readonly>
-	</div>
-	<div class="form-group col-md-6">
-		<label for="user-loginname">{{_("Login Name")}}</label>
-		<input type="text" class="form-control" id="user-loginname" name="loginname" value="{{ user.loginname }}" readonly>
+<div class="row mt-3">
+	<div class="col-12 col-md-5">
+		<h5>{{_("Profile")}}</h5>
+		<p>{{_("Your profile information is used by all services that are integrated into the Single-Sign-On. Your e-mail address is also used for password recovery.")}}</p>
+		<p>{{_(" Changes may take serveral minutes to be visible in all services.")}}</p>
 	</div>
-	<div class="form-group col-md-6">
-		<label for="user-displayname">{{_("Display Name")}}</label>
-		<input type="text" class="form-control" id="user-displayname" name="displayname" value="{{ user.displayname }}">
+	<div class="col-12 col-md-7">
+		<form class="form" action="{{ url_for("selfservice.update_profile") }}" method="POST">
+			<div class="form-row">
+				<div class="form-group col-12 col-md-9">
+					<label>{{_("Login Name")}}</label>
+					<input type="text" class="form-control" value="{{ user.loginname }}" readonly>
+				</div>
+				<div class="form-group col-12 col-md-3">
+					<label>{{_("User ID")}}</label>
+					<input type="text" class="form-control" value="{{ user.uid }}" readonly>
+				</div>
+			</div>
+			<div class="form-group">
+				<label>{{_("Display Name")}}</label>
+				<input type="text" class="form-control" id="user-displayname" name="displayname" value="{{ user.displayname }}">
+			</div>
+			<div class="form-group">
+				<label>{{_("E-Mail Address")}}</label>
+				<input type="email" class="form-control" id="user-mail" name="mail" value="{{ user.mail }}" required>
+				<small class="form-text text-muted">
+					{{_("We will send you a confirmation mail to this address if you change it")}}
+				</small>
+			</div>
+			<button type="submit" class="btn btn-primary btn-block">{{_("Update Profile")}}</button>
+		</form>
 	</div>
-	<div class="form-group col-md-6">
-		<label for="user-mail">{{_("Mail")}}</label>
-		<input type="email" class="form-control" id="user-mail" name="mail" value="{{ user.mail }}">
-		<small class="form-text text-muted">
-			{{_("We will send you a confirmation mail to set a new mail address.")}}
-		</small>
+</div>
+
+<hr>
+
+<div class="row mt-3">
+	<div class="col-12 col-md-5">
+		<h5>{{_("Password")}}</h5>
+		<p>{{_("Your login password for the Single-Sign-On. Only enter it on the Single-Sign-On login page! No other legit websites will ask you for this password. We do not ever need your password to assist you.")}}</p>
 	</div>
-	<div class="form-group col-md-6">
-		<label for="user-password1">{{_("Password")}}</label>
-		<input type="password" class="form-control" id="user-password1" name="password1" placeholder="●●●●●●●●">
-		<small class="form-text text-muted">
-			{{_("At least 8 and at most 256 characters, no other special requirements. But please don't be stupid, do use a password manager.")}}
-		</small>
+	<div class="col-12 col-md-7">
+		<form class="form" action="{{ url_for("selfservice.change_password") }}" method="POST">
+			<div class="form-group">
+				<input type="password" class="form-control" id="user-password1" name="password1" placeholder="{{_("New Password")}}" required>
+				<small class="form-text text-muted">
+					At least 8 and at most 256 characters, no other special requirements.
+				</small>
+			</div>
+			<div class="form-group">
+				<input type="password" class="form-control" id="user-password2" name="password2" placeholder="{{_("Repeat Password")}}" required>
+			</div>
+			<button type="submit" class="btn btn-primary btn-block">{{_("Change Password")}}</button>
+		</form>
 	</div>
-	<div class="form-group col-md-6">
-		<label for="user-password2">{{_("Password Repeat")}}</label>
-		<input type="password" class="form-control" id="user-password2" name="password2" placeholder="●●●●●●●●">
+</div>
+
+<hr>
+
+<div class="row mt-3">
+	<div class="col-12 col-md-5">
+		<h5>{{_("Two-Factor Authentication")}}</h5>
+		<p>{{_("Setting up Two-Factor Authentication (2FA) adds an additional step to the Single-Sign-On login and increases the security of your account significantly.")}}</p>
 	</div>
-	<div class="form-group">
-		{% if user.roles|length %}
-			{% if user.roles|length == 1 %}
-				{{_("You have this role")}}:
+	<div class="col-12 col-md-7">
+		<p>
+			{% if user.mfa_enabled %}
+			{{ _("Two-factor authentication is currently <strong>enabled</strong>.")|safe }}
 			{% else %}
-				{{_("You currently have these roles")}}:
+			{{ _("Two-factor authentication is currently <strong>disabled</strong>.")|safe }}
 			{% endif %}
-			<ul>
-				{% for role in user.roles|sort(attribute="name") %}
-				<li>{{ role.name }}</li>
-				{% endfor %}
-			</ul>
-		{% else %}
-			{{_("You currently don't have any roles.")}}
+		</p>
+		<a class="btn btn-primary btn-block" href="{{ url_for('mfa.setup') }}">{{_("Manage two-factor authentication")}}</a>
+	</div>
+</div>
+
+<hr>
+
+<div class="row mt-3">
+	<div class="col-12 col-md-5">
+		<h5>{{_("Roles")}}</h5>
+		<p>{{_("Aside from a set of base permissions, your roles determine the permissions of your account.")}}</p>
+		{% if config['SERVICES'] %}
+		<p>{{_("See <a href=\"%(services_url)s\">Services</a> for an overview of your current permissions.", services_url=url_for('services.index'))}}</p>
 		{% endif %}
 	</div>
-	<div class="form-group col-md-12">
-		<button type="submit" class="btn btn-primary float-right"><i class="fa fa-save" aria-hidden="true"></i> {{_("Save")}}</button>
+	<div class="col-12 col-md-7">
+		{% if config['ENABLE_INVITE'] %}
+		<p>{{_("Administrators and role moderators can invite you to new roles.")}}</p>
+		{% else %}
+		<p>{{_("Administrators can add new roles to your account.")}}</p>
+		{% endif %}
+		<table class="table">
+			<thead>
+				<tr>
+					<th scope="col">{{_("Name")}}</th>
+					<th scope="col">{{_("Description")}}</th>
+					<th scope="col"></th>
+				</tr>
+			</thead>
+			<tbody>
+				{% for role in user.roles|sort(attribute='name') %}
+				<tr>
+					<td>{{ role.name }}
+						{% if not user.mfa_enabled and role.groups.values()|selectattr('requires_mfa')|list %}
+						<i class="fas fa-exclamation-triangle text-warning" title="{{_("Some permissions in this role require you to setup two-factor authentication")}}"></i>
+						{% endif %}
+					</td>
+					<td>{{ role.description }}</td>
+					<td>
+						{% if config['ENABLE_ROLESELFSERVICE'] %}
+						<form action="{{ url_for("selfservice.leave_role", roleid=role.id) }}" method="POST" onsubmit="return confirm('Are you sure?');">
+							<button type="submit" class="btn btn-sm btn-danger float-right">{{_("Leave")}}</button>
+						</form>
+						{% endif %}
+					</td>
+				</tr>
+				{% endfor %}
+				{% if not user.roles %}
+				<tr class="table-secondary">
+					<td colspan=3 class="text-center">{{_("You currently don't have any roles")}}</td>
+				</tr>
+				{% endif %}
+			</tbody>
+		</table>
 	</div>
 </div>
-</form>
+
 {% endblock %}
diff --git a/uffd/selfservice/views.py b/uffd/selfservice/views.py
index 605512a7..fadc8fea 100644
--- a/uffd/selfservice/views.py
+++ b/uffd/selfservice/views.py
@@ -12,6 +12,7 @@ from uffd.csrf import csrf_protect
 from uffd.user.models import User
 from uffd.session import login_required
 from uffd.selfservice.models import PasswordToken, MailToken
+from uffd.role.models import Role
 from uffd.database import db
 from uffd.ldap import ldap
 from uffd.ratelimit import host_ratelimit, Ratelimit, format_delay
@@ -26,30 +27,37 @@ reset_ratelimit = Ratelimit('passwordreset', 1*60*60, 3)
 def index():
 	return render_template('selfservice/self.html', user=request.user)
 
-@bp.route("/update", methods=(['POST']))
+@bp.route("/updateprofile", methods=(['POST']))
 @csrf_protect(blueprint=bp)
 @login_required()
-def update():
-	password_changed = False
+def update_profile():
 	user = request.user
 	if request.values['displayname'] != user.displayname:
 		if user.set_displayname(request.values['displayname']):
 			flash(_('Display name changed.'))
 		else:
 			flash(_('Display name is not valid.'))
-	if request.values['password1']:
-		if not request.values['password1'] == request.values['password2']:
-			flash(_('Passwords do not match'))
-		else:
-			if user.set_password(request.values['password1']):
-				flash(_('Password changed.'))
-				password_changed = True
-			else:
-				flash(_('Password could not be set.'))
 	if request.values['mail'] != user.mail:
 		send_mail_verification(user.loginname, request.values['mail'])
 		flash(_('We sent you an email, please verify your mail address.'))
 	ldap.session.commit()
+	return redirect(url_for('selfservice.index'))
+
+@bp.route("/changepassword", methods=(['POST']))
+@csrf_protect(blueprint=bp)
+@login_required()
+def change_password():
+	password_changed = False
+	user = request.user
+	if not request.values['password1'] == request.values['password2']:
+		flash(_('Passwords do not match'))
+	else:
+		if user.set_password(request.values['password1']):
+			flash(_('Password changed'))
+			password_changed = True
+		else:
+			flash(_('Invalid password'))
+	ldap.session.commit()
 	# When using a user_connection, update the connection on password-change
 	if password_changed and current_app.config['LDAP_SERVICE_USER_BIND']:
 		session['user_pw'] = request.values['password1']
@@ -124,6 +132,21 @@ def token_mail(token):
 	db.session.commit()
 	return redirect(url_for('selfservice.index'))
 
+@bp.route("/leaverole/<int:roleid>", methods=(['POST']))
+@csrf_protect(blueprint=bp)
+@login_required()
+def leave_role(roleid):
+	if not current_app.config['ENABLE_ROLESELFSERVICE']:
+		flash('Leaving roles is disabled')
+		return redirect(url_for('selfservice.index'))
+	role = Role.query.get_or_404(roleid)
+	role.members.discard(request.user)
+	request.user.update_groups()
+	ldap.session.commit()
+	db.session.commit()
+	flash('You left role "%s"'%role.name)
+	return redirect(url_for('selfservice.index'))
+
 def send_mail_verification(loginname, newmail):
 	expired_tokens = MailToken.query.filter(MailToken.created < (datetime.datetime.now() - datetime.timedelta(days=2))).all()
 	duplicate_tokens = MailToken.query.filter(MailToken.loginname == loginname).all()
-- 
GitLab