Auth systems/OAuth/IRC log 2013-06-06

From MediaWiki.org
Jump to navigation Jump to search

Jun 07 11:11:29 <TimStarling> sorry, I only got halfway through reading that Tyler thread, despite the fact that robla asked me to comment on it more than a day ago
Jun 07 11:12:05 <TimStarling> but it seemed like Chris and Brad were pretty well on top of it
Jun 07 11:12:05 <csteipp> Tim, understandable. I was hoping to get your thoughts on Brad's design in general
Jun 07 11:12:50 <TimStarling> specifically on https://gerrit.wikimedia.org/r/#/c/66286/ ?
Jun 07 11:13:17 <csteipp> Yeah, I'm on the fence-- I like what Brad's implementation gives us, but I can also see their point about layering another authorization piece on top of user rights
Jun 07 11:13:19 <csteipp> Yeah
Jun 07 11:13:47 <anomie> I think the summary is basically that Tyler doesn't think OAuth should be an AuthPlugin, and he and Daniel Friesen think that we should do a parallel system to $wgGroupPermissions for OAuth rather than when I did.
Jun 07 11:16:02 <csteipp> anomie: my interpretation is they were wanting to keep around lists of permission groups in the oauth UI, and then the effective rights are their user rights & granted rights
Jun 07 11:16:13 <csteipp> So yeah, kinda parallel to group permissions
Jun 07 11:16:36 <csteipp> But not actually using groups
Jun 07 11:17:14 <anomie> They didn't say it explicitly, but something like $wgGroupPermissions but for OAuth seems like basically what they're getting at with their talk about "grouping" rights.
Jun 07 11:18:12 <TimStarling> well, a group as in $wgGroupPermissions is a group of users rather than a group of rights
Jun 07 11:19:01 <csteipp> Right, it was a "grouping" of rights
Jun 07 11:19:16 <anomie> Oh? I must be thinking about $wgGroupPermissions wrong then. I thought it defined groups of rights, then those users are associated with those groups to grant them the rights.
Jun 07 11:20:38 <TimStarling> maybe originally, but groups do all kinds of things now
Jun 07 11:21:08 <TimStarling> like $wgRevokePermissions
Jun 07 11:21:16 <Aaron|home> I was about to mention that ;)
Jun 07 11:21:36 <TimStarling> just reading the relevant post from daniel
Jun 07 11:23:45 <TimStarling> so what he's talking about is purely a UI concept
Jun 07 11:23:52 <TimStarling> right?
Jun 07 11:24:40 <Aaron|home> so where would all these rights be registered once authorized to be used by the user?
Jun 07 11:24:54 <csteipp> As I pointed out, it also affects the expectations of what is allowed, but yes, the implementation is UI
Jun 07 11:25:08 <anomie> He's talking UI, yes. I'm not sure how else we'd do what he's talking about though.
Jun 07 11:25:45 <csteipp> Aaron|home: In the db somewhere-- we will want a "grants" table
Jun 07 11:25:58 * Aaron|home is just to thing what happens if the rights system gets tweaked a bit, like splitting up a right
Jun 07 11:26:05 <Aaron|home> I guess we'd keep the old ones around for b/c
Jun 07 11:26:19 <csteipp> Aaron|home: Yep, exactly
Jun 07 11:26:21 <Aaron|home> if you stored right group names in the grants table, this wouldn't matter
Jun 07 11:26:39 <csteipp> I image if we group the rights in the UI, we would want to store the actual rights in the DB
Jun 07 11:26:41 <Aaron|home> not really a killing difference though
Jun 07 11:27:08 <anomie> Although that has another problem, if a group gets added rights then users haven't necessarily authorized the tools to have those rights.
Jun 07 11:27:11 <csteipp> So store 'edit','read','create', etc instead of "Create and Edit Pages"
Jun 07 11:27:19 <anomie> (re "if you stored right group names")
Jun 07 11:28:42 <csteipp> anomie: Yep. And that's what I don't like about the groupings--- if the user sees "Create and Edit Pages", the grouping changes, then another app is granted the same grouping, it could be different rights
Jun 07 11:29:05 <csteipp> I definitely don't think we want to store the grouping, and allow them to be changed. That would not be nice to users.
Jun 07 11:29:33 <anomie> Hmm. If we store 'edit','read','create', then another right gets added to "Create and Edit Pages", what happens in the UI now that there's no covering group name?
Jun 07 11:30:14 <anomie> E.g. "Create and Edit Pages" becomes 'edit','read','create','reply' or something
Jun 07 11:30:34 <Aaron|home> anomie: you mean like a UI that just list the current rights you give to an app (to revise them ect)?
Jun 07 11:30:40 <anomie> Aaron|home: Exactly
Jun 07 11:30:41 <Aaron|home> that would be a bit tricky
Jun 07 11:30:45 <csteipp> It does get messy.
Jun 07 11:31:09 <Aaron|home> "Create and Edit Pages v1" ;)
Jun 07 11:31:15 <anomie> Ugh
Jun 07 11:32:06 <csteipp> store an object that has grouping name and permisions? (I was going to make a joke about doing that, but actually, I'm not sure)
Jun 07 11:32:26 <Aaron|home> and if group A changed, but not B, and they add B, will A accidentally change the A rights?
Jun 07 11:32:39 <Aaron|home> I could picture that kind of bug in the form, heh
Jun 07 11:33:08 <Aaron|home> so yeah, maybe you'd have to show specific rights in that case
Jun 07 11:33:55 <anomie> Or do we just want to keep the thing I implemented instead of messing with rights like this?
Jun 07 11:34:18 <csteipp> that is the real question...
Jun 07 11:35:00 <csteipp> So anomie, the thing I'm worried about with your implementation is that it seems like it's one more thing for the api developers to remember to use, and they can get wrong
Jun 07 11:35:36 <TimStarling> if the API developers forget it, the default is sensible, right?
Jun 07 11:36:04 <anomie> The default is that the module requires a permission that is the same as the name of the module.
Jun 07 11:36:33 <TimStarling> one of the reasons for using API module names is because rights are not sufficiently fine-grained, right?
Jun 07 11:36:36 <anomie> So action=foobar would require the "foobar" permission. Query modules are slightly different, action=query&list=foobaz would require "query-foobaz".
Jun 07 11:36:59 <TimStarling> I mean, they are too fine-grained for some things, and too coarse for others
Jun 07 11:37:07 <Aaron|home> or sort of the opposite in a way, right
Jun 07 11:37:26 <anomie> It just seems a halfway sane default. The module can explicitly make its permissions more granular, or it can make it less granular if that makes sense (e.g. action=block and action=unblock both use the "block" permission)
Jun 07 11:39:13 <TimStarling> so daniel just proposes to add more rights for every case where the current system is too coarse?
Jun 07 11:39:41 <anomie> That's how it sounds to me, yes. Like "editmyuserjs" and "editmyusercss" rights.
Jun 07 11:40:03 <TimStarling> that particular example was wrong
Jun 07 11:40:14 <anomie> ?
Jun 07 11:40:23 <TimStarling> well, incomplete, maybe
Jun 07 11:40:51 <TimStarling> you would also need to change the edit permission so that it doesn't grant those two things
Jun 07 11:41:22 <TimStarling> which would break everyone's LocalSettings.php since everyone's $wgGroupPermissions would be wrong
Jun 07 11:41:44 <TimStarling> granting only edit where edit+editmyuserjs+editmyusercss was desired
Jun 07 11:43:10 <TimStarling> you would have to introduce a whole new rights concept underneath the current one, in order to maintain backwards compatibility
Jun 07 11:44:02 <anomie> An 'edit' grants 'edit-base'+'editmyuserjs'+'editmyusercss' sort of thing?
Jun 07 11:44:13 <TimStarling> yes
Jun 07 11:46:01 <TimStarling> I sympathise with the idea of having a unified rights concept across both OAuth and the regular UI, but it does seem like it would be pretty complicated
Jun 07 11:48:57 <csteipp> anomie: So using your grants for an api like centralauth's ApiQueryGlobalUserInfo where a user can get suppressed usernames if they have the rights for it.
Jun 07 11:49:22 <csteipp> The user would grant a client access to QueryGlobalUserInfo
Jun 07 11:50:12 <anomie> Yes. Actually "query-globaluserinfo" at the moment.
Jun 07 11:50:13 <csteipp> But if an admin didn't want the app looking up suppressed users, they would have to get the api author to make a QueryGlobalUserInfo-suppressed (or something) right, and then grant that explicitly?
Jun 07 11:50:35 <anomie> Yes
Jun 07 11:51:28 <csteipp> And the api author would in their code check for the right. And OAuth would differentiate, but the normal api probably would grant that automatically.
Jun 07 11:51:33 <TimStarling> is there any documentation of the current rights list?
Jun 07 11:52:49 <TimStarling> also do we know in exactly how many cases the current rights list is too coarse?
Jun 07 11:53:12 <csteipp> I assumed https://www.mediawiki.org/wiki/User_rights was reasonably up to date, but I think Aaron though it was missing a lot
Jun 07 11:53:14 <Aaron|home> csteipp: sounds scary :)
Jun 07 11:54:14 <Aaron|home> well extensions mostly
Jun 07 11:54:38 <TimStarling> in terms of procedure: I think it would be best if we can reach consensus with Daniel and Platonides
Jun 07 11:54:40 <anomie> csteipp: Yeah. The change to the API module would probably look something vaguely like https://gerrit.wikimedia.org/r/#/c/66286/2/includes/api/ApiProtect.php. For non-OAuth access, this whole new permission system is just ignored so things would work just as they do now; OAuth would override AuthPlugin::checkGrantsForApi() to make the checking actually check anything.
Jun 07 11:54:43 <csteipp> Aaron|home: Yeah, there are definitely some extension api authors who didn't even check for the user rights....
Jun 07 11:55:20 <Aaron|home> csteipp: it would be nice to rank rights into sensitivity tiers and not grant the higher ones (and ones not ranked at all) by default
Jun 07 11:55:42 <Aaron|home> I mean we already maintain $wgAvailableRights in extensions, this wouldn't be much more work
Jun 07 11:55:45 <TimStarling> it may be that it *is* possible to revoke editmyusercss from edit
Jun 07 11:55:50 <TimStarling> without a backwards compatibility layer
Jun 07 11:55:51 <anomie> TimStarling: Out of core modules, mainly editing I think is too coarse.
Jun 07 11:56:18 <TimStarling> assuming that's the extent of it, maybe that is not too much to ask of third party users
Jun 07 11:56:48 <TimStarling> since looking at the right list, it seems likely that we have done similar things in the past
Jun 07 11:57:17 <TimStarling> e.g. the introduction of the reupload right
Jun 07 11:57:34 <TimStarling> originally it would have just been upload
Jun 07 11:58:40 <Aaron|home> right
Jun 07 11:58:47 <TimStarling> how many pieces would edit have to be split into?
Jun 07 11:59:15 <TimStarling> I guess the answer is in anomie's patch
Jun 07 12:00:45 <TimStarling> that patch just adds override-watchlist-pref, right?
Jun 07 12:01:04 <anomie> editmyusercss and editmyuserjs. Also editing of sysop-protected pages without granting protect, or editprotected which allows overriding *all* protection levels. And maybe autoconfirmed too, since that controls more than just editing autoconfirmed-protected pages.
Jun 07 12:02:13 <anomie> "override-watchlist-pref" could easily go away. I put it in mainly so a tool could be prevented from making edits without adding them to the watchlist, but that's not exactly a huge deal if we would remove that.
Jun 07 12:04:07 <TimStarling> I see, in ApiBase::getPermissionsForTitle()
Jun 07 12:06:11 <anomie> Oh. And there's also "missing" rights for 'view-my-watchlist', 'edit-my-watchlist', 'see-my-private-info' (e.g. meta=userinfo&uiprop=realname|email|hasmsg|options)
Jun 07 12:06:13 <TimStarling> the editprotected right is known to be broken, there is a bug for that
Jun 07 12:06:52 <TimStarling> so I don't think there would be too many tears about changing what it does
Jun 07 12:08:25 <anomie> And I don't know if there are extensions with insufficiently-granular rights with respect to viewing log entries versus taking the action that creates those log entries.
Jun 07 12:12:19 <TimStarling> ok, so split edit and autoconfirmed, fix the broken protection system, introduce maybe 3 or 4 new rights to core for private data
Jun 07 12:13:19 <TimStarling> so maybe half a dozen new rights altogether, that have to be added on upgrade for anyone who is setting $wgGroupPermissions instead of modifying the version from DefaultSettings.php
Jun 07 12:16:13 <TimStarling> extension rights wouldn't need to be done as part of the initial project
Jun 07 12:17:55 <TimStarling> then there is this grouping system
Jun 07 12:19:15 <TimStarling> what was the reason again for preferring storage of right grants instead of group grants?
Jun 07 12:19:26 <csteipp> In case they chage
Jun 07 12:20:08 <csteipp> change. So if the user grants some set of rights, if it's updated, it seems (to me) that the app shouldn't be allowed those new rights, unless the user re-authorizes.
Jun 07 12:20:37 <TimStarling> I am thinking that maybe the app should be allowed the new rights
Jun 07 12:21:03 <TimStarling> since we are talking about splitting rights and that is a fairly likely reason for groups to be changed in the future
Jun 07 12:21:44 <TimStarling> groups could be specific enough that they wouldn't be updated to grant completely unrelated rights
Jun 07 12:22:49 <csteipp> That would be the hope. I'm assuming it's something we would require very high privileges to change, if not a shell config?
Jun 07 12:23:10 <TimStarling> shell config would be fine, I think
Jun 07 12:23:55 <TimStarling> the groups could be fixed, then you could have localisation for the names and descriptions of them
Jun 07 12:24:24 <TimStarling> but maybe we should think of a name other than "group", or at least some adjective
Jun 07 12:24:47 <csteipp> That would probably work.
Jun 07 12:25:11 <TimStarling> otherwise I'm going to get confused talking about groups of rights
Jun 07 12:25:53 <csteipp> "permissions" ?
Jun 07 12:26:17 <TimStarling> yes, that would work
Jun 07 12:26:37 <TimStarling> the current code already uses that word somewhere, doesn't it?
Jun 07 12:26:50 <TimStarling> I mean, current as in brad's patch
Jun 07 12:27:11 <anomie> Yes, it does.
Jun 07 12:28:23 <TimStarling> ok, so if we do this, it will probably set us back a week or two
Jun 07 12:28:42 <TimStarling> would you agree with that estimate, anomie?
Jun 07 12:29:57 <anomie> I've never been good at estimating time. I don't think we have much written besides my patch, so we haven't lost a whole lot.
Jun 07 12:30:25 <TimStarling> in exchange, we would get:
Jun 07 12:30:44 <TimStarling> * a happy Tyler, Daniel and Platonides
Jun 07 12:31:26 <TimStarling> * a simpler UI, due to explicit mappings between rights and UI permission display
Jun 07 12:31:51 <TimStarling> * more elegant backend due to the ability to use a unified rights concept
Jun 07 12:32:38 <TimStarling> disadvantage: explicit permissions mapping needs to be done for each extension that wants to use OAuth
Jun 07 12:33:34 <anomie> I can look at adding the extra rights to core, which shouldn't take extremely long to do but probably longer to make sure it's not missing anything.
Jun 07 12:34:21 <Aaron|home> TimStarling: so we store the "permissions" instead of rights and those always reflect their current extension definitions?
Jun 07 12:34:39 <TimStarling> Aaron|home: yes, just like groups
Jun 07 12:34:42 <csteipp> I think I'm only nervous about fixing protection (probably because I've never touched much of it). That seemed like a big project last time we talked.
Jun 07 12:34:44 * Aaron|home wonders why his wifi has two bars :/
Jun 07 12:34:55 <Aaron|home> seems like this could work
Jun 07 12:34:59 <TimStarling> yes, fixing protection is one of the two weeks of my estimate
Jun 07 12:35:14 <csteipp> A wifi card walked into two bars...
Jun 07 12:35:20 <TimStarling> one week for protection, one for everything else
Jun 07 12:35:26 <csteipp> gatcha
Jun 07 12:35:34 <anomie> TimStarling: I think last time we talked about this (February?), you said you had some notes about fixing protection?
Jun 07 12:36:02 <TimStarling> yes, I thought I had put them in a bug, but maybe not since I haven't found the bug yet
Jun 07 12:36:06 <TimStarling> if not, I will file the bug
Jun 07 12:37:42 <TimStarling> ok, I guess I will post something along these lines to wikitech-l, if everyone's happy with it
Jun 07 12:38:00 <anomie> I'll start looking at the non-protection-related permission bits of the core changes tomorrow.
Jun 07 12:38:40 <TimStarling> regarding IRC logs: I have xchat log everything
Jun 07 12:38:55 <TimStarling> so I can post logs of this channel, including previous discussions, if we want to do that
Jun 07 12:40:09 <TimStarling> ok, got to go
Jun 07 12:40:16 <csteipp> Maybe post todays? Thanks!
Jun 07 12:40:22 <Aaron|home> bye
Jun 07 12:40:30 <csteipp> Thanks everyone!
Jun 07 12:40:43 <TimStarling> yeah, I'll post today