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

Require email address to match domain #975

Closed
mhkeller opened this issue Oct 2, 2015 · 13 comments
Closed

Require email address to match domain #975

mhkeller opened this issue Oct 2, 2015 · 13 comments
Labels
Feature: Request Requested Feature

Comments

@mhkeller
Copy link

mhkeller commented Oct 2, 2015

When registering for an account, it would be great to restrict that ability to only users of a certain email domain.

@gmsecrieru
Copy link
Contributor

👍 Looks like a great settings-based feature!

@gmsecrieru gmsecrieru added the Feature: Request Requested Feature label Oct 2, 2015
@Ribeiro
Copy link

Ribeiro commented Oct 6, 2015

@gmsecrieru Could you please give some example on this requirement? Maybe I could work on this! Thankx =)

@gmsecrieru
Copy link
Contributor

@Ribeiro Well, I think there's more to the picture than meets the eye, but to get things moving you could:

  • Create a new Admin setting for the domain restriction - ref. here
  • Change Registrarion form validation based on the setting.

Feel free to ping us on the Demo Server should you have any questions, and thanks for your help!

@emcguinness
Copy link
Contributor

Definately sounds like a useful feature, just a few thoughts. First on features. Domain lockdown should not be to a single domain, it should be a list you can optionally add multiple domains to, bonus points if they accept regex. An option to negate the list to turn it into a blacklist could be handy for some people.

Then just a thought on implementation, how does this interact with the other login methods(Google, github, etc)? Easiest option is to have it work only with local logins, perhaps with a warning if you enable the feature while you have a third party sign in option enabled? The more useful, but perhaps complex, option could be to have it apply to these alternative login methods where it makes sense(for example gmail as that is also still an email address).

These are just some ramblings, feel free to ignore part or all of the above message.

@gmsecrieru
Copy link
Contributor

@emcguinness Thanks for putting this down!

👍 for allowing a list of domains, but that might require a few tweaks on the RocketChat.Settings behavior or a new type of setting -- just a guess here, would have to dig further to confirm. Pertaining to other logins methods, I see at least two roads:

  1. Disable other login methods, as e-mail registration is currently our default-by-design option
  2. Keep other methods setup but (obviously) limit e-mail registration to the whitelist

Maybe we should stick with a simpler/less complex scenario for this feature at this point, so that we can have more people contributing :)

@mhkeller
Copy link
Author

mhkeller commented Oct 7, 2015

Thanks for working on this! As a lazy way of implementing a list if that settings type doesn't yet exist would be ask for a comma-delimited string. In case they add spaces you could trim after you split on the comma.

Le Oct 7, 2015 à 07:20, George Secrieru [email protected] a écrit :

@emcguinness Thanks for putting this down!

:+1 for allowing a list of domains, but that might require a few tweaks on the RocketChat.Settings behavior or a new type of setting -- just a guess here, would have to dig further to confirm. Pertaining to other logins methods, I see at least two roads:

Disable other login methods, as e-mail registration is currently our default-by-design option
Keep other methods setup but (obviously) limit e-mail registration to the whitelist
Maybe we should stick with a simpler/less complex scenario for this feature at this point, so that we can have more people contributing :)


Reply to this email directly or view it on GitHub.

@Ribeiro
Copy link

Ribeiro commented Oct 7, 2015

@gmsecrieru I was just taking a look at the code and it seems I need manipulate:

  1. RocketChat.settings => adding valid domains range;
  2. accounts.coffee => should implement and call a function to validate user´s email if it is within RocketChat,settings valid domains range on Accounts.onCreateUser().

Am I taking the right approach? Please, let me know. Thankx !

@gmsecrieru
Copy link
Contributor

@Ribeiro Maybe we can leverage on configuration, looks like we have restrictCreationByEmailDomain for this!

@Ribeiro
Copy link

Ribeiro commented Oct 7, 2015

@gmsecrieru So on Rocket.Chat/server/lib/accounts.coffee we would use:

Accounts.config({
restrictCreationByEmailDomain: function(email) {
var domain = email.slice(email.lastIndexOf("@")+1); // or regex
var allowed = ["gmail.com", "outlook.com"];
return _.contains(allowed, domain);
},
...
});

Am I on the right direction here? ;)

@Ribeiro
Copy link

Ribeiro commented Oct 8, 2015

@gmsecrieru What do you think? Is there a domain whitelist? =)

Ribeiro added a commit to Ribeiro/Rocket.Chat that referenced this issue Oct 9, 2015
@gmsecrieru
Copy link
Contributor

@Ribeiro Sorry, been busy setting up unit tests. I will have this reviewed ASAP.

Thanks again for your effort!

@Ribeiro
Copy link

Ribeiro commented Oct 9, 2015

@gmsecrieru I´ve tried the meteor restrictCreationByEmailDomain approach but it refuses to accept a function as argument. It only accepts a string as argument. =(

@gmsecrieru
Copy link
Contributor

@Ribeiro please have a look here

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

No branches or pull requests

4 participants