[20:30:29] anomie: When an auth request is continued, is there any way for AuthManager to tie the first request to the continued request? Other than having the details in the session? [20:32:45] In the event there's a provider that let's a continuation request through without a csrf token, and happens to use some of the same state values that are set by another authentication provider, I'm wondering if we'll ever get in the situation where a user starts one auth method (so stuff is in their session), then they get csrf'ed, and logged in as an attacker. [20:33:48] ^ just wondering if you already thought that through, or if I should look into it more. [20:38:59] wouldn't that require the attacker to have some kind of capability that could also be used to steal or fixate an authenticated session? [20:41:46] tgr: Hopefully that's the case. [20:43:44] tgr: But for something like openid/oauth authentication, I *think* the user will be redirected around, then redirected back into the ACTION_LOGIN_CONTINUE flow without user interaction (so probably no csrf token checking) [20:45:04] So then we're do $session->get( 'AuthManager::authnState' );... and there isn't really a way to tie that request to the one that set AuthManager::authnState initially (other than having the correct contents stored) [20:46:29] Just feel like there might be a way that could be abused, so wondering if there was something to prevent that. [20:47:53] csteipp: the csrf token is included in the return url, see AuthManagerSpecialPage::performAuthenticationStep and AuthManagerSpecialPage::trySubmit [20:48:11] it's true that that's not handled in AuthManager but one layer higher [20:49:52] so if the client messes it up, it's possible to do a session fixation attack, although oauth has its own defense against that [20:58:58] tgr: Possibly. I was thinking a hypothetical OAuth extension would handle getting the user's identity, and then redirect back into Special:UserLogin with authAction set to continue. But it might make more sense to finish the oauth auth in the continuation. [20:59:23] ^ Then OAuth itself would protect it some [21:01:57] csteipp: I think so; https://gerrit.wikimedia.org/r/#/c/251930/21/auth/OAuthPrimaryAuthenticationProvider.php was my attempt at it [21:03:07] it just sets Special:Userlogin as the OAuth callback URL [21:04:10] tgr: Cool. Yeah, haven't gotten to any of the implementations yet :) [21:06:56] * robla was just talking about AuthMgr status with someone recently and was happy to see the status update in the Scrum of Scrums notes [21:10:50] csteipp: AuthManager-side, it's all in the session. I note that when a provider responds with a UI or REDIRECT, AuthManager is going to go back to that provider when it continues. For a primary it's based on the 'primary' key in the session data; for a secondary, both because it does them in order so so that'll be the first one that's not already marked as completed in the 'secondary' data in the session, and because any other won't be marked as [21:10:50] in-progress so they would all get their beginSecondaryAuthentication called instead of continueSecondaryAuthentication. And if that session data goes missing somehow, that's right at the top of AuthManager::continueAuthentication() with a "authmanager-authn-not-in-progress" error. [21:12:22] * robla is assuming that AuthManager support is necessary but not sufficient to get us to the level of abstraction where we could make have pseudo accounts for anonymous edits that don't reveal IP addresses [21:12:58] anomie: Ah, cool, thanks. Now that you mention it, I remember seeing that. [21:13:08] csteipp: So for an attacker to do some sort of session fixation through AuthManager, I think they'd already have had to fixate the session cookie somehow in which case why bother with AuthManager. [21:14:20] robla: IMO the tricky part about pseudo-accounts for anons is figuring some way to do them that isn't wide open to "oops, my current pseudo-account is getting a bad reputation, I'm going to clear my cookies and get an entirely fresh one" [21:15:11] Sure, people can do that already by registering throwaway accounts, but at least that's throttled I guess. [21:18:24] anomie: yeah, pseudo-accounts for anons on enwiki would be a really hard deployment. is there a useful interim step to an enwiki deployment? [21:19:43] (i.e. a good safe-to-fail place to try working it out?) [21:20:43] robla: Nothing comes to mind. Mainly we'd need buy-in that admins would still be able to track and deal with vandals. [21:20:44] from a purely technical point of view AuthManager seems sufficiently abstract for that; you could create a session provider mapping anonymous IPs to throwaway usernames, and then a secondary authentication provider would merge that with the real account on registration [21:23:38] tgr: that's really helpful, thanks! once AuthManager is in, do you think that a GSoC or Outreachy student could make a version that would work for a non-Wikimedia small wiki? [21:23:42] for an interim step, you could have a "create me a random account" feature, instead of doing it automatically on anon edits [21:24:23] oooh, neat [21:24:34] that would not give anons any capability they don't already have but would be very similar to the final system; not sure what would be the user value in that, though [21:25:53] I think the anonymous user value is "not having my IP address associated with my edit advertised to the world" [21:26:32] sure but it's already possible to register [21:27:46] it would become less effort if no username/password is needed and maybe the captcha is replaced by something like recaptcha2, but not sure if that's enough to get people to care [21:28:31] * robla starts looking for a Phab task and/or wiki page associated with psuedo-accounts [21:29:31] I don't see any obvious difficulties that would make it too hard for GSoC, but then lots of complex authentication stuff seems simple while you only have a very vague idea of it [21:33:37] This has some of the rationale: [21:34:10] (the period on the end of the URL matters) [21:35:14] I have seen a phab task for this, can't find it though [21:35:41] https://phabricator.wikimedia.org/T128055 is somewhat related [21:39:07] cool, thanks! [21:50:36] tgr: Is Password Change supposed to work when $wgDisableAuthManager=true? I'm getting an exception. [21:51:45] csteipp: yes; give me a few minutes and I'll look into it [21:53:21] are you looking at the first patch only? [21:54:58] tgr: No, I'm looking at the release notes patch (with all it's dependencies) [21:55:33] (checked out a couple days ago... so it could be out of date) [21:59:06] tgr: Yeah, `git review -d 282202`, login, and try to change password. No other config changes for authmanager besides the default. [22:12:19] Hm.. how do I fix mediawiki-vagrant to have its mw core clone authenticated so I can pull/push, and yet still allow vagrant git-update to work correctly? [22:12:36] Even if I do all my authenticated interaction outside vagrant (which Im fine with) I can't since the origin is https:// [22:12:46] and if I change it, then vagrant won't be able to use it [22:12:47] csteipp: yeah, the UI patch has a snapshot of the old special pages for feature flagging but I forgot to do that for ChangePassword [22:12:59] Krinkle: https://www.mediawiki.org/wiki/MediaWiki-Vagrant#Pushing_commits :D [22:14:32] Krinkle: the insteadOf trick is nice to avoid manually fixing all your repos, but you should always be able to just git-review -s [22:14:51] that will create another remote called gerrit, with an ssh: url [22:14:53] or just configure push over https.... [22:14:56] tgr: yeah, but I don't want a separate git remote (and wouldn;t happen since my gitreview config has defaultremote=origin) [22:15:08] the indexOf trick does everything I need [22:15:14] why would one need a separate remote? [22:19:20] MaxSem: are you aware of https://www.mediawiki.org/wiki/User:PerfektesChaos/WikidiffLX / https://www.mediawiki.org/wiki/User:PerfektesChaos/WikidiffLX/coding ? [22:19:51] tgr: (sorry to keep bugging you) I'm not getting a prompt to relogin when changing my password, even though it's been >300s since I last logged in. [22:20:09] I'm trying to figure out where that is supposed to be added, but not seeing it [22:20:55] is Special:ChangeCredentials supposed to set a security level? [22:21:16] SpecialPage::checkLoginSecurityLevel is the one that does the redirection [22:21:59] SpecialPage::getLoginSecurityLevel is used to declare it [22:24:58] it has to be called manually, which is bad but matches how other security-related SpecialPage methods are called [22:26:03] authentication-related special pages call it via AuthManagerSpecialPage::handleReauthBeforeExecute which is sort of reimplements checkLoginSecurityLevel to deal with interrupted POST requests nicer [22:26:07] Krinkle: besides the indexOf trick, git-update will fetch via http if it sees that the repo is configured for ssh, so you can just switch the remotes manually to use ssh. That's how mine are setup. [22:26:33] cool [22:49:44] csteipp: when is set wgReauthenticateTime to 1 I get redirected to the login page as expected [22:50:04] bd808: Working on https://gerrit.wikimedia.org/r/#/c/274333/5/includes/DefaultSettings.php a bit [22:50:21] tgr: Sorry, yeah. I missunderstood the comment and was trying to use -1 to disable the grace period. [22:50:23] it's not a clean setup so maybe I am missing some config dependency but I can't think of anything [22:50:47] So yeah, with 1 I get the prompt [22:51:00] 0 is an interesting case... ;) [22:51:12] bd808: btw, maybe know you what's going in my vagrant? in /v/mw/tests/phpunit; 'php phpunit.php includes/objectcache/' works fine (and so does sudo -u www-data php5 phpunit.php includes/objectcache/), But if I run instead 'php phpunit.php includes/libs/objectcache/' it fails with ".DB connection error: Unknown database 'includes/libs/objectcache/' [22:51:12] (127.0.0.1)" [22:51:16] maybe some multiwiki stuff corrupted this? [22:51:43] basically any @database test [22:52:56] hmmm.. it could be a multiwiki issue. You must only have one active wiki in the mw-v farm or it would be yelling at you about not passing --wiki=wiki in there too [22:53:44] indeed, fresh plain set up [22:54:16] passing --wiki devwiki makes it work [22:54:27] most of our test harness is scary voodoo to me [22:54:50] makes it rather long to run the tests though instead of 'php phpunit.php path//' we're now up to 'sudo -u www-data php5 --wiki devwiki phpunit.php path//' [22:54:52] ugh [22:55:04] the sudo part seems unneeded from what I can see, but oh well [22:55:14] that's `wpt path//` for me :) [22:55:23] webpagetest? [22:55:30] www php test [22:55:41] I also have wht for hhvm [22:55:48] ofc [22:56:30] actually, no. I wasn't using /libs/ in that comment [22:56:32] command [22:56:34] it still fails [22:56:42] this time with .DB connection error: Unknown database 'devwiki' (127.0.0.1) [22:57:09] https://phabricator.wikimedia.org/P2940 [22:57:12] oh its called wiki [22:57:18] oh yeah [22:57:26] k, passing now [22:57:44] a strange choice that was made on the olden days [23:00:31] bd808: https://gerrit.wikimedia.org/r/#/c/274333/7..8/includes/libs/objectcache/BagOStuff.php [23:06:22] looking better, yeah [23:06:52] scheduling the callback could be put inside the else where we increment [23:07:05] then it wouldn't happen at all unless there actually was a dup [23:58:48] bd808: is it appropriate for an extension to add extension-specific data to user sessions? [23:59:24] .. yes? [23:59:35] right, that's what I thought