Changeset 294

Timestamp:

03/29/12 13:22:59 (13 years ago)

Author:

Kris Deugau

Message:

/trunk

"Move action logging for change group default permissions" mutated
into "Move action logging for (change group default permissions),
(add user), (update user), (update user permissions)". See #35.

Location:

trunk

Files:

• DNSDB.pm (modified) (12 diffs)
• dns.cgi (modified) (8 diffs)
• templates/user.tmpl (modified) (1 diff)

trunk/DNSDB.pm

@@ -1115 +1115 @@
   my $inherit = shift || 0;
 
-  my $failmsg = '';
+  my $resultmsg = '';
 
   # see if we're switching from inherited to custom.  for bonus points,
@@ -1121 +1121 @@
   # to set/alter custom perms, and both if we're switching from custom to
   # inherited.
-  my $sth = $dbh->prepare("SELECT (u.permission_id=g.permission_id) AS was_inherited,u.permission_id,g.permission_id".
+  my $sth = $dbh->prepare("SELECT (u.permission_id=g.permission_id) AS was_inherited,u.permission_id,g.permission_id,".
+        ($type eq 'user' ? 'u.group_id,u.username' : 'u.parent_group_id,u.group_name').
         " FROM ".($type eq 'user' ? 'users' : 'groups')." u ".
         " JOIN groups g ON u.".($type eq 'user' ? '' : 'parent_')."group_id=g.group_id ".
@@ -1127 +1128 @@
   $sth->execute($id);
 
-  my ($wasinherited,$permid,$parpermid) = $sth->fetchrow_array;
+  my ($wasinherited,$permid,$parpermid,$parid,$name) = $sth->fetchrow_array;
 
 # hack phtoui
@@ -1168 +1169 @@
     } # (inherited->)? custom
 
+    if ($type eq 'user') {
+      $resultmsg = "Updated permissions for user $name";
+    } else {
+      $resultmsg = "Updated default permissions for group $name";
+    }
+    _log($dbh, (group_id => ($type eq 'user' ? $parid : $id), entry => $resultmsg));
     $dbh->commit;
   }; # end eval
@@ -1173 +1180 @@
     my $msg = $@;
     eval { $dbh->rollback; };
-    return ('FAIL',"$failmsg: $msg ($permid)");
-  } else {
-    return ('OK',$permid);
-  }
-
+    return ('FAIL',"Error changing permissions: $msg");
+  }
+
+  return ('OK',$resultmsg);
 } # end changePermissions()
 
@@ -2006 +2012 @@
   local $dbh->{RaiseError} = 1;
 
-  my $failmsg = '';
-
   # Wrap all the SQL in a transaction
   eval {
@@ -2059 +2063 @@
 ##fixme: add another table to hold name/email for log table?
 
+    _log($dbh, (group_id => $group, entry => "Added user $username ($fname $lname)"));
     # once we get here, we should have suceeded.
     $dbh->commit;
@@ -2066 +2071 @@
     my $msg = $@;
     eval { $dbh->rollback; };
-    return ('FAIL',$msg." $failmsg");
-  } else {
-    return ('OK',$user_id);
-  }
+    if ($config{log_failures}) {
+      _log($dbh, (group_id => $group, entry => "Error adding user $username: $msg"));
+      $dbh->commit;     # since we enabled transactions earlier
+    }
+    return ('FAIL',"Error adding user $username: $msg");
+  }
+
+  return ('OK',"User $username ($fname $lname) added");
 } # end addUser
 
@@ -2112 +2121 @@
   my $phone = shift || '';      # not going format-check
 
-  my $failmsg = '';
+  my $resultmsg = '';
 
   # Allow transactions, and raise an exception on errors so we can catch it later.
@@ -2124 +2133 @@
   # Actual blank passwords are bad, mm'kay?
   if (!$pass) {
-    $sth = $dbh->prepare("SELECT password FROM users WHERE user_id=?");
-    $sth->execute($uid);
-    ($pass) = $sth->fetchrow_array;
+    ($pass) = $dbh->selectrow_array("SELECT password FROM users WHERE user_id=?", undef, ($uid));
   } else {
     $pass = unix_md5_crypt($pass);
@@ -2132 +2139 @@
 
   eval {
-    my $sth = $dbh->prepare(q(
-        UPDATE users
-        SET username=?, password=?, firstname=?, lastname=?, phone=?, type=?, status=?
-        WHERE user_id=?
-        )
-      );
-    $sth->execute($username, $pass, $fname, $lname, $phone, $type, $state, $uid);
+    $dbh->do("UPDATE users SET username=?, password=?, firstname=?, lastname=?, phone=?, type=?, status=?".
+        " WHERE user_id=?", undef, ($username, $pass, $fname, $lname, $phone, $type, $state, $uid));
+    $resultmsg = "Updated user info for $username ($fname $lname)";
+    _log($dbh, group_id => $group, entry => $resultmsg);
     $dbh->commit;
   };
@@ -2144 +2148 @@
     my $msg = $@;
     eval { $dbh->rollback; };
-    return ('FAIL',"$failmsg: $msg");
-  } else {
-    return ('OK','OK');
-  }
+    if ($config{log_failures}) {
+      _log($dbh, (group_id => $group, entry => "Error updating user $username: $msg"));
+      $dbh->commit;     # since we enabled transactions earlier
+    }
+    return ('FAIL',"Error updating user $username: $msg");
+  }
+
+  return ('OK',$resultmsg);
 } # end updateUser()
 

trunk/dns.cgi

@@ -1016 +1016 @@
     my ($code,$msg) = changePermissions($dbh, 'group', $webvar{gid}, \%chperms);
     if ($code eq 'OK') {
-      logaction(0, $session->param("username"), $webvar{gid},
-        "Updated default permissions in group $webvar{gid} (".groupName($dbh, $webvar{gid}).")");
       if ($alterperms) {
         changepage(page => "grpman", warnmsg =>
@@ -1023 +1021 @@
                 groupName($dbh, $webvar{gid})." updated with reduced access");
       } else {
-        changepage(page => "grpman", resultmsg =>
-                "Updated default permissions in group ".groupName($dbh, $webvar{gid}));
+        changepage(page => "grpman", resultmsg => $msg);
       }
     } # fallthrough else
-    logaction(0, $session->param("username"), $webvar{gid}, "Failed to update default permissions in group ".
-        groupName($dbh, $webvar{gid}).": $msg")
-        if $config{log_failures};
     # no point in doing extra work
     fill_permissions($page, \%chperms);
@@ -1261 +1255 @@
     $page->param(add => 1) if $webvar{useraction} eq 'add';
 
-    my ($code,$msg);
+    # can't re-use $code and $msg for update if we want to be able to identify separate failure states
+    my ($code,$code2,$msg,$msg2) = ('OK','OK','OK','OK');
 
     my $alterperms = 0; # flag iff we need to force custom permissions due to user's current access limits
@@ -1315 +1310 @@
                 ($webvar{makeactive} eq 'on' ? 1 : 0), $webvar{accttype}, $permstring,
                 $webvar{fname}, $webvar{lname}, $webvar{phone});
-        logaction(0, $session->param("username"), $curgroup, "Added user $webvar{uname} (uid $msg)")
-                if $code eq 'OK';
       } else {
         changepage(page => "useradmin", errmsg => "You do not have permission to edit users")
@@ -1324 +1317 @@
           changepage(page => "useradmin", errmsg => "You do not have permission to edit the requested user");
         }
-# User update is icky.  I'd really like to do this in one atomic
-# operation, but that would duplicate a **lot** of code in DNSDB.pm
+# User update is icky.  I'd really like to do this in one atomic operation,
+# but that gets hairy by either duplicating a **lot** of code in DNSDB.pm
+# or self-torture trying to not commit the transaction until we're really done.
         # Allowing for changing group, but not coding web support just yet.
         ($code,$msg) = updateUser($dbh, $webvar{uid}, $webvar{uname}, $webvar{gid}, $webvar{pass1},
@@ -1332 +1326 @@
         if ($code eq 'OK') {
           $newperms{admin} = 1 if $webvar{accttype} eq 'S';
-          ($code,$msg) = changePermissions($dbh, 'user', $webvar{uid}, \%newperms, ($permstring eq 'i'));
-          logaction(0, $session->param("username"), $curgroup,
-                "Updated uid $webvar{uid}, user $webvar{uname} ($webvar{fname} $webvar{lname})");
+          ($code2,$msg2) = changePermissions($dbh, 'user', $webvar{uid}, \%newperms, ($permstring eq 'i'));
         }
       }
     }
 
-    if ($code eq 'OK') {
-
+    if ($code eq 'OK' && $code2 eq 'OK') {
+      my %pageparams = (page => "useradmin");
       if ($alterperms) {
-        changepage(page => "useradmin", warnmsg =>
-                "You can only grant permissions you hold.  $webvar{uname} ".
-                ($webvar{useraction} eq 'add' ? 'added' : 'updated')." with reduced access.");
+        $pageparams{warnmsg} = "You can only grant permissions you hold.\nUser ".
+                ($webvar{useraction} eq 'add' ? "$webvar{uname} added" : "info updated for $webvar{uname}").
+                ".\nPermissions ".($webvar{useraction} eq 'add' ? 'added' : 'updated')." with reduced access.";
       } else {
-        changepage(page => "useradmin", resultmsg => "Successfully ".
-                ($webvar{useraction} eq 'add' ? 'added' : 'updated')." user $webvar{uname}");
+        $pageparams{resultmsg} = "$msg".($webvar{useraction} eq 'add' ? '' : "\n$msg2");
       }
+      changepage(%pageparams);
 
     # add/update failed:
@@ -1367 +1359 @@
       $page->param(pass1 => $webvar{pass1});
       $page->param(pass2 => $webvar{pass2});
-      $page->param(errmsg => $msg);
+      $page->param(errmsg => "User info updated but permissions update failed: $msg2") if $code eq 'OK';
+      $page->param(errmsg => $msg) if $code ne 'OK';
       fill_permissions($page, \%newperms);
       fill_actypelist($webvar{accttype});
       fill_clonemelist();
-      logaction(0, $session->param("username"), $curgroup, "Failed to $webvar{useraction} user ".
-        "$webvar{uname}: $msg")
-        if $config{log_failures};
     }
 
@@ -1394 +1384 @@
     fill_actypelist($userinfo->{type});
     # not using this yet, but adding it now means we can *much* more easily do so later.
-    $page->param(gid => $webvar{group_id});
+    $page->param(gid => $userinfo->{group_id});
 
     my %curperms;

trunk/templates/user.tmpl

@@ -16 +16 @@
 <table border="0" cellspacing="2" cellpadding="2" width="450">
 <TMPL_IF add_failed>    <tr>
-                <td class="errhead" colspan="2">Error <TMPL_IF add>adding<TMPL_ELSE>updating</TMPL_IF> user <TMPL_VAR NAME=uname>: <TMPL_VAR NAME=errmsg></td>
+                <td class="errhead" colspan="2"><TMPL_VAR NAME=errmsg></td>
         </tr></TMPL_IF>
         <tr class="darkrowheader"><td colspan="2" align="center"><TMPL_IF add>Add<TMPL_ELSE>Edit</TMPL_IF> User</td></tr>