diff --git a/tests/migrations/__init__.py b/tests/migrations/__init__.py new file mode 100644 index 0000000000000000000000000000000000000000..e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 diff --git a/tests/migrations/test_missing_locks.py b/tests/migrations/test_missing_locks.py new file mode 100644 index 0000000000000000000000000000000000000000..4261853034158961e22cb71e152a614daf427ac6 --- /dev/null +++ b/tests/migrations/test_missing_locks.py @@ -0,0 +1,16 @@ +from uffd.database import db +from uffd.models.misc import lock_table, Lock + +from tests.utils import MigrationTestCase + +class TestForMissingLockRows(MigrationTestCase): + def test_check_missing_lock_rows(self): + self.upgrade('head') + existing_locks = {row[0] for row in db.session.execute(db.select([lock_table.c.name])).fetchall()} + for name in Lock.ALL_LOCKS - existing_locks: + self.fail(f'Lock "{name}" is missing. Make sure to add a migration that inserts it.') + +# Add something like this: +# conn = op.get_bind() +# lock_table = sa.table('lock', sa.column('name')) +# conn.execute(sa.insert(lock_table).values(name='NAME')) diff --git a/tests/migrations/versions/__init__.py b/tests/migrations/versions/__init__.py new file mode 100644 index 0000000000000000000000000000000000000000..e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 diff --git a/tests/migrations/versions/test_aeb07202a6c8_locking_and_new_id_allocation.py b/tests/migrations/versions/test_aeb07202a6c8_locking_and_new_id_allocation.py new file mode 100644 index 0000000000000000000000000000000000000000..5d5e96559f0c90bfcc2a4912d07531c658a2f05d --- /dev/null +++ b/tests/migrations/versions/test_aeb07202a6c8_locking_and_new_id_allocation.py @@ -0,0 +1,144 @@ +from uffd.database import db +from uffd.models.misc import lock_table, Lock + +from tests.utils import MigrationTestCase + +user_table = db.table('user', + db.column('id'), + db.column('unix_uid'), + db.column('loginname'), + db.column('displayname'), + db.column('primary_email_id'), + db.column('is_service_user'), +) + +user_email_table = db.table('user_email', + db.column('id'), + db.column('address'), + db.column('address_normalized'), + db.column('verified'), +) + +group_table = db.table('group', + db.column('id'), + db.column('unix_gid'), + db.column('name'), + db.column('description') +) + +uid_allocation_table = db.table('uid_allocation', db.column('id')) +gid_allocation_table = db.table('gid_allocation', db.column('id')) + +class TestMigration(MigrationTestCase): + REVISION = 'aeb07202a6c8' + + def setUpApp(self): + self.app.config['USER_MIN_UID'] = 10000 + self.app.config['USER_MAX_UID'] = 10005 + self.app.config['USER_SERVICE_MIN_UID'] = 10006 + self.app.config['USER_SERVICE_MAX_UID'] = 10010 + self.app.config['GROUP_MIN_GID'] = 20000 + self.app.config['GROUP_MAX_GID'] = 20005 + + def create_user(self, uid): + db.session.execute(db.insert(user_email_table).values( + address=f'email{uid}@example.com', + address_normalized=f'email{uid}@example.com', + verified=True + )) + email_id = db.session.execute( + db.select([user_email_table.c.id]) + .where(user_email_table.c.address == f'email{uid}@example.com') + ).scalar() + db.session.execute(db.insert(user_table).values( + unix_uid=uid, + loginname=f'user{uid}', + displayname='user', + primary_email_id=email_id, + is_service_user=False + )) + + def create_group(self, gid): + db.session.execute(db.insert(group_table).values(unix_gid=gid, name=f'group{gid}', description='')) + + def fetch_uid_allocations(self): + return [row[0] for row in db.session.execute( + db.select([uid_allocation_table]) + .order_by(uid_allocation_table.c.id) + ).fetchall()] + + def fetch_gid_allocations(self): + return [row[0] for row in db.session.execute( + db.select([gid_allocation_table]) + .order_by(gid_allocation_table.c.id) + ).fetchall()] + + def test_empty(self): + # No users/groups + self.upgrade() + self.assertEqual(self.fetch_uid_allocations(), []) + self.assertEqual(self.fetch_gid_allocations(), []) + + def test_gid_first_minus_one(self): + self.create_group(19999) + self.upgrade() + self.assertEqual(self.fetch_gid_allocations(), [19999]) + + def test_gid_first(self): + self.create_group(20000) + self.upgrade() + self.assertEqual(self.fetch_gid_allocations(), [20000]) + + def test_gid_first_plus_one(self): + self.create_group(20001) + self.upgrade() + self.assertEqual(self.fetch_gid_allocations(), [20000, 20001]) + + def test_gid_last_minus_one(self): + self.create_group(20004) + self.upgrade() + self.assertEqual(self.fetch_gid_allocations(), [20000, 20001, 20002, 20003, 20004]) + + def test_gid_last(self): + self.create_group(20005) + self.upgrade() + self.assertEqual(self.fetch_gid_allocations(), [20000, 20001, 20002, 20003, 20004, 20005]) + + def test_gid_last_plus_one(self): + self.create_group(20006) + self.upgrade() + self.assertEqual(self.fetch_gid_allocations(), [20006]) + + def test_gid_complex(self): + self.create_group(10) + self.create_group(20001) + self.create_group(20003) + self.create_group(20010) + self.upgrade() + self.assertEqual(self.fetch_gid_allocations(), [10, 20000, 20001, 20002, 20003, 20010]) + + # The code for UIDs is mostly the same as for GIDs, so we don't test all + # the edge cases again. + def test_uid_different_ranges(self): + self.create_user(10) + self.create_user(10000) + self.create_user(10002) + self.create_user(10007) + self.create_user(10009) + self.create_user(90000) + self.upgrade() + self.assertEqual(self.fetch_uid_allocations(), [10, 10000, 10001, 10002, 10006, 10007, 10008, 10009, 90000]) + + def test_uid_same_ranges(self): + self.app.config['USER_MIN_UID'] = 10000 + self.app.config['USER_MAX_UID'] = 10010 + self.app.config['USER_SERVICE_MIN_UID'] = 10000 + self.app.config['USER_SERVICE_MAX_UID'] = 10010 + self.create_user(10) + self.create_user(10000) + self.create_user(10002) + self.create_user(10007) + self.create_user(10009) + self.create_user(90000) + self.upgrade() + self.assertEqual(self.fetch_uid_allocations(), [10, 10000, 10001, 10002, 10003, 10004, 10005, 10006, 10007, 10008, 10009, 90000]) diff --git a/tests/models/test_misc.py b/tests/models/test_misc.py index 4b208658b5aa81c56bef2b5ecf929699d033cee6..aebd62b3c190671fbbf9ed3f07c5f6679cfef1de 100644 --- a/tests/models/test_misc.py +++ b/tests/models/test_misc.py @@ -1,12 +1,15 @@ +import time +import threading + from sqlalchemy.exc import IntegrityError from uffd.database import db -from uffd.models import FeatureFlag +from uffd.models import FeatureFlag, Lock from uffd.models.misc import feature_flag_table -from tests.utils import UffdTestCase +from tests.utils import ModelTestCase -class TestFeatureFlagModel(UffdTestCase): +class TestFeatureFlag(ModelTestCase): def test_disabled(self): flag = FeatureFlag('foo') self.assertFalse(flag) @@ -53,3 +56,29 @@ class TestFeatureFlagModel(UffdTestCase): with self.assertRaises(IntegrityError): flag.enable() self.assertTrue(flag) + +class TestLock(ModelTestCase): + DISABLE_SQLITE_MEMORY_DB = True + + def setUpApp(self): + self.lock = Lock('testlock') + + def run_lock_test(self): + result = [] + def func(): + with self.app.test_request_context(): + self.lock.acquire() + result.append('bar') + t = threading.Thread(target=func) + t.start() + time.sleep(1) + result.append('foo') + time.sleep(1) + db.session.rollback() + t.join() + return result + + def test_lock2(self): + self.assertEqual(self.run_lock_test(), ['bar', 'foo']) + self.lock.acquire() + self.assertEqual(self.run_lock_test(), ['foo', 'bar']) diff --git a/tests/models/test_user.py b/tests/models/test_user.py index 6d81e9ec8608d44efaadde0c684f367c3ecb11e5..71416643bf965157257bec2bd738df0124392218 100644 --- a/tests/models/test_user.py +++ b/tests/models/test_user.py @@ -3,9 +3,9 @@ import datetime import sqlalchemy from uffd.database import db -from uffd.models import User, UserEmail, Group, FeatureFlag +from uffd.models import User, UserEmail, Group, FeatureFlag, IDAlreadyAllocatedError, IDRangeExhaustedError -from tests.utils import UffdTestCase +from tests.utils import UffdTestCase, ModelTestCase class TestUserModel(UffdTestCase): def test_has_permission(self): @@ -36,9 +36,9 @@ class TestUserModel(UffdTestCase): self.app.config['USER_MIN_UID'] = 10000 self.app.config['USER_MAX_UID'] = 18999 self.app.config['USER_SERVICE_MIN_UID'] = 19000 - self.app.config['USER_SERVICE_MAX_UID'] =19999 - User.query.delete() - db.session.commit() + self.app.config['USER_SERVICE_MAX_UID'] = 19999 + db.drop_all() + db.create_all() user0 = User(loginname='user0', displayname='user0', primary_email_address='user0@example.com') user1 = User(loginname='user1', displayname='user1', primary_email_address='user1@example.com') user2 = User(loginname='user2', displayname='user2', primary_email_address='user2@example.com') @@ -53,6 +53,12 @@ class TestUserModel(UffdTestCase): db.session.add(user3) db.session.commit() self.assertEqual(user3.unix_uid, 10003) + db.session.delete(user2) + db.session.commit() + user4 = User(loginname='user4', displayname='user4', primary_email_address='user4@example.com') + db.session.add(user4) + db.session.commit() + self.assertEqual(user4.unix_uid, 10004) service0 = User(loginname='service0', displayname='service0', primary_email_address='service0@example.com', is_service_user=True) service1 = User(loginname='service1', displayname='service1', primary_email_address='service1@example.com', is_service_user=True) db.session.add_all([service0, service1]) @@ -65,8 +71,8 @@ class TestUserModel(UffdTestCase): self.app.config['USER_MAX_UID'] = 19999 self.app.config['USER_SERVICE_MIN_UID'] = 10000 self.app.config['USER_SERVICE_MAX_UID'] = 19999 - User.query.delete() - db.session.commit() + db.drop_all() + db.create_all() user0 = User(loginname='user0', displayname='user0', primary_email_address='user0@example.com') service0 = User(loginname='service0', displayname='service0', primary_email_address='service0@example.com', is_service_user=True) user1 = User(loginname='user1', displayname='user1', primary_email_address='user1@example.com') @@ -79,15 +85,15 @@ class TestUserModel(UffdTestCase): def test_unix_uid_generation_overflow(self): self.app.config['USER_MIN_UID'] = 10000 self.app.config['USER_MAX_UID'] = 10001 - User.query.delete() - db.session.commit() + db.drop_all() + db.create_all() user0 = User(loginname='user0', displayname='user0', primary_email_address='user0@example.com') user1 = User(loginname='user1', displayname='user1', primary_email_address='user1@example.com') db.session.add_all([user0, user1]) db.session.commit() self.assertEqual(user0.unix_uid, 10000) self.assertEqual(user1.unix_uid, 10001) - with self.assertRaises(sqlalchemy.exc.IntegrityError): + with self.assertRaises(sqlalchemy.exc.StatementError): user2 = User(loginname='user2', displayname='user2', primary_email_address='user2@example.com') db.session.add(user2) db.session.commit() @@ -285,32 +291,105 @@ class TestUserEmailModel(UffdTestCase): self.assert_cannot_add_address(user=admin, address='bar@example.com', verified=True) self.assert_cannot_add_address(user=admin, address='BAR@example.com', verified=True) -class TestGroupModel(UffdTestCase): +class TestIDAllocator(ModelTestCase): + def allocate_gids(self, *gids): + for gid in gids: + Group.unix_gid_allocator.allocate(gid) + + def fetch_gid_allocations(self): + return [row[0] for row in db.session.execute( + db.select([Group.unix_gid_allocator.allocation_table]) + .order_by(Group.unix_gid_allocator.allocation_table.c.id) + ).fetchall()] + + def test_empty(self): + self.assertEqual(Group.unix_gid_allocator.auto(20000, 20005), 20000) + self.assertEqual(self.fetch_gid_allocations(), [20000]) + + def test_first(self): + self.allocate_gids(20000) + self.assertEqual(Group.unix_gid_allocator.auto(20000, 20005), 20001) + self.assertEqual(self.fetch_gid_allocations(), [20000, 20001]) + + def test_out_of_range_before(self): + self.allocate_gids(19998) + self.assertEqual(Group.unix_gid_allocator.auto(20000, 20005), 20000) + self.assertEqual(self.fetch_gid_allocations(), [19998, 20000]) + + def test_out_of_range_right_before(self): + self.allocate_gids(19999) + self.assertEqual(Group.unix_gid_allocator.auto(20000, 20005), 20000) + self.assertEqual(self.fetch_gid_allocations(), [19999, 20000]) + + def test_out_of_range_after(self): + self.allocate_gids(20006) + self.assertEqual(Group.unix_gid_allocator.auto(20000, 20005), 20000) + self.assertEqual(self.fetch_gid_allocations(), [20000, 20006]) + + def test_gap_at_beginning(self): + self.allocate_gids(20001) + self.assertEqual(Group.unix_gid_allocator.auto(20000, 20005), 20000) + self.assertEqual(self.fetch_gid_allocations(), [20000, 20001]) + + def test_multiple_gaps(self): + self.allocate_gids(20000, 20001, 20003, 20005) + self.assertEqual(Group.unix_gid_allocator.auto(20000, 20005), 20002) + self.assertEqual(self.fetch_gid_allocations(), [20000, 20001, 20002, 20003, 20005]) + self.assertEqual(Group.unix_gid_allocator.auto(20000, 20005), 20004) + self.assertEqual(self.fetch_gid_allocations(), [20000, 20001, 20002, 20003, 20004, 20005]) + + def test_last(self): + self.allocate_gids(20000, 20001, 20002, 20003, 20004) + self.assertEqual(Group.unix_gid_allocator.auto(20000, 20005), 20005) + self.assertEqual(self.fetch_gid_allocations(), [20000, 20001, 20002, 20003, 20004, 20005]) + + def test_overflow(self): + self.allocate_gids(20000, 20001, 20002, 20003, 20004, 20005) + with self.assertRaises(IDRangeExhaustedError): + Group.unix_gid_allocator.auto(20000, 20005) + self.assertEqual(self.fetch_gid_allocations(), [20000, 20001, 20002, 20003, 20004, 20005]) + + def test_conflict(self): + self.allocate_gids(20000) + with self.assertRaises(IDAlreadyAllocatedError): + self.allocate_gids(20000) + self.assertEqual(self.fetch_gid_allocations(), [20000]) + +class TestGroup(ModelTestCase): def test_unix_gid_generation(self): self.app.config['GROUP_MIN_GID'] = 20000 self.app.config['GROUP_MAX_GID'] = 49999 - Group.query.delete() - db.session.commit() group0 = Group(name='group0', description='group0') group1 = Group(name='group1', description='group1') group2 = Group(name='group2', description='group2') - db.session.add_all([group0, group1, group2]) + group3 = Group(name='group3', description='group3', unix_gid=20004) + db.session.add_all([group0, group1, group2, group3]) db.session.commit() self.assertEqual(group0.unix_gid, 20000) self.assertEqual(group1.unix_gid, 20001) self.assertEqual(group2.unix_gid, 20002) - db.session.delete(group1) + self.assertEqual(group3.unix_gid, 20004) + db.session.delete(group2) db.session.commit() - group3 = Group(name='group3', description='group3') - db.session.add(group3) + group4 = Group(name='group4', description='group4') + group5 = Group(name='group5', description='group5') + db.session.add_all([group4, group5]) db.session.commit() - self.assertEqual(group3.unix_gid, 20003) + self.assertEqual(group4.unix_gid, 20003) + self.assertEqual(group5.unix_gid, 20005) - def test_unix_gid_generation(self): + def test_unix_gid_generation_conflict(self): self.app.config['GROUP_MIN_GID'] = 20000 - self.app.config['GROUP_MAX_GID'] = 20001 - Group.query.delete() + self.app.config['GROUP_MAX_GID'] = 49999 + group0 = Group(name='group0', description='group0', unix_gid=20023) + db.session.add(group0) db.session.commit() + with self.assertRaises(IDAlreadyAllocatedError): + Group(name='group1', description='group1', unix_gid=20023) + + def test_unix_gid_generation_overflow(self): + self.app.config['GROUP_MIN_GID'] = 20000 + self.app.config['GROUP_MAX_GID'] = 20001 group0 = Group(name='group0', description='group0') group1 = Group(name='group1', description='group1') db.session.add_all([group0, group1]) @@ -318,7 +397,7 @@ class TestGroupModel(UffdTestCase): self.assertEqual(group0.unix_gid, 20000) self.assertEqual(group1.unix_gid, 20001) db.session.commit() - with self.assertRaises(sqlalchemy.exc.IntegrityError): + with self.assertRaises(sqlalchemy.exc.StatementError): group2 = Group(name='group2', description='group2') db.session.add(group2) db.session.commit() diff --git a/tests/utils.py b/tests/utils.py index afea2e0ff8841b16ac278a8caa7f8c3cb5201c16..55a6164e19d48039238febada65329d06fecc1fa 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -2,6 +2,7 @@ import os import unittest from flask import url_for +import flask_migrate from uffd import create_app, db from uffd.models import User, Group, Mail @@ -21,43 +22,10 @@ def db_flush(): db.session.rollback() db.session.expire_all() -class UffdTestCase(unittest.TestCase): - def get_user(self): - return User.query.filter_by(loginname='testuser').one_or_none() - - def get_admin(self): - return User.query.filter_by(loginname='testadmin').one_or_none() - - def get_admin_group(self): - return Group.query.filter_by(name='uffd_admin').one_or_none() - - def get_access_group(self): - return Group.query.filter_by(name='uffd_access').one_or_none() - - def get_users_group(self): - return Group.query.filter_by(name='users').one_or_none() - - def get_mail(self): - return Mail.query.filter_by(uid='test').one_or_none() - - def login_as(self, user, ref=None): - # It is currently not possible to login while already logged in as another - # user, so make sure that we are not logged in first - self.client.get(path=url_for('session.logout'), follow_redirects=True) - loginname = None - password = None - if user == 'user': - loginname = 'testuser' - password = 'userpassword' - elif user == 'admin': - loginname = 'testadmin' - password = 'adminpassword' - return self.client.post(path=url_for('session.login', ref=ref), - data={'loginname': loginname, 'password': password}, follow_redirects=True) +class AppTestCase(unittest.TestCase): + DISABLE_SQLITE_MEMORY_DB = False def setUp(self): - # It would be far better to create a minimal app here, but since the - # session module depends on almost everything else, that is not really feasable config = { 'TESTING': True, 'DEBUG': True, @@ -66,9 +34,66 @@ class UffdTestCase(unittest.TestCase): 'MAIL_SKIP_SEND': True, 'SELF_SIGNUP': True, } - + if self.DISABLE_SQLITE_MEMORY_DB: + try: + os.remove('/tmp/uffd-migration-test-db.sqlite3') + except FileNotFoundError: + pass + config['SQLALCHEMY_DATABASE_URI'] = 'sqlite:////tmp/uffd-migration-test-db.sqlite3' self.app = create_app(config) self.setUpApp() + + def setUpApp(self): + pass + + def tearDown(self): + if self.DISABLE_SQLITE_MEMORY_DB: + try: + os.remove('/tmp/uffd-migration-test-db.sqlite3') + except FileNotFoundError: + pass + +class MigrationTestCase(AppTestCase): + DISABLE_SQLITE_MEMORY_DB = True + + REVISION = None + + def setUp(self): + super().setUp() + self.request_context = self.app.test_request_context() + self.request_context.__enter__() + if self.REVISION: + flask_migrate.upgrade(revision=self.REVISION + '-1') + + def upgrade(self, revision='+1'): + db.session.commit() + flask_migrate.upgrade(revision=revision) + + def downgrade(self, revision='-1'): + db.session.commit() + flask_migrate.downgrade(revision=revision) + + def tearDown(self): + db.session.rollback() + self.request_context.__exit__(None, None, None) + super().tearDown() + +class ModelTestCase(AppTestCase): + def setUp(self): + super().setUp() + self.request_context = self.app.test_request_context() + self.request_context.__enter__() + db.create_all() + db.session.commit() + + def tearDown(self): + db.session.rollback() + self.request_context.__exit__(None, None, None) + super().tearDown() + +class UffdTestCase(AppTestCase): + def setUp(self): + super().setUp() self.client = self.app.test_client() self.client.__enter__() # Just do some request so that we can use url_for @@ -90,11 +115,42 @@ class UffdTestCase(unittest.TestCase): self.setUpDB() db.session.commit() - def setUpApp(self): - pass - def setUpDB(self): pass def tearDown(self): self.client.__exit__(None, None, None) + super().tearDown() + + def get_user(self): + return User.query.filter_by(loginname='testuser').one_or_none() + + def get_admin(self): + return User.query.filter_by(loginname='testadmin').one_or_none() + + def get_admin_group(self): + return Group.query.filter_by(name='uffd_admin').one_or_none() + + def get_access_group(self): + return Group.query.filter_by(name='uffd_access').one_or_none() + + def get_users_group(self): + return Group.query.filter_by(name='users').one_or_none() + + def get_mail(self): + return Mail.query.filter_by(uid='test').one_or_none() + + def login_as(self, user, ref=None): + # It is currently not possible to login while already logged in as another + # user, so make sure that we are not logged in first + self.client.get(path=url_for('session.logout'), follow_redirects=True) + loginname = None + password = None + if user == 'user': + loginname = 'testuser' + password = 'userpassword' + elif user == 'admin': + loginname = 'testadmin' + password = 'adminpassword' + return self.client.post(path=url_for('session.login', ref=ref), + data={'loginname': loginname, 'password': password}, follow_redirects=True) diff --git a/uffd/migrations/versions/aeb07202a6c8_locking_and_new_id_allocation.py b/uffd/migrations/versions/aeb07202a6c8_locking_and_new_id_allocation.py new file mode 100644 index 0000000000000000000000000000000000000000..e1fe39fef4ba90d7a03eb01ce4f0192f0cbcafdc --- /dev/null +++ b/uffd/migrations/versions/aeb07202a6c8_locking_and_new_id_allocation.py @@ -0,0 +1,158 @@ +"""Locking and new ID allocation + +Revision ID: aeb07202a6c8 +Revises: 468995a9c9ee +Create Date: 2022-10-30 13:24:39.864612 + +""" +from alembic import op +import sqlalchemy as sa +from flask import current_app + +# revision identifiers, used by Alembic. +revision = 'aeb07202a6c8' +down_revision = '468995a9c9ee' +branch_labels = None +depends_on = None + +def upgrade(): + conn = op.get_bind() + meta = sa.MetaData(bind=conn) + user_table = sa.Table('user', meta, + sa.Column('id', sa.Integer(), autoincrement=True, nullable=False), + sa.Column('unix_uid', sa.Integer(), nullable=False), + sa.Column('loginname', sa.String(length=32), nullable=False), + sa.Column('displayname', sa.String(length=128), nullable=False), + sa.Column('primary_email_id', sa.Integer(), nullable=False), + sa.Column('recovery_email_id', sa.Integer(), nullable=True), + sa.Column('pwhash', sa.Text(), nullable=True), + sa.Column('is_service_user', sa.Boolean(), nullable=False), + sa.ForeignKeyConstraint(['primary_email_id'], ['user_email.id'], name=op.f('fk_user_primary_email_id_user_email'), onupdate='CASCADE'), + sa.ForeignKeyConstraint(['recovery_email_id'], ['user_email.id'], name=op.f('fk_user_recovery_email_id_user_email'), onupdate='CASCADE', ondelete='SET NULL'), + sa.PrimaryKeyConstraint('id', name=op.f('pk_user')), + sa.UniqueConstraint('loginname', name=op.f('uq_user_loginname')), + sa.UniqueConstraint('unix_uid', name=op.f('uq_user_unix_uid')) + ) + group_table = sa.Table('group', meta, + sa.Column('id', sa.Integer(), autoincrement=True, nullable=False), + sa.Column('unix_gid', sa.Integer(), nullable=False), + sa.Column('name', sa.String(length=32), nullable=False), + sa.Column('description', sa.String(length=128), nullable=False), + sa.PrimaryKeyConstraint('id', name=op.f('pk_group')), + sa.UniqueConstraint('name', name=op.f('uq_group_name')), + sa.UniqueConstraint('unix_gid', name=op.f('uq_group_unix_gid')) + ) + + lock_table = op.create_table('lock', + sa.Column('name', sa.String(length=32), nullable=False), + sa.PrimaryKeyConstraint('name', name=op.f('pk_lock')) + ) + conn.execute(sa.insert(lock_table).values(name='uid_allocation')) + conn.execute(sa.insert(lock_table).values(name='gid_allocation')) + + uid_allocation_table = op.create_table('uid_allocation', + sa.Column('id', sa.Integer(), nullable=False), + sa.PrimaryKeyConstraint('id', name=op.f('pk_uid_allocation')) + ) + # Completely block range USER_MAX_UID to max UID currently in use (within + # the UID range) to account for users deleted in the past. + max_user_uid = conn.execute( + sa.select([sa.func.max(user_table.c.unix_uid)]) + .where(user_table.c.unix_uid <= current_app.config['USER_MAX_UID']) + ).scalar() or 0 + if max_user_uid: + for uid in range(current_app.config['USER_MIN_UID'], max_user_uid + 1): + conn.execute(sa.insert(uid_allocation_table).values(id=uid)) + max_service_uid = conn.execute( + sa.select([sa.func.max(user_table.c.unix_uid)]) + .where(user_table.c.unix_uid <= current_app.config['USER_SERVICE_MAX_UID']) + ).scalar() or 0 + if max_service_uid: + for uid in range(current_app.config['USER_SERVICE_MIN_UID'], max_service_uid + 1): + if uid < current_app.config['USER_MIN_UID'] or uid > max_user_uid: + conn.execute(sa.insert(uid_allocation_table).values(id=uid)) + # Also block all UIDs outside of both ranges that are in use + # (just to be sure, there should not be any) + conn.execute(sa.insert(uid_allocation_table).from_select(['id'], + sa.select([user_table.c.unix_uid]).where(sa.and_( + # Out of range for user + sa.or_( + user_table.c.unix_uid < current_app.config['USER_MIN_UID'], + user_table.c.unix_uid > current_app.config['USER_MAX_UID'] + ), + # and out of range for service user + sa.or_( + user_table.c.unix_uid < current_app.config['USER_SERVICE_MIN_UID'], + user_table.c.unix_uid > current_app.config['USER_SERVICE_MAX_UID'] + ), + )) + )) + # Normally we would pass copy_from=user_table, so we don't lose any metadata, + # but this somehow causes an AttributeError (Neither 'ColumnClause' object + # nor 'Comparator' object has an attribute 'copy'). Also, we don't seem to + # lose anything without it. + with op.batch_alter_table('user', schema=None) as batch_op: + batch_op.create_foreign_key(batch_op.f('fk_user_unix_uid_uid_allocation'), 'uid_allocation', ['unix_uid'], ['id']) + + gid_allocation_table = op.create_table('gid_allocation', + sa.Column('id', sa.Integer(), nullable=False), + sa.PrimaryKeyConstraint('id', name=op.f('pk_gid_allocation')) + ) + group_table = sa.table('group', sa.column('unix_gid')) + # Completely block range GROUP_MAX_GID to max GID currently in use (within + # the GID range) to account for groups deleted in the past. + max_group_gid = conn.execute( + sa.select([sa.func.max(group_table.c.unix_gid)]) + .where(group_table.c.unix_gid <= current_app.config['GROUP_MAX_GID']) + ).scalar() or 0 + if max_group_gid: + for gid in range(current_app.config['GROUP_MIN_GID'], max_group_gid + 1): + conn.execute(sa.insert(gid_allocation_table).values(id=gid)) + # Also block out-of-range GIDs + conn.execute(sa.insert(gid_allocation_table).from_select(['id'], + sa.select([group_table.c.unix_gid]).where( + sa.or_( + group_table.c.unix_gid < current_app.config['GROUP_MIN_GID'], + group_table.c.unix_gid > current_app.config['GROUP_MAX_GID'] + ) + ) + )) + # See comment on batch_alter_table above + with op.batch_alter_table('group', schema=None) as batch_op: + batch_op.create_foreign_key(batch_op.f('fk_group_unix_gid_gid_allocation'), 'gid_allocation', ['unix_gid'], ['id']) + +def downgrade(): + meta = sa.MetaData(bind=op.get_bind()) + user_table = sa.Table('user', meta, + sa.Column('id', sa.Integer(), autoincrement=True, nullable=False), + sa.Column('unix_uid', sa.Integer(), nullable=False), + sa.Column('loginname', sa.String(length=32), nullable=False), + sa.Column('displayname', sa.String(length=128), nullable=False), + sa.Column('primary_email_id', sa.Integer(), nullable=False), + sa.Column('recovery_email_id', sa.Integer(), nullable=True), + sa.Column('pwhash', sa.Text(), nullable=True), + sa.Column('is_service_user', sa.Boolean(), nullable=False), + sa.ForeignKeyConstraint(['primary_email_id'], ['user_email.id'], name=op.f('fk_user_primary_email_id_user_email'), onupdate='CASCADE'), + sa.ForeignKeyConstraint(['recovery_email_id'], ['user_email.id'], name=op.f('fk_user_recovery_email_id_user_email'), onupdate='CASCADE', ondelete='SET NULL'), + sa.ForeignKeyConstraint(['unix_uid'], ['uid_allocation.id'], name=op.f('fk_user_unix_uid_uid_allocation')), + sa.PrimaryKeyConstraint('id', name=op.f('pk_user')), + sa.UniqueConstraint('loginname', name=op.f('uq_user_loginname')), + sa.UniqueConstraint('unix_uid', name=op.f('uq_user_unix_uid')) + ) + group_table = sa.Table('group', meta, + sa.Column('id', sa.Integer(), autoincrement=True, nullable=False), + sa.Column('unix_gid', sa.Integer(), nullable=False), + sa.Column('name', sa.String(length=32), nullable=False), + sa.Column('description', sa.String(length=128), nullable=False), + sa.ForeignKeyConstraint(['unix_gid'], ['gid_allocation.id'], name=op.f('fk_group_unix_gid_gid_allocation')), + sa.PrimaryKeyConstraint('id', name=op.f('pk_group')), + sa.UniqueConstraint('name', name=op.f('uq_group_name')), + sa.UniqueConstraint('unix_gid', name=op.f('uq_group_unix_gid')) + ) + with op.batch_alter_table('group', copy_from=group_table) as batch_op: + batch_op.drop_constraint(batch_op.f('fk_group_unix_gid_gid_allocation'), type_='foreignkey') + with op.batch_alter_table('user', copy_from=user_table) as batch_op: + batch_op.drop_constraint(batch_op.f('fk_user_unix_uid_uid_allocation'), type_='foreignkey') + op.drop_table('gid_allocation') + op.drop_table('uid_allocation') + op.drop_table('lock') diff --git a/uffd/models/__init__.py b/uffd/models/__init__.py index ad41798244d47dc7cbc92271958a121c7eb450ea..5fe24173b3ce2f5782e9fa089c5d21b50533833a 100644 --- a/uffd/models/__init__.py +++ b/uffd/models/__init__.py @@ -8,9 +8,9 @@ from .selfservice import PasswordToken from .service import RemailerMode, Service, ServiceUser, get_services from .session import DeviceLoginType, DeviceLoginInitiation, DeviceLoginConfirmation from .signup import Signup -from .user import User, UserEmail, Group +from .user import User, UserEmail, Group, IDAllocator, IDRangeExhaustedError, IDAlreadyAllocatedError from .ratelimit import RatelimitEvent, Ratelimit, HostRatelimit, host_ratelimit, format_delay -from .misc import FeatureFlag +from .misc import FeatureFlag, Lock __all__ = [ 'APIClient', @@ -23,7 +23,7 @@ __all__ = [ 'RemailerMode', 'Service', 'ServiceUser', 'get_services', 'DeviceLoginType', 'DeviceLoginInitiation', 'DeviceLoginConfirmation', 'Signup', - 'User', 'UserEmail', 'Group', + 'User', 'UserEmail', 'Group', 'IDAllocator', 'IDRangeExhaustedError', 'IDAlreadyAllocatedError', 'RatelimitEvent', 'Ratelimit', 'HostRatelimit', 'host_ratelimit', 'format_delay', - 'FeatureFlag', + 'FeatureFlag', 'Lock', ] diff --git a/uffd/models/misc.py b/uffd/models/misc.py index 3fdf954ba4ef0b8cecb8a8cd3e695ebfcb9ecabc..3df18be4cbbcae4ea5f31bb98e97e40382e29184 100644 --- a/uffd/models/misc.py +++ b/uffd/models/misc.py @@ -39,3 +39,40 @@ class FeatureFlag: func() FeatureFlag.unique_email_addresses = FeatureFlag('unique-email-addresses') + +lock_table = db.Table('lock', + db.Column('name', db.String(32), primary_key=True), +) + +class Lock: + ALL_LOCKS = set() + + def __init__(self, name): + self.name = name + assert name not in self.ALL_LOCKS + self.ALL_LOCKS.add(name) + + def acquire(self): + '''Acquire the lock until the end of the current transaction + + Calling acquire while the specific lock is already held has no effect.''' + if db.engine.name == 'sqlite': + # SQLite does not support with_for_update, but we can lock the whole DB + # with any write operation. So we do a dummy update. + db.session.execute(db.update(lock_table).where(False).values(name=None)) + elif db.engine.name in ('mysql', 'mariadb'): + result = db.session.execute(db.select([lock_table.c.name]).where(lock_table.c.name == self.name).with_for_update()).scalar() + if result is not None: + return + # We add all lock rows with migrations so we should never end up here + raise Exception(f'Lock "{self.name}" is missing') + else: + raise NotImplementedError() + +# Only executed when lock_table is created with db.create/db.create_all (e.g. +# during testing). Otherwise the rows are inserted with migrations. +@db.event.listens_for(lock_table, 'after_create') # pylint: disable=no-member +def insert_lock_rows(target, connection, **kwargs): # pylint: disable=unused-argument + for name in Lock.ALL_LOCKS: + db.session.execute(db.insert(lock_table).values(name=name)) + db.session.commit() diff --git a/uffd/models/user.py b/uffd/models/user.py index 481cac3c94e9c93222b0ce4733178107c72eeb7f..e3fa2f6a9116622f74cb7356d87d9a6685de784c 100644 --- a/uffd/models/user.py +++ b/uffd/models/user.py @@ -13,13 +13,95 @@ from uffd.database import db from uffd.remailer import remailer from uffd.utils import token_urlfriendly from uffd.password_hash import PasswordHashAttribute, LowEntropyPasswordHash, HighEntropyPasswordHash -from .misc import FeatureFlag - -# pylint: disable=E1101 -user_groups = db.Table('user_groups', - Column('user_id', Integer(), ForeignKey('user.id', onupdate='CASCADE', ondelete='CASCADE'), primary_key=True), - Column('group_id', Integer(), ForeignKey('group.id', onupdate='CASCADE', ondelete='CASCADE'), primary_key=True) -) +from .misc import FeatureFlag, Lock + +class IDRangeExhaustedError(Exception): + pass + +class IDAlreadyAllocatedError(ValueError): + pass + +# Helper class for UID/GID allocation that prevents reuse even if +# users/groups are deleted. +# +# To keep track of formerly used UIDs/GIDs, they are always also added to +# uid/gid allocation tables. Rows in these tables are never deleted. +# User/group tables have foreign key constraints to ensure that there can +# only ever be three cases for a given ID: +# +# 1. The ID was never used (does not exist in either user/group or allocation +# table) +# 2. The ID was used, but the user/group was deleted (it does not exist in +# user/group table, but it exists in the allocation table) +# 3. The ID is in use (it exists in both the user/group and the allocation +# table) +# +# For auto-allocation, there are a few edge cases to consider: +# +# 1. GIDs can be chosen freely in the web interface, e.g. one could easily +# create a group with the last GID in range. +# 2. For UIDs there are two ranges (for regular users and for service users). +# The ranges may either be the same or they may be different but +# non-overlapping. +# 3. ID ranges can be changed (e.g. extended to either side if the old range +# is exhausted). Existing IDs should not change. +# +# The approach we use here is to always auto-allocate the first unused id +# in range. This approach handles the three edge cases well and even behaves +# sanely in unsupported configurations like different but overlapping UID +# ranges. +class IDAllocator: + # pylint completely fails to understand SQLAlchemy's query functions + # pylint: disable=no-member + def __init__(self, name): + self.name = name + self.lock = Lock(f'{name}_allocation') + self.allocation_table = db.Table(f'{name}_allocation', db.Column('id', db.Integer(), primary_key=True)) + + def allocate(self, id): + self.lock.acquire() + result = db.session.execute( + db.select([self.allocation_table.c.id]) + .where(self.allocation_table.c.id == id) + ).scalar() + if result is not None: + raise IDAlreadyAllocatedError(f'Cannot allocate {self.name}: {id} is in use or was used in the past') + db.session.execute(db.insert(self.allocation_table).values(id=id)) + + def auto(self, min_id, max_id): + '''Auto-allocate and return an unused id in range''' + self.lock.acquire() + # We cannot easily iterate through a large range of numbers with generic + # SQL statements looking for unused ids. So to find the first unused id in + # range, we look for the first used id in range that is followed by an + # unused id. This does not work if there are no used ids in range (returns + # NULL) or if min_id is unused (returns higher id while it should return + # min_id). To fix this we also check if min_id is used or not. + tmp = db.aliased(self.allocation_table) + first_unused_id = db.session.execute( + db.select([db.func.min(self.allocation_table.c.id + 1)]) + .where(self.allocation_table.c.id >= min_id) + .where(db.not_(db.exists().where(tmp.c.id == self.allocation_table.c.id + 1))) + ).scalar() + min_id_used = db.session.execute( + db.select([db.exists() + .where(self.allocation_table.c.id == min_id)]) + ).scalar() + if not min_id_used: + first_unused_id = min_id + if first_unused_id > max_id: + raise IDRangeExhaustedError(f'Cannot auto-allocate {self.name}: Range is exhausted') + db.session.execute(db.insert(self.allocation_table).values(id=first_unused_id)) + return first_unused_id + +def user_unix_uid_default(context): + if context.get_current_parameters()['is_service_user']: + min_uid = current_app.config['USER_SERVICE_MIN_UID'] + max_uid = current_app.config['USER_SERVICE_MAX_UID'] + else: + min_uid = current_app.config['USER_MIN_UID'] + max_uid = current_app.config['USER_MAX_UID'] + return User.unix_uid_allocator.auto(min_uid, max_uid) class User(db.Model): # Allows 8 to 256 ASCII letters (lower and upper case), digits, spaces and @@ -38,8 +120,16 @@ class User(db.Model): __tablename__ = 'user' id = Column(Integer(), primary_key=True, autoincrement=True) - # Default is set in event handler below - unix_uid = Column(Integer(), unique=True, nullable=False) + + unix_uid_allocator = IDAllocator('uid') + unix_uid = Column(Integer(), ForeignKey('uid_allocation.id'), unique=True, nullable=False, default=user_unix_uid_default) + + @validates('unix_uid') + def validate_unix_uid(self, key, value): # pylint: disable=unused-argument + if self.unix_uid != value and value is not None: + self.unix_uid_allocator.allocate(value) + return value + loginname = Column(String(32), unique=True, nullable=False) displayname = Column(String(128), nullable=False) @@ -261,7 +351,7 @@ class UserEmail(db.Model): return self.verification_expires < datetime.datetime.utcnow() def finish_verification(self, secret): - # pylint: disable=using-constant-test + # pylint: disable=using-constant-test,no-member if self.verification_expired: return False if not self.verification_secret.verify(secret): @@ -280,41 +370,28 @@ def enable_unique_email_addresses(): def disable_unique_email_addresses(): UserEmail.query.update({UserEmail.enable_strict_constraints: None}) -def next_id_expr(column, min_value, max_value): - # db.func.max(column) + 1: highest used value in range + 1, NULL if no values in range - # db.func.min(..., max_value): clip to range - # db.func.coalesce(..., min_value): if NULL use min_value - # if range is exhausted, evaluates to max_value that violates the UNIQUE constraint - return db.select([db.func.coalesce(db.func.min(db.func.max(column) + 1, max_value), min_value)])\ - .where(column >= min_value)\ - .where(column <= max_value) - -# Emulates the behaviour of Column.default. We cannot use a static SQL -# expression like we do for Group.unix_gid, because we need context -# information. We also cannot set Column.default to a callable, because -# SQLAlchemy always treats the return value as a literal value and does -# not allow SQL expressions. -@db.event.listens_for(User, 'before_insert') -def set_default_unix_uid(mapper, connect, target): - # pylint: disable=unused-argument - if target.unix_uid is not None: - return - if target.is_service_user: - min_uid = current_app.config['USER_SERVICE_MIN_UID'] - max_uid = current_app.config['USER_SERVICE_MAX_UID'] - else: - min_uid = current_app.config['USER_MIN_UID'] - max_uid = current_app.config['USER_MAX_UID'] - target.unix_uid = next_id_expr(User.unix_uid, min_uid, max_uid) +# pylint: disable=E1101 +user_groups = db.Table('user_groups', + Column('user_id', Integer(), ForeignKey('user.id', onupdate='CASCADE', ondelete='CASCADE'), primary_key=True), + Column('group_id', Integer(), ForeignKey('group.id', onupdate='CASCADE', ondelete='CASCADE'), primary_key=True) +) -group_table = db.table('group', db.column('unix_gid')) -min_gid = db.bindparam('min_gid', unique=True, callable_=lambda: current_app.config['GROUP_MIN_GID'], type_=db.Integer) -max_gid = db.bindparam('max_gid', unique=True, callable_=lambda: current_app.config['GROUP_MAX_GID'], type_=db.Integer) +def group_unix_gid_default(): + return Group.unix_gid_allocator.auto(current_app.config['GROUP_MIN_GID'], current_app.config['GROUP_MAX_GID']) class Group(db.Model): __tablename__ = 'group' id = Column(Integer(), primary_key=True, autoincrement=True) - unix_gid = Column(Integer(), unique=True, nullable=False, default=next_id_expr(group_table.c.unix_gid, min_gid, max_gid)) + + unix_gid_allocator = IDAllocator('gid') + unix_gid = Column(Integer(), ForeignKey('gid_allocation.id'), unique=True, nullable=False, default=group_unix_gid_default) + + @validates('unix_gid') + def validate_unix_gid(self, key, value): # pylint: disable=unused-argument + if self.unix_gid != value and value is not None: + self.unix_gid_allocator.allocate(value) + return value + name = Column(String(32), unique=True, nullable=False) description = Column(String(128), nullable=False, default='') members = relationship('User', secondary='user_groups', back_populates='groups') diff --git a/uffd/translations/de/LC_MESSAGES/messages.mo b/uffd/translations/de/LC_MESSAGES/messages.mo index e20677fe03a9318c965249d14437600f175bd9cd..f7010b8fa6548e32d0a8e5ebeb97cc081ca8d9c0 100644 Binary files a/uffd/translations/de/LC_MESSAGES/messages.mo and b/uffd/translations/de/LC_MESSAGES/messages.mo differ diff --git a/uffd/translations/de/LC_MESSAGES/messages.po b/uffd/translations/de/LC_MESSAGES/messages.po index 4088baadc127476198c3da5ea05c141de20348c6..52d52102f02e5d68cc23fe92e0f42937b072bc92 100644 --- a/uffd/translations/de/LC_MESSAGES/messages.po +++ b/uffd/translations/de/LC_MESSAGES/messages.po @@ -7,7 +7,7 @@ msgid "" msgstr "" "Project-Id-Version: PROJECT VERSION\n" "Report-Msgid-Bugs-To: EMAIL@ADDRESS\n" -"POT-Creation-Date: 2022-10-25 22:00+0200\n" +"POT-Creation-Date: 2022-11-01 00:38+0100\n" "PO-Revision-Date: 2021-05-25 21:18+0200\n" "Last-Translator: FULL NAME <EMAIL@ADDRESS>\n" "Language: de\n" @@ -98,7 +98,7 @@ msgstr "Falsches Passwort" msgid "Login name or e-mail address is already in use" msgstr "Der Anmeldename oder die E-Mail-Adresse wird bereits verwendet" -#: uffd/models/user.py:37 +#: uffd/models/user.py:46 #, python-format msgid "" "At least %(minlen)d and at most %(maxlen)d characters. Only letters, " @@ -1633,23 +1633,27 @@ msgstr "Resultierende Gruppen (wird nur aktualisiert beim Speichern)" msgid "Groups" msgstr "Gruppen" -#: uffd/views/group.py:41 +#: uffd/views/group.py:42 +msgid "GID is already in use or was used in the past" +msgstr "GID wird oder wurde bereits verwendet" + +#: uffd/views/group.py:45 msgid "Invalid name" msgstr "Ungültiger Name" -#: uffd/views/group.py:52 +#: uffd/views/group.py:56 msgid "Group with this name or id already exists" msgstr "Gruppe mit diesem Namen oder dieser ID existiert bereits" -#: uffd/views/group.py:57 +#: uffd/views/group.py:61 msgid "Group created" msgstr "Gruppe erstellt" -#: uffd/views/group.py:59 +#: uffd/views/group.py:63 msgid "Group updated" msgstr "Gruppe aktualisiert" -#: uffd/views/group.py:68 +#: uffd/views/group.py:72 msgid "Deleted group" msgstr "Gruppe gelöscht" diff --git a/uffd/views/group.py b/uffd/views/group.py index d70751ef58127036ea722ccbb96cc842bcb41665..ec75963285e595b140cca4325c7397e519334cff 100644 --- a/uffd/views/group.py +++ b/uffd/views/group.py @@ -36,7 +36,11 @@ def update(id=None): if id is None: group = Group() if request.form['unix_gid']: - group.unix_gid = int(request.form['unix_gid']) + try: + group.unix_gid = int(request.form['unix_gid']) + except ValueError: + flash(_('GID is already in use or was used in the past')) + return render_template('group/show.html', group=group), 400 if not group.set_name(request.form['name']): flash(_('Invalid name')) return render_template('group/show.html', group=group), 400