revi-archive/phorge-phorge
1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-25 16:22:43 +01:00
Code Issues Releases Wiki Activity

Allow OAuth applications to be disabled instead of destroyed

Browse source 
Summary: Ref T7303. This interaction is very oldschool; modernize it to enable/disable instead of "nuke from orbit".

Test Plan:
  - Enabled applications.
  - Disabled applications.
  - Viewed applications in list view.
  - Generated new tokens.
  - Tried to use a token from a disabled application (got rebuffed).
  - Tried to use a token from an enabled application (worked fine).

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T7303

Differential Revision: https://secure.phabricator.com/D15620
This commit is contained in:
epriestley 2016-04-05 09:40:22 -07:00
parent c29bbbab19
commit 8dfc7d4201
14 changed files with 189 additions and 102 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
resources/sql/autopatches/20160405.oauth.2.disable.sql Normal file
View file

@ -0,0 +1,2 @@
ALTER TABLE {$NAMESPACE}_oauth_server.oauth_server_oauthserverclient
ADD isDisabled BOOL NOT NULL;

4
src/__phutil_library_map__.php
View file

@ -2702,7 +2702,7 @@ phutil_register_library_map(array(
'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 @@ phutil_register_library_map(array(
),
'PhabricatorOAuthClientAuthorizationQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
'PhabricatorOAuthClientController' => 'PhabricatorOAuthServerController',
'PhabricatorOAuthClientDeleteController' => 'PhabricatorOAuthClientController',
'PhabricatorOAuthClientDisableController' => 'PhabricatorOAuthClientController',
'PhabricatorOAuthClientEditController' => 'PhabricatorOAuthClientController',
'PhabricatorOAuthClientListController' => 'PhabricatorOAuthClientController',
'PhabricatorOAuthClientSecretController' => 'PhabricatorOAuthClientController',

5
src/applications/oauthserver/PhabricatorOAuthServer.php
View file

@ -172,6 +172,11 @@ final class PhabricatorOAuthServer extends Phobject {
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.

4
src/applications/oauthserver/application/PhabricatorOAuthServerApplication.php
View file

@ -53,8 +53,8 @@ final class PhabricatorOAuthServerApplication extends PhabricatorApplication {
'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',

9
src/applications/oauthserver/controller/PhabricatorOAuthServerAuthController.php
View file

@ -62,6 +62,15 @@ final class PhabricatorOAuthServerAuthController
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) {

19
src/applications/oauthserver/controller/PhabricatorOAuthServerTokenController.php
View file

@ -67,7 +67,7 @@ final class PhabricatorOAuthServerTokenController
$response->setError('invalid_grant');
$response->setErrorDescription(
pht(
'Authorization code %d not found.',
'Authorization code %s not found.',
$code));
return $response;
}
@ -102,11 +102,22 @@ final class PhabricatorOAuthServerTokenController
$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 @@ final class PhabricatorOAuthServerTokenController
$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 @@ final class PhabricatorOAuthServerTokenController
$response->setError('invalid_grant');
$response->setErrorDescription(
pht(
'Invalid authorization code %d.',
'Invalid authorization code %s.',
$code));
return $response;
}

40
src/applications/oauthserver/controller/client/PhabricatorOAuthClientDeleteController.php
View file

@ -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'));
}
}

67
src/applications/oauthserver/controller/client/PhabricatorOAuthClientDisableController.php Normal file
View file

@ -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);
}
}

55
src/applications/oauthserver/controller/client/PhabricatorOAuthClientTestController.php
View file

@ -15,36 +15,41 @@ final class PhabricatorOAuthClientTestController
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);
}
$id = $authorization->getID();
$panel_uri = '/settings/panel/oauthorizations/?id='.$id;
$access_token = $server->generateAccessToken();
return id(new AphrontRedirectResponse())->setURI($panel_uri);
$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 $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 @@ final class PhabricatorOAuthClientTestController
->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'));
}
}

42
src/applications/oauthserver/controller/client/PhabricatorOAuthClientViewController.php
View file

@ -51,6 +51,12 @@ final class PhabricatorOAuthClientViewController
->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 @@ final class PhabricatorOAuthClientViewController
$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 @@ final class PhabricatorOAuthClientViewController
->setDisabled(!$can_edit)
->setWorkflow(true));
$view->addAction(
id(new PhabricatorActionView())
->setName(pht('Delete Application'))
->setIcon('fa-times')
->setWorkflow(true)
->setDisabled(!$can_edit)
->setHref($client->getDeleteURI()));
$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('Create Test Authorization'))
->setIcon('fa-wrench')
->setName($disable_text)
->setIcon($disable_icon)
->setWorkflow(true)
->setDisabled(!$can_edit)
->setHref($disable_uri));
$view->addAction(
id(new PhabricatorActionView())
->setName(pht('Generate Test Token'))
->setIcon('fa-plus')
->setWorkflow(true)
->setDisabled($is_authorized)
->setHref($this->getApplicationURI("client/test/{$id}/")));
return $view;

9
src/applications/oauthserver/editor/PhabricatorOAuthServerEditor.php
View file

@ -16,6 +16,7 @@ final class PhabricatorOAuthServerEditor
$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 @@ final class PhabricatorOAuthServerEditor
return $object->getName();
case PhabricatorOAuthServerTransaction::TYPE_REDIRECT_URI:
return $object->getRedirectURI();
case PhabricatorOAuthServerTransaction::TYPE_DISABLED:
return $object->getIsDisabled();
}
}
@ -43,6 +46,8 @@ final class PhabricatorOAuthServerEditor
case PhabricatorOAuthServerTransaction::TYPE_NAME:
case PhabricatorOAuthServerTransaction::TYPE_REDIRECT_URI:
return $xaction->getNewValue();
case PhabricatorOAuthServerTransaction::TYPE_DISABLED:
return (int)$xaction->getNewValue();
}
}
@ -57,6 +62,9 @@ final class PhabricatorOAuthServerEditor
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 @@ final class PhabricatorOAuthServerEditor
switch ($xaction->getTransactionType()) {
case PhabricatorOAuthServerTransaction::TYPE_NAME:
case PhabricatorOAuthServerTransaction::TYPE_REDIRECT_URI:
case PhabricatorOAuthServerTransaction::TYPE_DISABLED:
return;
}

6
src/applications/oauthserver/query/PhabricatorOAuthServerClientSearchEngine.php
View file

@ -73,7 +73,7 @@ final class PhabricatorOAuthServerClientSearchEngine
array $clients,
PhabricatorSavedQuery $query,
array $handles) {
assert_instances_of($clients, 'PhabricatorOauthServerClient');
assert_instances_of($clients, 'PhabricatorOAuthServerClient');
$viewer = $this->requireViewer();
@ -86,6 +86,10 @@ final class PhabricatorOAuthServerClientSearchEngine
->setHref($client->getViewURI())
->setObject($client);
if ($client->getIsDisabled()) {
$item->setDisabled(true);
}
$list->addItem($item);
}

18
src/applications/oauthserver/storage/PhabricatorOAuthServerClient.php
View file

@ -11,9 +11,10 @@ final class PhabricatorOAuthServerClient
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 @@ final class PhabricatorOAuthServerClient
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 @@ final class PhabricatorOAuthServerClient
'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'),
),

11
src/applications/oauthserver/storage/PhabricatorOAuthServerTransaction.php
View file

@ -5,6 +5,7 @@ final class PhabricatorOAuthServerTransaction
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 @@ final class PhabricatorOAuthServerTransaction
$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();