From 2d99ad55964d15ab767cab186d0f1acea499ca04 Mon Sep 17 00:00:00 2001
From: Julian Rother <julianr@fsmpi.rwth-aachen.de>
Date: Wed, 16 Jun 2021 22:53:45 +0200
Subject: [PATCH] Replace ROLES_BASEROLES with db attribute and make base role
 membership implicit

---
 .../aff5f350dcdf_added_role_is_default.py     | 23 +++++
 tests/test_invite.py                          | 14 ++-
 tests/test_role.py                            | 88 ++++++++++++++++---
 tests/test_user.py                            | 35 ++++----
 uffd/default_config.cfg                       |  2 -
 uffd/invite/models.py                         |  1 -
 uffd/invite/templates/invite/new.html         |  2 +-
 uffd/role/models.py                           | 11 ++-
 uffd/role/templates/role/show.html            |  6 ++
 uffd/role/views.py                            | 30 +++++++
 .../templates/selfservice/self.html           |  2 +-
 uffd/signup/models.py                         |  5 --
 uffd/user/templates/user/list.html            |  2 +-
 uffd/user/templates/user/show.html            |  2 +-
 uffd/user/views_user.py                       |  8 +-
 15 files changed, 175 insertions(+), 56 deletions(-)
 create mode 100644 migrations/versions/aff5f350dcdf_added_role_is_default.py

diff --git a/migrations/versions/aff5f350dcdf_added_role_is_default.py b/migrations/versions/aff5f350dcdf_added_role_is_default.py
new file mode 100644
index 00000000..b903fbd7
--- /dev/null
+++ b/migrations/versions/aff5f350dcdf_added_role_is_default.py
@@ -0,0 +1,23 @@
+"""added role.is_default
+
+Revision ID: aff5f350dcdf
+Revises: a594d3b3e05b
+Create Date: 2021-06-15 21:24:13.158828
+
+"""
+from alembic import op
+import sqlalchemy as sa
+
+# revision identifiers, used by Alembic.
+revision = 'aff5f350dcdf'
+down_revision = 'a594d3b3e05b'
+branch_labels = None
+depends_on = None
+
+def upgrade():
+	with op.batch_alter_table('role', schema=None) as batch_op:
+		batch_op.add_column(sa.Column('is_default', sa.Boolean(name=op.f('ck_role_is_default')), nullable=False, default=False))
+
+def downgrade():
+	with op.batch_alter_table('role', schema=None) as batch_op:
+		batch_op.drop_column('is_default')
diff --git a/tests/test_invite.py b/tests/test_invite.py
index c21a15c1..8d4b7773 100644
--- a/tests/test_invite.py
+++ b/tests/test_invite.py
@@ -131,7 +131,7 @@ class TestInviteGrantModel(UffdTestCase):
 		ldap.session.commit()
 		db_flush()
 		user = User.query.get('uid=testuser,ou=users,dc=example,dc=com')
-		self.assertIn('baserole', [role.name for role in user.roles])
+		self.assertIn('baserole', [role.name for role in user.roles_recursive])
 		self.assertIn('testrole1', [role.name for role in user.roles])
 		self.assertIn('testrole2', [role.name for role in user.roles])
 		self.assertIn('cn=uffd_access,ou=groups,dc=example,dc=com', [group.dn for group in user.groups])
@@ -174,8 +174,7 @@ class TestInviteGrantModel(UffdTestCase):
 
 class TestInviteSignupModel(UffdTestCase):
 	def create_base_roles(self):
-		self.app.config['ROLES_BASEROLES'] = ['base']
-		baserole = Role(name='base')
+		baserole = Role(name='base', is_default=True)
 		baserole.groups.add(Group.query.get('cn=uffd_access,ou=groups,dc=example,dc=com'))
 		baserole.groups.add(Group.query.get('cn=users,ou=groups,dc=example,dc=com'))
 		db.session.add(baserole)
@@ -204,7 +203,7 @@ class TestInviteSignupModel(UffdTestCase):
 		self.assertEqual(user.displayname, 'New User')
 		self.assertEqual(user.mail, 'test@example.com')
 		self.assertEqual(signup.user.dn, user.dn)
-		self.assertIn(base_role, user.roles)
+		self.assertIn(base_role, user.roles_recursive)
 		self.assertIn(role1, user.roles)
 		self.assertIn(role2, user.roles)
 		self.assertIn(base_group1, user.groups)
@@ -233,8 +232,8 @@ class TestInviteSignupModel(UffdTestCase):
 		self.assertEqual(user.displayname, 'New User')
 		self.assertEqual(user.mail, 'test@example.com')
 		self.assertEqual(signup.user.dn, user.dn)
-		self.assertIn(base_role, user.roles)
-		self.assertEqual(len(user.roles), 1)
+		self.assertIn(base_role, user.roles_recursive)
+		self.assertEqual(len(user.roles_recursive), 1)
 		self.assertIn(base_group1, user.groups)
 		self.assertIn(base_group2, user.groups)
 		self.assertEqual(len(user.groups), 2)
@@ -637,8 +636,7 @@ class TestInviteUseViews(UffdTestCase):
 		self.assertFalse(Invite.query.filter_by(token=token).first().used)
 
 	def test_signup(self):
-		self.app.config['ROLES_BASEROLES'] = ['base']
-		baserole = Role(name='base')
+		baserole = Role(name='base', is_default=True)
 		baserole.groups.add(Group.query.get('cn=uffd_access,ou=groups,dc=example,dc=com'))
 		baserole.groups.add(Group.query.get('cn=users,ou=groups,dc=example,dc=com'))
 		db.session.add(baserole)
diff --git a/tests/test_role.py b/tests/test_role.py
index 74f4550e..4cadd82d 100644
--- a/tests/test_role.py
+++ b/tests/test_role.py
@@ -4,7 +4,8 @@ import time
 from flask import url_for, session
 
 # These imports are required, because otherwise we get circular imports?!
-from uffd import ldap, user
+from uffd.ldap import ldap
+from uffd import user
 
 from uffd.user.models import User, Group
 from uffd.role.models import Role
@@ -16,13 +17,14 @@ class TestUserRoleAttributes(UffdTestCase):
 	def test_roles_recursive(self):
 		user1 = User.query.get('uid=testuser,ou=users,dc=example,dc=com')
 		user1.update_groups()
-		baserole = Role(name='base')
-		role1 = Role(name='role1', members=[user1], included_roles=[baserole])
-		role2 = Role(name='role2', included_roles=[baserole])
-		db.session.add_all([baserole, role1, role2])
-		self.assertSetEqual(user1.roles_recursive, {baserole, role1})
-		baserole.included_roles.append(role2)
-		self.assertSetEqual(user1.roles_recursive, {baserole, role1, role2})
+		included_role = Role(name='included')
+		default_role = Role(name='default', is_default=True)
+		role1 = Role(name='role1', members=[user1], included_roles=[included_role])
+		role2 = Role(name='role2', included_roles=[included_role])
+		db.session.add_all([included_role, default_role, role1, role2])
+		self.assertSetEqual(user1.roles_recursive, {included_role, default_role, role1})
+		included_role.included_roles.append(role2)
+		self.assertSetEqual(user1.roles_recursive, {included_role, default_role, role1, role2})
 
 	def test_update_groups(self):
 		user1 = User.query.get('uid=testuser,ou=users,dc=example,dc=com')
@@ -42,12 +44,12 @@ class TestUserRoleAttributes(UffdTestCase):
 class TestRoleModel(UffdTestCase):
 	def test_indirect_members(self):
 		user1 = User.query.get('uid=testuser,ou=users,dc=example,dc=com')
-		user1.update_groups()
 		user2 = User.query.get('uid=testadmin,ou=users,dc=example,dc=com')
-		user2.update_groups()
-		baserole = Role(name='base', members=[user1])
-		role1 = Role(name='role1', included_roles=[baserole], members=[user2])
-		self.assertSetEqual(baserole.indirect_members, {user2})
+		included_role = Role(name='included', members=[user1])
+		default_role = Role(name='default', is_default=True)
+		role1 = Role(name='role1', included_roles=[included_role], members=[user2])
+		self.assertSetEqual(included_role.indirect_members, {user2})
+		self.assertSetEqual(default_role.indirect_members, {user1, user2})
 		self.assertSetEqual(role1.indirect_members, set())
 
 	def test_included_roles_recursive(self):
@@ -184,5 +186,65 @@ class TestRoleViews(UffdTestCase):
 		self.assertIsNone(Role.query.get(role_id))
 		# TODO: verify that group memberships are updated (currently not possible with ldap mock!)
 
+	def test_set_default(self):
+		for user in User.query.filter_by(loginname='service').all():
+			ldap.session.delete(user)
+		ldap.session.add(User(loginname='service', is_service_user=True, mail='service@example.com', displayname='Service'))
+		ldap.session.commit()
+		role = Role(name='test')
+		db.session.add(role)
+		user1 = User.query.get('uid=testuser,ou=users,dc=example,dc=com')
+		user2 = User.query.get('uid=testadmin,ou=users,dc=example,dc=com')
+		service_user = User.query.get('uid=service,ou=users,dc=example,dc=com')
+		self.assertSetEqual(set(user1.roles_recursive), set())
+		self.assertSetEqual(set(user2.roles_recursive), set())
+		self.assertSetEqual(set(service_user.roles_recursive), set())
+		role.members.add(user1)
+		role.members.add(service_user)
+		self.assertSetEqual(set(user1.roles_recursive), {role})
+		self.assertSetEqual(set(user2.roles_recursive), set())
+		self.assertSetEqual(set(service_user.roles_recursive), {role})
+		db.session.commit()
+		role_id = role.id
+		self.assertSetEqual(set(role.members), {user1, service_user})
+		r = self.client.get(path=url_for('role.set_default', roleid=role.id), follow_redirects=True)
+		dump('role_set_default', r)
+		self.assertEqual(r.status_code, 200)
+		role = Role.query.get(role_id)
+		service_user = User.query.get('uid=service,ou=users,dc=example,dc=com')
+		self.assertSetEqual(set(role.members), {service_user})
+		self.assertSetEqual(set(user1.roles_recursive), {role})
+		self.assertSetEqual(set(user2.roles_recursive), {role})
+		ldap.session.delete(service_user)
+		ldap.session.commit()
+
+	def test_unset_default(self):
+		for user in User.query.filter_by(loginname='service').all():
+			ldap.session.delete(user)
+		ldap.session.add(User(loginname='service', is_service_user=True, mail='service@example.com', displayname='Service'))
+		ldap.session.commit()
+		role = Role(name='test', is_default=True)
+		db.session.add(role)
+		user1 = User.query.get('uid=testuser,ou=users,dc=example,dc=com')
+		user2 = User.query.get('uid=testadmin,ou=users,dc=example,dc=com')
+		service_user = User.query.get('uid=service,ou=users,dc=example,dc=com')
+		role.members.add(service_user)
+		self.assertSetEqual(set(user1.roles_recursive), {role})
+		self.assertSetEqual(set(user2.roles_recursive), {role})
+		self.assertSetEqual(set(service_user.roles_recursive), {role})
+		db.session.commit()
+		role_id = role.id
+		self.assertSetEqual(set(role.members), {service_user})
+		r = self.client.get(path=url_for('role.unset_default', roleid=role.id), follow_redirects=True)
+		dump('role_unset_default', r)
+		self.assertEqual(r.status_code, 200)
+		role = Role.query.get(role_id)
+		service_user = User.query.get('uid=service,ou=users,dc=example,dc=com')
+		self.assertSetEqual(set(role.members), {service_user})
+		self.assertSetEqual(set(user1.roles_recursive), set())
+		self.assertSetEqual(set(user2.roles_recursive), set())
+		ldap.session.delete(service_user)
+		ldap.session.commit()
+
 class TestRoleViewsOL(TestRoleViews):
 	use_openldap = True
diff --git a/tests/test_user.py b/tests/test_user.py
index f9fb574b..e2e03de3 100644
--- a/tests/test_user.py
+++ b/tests/test_user.py
@@ -67,7 +67,7 @@ class TestUserViews(UffdTestCase):
 		self.assertEqual(r.status_code, 200)
 
 	def test_new(self):
-		db.session.add(Role(name='base'))
+		db.session.add(Role(name='base', is_default=True))
 		role1 = Role(name='role1')
 		db.session.add(role1)
 		role2 = Role(name='role2')
@@ -84,7 +84,7 @@ class TestUserViews(UffdTestCase):
 		dump('user_new_submit', r)
 		self.assertEqual(r.status_code, 200)
 		user = User.query.get('uid=newuser,ou=users,dc=example,dc=com')
-		roles = sorted([r.name for r in user.roles])
+		roles = sorted([r.name for r in user.roles_recursive])
 		self.assertIsNotNone(user)
 		self.assertFalse(user.is_service_user)
 		self.assertEqual(user.loginname, 'newuser')
@@ -97,7 +97,7 @@ class TestUserViews(UffdTestCase):
 		#self.assertTrue(ldap.test_user_bind(user.dn, 'newpassword'))
 
 	def test_new_service(self):
-		db.session.add(Role(name='base'))
+		db.session.add(Role(name='base', is_default=True))
 		role1 = Role(name='role1')
 		db.session.add(role1)
 		role2 = Role(name='role2')
@@ -160,7 +160,7 @@ class TestUserViews(UffdTestCase):
 
 	def test_update(self):
 		user = get_user()
-		db.session.add(Role(name='base'))
+		db.session.add(Role(name='base', is_default=True))
 		role1 = Role(name='role1')
 		db.session.add(role1)
 		role2 = Role(name='role2')
@@ -177,7 +177,7 @@ class TestUserViews(UffdTestCase):
 		dump('user_update_submit', r)
 		self.assertEqual(r.status_code, 200)
 		_user = get_user()
-		roles = sorted([r.name for r in _user.roles])
+		roles = sorted([r.name for r in _user.roles_recursive])
 		self.assertEqual(_user.displayname, 'New User')
 		self.assertEqual(_user.mail, 'newuser@example.com')
 		self.assertEqual(_user.uid, user.uid)
@@ -261,7 +261,6 @@ class TestUserViews(UffdTestCase):
 		self.assertIsNone(get_user())
 
 	def test_csvimport(self):
-		db.session.add(Role(name='base'))
 		role1 = Role(name='role1')
 		db.session.add(role1)
 		role2 = Role(name='role2')
@@ -293,42 +292,42 @@ newuser12,newuser12@example.com,{role1.id};{role1.id}
 		self.assertEqual(user.displayname, 'newuser1')
 		self.assertEqual(user.mail, 'newuser1@example.com')
 		roles = sorted([r.name for r in user.roles])
-		self.assertEqual(roles, ['base'])
+		self.assertEqual(roles, [])
 		user = User.query.get('uid=newuser2,ou=users,dc=example,dc=com')
 		self.assertIsNotNone(user)
 		self.assertEqual(user.loginname, 'newuser2')
 		self.assertEqual(user.displayname, 'newuser2')
 		self.assertEqual(user.mail, 'newuser2@example.com')
 		roles = sorted([r.name for r in user.roles])
-		self.assertEqual(roles, ['base', 'role1'])
+		self.assertEqual(roles, ['role1'])
 		user = User.query.get('uid=newuser3,ou=users,dc=example,dc=com')
 		self.assertIsNotNone(user)
 		self.assertEqual(user.loginname, 'newuser3')
 		self.assertEqual(user.displayname, 'newuser3')
 		self.assertEqual(user.mail, 'newuser3@example.com')
 		roles = sorted([r.name for r in user.roles])
-		self.assertEqual(roles, ['base', 'role1', 'role2'])
+		self.assertEqual(roles, ['role1', 'role2'])
 		user = User.query.get('uid=newuser4,ou=users,dc=example,dc=com')
 		self.assertIsNotNone(user)
 		self.assertEqual(user.loginname, 'newuser4')
 		self.assertEqual(user.displayname, 'newuser4')
 		self.assertEqual(user.mail, 'newuser4@example.com')
 		roles = sorted([r.name for r in user.roles])
-		self.assertEqual(roles, ['base'])
+		self.assertEqual(roles, [])
 		user = User.query.get('uid=newuser5,ou=users,dc=example,dc=com')
 		self.assertIsNotNone(user)
 		self.assertEqual(user.loginname, 'newuser5')
 		self.assertEqual(user.displayname, 'newuser5')
 		self.assertEqual(user.mail, 'newuser5@example.com')
 		roles = sorted([r.name for r in user.roles])
-		self.assertEqual(roles, ['base'])
+		self.assertEqual(roles, [])
 		user = User.query.get('uid=newuser6,ou=users,dc=example,dc=com')
 		self.assertIsNotNone(user)
 		self.assertEqual(user.loginname, 'newuser6')
 		self.assertEqual(user.displayname, 'newuser6')
 		self.assertEqual(user.mail, 'newuser6@example.com')
 		roles = sorted([r.name for r in user.roles])
-		self.assertEqual(roles, ['base', 'role1', 'role2'])
+		self.assertEqual(roles, ['role1', 'role2'])
 		self.assertIsNone(User.query.get('uid=newuser7,ou=users,dc=example,dc=com'))
 		self.assertIsNone(User.query.get('uid=newuser8,ou=users,dc=example,dc=com'))
 		self.assertIsNone(User.query.get('uid=newuser9,ou=users,dc=example,dc=com'))
@@ -338,7 +337,7 @@ newuser12,newuser12@example.com,{role1.id};{role1.id}
 		self.assertEqual(user.displayname, 'newuser10')
 		self.assertEqual(user.mail, 'newuser10@example.com')
 		roles = sorted([r.name for r in user.roles])
-		self.assertEqual(roles, ['base'])
+		self.assertEqual(roles, [])
 		user = User.query.get('uid=newuser11,ou=users,dc=example,dc=com')
 		self.assertIsNotNone(user)
 		self.assertEqual(user.loginname, 'newuser11')
@@ -346,15 +345,15 @@ newuser12,newuser12@example.com,{role1.id};{role1.id}
 		self.assertEqual(user.mail, 'newuser11@example.com')
 		# Currently the csv import is not very robust, imho newuser11 should have role1 and role2!
 		roles = sorted([r.name for r in user.roles])
-		#self.assertEqual(roles, ['base', 'role1', 'role2'])
-		self.assertEqual(roles, ['base', 'role2'])
+		#self.assertEqual(roles, ['role1', 'role2'])
+		self.assertEqual(roles, ['role2'])
 		user = User.query.get('uid=newuser12,ou=users,dc=example,dc=com')
 		self.assertIsNotNone(user)
 		self.assertEqual(user.loginname, 'newuser12')
 		self.assertEqual(user.displayname, 'newuser12')
 		self.assertEqual(user.mail, 'newuser12@example.com')
 		roles = sorted([r.name for r in user.roles])
-		self.assertEqual(roles, ['base', 'role1'])
+		self.assertEqual(roles, ['role1'])
 
 class TestUserViewsOL(TestUserViews):
 	use_openldap = True
@@ -390,7 +389,7 @@ class TestUserViewsOLUserAsUser(UffdTestCase):
 
 	def test_update_other_user(self):
 		user_ = get_admin()
-		db.session.add(Role(name='base'))
+		db.session.add(Role(name='base', is_default=True))
 		role1 = Role(name='role1')
 		db.session.add(role1)
 		role2 = Role(name='role2')
@@ -413,7 +412,7 @@ class TestUserViewsOLUserAsUser(UffdTestCase):
 		self.assertEqual(_user.loginname, user_.loginname)
 
 	def test_new(self):
-		db.session.add(Role(name='base'))
+		db.session.add(Role(name='base', is_default=True))
 		role1 = Role(name='role1')
 		db.session.add(role1)
 		role2 = Role(name='role2')
diff --git a/uffd/default_config.cfg b/uffd/default_config.cfg
index 022657fe..61a93cd1 100644
--- a/uffd/default_config.cfg
+++ b/uffd/default_config.cfg
@@ -79,8 +79,6 @@ LOGINNAME_BLACKLIST=['^admin$', '^root$']
 #MFA_RP_ID = 'example.com' # If unset, hostname from current request is used
 MFA_RP_NAME = 'Uffd Test Service' # Service name passed to U2F/FIDO2 authenticators
 
-ROLES_BASEROLES=['base']
-
 SQLALCHEMY_TRACK_MODIFICATIONS=False
 
 FOOTER_LINKS=[{"url": "https://example.com", "title": "example"}]
diff --git a/uffd/invite/models.py b/uffd/invite/models.py
index 47826cd0..87698455 100644
--- a/uffd/invite/models.py
+++ b/uffd/invite/models.py
@@ -113,7 +113,6 @@ class InviteSignup(Signup):
 			return None, 'Invite link is invalid'
 		user, msg = super().finish(password)
 		if user is not None:
-			# super().finish() already added ROLES_BASEROLES
 			for role in self.invite.roles:
 				user.roles.add(role)
 			user.update_groups()
diff --git a/uffd/invite/templates/invite/new.html b/uffd/invite/templates/invite/new.html
index a27d50d6..c6731bd8 100644
--- a/uffd/invite/templates/invite/new.html
+++ b/uffd/invite/templates/invite/new.html
@@ -37,7 +37,7 @@
 				</tr>
 			</thead>
 			<tbody>
-				{% for role in roles|sort(attribute="name") if role.name not in config['ROLES_BASEROLES'] %}
+				{% for role in roles|sort(attribute="name") if not role.is_default %}
 				<tr>
 					<td>
 						<div class="form-check">
diff --git a/uffd/role/models.py b/uffd/role/models.py
index dad00f53..f8bce756 100644
--- a/uffd/role/models.py
+++ b/uffd/role/models.py
@@ -51,7 +51,10 @@ def flatten_recursive(objs, attr):
 	return objs
 
 def get_roles_recursive(user):
-	return flatten_recursive(user.roles, 'included_roles')
+	base = set(user.roles)
+	if not user.is_service_user:
+		base.update(Role.query.filter_by(is_default=True))
+	return flatten_recursive(base, 'included_roles')
 
 User.roles_recursive = property(get_roles_recursive)
 
@@ -96,11 +99,17 @@ class Role(db.Model):
 	# and groups as well as deletion in the web interface.
 	locked = Column(Boolean(), default=False, nullable=False)
 
+	is_default = Column(Boolean(), default=False, nullable=False)
+
 	@property
 	def indirect_members(self):
 		users = set()
 		for role in flatten_recursive(self.including_roles, 'including_roles'):
 			users.update(role.members)
+		if self.is_default:
+			for user in User.query.all():
+				if not user.is_service_user:
+					users.add(user)
 		return users
 
 	@property
diff --git a/uffd/role/templates/role/show.html b/uffd/role/templates/role/show.html
index 81d96395..db2c3810 100644
--- a/uffd/role/templates/role/show.html
+++ b/uffd/role/templates/role/show.html
@@ -13,8 +13,14 @@ Name, moderator group, included roles and groups of this role are managed extern
 		<button type="submit" class="btn btn-primary"><i class="fa fa-save" aria-hidden="true"></i> Save</button>
 		<a href="{{ url_for("role.index") }}" class="btn btn-secondary">Cancel</a>
 		{% if role.id %}
+			{% if not role.is_default %}
+			<a href="{{ url_for("role.set_default", roleid=role.id) }}" onClick="return confirm('All non-service users will be removed as members from this role and get its permissions implicitly. Are you sure?');" class="btn btn-secondary">Set as default</a>
+			{% else %}
+			<a href="{{ url_for("role.unset_default", roleid=role.id) }}" onClick="return confirm('Are you sure?');" class="btn btn-secondary">Unset as default</a>
+			{% endif %}
 			<a href="{{ url_for("role.delete", roleid=role.id) }}"  onClick="return confirm('Are you sure?');" class="btn btn-danger {{ 'disabled' if role.locked }}"><i class="fa fa-trash" aria-hidden="true"></i> Delete</a>
 		{% else %}
+			<a href="#" class="btn btn-secondary disabled">Set as default</a>
 			<a href="#" class="btn btn-danger disabled"><i class="fa fa-trash" aria-hidden="true"></i> Delete</a>
 		{% endif %}
 	</div>
diff --git a/uffd/role/views.py b/uffd/role/views.py
index a613a191..3036a86d 100644
--- a/uffd/role/views.py
+++ b/uffd/role/views.py
@@ -116,3 +116,33 @@ def unlock(roleid):
 	role.locked = False
 	db.session.commit()
 	return redirect(url_for('role.show', roleid=role.id))
+
+@bp.route("/<int:roleid>/setdefault")
+@csrf_protect(blueprint=bp)
+def set_default(roleid):
+	role = Role.query.filter_by(id=roleid).one()
+	if role.is_default:
+		return redirect(url_for('role.show', roleid=role.id))
+	role.is_default = True
+	for user in set(role.members):
+		if not user.is_service_user:
+			role.members.discard(user)
+	role.update_member_groups()
+	db.session.commit()
+	ldap.session.commit()
+	return redirect(url_for('role.show', roleid=role.id))
+
+@bp.route("/<int:roleid>/unsetdefault")
+@csrf_protect(blueprint=bp)
+def unset_default(roleid):
+	role = Role.query.filter_by(id=roleid).one()
+	if not role.is_default:
+		return redirect(url_for('role.show', roleid=role.id))
+	old_members = set(role.members).union(role.indirect_members)
+	role.is_default = False
+	for user in old_members:
+		if not user.is_service_user:
+			user.update_groups()
+	db.session.commit()
+	ldap.session.commit()
+	return redirect(url_for('role.show', roleid=role.id))
diff --git a/uffd/selfservice/templates/selfservice/self.html b/uffd/selfservice/templates/selfservice/self.html
index 8d328be7..85d7b903 100644
--- a/uffd/selfservice/templates/selfservice/self.html
+++ b/uffd/selfservice/templates/selfservice/self.html
@@ -48,7 +48,7 @@
 				You currently have these roles:
 			{% endif %}
 			<ul>
-				{% for role in user.roles|sort(attribute="name") if role.name not in config["ROLES_BASEROLES"] %}
+				{% for role in user.roles|sort(attribute="name") %}
 				<li>{{ role.name }}</li>
 				{% endfor %}
 			</ul>
diff --git a/uffd/signup/models.py b/uffd/signup/models.py
index 2fabc8e5..7c8dd665 100644
--- a/uffd/signup/models.py
+++ b/uffd/signup/models.py
@@ -2,14 +2,12 @@ import secrets
 import datetime
 from crypt import crypt
 
-from flask import current_app
 from sqlalchemy import Column, String, Text, DateTime
 from ldapalchemy.dbutils import DBRelationship
 
 from uffd.database import db
 from uffd.ldap import ldap
 from uffd.user.models import User
-from uffd.role.models import Role
 
 class Signup(db.Model):
 	'''Model that represents a self-signup request
@@ -100,9 +98,6 @@ class Signup(db.Model):
 			return None, 'A user with this login name already exists'
 		user = User(loginname=self.loginname, displayname=self.displayname, mail=self.mail, password=password)
 		ldap.session.add(user)
-		for name in current_app.config['ROLES_BASEROLES']:
-			for role in Role.query.filter_by(name=name).all():
-				user.roles.add(role)
 		user.update_groups()
 		self.user = user
 		self.loginname = None
diff --git a/uffd/user/templates/user/list.html b/uffd/user/templates/user/list.html
index 93d7da52..a6854d9f 100644
--- a/uffd/user/templates/user/list.html
+++ b/uffd/user/templates/user/list.html
@@ -38,7 +38,7 @@
 						{{ user.displayname }}
 					</td>
 					<td>
-					{% for role in user.roles|sort(attribute="name") if not role.name in config["ROLES_BASEROLES"] or user.is_service_user %}
+					{% for role in user.roles|sort(attribute="name") %}
 						<a href="{{ url_for("role.show", roleid=role.id) }}">{{ role.name }}</a>{% if not loop.last %}, {% endif %}
 					{% endfor %}
 					</td>
diff --git a/uffd/user/templates/user/show.html b/uffd/user/templates/user/show.html
index d638f7fc..1511e74b 100644
--- a/uffd/user/templates/user/show.html
+++ b/uffd/user/templates/user/show.html
@@ -106,7 +106,7 @@
 								<div class="form-check">
 									<input class="form-check-input" type="checkbox" id="role-{{ role.id }}-checkbox" name="role-{{ role.id }}" value="1" aria-label="enabled"
 										{% if user in role.members %}checked {% endif %}
-										{% if role.name in config["ROLES_BASEROLES"] and not user.is_service_user %}disabled {% endif %}>
+										{% if role.is_default and not user.is_service_user %}disabled {% endif %}>
 								</div>
 							</td>
 							<td>
diff --git a/uffd/user/views_user.py b/uffd/user/views_user.py
index a76beceb..193a3507 100644
--- a/uffd/user/views_user.py
+++ b/uffd/user/views_user.py
@@ -62,10 +62,10 @@ def update(uid=None):
 	ldap.session.add(user)
 	user.roles.clear()
 	for role in Role.query.all():
+		if not user.is_service_user and role.is_default:
+			continue
 		if request.values.get('role-{}'.format(role.id), False):
 			user.roles.add(role)
-		elif not user.is_service_user and role.name in current_app.config["ROLES_BASEROLES"]:
-			user.roles.add(role)
 	user.update_groups()
 	ldap.session.commit()
 	db.session.commit()
@@ -100,7 +100,7 @@ def csvimport():
 
 	ignore_blacklist = request.values.get('ignore-loginname-blacklist', False)
 
-	roles = Role.query.all()
+	roles = Role.query.filter_by(is_default=False).all()
 	usersadded = 0
 	with io.StringIO(initial_value=csvdata) as csvfile:
 		csvreader = csv.reader(csvfile)
@@ -117,7 +117,7 @@ def csvimport():
 				continue
 			ldap.session.add(newuser)
 			for role in roles:
-				if (str(role.id) in row[2].split(';')) or role.name in current_app.config["ROLES_BASEROLES"]:
+				if str(role.id) in row[2].split(';'):
 					role.members.add(newuser)
 			newuser.update_groups()
 			try:
-- 
GitLab