[Phpgroupware-tracker] [bug #4260] Multiple bugs when updating a user data & modifying user access to applications

[nobody]

=================== BUG #4260: FULL BUG SNAPSHOT ===================
http://savannah.gnu.org/bugs/?func=detailbug&bug_id=4260&group_id=509

Submitted by: weaky                   Project: phpGroupWare                 
Submitted on: Thu 07/10/2003 at 01:45
Category:  API - Admin                Bug Group:  devel cvs                 
Severity:  5 - Major                  Priority:  High                       
Resolution:  None                     Assigned to:  None                    
Status:  Open                         Component Version:  None              
Platform Version:  Linux - RedHat     Reproducibility:  Every Time          

Summary:  Multiple bugs when updating a user data & modifying user access to 
applications

Original Submission:  (Note - I would have submitted a patch but wasn't sure of 
the implications of the suggested fix(es) so i'll just submit it as a big bug 
report :p)

- New HEAD install.
- Used setup script to set up new admin user. 
- Logged in.
- Used admin -> edit user to attempt to add permissions / modify a users 
details.
- Click update.

Initially came across a validation error, saying 'That loginid has already been 
taken'.
- Tracked this to class.boaccounts.inc.php, validate_user() function (approx 
line 600). It appears 'old_loginid' used to be past as part of the POST data 
but for some reason is now being passed through the querystring, and is 
therefore not being passed in the $values array to the validate_user() 
function. 

Suggested solution:
Either:
a) pass old_loginid as part of the POST data (if this won't impact other 
areas..). Note if you do this the save_user() function will need to be updated 
(approx line 397). Not sure if this will impact other areas - i presume it was 
put into the querystring in the first place for a reason...
b) modify the 'if' check on approx line 600 to include a check for:
 $values['account_lid'] != get_var('old_loginid',array('GET'))

This will get past the validation check. However the save_user function still 
needs updating due to another bug on approx line 397: 

'old_loginid'           => 
($values['old_loginid']?rawurldecode($GLOBALS['HTTP_GET_VARS']['old_loginid']):''),

needs to be:

'old_loginid'           => 
($GLOBALS['HTTP_GET_VARS']['old_loginid']?rawurldecode($GLOBALS['HTTP_GET_VARS']['old_loginid']):''),

... or something similar.

Anyway - implementing either a or b above will get past the validation. However 
now the submission is processed and the user is presented with an 'access 
denied' error. This is due to all their acl being deleted - also their user 
details are also deleted due to a separate bug. Dealing with the user details 
bug first:

---------------------
This appears that someone is in the middle of modifying the phpgwapi accounts 
classes. class.accounts_sql.inc.php currently has half the functions referring 
to accounts data fields as 'account_<fieldname>' (e.g. account_firstname) (e.g. 
the create() function) and the other half using data fields as without the 
account_ prefix (e.g. firstname) (e.g. the save_repository() function). 
Therefore in this instance, the admin class.boaccounts.inc.php save_user() 
function sets the $userData array to have the 'account_' prefixes. Therefore 
when these values are passed to the phpgwapi accounts update_data() function 
and save_repository() is then called, the save_repository function cannot find 
any data, and therefore the users details are updated with null values.

This requires either modification of the phpgwapi class.accounts_sql.inc.php 
file (not sure of the implications of this) OR adding some sort of hack to pass 
the expected data to update_data(). e.g. at the start of admin 
boaccount->update_data($userData) having something like:

//reformat the userdata to fit the modified save_repository function
foreach ($userData as $key => $value) {
        if (strpos($key,'account_') !== false) {
                $userData[substr($key,strpos($key,'_')+1)] = $value;
        }
}

This is obviously a pretty nasty hack & so preferably the phpgwapi accounts 
class should be just tidied up.

----------------------
Next the acl being deleted bug:

This is due to a typo in approx line 463 of admin/inc/class.boaccounts.inc.php. 
The line:

if ($_userData['account_permissions'])

needs to be:

if ($userData['account_permissions'])

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



No Followups Have Been Posted


CC list is empty


No files currently attached


For detailed info, follow this link:
http://savannah.gnu.org/bugs/?func=detailbug&bug_id=4260&group_id=509

_______________________________________________
  Message sent via/by Savannah
  http://savannah.gnu.org/