diff --git a/tests/commands/test_unique_email_addresses.py b/tests/commands/test_unique_email_addresses.py new file mode 100644 index 0000000000000000000000000000000000000000..0f7edcfae6363e9ec94e126b33952ff03b235012 --- /dev/null +++ b/tests/commands/test_unique_email_addresses.py @@ -0,0 +1,51 @@ +from uffd.database import db +from uffd.models import User, UserEmail, FeatureFlag + +from tests.utils import UffdTestCase + +class TestUniqueEmailAddressCommands(UffdTestCase): + def setUp(self): + super().setUp() + self.client.__exit__(None, None, None) + + def test_enable(self): + result = self.app.test_cli_runner().invoke(args=['unique-email-addresses', 'enable']) + self.assertEqual(result.exit_code, 0) + with self.app.test_request_context(): + self.assertTrue(FeatureFlag.unique_email_addresses) + + def test_enable_already_enabled(self): + with self.app.test_request_context(): + FeatureFlag.unique_email_addresses.enable() + db.session.commit() + result = self.app.test_cli_runner().invoke(args=['unique-email-addresses', 'enable']) + self.assertEqual(result.exit_code, 1) + + def test_enable_user_conflict(self): + with self.app.test_request_context(): + db.session.add(UserEmail(user=self.get_user(), address='foo@example.com')) + db.session.add(UserEmail(user=self.get_user(), address='FOO@example.com')) + db.session.commit() + result = self.app.test_cli_runner().invoke(args=['unique-email-addresses', 'enable']) + self.assertEqual(result.exit_code, 1) + + def test_enable_global_conflict(self): + with self.app.test_request_context(): + db.session.add(UserEmail(user=self.get_user(), address='foo@example.com', verified=True)) + db.session.add(UserEmail(user=self.get_admin(), address='FOO@example.com', verified=True)) + db.session.commit() + result = self.app.test_cli_runner().invoke(args=['unique-email-addresses', 'enable']) + self.assertEqual(result.exit_code, 1) + + def test_disable(self): + with self.app.test_request_context(): + FeatureFlag.unique_email_addresses.enable() + db.session.commit() + result = self.app.test_cli_runner().invoke(args=['unique-email-addresses', 'disable']) + self.assertEqual(result.exit_code, 0) + with self.app.test_request_context(): + self.assertFalse(FeatureFlag.unique_email_addresses) + + def test_disable_already_enabled(self): + result = self.app.test_cli_runner().invoke(args=['unique-email-addresses', 'disable']) + self.assertEqual(result.exit_code, 1) diff --git a/tests/commands/test_user.py b/tests/commands/test_user.py index bae05ebfaa09ffb1eeb2b59fdc3f0794fd6b2923..3612fc3ba5ec215443a4ca5de225129080cfa635 100644 --- a/tests/commands/test_user.py +++ b/tests/commands/test_user.py @@ -1,5 +1,5 @@ from uffd.database import db -from uffd.models import User, Group, Role, RoleGroup +from uffd.models import User, Group, Role, RoleGroup, FeatureFlag from tests.utils import UffdTestCase @@ -36,6 +36,16 @@ class TestUserCLI(UffdTestCase): self.assertEqual(result.exit_code, 1) result = self.app.test_cli_runner().invoke(args=['user', 'create', 'testuser', '--mail', 'foobar@example.com']) # conflicting name self.assertEqual(result.exit_code, 1) + + with self.app.test_request_context(): + FeatureFlag.unique_email_addresses.enable() + db.session.commit() + result = self.app.test_cli_runner().invoke(args=['user', 'create', 'newuser', '--mail', 'test@example.com']) # conflicting email address + self.assertEqual(result.exit_code, 1) + with self.app.test_request_context(): + FeatureFlag.unique_email_addresses.disable() + db.session.commit() + result = self.app.test_cli_runner().invoke(args=['user', 'create', 'newuser', '--mail', 'newmail@example.com', '--displayname', 'New Display Name', '--password', 'newpassword', '--add-role', 'admin']) self.assertEqual(result.exit_code, 0) @@ -53,6 +63,16 @@ class TestUserCLI(UffdTestCase): self.assertEqual(result.exit_code, 1) result = self.app.test_cli_runner().invoke(args=['user', 'update', 'testuser', '--mail', '']) # invalid mail self.assertEqual(result.exit_code, 1) + + with self.app.test_request_context(): + FeatureFlag.unique_email_addresses.enable() + db.session.commit() + result = self.app.test_cli_runner().invoke(args=['user', 'update', 'testuser', '--mail', 'admin@example.com']) # conflicting mail + self.assertEqual(result.exit_code, 1) + with self.app.test_request_context(): + FeatureFlag.unique_email_addresses.disable() + db.session.commit() + result = self.app.test_cli_runner().invoke(args=['user', 'update', 'testuser', '--password', '']) # invalid password self.assertEqual(result.exit_code, 1) result = self.app.test_cli_runner().invoke(args=['user', 'update', 'testuser', '--displayname', '']) # invalid display name diff --git a/tests/models/test_misc.py b/tests/models/test_misc.py new file mode 100644 index 0000000000000000000000000000000000000000..4b208658b5aa81c56bef2b5ecf929699d033cee6 --- /dev/null +++ b/tests/models/test_misc.py @@ -0,0 +1,55 @@ +from sqlalchemy.exc import IntegrityError + +from uffd.database import db +from uffd.models import FeatureFlag +from uffd.models.misc import feature_flag_table + +from tests.utils import UffdTestCase + +class TestFeatureFlagModel(UffdTestCase): + def test_disabled(self): + flag = FeatureFlag('foo') + self.assertFalse(flag) + self.assertFalse(db.session.execute(db.select([flag.expr])).scalar()) + + def test_enabled(self): + db.session.execute(db.insert(feature_flag_table).values(name='foo')) + flag = FeatureFlag('foo') + self.assertTrue(flag) + self.assertTrue(db.session.execute(db.select([flag.expr])).scalar()) + + def test_toggle(self): + flag = FeatureFlag('foo') + hooks_called = [] + + @flag.enable_hook + def enable_hook1(): + hooks_called.append('enable1') + + @flag.enable_hook + def enable_hook2(): + hooks_called.append('enable2') + + @flag.disable_hook + def disable_hook1(): + hooks_called.append('disable1') + + @flag.disable_hook + def disable_hook2(): + hooks_called.append('disable2') + + hooks_called.clear() + flag.enable() + self.assertTrue(flag) + self.assertEqual(hooks_called, ['enable1', 'enable2']) + hooks_called.clear() + flag.disable() + self.assertFalse(flag) + self.assertEqual(hooks_called, ['disable1', 'disable2']) + flag.disable() # does nothing + self.assertFalse(flag) + flag.enable() + self.assertTrue(flag) + with self.assertRaises(IntegrityError): + flag.enable() + self.assertTrue(flag) diff --git a/tests/models/test_signup.py b/tests/models/test_signup.py index cfed4fb23324c8071e91ffd5b459dba6bb5eadc7..acdeda15f6a0c9bd9e58b35d82823adb12f67470 100644 --- a/tests/models/test_signup.py +++ b/tests/models/test_signup.py @@ -1,7 +1,7 @@ import datetime from uffd.database import db -from uffd.models import Signup, User +from uffd.models import Signup, User, FeatureFlag from tests.utils import UffdTestCase, db_flush @@ -44,7 +44,7 @@ class TestSignupModel(UffdTestCase): self.assertEqual(signup.user_id, prev_id) def test_password(self): - signup = Signup(loginname='newuser', displayname='New User', mail='test@example.com') + signup = Signup(loginname='newuser', displayname='New User', mail='new@example.com') self.assertFalse(signup.password.verify('notsecret')) self.assertFalse(signup.password.verify('')) self.assertFalse(signup.password.verify('wrongpassword')) @@ -54,13 +54,13 @@ class TestSignupModel(UffdTestCase): def test_expired(self): # TODO: Find a better way to test this! - signup = Signup(loginname='newuser', displayname='New User', mail='test@example.com', password='notsecret') + signup = Signup(loginname='newuser', displayname='New User', mail='new@example.com', password='notsecret') self.assertFalse(signup.expired) signup.created = created=datetime.datetime.utcnow() - datetime.timedelta(hours=49) self.assertTrue(signup.expired) def test_completed(self): - signup = Signup(loginname='newuser', displayname='New User', mail='test@example.com', password='notsecret') + signup = Signup(loginname='newuser', displayname='New User', mail='new@example.com', password='notsecret') self.assertFalse(signup.completed) signup.finish('notsecret') db.session.commit() @@ -69,29 +69,29 @@ class TestSignupModel(UffdTestCase): self.assertTrue(signup.completed) def test_validate(self): - signup = Signup(loginname='newuser', displayname='New User', mail='test@example.com', password='notsecret') + signup = Signup(loginname='newuser', displayname='New User', mail='new@example.com', password='notsecret') self.assert_validate_valid(signup) self.assert_validate_valid(refetch_signup(signup)) def test_validate_completed(self): - signup = Signup(loginname='newuser', displayname='New User', mail='test@example.com', password='notsecret') + signup = Signup(loginname='newuser', displayname='New User', mail='new@example.com', password='notsecret') self.assert_finish_success(signup, 'notsecret') self.assert_validate_invalid(signup) self.assert_validate_invalid(refetch_signup(signup)) def test_validate_expired(self): - signup = Signup(loginname='newuser', displayname='New User', mail='test@example.com', + signup = Signup(loginname='newuser', displayname='New User', mail='new@example.com', password='notsecret', created=datetime.datetime.utcnow()-datetime.timedelta(hours=49)) self.assert_validate_invalid(signup) self.assert_validate_invalid(refetch_signup(signup)) def test_validate_loginname(self): - signup = Signup(loginname='', displayname='New User', mail='test@example.com', password='notsecret') + signup = Signup(loginname='', displayname='New User', mail='new@example.com', password='notsecret') self.assert_validate_invalid(signup) self.assert_validate_invalid(refetch_signup(signup)) def test_validate_displayname(self): - signup = Signup(loginname='newuser', displayname='', mail='test@example.com', password='notsecret') + signup = Signup(loginname='newuser', displayname='', mail='new@example.com', password='notsecret') self.assert_validate_invalid(signup) self.assert_validate_invalid(refetch_signup(signup)) @@ -101,52 +101,58 @@ class TestSignupModel(UffdTestCase): self.assert_validate_invalid(refetch_signup(signup)) def test_validate_password(self): - signup = Signup(loginname='newuser', displayname='New User', mail='test@example.com') + signup = Signup(loginname='newuser', displayname='New User', mail='new@example.com') self.assertFalse(signup.set_password('')) self.assert_validate_invalid(signup) self.assert_validate_invalid(refetch_signup(signup)) def test_validate_exists(self): - signup = Signup(loginname='testuser', displayname='New User', mail='test@example.com', password='notsecret') + signup = Signup(loginname='testuser', displayname='New User', mail='new@example.com', password='notsecret') self.assert_validate_invalid(signup) self.assert_validate_invalid(refetch_signup(signup)) def test_finish(self): - signup = Signup(loginname='newuser', displayname='New User', mail='test@example.com', password='notsecret') + signup = Signup(loginname='newuser', displayname='New User', mail='new@example.com', password='notsecret') self.assert_finish_success(signup, 'notsecret') user = User.query.filter_by(loginname='newuser').one_or_none() self.assertEqual(user.loginname, 'newuser') self.assertEqual(user.displayname, 'New User') - self.assertEqual(user.primary_email.address, 'test@example.com') + self.assertEqual(user.primary_email.address, 'new@example.com') def test_finish_completed(self): - signup = Signup(loginname='newuser', displayname='New User', mail='test@example.com', password='notsecret') + signup = Signup(loginname='newuser', displayname='New User', mail='new@example.com', password='notsecret') self.assert_finish_success(signup, 'notsecret') self.assert_finish_failure(refetch_signup(signup), 'notsecret') def test_finish_expired(self): # TODO: Find a better way to test this! - signup = Signup(loginname='newuser', displayname='New User', mail='test@example.com', + signup = Signup(loginname='newuser', displayname='New User', mail='new@example.com', password='notsecret', created=datetime.datetime.utcnow()-datetime.timedelta(hours=49)) self.assert_finish_failure(signup, 'notsecret') self.assert_finish_failure(refetch_signup(signup), 'notsecret') def test_finish_wrongpassword(self): - signup = Signup(loginname='newuser', displayname='New User', mail='test@example.com') + signup = Signup(loginname='newuser', displayname='New User', mail='new@example.com') self.assert_finish_failure(signup, '') self.assert_finish_failure(signup, 'wrongpassword') signup = refetch_signup(signup) self.assert_finish_failure(signup, '') self.assert_finish_failure(signup, 'wrongpassword') - signup = Signup(loginname='newuser', displayname='New User', mail='test@example.com', password='notsecret') + signup = Signup(loginname='newuser', displayname='New User', mail='new@example.com', password='notsecret') self.assert_finish_failure(signup, 'wrongpassword') self.assert_finish_failure(refetch_signup(signup), 'wrongpassword') def test_finish_duplicate(self): - signup = Signup(loginname='testuser', displayname='New User', mail='test@example.com', password='notsecret') + signup = Signup(loginname='testuser', displayname='New User', mail='new@example.com', password='notsecret') self.assert_finish_failure(signup, 'notsecret') self.assert_finish_failure(refetch_signup(signup), 'notsecret') + def test_finish_duplicate_email_strict_uniqueness(self): + FeatureFlag.unique_email_addresses.enable() + db.session.commit() + signup = Signup(loginname='newuser', displayname='New User', mail='test@example.com', password='notsecret') + self.assert_finish_failure(signup, 'notsecret') + def test_duplicate(self): signup = Signup(loginname='newuser', displayname='New User', mail='test1@example.com', password='notsecret') self.assert_validate_valid(signup) diff --git a/tests/models/test_user.py b/tests/models/test_user.py index 42fabfcf296b14422ff51a31f997a7daacc23a87..6d81e9ec8608d44efaadde0c684f367c3ecb11e5 100644 --- a/tests/models/test_user.py +++ b/tests/models/test_user.py @@ -3,7 +3,7 @@ import datetime import sqlalchemy from uffd.database import db -from uffd.models import User, UserEmail, Group +from uffd.models import User, UserEmail, Group, FeatureFlag from tests.utils import UffdTestCase @@ -119,6 +119,30 @@ class TestUserModel(UffdTestCase): self.assertEqual({user.all_emails[0].address, user.all_emails[1].address}, {'foobar@example.com', 'other@example.com'}) class TestUserEmailModel(UffdTestCase): + def test_normalize_address(self): + ref = UserEmail.normalize_address('foo@example.com') + self.assertEqual(ref, UserEmail.normalize_address('foo@example.com')) + self.assertEqual(ref, UserEmail.normalize_address('Foo@Example.Com')) + self.assertEqual(ref, UserEmail.normalize_address(' foo@example.com ')) + self.assertNotEqual(ref, UserEmail.normalize_address('bar@example.com')) + self.assertNotEqual(ref, UserEmail.normalize_address('foo @example.com')) + # "No-Break Space" instead of SPACE (Unicode normalization + stripping) + self.assertEqual(ref, UserEmail.normalize_address('\u00A0foo@example.com ')) + # Pre-composed "Angstrom Sign" vs. "A" + "Combining Ring Above" (Unicode normalization) + self.assertEqual(UserEmail.normalize_address('\u212B@example.com'), UserEmail.normalize_address('A\u030A@example.com')) + + def test_address(self): + email = UserEmail() + self.assertIsNone(email.address) + self.assertIsNone(email.address_normalized) + email.address = 'Foo@example.com' + self.assertEqual(email.address, 'Foo@example.com') + self.assertEqual(email.address_normalized, UserEmail.normalize_address('Foo@example.com')) + with self.assertRaises(ValueError): + email.address = 'bar@example.com' + with self.assertRaises(ValueError): + email.address = None + def test_set_address(self): email = UserEmail() self.assertFalse(email.set_address('invalid')) @@ -134,6 +158,23 @@ class TestUserEmailModel(UffdTestCase): self.assertTrue(email.set_address('foobar@example.com')) self.assertEqual(email.address, 'foobar@example.com') + def test_verified(self): + email = UserEmail(user=self.get_user(), address='foo@example.com') + db.session.add(email) + self.assertEqual(email.verified, False) + self.assertEqual(UserEmail.query.filter_by(address='foo@example.com', verified=True).count(), 0) + self.assertEqual(UserEmail.query.filter_by(address='foo@example.com', verified=False).count(), 1) + email.verified = True + self.assertEqual(email.verified, True) + self.assertEqual(UserEmail.query.filter_by(address='foo@example.com', verified=True).count(), 1) + self.assertEqual(UserEmail.query.filter_by(address='foo@example.com', verified=False).count(), 0) + with self.assertRaises(ValueError): + email.verified = False + self.assertEqual(email.verified, True) + with self.assertRaises(ValueError): + email.verified = None + self.assertEqual(email.verified, True) + def test_verification(self): email = UserEmail(address='foo@example.com') self.assertFalse(email.finish_verification('test')) @@ -150,6 +191,100 @@ class TestUserEmailModel(UffdTestCase): self.assertFalse(email.verification_secret) self.assertTrue(email.verification_expired) + def test_enable_strict_constraints(self): + email = UserEmail(address='foo@example.com', user=self.get_user()) + db.session.add(email) + db.session.commit() + self.assertIsNone(email.enable_strict_constraints) + FeatureFlag.unique_email_addresses.enable() + self.assertTrue(email.enable_strict_constraints) + FeatureFlag.unique_email_addresses.disable() + self.assertIsNone(email.enable_strict_constraints) + + def assert_can_add_address(self, **kwargs): + user_email = UserEmail(**kwargs) + db.session.add(user_email) + db.session.commit() + db.session.delete(user_email) + db.session.commit() + + def assert_cannot_add_address(self, **kwargs): + with self.assertRaises(sqlalchemy.exc.IntegrityError): + db.session.add(UserEmail(**kwargs)) + db.session.commit() + db.session.rollback() + + def test_unique_constraints_old(self): + # The same user cannot add the same exact address multiple times, but + # different users can have the same address + user = self.get_user() + admin = self.get_admin() + db.session.add(UserEmail(user=user, address='foo@example.com')) + db.session.add(UserEmail(user=user, address='bar@example.com', verified=True)) + db.session.commit() + + self.assert_can_add_address(user=user, address='foobar@example.com') + self.assert_can_add_address(user=user, address='foobar@example.com', verified=True) + + self.assert_cannot_add_address(user=user, address='foo@example.com') + self.assert_can_add_address(user=user, address='FOO@example.com') + self.assert_cannot_add_address(user=user, address='bar@example.com') + self.assert_can_add_address(user=user, address='BAR@example.com') + + self.assert_cannot_add_address(user=user, address='foo@example.com', verified=True) + self.assert_can_add_address(user=user, address='FOO@example.com', verified=True) + self.assert_cannot_add_address(user=user, address='bar@example.com', verified=True) + self.assert_can_add_address(user=user, address='BAR@example.com', verified=True) + + self.assert_can_add_address(user=admin, address='foobar@example.com') + self.assert_can_add_address(user=admin, address='foobar@example.com', verified=True) + + self.assert_can_add_address(user=admin, address='foo@example.com') + self.assert_can_add_address(user=admin, address='FOO@example.com') + self.assert_can_add_address(user=admin, address='bar@example.com') + self.assert_can_add_address(user=admin, address='BAR@example.com') + + self.assert_can_add_address(user=admin, address='foo@example.com', verified=True) + self.assert_can_add_address(user=admin, address='FOO@example.com', verified=True) + self.assert_can_add_address(user=admin, address='bar@example.com', verified=True) + self.assert_can_add_address(user=admin, address='BAR@example.com', verified=True) + + def test_unique_constraints_strict(self): + FeatureFlag.unique_email_addresses.enable() + # The same user cannot add the same (normalized) address multiple times, + # and different users cannot have the same verified (normalized) address + user = self.get_user() + admin = self.get_admin() + db.session.add(UserEmail(user=user, address='foo@example.com')) + db.session.add(UserEmail(user=user, address='bar@example.com', verified=True)) + db.session.commit() + + self.assert_can_add_address(user=user, address='foobar@example.com') + self.assert_can_add_address(user=user, address='foobar@example.com', verified=True) + + self.assert_cannot_add_address(user=user, address='foo@example.com') + self.assert_cannot_add_address(user=user, address='FOO@example.com') + self.assert_cannot_add_address(user=user, address='bar@example.com') + self.assert_cannot_add_address(user=user, address='BAR@example.com') + + self.assert_cannot_add_address(user=user, address='foo@example.com', verified=True) + self.assert_cannot_add_address(user=user, address='FOO@example.com', verified=True) + self.assert_cannot_add_address(user=user, address='bar@example.com', verified=True) + self.assert_cannot_add_address(user=user, address='BAR@example.com', verified=True) + + self.assert_can_add_address(user=admin, address='foobar@example.com') + self.assert_can_add_address(user=admin, address='foobar@example.com', verified=True) + + self.assert_can_add_address(user=admin, address='foo@example.com') + self.assert_can_add_address(user=admin, address='FOO@example.com') + self.assert_can_add_address(user=admin, address='bar@example.com') + self.assert_can_add_address(user=admin, address='BAR@example.com') + + self.assert_can_add_address(user=admin, address='foo@example.com', verified=True) + self.assert_can_add_address(user=admin, address='FOO@example.com', verified=True) + 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): def test_unix_gid_generation(self): self.app.config['GROUP_MIN_GID'] = 20000 diff --git a/tests/views/test_selfservice.py b/tests/views/test_selfservice.py index 8260c04c8db1bb5227543678db3de63f321a15e1..5bf3e8f46e1693520c6704dccba4f4910bc45639 100644 --- a/tests/views/test_selfservice.py +++ b/tests/views/test_selfservice.py @@ -4,7 +4,7 @@ import re from flask import url_for, request from uffd.database import db -from uffd.models import PasswordToken, UserEmail, Role, RoleGroup, Service, ServiceUser +from uffd.models import PasswordToken, UserEmail, Role, RoleGroup, Service, ServiceUser, FeatureFlag from tests.utils import dump, UffdTestCase @@ -114,7 +114,7 @@ class TestSelfservice(UffdTestCase): email.verification_expires = datetime.datetime.utcnow() - datetime.timedelta(days=1) db.session.add(email) db.session.commit() - r = self.client.get(path=url_for('selfservice.verify_email', email_id=email.id, secret='invalidsecret'), follow_redirects=True) + r = self.client.get(path=url_for('selfservice.verify_email', email_id=email.id, secret=secret), follow_redirects=True) dump('selfservice_verify_email_expired', r) self.assertFalse(email.verified) @@ -137,6 +137,20 @@ class TestSelfservice(UffdTestCase): self.assertTrue(email.verified) self.assertEqual(self.get_user().primary_email, email) + def test_verify_email_duplicate_strict_uniqueness(self): + FeatureFlag.unique_email_addresses.enable() + db.session.commit() + self.login_as('user') + email = UserEmail(user=self.get_user(), address='admin@example.com') + secret = email.start_verification() + db.session.add(email) + db.session.commit() + email_id = email.id + r = self.client.get(path=url_for('selfservice.verify_email', email_id=email.id, secret=secret), follow_redirects=True) + dump('selfservice_verify_email_duplicate_strict_uniqueness', r) + email = UserEmail.query.get(email_id) + self.assertFalse(email.verified) + def test_retry_email_verification(self): self.login_as('user') email = UserEmail(user=self.get_user(), address='new@example.com') diff --git a/tests/views/test_signup.py b/tests/views/test_signup.py index 8242481e284be17deae83bcc6ac21a19fe75dc26..72f80488fb5ad6b58662f330ab66ca8c8a331a40 100644 --- a/tests/views/test_signup.py +++ b/tests/views/test_signup.py @@ -3,7 +3,7 @@ import datetime from flask import url_for, request from uffd.database import db -from uffd.models import Signup, Role, RoleGroup +from uffd.models import Signup, Role, RoleGroup, FeatureFlag from uffd.views.session import login_get_user from tests.utils import dump, UffdTestCase, db_flush @@ -26,7 +26,7 @@ class TestSignupViews(UffdTestCase): self.assertEqual(r.status_code, 200) self.assertEqual(Signup.query.filter_by(loginname='newuser').all(), []) r = self.client.post(path=url_for('signup.signup_submit'), follow_redirects=True, - data={'loginname': 'newuser', 'displayname': 'New User', 'mail': 'test@example.com', + data={'loginname': 'newuser', 'displayname': 'New User', 'mail': 'new@example.com', 'password1': 'notsecret', 'password2': 'notsecret'}) dump('test_signup_submit', r) self.assertEqual(r.status_code, 200) @@ -36,7 +36,7 @@ class TestSignupViews(UffdTestCase): signup = signups[0] self.assertEqual(signup.loginname, 'newuser') self.assertEqual(signup.displayname, 'New User') - self.assertEqual(signup.mail, 'test@example.com') + self.assertEqual(signup.mail, 'new@example.com') self.assertIn(signup.token, str(self.app.last_mail.get_content())) self.assertTrue(signup.password.verify('notsecret')) self.assertTrue(signup.validate()[0]) @@ -48,7 +48,7 @@ class TestSignupViews(UffdTestCase): self.assertEqual(r.status_code, 200) self.assertEqual(Signup.query.filter_by(loginname='newuser').all(), []) r = self.client.post(path=url_for('signup.signup_submit'), follow_redirects=True, - data={'loginname': 'newuser', 'displayname': 'New User', 'mail': 'test@example.com', + data={'loginname': 'newuser', 'displayname': 'New User', 'mail': 'new@example.com', 'password1': 'notsecret', 'password2': 'notsecret'}) dump('test_signup_submit_disabled', r) self.assertEqual(r.status_code, 200) @@ -57,7 +57,7 @@ class TestSignupViews(UffdTestCase): def test_signup_wrongpassword(self): r = self.client.post(path=url_for('signup.signup_submit'), follow_redirects=True, - data={'loginname': 'newuser', 'displayname': 'New User', 'mail': 'test@example.com', + data={'loginname': 'newuser', 'displayname': 'New User', 'mail': 'new@example.com', 'password1': 'notsecret', 'password2': 'notthesame'}) dump('test_signup_wrongpassword', r) self.assertEqual(r.status_code, 200) @@ -65,7 +65,7 @@ class TestSignupViews(UffdTestCase): def test_signup_invalid(self): r = self.client.post(path=url_for('signup.signup_submit'), follow_redirects=True, - data={'loginname': '', 'displayname': 'New User', 'mail': 'test@example.com', + data={'loginname': '', 'displayname': 'New User', 'mail': 'new@example.com', 'password1': 'notsecret', 'password2': 'notsecret'}) dump('test_signup_invalid', r) self.assertEqual(r.status_code, 200) @@ -74,7 +74,7 @@ class TestSignupViews(UffdTestCase): def test_signup_mailerror(self): self.app.config['MAIL_SKIP_SEND'] = 'fail' r = self.client.post(path=url_for('signup.signup_submit'), follow_redirects=True, - data={'loginname': 'newuser', 'displayname': 'New User', 'mail': 'test@example.com', + data={'loginname': 'newuser', 'displayname': 'New User', 'mail': 'new@example.com', 'password1': 'notsecret', 'password2': 'notsecret'}) dump('test_signup_mailerror', r) self.assertEqual(r.status_code, 200) @@ -93,7 +93,7 @@ class TestSignupViews(UffdTestCase): self.assertEqual(r.status_code, 200) self.app.last_mail = None r = self.client.post(path=url_for('signup.signup_submit'), follow_redirects=True, - data={'loginname': 'newuser', 'displayname': 'New User', 'mail': 'test@example.com', + data={'loginname': 'newuser', 'displayname': 'New User', 'mail': 'new@example.com', 'password1': 'notsecret', 'password2': 'notsecret'}) dump('test_signup_hostlimit', r) self.assertEqual(r.status_code, 200) @@ -103,12 +103,12 @@ class TestSignupViews(UffdTestCase): def test_signup_maillimit(self): for i in range(3): r = self.client.post(path=url_for('signup.signup_submit'), follow_redirects=True, - data={'loginname': 'newuser%d'%i, 'displayname': 'New User', 'mail': 'test@example.com', + data={'loginname': 'newuser%d'%i, 'displayname': 'New User', 'mail': 'new@example.com', 'password1': 'notsecret', 'password2': 'notsecret'}) self.assertEqual(r.status_code, 200) self.app.last_mail = None r = self.client.post(path=url_for('signup.signup_submit'), follow_redirects=True, - data={'loginname': 'newuser', 'displayname': 'New User', 'mail': 'test@example.com', + data={'loginname': 'newuser', 'displayname': 'New User', 'mail': 'new@example.com', 'password1': 'notsecret', 'password2': 'notsecret'}) dump('test_signup_maillimit', r) self.assertEqual(r.status_code, 200) @@ -158,7 +158,7 @@ class TestSignupViews(UffdTestCase): db.session.add(baserole) baserole.groups[self.get_access_group()] = RoleGroup() db.session.commit() - signup = Signup(loginname='newuser', displayname='New User', mail='test@example.com', password='notsecret') + signup = Signup(loginname='newuser', displayname='New User', mail='new@example.com', password='notsecret') signup = refetch_signup(signup) self.assertFalse(signup.completed) self.assertIsNone(login_get_user('newuser', 'notsecret')) @@ -175,7 +175,7 @@ class TestSignupViews(UffdTestCase): self.assertTrue(signup.completed) self.assertEqual(signup.user.loginname, 'newuser') self.assertEqual(signup.user.displayname, 'New User') - self.assertEqual(signup.user.primary_email.address, 'test@example.com') + self.assertEqual(signup.user.primary_email.address, 'new@example.com') self.assertIsNotNone(login_get_user('newuser', 'notsecret')) def test_confirm_loggedin(self): @@ -184,7 +184,7 @@ class TestSignupViews(UffdTestCase): baserole.groups[self.get_access_group()] = RoleGroup() db.session.commit() self.login_as('user') - signup = Signup(loginname='newuser', displayname='New User', mail='test@example.com', password='notsecret') + signup = Signup(loginname='newuser', displayname='New User', mail='new@example.com', password='notsecret') signup = refetch_signup(signup) self.assertFalse(signup.completed) self.assertIsNotNone(request.user) @@ -207,7 +207,7 @@ class TestSignupViews(UffdTestCase): self.assertEqual(r.status_code, 200) def test_confirm_expired(self): - signup = Signup(loginname='newuser', displayname='New User', mail='test@example.com', password='notsecret') + signup = Signup(loginname='newuser', displayname='New User', mail='new@example.com', password='notsecret') signup.created = datetime.datetime.utcnow() - datetime.timedelta(hours=49) signup = refetch_signup(signup) r = self.client.get(path=url_for('signup.signup_confirm', signup_id=signup.id, token=signup.token), follow_redirects=True) @@ -218,7 +218,7 @@ class TestSignupViews(UffdTestCase): self.assertEqual(r.status_code, 200) def test_confirm_completed(self): - signup = Signup(loginname=self.get_user().loginname, displayname='New User', mail='test@example.com', password='notsecret') + signup = Signup(loginname=self.get_user().loginname, displayname='New User', mail='new@example.com', password='notsecret') signup.user = self.get_user() signup = refetch_signup(signup) self.assertTrue(signup.completed) @@ -230,7 +230,7 @@ class TestSignupViews(UffdTestCase): self.assertEqual(r.status_code, 200) def test_confirm_wrongpassword(self): - signup = Signup(loginname='newuser', displayname='New User', mail='test@example.com', password='notsecret') + signup = Signup(loginname='newuser', displayname='New User', mail='new@example.com', password='notsecret') signup = refetch_signup(signup) r = self.client.post(path=url_for('signup.signup_confirm_submit', signup_id=signup.id, token=signup.token), follow_redirects=True, data={'password': 'wrongpassword'}) dump('test_signup_confirm_wrongpassword', r) @@ -240,7 +240,7 @@ class TestSignupViews(UffdTestCase): def test_confirm_error(self): # finish returns None and error message (here: because the user already exists) - signup = Signup(loginname=self.get_user().loginname, displayname='New User', mail='test@example.com', password='notsecret') + signup = Signup(loginname=self.get_user().loginname, displayname='New User', mail='new@example.com', password='notsecret') signup = refetch_signup(signup) r = self.client.post(path=url_for('signup.signup_confirm_submit', signup_id=signup.id, token=signup.token), follow_redirects=True, data={'password': 'notsecret'}) dump('test_signup_confirm_error', r) @@ -248,13 +248,28 @@ class TestSignupViews(UffdTestCase): self.assertEqual(r.status_code, 200) self.assertFalse(signup.completed) + def test_confirm_error_email_uniqueness(self): + FeatureFlag.unique_email_addresses.enable() + db.session.commit() + # finish returns None and error message (here: because the email address already exists) + # This case is interesting, because the error also invalidates the ORM session + signup = Signup(loginname='newuser', displayname='New User', mail='test@example.com', password='notsecret') + db.session.add(signup) + db.session.commit() + signup_id = signup.id + r = self.client.post(path=url_for('signup.signup_confirm_submit', signup_id=signup.id, token=signup.token), follow_redirects=True, data={'password': 'notsecret'}) + dump('test_signup_confirm_error_email_uniqueness', r) + self.assertEqual(r.status_code, 200) + signup = Signup.query.get(signup_id) + self.assertFalse(signup.completed) + def test_confirm_hostlimit(self): for i in range(20): - signup = Signup(loginname='newuser', displayname='New User', mail='test@example.com', password='notsecret') + signup = Signup(loginname='newuser', displayname='New User', mail='new@example.com', password='notsecret') signup = refetch_signup(signup) r = self.client.post(path=url_for('signup.signup_confirm_submit', signup_id=signup.id, token=signup.token), follow_redirects=True, data={'password': 'wrongpassword%d'%i}) self.assertEqual(r.status_code, 200) - signup = Signup(loginname='newuser', displayname='New User', mail='test@example.com', password='notsecret') + signup = Signup(loginname='newuser', displayname='New User', mail='new@example.com', password='notsecret') signup = refetch_signup(signup) self.assertFalse(signup.completed) r = self.client.post(path=url_for('signup.signup_confirm_submit', signup_id=signup.id, token=signup.token), follow_redirects=True, data={'password': 'notsecret'}) @@ -263,7 +278,7 @@ class TestSignupViews(UffdTestCase): self.assertFalse(signup.completed) def test_confirm_confirmlimit(self): - signup = Signup(loginname='newuser', displayname='New User', mail='test@example.com', password='notsecret') + signup = Signup(loginname='newuser', displayname='New User', mail='new@example.com', password='notsecret') signup = refetch_signup(signup) self.assertFalse(signup.completed) for i in range(5): diff --git a/tests/views/test_user.py b/tests/views/test_user.py index 308167bfce88e01737d211aba18f311a58e10870..e51d305f0eafba27d790978c7f22a671b76d0741 100644 --- a/tests/views/test_user.py +++ b/tests/views/test_user.py @@ -1,13 +1,14 @@ from flask import url_for from uffd.database import db -from uffd.models import User, UserEmail, Group, Role, Service, ServiceUser +from uffd.models import User, UserEmail, Group, Role, Service, ServiceUser, FeatureFlag from tests.utils import dump, UffdTestCase class TestUserViews(UffdTestCase): def setUp(self): super().setUp() + self.app.last_mail = None self.login_as('admin') def test_index(self): @@ -27,7 +28,7 @@ class TestUserViews(UffdTestCase): dump('user_new', r) self.assertEqual(r.status_code, 200) self.assertIsNone(User.query.filter_by(loginname='newuser').one_or_none()) - r = self.client.post(path=url_for('user.update'), + r = self.client.post(path=url_for('user.create'), data={'loginname': 'newuser', 'email': 'newuser@example.com', 'displayname': 'New User', f'role-{role1_id}': '1', 'password': 'newpassword'}, follow_redirects=True) dump('user_new_submit', r) @@ -39,11 +40,12 @@ class TestUserViews(UffdTestCase): self.assertEqual(user_.loginname, 'newuser') self.assertEqual(user_.displayname, 'New User') self.assertEqual(user_.primary_email.address, 'newuser@example.com') + self.assertFalse(user_.password) self.assertGreaterEqual(user_.unix_uid, self.app.config['USER_MIN_UID']) self.assertLessEqual(user_.unix_uid, self.app.config['USER_MAX_UID']) role1 = Role(name='role1') self.assertEqual(roles, ['base', 'role1']) - # TODO: confirm Mail is send, login not yet possible + self.assertIsNotNone(self.app.last_mail) def test_new_service(self): db.session.add(Role(name='base', is_default=True)) @@ -57,7 +59,7 @@ class TestUserViews(UffdTestCase): dump('user_new_service', r) self.assertEqual(r.status_code, 200) self.assertIsNone(User.query.filter_by(loginname='newuser').one_or_none()) - r = self.client.post(path=url_for('user.update'), + r = self.client.post(path=url_for('user.create'), data={'loginname': 'newuser', 'email': 'newuser@example.com', 'displayname': 'New User', f'role-{role1_id}': '1', 'password': 'newpassword', 'serviceaccount': '1'}, follow_redirects=True) dump('user_new_submit', r) @@ -70,12 +72,13 @@ class TestUserViews(UffdTestCase): self.assertEqual(user.displayname, 'New User') self.assertEqual(user.primary_email.address, 'newuser@example.com') self.assertTrue(user.unix_uid) + self.assertFalse(user.password) role1 = Role(name='role1') self.assertEqual(roles, ['role1']) - # TODO: confirm Mail is send, login not yet possible + self.assertIsNone(self.app.last_mail) def test_new_invalid_loginname(self): - r = self.client.post(path=url_for('user.update'), + r = self.client.post(path=url_for('user.create'), data={'loginname': '!newuser', 'email': 'newuser@example.com', 'displayname': 'New User', 'password': 'newpassword'}, follow_redirects=True) dump('user_new_invalid_loginname', r) @@ -83,23 +86,42 @@ class TestUserViews(UffdTestCase): self.assertIsNone(User.query.filter_by(loginname='newuser').one_or_none()) def test_new_empty_loginname(self): - r = self.client.post(path=url_for('user.update'), + r = self.client.post(path=url_for('user.create'), data={'loginname': '', 'email': 'newuser@example.com', 'displayname': 'New User', 'password': 'newpassword'}, follow_redirects=True) dump('user_new_empty_loginname', r) self.assertEqual(r.status_code, 200) self.assertIsNone(User.query.filter_by(loginname='newuser').one_or_none()) + def test_new_conflicting_loginname(self): + self.assertEqual(User.query.filter_by(loginname='testuser').count(), 1) + r = self.client.post(path=url_for('user.create'), + data={'loginname': 'testuser', 'email': 'newuser@example.com', 'displayname': 'New User', + 'password': 'newpassword'}, follow_redirects=True) + dump('user_new_conflicting_loginname', r) + self.assertEqual(r.status_code, 200) + self.assertEqual(User.query.filter_by(loginname='testuser').count(), 1) + def test_new_empty_email(self): - r = self.client.post(path=url_for('user.update'), + r = self.client.post(path=url_for('user.create'), data={'loginname': 'newuser', 'email': '', 'displayname': 'New User', 'password': 'newpassword'}, follow_redirects=True) dump('user_new_empty_email', r) self.assertEqual(r.status_code, 200) self.assertIsNone(User.query.filter_by(loginname='newuser').one_or_none()) + def test_new_conflicting_email(self): + FeatureFlag.unique_email_addresses.enable() + db.session.commit() + r = self.client.post(path=url_for('user.create'), + data={'loginname': 'newuser', 'email': 'test@example.com', 'displayname': 'New User', + 'password': 'newpassword'}, follow_redirects=True) + dump('user_new_conflicting_email', r) + self.assertEqual(r.status_code, 200) + self.assertIsNone(User.query.filter_by(loginname='newuser').one_or_none()) + def test_new_invalid_display_name(self): - r = self.client.post(path=url_for('user.update'), + r = self.client.post(path=url_for('user.create'), data={'loginname': 'newuser', 'email': 'newuser@example.com', 'displayname': 'A'*200, 'password': 'newpassword'}, follow_redirects=True) dump('user_new_invalid_display_name', r) @@ -235,6 +257,78 @@ class TestUserViews(UffdTestCase): } ) + def test_update_email_conflict(self): + user = self.get_user() + user_id = user.id + email_id = user.primary_email.id + email_address = user.primary_email.address + r = self.client.post(path=url_for('user.update', id=user.id), + data={'loginname': 'testuser', + f'email-{email_id}-present': '1', + f'newemail-1-address': user.primary_email.address}, + follow_redirects=True) + dump('user_update_email_conflict', r) + self.assertEqual(r.status_code, 200) + self.assertEqual(UserEmail.query.filter_by(user_id=user_id).count(), 1) + + def test_update_email_strict_uniqueness(self): + FeatureFlag.unique_email_addresses.enable() + db.session.commit() + user = self.get_user() + email = UserEmail(user=user, address='foo@example.com') + service1 = Service(name='service1', enable_email_preferences=True) + service2 = Service(name='service2', enable_email_preferences=True) + db.session.add_all([service1, service2]) + db.session.commit() + email1_id = user.primary_email.id + email2_id = email.id + service1_id = service1.id + service2_id = service2.id + r = self.client.post(path=url_for('user.update', id=user.id), + data={'loginname': 'testuser', + f'email-{email1_id}-present': '1', + f'email-{email2_id}-present': '1', + f'email-{email2_id}-verified': '1', + f'newemail-1-address': 'new1@example.com', + f'newemail-2-address': 'new2@example.com', f'newemail-2-verified': '1', + 'primary_email': email2_id, 'recovery_email': email1_id, + f'service_{service1_id}_email': 'primary', + f'service_{service2_id}_email': email2_id, + 'displayname': 'Test User', 'password': ''}, + follow_redirects=True) + dump('user_update_email_strict_uniqueness', r) + self.assertEqual(r.status_code, 200) + user = self.get_user() + self.assertEqual(user.primary_email.id, email2_id) + self.assertEqual(user.recovery_email.id, email1_id) + self.assertEqual(ServiceUser.query.get((service1.id, user.id)).service_email, None) + self.assertEqual(ServiceUser.query.get((service2.id, user.id)).service_email.id, email2_id) + self.assertEqual( + {email.address: email.verified for email in user.all_emails}, + { + 'test@example.com': True, + 'foo@example.com': True, + 'new1@example.com': False, + 'new2@example.com': True, + } + ) + + def test_update_email_strict_uniqueness_conflict(self): + FeatureFlag.unique_email_addresses.enable() + db.session.commit() + user = self.get_user() + user_id = user.id + email_id = user.primary_email.id + email_address = user.primary_email.address + r = self.client.post(path=url_for('user.update', id=user.id), + data={'loginname': 'testuser', + f'email-{email_id}-present': '1', + f'newemail-1-address': user.primary_email.address}, + follow_redirects=True) + dump('user_update_email_strict_uniqueness_conflict', r) + self.assertEqual(r.status_code, 200) + self.assertEqual(UserEmail.query.filter_by(user_id=user_id).count(), 1) + def test_update_invalid_display_name(self): user_unupdated = self.get_user() email_id = str(user_unupdated.primary_email.id) diff --git a/uffd/commands/__init__.py b/uffd/commands/__init__.py index e3ad7436096922115fb7129f0684251e73ac5a74..4acc9607f3d0077a57f5b8cbb3ac4d323215a4b5 100644 --- a/uffd/commands/__init__.py +++ b/uffd/commands/__init__.py @@ -5,6 +5,7 @@ from .profile import profile_command from .gendevcert import gendevcert_command from .cleanup import cleanup_command from .roles_update_all import roles_update_all_command +from .unique_email_addresses import unique_email_addresses_command def init_app(app): app.cli.add_command(user_command) @@ -14,3 +15,4 @@ def init_app(app): app.cli.add_command(profile_command) app.cli.add_command(cleanup_command) app.cli.add_command(roles_update_all_command) + app.cli.add_command(unique_email_addresses_command) diff --git a/uffd/commands/group.py b/uffd/commands/group.py index a98e23b3f9bac00591cdb4d0c62e3e12a3b1584b..43a6519db92ec6ac58d9c6acd5c095522293ebfe 100644 --- a/uffd/commands/group.py +++ b/uffd/commands/group.py @@ -38,8 +38,9 @@ def create(name, description): try: db.session.add(group) db.session.commit() - except IntegrityError as ex: - raise click.ClickException(f'Group creation failed: {ex}') + except IntegrityError: + # pylint: disable=raise-missing-from + raise click.ClickException(f'A group with name "{name}" already exists') @group_command.command(help='Update group attributes') @click.argument('name') diff --git a/uffd/commands/role.py b/uffd/commands/role.py index 584aaa0d3ec8da7ad4459827ea29ff81e6849c4f..035bdf314f5104d045bb0c4a62136e42d9c7b64c 100644 --- a/uffd/commands/role.py +++ b/uffd/commands/role.py @@ -89,8 +89,9 @@ def create(name, description, default, moderator_group, add_group, add_role): db.session.add(role) role.update_member_groups() db.session.commit() - except IntegrityError as ex: - raise click.ClickException(f'Role creation failed: {ex}') + except IntegrityError: + # pylint: disable=raise-missing-from + raise click.ClickException(f'A role with name "{name}" already exists') @role_command.command(help='Update role attributes') @click.argument('name') diff --git a/uffd/commands/unique_email_addresses.py b/uffd/commands/unique_email_addresses.py new file mode 100644 index 0000000000000000000000000000000000000000..62ac096de430dacbca73f3497ba4ff27bfc34f88 --- /dev/null +++ b/uffd/commands/unique_email_addresses.py @@ -0,0 +1,67 @@ +import click +from flask.cli import with_appcontext +from sqlalchemy.exc import IntegrityError + +from uffd.database import db +from uffd.models import User, UserEmail, FeatureFlag + +# pylint completely fails to understand SQLAlchemy's query functions +# pylint: disable=no-member + +@click.group('unique-email-addresses', help='Enable/disable e-mail address uniqueness checks') +def unique_email_addresses_command(): + pass + +@unique_email_addresses_command.command('enable') +@with_appcontext +def enable_unique_email_addresses_command(): + if FeatureFlag.unique_email_addresses: + raise click.ClickException('Uniqueness checks for e-mail addresses are already enabled') + query = db.select([UserEmail.address_normalized, UserEmail.user_id])\ + .group_by(UserEmail.address_normalized, UserEmail.user_id)\ + .having(db.func.count(UserEmail.id.distinct()) > 1) + for address_normalized, user_id in db.session.execute(query).fetchall(): + user = User.query.get(user_id) + user_emails = UserEmail.query.filter_by(address_normalized=address_normalized, user_id=user_id) + click.echo(f'User "{user.loginname}" has the same e-mail address multiple times:', err=True) + for user_email in user_emails: + if user_email.verified: + click.echo(f'- {user_email.address}', err=True) + else: + click.echo(f'- {user_email.address} (unverified)', err=True) + click.echo() + query = db.select([UserEmail.address_normalized, UserEmail.address])\ + .where(UserEmail.verified)\ + .group_by(UserEmail.address_normalized)\ + .having(db.func.count(UserEmail.id.distinct()) > 1) + for address_normalized, address in db.session.execute(query).fetchall(): + click.echo(f'E-mail address "{address}" is used by multiple users:', err=True) + user_emails = UserEmail.query.filter_by(address_normalized=address_normalized, verified=True) + for user_email in user_emails: + if user_email.address != address: + click.echo(f'- {user_email.user.loginname} ({user_email.address})', err=True) + else: + click.echo(f'- {user_email.user.loginname}', err=True) + click.echo() + try: + FeatureFlag.unique_email_addresses.enable() + except IntegrityError: + # pylint: disable=raise-missing-from + raise click.ClickException('''Some existing e-mail addresses violate uniqueness checks + +You need to fix this manually in the admin interface. Then run this command +again to continue.''') + db.session.commit() + click.echo('Uniqueness checks for e-mail addresses enabled') + +@unique_email_addresses_command.command('disable') +@with_appcontext +def disable_unique_email_addresses_command(): + if not FeatureFlag.unique_email_addresses: + raise click.ClickException('Uniqueness checks for e-mail addresses are already disabled') + click.echo('''Please note that the option to disable email address uniqueness checks will +be remove in uffd v3. +''', err=True) + FeatureFlag.unique_email_addresses.disable() + db.session.commit() + click.echo('Uniqueness checks for e-mail addresses disabled') diff --git a/uffd/commands/user.py b/uffd/commands/user.py index cf3dd6e0be916548f3686c75be50073dca8bc767..2db24b3fe83cacda6207d4b538ff5f717561b88b 100644 --- a/uffd/commands/user.py +++ b/uffd/commands/user.py @@ -70,15 +70,15 @@ def create(loginname, mail, displayname, service, password, prompt_password, add if displayname is None: displayname = loginname user = User(is_service_user=service) - user = User(is_service_user=service) if not user.set_loginname(loginname, ignore_blocklist=True): raise click.ClickException('Invalid loginname') try: db.session.add(user) update_attrs(user, mail, displayname, password, prompt_password, add_role=add_role) db.session.commit() - except IntegrityError as ex: - raise click.ClickException(f'User creation failed: {ex}') + except IntegrityError: + # pylint: disable=raise-missing-from + raise click.ClickException('Login name or e-mail address is already in use') @user_command.command(help='Update user attributes and roles') @click.argument('loginname') @@ -94,8 +94,12 @@ def update(loginname, mail, displayname, password, prompt_password, clear_roles, user = User.query.filter_by(loginname=loginname).one_or_none() if user is None: raise click.ClickException(f'User {loginname} not found') - update_attrs(user, mail, displayname, password, prompt_password, clear_roles, add_role, remove_role) - db.session.commit() + try: + update_attrs(user, mail, displayname, password, prompt_password, clear_roles, add_role, remove_role) + db.session.commit() + except IntegrityError: + # pylint: disable=raise-missing-from + raise click.ClickException('E-mail address is already in use') @user_command.command(help='Delete user') @click.argument('loginname') diff --git a/uffd/migrations/versions/468995a9c9ee_unique_email_addresses.py b/uffd/migrations/versions/468995a9c9ee_unique_email_addresses.py new file mode 100644 index 0000000000000000000000000000000000000000..2c776195e443b307262c203f8ea94fee48ab05c9 --- /dev/null +++ b/uffd/migrations/versions/468995a9c9ee_unique_email_addresses.py @@ -0,0 +1,102 @@ +"""Unique email addresses + +Revision ID: 468995a9c9ee +Revises: 2b68f688bec1 +Create Date: 2022-10-21 01:25:01.469670 + +""" +import unicodedata + +from alembic import op +import sqlalchemy as sa + +revision = '468995a9c9ee' +down_revision = '2b68f688bec1' +branch_labels = None +depends_on = None + +def normalize_address(value): + return unicodedata.normalize('NFKC', value).lower().strip() + +def iter_rows_paged(table, pk='id', limit=1000): + conn = op.get_bind() + pk_column = getattr(table.c, pk) + last_pk = None + while True: + expr = table.select().order_by(pk_column).limit(limit) + if last_pk is not None: + expr = expr.where(pk_column > last_pk) + result = conn.execute(expr) + pk_index = list(result.keys()).index(pk) + rows = result.fetchall() + if not rows: + break + yield from rows + last_pk = rows[-1][pk_index] + +def upgrade(): + with op.batch_alter_table('user_email', schema=None) as batch_op: + batch_op.add_column(sa.Column('address_normalized', sa.String(length=128), nullable=True)) + batch_op.add_column(sa.Column('enable_strict_constraints', sa.Boolean(), nullable=True)) + batch_op.alter_column('verified', existing_type=sa.Boolean(), nullable=True) + meta = sa.MetaData(bind=op.get_bind()) + user_email_table = sa.Table('user_email', meta, + sa.Column('id', sa.Integer(), autoincrement=True, nullable=False), + sa.Column('user_id', sa.Integer(), nullable=True), + sa.Column('address', sa.String(length=128), nullable=False), + sa.Column('address_normalized', sa.String(length=128), nullable=True), + sa.Column('enable_strict_constraints', sa.Boolean(), nullable=True), + sa.Column('verified', sa.Boolean(), nullable=True), + sa.Column('verification_legacy_id', sa.Integer(), nullable=True), + sa.Column('verification_secret', sa.Text(), nullable=True), + sa.Column('verification_expires', sa.DateTime(), nullable=True), + sa.ForeignKeyConstraint(['user_id'], ['user.id'], name=op.f('fk_user_email_user_id_user'), onupdate='CASCADE', ondelete='CASCADE'), + sa.PrimaryKeyConstraint('id', name=op.f('pk_user_email')), + sa.UniqueConstraint('user_id', 'address', name='uq_user_email_user_id_address') + ) + for row in iter_rows_paged(user_email_table): + id = row[0] + address = row[2] + verified = row[5] + op.execute(user_email_table.update()\ + .where(user_email_table.c.id == id)\ + .values( + address_normalized=normalize_address(address), + verified=(True if verified else None) + ) + ) + with op.batch_alter_table('user_email', copy_from=user_email_table) as batch_op: + batch_op.alter_column('address_normalized', existing_type=sa.String(length=128), nullable=False) + batch_op.create_unique_constraint('uq_user_email_address_normalized_verified', ['address_normalized', 'verified', 'enable_strict_constraints']) + batch_op.create_unique_constraint('uq_user_email_user_id_address_normalized', ['user_id', 'address_normalized', 'enable_strict_constraints']) + op.create_table('feature_flag', + sa.Column('name', sa.String(32), nullable=False), + sa.PrimaryKeyConstraint('name', name=op.f('pk_feature_flag')), + ) + +def downgrade(): + meta = sa.MetaData(bind=op.get_bind()) + user_email_table = sa.Table('user_email', meta, + sa.Column('id', sa.Integer(), autoincrement=True, nullable=False), + sa.Column('user_id', sa.Integer(), nullable=True), + sa.Column('address', sa.String(length=128), nullable=False), + sa.Column('address_normalized', sa.String(length=128), nullable=False), + sa.Column('enable_strict_constraints', sa.Boolean(), nullable=True), + sa.Column('verified', sa.Boolean(), nullable=True), + sa.Column('verification_legacy_id', sa.Integer(), nullable=True), + sa.Column('verification_secret', sa.Text(), nullable=True), + sa.Column('verification_expires', sa.DateTime(), nullable=True), + sa.ForeignKeyConstraint(['user_id'], ['user.id'], name=op.f('fk_user_email_user_id_user'), onupdate='CASCADE', ondelete='CASCADE'), + sa.PrimaryKeyConstraint('id', name=op.f('pk_user_email')), + sa.UniqueConstraint('user_id', 'address', name='uq_user_email_user_id_address'), + sa.UniqueConstraint('address_normalized', 'verified', 'enable_strict_constraints', name='uq_user_email_address_normalized_verified'), + sa.UniqueConstraint('user_id', 'address_normalized', 'enable_strict_constraints', name='uq_user_email_user_id_address_normalized') + ) + op.execute(user_email_table.update().where(user_email_table.c.verified == None).values(verified=False)) + with op.batch_alter_table('user_email', copy_from=user_email_table) as batch_op: + batch_op.drop_constraint('uq_user_email_user_id_address_normalized', type_='unique') + batch_op.drop_constraint('uq_user_email_address_normalized_verified', type_='unique') + batch_op.alter_column('verified', existing_type=sa.Boolean(), nullable=False) + batch_op.drop_column('enable_strict_constraints') + batch_op.drop_column('address_normalized') + op.drop_table('feature_flag') diff --git a/uffd/models/__init__.py b/uffd/models/__init__.py index 1644e4729cf9bc714d733c93ed870b7b08f06e60..ad41798244d47dc7cbc92271958a121c7eb450ea 100644 --- a/uffd/models/__init__.py +++ b/uffd/models/__init__.py @@ -10,6 +10,7 @@ from .session import DeviceLoginType, DeviceLoginInitiation, DeviceLoginConfirma from .signup import Signup from .user import User, UserEmail, Group from .ratelimit import RatelimitEvent, Ratelimit, HostRatelimit, host_ratelimit, format_delay +from .misc import FeatureFlag __all__ = [ 'APIClient', @@ -24,4 +25,5 @@ __all__ = [ 'Signup', 'User', 'UserEmail', 'Group', 'RatelimitEvent', 'Ratelimit', 'HostRatelimit', 'host_ratelimit', 'format_delay', + 'FeatureFlag', ] diff --git a/uffd/models/misc.py b/uffd/models/misc.py new file mode 100644 index 0000000000000000000000000000000000000000..3fdf954ba4ef0b8cecb8a8cd3e695ebfcb9ecabc --- /dev/null +++ b/uffd/models/misc.py @@ -0,0 +1,41 @@ +from uffd.database import db + +# pylint completely fails to understand SQLAlchemy's query functions +# pylint: disable=no-member + +feature_flag_table = db.Table('feature_flag', + db.Column('name', db.String(32), primary_key=True), +) + +class FeatureFlag: + def __init__(self, name): + self.name = name + self.enable_hooks = [] + self.disable_hooks = [] + + @property + def expr(self): + return db.exists().where(feature_flag_table.c.name == self.name) + + def __bool__(self): + return db.session.execute(db.select([self.expr])).scalar() + + def enable_hook(self, func): + self.enable_hooks.append(func) + return func + + def enable(self): + db.session.execute(db.insert(feature_flag_table).values(name=self.name)) + for func in self.enable_hooks: + func() + + def disable_hook(self, func): + self.disable_hooks.append(func) + return func + + def disable(self): + db.session.execute(db.delete(feature_flag_table).where(feature_flag_table.c.name == self.name)) + for func in self.disable_hooks: + func() + +FeatureFlag.unique_email_addresses = FeatureFlag('unique-email-addresses') diff --git a/uffd/models/signup.py b/uffd/models/signup.py index e56f565e5120595b98c48cf57ab299e6f06884cd..2df2fba002c35cee219fcedaa18c23aa8c4441b7 100644 --- a/uffd/models/signup.py +++ b/uffd/models/signup.py @@ -2,6 +2,7 @@ import datetime from flask_babel import gettext as _ from sqlalchemy import Column, Integer, String, DateTime, Text, ForeignKey +from sqlalchemy.exc import IntegrityError from sqlalchemy.orm import relationship, backref from sqlalchemy.ext.hybrid import hybrid_property @@ -104,8 +105,14 @@ class Signup(db.Model): return None, _('Wrong password') if User.query.filter_by(loginname=self.loginname).all(): return None, _('A user with this login name already exists') + # Flush to make sure the flush below does not catch unrelated errors + db.session.flush() user = User(loginname=self.loginname, displayname=self.displayname, primary_email_address=self.mail, password=self.password) db.session.add(user) + try: + db.session.flush() + except IntegrityError: + return None, _('Login name or e-mail address is already in use') user.update_groups() # pylint: disable=no-member self.user = user self.loginname = None diff --git a/uffd/models/user.py b/uffd/models/user.py index 374afef6fd1d881673bcdfc47de56adfab045972..481cac3c94e9c93222b0ce4733178107c72eeb7f 100644 --- a/uffd/models/user.py +++ b/uffd/models/user.py @@ -1,6 +1,7 @@ import string import re import datetime +import unicodedata from flask import current_app, escape from flask_babel import lazy_gettext @@ -12,6 +13,7 @@ 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', @@ -171,29 +173,67 @@ class UserEmail(db.Model): raise ValueError('UserEmail.user cannot be changed once set') return value + @classmethod + def normalize_address(cls, value): + return unicodedata.normalize('NFKC', value).lower().strip() + address = Column(String(128), nullable=False) + address_normalized = Column(String(128), nullable=False) @validates('address') def validate_address(self, key, value): # pylint: disable=unused-argument if self.address is not None and self.address != value: raise ValueError('UserEmail.address cannot be changed once set') + self.address_normalized = self.normalize_address(value) return value - verified = Column(Boolean(), default=False, nullable=False) + # True or None/NULL (not False, see constraints below) + _verified = Column('verified', Boolean(), nullable=True) + + @hybrid_property + def verified(self): + # pylint: disable=singleton-comparison + return self._verified != None - @validates('verified') - def validate_verified(self, key, value): # pylint: disable=unused-argument - if self.verified and not value: + @verified.setter + def verified(self, value): + if self._verified and not value: raise ValueError('UserEmail cannot be unverified once verified') - return value + self._verified = True if value else None verification_legacy_id = Column(Integer()) # id of old MailToken _verification_secret = Column('verification_secret', Text()) verification_secret = PasswordHashAttribute('_verification_secret', HighEntropyPasswordHash) verification_expires = Column(DateTime) + # Until uffd v3, we make the stricter unique constraints optional, by having + # enable_strict_constraints act as a switch to enable/disable the constraints + # on a per-row basis. + # True or None/NULL if disabled (not False, see constraints below) + enable_strict_constraints = Column( + Boolean(), + nullable=True, + default=db.select([db.case([(FeatureFlag.unique_email_addresses.expr, True)], else_=None)]) + ) + + # The unique constraints rely on the common interpretation of SQL92, that if + # any column in a unique constraint is NULL, the unique constraint essentially + # does not apply to the row. This is how SQLite, MySQL/MariaDB, PostgreSQL and + # other common databases behave. A few others like Microsoft SQL Server do not + # follow this, but we don't support them anyway. __table_args__ = ( - db.UniqueConstraint('user_id', 'address', name='uq_user_email_user_id_address'), + # A user cannot have the same address more than once, regardless of verification status + db.UniqueConstraint('user_id', 'address', name='uq_user_email_user_id_address'), # Legacy, to be removed in v3 + # Same unique constraint as uq_user_email_user_id_address, but with + # address_normalized instead of address. Only active if + # enable_strict_constraints is not NULL. + db.UniqueConstraint('user_id', 'address_normalized', 'enable_strict_constraints', + name='uq_user_email_user_id_address_normalized'), + # The same verified address can only exist once. Only active if + # enable_strict_constraints is not NULL. Unverified addresses are ignored, + # since verified is NULL in that case. + db.UniqueConstraint('address_normalized', 'verified', 'enable_strict_constraints', + name='uq_user_email_address_normalized_verified'), ) def set_address(self, value): @@ -232,6 +272,14 @@ class UserEmail(db.Model): self.verified = True return True +@FeatureFlag.unique_email_addresses.enable_hook +def enable_unique_email_addresses(): + UserEmail.query.update({UserEmail.enable_strict_constraints: True}) + +@FeatureFlag.unique_email_addresses.disable_hook +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 diff --git a/uffd/templates/user/show.html b/uffd/templates/user/show.html index 30a817388a63edd0bd32143c1eae0090ef5612c3..bc22d8e54c5d83242b39e0ec8c7a602fd1096dd0 100644 --- a/uffd/templates/user/show.html +++ b/uffd/templates/user/show.html @@ -16,7 +16,11 @@ {% endmacro %} {% block body %} +{% if user.id %} <form action="{{ url_for("user.update", id=user.id) }}" method="POST"> +{% else %} +<form action="{{ url_for("user.create") }}" method="POST"> +{% endif %} <div class="align-self-center"> <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> diff --git a/uffd/translations/de/LC_MESSAGES/messages.mo b/uffd/translations/de/LC_MESSAGES/messages.mo index 52af6cfad16ce638fbfb8e1ea2d018fd317566d8..e20677fe03a9318c965249d14437600f175bd9cd 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 39b097466cd41300c3ed2fb86a27b46082cbd8dc..4088baadc127476198c3da5ea05c141de20348c6 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-20 17:36+0200\n" +"POT-Creation-Date: 2022-10-25 22:00+0200\n" "PO-Revision-Date: 2021-05-25 21:18+0200\n" "Last-Translator: FULL NAME <EMAIL@ADDRESS>\n" "Language: de\n" @@ -30,7 +30,7 @@ msgstr "Einladungslink weist keine Rollen zu" msgid "Invite link does not grant any new roles" msgstr "Einladungslink weist keine neuen Rollen zu" -#: uffd/models/invite.py:91 uffd/models/signup.py:115 +#: uffd/models/invite.py:91 uffd/models/signup.py:122 #: uffd/templates/mfa/setup.html:225 msgid "Success" msgstr "Erfolgreich" @@ -61,40 +61,44 @@ msgstr "eine Stunde" msgid "%(hours)d hours" msgstr "%(hours)d Stunden" -#: uffd/models/signup.py:77 uffd/models/signup.py:102 +#: uffd/models/signup.py:78 uffd/models/signup.py:103 msgid "Invalid signup request" msgstr "Ungültiger Account-Registrierungs-Link" -#: uffd/models/signup.py:79 +#: uffd/models/signup.py:80 msgid "Login name is invalid" msgstr "Anmeldename ist ungültig" -#: uffd/models/signup.py:81 +#: uffd/models/signup.py:82 msgid "Display name is invalid" msgstr "Anzeigename ist ungültig" -#: uffd/models/signup.py:83 uffd/views/selfservice.py:112 uffd/views/user.py:54 -#: uffd/views/user.py:73 +#: uffd/models/signup.py:84 uffd/views/selfservice.py:112 uffd/views/user.py:51 +#: uffd/views/user.py:99 msgid "E-Mail address is invalid" msgstr "Ungültige E-Mail-Adresse" -#: uffd/models/signup.py:85 uffd/views/selfservice.py:49 +#: uffd/models/signup.py:86 uffd/views/selfservice.py:49 msgid "Invalid password" msgstr "Passwort ungültig" -#: uffd/models/signup.py:87 uffd/models/signup.py:106 +#: uffd/models/signup.py:88 uffd/models/signup.py:107 msgid "A user with this login name already exists" msgstr "Ein Account mit diesem Anmeldenamen existiert bereits" -#: uffd/models/signup.py:88 +#: uffd/models/signup.py:89 msgid "Valid" msgstr "Gültig" -#: uffd/models/signup.py:104 uffd/views/signup.py:104 +#: uffd/models/signup.py:105 uffd/views/signup.py:104 msgid "Wrong password" msgstr "Falsches Passwort" -#: uffd/models/user.py:35 +#: uffd/models/signup.py:115 uffd/views/user.py:62 +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 #, python-format msgid "" "At least %(minlen)d and at most %(maxlen)d characters. Only letters, " @@ -140,8 +144,8 @@ msgstr "Über uffd" #: uffd/templates/group/list.html:8 uffd/templates/invite/list.html:6 #: uffd/templates/mail/list.html:8 uffd/templates/role/list.html:8 -#: uffd/templates/service/index.html:8 uffd/templates/service/show.html:73 -#: uffd/templates/service/show.html:101 uffd/templates/user/list.html:8 +#: uffd/templates/service/index.html:8 uffd/templates/service/show.html:75 +#: uffd/templates/service/show.html:103 uffd/templates/user/list.html:8 msgid "New" msgstr "Neu" @@ -158,8 +162,8 @@ msgstr "GID" #: uffd/templates/rolemod/list.html:9 uffd/templates/rolemod/show.html:44 #: uffd/templates/selfservice/self.html:189 #: uffd/templates/service/index.html:14 uffd/templates/service/show.html:20 -#: uffd/templates/service/show.html:107 uffd/templates/user/show.html:189 -#: uffd/templates/user/show.html:221 +#: uffd/templates/service/show.html:109 uffd/templates/user/show.html:193 +#: uffd/templates/user/show.html:225 msgid "Name" msgstr "Name" @@ -167,14 +171,14 @@ msgstr "Name" #: uffd/templates/invite/new.html:36 uffd/templates/role/list.html:15 #: uffd/templates/role/show.html:48 uffd/templates/rolemod/list.html:10 #: uffd/templates/rolemod/show.html:26 uffd/templates/selfservice/self.html:190 -#: uffd/templates/user/show.html:190 uffd/templates/user/show.html:222 +#: uffd/templates/user/show.html:194 uffd/templates/user/show.html:226 msgid "Description" msgstr "Beschreibung" #: uffd/templates/group/show.html:8 uffd/templates/mail/show.html:27 #: uffd/templates/role/show.html:13 uffd/templates/rolemod/show.html:8 #: uffd/templates/service/api.html:15 uffd/templates/service/oauth2.html:15 -#: uffd/templates/service/show.html:16 uffd/templates/user/show.html:22 +#: uffd/templates/service/show.html:16 uffd/templates/user/show.html:26 msgid "Save" msgstr "Speichern" @@ -185,7 +189,7 @@ msgstr "Speichern" #: uffd/templates/service/show.html:10 #: uffd/templates/session/deviceauth.html:39 #: uffd/templates/session/deviceauth.html:49 -#: uffd/templates/session/devicelogin.html:29 uffd/templates/user/show.html:23 +#: uffd/templates/session/devicelogin.html:29 uffd/templates/user/show.html:27 msgid "Cancel" msgstr "Abbrechen" @@ -193,7 +197,7 @@ msgstr "Abbrechen" #: uffd/templates/role/show.html:21 uffd/templates/selfservice/self.html:61 #: uffd/templates/selfservice/self.html:204 uffd/templates/service/api.html:11 #: uffd/templates/service/oauth2.html:11 uffd/templates/service/show.html:12 -#: uffd/templates/user/show.html:25 uffd/templates/user/show.html:177 +#: uffd/templates/user/show.html:29 uffd/templates/user/show.html:181 msgid "Are you sure?" msgstr "Wirklich fortfahren?" @@ -203,8 +207,8 @@ msgstr "Wirklich fortfahren?" #: uffd/templates/role/show.html:21 uffd/templates/role/show.html:24 #: uffd/templates/selfservice/self.html:62 uffd/templates/service/api.html:12 #: uffd/templates/service/oauth2.html:12 uffd/templates/service/show.html:13 -#: uffd/templates/user/show.html:25 uffd/templates/user/show.html:27 -#: uffd/templates/user/show.html:100 +#: uffd/templates/user/show.html:29 uffd/templates/user/show.html:31 +#: uffd/templates/user/show.html:104 msgid "Delete" msgstr "Löschen" @@ -234,7 +238,7 @@ msgid "Created by" msgstr "Erstellt durch" #: uffd/templates/invite/list.html:14 uffd/templates/service/api.html:34 -#: uffd/templates/service/show.html:108 +#: uffd/templates/service/show.html:110 msgid "Permissions" msgstr "Berechtigungen" @@ -262,7 +266,7 @@ msgstr "Account-Registrierung" msgid "user signups" msgstr "Account-Registrierungen" -#: uffd/templates/invite/list.html:49 uffd/templates/user/show.html:174 +#: uffd/templates/invite/list.html:49 uffd/templates/user/show.html:178 msgid "Disabled" msgstr "Deaktiviert" @@ -456,7 +460,7 @@ msgid "One address per line" msgstr "Eine Adresse pro Zeile" #: uffd/templates/mfa/auth.html:6 uffd/templates/selfservice/self.html:158 -#: uffd/templates/user/show.html:172 +#: uffd/templates/user/show.html:176 msgid "Two-Factor Authentication" msgstr "Zwei-Faktor-Authentifizierung" @@ -595,7 +599,7 @@ msgid "Reset two-factor configuration" msgstr "Zwei-Faktor-Authentifizierung zurücksetzen" #: uffd/templates/mfa/setup.html:46 uffd/templates/mfa/setup_recovery.html:5 -#: uffd/templates/user/show.html:175 +#: uffd/templates/user/show.html:179 msgid "Recovery Codes" msgstr "Wiederherstellungscodes" @@ -633,7 +637,7 @@ msgstr "Generiere neue Wiederherstellungscodes" msgid "You have no remaining recovery codes." msgstr "Du hast keine Wiederherstellungscodes übrig." -#: uffd/templates/mfa/setup.html:85 uffd/templates/user/show.html:175 +#: uffd/templates/mfa/setup.html:85 uffd/templates/user/show.html:179 msgid "Authenticator Apps (TOTP)" msgstr "Authentifikator-Apps (TOTP)" @@ -663,7 +667,7 @@ msgstr "Registriert am" msgid "No authenticator apps registered yet" msgstr "Bisher keine Authentifikator-Apps registriert" -#: uffd/templates/mfa/setup.html:134 uffd/templates/user/show.html:175 +#: uffd/templates/mfa/setup.html:134 uffd/templates/user/show.html:179 msgid "U2F and FIDO2 Devices" msgstr "U2F und FIDO2 Geräte" @@ -968,7 +972,7 @@ msgstr "Passwort vergessen" #: uffd/templates/selfservice/forgot_password.html:9 #: uffd/templates/selfservice/self.html:21 uffd/templates/session/login.html:12 #: uffd/templates/signup/start.html:9 uffd/templates/user/list.html:18 -#: uffd/templates/user/show.html:62 +#: uffd/templates/user/show.html:66 msgid "Login Name" msgstr "Anmeldename" @@ -989,7 +993,7 @@ msgstr "" "Authentifizierung.\n" "\tDiese Berechtigungen werden erst aktiv, wenn du dies getan hast." -#: uffd/templates/selfservice/self.html:14 uffd/templates/user/show.html:32 +#: uffd/templates/selfservice/self.html:14 uffd/templates/user/show.html:36 msgid "Profile" msgstr "Profil" @@ -1006,7 +1010,7 @@ msgid "Changes may take several minutes to be visible in all services." msgstr "Änderungen sind erst nach einigen Minuten in allen Diensten sichtbar." #: uffd/templates/selfservice/self.html:25 uffd/templates/signup/start.html:22 -#: uffd/templates/user/list.html:19 uffd/templates/user/show.html:77 +#: uffd/templates/user/list.html:19 uffd/templates/user/show.html:81 msgid "Display Name" msgstr "Anzeigename" @@ -1014,7 +1018,7 @@ msgstr "Anzeigename" msgid "Update Profile" msgstr "Änderungen speichern" -#: uffd/templates/selfservice/self.html:37 uffd/templates/user/show.html:94 +#: uffd/templates/selfservice/self.html:37 uffd/templates/user/show.html:98 msgid "E-Mail Addresses" msgstr "E-Mail-Adressen" @@ -1039,7 +1043,7 @@ msgstr "Neue E-Mail-Adresse" msgid "Add address" msgstr "Adresse hinzufügen" -#: uffd/templates/selfservice/self.html:55 uffd/templates/user/show.html:110 +#: uffd/templates/selfservice/self.html:55 uffd/templates/user/show.html:114 msgid "primary" msgstr "primär" @@ -1047,7 +1051,7 @@ msgstr "primär" msgid "unverified" msgstr "nicht bestätigt" -#: uffd/templates/selfservice/self.html:62 uffd/views/selfservice.py:171 +#: uffd/templates/selfservice/self.html:62 uffd/views/selfservice.py:175 msgid "Cannot delete primary e-mail address" msgstr "Primäre E-Mail-Adresse kann nicht gelöscht werden" @@ -1086,8 +1090,8 @@ msgid "Address for Password Reset E-Mails" msgstr "Adresse für Passwort-Zurücksetzen-E-Mails" #: uffd/templates/selfservice/self.html:101 -#: uffd/templates/selfservice/self.html:115 uffd/templates/user/show.html:139 -#: uffd/templates/user/show.html:149 +#: uffd/templates/selfservice/self.html:115 uffd/templates/user/show.html:143 +#: uffd/templates/user/show.html:153 msgid "Use primary address" msgstr "Primäre Adresse verwenden" @@ -1106,7 +1110,7 @@ msgstr "E-Mail-Einstellungen speichern" #: uffd/templates/selfservice/self.html:135 #: uffd/templates/session/login.html:16 uffd/templates/signup/start.html:36 -#: uffd/templates/user/show.html:159 +#: uffd/templates/user/show.html:163 msgid "Password" msgstr "Passwort" @@ -1151,7 +1155,7 @@ msgid "Manage two-factor authentication" msgstr "Zwei-Faktor-Authentifizierung (2FA) verwalten" #: uffd/templates/selfservice/self.html:177 uffd/templates/user/list.html:20 -#: uffd/templates/user/show.html:35 uffd/templates/user/show.html:184 +#: uffd/templates/user/show.html:39 uffd/templates/user/show.html:188 #: uffd/views/role.py:21 msgid "Roles" msgstr "Rollen" @@ -1227,7 +1231,7 @@ msgstr "Zugriff auf Mail-Weiterleitungen" msgid "Resolve remailer addresses" msgstr "Auflösen von Remailer-Adressen" -#: uffd/templates/service/api.html:51 uffd/templates/service/show.html:37 +#: uffd/templates/service/api.html:51 uffd/templates/service/show.html:38 msgid "This option has no effect: Remailer config options are unset" msgstr "Diese Option hat keine Auswirkung: Remailer ist nicht konfiguriert" @@ -1235,7 +1239,7 @@ msgstr "Diese Option hat keine Auswirkung: Remailer ist nicht konfiguriert" msgid "Access uffd metrics" msgstr "Zugriff auf uffd-Metriken" -#: uffd/templates/service/oauth2.html:20 uffd/templates/service/show.html:79 +#: uffd/templates/service/oauth2.html:20 uffd/templates/service/show.html:81 msgid "Client ID" msgstr "Client-ID" @@ -1308,21 +1312,21 @@ msgstr "Mitglieder der Gruppe \"%(group_name)s\" haben Zugriff" msgid "Hide e-mail addresses with remailer" msgstr "E-Mail-Adressen mit Remailer verstecken" -#: uffd/templates/service/show.html:41 +#: uffd/templates/service/show.html:43 msgid "Remailer disabled" msgstr "Remailer deaktiviert" -#: uffd/templates/service/show.html:44 +#: uffd/templates/service/show.html:46 msgid "Remailer enabled" msgstr "Remailer aktiviert" -#: uffd/templates/service/show.html:47 +#: uffd/templates/service/show.html:49 msgid "Remailer enabled (deprecated, case-sensitive format)" msgstr "" "Remailer aktiviert (veraltetes, Groß-/Kleinschreibung-unterscheidendes " "Format)" -#: uffd/templates/service/show.html:51 +#: uffd/templates/service/show.html:53 msgid "" "Some services notify users about changes to their e-mail address. " "Modifying this setting immediatly affects the e-mail addresses of all " @@ -1333,13 +1337,13 @@ msgstr "" "-Mail-Adressen aller Nutzer aus und kann zu massenhaftem Versand von " "Benachrichtigungs-E-Mails führen." -#: uffd/templates/service/show.html:58 +#: uffd/templates/service/show.html:60 msgid "Allow users to select a different e-mail address for this service" msgstr "" "Ermögliche Nutzern für diesen Dienst eine andere E-Mail-Adresse " "auszuwählen" -#: uffd/templates/service/show.html:60 +#: uffd/templates/service/show.html:62 msgid "If disabled, the service always uses the primary e-mail address." msgstr "Wenn deaktiviert, wird immer die primäre E-Mail-Adresse verwendet." @@ -1445,7 +1449,7 @@ msgstr "Überprüfen" msgid "At least one and at most 128 characters, no other special requirements." msgstr "Mindestens 1 und maximal 128 Zeichen, keine weiteren Einschränkungen." -#: uffd/templates/signup/start.html:29 uffd/templates/user/show.html:86 +#: uffd/templates/signup/start.html:29 uffd/templates/user/show.html:90 msgid "E-Mail Address" msgstr "E-Mail-Adresse" @@ -1505,7 +1509,7 @@ msgstr "CSV-Import" msgid "UID" msgstr "UID" -#: uffd/templates/user/list.html:34 uffd/templates/user/show.html:44 +#: uffd/templates/user/list.html:34 uffd/templates/user/show.html:48 msgid "service" msgstr "service" @@ -1523,7 +1527,7 @@ msgstr "" "Anzeigename oder das Password können (derzeit) nicht gesetzt werden. " "Beispiel:" -#: uffd/templates/user/list.html:75 uffd/templates/user/show.html:72 +#: uffd/templates/user/list.html:75 uffd/templates/user/show.html:76 msgid "Ignore login name blocklist" msgstr "Liste der nicht erlaubten Anmeldenamen ignorieren" @@ -1535,19 +1539,19 @@ msgstr "Importieren" msgid "New address" msgstr "Neue Adresse" -#: uffd/templates/user/show.html:42 +#: uffd/templates/user/show.html:46 msgid "User ID" msgstr "Account ID" -#: uffd/templates/user/show.html:50 +#: uffd/templates/user/show.html:54 msgid "will be choosen" msgstr "wird automatisch bestimmt" -#: uffd/templates/user/show.html:57 +#: uffd/templates/user/show.html:61 msgid "Service User" msgstr "Service-Account" -#: uffd/templates/user/show.html:65 +#: uffd/templates/user/show.html:69 msgid "" "Only letters, numbers, dashes (\"-\") and underscores (\"_\") are " "allowed. At most 32, at least 2 characters. There is a word blocklist. " @@ -1557,7 +1561,7 @@ msgstr "" "erlaubt. Maximal 32, mindestens 2 Zeichen. Es gibt eine Liste nicht " "erlaubter Namen. Muss einmalig sein." -#: uffd/templates/user/show.html:80 +#: uffd/templates/user/show.html:84 msgid "" "If you leave this empty it will be set to the login name. At most 128, at" " least 2 characters. No character restrictions." @@ -1565,7 +1569,7 @@ msgstr "" "Wenn das Feld leer bleibt, wird der Anmeldename verwendet. Maximal 128, " "mindestens 2 Zeichen. Keine Zeichenbeschränkung." -#: uffd/templates/user/show.html:89 +#: uffd/templates/user/show.html:93 msgid "" "Make sure the address is correct! Services might use e-mail addresses as " "account identifiers and rely on them being unique and verified." @@ -1574,15 +1578,15 @@ msgstr "" " E-Mail-Adresse um Accounts zu identifizieren und verlassen sich darauf, " "dass diese verifiziert und einzigartig sind." -#: uffd/templates/user/show.html:98 +#: uffd/templates/user/show.html:102 msgid "Address" msgstr "Adresse" -#: uffd/templates/user/show.html:99 +#: uffd/templates/user/show.html:103 msgid "Verified" msgstr "Verifiziert" -#: uffd/templates/user/show.html:125 +#: uffd/templates/user/show.html:129 msgid "" "Make sure that addresses you add are correct! Services might use e-mail " "addresses as account identifiers and rely on them being unique and " @@ -1592,36 +1596,36 @@ msgstr "" "Dienste verwenden die E-Mail-Adresse um Accounts zu identifizieren und " "verlassen sich darauf, dass diese verifiziert und einzigartig sind." -#: uffd/templates/user/show.html:129 +#: uffd/templates/user/show.html:133 msgid "Primary E-Mail Address" msgstr "Primäre E-Mail-Adresse" -#: uffd/templates/user/show.html:137 +#: uffd/templates/user/show.html:141 msgid "Recovery E-Mail Address" msgstr "Wiederherstellungs-E-Mail-Adresse" -#: uffd/templates/user/show.html:147 +#: uffd/templates/user/show.html:151 #, python-format msgid "Address for %(name)s" msgstr "Adresse für %(name)s" -#: uffd/templates/user/show.html:163 +#: uffd/templates/user/show.html:167 msgid "E-Mail to set it will be sent" msgstr "Mail zum Setzen wird versendet" -#: uffd/templates/user/show.html:174 +#: uffd/templates/user/show.html:178 msgid "Status:" msgstr "Status:" -#: uffd/templates/user/show.html:174 +#: uffd/templates/user/show.html:178 msgid "Enabled" msgstr "Aktiv" -#: uffd/templates/user/show.html:177 +#: uffd/templates/user/show.html:181 msgid "Reset 2FA" msgstr "2FA zurücksetzen" -#: uffd/templates/user/show.html:217 +#: uffd/templates/user/show.html:221 msgid "Resulting groups (only updated after save)" msgstr "Resultierende Gruppen (wird nur aktualisiert beim Speichern)" @@ -1864,13 +1868,13 @@ msgstr "Passwort geändert" msgid "E-Mail address already exists" msgstr "E-Mail-Adresse existiert bereits" -#: uffd/views/selfservice.py:124 uffd/views/selfservice.py:158 -#: uffd/views/selfservice.py:223 +#: uffd/views/selfservice.py:124 uffd/views/selfservice.py:162 +#: uffd/views/selfservice.py:227 #, python-format msgid "E-Mail to \"%(mail_address)s\" could not be sent!" msgstr "E-Mail an \"%(mail_address)s\" konnte nicht gesendet werden!" -#: uffd/views/selfservice.py:126 uffd/views/selfservice.py:160 +#: uffd/views/selfservice.py:126 uffd/views/selfservice.py:164 msgid "We sent you an email, please verify your mail address." msgstr "Wir haben dir eine E-Mail gesendet, bitte prüfe deine E-Mail-Adresse." @@ -1882,19 +1886,23 @@ msgstr "" "Dieser Link wurde für einen anderen Account erstellt. Melde dich mit dem " "richtigen Account an um Fortzufahren." -#: uffd/views/selfservice.py:148 +#: uffd/views/selfservice.py:150 +msgid "E-Mail address is already used by another account" +msgstr "E-Mail-Adresse wird bereits von einem anderen Account verwendet" + +#: uffd/views/selfservice.py:152 msgid "E-Mail address verified" msgstr "E-Mail-Adresse verifiziert" -#: uffd/views/selfservice.py:173 +#: uffd/views/selfservice.py:177 msgid "E-Mail address deleted" msgstr "E-Mail-Adresse gelöscht" -#: uffd/views/selfservice.py:194 +#: uffd/views/selfservice.py:198 msgid "E-Mail preferences updated" msgstr "E-Mail-Einstellungen geändert" -#: uffd/views/selfservice.py:205 +#: uffd/views/selfservice.py:209 #, python-format msgid "You left role %(role_name)s" msgstr "Rolle %(role_name)s verlassen" @@ -1949,7 +1957,7 @@ msgstr "Ungültiger Account-Registrierungs-Link" msgid "Too many failed attempts! Please wait %(delay)s." msgstr "Zu viele fehlgeschlagene Versuche! Bitte warte mindestens %(delay)s." -#: uffd/views/signup.py:112 +#: uffd/views/signup.py:113 msgid "Your account was successfully created" msgstr "Account erfolgreich erstellt" @@ -1957,33 +1965,39 @@ msgstr "Account erfolgreich erstellt" msgid "Users" msgstr "Accounts" -#: uffd/views/user.py:51 +#: uffd/views/user.py:48 msgid "Login name does not meet requirements" msgstr "Anmeldename entspricht nicht den Anforderungen" -#: uffd/views/user.py:98 +#: uffd/views/user.py:55 uffd/views/user.py:129 msgid "Display name does not meet requirements" msgstr "Anzeigename entspricht nicht den Anforderungen" -#: uffd/views/user.py:104 -msgid "Password is invalid" -msgstr "Passwort ist ungültig" - -#: uffd/views/user.py:120 +#: uffd/views/user.py:74 msgid "Service user created" msgstr "Service-Account erstellt" -#: uffd/views/user.py:123 +#: uffd/views/user.py:77 msgid "User created. We sent the user a password reset link by e-mail" msgstr "" "Account erstellt. E-Mail mit einem Link zum Setzen des Passworts wurde " "versendet." -#: uffd/views/user.py:125 +#: uffd/views/user.py:106 +msgid "E-Mail address already exists or is used by another account" +msgstr "" +"E-Mail-Adresse existiert bereits oder wird von einem anderen Account " +"verwendet" + +#: uffd/views/user.py:134 +msgid "Password is invalid" +msgstr "Passwort ist ungültig" + +#: uffd/views/user.py:146 msgid "User updated" msgstr "Account aktualisiert" -#: uffd/views/user.py:135 +#: uffd/views/user.py:156 msgid "Deleted user" msgstr "Account gelöscht" diff --git a/uffd/views/selfservice.py b/uffd/views/selfservice.py index 5d9b337bce7b5aeb6c8610936c551bb82a45863a..b4cbd4c826a061d6c995041fae56485fa836b352 100644 --- a/uffd/views/selfservice.py +++ b/uffd/views/selfservice.py @@ -144,7 +144,11 @@ def verify_email(secret, email_id=None, legacy_id=None): return redirect(url_for('selfservice.index')) if legacy_id is not None: request.user.primary_email = email - db.session.commit() + try: + db.session.commit() + except IntegrityError: + flash(_('E-Mail address is already used by another account')) + return redirect(url_for('selfservice.index')) flash(_('E-Mail address verified')) return redirect(url_for('selfservice.index')) diff --git a/uffd/views/signup.py b/uffd/views/signup.py index edbd3dcb5b073a2683a516816959273334414f0a..a5a779da947951a0808792e47757b958cfa94179 100644 --- a/uffd/views/signup.py +++ b/uffd/views/signup.py @@ -105,6 +105,7 @@ def signup_confirm_submit(signup_id, token): return render_template('signup/confirm.html', signup=signup) user, msg = signup.finish(request.form['password']) if user is None: + db.session.rollback() flash(msg, 'error') return render_template('signup/confirm.html', signup=signup) db.session.commit() diff --git a/uffd/views/user.py b/uffd/views/user.py index b312566796730e353a47f704a42155a6c5fcdd69..b3ba6788512544303d3d210e4cf043ff0c96d011 100644 --- a/uffd/views/user.py +++ b/uffd/views/user.py @@ -37,75 +37,103 @@ def show(id=None): user = User() if id is None else User.query.get_or_404(id) return render_template('user/show.html', user=user, roles=Role.query.all()) -@bp.route("/<int:id>/update", methods=['POST']) @bp.route("/new", methods=['POST']) @csrf_protect(blueprint=bp) -def update(id=None): +def create(): + user = User() + if request.form.get('serviceaccount'): + user.is_service_user = True + ignore_blocklist = request.form.get('ignore-loginname-blocklist', False) + if not user.set_loginname(request.form['loginname'], ignore_blocklist=ignore_blocklist): + flash(_('Login name does not meet requirements')) + return redirect(url_for('user.show')) + if not user.set_primary_email_address(request.form['email']): + flash(_('E-Mail address is invalid')) + return redirect(url_for('user.show')) + new_displayname = request.form['displayname'] if request.form['displayname'] else request.form['loginname'] + if user.displayname != new_displayname and not user.set_displayname(new_displayname): + flash(_('Display name does not meet requirements')) + return redirect(url_for('user.show')) + + db.session.add(user) + try: + db.session.flush() + except IntegrityError: + flash(_('Login name or e-mail address is already in use')) + return redirect(url_for('user.show')) + + 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.append(role) + user.update_groups() + + db.session.commit() + if user.is_service_user: + flash(_('Service user created')) + else: + send_passwordreset(user, new=True) + flash(_('User created. We sent the user a password reset link by e-mail')) + return redirect(url_for('user.show', id=user.id)) + +@bp.route("/<int:id>/update", methods=['POST']) +@csrf_protect(blueprint=bp) +def update(id): # pylint: disable=too-many-branches,too-many-statements - if id is None: - user = User() - ignore_blocklist = request.form.get('ignore-loginname-blocklist', False) - if request.form.get('serviceaccount'): - user.is_service_user = True - if not user.set_loginname(request.form['loginname'], ignore_blocklist=ignore_blocklist): - flash(_('Login name does not meet requirements')) - return redirect(url_for('user.show')) - if not user.set_primary_email_address(request.form['email']): + user = User.query.get_or_404(id) + + for email in user.all_emails: + if f'email-{email.id}-present' in request.form: + email.verified = email.verified or (request.form.get(f'email-{email.id}-verified') == '1') + for key, value in request.form.items(): + parts = key.split('-') + if not parts[0] == 'newemail' or not parts[2] == 'address' or not value: + continue + tmp_id = parts[1] + email = UserEmail( + user=user, + verified=(request.form.get(f'newemail-{tmp_id}-verified') == '1'), + ) + if not email.set_address(value): flash(_('E-Mail address is invalid')) - return redirect(url_for('user.show')) - else: - user = User.query.get_or_404(id) + return redirect(url_for('user.show', id=id)) + db.session.add(email) - for email in user.all_emails: - if f'email-{email.id}-present' in request.form: - email.verified = email.verified or (request.form.get(f'email-{email.id}-verified') == '1') + try: + db.session.flush() + except IntegrityError: + flash(_('E-Mail address already exists or is used by another account')) + return redirect(url_for('user.show', id=id)) - for key, value in request.form.items(): - parts = key.split('-') - if not parts[0] == 'newemail' or not parts[2] == 'address' or not value: - continue - tmp_id = parts[1] - email = UserEmail( - user=user, - verified=(request.form.get(f'newemail-{tmp_id}-verified') == '1'), - ) - if not email.set_address(value): - flash(_('E-Mail address is invalid')) - return redirect(url_for('user.show', id=id)) - db.session.add(email) - - verified_emails = UserEmail.query.filter_by(user=user, verified=True) - user.primary_email = verified_emails.filter_by(id=request.form['primary_email']).one() - if request.form['recovery_email'] == 'primary': - user.recovery_email = None + verified_emails = UserEmail.query.filter_by(user=user, verified=True) + user.primary_email = verified_emails.filter_by(id=request.form['primary_email']).one() + if request.form['recovery_email'] == 'primary': + user.recovery_email = None + else: + user.recovery_email = verified_emails.filter_by(id=request.form['recovery_email']).one() + for service_user in user.service_users: + if not service_user.service.enable_email_preferences: + continue + value = request.form.get(f'service_{service_user.service.id}_email', 'primary') + if value == 'primary': + service_user.service_email = None else: - user.recovery_email = verified_emails.filter_by(id=request.form['recovery_email']).one() - for service_user in user.service_users: - if not service_user.service.enable_email_preferences: - continue - value = request.form.get(f'service_{service_user.service.id}_email', 'primary') - if value == 'primary': - service_user.service_email = None - else: - service_user.service_email = verified_emails.filter_by(id=value).one() - - for email in user.all_emails: - if request.form.get(f'email-{email.id}-delete') == '1': - db.session.delete(email) + service_user.service_email = verified_emails.filter_by(id=value).one() + for email in user.all_emails: + if request.form.get(f'email-{email.id}-delete') == '1': + db.session.delete(email) new_displayname = request.form['displayname'] if request.form['displayname'] else request.form['loginname'] if user.displayname != new_displayname and not user.set_displayname(new_displayname): flash(_('Display name does not meet requirements')) return redirect(url_for('user.show', id=id)) - new_password = request.form.get('password') - if id is not None and new_password: + if new_password: if not user.set_password(new_password): flash(_('Password is invalid')) return redirect(url_for('user.show', id=id)) - db.session.add(user) - user.roles.clear() for role in Role.query.all(): if not user.is_service_user and role.is_default: @@ -115,14 +143,7 @@ def update(id=None): user.update_groups() db.session.commit() - if id is None: - if user.is_service_user: - flash(_('Service user created')) - else: - send_passwordreset(user, new=True) - flash(_('User created. We sent the user a password reset link by e-mail')) - else: - flash(_('User updated')) + flash(_('User updated')) return redirect(url_for('user.show', id=user.id)) @bp.route("/<int:id>/del")