An opinion about the role GUI/code design currently proposed for OMP
Giving feedback on http://pkp.sfu.ca/bugzilla/show_bug.cgi?id=4987 I had a long discussion with Tyler about the proposed authorization system for OMP.
Here my understanding of the current approach:
The basic permission building block is the handler operation. Basic access "roles" are then "hard coded" into the operations ("validation"). The number of roles will be considerably increased but the basic concept remains. Every operation is being assigned to one fixed role. This means that most of the time pre-defined roles are intersection-free: An operation that is part of one role will not be part of another role at the same time.
The "flexible role" approach enables users to assigns synonyms to these "hard coded" roles.
The full system would look something like this:
- Users are assigned to role synonyms ("flexible roles") or pre-defined base roles.
- "Flexible role" synonyms will be resolved to the pre-defined "base roles" if necessary.
- "Validation" (Authorization) is being executed on handler operations level by checking whether the required pre-defined role for a given operation is part of the user's assigned roles.
The difficulties I have with this approach are:
- The terminology chosen for the approach is not very accessible for somebody not part of the PKP team. What PKP calls "validation" is usually called "authorization". (The term "validation" is usually used in the context of checking form values.) What PKP calls "roles" in the current approach would probably be termed "access object group" by other frameworks or projects. Of course words are just labels. But it certainly helps to keep the code basis auto-documenting and accessible if we use a terminology that is not specific to our project (where possible).
- If I understand the proposed system correctly then there are no roles (in the classic sense) at all but permission groups are directly assigned to users. The difference between a "real" role and an access object group is that roles can have intersections and access object groups cannot. In other words: I can assign permission to the same access object to two distinct roles but I cannot usually assign the same access object to two access object groups. Access object groups are usually strictly hierarchical while roles can have intersections. Introducing "real" roles between access object groups and users will drastically reduce the number of required roles and the number of necessary assignments per user and thereby increase end-user usability. There have been purely hierarchical access object group systems historically but they are no longer "state of the art"...
- Implementing custom roles by assigning "synonyms" to pre-defined roles is IMO quite unorthodox. It is ok to use this approach if custom roles will always be 1:1 assignable to pre-defined roles in the future. But IMO a more flexible approach is possible without extra cost. I'm also afraid that usability may suffer if we don't use an authorization approach that is well known to users.
- Hard coding roles into handler operations is IMO also not the best solution. I guess that the new role system will force us to touch all validation methods anyway. Why not benefiting from this opportunity and refactor authorization into a central place (e.g. somewhere in the dispatcher) which is generally considered best practice for an authorization system.
An alternative implementation proposal
The approach I propose is very similar to what has been implemented with great success by major frameworks like CakePHP, phpgacl, Spring Framework, Flow 3 and probably hundreds of other projects. I believe that the approach I propose is even cheaper to implement than the currently proposed approach (the way I understand it). And above all: I'm quite sure that it will considerably improve usability of custom role configuration and day-to-day user-to-role assignments.
I propose to establish the following entities to implement authorization:
- access objects (=currently page or component handlers, can be extended to other objects)
- access operations (=currently page or component operations, can be extended to other operations)
- access objects and access operations can be grouped together into access object groups (=in our case this could be workflows like submission, proofreading, etc.)
- access object groups are assigned to roles (=editor role, author role or any "flexible" custom role)
- users are assigned to roles
- The GUI for custom role administration could IMO be greatly simplified:
____________________________________________________ | Drop down: role | | | | Available workflows: Assigned workflows: | | _____________________ _____________________ | | |workflow 1 | |workflow 6 | | | |workflow 2 | |workflow 7 | | | |workflow 3 | -> |workflow 8 | | | |workflow 4 | <- | | | | |workflow 5 | | | | | |... | | | | | --------------------- --------------------- | | | | Button: Create new role | Drop down: based on | |__________________________________________________|
This will replace all six role configuration sections in press setup step 3 (Workflows). The workflows can also be graphically grouped together into subgroups if we have too many of them. (Think of a tree-like arrangement on the left side.) As they are strictly hierarchical, this is not a problem. The many new "roles" that have been created for OMP will be labeled "workflows" (=access object groups) instead.
- The role-to-user assignment would remain exactly as it is (with custom roles appearing as any other role).
- On the database side we need two new tables. One that assigns access object groups to roles and another that assigns low-level access object-operation tuples to access object groups. The existing user to role assignment does not need to be touched. The access objects (handlers) and access operations (handler operations) can be quickly retrieved in a semi-automatic way from the existing index.php pages for page handlers to create the initial database entries in the permission tables.
- Authorization (="user validation") can be removed (with a few exceptions) from the handlers and replaced by a single authorization call in the dispatcher (after the handler operation has been identified but before the handler is actually being called). It comes down to 1 line of code in the dispatcher and a single "AuthorizationManager" class with maybe 50-100 lines of code that does all the work.
- A nice side-effect: Once we have the operation to handler mapping in the database we can also easily get rid of the index.php files altogether and retrieve the contained routing information from the database instead (solves #4876).
Generally I think this kind of design would greatly improve the flexibility of our authorization system while simplifying it at the same time. We'd drastically reduce the amount of authorization code necessary. We'd move from decentralized to centralized authorization and from a blacklist to a whitelist approach which both should reduce the probability of security breaches. And we'd IMO get a much more usable and easy-to-understand user interface for the end user. We'd get fully flexible "custom roles" that can be configured in a more intuitive way (=appealing to concepts already known by the user) than they are now. And "custom roles" would no longer be restricted to mere "synonyms" of hard coded roles.