Page MenuHomePhorge
Files F2896155

No OneTemporary
Actions

View Options

diff --git a/src/applications/oauthserver/PhabricatorOAuthServer.php b/src/applications/oauthserver/PhabricatorOAuthServer.php
index 46f651fede..2180e53967 100644
--- a/src/applications/oauthserver/PhabricatorOAuthServer.php
+++ b/src/applications/oauthserver/PhabricatorOAuthServer.php
@@ -1,239 +1,270 @@
<?php
/**
* Implements core OAuth 2.0 Server logic.
*
* This class should be used behind business logic that parses input to
* determine pertinent @{class:PhabricatorUser} $user,
* @{class:PhabricatorOAuthServerClient} $client(s),
* @{class:PhabricatorOAuthServerAuthorizationCode} $code(s), and.
* @{class:PhabricatorOAuthServerAccessToken} $token(s).
*
* For an OAuth 2.0 server, there are two main steps:
*
* 1) Authorization - the user authorizes a given client to access the data
* the OAuth 2.0 server protects. Once this is achieved / if it has
* been achived already, the OAuth server sends the client an authorization
* code.
* 2) Access Token - the client should send the authorization code received in
* step 1 along with its id and secret to the OAuth server to receive an
* access token. This access token can later be used to access Phabricator
* data on behalf of the user.
*
* @task auth Authorizing @{class:PhabricatorOAuthServerClient}s and
* generating @{class:PhabricatorOAuthServerAuthorizationCode}s
* @task token Validating @{class:PhabricatorOAuthServerAuthorizationCode}s
* and generating @{class:PhabricatorOAuthServerAccessToken}s
* @task internal Internals
*
* @group oauthserver
*/
final class PhabricatorOAuthServer {
const AUTHORIZATION_CODE_TIMEOUT = 300;
const ACCESS_TOKEN_TIMEOUT = 3600;
private $user;
private $client;
/**
* @group internal
*/
private function getUser() {
if (!$this->user) {
throw new Exception('You must setUser before you can getUser!');
}
return $this->user;
}
public function setUser(PhabricatorUser $user) {
$this->user = $user;
return $this;
}
/**
* @group internal
*/
private function getClient() {
if (!$this->client) {
throw new Exception('You must setClient before you can getClient!');
}
return $this->client;
}
public function setClient(PhabricatorOAuthServerClient $client) {
$this->client = $client;
return $this;
}
/**
* @task auth
* @return tuple <bool hasAuthorized, ClientAuthorization or null>
*/
public function userHasAuthorizedClient(array $scope) {
$authorization = id(new PhabricatorOAuthClientAuthorization())->
loadOneWhere('userPHID = %s AND clientPHID = %s',
$this->getUser()->getPHID(),
$this->getClient()->getPHID());
if (empty($authorization)) {
return array(false, null);
}
if ($scope) {
$missing_scope = array_diff_key($scope,
$authorization->getScope());
} else {
$missing_scope = false;
}
if ($missing_scope) {
return array(false, $authorization);
}
return array(true, $authorization);
}
/**
* @task auth
*/
public function authorizeClient(array $scope) {
$authorization = new PhabricatorOAuthClientAuthorization();
$authorization->setUserPHID($this->getUser()->getPHID());
$authorization->setClientPHID($this->getClient()->getPHID());
$authorization->setScope($scope);
$authorization->save();
return $authorization;
}
/**
* @task auth
*/
public function generateAuthorizationCode(PhutilURI $redirect_uri) {
$code = Filesystem::readRandomCharacters(32);
$client = $this->getClient();
$authorization_code = new PhabricatorOAuthServerAuthorizationCode();
$authorization_code->setCode($code);
$authorization_code->setClientPHID($client->getPHID());
$authorization_code->setClientSecret($client->getSecret());
$authorization_code->setUserPHID($this->getUser()->getPHID());
$authorization_code->setRedirectURI((string) $redirect_uri);
$authorization_code->save();
return $authorization_code;
}
/**
* @task token
*/
public function generateAccessToken() {
$token = Filesystem::readRandomCharacters(32);
$access_token = new PhabricatorOAuthServerAccessToken();
$access_token->setToken($token);
$access_token->setUserPHID($this->getUser()->getPHID());
$access_token->setClientPHID($this->getClient()->getPHID());
$access_token->save();
return $access_token;
}
/**
* @task token
*/
public function validateAuthorizationCode(
PhabricatorOAuthServerAuthorizationCode $test_code,
PhabricatorOAuthServerAuthorizationCode $valid_code) {
// check that all the meta data matches
if ($test_code->getClientPHID() != $valid_code->getClientPHID()) {
return false;
}
if ($test_code->getClientSecret() != $valid_code->getClientSecret()) {
return false;
}
// check that the authorization code hasn't timed out
$created_time = $test_code->getDateCreated();
$must_be_used_by = $created_time + self::AUTHORIZATION_CODE_TIMEOUT;
return (time() < $must_be_used_by);
}
/**
* @task token
*/
public function validateAccessToken(
PhabricatorOAuthServerAccessToken $token,
$required_scope) {
$created_time = $token->getDateCreated();
$must_be_used_by = $created_time + self::ACCESS_TOKEN_TIMEOUT;
$expired = time() > $must_be_used_by;
$authorization = id(new PhabricatorOAuthClientAuthorization())
->loadOneWhere(
'userPHID = %s AND clientPHID = %s',
$token->getUserPHID(),
$token->getClientPHID());
if (!$authorization) {
return false;
}
$token_scope = $authorization->getScope();
if (!isset($token_scope[$required_scope])) {
return false;
}
$valid = true;
if ($expired) {
$valid = false;
// check if the scope includes "offline_access", which makes the
// token valid despite being expired
if (isset(
$token_scope[PhabricatorOAuthServerScope::SCOPE_OFFLINE_ACCESS])) {
$valid = true;
}
}
return $valid;
}
/**
* See http://tools.ietf.org/html/draft-ietf-oauth-v2-23#section-3.1.2
* for details on what makes a given redirect URI "valid".
*/
public function validateRedirectURI(PhutilURI $uri) {
- if (PhabricatorEnv::isValidRemoteWebResource($uri)) {
- if ($uri->getFragment()) {
- return false;
- }
- if ($uri->getDomain()) {
- return true;
- }
+ if (!PhabricatorEnv::isValidRemoteWebResource($uri)) {
+ return false;
}
- return false;
+
+ if ($uri->getFragment()) {
+ return false;
+ }
+
+ if (!$uri->getDomain()) {
+ return false;
+ }
+
+ return true;
}
/**
* If there's a URI specified in an OAuth request, it must be validated in
* its own right. Further, it must have the same domain and (at least) the
* same query parameters as the primary URI.
*/
- public function validateSecondaryRedirectURI(PhutilURI $secondary_uri,
- PhutilURI $primary_uri) {
- $valid = $this->validateRedirectURI($secondary_uri);
- if ($valid) {
- $valid_domain = ($secondary_uri->getDomain() ==
- $primary_uri->getDomain());
- $good_params = $primary_uri->getQueryParams();
- $test_params = $secondary_uri->getQueryParams();
- $missing_params = array_diff_key($good_params, $test_params);
- $valid = $valid_domain && empty($missing_params);
+ public function validateSecondaryRedirectURI(
+ PhutilURI $secondary_uri,
+ PhutilURI $primary_uri) {
+
+ // The secondary URI must be valid.
+ if (!$this->validateRedirectURI($secondary_uri)) {
+ return false;
}
- return $valid;
+
+ // Both URIs must point at the same domain.
+ if ($secondary_uri->getDomain() != $primary_uri->getDomain()) {
+ return false;
+ }
+
+ // Any query parameters present in the first URI must be exactly present
+ // in the second URI.
+ $need_params = $primary_uri->getQueryParams();
+ $have_params = $secondary_uri->getQueryParams();
+
+ foreach ($need_params as $key => $value) {
+ if (!array_key_exists($key, $have_params)) {
+ return false;
+ }
+ if ((string)$have_params[$key] != (string)$value) {
+ return false;
+ }
+ }
+
+ // If the first URI is HTTPS, the second URI must also be HTTPS. This
+ // defuses an attack where a third party with control over the network
+ // tricks you into using HTTP to authenticate over a link which is supposed
+ // to be HTTPS only and sniffs all your token cookies.
+ if (strtolower($primary_uri->getProtocol()) == 'https') {
+ if (strtolower($secondary_uri->getProtocol()) != 'https') {
+ return false;
+ }
+ }
+
+ return true;
}
}
diff --git a/src/applications/oauthserver/__tests__/PhabricatorOAuthServerTestCase.php b/src/applications/oauthserver/__tests__/PhabricatorOAuthServerTestCase.php
index 3e8e4a8ca7..070c822737 100644
--- a/src/applications/oauthserver/__tests__/PhabricatorOAuthServerTestCase.php
+++ b/src/applications/oauthserver/__tests__/PhabricatorOAuthServerTestCase.php
@@ -1,66 +1,96 @@
<?php
final class PhabricatorOAuthServerTestCase
extends PhabricatorTestCase {
public function testValidateRedirectURI() {
static $map = array(
'http://www.google.com' => true,
'http://www.google.com/' => true,
'http://www.google.com/auth' => true,
'www.google.com' => false,
'http://www.google.com/auth#invalid' => false
);
$server = new PhabricatorOAuthServer();
foreach ($map as $input => $expected) {
$uri = new PhutilURI($input);
$result = $server->validateRedirectURI($uri);
$this->assertEqual(
$expected,
$result,
"Validation of redirect URI '{$input}'");
}
}
public function testValidateSecondaryRedirectURI() {
$server = new PhabricatorOAuthServer();
$primary_uri = new PhutilURI('http://www.google.com');
static $test_domain_map = array(
'http://www.google.com' => true,
'http://www.google.com/' => true,
'http://www.google.com/auth' => true,
'http://www.google.com/?auth' => true,
'www.google.com' => false,
'http://www.google.com/auth#invalid' => false,
'http://www.example.com' => false
);
foreach ($test_domain_map as $input => $expected) {
$uri = new PhutilURI($input);
$this->assertEqual(
$expected,
$server->validateSecondaryRedirectURI($uri, $primary_uri),
"Validation of redirect URI '{$input}' ".
"relative to '{$primary_uri}'");
}
$primary_uri = new PhutilURI('http://www.google.com/?auth');
static $test_query_map = array(
'http://www.google.com' => false,
'http://www.google.com/' => false,
'http://www.google.com/auth' => false,
'http://www.google.com/?auth' => true,
'http://www.google.com/?auth&stuff' => true,
'http://www.google.com/?stuff' => false,
);
foreach ($test_query_map as $input => $expected) {
$uri = new PhutilURI($input);
$this->assertEqual(
$expected,
$server->validateSecondaryRedirectURI($uri, $primary_uri),
"Validation of secondary redirect URI '{$input}' ".
"relative to '{$primary_uri}'");
}
+ $primary_uri = new PhutilURI('https://secure.example.com/');
+ $tests = array(
+ 'https://secure.example.com/' => true,
+ 'http://secure.example.com/' => false,
+ );
+ foreach ($tests as $input => $expected) {
+ $uri = new PhutilURI($input);
+ $this->assertEqual(
+ $expected,
+ $server->validateSecondaryRedirectURI($uri, $primary_uri),
+ "Validation (https): {$input}");
+ }
+
+ $primary_uri = new PhutilURI('http://example.com/?z=2&y=3');
+ $tests = array(
+ 'http://example.com?z=2&y=3' => true,
+ 'http://example.com?y=3&z=2' => true,
+ 'http://example.com?y=3&z=2&x=1' => true,
+ 'http://example.com?y=2&z=3' => false,
+ 'http://example.com?y&x' => false,
+ 'http://example.com?z=2&x=3' => false,
+ );
+ foreach ($tests as $input => $expected) {
+ $uri = new PhutilURI($input);
+ $this->assertEqual(
+ $expected,
+ $server->validateSecondaryRedirectURI($uri, $primary_uri),
+ "Validation (params): {$input}");
+ }
+
}
}

File Metadata

Mime Type
text/x-diff
Expires
Jan 19 2025, 22:39 (6 w, 3 d ago)
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
1129479
Default Alt Text
(12 KB)

Event Timeline

Content licensed under Creative Commons Attribution-ShareAlike 4.0 (CC-BY-SA) unless otherwise noted; code licensed under Apache 2.0 or other open source licenses. · CC BY-SA 4.0 · Apache 2.0