From e96015a92caac57254caa10e614f6c67322033a3 Mon Sep 17 00:00:00 2001 From: Jamie Cameron Date: Mon, 30 May 2011 18:09:29 -0700 Subject: [PATCH] Make all user and group links be by name instead of by index. --- useradmin/CHANGELOG | 2 ++ useradmin/cgi_args.pl | 4 ++-- useradmin/delete_group.cgi | 11 +++++------ useradmin/delete_user.cgi | 7 +++---- useradmin/edit_group.cgi | 10 ++++++---- useradmin/edit_user.cgi | 9 +++++---- useradmin/lang/en | 2 ++ useradmin/save_group.cgi | 11 +++++++---- useradmin/save_user.cgi | 17 +++++++++-------- useradmin/search_group.cgi | 2 +- useradmin/search_user.cgi | 2 +- useradmin/user-lib.pl | 4 ++-- 12 files changed, 45 insertions(+), 36 deletions(-) diff --git a/useradmin/CHANGELOG b/useradmin/CHANGELOG index 5f4ee7f38..deec8b5d3 100644 --- a/useradmin/CHANGELOG +++ b/useradmin/CHANGELOG @@ -66,3 +66,5 @@ When deleting a user's personal group and user deletion in other modules is enab Added support for SHA512 format passwords. ---- Changes since 1.540 ---- Fixed an XSS vulnerability that can be triggered if an attacker has the ability to change the real name of a Unix user. +---- Changes since 1.550 ---- +Updated all links to users and groups to be by name instead of by index, to avoid incorrect links if the passwd or group files are changed manually or by another Webmin session. diff --git a/useradmin/cgi_args.pl b/useradmin/cgi_args.pl index 36d885df1..5089f309f 100755 --- a/useradmin/cgi_args.pl +++ b/useradmin/cgi_args.pl @@ -8,12 +8,12 @@ if ($cgi eq 'edit_user.cgi') { # Link to first available user my @allulist = &list_users(); my @ulist = &list_allowed_users(\%access, \@allulist); - return @ulist ? "num=".&urlize($ulist[0]->{'num'}) : "none"; + return @ulist ? "user=".&urlize($ulist[0]->{'user'}) : "none"; } elsif ($cgi eq 'edit_group.cgi') { my @allglist = &list_groups(); my @glist = &list_allowed_groups(\%access, \@allglist); - return @glist ? "num=".&urlize($glist[0]->{'num'}) : "none"; + return @glist ? "group=".&urlize($glist[0]->{'group'}) : "none"; } return undef; } diff --git a/useradmin/delete_group.cgi b/useradmin/delete_group.cgi index cdb34d136..57e742be5 100755 --- a/useradmin/delete_group.cgi +++ b/useradmin/delete_group.cgi @@ -5,8 +5,8 @@ require './user-lib.pl'; &ReadParse(); @glist = &list_groups(); -$group = $glist[$in{'num'}]; -$group || &error($text{'gdel_enum'}); +($group) = grep { $_->{'group'} eq $in{'group'} } @glist; +$group || &error($text{'gedit_egone'}); $| = 1; &error_setup($text{'gdel_err'}); &can_edit_group(\%access, $group) || &error($text{'gdel_egroup'}); @@ -55,7 +55,7 @@ if ($in{'confirmed'}) { print "$text{'gdel_done'}

\n"; done: - &ui_print_footer("", $text{'index_return'}); + &ui_print_footer("index.cgi?mode=groups", $text{'index_return'}); } else { # check if this is anyone's primary group @@ -71,13 +71,12 @@ else { # Ask if the user is sure print &ui_confirmation_form("delete_group.cgi", &text('gdel_sure', $group->{'group'}), - [ [ "num", $in{'num'} ], - [ "group", $group->{'group'} ] ], + [ [ "group", $group->{'group'} ] ], [ [ "confirmed", $text{'gdel_del'} ] ], ui_checkbox("others", 1, $text{'gdel_dothers'}, $config{'default_other'}), ); - &ui_print_footer("", $text{'index_return'}); + &ui_print_footer("index.cgi?mode=groups", $text{'index_return'}); } diff --git a/useradmin/delete_user.cgi b/useradmin/delete_user.cgi index dc1e1f305..20f018ca3 100755 --- a/useradmin/delete_user.cgi +++ b/useradmin/delete_user.cgi @@ -6,8 +6,8 @@ require './user-lib.pl'; &ReadParse(); &lock_user_files(); @ulist = &list_users(); -$user = $ulist[$in{'num'}]; -$user || &error($text{'udel_enum'}); +($user) = grep { $_->{'user'} eq $in{'user'} } @ulist; +$user || &error($text{'uedit_egone'}); &error_setup($text{'udel_err'}); &can_edit_user(\%access, $user) || &error($text{'udel_euser'}); $access{'udelete'} || &error($text{'udel_euser'}); @@ -156,8 +156,7 @@ else { } print &ui_confirmation_form("delete_user.cgi", $msg, - [ [ "num", $in{'num'} ], - [ "user", $user->{'user'} ], + [ [ "user", $user->{'user'} ], [ "confirmed", 1 ] ], \@buts, $access{'dothers'} == 1 ? diff --git a/useradmin/edit_group.cgi b/useradmin/edit_group.cgi index d23e267da..9d2a001e1 100755 --- a/useradmin/edit_group.cgi +++ b/useradmin/edit_group.cgi @@ -6,14 +6,16 @@ require './user-lib.pl'; &ReadParse(); # Get group and show page header -$n = $in{'num'}; +$n = $in{'group'}; if ($n eq "") { $access{'gcreate'} == 1 || &error($text{'gedit_ecreate'}); &ui_print_header(undef, $text{'gedit_title2'}, "", "create_group"); } else { @glist = &list_groups(); - %group = %{$glist[$n]}; + ($ginfo_hash) = grep { $_->{'group'} eq $n } @glist; + $ginfo_hash || &error($text{'gedit_egone'}); + %group = %$ginfo_hash; &can_edit_group(\%access, \%group) || &error($text{'gedit_eedit'}); &ui_print_header(undef, $text{'gedit_title'}, "", "edit_group"); @@ -24,7 +26,7 @@ else { # Start of form print &ui_form_start("save_group.cgi", "post"); -print &ui_hidden("num", $n) if ($n ne ""); +print &ui_hidden("old", $n) if ($n ne ""); print &ui_table_start($text{'gedit_details'}, "width=100%", 2, [ "width=30%" ]); # Group name @@ -99,7 +101,7 @@ if ($n ne "") { foreach $u (@upri) { if (&can_edit_user(\%access, $u)) { push(@uprilinks, "$u->{'user'}"); + "user=$u->{'user'}'>$u->{'user'}"); } else { push(@uprilinks, $u->{'user'}); diff --git a/useradmin/edit_user.cgi b/useradmin/edit_user.cgi index c6708ca82..2d83587f8 100755 --- a/useradmin/edit_user.cgi +++ b/useradmin/edit_user.cgi @@ -7,14 +7,16 @@ require 'timelocal.pl'; &ReadParse(); # Show header and get the user -$n = $in{'num'}; +$n = $in{'user'}; if ($n eq "") { $access{'ucreate'} || &error($text{'uedit_ecreate'}); &ui_print_header(undef, $text{'uedit_title2'}, "", "create_user"); } else { @ulist = &list_users(); - %uinfo = %{$ulist[$n]}; + ($uinfo_hash) = grep { $_->{'user'} eq $n } @ulist; + $uinfo_hash || &error($text{'uedit_egone'}); + %uinfo = %$uinfo_hash; &can_edit_user(\%access, \%uinfo) || &error($text{'uedit_eedit'}); &ui_print_header(undef, $text{'uedit_title'}, "", "edit_user"); } @@ -37,8 +39,7 @@ if ($shells{'shells'}) { # Start of the form print &ui_form_start("save_user.cgi", "post"); -print &ui_hidden("num", $n) if ($n ne ""); -print &ui_hidden("old", $uinfo{'user'}) if ($n ne ""); +print &ui_hidden("old", $n) if ($n ne ""); print &ui_table_start($text{'uedit_details'}, "width=100%", 2, \@tds); # Username diff --git a/useradmin/lang/en b/useradmin/lang/en index 7486a2e88..dac5b03d4 100644 --- a/useradmin/lang/en +++ b/useradmin/lang/en @@ -111,6 +111,7 @@ uedit_logins=Show Logins uedit_mail=Read Email uedit_swit=Login to Usermin uedit_ecreate=You cannot create new users +uedit_egone=Selected user no longer exists! uedit_eedit=You cannot edit this user uedit_admin=Only root can change password uedit_admchg=User must choose new password @@ -188,6 +189,7 @@ gedit_oneperline=(One per line) gedit_homedirs=Home directories gedit_allfiles=All files gedit_ecreate=You cannot create new groups +gedit_egone=Selected group no longer exists! gedit_eedit=You cannot edit this group gedit_cothers=Create group in other modules? gedit_mothers=Modify group in other modules? diff --git a/useradmin/save_group.cgi b/useradmin/save_group.cgi index 61cd39c2b..c90a75713 100755 --- a/useradmin/save_group.cgi +++ b/useradmin/save_group.cgi @@ -9,7 +9,7 @@ require 'timelocal.pl'; if ($in{'delete'}) { # Redirect to deletion page - &redirect("delete_group.cgi?num=$in{'num'}"); + &redirect("delete_group.cgi?group=$in{'old'}"); return; } @@ -24,9 +24,12 @@ $in{'gid'} =~ s/\r|\n//g; &lock_user_files(); @glist = &list_groups(); -if ($in{'num'} ne "") { +if ($in{'old'} ne "") { # get old group - %ogroup = %{$glist[$in{'num'}]}; + @glist = &list_groups(); + ($ginfo_hash) = grep { $_->{'group'} eq $in{'old'} } @glist; + $ginfo_hash || &error($text{'gedit_egone'}); + %ogroup = %$ginfo_hash; $group{'group'} = $ogroup{'group'}; &can_edit_group(\%access, \%ogroup) || &error($text{'gsave_eedit'}); } @@ -43,7 +46,7 @@ else { } # Validate and save inputs -if (!$in{'gid_def'} || $in{'num'} ne '') { +if (!$in{'gid_def'} || $in{'old'} ne '') { # Only do GID checks if not automatic $in{'gid'} =~ /^[0-9]+$/ || &error(&text('gsave_egid', $in{'gid'})); !$access{'lowgid'} || $in{'gid'} >= $access{'lowgid'} || diff --git a/useradmin/save_user.cgi b/useradmin/save_user.cgi index 9c07002ea..628766c85 100755 --- a/useradmin/save_user.cgi +++ b/useradmin/save_user.cgi @@ -22,8 +22,7 @@ elsif ($in{'switch'}) { return; } elsif ($in{'delete'}) { - &redirect("delete_user.cgi?user=".&urlize($in{'old'}). - "&num=".$in{'num'}); + &redirect("delete_user.cgi?user=".&urlize($in{'old'})); return; } @@ -54,9 +53,11 @@ $err = &check_username_restrictions($in{'user'}); &lock_user_files(); @ulist = &list_users(); @glist = &list_groups(); -if ($in{'num'} ne "") { +if ($in{'old'} ne "") { # Get old user info - %ouser = %{$ulist[$in{'num'}]}; + ($ouser_hash) = grep { $_->{'user'} eq $in{'old'} } @ulist; + $ouser_hash || &error($text{'uedit_egone'}); + %ouser = %$ouser_hash; if (!$access{'urename'} && $ouser{'user'} ne $user{'user'}) { &error($text{'usave_erename'}); } @@ -79,7 +80,7 @@ else { if ($ou->{'user'} eq $in{'user'}); } } -if (($in{'num'} eq '' || $user{'user'} ne $ouser{'user'}) && +if (($in{'old'} eq '' || $user{'user'} ne $ouser{'user'}) && $config{'alias_check'} && &foreign_check("sendmail")) { # Check if the new username conflicts with a sendmail alias &foreign_require("sendmail", "sendmail-lib.pl"); @@ -93,7 +94,7 @@ if (($in{'num'} eq '' || $user{'user'} ne $ouser{'user'}) && } # Validate and store basic inputs -if (!$in{'uid_def'} || $in{'num'} ne '') { +if (!$in{'uid_def'} || $in{'old'} ne '') { # Only do UID checks if not automatic $in{'uid'} =~ /^\-?[0-9]+$/ || &error(&text('usave_euid', $in{'uid'})); if (!%ouser || $ouser{'uid'} != $in{'uid'}) { @@ -154,7 +155,7 @@ if ($access{'shells'} ne "*") { } } $user{'uid'} = $in{'uid'}; -if ($in{'num'} ne "" || !$in{'gidmode'}) { +if ($in{'old'} ne "" || !$in{'gidmode'}) { # Selecting existing group $user{'gid'} = &my_getgrnam($in{'gid'}); if ($user{'gid'} eq "") { &error(&text('usave_egid', $in{'gid'})); } @@ -239,7 +240,7 @@ foreach $gname (@sgnames) { } if ($access{'ugroups'} ne "*") { - if ($in{'num'} ne "") { + if ($in{'old'} ne "") { # existing users can only be added to or removed from # allowed groups if ($ouser{'gid'} != $user{'gid'}) { diff --git a/useradmin/search_group.cgi b/useradmin/search_group.cgi index b2af15c7a..347331ded 100755 --- a/useradmin/search_group.cgi +++ b/useradmin/search_group.cgi @@ -22,7 +22,7 @@ for($i=0; $i<@glist; $i++) { } } if (@match == 1) { - &redirect("edit_group.cgi?num=".$match[0]->{'num'}); + &redirect("edit_group.cgi?group=".$match[0]->{'group'}); } else { &ui_print_header(undef, $text{'search_title'}, ""); diff --git a/useradmin/search_user.cgi b/useradmin/search_user.cgi index bcbd4f496..bb87e8697 100755 --- a/useradmin/search_user.cgi +++ b/useradmin/search_user.cgi @@ -42,7 +42,7 @@ for($i=0; $i<@ulist; $i++) { } } if (@match == 1) { - &redirect("edit_user.cgi?num=".$match[0]->{'num'}); + &redirect("edit_user.cgi?user=".$match[0]->{'user'}); } else { &ui_print_header(undef, $text{'search_title'}, ""); diff --git a/useradmin/user-lib.pl b/useradmin/user-lib.pl index facf25766..b5e1ee941 100755 --- a/useradmin/user-lib.pl +++ b/useradmin/user-lib.pl @@ -2463,7 +2463,7 @@ elsif ($_[0]->{'dn'}) { "$dis"; } else { - return "". + return "". "$dis"; } } @@ -2483,7 +2483,7 @@ elsif ($_[0]->{'dn'}) { &html_escape($_[0]->{'group'}).""; } else { - return "". + return "". &html_escape($_[0]->{'group'}).""; } }