globaleaks / GlobaLeaks

GlobaLeaks is free, open-source software enabling anyone to easily set up and maintain a secure whistleblowing platform.
https://www.globaleaks.org
Other
1.21k stars 267 forks source link

traceback when adding a duplicated user #3815

Closed mapreri closed 8 months ago

mapreri commented 9 months ago

What version of GlobaLeaks are you using?

4.13.18

What browser(s) are you seeing the problem on?

All

What operating system(s) are you seeing the problem on?

Linux

Describe the issue

An administrator tried to create a user, accidentally using the same username of another user already present in the same site.

I got this error via email:

SQLAlchemy error ``` sqlalchemy.exc.IntegrityError Wraps a DB-API IntegrityError. Traceback (most recent call last): File "/usr/lib/python3/dist-packages/sqlalchemy/engine/base.py", line 1802, in _execute_context self.dialect.do_execute( File "/usr/lib/python3/dist-packages/sqlalchemy/engine/default.py", line 732, in do_execute cursor.execute(statement, parameters) sqlite3.IntegrityError: UNIQUE constraint failed: user.tid, user.username The above exception was the direct cause of the following exception: Traceback (most recent call last): File "/usr/lib/python3/dist-packages/twisted/internet/defer.py", line 1656, in _inlineCallbacks result = current_context.run( File "/usr/lib/python3/dist-packages/twisted/python/failure.py", line 489, in throwExceptionIntoGenerator return g.throw(self.type, self.value, self.tb) File "/usr/lib/python3/dist-packages/globaleaks/handlers/admin/user.py", line 187, in post user = yield create_user(self.request.tid, self.session, request, self.request.language) File "/usr/lib/python3/dist-packages/twisted/python/threadpool.py", line 244, in inContext result = inContext.theWork() # type: ignore[attr-defined] File "/usr/lib/python3/dist-packages/twisted/python/threadpool.py", line 260, in inContext.theWork = lambda: context.call( # type: ignore[attr-defined] File "/usr/lib/python3/dist-packages/twisted/python/context.py", line 117, in callWithContext return self.currentContext().callWithContext(ctx, func, *args, **kw) File "/usr/lib/python3/dist-packages/twisted/python/context.py", line 82, in callWithContext return func(*args, **kw) File "/usr/lib/python3/dist-packages/globaleaks/orm.py", line 185, in _wrap result = function(session, *args, **kwargs) File "/usr/lib/python3/dist-packages/globaleaks/handlers/admin/user.py", line 112, in create_user return user_serialize_user(session, db_create_user(session, tid, user_session, request, language), language) File "/usr/lib/python3/dist-packages/globaleaks/handlers/admin/user.py", line 78, in db_create_user session.flush() File "/usr/lib/python3/dist-packages/sqlalchemy/orm/session.py", line 3363, in flush self._flush(objects) File "/usr/lib/python3/dist-packages/sqlalchemy/orm/session.py", line 3502, in _flush with util.safe_reraise(): File "/usr/lib/python3/dist-packages/sqlalchemy/util/langhelpers.py", line 70, in __exit__ compat.raise_( File "/usr/lib/python3/dist-packages/sqlalchemy/util/compat.py", line 207, in raise_ raise exception File "/usr/lib/python3/dist-packages/sqlalchemy/orm/session.py", line 3463, in _flush flush_context.execute() File "/usr/lib/python3/dist-packages/sqlalchemy/orm/unitofwork.py", line 456, in execute rec.execute(self) File "/usr/lib/python3/dist-packages/sqlalchemy/orm/unitofwork.py", line 630, in execute util.preloaded.orm_persistence.save_obj( File "/usr/lib/python3/dist-packages/sqlalchemy/orm/persistence.py", line 244, in save_obj _emit_insert_statements( File "/usr/lib/python3/dist-packages/sqlalchemy/orm/persistence.py", line 1221, in _emit_insert_statements result = connection._execute_20( File "/usr/lib/python3/dist-packages/sqlalchemy/engine/base.py", line 1614, in _execute_20 return meth(self, args_10style, kwargs_10style, execution_options) File "/usr/lib/python3/dist-packages/sqlalchemy/sql/elements.py", line 325, in _execute_on_connection return connection._execute_clauseelement( File "/usr/lib/python3/dist-packages/sqlalchemy/engine/base.py", line 1481, in _execute_clauseelement ret = self._execute_context( File "/usr/lib/python3/dist-packages/sqlalchemy/engine/base.py", line 1845, in _execute_context self._handle_dbapi_exception( File "/usr/lib/python3/dist-packages/sqlalchemy/engine/base.py", line 2026, in _handle_dbapi_exception util.raise_( File "/usr/lib/python3/dist-packages/sqlalchemy/util/compat.py", line 207, in raise_ raise exception File "/usr/lib/python3/dist-packages/sqlalchemy/engine/base.py", line 1802, in _execute_context self.dialect.do_execute( File "/usr/lib/python3/dist-packages/sqlalchemy/engine/default.py", line 732, in do_execute cursor.execute(statement, parameters) sqlalchemy.exc.IntegrityError: (sqlite3.IntegrityError) UNIQUE constraint failed: user.tid, user.username [SQL: INSERT INTO user (id, tid, creation_date, username, salt, hash, name, description, public_name, role, enabled, last_login, mail_address, language, password_change_needed, password_change_date, crypto_prv_key, crypto_pub_key, crypto_rec_key, crypto_bkp_key, crypto_escrow_prv_key, crypto_escrow_bkp1_key, crypto_escrow_bkp2_key, change_email_address, change_email_token, change_email_date, notification, forcefully_selected, can_delete_submission, can_postpone_expiration, can_grant_access_to_reports, can_transfer_access_to_reports, can_mask_information, can_redact_information, can_edit_general_settings, readonly, two_factor_secret, reminder_date, pgp_key_fingerprint, pgp_key_public, pgp_key_expiration, accepted_privacy_policy, clicked_recovery_key) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)] [parameters: (', '', None, '1970-01-01 00:00:00.000000', 1, 0, 0, 1, 0, 0, 0, 0, 0, 0, '', '1970-01-01 00:00:00.000000', '', '', '1970-01-01 00:00:00.000000', '1970-01-01 00:00:00.000000', 0)] (Background on this error at: https://sqlalche.me/e/14/gkpj) ```

I've been told (I honestly didn't try this myself) that from the web interface, they only got a very generic red float box saying "failed to create user" or something similar.

Proposed solution

I think you should have a nicer form validation step on submission without trying to commit to database, failing, and have to notify all the admins of this trivial error.

Thank you!

evilaliv3 commented 9 months ago

Thank you @mapreri

You are correct about this topic and with your suggestion for improval.

Globaleaks for security reason was written small and contains all the checks (in this case unique and foreign constraints) that prevent situations of this kind but still miss error messages like the one that you mention.

The main reason of this is the fact that adding a single text to the software require translation in 90 languages and thus requires to make things right at first

As the software is open source wow that the community of users is growing we hope the community could study with us this aspects and join us in the development.

Do you have by any chance will and possibility to support this improvement?

mapreri commented 9 months ago

Do you have by any chance will and possibility to support this improvement?

Alas, I can't imagine myself dedicating this amount of time to globaleaks as a contributor, I'm sorry. :(

I might try to ask my employer, however that will depend a lot on how things go... not clear yet.

evilaliv3 commented 8 months ago

Closing as duplicate of https://github.com/globaleaks/GlobaLeaks/issues/3875