Skip to content

Commit

Permalink
Add better UX
Browse files Browse the repository at this point in the history
  • Loading branch information
iliajie committed Mar 15, 2022
1 parent e45de25 commit f5eccbb
Show file tree
Hide file tree
Showing 4 changed files with 19 additions and 4 deletions.
6 changes: 5 additions & 1 deletion add.cgi
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ $d && &virtual_server::can_edit_domain($d) || &error($text{'index_ecannot'});
my $pub = &virtual_server::public_html_dir($d);
my $cgi = &virtual_server::cgi_bin_dir($d);
my $dir;
my $tdir_escaped = $in{'dir_def'} == 1 ? "<em>".&html_escape($text{'add_all'})."</em>" :
$in{'cgi'} ? "<tt>".&html_escape("$d->{'cgi_bin_dir'}/$in{'cgi'}")."</tt>" :
$in{'dir'} ? "<tt>".&html_escape("$d->{'public_html_dir'}/$in{'dir'}")."</tt>" : "";

if ($in{'dir_def'} == 1) {
# Whole website
$dir = $pub;
Expand Down Expand Up @@ -85,5 +89,5 @@ my $dirstr = [ $dir, $usersfile, 0, 0, undef ];
push(@dirs, $dirstr);
&htaccess_htpasswd::save_directories(\@dirs);

&redirect("index.cgi?dom=$in{'dom'}");
&redirect("index.cgi?dom=$in{'dom'}&added=1&type=".&urlize($tdir_escaped));

4 changes: 2 additions & 2 deletions add_form.cgi
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@ else {
print &ui_table_row(&hlink($text{'add_dir'}, 'add_dir'),
&ui_radio_table("dir_def", 1,
[ [ 1, $text{'add_all'} ],
[ 0, $text{'add_subdir'}, &ui_textbox("dir", undef, 30) ],
[ 2, $text{'add_subcgi'}, &ui_textbox("cgi", undef, 30) ] ]));
[ 0, $text{'add_subdir'}, &ui_textbox("dir", undef, 40, undef, undef, " placeholder=\"$text{'add_tip_under'} @{[&quote_escape($d->{'public_html_dir'})]}/\"") ],
[ 2, $text{'add_subcgi'}, &ui_textbox("cgi", undef, 40, undef, undef, " placeholder=\"$text{'add_tip_under'} @{[&quote_escape($d->{'cgi_bin_dir'})]}/\"") ] ]));

# Authentication realm
print &ui_table_row(&hlink($text{'add_desc'}, 'add_desc'),
Expand Down
10 changes: 9 additions & 1 deletion index.cgi
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,15 @@ my @dirs = &htaccess_htpasswd::list_directories();

&ui_print_header($d ? &virtual_server::domain_in($d) : undef,
$text{'index_title'}, "", "intro", 0, 1);

if ($in{'added'}) {
print &ui_alert_box(
&text("index_alert_added_desc",
"@{[&virtual_server::get_webprefix_safe()]}/virtual-server/list_users.cgi?dom=".&urlize($in{'dom'}),
$in{'type'}
),
"success", undef, 1, $text{'index_alert_added_title'}, 'fa-lock'
);
}
# Build table of directories
my @table = ( );
foreach my $dir (@dirs) {
Expand Down
3 changes: 3 additions & 0 deletions lang/en
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ index_return=protected directories
index_hdir=Entire website
index_find=Find Protected Directories
index_finddesc=Find other directories under $1 that have been protected manually, so that their allowed users can be managed.
index_alert_added_desc=You can control which users have access to $2 using <a href='$1'>Edit Users: Mail and FTP Users</a> page.
index_alert_added_title=Protection Enabled!

feat_name=Protected web directories
feat_edep=The Protected Web Directories module is not installed
Expand Down Expand Up @@ -39,6 +41,7 @@ add_eclash2=The users file $1 already exists
add_eclash3=The protection file $1 already exists and contains the $2 directive
add_edesc=No description entered
add_esymlink=The protection file $1 is a symbolic link
add_tip_under=i.e. under

delete_err=Failed to remove protection
delete_enone=None selected
Expand Down

6 comments on commit f5eccbb

@iliajie
Copy link
Contributor Author

@iliajie iliajie commented on f5eccbb Mar 15, 2022

Choose a reason for hiding this comment

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

Added few improvements to the UI.

  1. Display post creation dialog clearly saying how the users can be added to protected directory and a link for quicker navigation:

image

  1. Display placeholders on inputs for clarity:

image

@fakemoth
Copy link

@fakemoth fakemoth commented on f5eccbb Sep 5, 2022

Choose a reason for hiding this comment

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

Can we improve that? Something like:

  • have any user, and not just append @domain.tld they do not need to be "real" users, in Virtualmin's tradition
  • have the password field right there in the interface, not to go back to user management; maybe check for the directory, if doesn’t exit, create it;
  • have also a field to exclude IPs or whole networks, see bellow;
  • deal with TLS as Let's Encrypt has no longer access there, a big problem - well known should be excluded;
  • write the options in the Apache config file, not .htaccess, as for example one can get "Failed to protect new directory : The protection file /home/domain.tld/public_html/.htaccess is a symbolic link", for example I use (not easily editable by users though, good idea here also to write passwords in a different file outside web root):
    <Location / >
    AuthName "Restricted"
    AuthType Basic
    AuthUserFile /home/domain.tld/.htpasswd

    Require ip X.Y.Z.W/27
    Require valid-user

@iliajie
Copy link
Contributor Author

@iliajie iliajie commented on f5eccbb Sep 5, 2022

Choose a reason for hiding this comment

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

deal with TLS as Let's Encrypt has no longer access there, a big problem - well known should be excluded;

This will do.

have also a field to exclude IPs or whole networks, see bellow;

This may be a good improvement. @jcameron Jamie, what are your thoughts on this?

@fakemoth
Copy link

Choose a reason for hiding this comment

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

Something like:
<Location / >
AuthName "Restricted"
AuthType Basic
AuthUserFile /home/domain.tld/.htpasswd
SetEnvIf Request_URI /.well-known noauth=1

Require env noauth
Require env REDIRECT_noauth
Require ip 1.2.3.4/27
Require valid-user

@jcameron
Copy link
Collaborator

Choose a reason for hiding this comment

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

Sure, I wouldn't object to a feature to also limit a path to certain IPs.

@fakemoth
Copy link

Choose a reason for hiding this comment

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

But can you also move the options from .htaccess to the virtual servers config file? Having a problem with sites that have an .htaccess file and just symlinks to it. Virtualmin of course complains that it can't write the file

Please sign in to comment.