From 0a3ba8aca91622feaf6f690fa1ebe266ef653bc7 Mon Sep 17 00:00:00 2001
From: JensHoffmann <jens.hoffmann@cuesoft.eu>
Date: Wed, 11 Dec 2013 22:40:01 +0100
Subject: [PATCH] Bug #2503: Improve setting of password/auth_driver

UserChangePassword followed this strategy:
  a1. Eventually encrypt user password if the users
      auth_driver is CORE_AUTH
  a2. Set (probably encrypted) password with
      User::set_password
  a3. User::set_password tries to validate (probably
      encrypted) password instead of the raw password

UserChangeAuth did something similar:
  b1. If password is given (not empty) do a1 through a3
  b2. Set auth_driver

The change proposes the following:
 * In set_password:
    1. Validate the raw password
    2. Do encryption if needed
 * In UserChangePassword: simply call set_password
 * In UserChangeAuth: set auth_driver *before* calling
   set_password, such that set_password does the right thing
   if the auth_driver changes

Note: I needed to move the implementation of set_password
from User.h to User.cc since it seems impossible to access
UserPool::CORE_AUTH from within User.h.
---
 include/User.h               | 18 ++----------------
 src/rm/RequestManagerUser.cc | 21 +++------------------
 src/um/User.cc               | 26 ++++++++++++++++++++++++++
 3 files changed, 31 insertions(+), 34 deletions(-)

diff --git a/include/User.h b/include/User.h
index 58d170f3a5..38c105d421 100644
--- a/include/User.h
+++ b/include/User.h
@@ -21,6 +21,7 @@
 #include "UserTemplate.h"
 #include "ObjectCollection.h"
 #include "QuotasSQL.h"
+#include "NebulaUtil.h"
 
 class UserQuotas;
 
@@ -112,22 +113,7 @@ public:
      *    @param error_str Returns the error reason, if any
      *    @returns -1 if the password is not valid
      */
-    int set_password(const string& passwd, string& error_str)
-    {
-        int rc = 0;
-
-        if (pass_is_valid(passwd, error_str))
-        {
-            password = passwd;
-            invalidate_session();
-        }
-        else
-        {
-            rc = -1;
-        }
-
-        return rc;
-    };
+    int set_password(const string& passwd, string& error_str);
 
     /**
      *  Returns user password
diff --git a/src/rm/RequestManagerUser.cc b/src/rm/RequestManagerUser.cc
index 03ab3f6316..e6ffa49635 100644
--- a/src/rm/RequestManagerUser.cc
+++ b/src/rm/RequestManagerUser.cc
@@ -15,7 +15,6 @@
 /* -------------------------------------------------------------------------- */
 
 #include "RequestManagerUser.h"
-#include "NebulaUtil.h"
 
 using namespace std;
 
@@ -70,11 +69,6 @@ int UserChangePassword::user_action(int     user_id,
         return -1;
     }
 
-    if (user->get_auth_driver() == UserPool::CORE_AUTH)
-    {
-        new_pass = one_util::sha1_digest(new_pass);
-    }
-
     int rc = user->set_password(new_pass, error_str);
 
     if ( rc == 0 )
@@ -125,22 +119,13 @@ int UserChangeAuth::user_action(int     user_id,
         return -1;
     }
 
-    if ( !new_pass.empty() )
-    {
-        if ( new_auth == UserPool::CORE_AUTH)
-        {
-            new_pass = one_util::sha1_digest(new_pass);
-        }
+    rc = user->set_auth_driver(new_auth, error_str);
 
-        // The password may be invalid, try to change it first
+    if ( rc == 0 && !new_pass.empty() )
+    {
         rc = user->set_password(new_pass, error_str);
     }
 
-    if ( rc == 0 )
-    {
-        rc = user->set_auth_driver(new_auth, error_str);
-    }
-
     if ( rc == 0 )
     {
         pool->update(user);
diff --git a/src/um/User.cc b/src/um/User.cc
index 6376d9bb0d..77e68ccc0a 100644
--- a/src/um/User.cc
+++ b/src/um/User.cc
@@ -338,6 +338,32 @@ int User::split_secret(const string secret, string& user, string& pass)
 /* -------------------------------------------------------------------------- */
 /* -------------------------------------------------------------------------- */
 
+int User::set_password(const string& passwd, string& error_str)
+{
+    int rc = 0;
+
+    if (pass_is_valid(passwd, error_str))
+    {
+        if (auth_driver == UserPool::CORE_AUTH)
+        {
+            password = one_util::sha1_digest(passwd);
+        }
+        else
+        {
+            password = passwd;
+        }
+
+        invalidate_session();
+    }
+    else
+    {
+        rc = -1;
+    }
+
+    return rc;
+};
+
+
 bool User::pass_is_valid(const string& pass, string& error_str)
 {
     if ( pass.empty() )