Page Menu
Home
Phabricator
Search
Configure Global Search
Log In
Files
F13995142
D15620.id37643.diff
No One
Temporary
Actions
View File
Edit File
Delete File
View Transforms
Subscribe
Mute Notifications
Award Token
Flag For Later
Size
21 KB
Referenced Files
None
Subscribers
None
D15620.id37643.diff
View Options
diff --git a/resources/sql/autopatches/20160405.oauth.2.disable.sql b/resources/sql/autopatches/20160405.oauth.2.disable.sql
new file mode 100644
--- /dev/null
+++ b/resources/sql/autopatches/20160405.oauth.2.disable.sql
@@ -0,0 +1,2 @@
+ALTER TABLE {$NAMESPACE}_oauth_server.oauth_server_oauthserverclient
+ ADD isDisabled BOOL NOT NULL;
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
@@ -2702,7 +2702,7 @@
'PhabricatorOAuthClientAuthorization' => 'applications/oauthserver/storage/PhabricatorOAuthClientAuthorization.php',
'PhabricatorOAuthClientAuthorizationQuery' => 'applications/oauthserver/query/PhabricatorOAuthClientAuthorizationQuery.php',
'PhabricatorOAuthClientController' => 'applications/oauthserver/controller/client/PhabricatorOAuthClientController.php',
- 'PhabricatorOAuthClientDeleteController' => 'applications/oauthserver/controller/client/PhabricatorOAuthClientDeleteController.php',
+ 'PhabricatorOAuthClientDisableController' => 'applications/oauthserver/controller/client/PhabricatorOAuthClientDisableController.php',
'PhabricatorOAuthClientEditController' => 'applications/oauthserver/controller/client/PhabricatorOAuthClientEditController.php',
'PhabricatorOAuthClientListController' => 'applications/oauthserver/controller/client/PhabricatorOAuthClientListController.php',
'PhabricatorOAuthClientSecretController' => 'applications/oauthserver/controller/client/PhabricatorOAuthClientSecretController.php',
@@ -7194,7 +7194,7 @@
),
'PhabricatorOAuthClientAuthorizationQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
'PhabricatorOAuthClientController' => 'PhabricatorOAuthServerController',
- 'PhabricatorOAuthClientDeleteController' => 'PhabricatorOAuthClientController',
+ 'PhabricatorOAuthClientDisableController' => 'PhabricatorOAuthClientController',
'PhabricatorOAuthClientEditController' => 'PhabricatorOAuthClientController',
'PhabricatorOAuthClientListController' => 'PhabricatorOAuthClientController',
'PhabricatorOAuthClientSecretController' => 'PhabricatorOAuthClientController',
diff --git a/src/applications/oauthserver/PhabricatorOAuthServer.php b/src/applications/oauthserver/PhabricatorOAuthServer.php
--- a/src/applications/oauthserver/PhabricatorOAuthServer.php
+++ b/src/applications/oauthserver/PhabricatorOAuthServer.php
@@ -172,6 +172,11 @@
return null;
}
+ $application = $authorization->getClient();
+ if ($application->getIsDisabled()) {
+ return null;
+ }
+
// TODO: This should probably be reworked; expiration should be an
// exclusive property of the token. For now, this logic reads: tokens for
// authorizations with "offline_access" never expire.
diff --git a/src/applications/oauthserver/application/PhabricatorOAuthServerApplication.php b/src/applications/oauthserver/application/PhabricatorOAuthServerApplication.php
--- a/src/applications/oauthserver/application/PhabricatorOAuthServerApplication.php
+++ b/src/applications/oauthserver/application/PhabricatorOAuthServerApplication.php
@@ -53,8 +53,8 @@
'token/' => 'PhabricatorOAuthServerTokenController',
$this->getEditRoutePattern('edit/') =>
'PhabricatorOAuthClientEditController',
- 'client/' => array(
- 'delete/(?P<id>\d+)/' => 'PhabricatorOAuthClientDeleteController',
+ 'client/' => array(
+ 'disable/(?P<id>\d+)/' => 'PhabricatorOAuthClientDisableController',
'view/(?P<id>\d+)/' => 'PhabricatorOAuthClientViewController',
'secret/(?P<id>\d+)/' => 'PhabricatorOAuthClientSecretController',
'test/(?P<id>\d+)/' => 'PhabricatorOAuthClientTestController',
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
@@ -62,6 +62,15 @@
phutil_tag('strong', array(), 'client_id')));
}
+ if ($client->getIsDisabled()) {
+ return $this->buildErrorResponse(
+ 'invalid_request',
+ pht('Application Disabled'),
+ pht(
+ 'The %s OAuth application has been disabled.',
+ phutil_tag('strong', array(), 'client_id')));
+ }
+
$name = $client->getName();
$server->setClient($client);
if ($redirect_uri) {
diff --git a/src/applications/oauthserver/controller/PhabricatorOAuthServerTokenController.php b/src/applications/oauthserver/controller/PhabricatorOAuthServerTokenController.php
--- a/src/applications/oauthserver/controller/PhabricatorOAuthServerTokenController.php
+++ b/src/applications/oauthserver/controller/PhabricatorOAuthServerTokenController.php
@@ -67,7 +67,7 @@
$response->setError('invalid_grant');
$response->setErrorDescription(
pht(
- 'Authorization code %d not found.',
+ 'Authorization code %s not found.',
$code));
return $response;
}
@@ -102,11 +102,22 @@
$response->setError('invalid_client');
$response->setErrorDescription(
pht(
- 'Client with %s %d not found.',
+ 'Client with %s %s not found.',
'client_id',
$client_phid));
return $response;
}
+
+ if ($client->getIsDisabled()) {
+ $response->setError('invalid_client');
+ $response->setErrorDescription(
+ pht(
+ 'OAuth application "%s" has been disabled.',
+ $client->getName()));
+
+ return $response;
+ }
+
$server->setClient($client);
$user_phid = $auth_code->getUserPHID();
@@ -116,7 +127,7 @@
$response->setError('invalid_grant');
$response->setErrorDescription(
pht(
- 'User with PHID %d not found.',
+ 'User with PHID %s not found.',
$user_phid));
return $response;
}
@@ -132,7 +143,7 @@
$response->setError('invalid_grant');
$response->setErrorDescription(
pht(
- 'Invalid authorization code %d.',
+ 'Invalid authorization code %s.',
$code));
return $response;
}
diff --git a/src/applications/oauthserver/controller/client/PhabricatorOAuthClientDeleteController.php b/src/applications/oauthserver/controller/client/PhabricatorOAuthClientDeleteController.php
deleted file mode 100644
--- a/src/applications/oauthserver/controller/client/PhabricatorOAuthClientDeleteController.php
+++ /dev/null
@@ -1,40 +0,0 @@
-<?php
-
-final class PhabricatorOAuthClientDeleteController
- extends PhabricatorOAuthClientController {
-
- public function handleRequest(AphrontRequest $request) {
- $viewer = $this->getViewer();
-
- $client = id(new PhabricatorOAuthServerClientQuery())
- ->setViewer($viewer)
- ->withIDs(array($request->getURIData('id')))
- ->requireCapabilities(
- array(
- PhabricatorPolicyCapability::CAN_VIEW,
- PhabricatorPolicyCapability::CAN_EDIT,
- ))
- ->executeOne();
- if (!$client) {
- return new Aphront404Response();
- }
-
- // TODO: This should be "disable", not "delete"!
-
- if ($request->isFormPost()) {
- $client->delete();
- $app_uri = $this->getApplicationURI();
- return id(new AphrontRedirectResponse())->setURI($app_uri);
- }
-
- return $this->newDialog()
- ->setTitle(pht('Delete OAuth Application?'))
- ->appendParagraph(
- pht(
- 'Really delete the OAuth application %s?',
- phutil_tag('strong', array(), $client->getName())))
- ->addCancelButton($client->getViewURI())
- ->addSubmitButton(pht('Delete Application'));
- }
-
-}
diff --git a/src/applications/oauthserver/controller/client/PhabricatorOAuthClientDisableController.php b/src/applications/oauthserver/controller/client/PhabricatorOAuthClientDisableController.php
new file mode 100644
--- /dev/null
+++ b/src/applications/oauthserver/controller/client/PhabricatorOAuthClientDisableController.php
@@ -0,0 +1,67 @@
+<?php
+
+final class PhabricatorOAuthClientDisableController
+ extends PhabricatorOAuthClientController {
+
+ public function handleRequest(AphrontRequest $request) {
+ $viewer = $this->getViewer();
+
+ $client = id(new PhabricatorOAuthServerClientQuery())
+ ->setViewer($viewer)
+ ->withIDs(array($request->getURIData('id')))
+ ->requireCapabilities(
+ array(
+ PhabricatorPolicyCapability::CAN_VIEW,
+ PhabricatorPolicyCapability::CAN_EDIT,
+ ))
+ ->executeOne();
+ if (!$client) {
+ return new Aphront404Response();
+ }
+
+ $done_uri = $client->getViewURI();
+ $is_disable = !$client->getIsDisabled();
+
+ if ($request->isFormPost()) {
+ $xactions = array();
+
+ $xactions[] = id(new PhabricatorOAuthServerTransaction())
+ ->setTransactionType(PhabricatorOAuthServerTransaction::TYPE_DISABLED)
+ ->setNewValue((int)$is_disable);
+
+ $editor = id(new PhabricatorOAuthServerEditor())
+ ->setActor($viewer)
+ ->setContentSourceFromRequest($request)
+ ->setContinueOnNoEffect(true)
+ ->setContinueOnMissingFields(true)
+ ->applyTransactions($client, $xactions);
+
+ return id(new AphrontRedirectResponse())->setURI($done_uri);
+ }
+
+ if ($is_disable) {
+ $title = pht('Disable OAuth Application');
+ $body = pht(
+ 'Really disable the %s OAuth application? Users will no longer be '.
+ 'able to authenticate against it, nor access Phabricator using '.
+ 'tokens generated by this application.',
+ phutil_tag('strong', array(), $client->getName()));
+ $button = pht('Disable Application');
+ } else {
+ $title = pht('Enable OAuth Application');
+ $body = pht(
+ 'Really enable the %s OAuth application? Users will be able to '.
+ 'authenticate against it, and existing tokens will become usable '.
+ 'again.',
+ phutil_tag('strong', array(), $client->getName()));
+ $button = pht('Enable Application');
+ }
+
+ return $this->newDialog()
+ ->setTitle($title)
+ ->appendParagraph($body)
+ ->addCancelButton($done_uri)
+ ->addSubmitButton($button);
+ }
+
+}
diff --git a/src/applications/oauthserver/controller/client/PhabricatorOAuthClientTestController.php b/src/applications/oauthserver/controller/client/PhabricatorOAuthClientTestController.php
--- a/src/applications/oauthserver/controller/client/PhabricatorOAuthClientTestController.php
+++ b/src/applications/oauthserver/controller/client/PhabricatorOAuthClientTestController.php
@@ -15,36 +15,41 @@
return new Aphront404Response();
}
- $view_uri = $client->getViewURI();
-
- // Look for an existing authorization.
- $authorization = id(new PhabricatorOAuthClientAuthorizationQuery())
- ->setViewer($viewer)
- ->withUserPHIDs(array($viewer->getPHID()))
- ->withClientPHIDs(array($client->getPHID()))
- ->executeOne();
- if ($authorization) {
- return $this->newDialog()
- ->setTitle(pht('Already Authorized'))
- ->appendParagraph(
- pht(
- 'You have already authorized this application to access your '.
- 'account.'))
- ->addCancelButton($view_uri, pht('Close'));
- }
+ $done_uri = $client->getViewURI();
if ($request->isFormPost()) {
$server = id(new PhabricatorOAuthServer())
->setUser($viewer)
->setClient($client);
- $scope = array();
- $authorization = $server->authorizeClient($scope);
+ // Create an authorization if we don't already have one.
+ $authorization = id(new PhabricatorOAuthClientAuthorizationQuery())
+ ->setViewer($viewer)
+ ->withUserPHIDs(array($viewer->getPHID()))
+ ->withClientPHIDs(array($client->getPHID()))
+ ->executeOne();
+ if (!$authorization) {
+ $scope = array();
+ $authorization = $server->authorizeClient($scope);
+ }
+
+ $access_token = $server->generateAccessToken();
- $id = $authorization->getID();
- $panel_uri = '/settings/panel/oauthorizations/?id='.$id;
+ $form = id(new AphrontFormView())
+ ->setViewer($viewer)
+ ->appendInstructions(
+ pht(
+ 'Keep this token private, it allows any bearer to access '.
+ 'your account on behalf of this application.'))
+ ->appendChild(
+ id(new AphrontFormTextControl())
+ ->setLabel(pht('Token'))
+ ->setValue($access_token->getToken()));
- return id(new AphrontRedirectResponse())->setURI($panel_uri);
+ return $this->newDialog()
+ ->setTitle(pht('OAuth Access Token'))
+ ->appendForm($form)
+ ->addCancelButton($done_uri, pht('Close'));
}
// TODO: It would be nice to put scope options in this dialog, maybe?
@@ -53,10 +58,10 @@
->setTitle(pht('Authorize Application?'))
->appendParagraph(
pht(
- 'This will create an authorization, permitting %s to access '.
- 'your account.',
+ 'This will create an authorization and OAuth token, permitting %s '.
+ 'to access your account.',
phutil_tag('strong', array(), $client->getName())))
- ->addCancelButton($view_uri)
+ ->addCancelButton($done_uri)
->addSubmitButton(pht('Authorize Application'));
}
}
diff --git a/src/applications/oauthserver/controller/client/PhabricatorOAuthClientViewController.php b/src/applications/oauthserver/controller/client/PhabricatorOAuthClientViewController.php
--- a/src/applications/oauthserver/controller/client/PhabricatorOAuthClientViewController.php
+++ b/src/applications/oauthserver/controller/client/PhabricatorOAuthClientViewController.php
@@ -51,6 +51,12 @@
->setHeader(pht('OAuth Application: %s', $client->getName()))
->setPolicyObject($client);
+ if ($client->getIsDisabled()) {
+ $header->setStatus('fa-ban', 'indigo', pht('Disabled'));
+ } else {
+ $header->setStatus('fa-check', 'green', pht('Enabled'));
+ }
+
return $header;
}
@@ -62,12 +68,6 @@
$client,
PhabricatorPolicyCapability::CAN_EDIT);
- $authorization = id(new PhabricatorOAuthClientAuthorizationQuery())
- ->setViewer($viewer)
- ->withUserPHIDs(array($viewer->getPHID()))
- ->withClientPHIDs(array($client->getPHID()))
- ->executeOne();
- $is_authorized = (bool)$authorization;
$id = $client->getID();
$view = id(new PhabricatorActionListView())
@@ -89,20 +89,30 @@
->setDisabled(!$can_edit)
->setWorkflow(true));
+ $is_disabled = $client->getIsDisabled();
+ if ($is_disabled) {
+ $disable_text = pht('Enable Application');
+ $disable_icon = 'fa-check';
+ } else {
+ $disable_text = pht('Disable Application');
+ $disable_icon = 'fa-ban';
+ }
+
+ $disable_uri = $this->getApplicationURI("client/disable/{$id}/");
+
$view->addAction(
id(new PhabricatorActionView())
- ->setName(pht('Delete Application'))
- ->setIcon('fa-times')
+ ->setName($disable_text)
+ ->setIcon($disable_icon)
->setWorkflow(true)
->setDisabled(!$can_edit)
- ->setHref($client->getDeleteURI()));
+ ->setHref($disable_uri));
$view->addAction(
id(new PhabricatorActionView())
- ->setName(pht('Create Test Authorization'))
- ->setIcon('fa-wrench')
+ ->setName(pht('Generate Test Token'))
+ ->setIcon('fa-plus')
->setWorkflow(true)
- ->setDisabled($is_authorized)
->setHref($this->getApplicationURI("client/test/{$id}/")));
return $view;
diff --git a/src/applications/oauthserver/editor/PhabricatorOAuthServerEditor.php b/src/applications/oauthserver/editor/PhabricatorOAuthServerEditor.php
--- a/src/applications/oauthserver/editor/PhabricatorOAuthServerEditor.php
+++ b/src/applications/oauthserver/editor/PhabricatorOAuthServerEditor.php
@@ -16,6 +16,7 @@
$types[] = PhabricatorOAuthServerTransaction::TYPE_NAME;
$types[] = PhabricatorOAuthServerTransaction::TYPE_REDIRECT_URI;
+ $types[] = PhabricatorOAuthServerTransaction::TYPE_DISABLED;
$types[] = PhabricatorTransactions::TYPE_VIEW_POLICY;
$types[] = PhabricatorTransactions::TYPE_EDIT_POLICY;
@@ -32,6 +33,8 @@
return $object->getName();
case PhabricatorOAuthServerTransaction::TYPE_REDIRECT_URI:
return $object->getRedirectURI();
+ case PhabricatorOAuthServerTransaction::TYPE_DISABLED:
+ return $object->getIsDisabled();
}
}
@@ -43,6 +46,8 @@
case PhabricatorOAuthServerTransaction::TYPE_NAME:
case PhabricatorOAuthServerTransaction::TYPE_REDIRECT_URI:
return $xaction->getNewValue();
+ case PhabricatorOAuthServerTransaction::TYPE_DISABLED:
+ return (int)$xaction->getNewValue();
}
}
@@ -57,6 +62,9 @@
case PhabricatorOAuthServerTransaction::TYPE_REDIRECT_URI:
$object->setRedirectURI($xaction->getNewValue());
return;
+ case PhabricatorOAuthServerTransaction::TYPE_DISABLED:
+ $object->setIsDisabled($xaction->getNewValue());
+ return;
}
return parent::applyCustomInternalTransaction($object, $xaction);
@@ -69,6 +77,7 @@
switch ($xaction->getTransactionType()) {
case PhabricatorOAuthServerTransaction::TYPE_NAME:
case PhabricatorOAuthServerTransaction::TYPE_REDIRECT_URI:
+ case PhabricatorOAuthServerTransaction::TYPE_DISABLED:
return;
}
diff --git a/src/applications/oauthserver/query/PhabricatorOAuthServerClientSearchEngine.php b/src/applications/oauthserver/query/PhabricatorOAuthServerClientSearchEngine.php
--- a/src/applications/oauthserver/query/PhabricatorOAuthServerClientSearchEngine.php
+++ b/src/applications/oauthserver/query/PhabricatorOAuthServerClientSearchEngine.php
@@ -73,7 +73,7 @@
array $clients,
PhabricatorSavedQuery $query,
array $handles) {
- assert_instances_of($clients, 'PhabricatorOauthServerClient');
+ assert_instances_of($clients, 'PhabricatorOAuthServerClient');
$viewer = $this->requireViewer();
@@ -86,6 +86,10 @@
->setHref($client->getViewURI())
->setObject($client);
+ if ($client->getIsDisabled()) {
+ $item->setDisabled(true);
+ }
+
$list->addItem($item);
}
diff --git a/src/applications/oauthserver/storage/PhabricatorOAuthServerClient.php b/src/applications/oauthserver/storage/PhabricatorOAuthServerClient.php
--- a/src/applications/oauthserver/storage/PhabricatorOAuthServerClient.php
+++ b/src/applications/oauthserver/storage/PhabricatorOAuthServerClient.php
@@ -11,9 +11,10 @@
protected $name;
protected $redirectURI;
protected $creatorPHID;
- protected $isTrusted = 0;
+ protected $isTrusted;
protected $viewPolicy;
protected $editPolicy;
+ protected $isDisabled;
public function getEditURI() {
$id = $this->getID();
@@ -25,17 +26,14 @@
return "/oauthserver/client/view/{$id}/";
}
- public function getDeleteURI() {
- $id = $this->getID();
- return "/oauthserver/client/delete/{$id}/";
- }
-
public static function initializeNewClient(PhabricatorUser $actor) {
return id(new PhabricatorOAuthServerClient())
->setCreatorPHID($actor->getPHID())
->setSecret(Filesystem::readRandomCharacters(32))
->setViewPolicy(PhabricatorPolicies::POLICY_USER)
- ->setEditPolicy($actor->getPHID());
+ ->setEditPolicy($actor->getPHID())
+ ->setIsDisabled(0)
+ ->setIsTrusted(0);
}
protected function getConfiguration() {
@@ -46,13 +44,9 @@
'secret' => 'text32',
'redirectURI' => 'text255',
'isTrusted' => 'bool',
+ 'isDisabled' => 'bool',
),
self::CONFIG_KEY_SCHEMA => array(
- 'key_phid' => null,
- 'phid' => array(
- 'columns' => array('phid'),
- 'unique' => true,
- ),
'creatorPHID' => array(
'columns' => array('creatorPHID'),
),
diff --git a/src/applications/oauthserver/storage/PhabricatorOAuthServerTransaction.php b/src/applications/oauthserver/storage/PhabricatorOAuthServerTransaction.php
--- a/src/applications/oauthserver/storage/PhabricatorOAuthServerTransaction.php
+++ b/src/applications/oauthserver/storage/PhabricatorOAuthServerTransaction.php
@@ -5,6 +5,7 @@
const TYPE_NAME = 'oauthserver.name';
const TYPE_REDIRECT_URI = 'oauthserver.redirect-uri';
+ const TYPE_DISABLED = 'oauthserver.disabled';
public function getApplicationName() {
return 'oauth_server';
@@ -44,6 +45,16 @@
$this->renderHandleLink($author_phid),
$old,
$new);
+ case self::TYPE_DISABLED:
+ if ($new) {
+ return pht(
+ '%s disabled this application.',
+ $this->renderHandleLink($author_phid));
+ } else {
+ return pht(
+ '%s enabled this application.',
+ $this->renderHandleLink($author_phid));
+ }
}
return parent::getTitle();
File Metadata
Details
Attached
Mime Type
text/plain
Expires
Thu, Oct 24, 10:54 AM (3 w, 4 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6712140
Default Alt Text
D15620.id37643.diff (21 KB)
Attached To
Mode
D15620: Allow OAuth applications to be disabled instead of destroyed
Attached
Detach File
Event Timeline
Log In to Comment