On 7/10/07, Daniel Cannon <cannon.danielc(a)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.