Variety of changes to the way we do user editing:

1) Allow admins to edit the admin bit of other admins
2) Don't allow admins to delete themselves (partial fix for ticket #213)
3) Inline user::update().  Don't do form processing in helper methods!
4) Inline user::_get_edit_form() so that we can treat edit forms differently.
   Trying to hard to make common functions makes for weird edge cases.

4 files changed, 69 insertions, 47 deletions

diff --git a/modules/user/controllers/admin_users.php b/modules/user/controllers/admin_users.php
index a4491a71..3ea6c2a5 100644
--- a/modules/user/controllers/admin_users.php
+++ b/modules/user/controllers/admin_users.php
@@ -41,6 +41,7 @@ class Admin_Users_Controller extends Controller {
       $user = user::create(
         $name, $form->add_user->full_name->value, $form->add_user->password->value);
       $user->email = $form->add_user->email->value;
+      $user->admin = $form->add_user->admin->checked;
 
       if ($form->add_user->locale) {
         $desired_locale = $form->add_user->locale->value;
@@ -62,6 +63,10 @@ class Admin_Users_Controller extends Controller {
 
   public function delete_user($id) {
     access::verify_csrf();
+    if ($id == user::active()->id) {
+      access::forbidden();
+    }
+
     $user = ORM::factory("user", $id);
     if (!$user->loaded) {
       kohana::show_404();
@@ -100,10 +105,37 @@ class Admin_Users_Controller extends Controller {
     $form = user::get_edit_form_admin($user);
     $valid = $form->validate();
     if ($valid) {
-      $valid = user::update($user, $form);
+      $new_name = $form->edit_user->inputs["name"]->value;
+      if ($new_name != $user->name &&
+          ORM::factory("user")
+          ->where("name", $new_name)
+          ->where("id !=", $user->id)
+          ->find()
+          ->loaded) {
+        $form->edit_user->inputs["name"]->add_error("in_use", 1);
+        $valid = false;
+      } else {
+        $user->name = $new_name;
+      }
     }
 
     if ($valid) {
+      $user->full_name = $form->edit_user->full_name->value;
+      if ($form->edit_user->password->value) {
+        $user->password = $form->edit_user->password->value;
+      }
+      $user->email = $form->edit_user->email->value;
+      if ($form->edit_user->locale) {
+        $desired_locale = $form->edit_user->locale->value;
+        $user->locale = $desired_locale == "none" ? null : $desired_locale;
+      }
+
+      // An admin can change the admin status for any user but themselves
+      if ($user->id != user::active()->id) {
+        $user->admin = $form->edit_user->admin->checked;
+      }
+      $user->save();
+
       message::success(t("Changed user %user_name", array("user_name" => $user->name)));
       print json_encode(array("result" => "success"));
     } else {
@@ -118,7 +150,12 @@ class Admin_Users_Controller extends Controller {
       kohana::show_404();
     }
 
-    print user::get_edit_form_admin($user);
+    $form = user::get_edit_form_admin($user);
+    // Don't allow the user to control their own admin bit, else you can lock yourself out
+    if ($user->id == user::active()->id) {
+      $form->edit_user->admin->disabled(1);
+    }
+    print $form;
   }
 
   public function add_user_to_group($user_id, $group_id) {
diff --git a/modules/user/controllers/users.php b/modules/user/controllers/users.php
index 55970ded..7f62a931 100644
--- a/modules/user/controllers/users.php
+++ b/modules/user/controllers/users.php
@@ -28,10 +28,17 @@ class Users_Controller extends REST_Controller {
     $form = user::get_edit_form($user);
     $valid = $form->validate();
     if ($valid) {
-      $valid = user::update($user, $form);
-    }
+      $user->full_name = $form->edit_user->full_name->value;
+      if ($form->edit_user->password->value) {
+        $user->password = $form->edit_user->password->value;
+      }
+      $user->email = $form->edit_user->email->value;
+      if ($form->edit_user->locale) {
+        $desired_locale = $form->edit_user->locale->value;
+        $user->locale = $desired_locale == "none" ? null : $desired_locale;
+      }
+      $user->save();
 
-    if ($valid) {
       print json_encode(
         array("result" => "success",
               "resource" => url::site("users/{$user->id}")));
diff --git a/modules/user/helpers/user.php b/modules/user/helpers/user.php
index 8bb9dc98..570f6282 100644
--- a/modules/user/helpers/user.php
+++ b/modules/user/helpers/user.php
@@ -25,15 +25,22 @@
  */
 class user_Core {
   static function get_edit_form($user) {
-    return self::_get_edit_form($user, "users/$user->id?_method=put", t("Save"));
+    $form = new Forge("users/$user->id?_method=put", "", "post", array("id" => "gEditUserForm"));
+    $group = $form->group("edit_user")->label(t("Edit User: %name", array("name" => $user->name)));
+    $group->input("full_name")->label(t("Full Name"))->id("gFullName")->value($user->full_name);
+    self::_add_locale_dropdown($group, $user);
+    $group->password("password")->label(t("Password"))->id("gPassword");
+    $group->password("password2")->label(t("Confirm Password"))->id("gPassword2")
+      ->matches($group->password);
+    $group->input("email")->label(t("Email"))->id("gEmail")->value($user->email);
+    $group->input("url")->label(t("URL"))->id("gUrl")->value($user->url);
+    $group->submit("")->value(t("Save"));
+    $form->add_rules_from($user);
+    return $form;
   }
 
   static function get_edit_form_admin($user) {
-    return self::_get_edit_form($user, "admin/users/edit_user/$user->id", t("Modify User"));
-  }
-
-  private static function _get_edit_form($user, $action, $save_text) {
-    $form = new Forge($action, "", "post", array("id" => "gEditUserForm"));
+    $form = new Forge("admin/users/edit_user/$user->id", "", "post", array("id" => "gEditUserForm"));
     $group = $form->group("edit_user")->label(t("Edit User"));
     $group->input("name")->label(t("Name"))->id("gName")->value($user->name);
     $group->inputs["name"]->error_messages(
@@ -45,7 +52,8 @@ class user_Core {
       ->matches($group->password);
     $group->input("email")->label(t("Email"))->id("gEmail")->value($user->email);
     $group->input("url")->label(t("URL"))->id("gUrl")->value($user->url);
-    $group->submit("")->value($save_text);
+    $group->checkbox("admin")->label(t("Admin"))->id("gAdmin")->checked($user->admin);
+    $group->submit("")->value(t("Modify User"));
     $form->add_rules_from($user);
     $form->edit_user->password->rules("-required");
     return $form;
@@ -54,9 +62,8 @@ class user_Core {
   static function get_add_form_admin() {
     $form = new Forge("admin/users/add_user", "", "post", array("id" => "gAddUserForm"));
     $group = $form->group("add_user")->label(t("Add User"));
-    $group->input("name")->label(t("Name"))->id("gName");
-    $group->inputs["name"]->error_messages(
-      "in_use", t("There is already a user with that name"));
+    $group->input("name")->label(t("Name"))->id("gName")
+      ->error_messages("in_use", t("There is already a user with that name"));
     $group->input("full_name")->label(t("Full Name"))->id("gFullName");
     $group->password("password")->label(t("Password"))->id("gPassword");
     $group->password("password2")->label(t("Confirm Password"))->id("gPassword2")
@@ -64,6 +71,7 @@ class user_Core {
     $group->input("email")->label(t("Email"))->id("gEmail");
     $group->input("url")->label(t("URL"))->id("gUrl");
     self::_add_locale_dropdown($group);
+    $group->checkbox("admin")->label(t("Admin"))->id("gAdmin");
     $group->submit("")->value(t("Add User"));
     $user = ORM::factory("user");
     $form->add_rules_from($user);
@@ -312,34 +320,4 @@ class user_Core {
     }
     return $salt . md5($salt . $password);
   }
-
-  /**
-   *
-   */
-  static function update($user, $form) {
-    $valid = true;
-    $new_name = $form->edit_user->inputs["name"]->value;
-    if ($new_name != $user->name &&
-        ORM::factory("user")
-               ->where("name", $new_name)
-               ->where("id !=", $user->id)
-               ->find()
-               ->loaded) {
-      $form->edit_user->inputs["name"]->add_error("in_use", 1);
-      $valid = false;
-    } else {
-      $user->name = $new_name;
-      $user->full_name = $form->edit_user->full_name->value;
-      if ($form->edit_user->password->value) {
-        $user->password = $form->edit_user->password->value;
-      }
-      $user->email = $form->edit_user->email->value;
-      if ($form->edit_user->locale) {
-        $desired_locale = $form->edit_user->locale->value;
-        $user->locale = $desired_locale == "none" ? null : $desired_locale;
-      }
-      $user->save();
-   }
-    return $valid;
-  }
 }
\ No newline at end of file
diff --git a/modules/user/models/user.php b/modules/user/models/user.php
index 7299a36b..432d86fb 100644
--- a/modules/user/models/user.php
+++ b/modules/user/models/user.php
@@ -21,10 +21,10 @@ class User_Model extends ORM {
   protected $has_and_belongs_to_many = array("groups");
 
   var $rules = array(
-    "name" => "required|length[1,32]",
+    "name" => "length[1,32]",
     "full_name" => "length[0,255]",
     "email" => "valid_email|length[1,255]",
-    "password" => "required|length[1,40]",
+    "password" => "length[1,40]",
     "locale" => "length[2,10]");
 
   public function __set($column, $value) {