usmannasir / cyberpanel

Cyber Panel - The hosting control panel for OpenLiteSpeed
GNU General Public License v3.0
1.6k stars 613 forks source link

creating site even with localhost #602

Closed usmannasir closed 3 years ago

usmannasir commented 3 years ago

https://github.com/usmannasir/cyberpanel/blob/aa697332c5f4a2ae51f94afce68ccc808f585153/cyberpanel.sh#L1788

@qtwrk this line is creating site even for localhost, can you double check this please.

usmannasir commented 3 years ago

@whattheserver

usmannasir commented 3 years ago

centos-s-1vcpu-1gb-intel-fra1-01 resolves to valid IP. Setting up hostname SSL

meramsey commented 3 years ago

I added that line @usmannasir .

Well this "centos-s-1vcpu-1gb-intel-fra1-01" is not a valid hostname. This apparently still passes the localhost check I'm guessing due to there being a hosts file entry for this bad value?

So yes it creates a bad value due to there being a bad value that slips through the hostname check.

We can take that out, but not having set a valid hostname before running the installer is going to cause tons of other issues such as all the email configs not being set right either.... which is going to cause those to fail still even if we remove the above.... which is not a cyberpanel installer issue but a failure of setting up these values beforehand.

The reason for that line is so that for those who are properly setting or ensuring they have a valid hostname before installing don't have to jump through hoops with invalid SSL warnings to get into the admin panel the first time.

If there is a better way to accomplish that im all for it. I don't see the point in removing that line entirely though. If you have bad data and the installer is working with bad data your going to have bad results this just makes it more obvious when someone has mis-configured or not configured their system at all in regards to the hostname.

Feel free to slack me if youd like but i don't see this as an issue. At best we can try to find another way to validate the hostname to try to minimize it attempting this on invalid local hostname values.

If someone sets a valid hostname

usmannasir commented 3 years ago

I will try to add this line at the end of install.py and check for valid hostname using a domain name. Or is there any option to pass on to dig to bypass local hosts file

meramsey commented 3 years ago

So after looking at this again I realized i put the wrong conditional for the check. I forgot to put the -z conditional back to -n which is why the hostname slipped through as the logic I committed was backwards which is so :facepalm: on my part so I apologize for that and going to fix that.

I'm also incorporating your suggestion about having it do the dig via an external DNS server.

When testing even with it in a local hosts file this seems to be pretty effective

root@cyberpanel:~# grep centos-s-1vcpu-1gb-intel-fra1-01 /etc/hosts
127.0.1.1 cyberpanel.whattheserver.com cyberpanel centos-s-1vcpu-1gb-intel-fra1-01
127.0.0.1 localhost cyberpanel.whattheserver.com centos-s-1vcpu-1gb-intel-fra1-01

Testing the proper thing conditional with that fix

root@cyberpanel:~# HostName='centos-s-1vcpu-1gb-intel-fra1-01';
root@cyberpanel:~# if [ -z "$(dig @1.1.1.1 +short "$HostName")" ]; then echo "$HostName does Not Resolve"; elif [ -n "$(dig @1.1.1.1 +short "$HostName")" ]; then echo "$HostName Does Resolve"; fi
centos-s-1vcpu-1gb-intel-fra1-01 does Not Resolve
root@cyberpanel:~#

Once again my apologies @usmannasir that was my mistake.

meramsey commented 3 years ago

Closing this now as it should be resolved now as stable/2.1.1/2.1.1-dev have all had the fix applied Other commits for reference. https://github.com/usmannasir/cyberpanel/commit/ae5eafff8e1747804fbc5fbb91e60822daebff1d https://github.com/usmannasir/cyberpanel/commit/477455b36bb9df1d5e4c33372afa93b2082f7a58