Re: [Mantisbt-dev] Permissions and User actions

[Julian F. <ju...@be...>]

Onken, Luebbe wrote:
> Hi there,
> 
> since your replies to my nonexistent postings from tuesday are nonexistent as
> well, I'll start a second discussion about another thing I've been thinking
> about for a while.
> 
> I suggest to do something about the way permissions and user actions are
> handled in mantis. Currently we've got a lot of lines like:
> 
> if ( ( $v_status >= RESOLVED ) && 
> 	( access_level_check_greater_or_equal( $g_reopen_bug_threshold ) ||
> 	( $v_reporter_id == $t_user_id )) )
> 
> in the code. In addidtion there are functions like:
> project_access_check( $f_id ) and
> access_bug_check( $f_id, $v_view_state ),
> that give you information about what a user is allowed to do as well.
> Especially the if statements contain some danger (and have posed problems in
> the past), since they sometimes should be the same in different pages, but
> they aren't.
> Example, not necessarily true:
> -----
> why the h... can I reopen a bug on the advanced page and not on the simple
> page? Ist because on one page the code reads as above and on the other it
> reads:
> if ( ( $v_status >= RESOLVED ) && 
> 	( access_level_check_greater_or_equal( DEVELOPER ) ||
> 	( $v_reporter_id == $t_user_id )) )
> Without an additional comment that line doesn't tell me, which permission I
> check.
> -----
> 
> What I suggest to do is the following:
> Implement a UserClass that contains all the permission checks with their
> results as properties and is instatiated once when a page is loaded.
> 
> $User = new UserClass($UserID,$ProjectID,$BugID);
> 
> Instead of using the above if statement I can then say things like:
> 
> if !$User->mayViewBug { Redirect to Main Page (or to referrer) };
> if $User->mayReopen { Print Reopen Form };
> if $User->mayMonitor { Print Monitor Form };
> 
> It would make the code a lot easier to write/read and all the permission
> checks would be in one place e.g. could be changed easily.

:) Wow, you have exactly the same issues with Mantis that I did... 
We've been talking over the last few weeks about doing exactly this 
(though not with classes becauses Ken wants to stay away from classes 
whenever possible).  The id is to have a function called something like 
access_check() that you pass an action token into like 
ACTION_BUG_REOPEN, ACTION_BUG_CLOSE, etc

The tricky part is figuring out how to pass in all the potentially 
important information to make the decision, though it may not be as bad 
as it seems.  You're always going to need to pass in the thing you are 
doing the action on (the bug id, project id, bugnote id, etc) and maybe 
there isn't anything else much that would affect it (other than maybe 
current project which you can get at anyway)

Anyway, we haven't started designs on it yet but it's next on my list 
once all the code is refactored and all the register_globals usage 
removed.  So if you have any thoughts or suggestions throw them out here.

Oh, the other advantage to using action tokens is that we can also have 
action_done() that gets passed the same token and can provide hooks for 
post-processing (like sending emails, or whatever).

Julian


-- 
ju...@be...
Beta4 Productions (http://www.beta4.com)