sudo-su
sudo-su copied to clipboard
viacreative
TLD Restrictions don't make sense
While the goal of restricting default access is admirable, the current implementation is technically a gaping hole for possible problems. First, .dev is a registered top-level domain. .local could be used in the future as a generic TLD. It is not reserved.
It would be best to have the default allowed TLD's those defined in RFC 2606 as reserved. Meaning they can't be registered for public use on the web. Further extended that list, to still allow functionality on the localhost{?:port} origin as well since localhost itself is also reserved for non-web-routable work as well.
Thank you for your time, -Garbee
Very good point, thanks for pointing this out. Perhaps a better alternative would be to have the developer provide an array of hosts (e.g. foobar.dev) in the config, rather than TLDs. What are your thoughts on this @Garbee?
This way, in order to enable the package in production, they would have to consciously enter the production website URL into the config. I don't see how this can be done accidentally!
Sorry for entering the discussion but I just wanted to give my views. I think idea of having a config for allowed hosts is good but by default you should support some defaults like - localhost, 127.0.0.1 etc. And BTW this extension is very helpful.
@asachanfbd No need to apologise, your views are very helpful! And thanks for the kind words!
I agree, I think the allowed hosts is a good approach, and we can include some defaults. I'll see about implementing this in the next few days. If somebody wants to PR in the meantime, please feel free -- that'd be absolutely awesome!
Yea, doing the actual hosts is a far stricter (and imo more appropriate) setup.
Perhaps while working on this logic again port controls could also be introduced. Let's say you put foobar.dev:8342 into the config, this would then only enable the tool if foobar.dev:8342 is used to access the app. foobar.dev on normal port 80 or 443 wouldn't trigger it. And simply omitting the port in the config will let the tool operate on any given port so long as the domain name matches.
Just a mostly-simple addition that will make the configuration implementation as robust as possible. Because, some companies do quirky things internally to get to different stages of an app.
Agreed @Garbee, thanks for bringing this to my attention! Port restrictions are also a good option.
Honestly, I think restricting based on TLD or url accessing the app is out of the scope of this packages job. What if I want to put this actually in production, who says I can't?
If you absolutely have to, I'd just check to make sure the app is not in production mode, and disable if it is, but make this configurable so it can be turned off.
People can put their production URLs in just fine if they wish that. I think it's nicer to have it built into the config somehow compared to instructing people how to setup their apps to scope access on their own. Then you aren't supporting people who don't understand that aspect of things.
I could probably make a draft of the changes proposed if we all agree on what it should contain.
- Port restrictions
- Change of default TLD's
- Specific domain restrictions
Anything I missed?
I think we were gonna replace the TLD restrictions completely with the specific domain restrictions?
So something like this?
// Valid domain restrictions
'foobar.dev',
'localhost',
'localhost:8080',
'foobar.dev:1234'
Perhaps allow wildcards?
*.dev