Skip to content

Commit

Permalink
fixes bugs and tests for guzzle6 changes
Browse files Browse the repository at this point in the history
  • Loading branch information
bshaffer committed Dec 16, 2015
1 parent 22c51e6 commit da09b25
Show file tree
Hide file tree
Showing 22 changed files with 109 additions and 87 deletions.
20 changes: 16 additions & 4 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,27 @@ language: php
sudo: false

php:
- 5.4
- 5.5
- 5.6
- 7.0
- hhvm

env:
- FIREBASE_JWT_VERSION=2.0.0 GUZZLE_VERSION=5.3
- FIREBASE_JWT_VERSION=2.0.0 GUZZLE_VERSION=~6.0
- FIREBASE_JWT_VERSION=3.0.0 GUZZLE_VERSION=5.3
- FIREBASE_JWT_VERSION=3.0.0 GUZZLE_VERSION=~6.0
global:
- FIREBASE_JWT_VERSION=3.0.0
matrix:
- GUZZLE_VERSION=5.3
- GUZZLE_VERSION=~6.0

matrix:
exclude:
- php: 5.4
env: GUZZLE_VERSION=~6.0
# run one test with minimum firebase version
include:
- php: 5.5
env: FIREBASE_JWT_VERSION=2.0.0 GUZZLE_VERSION=~5.3

before_script:
- composer install
Expand Down
4 changes: 2 additions & 2 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@
"license": "Apache-2.0",
"require": {
"firebase/php-jwt": "~2.0|~3.0",
"guzzlehttp/guzzle": "5.3|~6.0",
"php": ">=5.5",
"guzzlehttp/guzzle": "~5.2|~6.0",
"php": ">=5.4",
"guzzlehttp/psr7": "1.2.*",
"psr/http-message": "1.0.*"
},
Expand Down
10 changes: 8 additions & 2 deletions src/Credentials/GCECredentials.php
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
/**
* GCECredentials supports authorization on Google Compute Engine.
*
* It can be used to authorize requests using the AuthTokenFetcher, but will
* It can be used to authorize requests using the AuthTokenMiddleware, but will
* only succeed if being run on GCE:
*
* use Google\Auth\Credentials\GCECredentials;
Expand Down Expand Up @@ -152,7 +152,13 @@ public function fetchAuthToken(callable $httpHandler = null)
)
);
$body = (string) $resp->getBody();
return json_decode($body, true);

// Assume it's JSON; if it's not throw an exception
if (null === $json = json_decode($body, true)) {
throw new \Exception('Invalid JSON response');
}

return $json;
}

/**
Expand Down
2 changes: 1 addition & 1 deletion src/Credentials/ServiceAccountCredentials.php
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
* console, which should contain a private_key and client_email fields that it
* uses.
*
* Use it with AuthTokenFetcher to authorize http requests:
* Use it with AuthTokenMiddleware to authorize http requests:
*
* use Google\Auth\Credentials\ServiceAccountCredentials;
* use Google\Auth\Middleware\AuthTokenMiddleware;
Expand Down
2 changes: 1 addition & 1 deletion src/HttpHandler/Guzzle5HttpHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ public function __invoke(RequestInterface $request, array $options = [])

return new Response(
$response->getStatusCode(),
$response->getHeaders(),
$response->getHeaders() ?: [],
$response->getBody(),
$response->getProtocolVersion(),
$response->getReasonPhrase()
Expand Down
4 changes: 2 additions & 2 deletions src/HttpHandler/HttpHandlerFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,10 @@ class HttpHandlerFactory
* @return Guzzle5HttpHandler|Guzzle6HttpHandler
* @throws \Exception
*/
public static function build()
public static function build(ClientInterface $client = null)
{
$version = ClientInterface::VERSION;
$client = new Client();
$client = $client ?: new Client();

switch ($version[0]) {
case '5':
Expand Down
4 changes: 3 additions & 1 deletion src/Middleware/SimpleMiddleware.php
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,9 @@ public function __invoke(callable $handler)
return $handler($request, $options);
}

$uri = $request->getUri()->withQuery(Psr7\build_query($this->config));
$query = Psr7\parse_query($request->getUri()->getQuery());
$params = array_merge($query, $this->config);
$uri = $request->getUri()->withQuery(Psr7\build_query($params));
$request = $request->withUri($uri);
return $handler($request, $options);
};
Expand Down
33 changes: 20 additions & 13 deletions src/OAuth2.php
Original file line number Diff line number Diff line change
Expand Up @@ -415,7 +415,7 @@ public function generateCredentialsRequest()
}
unset($params['grant_type']);
if (!is_null($grantType)) {
$params['grant_type'] = strval($grantType);
$params['grant_type'] = $grantType;
}
$params = array_merge($params, $this->getExtensionParams());
}
Expand Down Expand Up @@ -484,9 +484,12 @@ public function parseTokenResponse(ResponseInterface $resp)
parse_str($body, $res);
return $res;
} else {
// Assume it's JSON; if it's not there needs to be an exception, so
// we use the json decode exception instead of adding a new one.
return json_decode($body, true);
// Assume it's JSON; if it's not throw an exception
if (null === $res = json_decode($body, true)) {
throw new \Exception('Invalid JSON response');
}

return $res;
}
}

Expand Down Expand Up @@ -568,8 +571,6 @@ public function buildFullAuthorizationUri(array $config = [])
'redirect_uri' => $this->redirectUri,
'state' => $this->state,
'scope' => $this->getScope(),
'prompt' => null,
'approval_prompt' => null
], $config);

// Validate the auth_params
Expand All @@ -580,7 +581,7 @@ public function buildFullAuthorizationUri(array $config = [])
if (is_null($params['redirect_uri'])) {
throw new \InvalidArgumentException('missing the required redirect URI');
}
if ($params['prompt'] && $params['approval_prompt']) {
if (!empty($params['prompt']) && !empty($params['approval_prompt'])) {
throw new \InvalidArgumentException(
'prompt and approval_prompt are mutually exclusive');
}
Expand Down Expand Up @@ -653,12 +654,11 @@ public function setRedirectUri($uri)
$this->redirectUri = null;
return;
}
$u = $this->coerceUri($uri);
if (!$this->isAbsoluteUri($u)) {
if (!$this->isAbsoluteUri($uri)) {
throw new \InvalidArgumentException(
'Redirect URI must be absolute');
}
$this->redirectUri = $u;
$this->redirectUri = (string) $uri;
}

/**
Expand Down Expand Up @@ -729,7 +729,12 @@ public function setGrantType($gt)
if (in_array($gt, self::$knownGrantTypes)) {
$this->grantType = $gt;
} else {
$this->grantType = Psr7\uri_for($gt);
// validate URI
if (!$this->isAbsoluteUri($gt)) {
throw new \InvalidArgumentException(
'invalid grant type');
}
$this->grantType = (string) $gt;
}
}

Expand Down Expand Up @@ -1123,11 +1128,13 @@ private function jwtEncode($assertion, $signingKey, $signingAlgorithm)
* Determines if the URI is absolute based on its scheme and host or path
* (RFC 3986)
*
* @param UriInterface $u
* @param string $uri
* @return bool
*/
private function isAbsoluteUri(UriInterface $u)
private function isAbsoluteUri($uri)
{
$u = $this->coerceUri($uri);

return $u->getScheme() && ($u->getHost() || $u->getPath());
}

Expand Down
9 changes: 2 additions & 7 deletions tests/ApplicationDefaultCredentialsTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,7 @@
use Google\Auth\ApplicationDefaultCredentials;
use Google\Auth\Credentials\GCECredentials;
use Google\Auth\Credentials\ServiceAccountCredentials;
use Google\Auth\HttpHandler\Guzzle6HttpHandler;
use GuzzleHttp\Client;
use GuzzleHttp\Psr7;
use GuzzleHttp\Psr7\Response;

class ADCGetTest extends \PHPUnit_Framework_TestCase
{
Expand Down Expand Up @@ -180,15 +177,13 @@ public function testSuccedsIfNoDefaultFilesButIsOnGCE()
}

// @todo consider a way to DRY this and above class up
class ADCGetSubscriberTest extends \PHPUnit_Framework_TestCase
class ADCGetSubscriberTest extends BaseTest
{
private $originalHome;

protected function setUp()
{
if (!interface_exists('GuzzleHttp\Event\SubscriberInterface')) {
$this->markTestSkipped();
}
$this->onlyGuzzle5();

$this->originalHome = getenv('HOME');
}
Expand Down
24 changes: 24 additions & 0 deletions tests/BaseTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
<?php

namespace Google\Auth\Tests;

use GuzzleHttp\ClientInterface;

abstract class BaseTest extends \PHPUnit_Framework_TestCase
{
public function onlyGuzzle6()
{
$version = ClientInterface::VERSION;
if ('6' !== $version[0]) {
$this->markTestSkipped('Guzzle 6 only');
}
}

public function onlyGuzzle5()
{
$version = ClientInterface::VERSION;
if ('5' !== $version[0]) {
$this->markTestSkipped('Guzzle 5 only');
}
}
}
9 changes: 4 additions & 5 deletions tests/Credentials/GCECredentialsTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -79,19 +79,18 @@ public function testShouldBeEmptyIfNotOnGCE()
}

/**
* @ExpectedException \GuzzleHttp\Exception\ParseException
* @todo psr7 responses are not throwing a parseexception. do we need this?
* @expectedException Exception
* @expectedExceptionMessage Invalid JSON response
*/
public function testShouldFailIfResponseIsNotJson()
{
$this->markTestSkipped();
$notJson = '{"foo": , this is cannot be passed as json" "bar"}';
$httpHandler = getHandler([
buildResponse(200, [GCECredentials::FLAVOR_HEADER => 'Google']),
buildResponse(200, [], Psr7\stream_for($notJson)),
buildResponse(200, [], $notJson),
]);
$g = new GCECredentials();
$this->assertEquals(array(), $g->fetchAuthToken($httpHandler));
$g->fetchAuthToken($httpHandler);
}

public function testShouldReturnTokenInfo()
Expand Down
6 changes: 2 additions & 4 deletions tests/HttpHandler/Guzzle5HttpHandlerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,11 @@
use GuzzleHttp\Client;
use GuzzleHttp\Message\Response;

class Guzzle5HttpHandlerTest extends \PHPUnit_Framework_TestCase
class Guzzle5HttpHandlerTest extends BaseTest
{
public function setUp()
{
if (!interface_exists('GuzzleHttp\Event\SubscriberInterface')) {
$this->markTestSkipped();
}
$this->onlyGuzzle5();

$this->mockPsr7Request =
$this
Expand Down
6 changes: 2 additions & 4 deletions tests/HttpHandler/Guzzle6HttpHandlerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,11 @@
use GuzzleHttp\Psr7\Response;


class Guzzle6HttpHandlerTest extends \PHPUnit_Framework_TestCase
class Guzzle6HttpHandlerTest extends BaseTest
{
public function setUp()
{
if (!class_exists('GuzzleHttp\HandlerStack')) {
$this->markTestSkipped();
}
$this->onlyGuzzle6();

$this->mockRequest =
$this
Expand Down
10 changes: 3 additions & 7 deletions tests/HttpHandler/HttpHandlerFactoryTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,23 +19,19 @@

use Google\Auth\HttpHandler\HttpHandlerFactory;

class HttpHandlerFactoryTest extends \PHPUnit_Framework_TestCase
class HttpHandlerFactoryTest extends BaseTest
{
public function testBuildsGuzzle5Handler()
{
if (!interface_exists('GuzzleHttp\Event\SubscriberInterface')) {
$this->markTestSkipped();
}
$this->onlyGuzzle5();

$handler = HttpHandlerFactory::build();
$this->assertInstanceOf('Google\Auth\HttpHandler\Guzzle5HttpHandler', $handler);
}

public function testBuildsGuzzle6Handler()
{
if (!class_exists('GuzzleHttp\HandlerStack')) {
$this->markTestSkipped();
}
$this->onlyGuzzle6();

$handler = HttpHandlerFactory::build();
$this->assertInstanceOf('Google\Auth\HttpHandler\Guzzle6HttpHandler', $handler);
Expand Down
6 changes: 2 additions & 4 deletions tests/Middleware/AuthTokenMiddlewareTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -22,17 +22,15 @@
use GuzzleHttp\Psr7\Request;
use GuzzleHttp\Psr7\Response;

class AuthTokenMiddlewareTest extends \PHPUnit_Framework_TestCase
class AuthTokenMiddlewareTest extends BaseTest
{
private $mockFetcher;
private $mockCache;
private $mockRequest;

protected function setUp()
{
if (!class_exists('GuzzleHttp\HandlerStack')) {
$this->markTestSkipped();
}
$this->onlyGuzzle6();

$this->mockFetcher =
$this
Expand Down
6 changes: 2 additions & 4 deletions tests/Middleware/ScopedAccessTokenMiddlewareTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
use GuzzleHttp\Psr7\Request;
use GuzzleHttp\Psr7\Response;

class ScopedAccessTokenMiddlewareTest extends \PHPUnit_Framework_TestCase
class ScopedAccessTokenMiddlewareTest extends BaseTest
{
const TEST_SCOPE = 'https://www.googleapis.com/auth/cloud-taskqueue';

Expand All @@ -31,9 +31,7 @@ class ScopedAccessTokenMiddlewareTest extends \PHPUnit_Framework_TestCase

protected function setUp()
{
if (!class_exists('GuzzleHttp\HandlerStack')) {
$this->markTestSkipped();
}
$this->onlyGuzzle6();

$this->mockCache =
$this
Expand Down
6 changes: 2 additions & 4 deletions tests/Middleware/SimpleMiddlewareTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
use GuzzleHttp\Psr7\Response;
use GuzzleHttp\Psr7\Uri;

class SimpleMiddlewareTest extends \PHPUnit_Framework_TestCase
class SimpleMiddlewareTest extends BaseTest
{
private $mockRequest;

Expand All @@ -32,9 +32,7 @@ class SimpleMiddlewareTest extends \PHPUnit_Framework_TestCase
*/
protected function setUp()
{
if (!class_exists('GuzzleHttp\HandlerStack')) {
$this->markTestSkipped();
}
$this->onlyGuzzle6();

$this->mockRequest =
$this
Expand Down
Loading

0 comments on commit da09b25

Please sign in to comment.