diff --git a/lib/galaxy/managers/users.py b/lib/galaxy/managers/users.py index aaeae3a56496..1beda4106fd6 100644 --- a/lib/galaxy/managers/users.py +++ b/lib/galaxy/managers/users.py @@ -197,7 +197,7 @@ def purge(self, user, flush=True): private_role = self.app.security_agent.get_private_user_role(user) if private_role is None: raise exceptions.InconsistentDatabase( - "User '%s' private role is missing while attempting to purge deleted user." % user.email + f"User {user.email} private role is missing while attempting to purge deleted user." ) # Delete History for active_history in user.active_histories: @@ -664,23 +664,11 @@ def get_or_create_remote_user(self, remote_user_email): self.app.security_agent.user_set_default_permissions(user) self.app.security_agent.user_set_default_permissions(user, history=True, dataset=True) elif user is None: - username = remote_user_email.split("@", 1)[0].lower() random.seed() user = self.app.model.User(email=remote_user_email) user.set_random_password(length=12) user.external = True - # Replace invalid characters in the username - for char in [x for x in username if x not in f"{string.ascii_lowercase + string.digits}-."]: - username = username.replace(char, "-") - # Find a unique username - user can change it later - stmt = select(self.app.model.User).filter_by(username=username).limit(1) - if self.session().scalars(stmt).first(): - i = 1 - stmt = select(self.app.model.User).filter_by(username=f"{username}-{str(i)}").limit(1) - while self.session().scalars(stmt).first(): - i += 1 - username += f"-{str(i)}" - user.username = username + user.username = username_from_email(self.session(), remote_user_email, self.app.model.User) self.session().add(user) with transaction(self.session()): self.session().commit() @@ -896,3 +884,31 @@ def get_user_by_email(session, email: str, model_class=User, case_sensitive=True def get_user_by_username(session, username: str, model_class=User): stmt = select(model_class).filter(model_class.username == username).limit(1) return session.scalars(stmt).first() + + +def username_from_email(session, email, model_class=User): + """Get next available username generated based on email""" + username = email.split("@", 1)[0].lower() + username = filter_out_invalid_username_characters(username) + if username_exists(session, username, model_class): + username = generate_next_available_username(session, username, model_class) + return username + + +def filter_out_invalid_username_characters(username): + """Replace invalid characters in username""" + for char in [x for x in username if x not in f"{string.ascii_lowercase + string.digits}-."]: + username = username.replace(char, "-") + return username + + +def username_exists(session, username: str, model_class=User): + return bool(get_user_by_username(session, username, model_class)) + + +def generate_next_available_username(session, username, model_class=User): + """Generate unique username; user can change it later""" + i = 1 + while session.execute(select(model_class).where(model_class.username == f"{username}-{i}")).first(): + i += 1 + return f"{username}-{i}" diff --git a/lib/galaxy/model/migrations/alembic/versions_gxy/c63848676caf_update_username_column_schema_and_data.py b/lib/galaxy/model/migrations/alembic/versions_gxy/c63848676caf_update_username_column_schema_and_data.py new file mode 100644 index 000000000000..bd43e53bd443 --- /dev/null +++ b/lib/galaxy/model/migrations/alembic/versions_gxy/c63848676caf_update_username_column_schema_and_data.py @@ -0,0 +1,89 @@ +"""Update username column schema and data + +Revision ID: c63848676caf +Revises: c14a3c93d66b +Create Date: 2024-06-11 13:41:36.411803 + +""" + +import string + +from alembic import op +from sqlalchemy import ( + or_, + select, + update, +) + +from galaxy.model import User +from galaxy.model.migrations.util import ( + alter_column, + transaction, +) + +# revision identifiers, used by Alembic. +revision = "c63848676caf" +down_revision = "c14a3c93d66b" +branch_labels = None +depends_on = None + +table_name = "galaxy_user" +column_name = "username" + + +def upgrade(): + with transaction(): + _generate_missing_usernames() + alter_column(table_name, column_name, nullable=False) + + +def downgrade(): + with transaction(): + # The data migration is one-way: we can change back the database schema, + # but we can't remove the generated username values. + alter_column(table_name, column_name, nullable=True) + + +def _generate_missing_usernames(): + stmt = select(User.id, User.email).where(or_(User.username.is_(None), User.username == "")) + connection = op.get_bind() + users = connection.execute(stmt).all() + for id, email in users: + new_username = username_from_email(connection, email) + update_stmt = update(User).where(User.id == id).values(username=new_username) + connection.execute(update_stmt) + + +# The code below is a near-duplicate of similar code in managers.users. The duplication is +# intentional: we want to preserve this logic in the migration script. The only differences are: +# (1) this code uses a Connection instead of a Session; +# (2) the username_exists function inlines the Select statement from managers.users::get_user_by_username. + + +def username_from_email(connection, email, model_class=User): + # This function is also called from database revision scripts, which do not provide a session. + username = email.split("@", 1)[0].lower() + username = filter_out_invalid_username_characters(username) + if username_exists(connection, username, model_class): + username = generate_next_available_username(connection, username, model_class) + return username + + +def filter_out_invalid_username_characters(username): + """Replace invalid characters in username""" + for char in [x for x in username if x not in f"{string.ascii_lowercase + string.digits}-."]: + username = username.replace(char, "-") + return username + + +def username_exists(connection, username: str, model_class=User): + stmt = select(model_class).filter(model_class.username == username).limit(1) + return bool(connection.execute(stmt).first()) + + +def generate_next_available_username(connection, username, model_class=User): + """Generate unique username; user can change it later""" + i = 1 + while connection.execute(select(model_class).where(model_class.username == f"{username}-{i}")).first(): + i += 1 + return f"{username}-{i}" diff --git a/lib/galaxy/model/migrations/alembic/versions_gxy/ddbdbc40bdc1_add_workflow_comment_table.py b/lib/galaxy/model/migrations/alembic/versions_gxy/ddbdbc40bdc1_add_workflow_comment_table.py index b0cfac349b77..8b2fd0b23efa 100644 --- a/lib/galaxy/model/migrations/alembic/versions_gxy/ddbdbc40bdc1_add_workflow_comment_table.py +++ b/lib/galaxy/model/migrations/alembic/versions_gxy/ddbdbc40bdc1_add_workflow_comment_table.py @@ -1,4 +1,5 @@ """add workflow_comment table + Revision ID: ddbdbc40bdc1 Revises: 92fb564c7095 Create Date: 2023-08-14 13:41:59.442243 diff --git a/lib/galaxy/model/migrations/dbscript.py b/lib/galaxy/model/migrations/dbscript.py index 58d83755e777..2e17c361d4f6 100644 --- a/lib/galaxy/model/migrations/dbscript.py +++ b/lib/galaxy/model/migrations/dbscript.py @@ -42,8 +42,8 @@ "23.2": "8a19186a6ee7", "release_24.0": "55f02fd8ab6c", "24.0": "55f02fd8ab6c", - "release_24.1": "c14a3c93d66b", - "24.1": "c14a3c93d66b", + "release_24.1": "c63848676caf", + "24.1": "c63848676caf", }