1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-18 21:02:41 +01:00

Unbreak OAuth Registration

Summary:
@vrana patched an important external-CSRF-leaking hole recently (D1558), but
since we are sloppy in building this form it got caught in the crossfire.

We set action to something like "http://this.server.com/oauth/derp/", but that
triggers CSRF protection by removing CSRF tokens from the form. This makes OAuth
login not work.

Instead, use the local path only so we generate a CSRF token.

Test Plan: Registered locally via oauth.

Reviewers: vrana, btrahan

Reviewed By: vrana

CC: aran, epriestley, demo

Maniphest Tasks: T853

Differential Revision: https://secure.phabricator.com/D1597
This commit is contained in:
epriestley 2012-02-08 10:26:04 -08:00
parent 8482569a47
commit ecd4b03a4e
2 changed files with 8 additions and 1 deletions

View file

@ -121,13 +121,19 @@ class PhabricatorOAuthDefaultRegistrationController
$error_view->setErrors($errors);
}
// Strip the URI down to the path, because otherwise we'll trigger
// external CSRF protection (by having a protocol in the form "action")
// and generate a form with no CSRF token.
$action_uri = new PhutilURI($provider->getRedirectURI());
$action_path = $action_uri->getPath();
$form = new AphrontFormView();
$form
->addHiddenInput('token', $provider->getAccessToken())
->addHiddenInput('expires', $oauth_info->getTokenExpires())
->addHiddenInput('state', $this->getOAuthState())
->setUser($request->getUser())
->setAction($provider->getRedirectURI())
->setAction($action_path)
->appendChild(
id(new AphrontFormTextControl())
->setLabel('Username')

View file

@ -16,6 +16,7 @@ phutil_require_module('phabricator', 'view/form/control/text');
phutil_require_module('phabricator', 'view/form/error');
phutil_require_module('phabricator', 'view/layout/panel');
phutil_require_module('phutil', 'parser/uri');
phutil_require_module('phutil', 'utils');