add email_scope option for email uniqueness validator

[flynfish]

This is a reboot of the closed PR #4793 with some tests. Issue #4767 has a lot of information about why this would be useful.

Description from #4793

• per subdomain auth;
• soft deletion and paranoid like support;
• example from https://github.com/plataformatec/devise/wiki/How-To:-Allow-users-to-sign-in-using-their-username-or-email-address.

[ACPK]

@flynfish @tegon - Do you know the status of this? I'd like to implement Device for a project that uses subdomains.

[flynfish]

@ACPK I haven't heard anything else from @tegon yet. We are using my fork at the moment until this can hopefully be merged.

[tegon]

Sorry guys, I wasn't able to look at this yet, but I'll definitely take a look as soon as I can.

[christianrolle]

@tegon scoping the email makes totally sense in a multi tenant scenario. We're gonna fork it with that important enhancement as well. Hope we're able to come back to trunk soon.

[bananatron]

Scoping email validations is HUGE - thanks so much for this!

[christianrolle]

@flynfish Did you have the chance to push this change forward? We certainly should get it done.

[flynfish]

@flynfish Did you have the chance to push this change forward? We certainly should get it done.

I didn't have a chance, but thanks for the reminder. I'm gonna set a goal to get the comments addressed in the next week.

[donny-nyc]

@flynfish we're looking to enable this in our project. Would you recommend forking for now?

[Unnumbered]

any news?

[xhocquet]

Hey @tegon , I'm stepping in to try and push this over the finish line for my colleague @flynfish

I've split the test case out and reverted some unnecessary changes caused by conflating the two test users. Could you take another look and let me know what you think? Thanks! 🙏

[xhocquet]

Hey @carlosantoniodasilva , I noticed you were actively merging PRs, could you help us get this one across the finish line? I think the community would appreciate this feature!

[carlosantoniodasilva]

@xhocquet hey, yes! Appreciate the ping. I've recently started to catch up on some of the issues/PRs here, just want to wrap up a few things on the other projects and then Devise is next. I'm putting this one high in the list to follow-up on first, please stay put for a little longer, I should get to it soon.

[danzanzini]

Hello @carlosantoniodasilva, I was taking a look at this PR and I haven't found where these changes impact the migration generated to create users.

As far as I know, the migration generated by Devise adds a database index to ensure email uniqueness. Without changing this index, this feature will not work since the new user creation will be blocked at the database layer.

Another thing to take into consideration is that the recoverable module may not work well because of the duplicated e-mails. As described in this wiki post:

it may be the case that users can have multiple accounts with the same e-mail address. This will cause the “recoverable” module not to work well, as it will only include a link in the password reset e-mail to the first account for which it finds a matching e-mail.

I'm not so sure about all of this, but can you take a look when reviewing this PR? If you find that these points are blockers for this PR, I will be glad to help to find possible solutions and work on them.

Thank you

[xhocquet]

Thanks for the feedback @danzanzini , some thoughts -

By following the referenced wiki post, the migration created by Devise generators would already be insufficient since they would not contain the 'username' for example. One option could be to add a commented out row with the scoped index in the migration to guide developers to the right usage?

Given the migration gotcha and the incompatibility with the recoverable module, maybe it makes sense to create a new wiki for scoped emails outlining the caveats?

[danzanzini]

@xhocquet creating a wiki article is a good idea and should solve all these problems, I think it will also remove the need for the commented line in the migration since it would be noise for most users.

I'll work on a first version and then bring it here for discussion.

Thank you :)

[danzanzini]

@xhocquet Here is the ~first version of the wiki page~ wiki page, feel free to make any adjustments you find necessary.

Since this feature is not merged, I haven't added it to the wiki index.

[xhocquet]

@danzanzini Small typo fix but otherwise the wiki looks great! I'm not sure if there's other considerations for this change, but the existing page seems to reflect the two you mentioned succinctly.

Edit: Realized I changed the title and therefore the link above became invalid. New wiki link

[vickodin]

On the other hand, we have next 2 options in config/initializers/devise.rb

  # ==> Configuration for any authentication mechanism
  # Configure which keys are used when authenticating a user. The default is
  # just :email. You can configure it to use [:username, :subdomain], so for
  # authenticating a user, both parameters are required. Remember that those
  # parameters are used only when authenticating and not when retrieving from
  # session. If you need permissions, you should implement that in a before filter.
  # You can also supply a hash where the value is a boolean determining whether
  # or not authentication should be aborted when the value is not present.
  # config.authentication_keys = [:email]

  # Configure parameters from the request object used for authentication. Each entry
  # given should be a request method and it will automatically be passed to the
  # find_for_authentication method and considered in your model lookup. For instance,
  # if you set :request_keys to [:subdomain], :subdomain will be used on authentication.
  # The same considerations mentioned for authentication_keys also apply to request_keys.
  # config.request_keys = []

I'm not yet understand why we need request_keys, but we can use: config.authentication_keys = [:account_id, :email] instead of email_scope

What do you think?

[xhocquet]

@vickodin I think your suggestion is not directly related to the problem this PR is trying to solve. The problem we are trying to solve is that multiple users with the same email cannot be created because of Devise validations.

The configs you point to could be used to lookup these users after they are created. Personally we would not use this and would instead use domain-based scoping to load our user records.

[vickodin]

@vickodin I think your suggestion is not directly related to the problem this PR is trying to solve. The problem we are trying to solve is that multiple users with the same email cannot be created because of Devise validations.

The configs you point to could be used to lookup these users after they are created. Personally we would not use this and would instead use domain-based scoping to load our user records.

@xhocquet Thank you for explanations!

Then when this PR will be merged?

Thanks in advance!

[xhocquet]

Hey @carlosantoniodasilva, happy new year! Hoping to bump this in your notifications 😄

[carlosantoniodasilva]

@xhocquet happy new year! Consider it bumped 😉

There may be more to it than just the validation so I need to look into it a bit more carefully in the coming weeks.

[nexxus-vi]

+1, we need this please :)

[poetry-like-code]

Although this request seems to be dragging for morethan 3 years now, i don't see anyone venting out any frustration, kudos to everyone for that. :) And +1, we need this too :) kindly fast-track.

[stephenpassero]

@carlosantoniodasilva I noticed @poetry-like-code brought up this PR recently, but didn't see any changes. Is there any update on getting this merged?

Thanks!

[carlosantoniodasilva]

@stephenpassero appreciate the ping, unfortunately no updates at this time yet.

This is still where it stands:

There may be more to it than just the validation so I need to look into it a bit more carefully in the coming weeks.

Unfortunately 3+ months and I haven't stopped to look into this carefully yet, but I'm bumping it again in my list here. First I have to get a release out there. 😅

[akshaysmurthy]

Until this is merged, what is the recommendation for people like me who wants to implement this (email uniqueness on a scope) in our app today? 😄

[danzanzini]

Hi @akshaysmurthy, I did these steps to implement it in my app:

1. Changed the unique index of the email column:

def up
  remove_index :users, :email
  add_index :users, [:email, :tenant_id], unique: true
end

2. Overwrote the will_save_change_to_email? method so Devise skips its built-in validations. This change may impact other parts of your application if this method is used anywhere else.

# user.rb
def will_save_change_to_email?
  false
end

3. Created my own email validations:

# user.rb

validates :email, uniqueness: { scope: :tenant_id }
validates :email, format: /\A[^@\s]+@[^@\s]+\z/

Hope it helps

[mrabetr]

when is this due to be merged please? thanks

[flynfish]

@tegon @vickodin @carlosantoniodasilva it would be really helpful to the community if we could get this merged or if you could provide any insight into what we can do to help out further