[drupal-devel] [bug] Remove code containing user access controls from profile theme functions

[robertDouglass]

Issue status update for 
http://drupal.org/node/27949
Post a follow up: 
http://drupal.org/project/comments/add/27949

 Project:      Drupal
 Version:      cvs
 Component:    profile.module
 Category:     bug reports
 Priority:     normal
 Assigned to:  robertDouglass
 Reported by:  robertDouglass
 Updated by:   robertDouglass
 Status:       patch (code needs review)
 Attachment:   http://drupal.org/files/issues/profile_fix_acces_control_in_theme2.txt (2.95 KB)

updated with $account




robertDouglass



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

Mon, 01 Aug 2005 12:32:07 +0000 : robertDouglass

Attachment: http://drupal.org/files/issues/profile_fix_acces_control_in_theme.txt (2.36 KB)

The two theme functions in profile.module both violate good theming
practice by running user control logic in the middle of them. Worse
yet, this isn't immediately visible since it happens in yet another
function. Thus themers overriding these functions to style profile
pages[1] inadvertently break access control, thus leading to the
misperception that overriding theme functions is inherently
dangerous[2].


[1] http://drupal.org/node/16011
[2] http://drupal.org/node/16821




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

Thu, 18 Aug 2005 12:37:40 +0000 : robertDouglass

patch still applies. Anyone care to review?




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

Thu, 18 Aug 2005 12:41:46 +0000 : Dublin Drupaller

Would like to review and help Robert, but I don't have a CVS version of
drupal installed..will the patch work with 4.6.x?


Dub




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

Thu, 18 Aug 2005 12:45:25 +0000 : chx

Just by looking at the code: -1. Never write $user we use $account in
these places. There is a global $user and you do not want accidental
mixup.


If you change that, I think I'll like it :)




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

Thu, 18 Aug 2005 13:43:33 +0000 : robertDouglass

chx: $account is already in use and I didn't want to replace it
($account = user_load()) - or do you think that would be ok in this
case?




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

Thu, 18 Aug 2005 20:26:20 +0000 : chx

feel free to overwrite, it's common with nodes to do similar.