Page MenuHomePhabricator
Files F13959889

D15621.diff
No OneTemporary
Actions

D15621.diff
View Options

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
@@ -177,19 +177,6 @@
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.
-
- $is_expired = $token->isExpired();
- if ($is_expired) {
- $offline_access = PhabricatorOAuthServerScope::SCOPE_OFFLINE_ACCESS;
- $authorization_scope = $authorization->getScope();
- if (empty($authorization_scope[$offline_access])) {
- return null;
- }
- }
-
return $authorization;
}
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
@@ -2,120 +2,20 @@
final class PhabricatorOAuthServerScope extends Phobject {
- const SCOPE_OFFLINE_ACCESS = 'offline_access';
- const SCOPE_WHOAMI = 'whoami';
-
- public static function getScopesDict() {
- return array(
- self::SCOPE_OFFLINE_ACCESS => 1,
- self::SCOPE_WHOAMI => 1,
- );
- }
-
- public static function getDefaultScope() {
- return self::SCOPE_WHOAMI;
- }
-
- public static function getCheckboxControl(
- array $current_scopes) {
-
- $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($current_scopes[$scope])) {
- continue;
- }
-
- $checkboxes->addCheckbox(
- $name = $scope,
- $value = 1,
- $label = self::getCheckboxLabel($scope),
- $checked = isset($current_scopes[$scope]));
- $have_options = true;
- }
-
- if ($have_options) {
- $checkboxes->setLabel(pht('Scope'));
- return $checkboxes;
- }
-
- return null;
+ public static function getScopeMap() {
+ return array();
}
- private static function getCheckboxLabel($scope) {
- $label = null;
- switch ($scope) {
- case self::SCOPE_OFFLINE_ACCESS:
- $label = pht('Make access tokens granted to this client never expire.');
- break;
- case self::SCOPE_WHOAMI:
- $label = pht('Read access to Conduit method %s.', 'user.whoami');
- break;
- }
-
- return $label;
- }
+ public static function filterScope(array $scope) {
+ $valid_scopes = self::getScopeMap();
- public static function getScopesFromRequest(AphrontRequest $request) {
- $scopes = self::getScopesDict();
- $requested_scopes = array();
- foreach ($scopes as $scope => $bit) {
- if ($request->getBool($scope)) {
- $requested_scopes[$scope] = 1;
+ foreach ($scope as $key => $scope_item) {
+ if (!isset($valid_scopes[$scope_item])) {
+ unset($scope[$key]);
}
}
- $requested_scopes[self::getDefaultScope()] = 1;
- return $requested_scopes;
- }
-
- /**
- * A scopes list is considered valid if each scope is a known scope
- * and each scope is seen only once. Otherwise, the list is invalid.
- */
- public static function validateScopesList($scope_list) {
- $scopes = explode(' ', $scope_list);
- $known_scopes = self::getScopesDict();
- $seen_scopes = array();
- foreach ($scopes as $scope) {
- if (!isset($known_scopes[$scope])) {
- return false;
- }
- if (isset($seen_scopes[$scope])) {
- return false;
- }
- $seen_scopes[$scope] = 1;
- }
-
- return true;
- }
-
- /**
- * A scopes dictionary is considered valid if each key is a known scope.
- * Otherwise, the dictionary is invalid.
- */
- public static function validateScopesDict($scope_dict) {
- $known_scopes = self::getScopesDict();
- $unknown_scopes = array_diff_key($scope_dict,
- $known_scopes);
- return empty($unknown_scopes);
- }
- /**
- * Transforms a space-delimited scopes list into a scopes dict. The list
- * should be validated by @{method:validateScopesList} before
- * transformation.
- */
- public static function scopesListToDict($scope_list) {
- $scopes = explode(' ', $scope_list);
- return array_fill_keys($scopes, 1);
+ return $scope;
}
}
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
@@ -14,7 +14,6 @@
$server = new PhabricatorOAuthServer();
$client_phid = $request->getStr('client_id');
- $scope = $request->getStr('scope');
$redirect_uri = $request->getStr('redirect_uri');
$response_type = $request->getStr('response_type');
@@ -114,24 +113,11 @@
implode(', ', array('code'))));
}
- if ($scope) {
- if (!PhabricatorOAuthServerScope::validateScopesList($scope)) {
- 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);
- } else {
- return $this->buildErrorResponse(
- 'invalid_request',
- pht('Malformed Request'),
- pht(
- 'Required parameter %s was not present in the request.',
- phutil_tag('strong', array(), 'scope')));
- }
+
+ $requested_scope = $request->getStrList('scope');
+ $requested_scope = array_fuse($requested_scope);
+
+ $scope = PhabricatorOAuthServerScope::filterScope($requested_scope);
// NOTE: We're always requiring a confirmation dialog to redirect.
// Partly this is a general defense against redirect attacks, and
@@ -142,8 +128,6 @@
list($is_authorized, $authorization) = $auth_info;
if ($request->isFormPost()) {
- $scope = PhabricatorOAuthServerScope::getScopesFromRequest($request);
-
if ($authorization) {
$authorization->setScope($scope)->save();
} else {
@@ -212,16 +196,9 @@
// Here, we're confirming authorization for the application.
if ($authorization) {
- $desired_scopes = array_merge($scope, $authorization->getScope());
+ $missing_scope = array_diff_key($scope, $authorization->getScope());
} else {
- $desired_scopes = $scope;
- }
-
- if (!PhabricatorOAuthServerScope::validateScopesDict($desired_scopes)) {
- return $this->buildErrorResponse(
- 'invalid_scope',
- pht('Invalid Scope'),
- pht('The requested scope is invalid, unknown, or malformed.'));
+ $missing_scope = $scope;
}
$form = id(new AphrontFormView())
@@ -230,9 +207,7 @@
->addHiddenInput('response_type', $response_type)
->addHiddenInput('state', $state)
->addHiddenInput('scope', $request->getStr('scope'))
- ->setUser($viewer)
- ->appendChild(
- PhabricatorOAuthServerScope::getCheckboxControl($desired_scopes));
+ ->setUser($viewer);
$cancel_msg = pht('The user declined to authorize this application.');
$cancel_uri = $this->addQueryParams(
@@ -242,7 +217,7 @@
'error_description' => $cancel_msg,
));
- return $this->newDialog()
+ $dialog = $this->newDialog()
->setShortTitle(pht('Authorize Access'))
->setTitle(pht('Authorize "%s"?', $name))
->setSubmitURI($request->getRequestURI()->getPath())
@@ -253,9 +228,41 @@
'access your Phabricator account data, including your primary '.
'email address?',
phutil_tag('strong', array(), $name)))
- ->appendChild($form->buildLayoutView())
+ ->appendForm($form)
->addSubmitButton(pht('Authorize Access'))
->addCancelButton((string)$cancel_uri, pht('Do Not Authorize'));
+
+ if ($missing_scope) {
+ $dialog->appendParagraph(
+ pht(
+ 'This application has requested these additional permissions. '.
+ 'Authorizing it will grant it the permissions it requests:'));
+ foreach ($missing_scope as $scope_key => $ignored) {
+ // TODO: Once we introduce more scopes, explain them here.
+ }
+ }
+
+ $unknown_scope = array_diff_key($requested_scope, $scope);
+ if ($unknown_scope) {
+ $dialog->appendParagraph(
+ pht(
+ 'This application also requested additional unrecognized '.
+ 'permissions. These permissions may have existed in an older '.
+ 'version of Phabricator, or may be from a future version of '.
+ 'Phabricator. They will not be granted.'));
+
+ $unknown_form = id(new AphrontFormView())
+ ->setViewer($viewer)
+ ->appendChild(
+ id(new AphrontFormTextControl())
+ ->setLabel(pht('Unknown Scope'))
+ ->setValue(implode(', ', array_keys($unknown_scope)))
+ ->setDisabled(true));
+
+ $dialog->appendForm($unknown_form);
+ }
+
+ return $dialog;
}
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
@@ -154,8 +154,7 @@
unset($unguarded);
$result = array(
'access_token' => $access_token->getToken(),
- 'token_type' => 'Bearer',
- 'expires_in' => $access_token->getExpiresDuration(),
+ 'token_type' => 'Bearer',
);
return $response->setContent($result);
} catch (Exception $e) {
diff --git a/src/applications/oauthserver/storage/PhabricatorOAuthServerAccessToken.php b/src/applications/oauthserver/storage/PhabricatorOAuthServerAccessToken.php
--- a/src/applications/oauthserver/storage/PhabricatorOAuthServerAccessToken.php
+++ b/src/applications/oauthserver/storage/PhabricatorOAuthServerAccessToken.php
@@ -22,18 +22,4 @@
) + parent::getConfiguration();
}
- public function isExpired() {
- $now = PhabricatorTime::getNow();
- $expires_epoch = $this->getExpiresEpoch();
- return ($now > $expires_epoch);
- }
-
- public function getExpiresEpoch() {
- return $this->getDateCreated() + 3600;
- }
-
- public function getExpiresDuration() {
- return PhabricatorTime::getNow() - $this->getExpiresEpoch();
- }
-
}
diff --git a/src/applications/people/conduit/UserWhoAmIConduitAPIMethod.php b/src/applications/people/conduit/UserWhoAmIConduitAPIMethod.php
--- a/src/applications/people/conduit/UserWhoAmIConduitAPIMethod.php
+++ b/src/applications/people/conduit/UserWhoAmIConduitAPIMethod.php
@@ -19,7 +19,7 @@
}
public function getRequiredScope() {
- return PhabricatorOAuthServerScope::SCOPE_WHOAMI;
+ return self::SCOPE_ALWAYS;
}
protected function execute(ConduitAPIRequest $request) {
diff --git a/src/docs/contributor/using_oauthserver.diviner b/src/docs/contributor/using_oauthserver.diviner
--- a/src/docs/contributor/using_oauthserver.diviner
+++ b/src/docs/contributor/using_oauthserver.diviner
@@ -110,11 +110,7 @@
NOTE: See "Scopes" section below for more information on what data is
currently exposed through the OAuth Server.
-= Scopes =
+Scopes
+======
-There are only two scopes supported at this time.
-
-- **offline_access** - allows an access token to work indefinitely without
- expiring.
-- **whoami** - allows the client to access the results of Conduit.whoami on
- behalf of the resource owner.
+//This section has not been written yet.//

File Metadata

Mime Type
text/plain
Expires
Oct 15 2024, 9:12 PM (5 w, 3 h ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6714446
Default Alt Text
D15621.diff (12 KB)

Event Timeline

Phabricator · Built by Phacility