On 7/10/07, Daniel Cannon cannon.danielc@gmail.com wrote:
Firstly, everyone seems to agree that we need to change the behavior of Title::userCan and access control on MediaWiki in general.
I wouldn't go that far. Change the specific implementation of some functions that are serviceable but a bit awkward, is how I'd put it. At least, for most cases: read permissions do need to be overhauled if they're meant to actually work, but we knew that anyway.
Part of this reform may include Aaron's suggestion
I think you mean Andrew? Also, it was agreed(/mandated) that userCan cannot change its return type or anything like that. We're making new methods.
The far more pressing issue here, however, and indeed the one that is most important to the developers of extensions and the API and similar frameworks, is the need for a single, reliable, complete, centralized access-checking mechanism. At present we have only basic centralized access-checking, such as Title::userCan, and for more complex cases we are forced to rely upon individual interpretation of what a certain right implies.
I don't see this at all. All rights are basically well-defined. You just have to make sure you check if the user is allowed to do them.
In my opinion, access-checking is performed best not independent of but rather as part of all accessors--that is, rather than relying upon extensions to check that the user can read or modify a bit of data before doing so, simply don't allow access to the data without first performing checks to verify that the user can access it. By this design, if Special:MySuperCoolSpecialPage calls Title::getPageContent() on a page the user cannot read, he gets an access exception.
Lovely in theory, but impossible in practice. You don't know that it's calling the page for the purpose of serving the content to any particular user. What if the special page is "E-mail this to a friend"? Then it's the friend whose read permissions need to be checked. Or is it? Maybe you want users to be able to send anyone pages that the sender can read. Or maybe you want them to only be able to send pages that both they can read *and* the recipient can read. Only the caller could even conceivably make such a decision.
Despite your citation of Yurik's unrelated troubles (which involve the fact that functions have to be used at all to check permissions), I see no usage case where this kind of change is actually helpful. The only possibility is if someone forgets a permissions check somewhere, but that strikes me as not nearly likely enough to merit such a complicated and error-prone structure to prevent it.
It also frees us up from having to use stuff like OutputPage::readOnlyPage() in that the message to use is determined by the assertUserCan method, and it saves us from a *multitude* of invocations--in fact (theoretically of course) we shouldn't have to do *any* access checking outside of that one method; our cost is only having to update accessors to include _one_ invocation of that method.
Werdna's implementation-in-progress that sparked this thread works fine for that.
Now to touch upon a few points that Simetrical, Yuri, Andrew, and I discussed (and then I assure you my blabber is coming to an end..). For one, we really need not one, but two userCan methods. One should be in Title . . . The other should find itself in User.php
I still think they both belong in User.php, as part of the same method. I don't view your counterproposal as a suitable alternative, because it's perfectly conceivable that some actions would fit best with two Titles (move), a Revision (oversight/rev_deleted), or something from an extension. Passing arrays of objects of arbitrary type is no different than we do for wfRunHooks, for instance.
One last suggestion, one that is likely further down the road if on the road at all, would be a migration or incorporation of user rights from our current internal system ($wgGroupPermissions, etc.) to a dynamic table that associates rights with pages or other entities, or to the simple addition of a couple of fields to the page table. Such a table could have the structure: - page_id (the id of the page) - page_name (the name of the page) - page_namespace (the pagenamespace) - edit_right (the right required to edit this page) - move_right (the right required to move this page) - read_right (the right required to read this page)
We have this table already. It's called page_restrictions. If you mean we should duplicate all group permissions in every row of the page table, good grief, that's what joins are for. It's wildly impractical to reproduce every group permission two million times, and there's nothing to be gained from it.
The table could be updated through an update script that would simply set the rights for each page according to what would make userCan happy.
userCan requires a User object, not just usergroups. You can't compartmentalize its current flexibility of output into the database. userCan hooks can prevent pages in the Project namespace from being moved by non-sysop autoconfirmed users in the hour after midnight if they like. We have to cut that out for read permissions, but I'm not sure whether or not it's fully necessary to do it for other permissions. It could certainly accelerate batch actions a fair bit if we did, but there are few enough of those that I'm not sure it's necessary.