jazzband / django-invitations

Generic invitations app for Django
GNU General Public License v3.0
559 stars 166 forks source link

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

Open Flimm opened 2 months ago

Flimm commented 2 months ago

Summary:

I propose that the Invitation be modified like this:

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.