Page MenuHomePhabricator
Files F14034577

D8517.diff
No OneTemporary
Actions

D8517.diff
View Options

diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -1100,6 +1100,7 @@
'PhabricatorApplicationMetaMTA' => 'applications/metamta/application/PhabricatorApplicationMetaMTA.php',
'PhabricatorApplicationNotifications' => 'applications/notification/application/PhabricatorApplicationNotifications.php',
'PhabricatorApplicationNuance' => 'applications/nuance/application/PhabricatorApplicationNuance.php',
+ 'PhabricatorApplicationOAuthServer' => 'applications/oauthserver/application/PhabricatorApplicationOAuthServer.php',
'PhabricatorApplicationOwners' => 'applications/owners/application/PhabricatorApplicationOwners.php',
'PhabricatorApplicationPHIDTypeApplication' => 'applications/meta/phid/PhabricatorApplicationPHIDTypeApplication.php',
'PhabricatorApplicationPHPAST' => 'applications/phpast/application/PhabricatorApplicationPHPAST.php',
@@ -1717,6 +1718,7 @@
'PhabricatorOAuthServerAuthorizationCode' => 'applications/oauthserver/storage/PhabricatorOAuthServerAuthorizationCode.php',
'PhabricatorOAuthServerClient' => 'applications/oauthserver/storage/PhabricatorOAuthServerClient.php',
'PhabricatorOAuthServerClientQuery' => 'applications/oauthserver/query/PhabricatorOAuthServerClientQuery.php',
+ 'PhabricatorOAuthServerConsoleController' => 'applications/oauthserver/controller/PhabricatorOAuthServerConsoleController.php',
'PhabricatorOAuthServerController' => 'applications/oauthserver/controller/PhabricatorOAuthServerController.php',
'PhabricatorOAuthServerDAO' => 'applications/oauthserver/storage/PhabricatorOAuthServerDAO.php',
'PhabricatorOAuthServerScope' => 'applications/oauthserver/PhabricatorOAuthServerScope.php',
@@ -3753,6 +3755,7 @@
'PhabricatorApplicationMetaMTA' => 'PhabricatorApplication',
'PhabricatorApplicationNotifications' => 'PhabricatorApplication',
'PhabricatorApplicationNuance' => 'PhabricatorApplication',
+ 'PhabricatorApplicationOAuthServer' => 'PhabricatorApplication',
'PhabricatorApplicationOwners' => 'PhabricatorApplication',
'PhabricatorApplicationPHIDTypeApplication' => 'PhabricatorPHIDType',
'PhabricatorApplicationPHPAST' => 'PhabricatorApplication',
@@ -4454,6 +4457,7 @@
'PhabricatorOAuthServerAuthorizationCode' => 'PhabricatorOAuthServerDAO',
'PhabricatorOAuthServerClient' => 'PhabricatorOAuthServerDAO',
'PhabricatorOAuthServerClientQuery' => 'PhabricatorOffsetPagedQuery',
+ 'PhabricatorOAuthServerConsoleController' => 'PhabricatorOAuthServerController',
'PhabricatorOAuthServerController' => 'PhabricatorController',
'PhabricatorOAuthServerDAO' => 'PhabricatorLiskDAO',
'PhabricatorOAuthServerTestCase' => 'PhabricatorTestCase',
diff --git a/src/aphront/configuration/AphrontDefaultApplicationConfiguration.php b/src/aphront/configuration/AphrontDefaultApplicationConfiguration.php
--- a/src/aphront/configuration/AphrontDefaultApplicationConfiguration.php
+++ b/src/aphront/configuration/AphrontDefaultApplicationConfiguration.php
@@ -19,26 +19,6 @@
public function getURIMap() {
return $this->getResourceURIMapRules() + array(
- '/oauthserver/' => array(
- 'auth/' => 'PhabricatorOAuthServerAuthController',
- 'test/' => 'PhabricatorOAuthServerTestController',
- 'token/' => 'PhabricatorOAuthServerTokenController',
- 'clientauthorization/' => array(
- '' => 'PhabricatorOAuthClientAuthorizationListController',
- 'delete/(?P<phid>[^/]+)/' =>
- 'PhabricatorOAuthClientAuthorizationDeleteController',
- 'edit/(?P<phid>[^/]+)/' =>
- 'PhabricatorOAuthClientAuthorizationEditController',
- ),
- 'client/' => array(
- '' => 'PhabricatorOAuthClientListController',
- 'create/' => 'PhabricatorOAuthClientEditController',
- 'delete/(?P<phid>[^/]+)/' => 'PhabricatorOAuthClientDeleteController',
- 'edit/(?P<phid>[^/]+)/' => 'PhabricatorOAuthClientEditController',
- 'view/(?P<phid>[^/]+)/' => 'PhabricatorOAuthClientViewController',
- ),
- ),
-
'/~/' => array(
'' => 'DarkConsoleController',
'data/(?P<key>[^/]+)/' => 'DarkConsoleDataController',
diff --git a/src/applications/oauthserver/application/PhabricatorApplicationOAuthServer.php b/src/applications/oauthserver/application/PhabricatorApplicationOAuthServer.php
new file mode 100644
--- /dev/null
+++ b/src/applications/oauthserver/application/PhabricatorApplicationOAuthServer.php
@@ -0,0 +1,62 @@
+<?php
+
+final class PhabricatorApplicationOAuthServer extends PhabricatorApplication {
+
+ public function getBaseURI() {
+ return '/oauthserver/';
+ }
+
+ public function getShortDescription() {
+ return pht('OAuth Provider');
+ }
+
+ public function getIconName() {
+ return 'oauthserver';
+ }
+
+ public function getTitleGlyph() {
+ return "~";
+ }
+
+ public function getFlavorText() {
+ return pht('yerps');
+ }
+
+ public function getApplicationGroup() {
+ return self::GROUP_UTILITIES;
+ }
+
+ public function canUninstall() {
+ return false;
+ }
+
+ public function isBeta() {
+ return true;
+ }
+
+ public function getRoutes() {
+ return array(
+ '/oauthserver/' => array(
+ '' => 'PhabricatorOAuthServerConsoleController',
+ 'auth/' => 'PhabricatorOAuthServerAuthController',
+ 'test/' => 'PhabricatorOAuthServerTestController',
+ 'token/' => 'PhabricatorOAuthServerTokenController',
+ 'clientauthorization/' => array(
+ '' => 'PhabricatorOAuthClientAuthorizationListController',
+ 'delete/(?P<phid>[^/]+)/' =>
+ 'PhabricatorOAuthClientAuthorizationDeleteController',
+ 'edit/(?P<phid>[^/]+)/' =>
+ 'PhabricatorOAuthClientAuthorizationEditController',
+ ),
+ 'client/' => array(
+ '' => 'PhabricatorOAuthClientListController',
+ 'create/' => 'PhabricatorOAuthClientEditController',
+ 'delete/(?P<phid>[^/]+)/' => 'PhabricatorOAuthClientDeleteController',
+ 'edit/(?P<phid>[^/]+)/' => 'PhabricatorOAuthClientEditController',
+ 'view/(?P<phid>[^/]+)/' => 'PhabricatorOAuthClientViewController',
+ ),
+ ),
+ );
+ }
+
+}
diff --git a/src/applications/oauthserver/controller/PhabricatorOAuthServerAuthController.php b/src/applications/oauthserver/controller/PhabricatorOAuthServerAuthController.php
--- a/src/applications/oauthserver/controller/PhabricatorOAuthServerAuthController.php
+++ b/src/applications/oauthserver/controller/PhabricatorOAuthServerAuthController.php
@@ -11,140 +11,177 @@
}
public function processRequest() {
- $request = $this->getRequest();
- $current_user = $request->getUser();
+ $request = $this->getRequest();
+ $viewer = $request->getUser();
+
$server = new PhabricatorOAuthServer();
$client_phid = $request->getStr('client_id');
$scope = $request->getStr('scope', array());
$redirect_uri = $request->getStr('redirect_uri');
- $state = $request->getStr('state');
$response_type = $request->getStr('response_type');
- $response = new PhabricatorOAuthResponse();
// state is an opaque value the client sent us for their own purposes
// we just need to send it right back to them in the response!
- if ($state) {
- $response->setState($state);
- }
+ $state = $request->getStr('state');
+
if (!$client_phid) {
- $response->setError('invalid_request');
- $response->setErrorDescription(
- 'Required parameter client_id not specified.');
- return $response;
+ return $this->buildErrorResponse(
+ 'invalid_request',
+ pht('Malformed Request'),
+ pht(
+ 'Required parameter %s was not present in the request.',
+ phutil_tag('strong', array(), 'client_id')));
}
- $server->setUser($current_user);
+
+ $server->setUser($viewer);
+ $is_authorized = false;
+ $authorization = null;
+ $uri = null;
+ $name = null;
// one giant try / catch around all the exciting database stuff so we
// can return a 'server_error' response if something goes wrong!
try {
$client = id(new PhabricatorOAuthServerClient())
->loadOneWhere('phid = %s', $client_phid);
+
if (!$client) {
- $response->setError('invalid_request');
- $response->setErrorDescription(
- 'Client with id '.$client_phid.' not found.');
- return $response;
+ return $this->buildErrorResponse(
+ 'invalid_request',
+ pht('Invalid Client Application'),
+ pht(
+ 'Request parameter %s does not specify a valid client application.',
+ phutil_tag('strong', array(), 'client_id')));
}
+
+ $name = $client->getName();
$server->setClient($client);
if ($redirect_uri) {
$client_uri = new PhutilURI($client->getRedirectURI());
$redirect_uri = new PhutilURI($redirect_uri);
if (!($server->validateSecondaryRedirectURI($redirect_uri,
$client_uri))) {
- $response->setError('invalid_request');
- $response->setErrorDescription(
- 'The specified redirect URI is invalid. The redirect URI '.
- 'must be a fully-qualified domain with no fragments and '.
- 'must have the same domain and at least the same query '.
- 'parameters as the redirect URI the client registered.');
- return $response;
+ return $this->buildErrorResponse(
+ 'invalid_request',
+ pht('Invalid Redirect URI'),
+ pht(
+ 'Request parameter %s specifies an invalid redirect URI. '.
+ 'The redirect URI must be a fully-qualified domain with no '.
+ 'fragments, and must have the same domain and at least '.
+ 'the same query parameters as the redirect URI the client '.
+ 'registered.',
+ phutil_tag('strong', array(), 'redirect_uri')));
}
- $uri = $redirect_uri;
+ $uri = $redirect_uri;
} else {
- $uri = new PhutilURI($client->getRedirectURI());
+ $uri = new PhutilURI($client->getRedirectURI());
}
- // we've now validated this request enough overall such that we
- // can safely redirect to the client with the response
- $response->setClientURI($uri);
if (empty($response_type)) {
- $response->setError('invalid_request');
- $response->setErrorDescription(
- 'Required parameter response_type not specified.');
- return $response;
+ return $this->buildErrorResponse(
+ 'invalid_request',
+ pht('Invalid Response Type'),
+ pht(
+ 'Required request parameter %s is missing.',
+ phutil_tag('strong', array(), 'response_type')));
}
+
if ($response_type != 'code') {
- $response->setError('unsupported_response_type');
- $response->setErrorDescription(
- 'The authorization server does not support obtaining an '.
- 'authorization code using the specified response_type. '.
- 'You must specify the response_type as "code".');
- return $response;
+ return $this->buildErrorResponse(
+ 'unsupported_response_type',
+ pht('Unsupported Response Type'),
+ pht(
+ 'Request parameter %s specifies an unsupported response type. '.
+ 'Valid response types are: %s.',
+ phutil_tag('strong', array(), 'response_type'),
+ implode(', ', array('code'))));
}
+
if ($scope) {
if (!PhabricatorOAuthServerScope::validateScopesList($scope)) {
- $response->setError('invalid_scope');
- $response->setErrorDescription(
- 'The requested scope is invalid, unknown, or malformed.');
- return $response;
+ return $this->buildErrorResponse(
+ 'invalid_scope',
+ pht('Invalid Scope'),
+ pht(
+ 'Request parameter %s specifies an unsupported scope.',
+ phutil_tag('strong', array(), 'scope')));
}
$scope = PhabricatorOAuthServerScope::scopesListToDict($scope);
}
- list($is_authorized,
- $authorization) = $server->userHasAuthorizedClient($scope);
- if ($is_authorized) {
- $return_auth_code = true;
- $unguarded_write = AphrontWriteGuard::beginScopedUnguardedWrites();
- } else if ($request->isFormPost()) {
+ // NOTE: We're always requiring a confirmation dialog to redirect.
+ // Partly this is a general defense against redirect attacks, and
+ // partly this shakes off anchors in the URI (which are not shaken
+ // by 302'ing).
+
+ $auth_info = $server->userHasAuthorizedClient($scope);
+ list($is_authorized, $authorization) = $auth_info;
+
+ if ($request->isFormPost()) {
+ // TODO: We should probably validate this more? It feels a little funky.
$scope = PhabricatorOAuthServerScope::getScopesFromRequest($request);
+
if ($authorization) {
$authorization->setScope($scope)->save();
} else {
- $authorization = $server->authorizeClient($scope);
+ $authorization = $server->authorizeClient($scope);
}
- $return_auth_code = true;
- $unguarded_write = null;
- } else {
- $return_auth_code = false;
- $unguarded_write = null;
+
+ $is_authorized = true;
}
+ } catch (Exception $e) {
+ return $this->buildErrorResponse(
+ 'server_error',
+ pht('Server Error'),
+ pht(
+ 'The authorization server encountered an unexpected condition '.
+ 'which prevented it from fulfilling the request.'));
+ }
- if ($return_auth_code) {
- // step 1 -- generate authorization code
- $auth_code =
- $server->generateAuthorizationCode($uri);
+ // When we reach this part of the controller, we can be in two states:
+ //
+ // 1. The user has not authorized the application yet. We want to
+ // give them an "Authorize this application?" dialog.
+ // 2. The user has authorized the application. We want to give them
+ // a "Confirm Login" dialog.
- // step 2 return it
- $content = array(
+ if ($is_authorized) {
+
+ // The second case is simpler, so handle it first. The user either
+ // authorized the application previously, or has just authorized the
+ // application. Show them a confirm dialog with a normal link back to
+ // the application. This shakes anchors from the URI.
+
+ $unguarded = AphrontWriteGuard::beginScopedUnguardedWrites();
+ $auth_code = $server->generateAuthorizationCode($uri);
+ unset($unguarded);
+
+ $full_uri = $this->addQueryParams(
+ $uri,
+ array(
'code' => $auth_code->getCode(),
'scope' => $authorization->getScopeString(),
- );
- $response->setContent($content);
- return $response;
- }
- unset($unguarded_write);
- } catch (Exception $e) {
- // Note we could try harder to determine between a server_error
- // vs temporarily_unavailable. Good enough though.
- $response->setError('server_error');
- $response->setErrorDescription(
- 'The authorization server encountered an unexpected condition '.
- 'which prevented it from fulfilling the request. ');
- return $response;
- }
+ 'state' => $state,
+ ));
+
+ // TODO: It would be nice to give the user more options here, like
+ // reviewing permissions, canceling the authorization, or aborting
+ // the workflow.
- // display time -- make a nice form for the user to grant the client
- // access to the granularity specified by $scope
- $name = $client->getName();
- $title = pht('Authorize %s?', $name);
- $panel = new AphrontPanelView();
- $panel->setWidth(AphrontPanelView::WIDTH_FORM);
- $panel->setHeader($title);
+ $dialog = id(new AphrontDialogView())
+ ->setUser($viewer)
+ ->setTitle(pht('Authenticate: %s', $name))
+ ->appendParagraph(
+ pht(
+ 'This application ("%s") is authorized to use your Phabricator '.
+ 'credentials. Continue to complete the authentication workflow.',
+ phutil_tag('strong', array(), $name)))
+ ->addCancelButton((string)$full_uri, pht('Continue to Application'));
- $description =
- "Do want to authorize {$name} to access your ".
- "Phabricator account data?";
+ return id(new AphrontDialogResponse())->setDialog($dialog);
+ }
+
+ // Here, we're confirming authorization for the application.
if ($scope) {
if ($authorization) {
@@ -154,10 +191,10 @@
$desired_scopes = $scope;
}
if (!PhabricatorOAuthServerScope::validateScopesDict($desired_scopes)) {
- $response->setError('invalid_scope');
- $response->setErrorDescription(
- 'The requested scope is invalid, unknown, or malformed.');
- return $response;
+ return $this->buildErrorResponse(
+ 'invalid_scope',
+ pht('Invalid Scope'),
+ pht('The requested scope is invalid, unknown, or malformed.'));
}
} else {
$desired_scopes = array(
@@ -166,31 +203,67 @@
);
}
- $cancel_uri = clone $uri;
- $cancel_params = array(
- 'error' => 'access_denied',
- 'error_description' =>
- 'The resource owner (aka the user) denied the request.'
- );
- $cancel_uri->setQueryParams($cancel_params);
-
$form = id(new AphrontFormView())
- ->setUser($current_user)
+ ->addHiddenInput('client_id', $client_phid)
+ ->addHiddenInput('redirect_uri', $redirect_uri)
+ ->addHiddenInput('response_type', $response_type)
+ ->addHiddenInput('state', $state)
+ ->addHiddenInput('scope', $request->getStr('scope'))
+ ->setUser($viewer)
->appendChild(
- id(new AphrontFormStaticControl())
- ->setValue($description))
- ->appendChild(
- PhabricatorOAuthServerScope::getCheckboxControl($desired_scopes))
- ->appendChild(
- id(new AphrontFormSubmitControl())
- ->setValue('Authorize')
- ->addCancelButton($cancel_uri));
+ PhabricatorOAuthServerScope::getCheckboxControl($desired_scopes));
+
+ $cancel_msg = pht('The user declined to authorize this application.');
+ $cancel_uri = $this->addQueryParams(
+ $uri,
+ array(
+ 'error' => 'access_denied',
+ 'error_description' => $cancel_msg,
+ ));
+
+ $dialog = id(new AphrontDialogView())
+ ->setUser($viewer)
+ ->setTitle(pht('Authorize "%s"?', $name))
+ ->setSubmitURI($request->getRequestURI()->getPath())
+ ->setWidth(AphrontDialogView::WIDTH_FORM)
+ ->appendParagraph(
+ pht(
+ 'Do you want to authorize the external application "%s" to '.
+ 'access your Phabricator account data?',
+ phutil_tag('strong', array(), $name)))
+ ->appendChild($form->buildLayoutView())
+ ->addSubmitButton(pht('Authorize Access'))
+ ->addCancelButton((string)$cancel_uri, pht('Do Not Authorize'));
+
+ return id(new AphrontDialogResponse())->setDialog($dialog);
+ }
+
+
+ private function buildErrorResponse($code, $title, $message) {
+ $viewer = $this->getRequest()->getUser();
+
+ $dialog = id(new AphrontDialogView())
+ ->setUser($viewer)
+ ->setTitle(pht('OAuth: %s', $title))
+ ->appendParagraph($message)
+ ->appendParagraph(
+ pht('OAuth Error Code: %s', phutil_tag('tt', array(), $code)))
+ ->addCancelButton('/', pht('Alas!'));
+
+ return id(new AphrontDialogResponse())->setDialog($dialog);
+ }
+
+
+ private function addQueryParams(PhutilURI $uri, array $params) {
+ $full_uri = clone $uri;
- $panel->appendChild($form);
+ foreach ($params as $key => $value) {
+ if (strlen($value)) {
+ $full_uri->setQueryParam($key, $value);
+ }
+ }
- return $this->buildStandardPageResponse(
- $panel,
- array('title' => $title));
+ return $full_uri;
}
}
diff --git a/src/applications/oauthserver/controller/PhabricatorOAuthServerConsoleController.php b/src/applications/oauthserver/controller/PhabricatorOAuthServerConsoleController.php
new file mode 100644
--- /dev/null
+++ b/src/applications/oauthserver/controller/PhabricatorOAuthServerConsoleController.php
@@ -0,0 +1,43 @@
+<?php
+
+final class PhabricatorOAuthServerConsoleController
+ extends PhabricatorOAuthServerController {
+
+ public function processRequest() {
+ $request = $this->getRequest();
+ $viewer = $request->getUser();
+
+ $menu = id(new PHUIObjectItemListView())
+ ->setUser($viewer);
+
+ $menu->addItem(
+ id(new PHUIObjectItemView())
+ ->setHeader(pht('Authorizations'))
+ ->setHref($this->getApplicationURI('clientauthorization/'))
+ ->addAttribute(
+ pht(
+ 'Review your authorizations.')));
+
+ $menu->addItem(
+ id(new PHUIObjectItemView())
+ ->setHeader(pht('Applications'))
+ ->setHref($this->getApplicationURI('client/'))
+ ->addAttribute(
+ pht(
+ 'Create a new OAuth application.')));
+
+ $crumbs = $this->buildApplicationCrumbs();
+ $crumbs->addTextCrumb(pht('Console'));
+
+ return $this->buildApplicationPage(
+ array(
+ $crumbs,
+ $menu,
+ ),
+ array(
+ 'title' => pht('OAuth Server Console'),
+ 'device' => true,
+ ));
+ }
+
+}

File Metadata

Mime Type
text/plain
Expires
Mon, Nov 11, 1:04 AM (1 w, 1 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6720365
Default Alt Text
D8517.diff (21 KB)

Event Timeline

Phabricator · Built by Phacility