Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proposal: drop email constraint and add (invitee, email) constraint #251

Open
Flimm opened this issue Jun 14, 2024 · 0 comments
Open

Proposal: drop email constraint and add (invitee, email) constraint #251

Flimm opened this issue Jun 14, 2024 · 0 comments

Comments

@Flimm
Copy link
Contributor

Flimm commented Jun 14, 2024

Summary:

I propose that the Invitation be modified like this:

  • Remove the case-sensitive unique constraint on email
  • Create a new unique constraint on the composed key (inviter, email), with email being case insensitive
  • Because the email index has gone from being case-sensitive to case-insensitive, the migration needs to handle duplicates that differ only in case. I propose deleting duplicates in the migration.

I wanted to run this design by you all, before attempting to implement it.

This design would solve these issues:

Issue 1: duplicate email addresses that differ only in case cause problems

The Invitation model has a unique constraint on the email field:

class Invitation(AbstractBaseInvitation):
    email = models.EmailField(
        unique=True,
        verbose_name=_("e-mail address"),
        max_length=app_settings.EMAIL_MAX_LENGTH,
    )

Email addresses are not considered case-sensitive everywhere else in the code, and yet this constraint is case-sensitive. This can cause weird issues with edge cases. To fix this, the constraint can be made case-insensitive, and the migration can delete duplicates to avoid errors.

Issue 2: One user's invitations leak to another and cause errors

If two users try to invite the same email address, the second one will receive the AlreadyInvited exception, or similar. This is confusing, and potentially a security vulnerability. We have leaked private information. User A may not want user B to know that A has invited the same email address as user B.

With the composed key (invitee, email), each user can invite as many email addresses as they wish, without conflicting with any body else's invitations. The code would need to be adjusted for this. Application developers won't need to change their code much to adjust for this change, as their code assumes a stricter constraint that is compatible with a less strict constraint.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant