Skip to content

Commit

Permalink
add unique index to enforce no duplicate on case-insensitive user emails
Browse files Browse the repository at this point in the history
  • Loading branch information
fmigneault-crim committed Sep 2, 2022
1 parent 008cf35 commit 9c3d8ad
Show file tree
Hide file tree
Showing 5 changed files with 45 additions and 7 deletions.
6 changes: 5 additions & 1 deletion CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,11 @@ Changes

Features / Changes
~~~~~~~~~~~~~~~~~~~~~
* Nothing new for the moment.
* | Add database unique index to ensure case-insensitive ``User`` email cannot be stored.
|
| **IMPORTANT**:
| If any ``User`` entries with duplicate case-insensitive emails are present in the database, the application
will fail when performing the database migration. Resolve those cases manually before starting `Magpie`.
Bug Fixes
~~~~~~~~~~~~~~~~~~~~~
Expand Down
7 changes: 5 additions & 2 deletions magpie/alembic/script.py.mako
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,12 @@ Create Date: ${create_date}

import sqlalchemy as sa
from alembic import op
from sqlalchemy.orm.session import sessionmaker

# Revision identifiers, used by Alembic.
# pylint: disable=C0103,invalid-name # revision control variables not uppercase
%if revision:
revision = "${revision}"
%if up_revision:
revision = "${up_revision}"
%else:
revision = None
%endif
Expand All @@ -32,6 +33,8 @@ depends_on = "${depends_on}"
depends_on = None
%endif

Session = sessionmaker()


def upgrade():
${upgrades if upgrades else "pass"}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
"""
Case Insensitive Email Constraint
Revision ID: 5e5acc33adce
Revises: 0c6269f410cd
Create Date: 2022-09-01 21:16:40.175730
"""

from alembic import op
from sqlalchemy import text

# Revision identifiers, used by Alembic.
# pylint: disable=C0103,invalid-name # revision control variables not uppercase
revision = "5e5acc33adce"
down_revision = "0c6269f410cd"
branch_labels = None
depends_on = None


def upgrade():
op.create_index(
"ix_users_email_unique_case_insensitive",
"users",
[text("lower(email)")],
unique=True,
)


def downgrade():
op.drop_index("ix_users_email_unique_case_insensitive", "users")
7 changes: 4 additions & 3 deletions magpie/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
from sqlalchemy.ext.declarative import declarative_base, declared_attr
from sqlalchemy.ext.hybrid import hybrid_property
from sqlalchemy.orm import relationship
from sqlalchemy.sql import func
from ziggurat_foundations import ziggurat_model_init
from ziggurat_foundations.models.base import BaseModel, get_db_session
from ziggurat_foundations.models.external_identity import ExternalIdentityMixin
Expand Down Expand Up @@ -512,16 +513,16 @@ def by_name_or_email(cls, user_name, email, status=None, db_session=None):
if status is not None:
status = [int(status) for status in status]
query = query.in_(status)
return query.filter((User.user_name == user_name) | (User.email == email)).first()
return query.filter((User.user_name == user_name) | (func.lower(User.email) == email.lower())).first()
user = cls.by_user_name(user_name=user_name, status=status, db_session=db_session)
if user is not None:
return user
if status is UserStatuses.Pending:
return db_session.query(UserPending).filter(UserPending.email == email).first()
return db_session.query(UserPending).filter(func.lower(UserPending.email) == email.lower()).first()
user = super(UserSearchService, cls).by_email(email=email, db_session=db_session)
if user is not None and UserStatuses.get(user.status) in status:
return user
return db_session.query(UserPending).filter(UserPending.email == email).first()
return db_session.query(UserPending).filter(func.lower(UserPending.email) == email.lower()).first()


class ExternalIdentity(ExternalIdentityMixin, Base):
Expand Down
2 changes: 1 addition & 1 deletion magpie/ui/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -558,7 +558,7 @@ def create_user(self, data):

# soft pre-checks
user_details = self.get_user_details(status="all", cookies=admin_cookies)
if user_email in [usr["email"].lower() for usr in user_details]:
if (user_email or "").lower() in [usr["email"].lower() for usr in user_details]:
data["invalid_user_email"] = True
data["reason_user_email"] = "Conflict"
if user_email == "":
Expand Down

0 comments on commit 9c3d8ad

Please sign in to comment.