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

Stricter whitelist rules #2213

Merged
merged 6 commits into from Apr 25, 2017
Merged

Stricter whitelist rules #2213

merged 6 commits into from Apr 25, 2017

Conversation

Fiaxhs
Copy link
Contributor

@Fiaxhs Fiaxhs commented Apr 20, 2017

I was browsing an instance where you need a whitelisted email domain, however I figured out you could circumvent the protection by using the whitelisted domain as a subdomain of your email address.

Copy link
Contributor

@mjankowski mjankowski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add a spec showing that blacklist doesn't have the same issue?

Also ... instead of the regex approach here, what if we just extracted the hostname portion of the email and did a (case insensitive?) check that it was in the list?

@Fiaxhs
Copy link
Contributor Author

Fiaxhs commented Apr 20, 2017

spec for blacklist: on it.
We could drop the regexp instance yes, but only for whitelisting then.

@Fiaxhs
Copy link
Contributor Author

Fiaxhs commented Apr 20, 2017

I tried to ditch the whitelist regexp, but it created some important issue in my opinion: you can't use it as wildcard to whitelist subdomains.

The instance I found this issue on is a big institution using institution.tld as top domain for emails and every service within this instituion as a specific subdomain: user@service1.institution.tld, user@service2.institution.tld etc.

By removing the regexp, the admin has the tedious task to list all subdomain of whitelisted subdomains (and add them if they're ever created).

I added a spec for specific subdomain blacklist + top domain whitelist.

Anyway, tell me what's the desired approach :)

@@ -87,5 +103,20 @@
user = User.new(email: 'foo@mastodon.space', account: account, password: password)
expect(user.valid?).to be_truthy
end

it 'should not allow a smart user to be created unless they are whitelisted' do
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of "smart user" could you just describe what is being tested? This will become unclear in the future.

@Gargron Gargron merged commit 7177e37 into mastodon:master Apr 25, 2017
seefood pushed a commit to Toootim/mastodon that referenced this pull request Apr 26, 2017
* Stricter whitelist rules

* Linting

* Added spec for blacklisting

* Test subdomain blacklist on domain whitelist

* No need to split

* Change spec name
seefood pushed a commit to Toootim/mastodon that referenced this pull request Apr 28, 2017
* Stricter whitelist rules

* Linting

* Added spec for blacklisting

* Test subdomain blacklist on domain whitelist

* No need to split

* Change spec name
ClearlyClaire added a commit to ClearlyClaire/mastodon that referenced this pull request May 8, 2023
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

Successfully merging this pull request may close these issues.

None yet

3 participants