[drupal-devel] [bug] Moving user_module_invoke after retrieving roles from database

[moshe weitzman]

Issue status update for http://drupal.org/node/20058

 Project:      Drupal
 Version:      cvs
 Component:    user.module
 Category:     bug reports
 Priority:     normal
 Assigned to:  Anonymous
 Reported by:  kees
 Updated by:   moshe weitzman
 Status:       patch

I like this too. Dries has exopressed concern that modules will
misbehave and ruin security by changing roles for a user. This is
always possible with hook_block(), hook_menu() or many other places.
You simply have to trust the modules that you run on your site. They
can do whatever they want.




moshe weitzman



Previous comments:
------------------------------------------------------------------------

April 6, 2005 - 06:06 : kees

Hi,


To be able to update the $user object from a module, the _user('load')
hook must be called AFTER loading the current roles from the database,
and not before. It is simply a matter of moving one line a few lines
down. Here is the unified patch:


--- drupal-4.5.2-hyper/modules/user.module      2005-04-01
10:16:15.000000000 +0200
+++ drupal-4.5.2/modules/user.module    2005-01-05 21:17:33.000000000
+0100
@@ -69,6 +69,7 @@
   if (db_num_rows($result)) {
     $user = db_fetch_object($result);
     $user = drupal_unpack($user);
+    user_module_invoke('load', $array, $user);
     $user->roles = array();
     $result = db_query('SELECT r.rid, r.name FROM {role} r INNER
JOIN {users_roles} ur ON ur.rid = r.rid WHERE ur.uid = %d',
$user->uid);
@@ -79,7 +80,6 @@
   else {
     $user = new StdClass();
   }
-  user_module_invoke('load', $array, $user);
   return $user;
 }
@@ -1191,7 +1191,6 @@
     $op = arg(2) ? arg(2) : arg(1);
   }
-
   switch ($op) {
     case t('E-mail new password'):
     case 'password':





------------------------------------------------------------------------

April 6, 2005 - 07:47 : Prometheus6

Your statement makes sense but I think the patch has it backward.
--- drupal-4.5.2-hyper/modules/user.module 2005-04-01
10:16:15.000000000 +0200
+++ drupal-4.5.2/modules/user.module 2005-01-05 21:17:33.000000000
+0100
It's deleting stuff from the newer file and adding stuff from the older
file.




------------------------------------------------------------------------

April 6, 2005 - 09:16 : chx

Attachment: http://drupal.org/files/issues/user_load_invoke.patch (887 bytes)

Here is a cvs diff against HEAD. Please note that I have moved the
user_invoke after loading the roles but NOT after the if. I think I
like this change.