Page MenuHomePhabricator
Files F14001828

D11705.id28168.diff
No OneTemporary
Actions

D11705.id28168.diff
View Options

diff --git a/resources/sql/autopatches/20150206.oauthclient.scope.sql b/resources/sql/autopatches/20150206.oauthclient.scope.sql
new file mode 100644
--- /dev/null
+++ b/resources/sql/autopatches/20150206.oauthclient.scope.sql
@@ -0,0 +1,5 @@
+ALTER TABLE {$NAMESPACE}_oauth_server.oauth_server_oauthserverclient
+ ADD scope LONGTEXT NOT NULL COLLATE {$COLLATE_TEXT};
+
+UPDATE {$NAMESPACE}_oauth_server.oauth_server_oauthserverclient
+ SET scope = '{"whoami":1}' WHERE scope = '';
diff --git a/src/applications/oauthserver/PhabricatorOAuthServerScope.php b/src/applications/oauthserver/PhabricatorOAuthServerScope.php
--- a/src/applications/oauthserver/PhabricatorOAuthServerScope.php
+++ b/src/applications/oauthserver/PhabricatorOAuthServerScope.php
@@ -18,22 +18,64 @@
);
}
- static public function getCheckboxControl($current_scopes) {
+ static public function getDefaultScope() {
+ return self::SCOPE_WHOAMI;
+ }
+
+ static public function getDefaultScopeDict() {
+ return array(self::getDefaultScope() => 1);
+ }
+
+ static public function getUserCheckboxControl(
+ array $current_scopes,
+ array $client_scopes) {
+ return self::getCheckboxControl(
+ $current_scopes,
+ array_diff_key(self::getScopesDict(), $client_scopes),
+ pht('Scope'));
+ }
+
+ static public function getClientCheckboxControl(array $current_scopes) {
+ return self::getCheckboxControl(
+ $current_scopes,
+ array(),
+ pht('Request Scope'));
+ }
+
+ static private function getCheckboxControl(
+ array $current_scopes,
+ array $blacklist_scopes,
+ $label) {
+
+ $have_options = false;
$scopes = self::getScopesDict();
$scope_keys = array_keys($scopes);
sort($scope_keys);
+ $default_scope = self::getDefaultScope();
$checkboxes = new AphrontFormCheckboxControl();
foreach ($scope_keys as $scope) {
+ if ($scope == $default_scope) {
+ continue;
+ }
+ if (isset($blacklist_scopes[$scope])) {
+ continue;
+ }
+
$checkboxes->addCheckbox(
$name = $scope,
$value = 1,
- $label = self::getCheckboxLabel($scope),
+ $check_label = self::getCheckboxLabel($scope),
$checked = isset($current_scopes[$scope]));
+ $have_options = true;
+ }
+
+ if ($have_options) {
+ $checkboxes->setLabel($label);
+ return $checkboxes;
}
- $checkboxes->setLabel('Scope');
- return $checkboxes;
+ return null;
}
static private function getCheckboxLabel($scope) {
@@ -58,6 +100,7 @@
$requested_scopes[$scope] = 1;
}
}
+ $requested_scopes[self::getDefaultScope()] = 1;
return $requested_scopes;
}
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
@@ -13,7 +13,7 @@
$server = new PhabricatorOAuthServer();
$client_phid = $request->getStr('client_id');
- $scope = $request->getStr('scope', array());
+ $scope = $request->getStr('scope');
$redirect_uri = $request->getStr('redirect_uri');
$response_type = $request->getStr('response_type');
@@ -130,7 +130,6 @@
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) {
@@ -209,10 +208,7 @@
pht('The requested scope is invalid, unknown, or malformed.'));
}
} else {
- $desired_scopes = array(
- PhabricatorOAuthServerScope::SCOPE_WHOAMI => 1,
- PhabricatorOAuthServerScope::SCOPE_OFFLINE_ACCESS => 1,
- );
+ $desired_scopes = $client->getScope();
}
$form = id(new AphrontFormView())
@@ -223,7 +219,10 @@
->addHiddenInput('scope', $request->getStr('scope'))
->setUser($viewer)
->appendChild(
- PhabricatorOAuthServerScope::getCheckboxControl($desired_scopes));
+ PhabricatorOAuthServerScope::getUserCheckboxControl(
+ $desired_scopes,
+ $client->getScope(),
+ pht('Scope')));
$cancel_msg = pht('The user declined to authorize this application.');
$cancel_uri = $this->addQueryParams(
diff --git a/src/applications/oauthserver/controller/client/PhabricatorOAuthClientEditController.php b/src/applications/oauthserver/controller/client/PhabricatorOAuthClientEditController.php
--- a/src/applications/oauthserver/controller/client/PhabricatorOAuthClientEditController.php
+++ b/src/applications/oauthserver/controller/client/PhabricatorOAuthClientEditController.php
@@ -3,8 +3,7 @@
final class PhabricatorOAuthClientEditController
extends PhabricatorOAuthClientController {
- public function processRequest() {
- $request = $this->getRequest();
+ public function handleRequest(AphrontRequest $request) {
$viewer = $request->getUser();
$phid = $this->getClientPHID();
@@ -66,6 +65,8 @@
$client->setViewPolicy($request->getStr('viewPolicy'));
$client->setEditPolicy($request->getStr('editPolicy'));
+ $client->setScope(
+ PhabricatorOAuthServerScope::getScopesFromRequest($request));
if (!$errors) {
$client->save();
$view_uri = $client->getViewURI();
@@ -93,6 +94,9 @@
->setValue($client->getRedirectURI())
->setError($e_redirect))
->appendChild(
+ PhabricatorOAuthServerScope::getClientCheckboxControl(
+ $client->getScope()))
+ ->appendChild(
id(new AphrontFormPolicyControl())
->setUser($viewer)
->setCapability(PhabricatorPolicyCapability::CAN_VIEW)
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
@@ -3,8 +3,7 @@
final class PhabricatorOAuthClientViewController
extends PhabricatorOAuthClientController {
- public function processRequest() {
- $request = $this->getRequest();
+ public function handleRequest(AphrontRequest $request) {
$viewer = $request->getUser();
$client = id(new PhabricatorOAuthServerClientQuery())
@@ -117,6 +116,12 @@
pht('Redirect URI'),
$client->getRedirectURI());
+ $scopes = $client->getScope();
+ ksort($scopes);
+ $view->addProperty(
+ pht('Request Scope'),
+ implode(', ', array_keys($scopes)));
+
$view->addProperty(
pht('Created'),
phabricator_datetime($client->getDateCreated(), $viewer));
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
@@ -10,6 +10,7 @@
protected $name;
protected $redirectURI;
protected $creatorPHID;
+ protected $scope;
protected $viewPolicy;
protected $editPolicy;
@@ -29,6 +30,7 @@
return id(new PhabricatorOAuthServerClient())
->setCreatorPHID($actor->getPHID())
->setSecret(Filesystem::readRandomCharacters(32))
+ ->setScope(PhabricatorOAuthServerScope::getDefaultScopeDict())
->setViewPolicy(PhabricatorPolicies::POLICY_USER)
->setEditPolicy($actor->getPHID());
}
@@ -36,10 +38,14 @@
protected function getConfiguration() {
return array(
self::CONFIG_AUX_PHID => true,
+ self::CONFIG_SERIALIZATION => array(
+ 'scope' => self::SERIALIZATION_JSON,
+ ),
self::CONFIG_COLUMN_SCHEMA => array(
'name' => 'text255',
'secret' => 'text32',
'redirectURI' => 'text255',
+ 'scope' => 'text',
),
self::CONFIG_KEY_SCHEMA => array(
'key_phid' => null,

File Metadata

Mime Type
text/plain
Expires
Sat, Oct 26, 12:23 PM (3 w, 4 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6729182
Default Alt Text
D11705.id28168.diff (8 KB)

Event Timeline

Phabricator · Built by Phacility