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

Added check for hide_admin_url config item #175

Merged
merged 5 commits into from Oct 24, 2014

Conversation

Perturbatio
Copy link
Contributor

Added check for display_admin_url config item which if not
set to 1 will prevent the server from revealing the admin URL
when the user visits the incorrect domain
(more useful in combination with the musthost config item)

This allows the user to lock the admin to a specific domain and
prevent the server revealing that domain to the public

Added check for display_admin_url config item which if not 
set to 1 will prevent the server from revealing the admin URL
when the user visits the incorrect domain
(more useful in combination with the musthost config item)
The default value for this is 1 (to preserve backward
compatibility).
@jcameron
Copy link
Collaborator

Looks like this new option makes the new behavior the default. Can you invert this so that Webmin behaves as it does now unless some new option is set (like hide_admin_url) ?

I've inverted the comparison of display_admin_url so 
that it must be set to 0 in order to disable it
@Perturbatio
Copy link
Contributor Author

I've updated the code now, do I need to resubmit the pull request?

@jcameron
Copy link
Collaborator

Looks better, but you should also remove the default setting from Perturbatio@4b59012

@Perturbatio
Copy link
Contributor Author

That default setting should actually allow normal activity, I just thought it'd be more intuitive to a user when editing the config if the value was already there

@jcameron
Copy link
Collaborator

Wait, I think you got mixed up by my previous suggestion. The new config option should be called "hide_admin_url", and should have a default of zero - which means it could be left out of the default config block entirely.

changed display_admin_url to hide_admin_url and 
changed comparator to test if this value is not set to 1
@Perturbatio
Copy link
Contributor Author

ok, updated again :)

On 24 October 2014 15:26, Jamie Cameron notifications@github.com wrote:

Wait, I think you got mixed up by my previous suggestion. The new config
option should be called "hide_admin_url", and should have a default of zero

  • which means it could be left out of the default config block entirely.


Reply to this email directly or view it on GitHub
#175 (comment).

@Perturbatio Perturbatio changed the title Added check for display_admin_url config item Added check for hide_admin_url config item Oct 24, 2014
jcameron added a commit that referenced this pull request Oct 24, 2014
Added check for hide_admin_url config item
@jcameron jcameron merged commit 6b02088 into webmin:master Oct 24, 2014
@jcameron
Copy link
Collaborator

Thanks!

@Perturbatio Perturbatio deleted the develop branch October 30, 2014 19:20
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

2 participants