From c03a217ab39b274c8b0b75d13f27935baf107b2b Mon Sep 17 00:00:00 2001 From: Mark Story Date: Mon, 16 May 2016 22:54:02 -0400 Subject: [PATCH 1/4] Give Http\Client\Request a more useful constructor. Having a more useful constructor helps keep the request simpler inside. --- src/Http/Client.php | 5 +---- src/Http/Client/Request.php | 12 +++++++++--- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/src/Http/Client.php b/src/Http/Client.php index f5be7612857..7c6f75d8db6 100644 --- a/src/Http/Client.php +++ b/src/Http/Client.php @@ -421,10 +421,7 @@ public function buildUrl($url, $query = [], $options = []) */ protected function _createRequest($method, $url, $data, $options) { - $request = new Request(); - $request = $request->withMethod($method) - ->url($url) - ->body($data); + $request = new Request($url, $method, $data); if (isset($options['type'])) { $request->header($this->_typeHeaders($options['type'])); diff --git a/src/Http/Client/Request.php b/src/Http/Client/Request.php index 591f39f1b25..0480c876d68 100644 --- a/src/Http/Client/Request.php +++ b/src/Http/Client/Request.php @@ -34,11 +34,17 @@ class Request extends Message implements RequestInterface * Constructor * * Provides backwards compatible defaults for some properties. + * + * @param string $url The request URL + * @param string $method The HTTP method to use. + * @param array|string $body The request body to use. */ - public function __construct() + public function __construct($url = '', $method = self::METHOD_GET, $data = null) { - $this->method = static::METHOD_GET; - + $this->validateMethod($method); + $this->method = $method; + $this->uri = $this->createUri($url); + $this->body($data); $this->header([ 'Connection' => 'close', 'User-Agent' => 'CakePHP' From 38d3cc5d7096d6ec76153d219366f2b40cdeb17f Mon Sep 17 00:00:00 2001 From: Mark Story Date: Mon, 16 May 2016 23:09:54 -0400 Subject: [PATCH 2/4] Start using PSR7 interfaces more internally. --- src/Http/Client.php | 14 ++++++-------- src/Http/Client/Auth/Digest.php | 6 +++--- src/Http/Client/Auth/Oauth.php | 24 ++++++++++-------------- src/Http/Client/Request.php | 23 ++++++++++++++++++----- 4 files changed, 37 insertions(+), 30 deletions(-) diff --git a/src/Http/Client.php b/src/Http/Client.php index 7c6f75d8db6..eb5da7be05b 100644 --- a/src/Http/Client.php +++ b/src/Http/Client.php @@ -421,17 +421,15 @@ public function buildUrl($url, $query = [], $options = []) */ protected function _createRequest($method, $url, $data, $options) { - $request = new Request($url, $method, $data); - + $headers = isset($options['headers']) ? (array)$options['headers'] : []; if (isset($options['type'])) { - $request->header($this->_typeHeaders($options['type'])); - } - if (isset($options['headers'])) { - $request->header($options['headers']); + $headers = array_merge($headers, $this->_typeHeaders($options['type'])); } - if (is_string($data) && !$request->header('content-type')) { - $request->header('Content-Type', 'application/x-www-form-urlencoded'); + if (is_string($data) && !isset($headers['Content-Type']) && !isset($headers['content-type'])) { + $headers['Content-Type'] = 'application/x-www-form-urlencoded'; } + + $request = new Request($url, $method, $headers, $data); $request->cookie($this->_cookies->get($url)); if (isset($options['cookies'])) { $request->cookie($options['cookies']); diff --git a/src/Http/Client/Auth/Digest.php b/src/Http/Client/Auth/Digest.php index b17c83a7e08..fc5c14fe90f 100644 --- a/src/Http/Client/Auth/Digest.php +++ b/src/Http/Client/Auth/Digest.php @@ -85,12 +85,12 @@ protected function _getServerInfo(Request $request, $credentials) ['auth' => []] ); - if (!$response->header('WWW-Authenticate')) { + if (!$response->getHeader('WWW-Authenticate')) { return []; } preg_match_all( '@(\w+)=(?:(?:")([^"]+)"|([^\s,$]+))@', - $response->header('WWW-Authenticate'), + $response->getHeaderLine('WWW-Authenticate'), $matches, PREG_SET_ORDER ); @@ -112,7 +112,7 @@ protected function _getServerInfo(Request $request, $credentials) */ protected function _generateHeader(Request $request, $credentials) { - $path = parse_url($request->url(), PHP_URL_PATH); + $path = $request->getUri()->getPath(); $a1 = md5($credentials['username'] . ':' . $credentials['realm'] . ':' . $credentials['password']); $a2 = md5($request->method() . ':' . $path); diff --git a/src/Http/Client/Auth/Oauth.php b/src/Http/Client/Auth/Oauth.php index 9ab3345240a..df4296bd1e4 100644 --- a/src/Http/Client/Auth/Oauth.php +++ b/src/Http/Client/Auth/Oauth.php @@ -150,8 +150,8 @@ protected function _hmacSha1($request, $credentials) public function baseString($request, $oauthValues) { $parts = [ - $request->method(), - $this->_normalizedUrl($request->url()), + $request->getMethod(), + $this->_normalizedUrl($request->getUri()), $this->_normalizedParams($request, $oauthValues), ]; $parts = array_map([$this, '_encode'], $parts); @@ -163,27 +163,23 @@ public function baseString($request, $oauthValues) * * Section 9.1.2. of the Oauth spec * - * @param string $url URL + * @param Psr\Http\Message\UriInterface $url URL * @return string Normalized URL - * @throws \Cake\Core\Exception\Exception On invalid URLs */ - protected function _normalizedUrl($url) + protected function _normalizedUrl($uri) { - $parts = parse_url($url); - if (!$parts) { - throw new Exception('Unable to parse URL'); - } - $scheme = strtolower($parts['scheme'] ?: 'http'); + $scheme = $uri->getScheme(); $defaultPorts = [ 'http' => 80, 'https' => 443 ]; - if (isset($parts['port']) && $parts['port'] != $defaultPorts[$scheme]) { - $parts['host'] .= ':' . $parts['port']; + $port = $uri->getPort(); + if ($port && $port != $defaultPorts[$scheme]) { + $parts['host'] .= ':' . $port; } $out = $scheme . '://'; - $out .= strtolower($parts['host']); - $out .= $parts['path']; + $out .= strtolower($uri->getHost()); + $out .= $uri->getPath(); return $out; } diff --git a/src/Http/Client/Request.php b/src/Http/Client/Request.php index 0480c876d68..8b5b9625fa5 100644 --- a/src/Http/Client/Request.php +++ b/src/Http/Client/Request.php @@ -37,18 +37,20 @@ class Request extends Message implements RequestInterface * * @param string $url The request URL * @param string $method The HTTP method to use. + * @param array $headers The HTTP headers to set. * @param array|string $body The request body to use. */ - public function __construct($url = '', $method = self::METHOD_GET, $data = null) + public function __construct($url = '', $method = self::METHOD_GET, array $headers = [], $data = null) { $this->validateMethod($method); $this->method = $method; $this->uri = $this->createUri($url); $this->body($data); - $this->header([ + $headers += [ 'Connection' => 'close', 'User-Agent' => 'CakePHP' - ]); + ]; + $this->addHeaders($headers); } /** @@ -139,12 +141,23 @@ public function header($name = null, $value = null) if ($value !== null && !is_array($name)) { $name = [$name => $value]; } - foreach ($name as $key => $val) { + $this->addHeaders($name); + return $this; + } + + /** + * Add an array of headers to the request. + * + * @param array $headers The headers to add. + * @return void + */ + protected function addHeaders($headers) + { + foreach ($headers as $key => $val) { $normalized = strtolower($key); $this->headers[$key] = (array)$val; $this->headerNames[$normalized] = $key; } - return $this; } /** From 01f016a8497dddc671b7866a35e6e5d06130328a Mon Sep 17 00:00:00 2001 From: Mark Story Date: Tue, 17 May 2016 00:24:50 -0400 Subject: [PATCH 3/4] Use PSR7 interfaces internally in auth plugins. Use PSR7 methods in the auth plugins. I've changed the return value for authentication adapters such that they can return a modified request. The test case using the CompatAuth stub ensure that the current behavior continues to work as well for backwards compatibility reasons. --- src/Http/Client.php | 14 ++--- src/Http/Client/Auth/Basic.php | 10 ++-- src/Http/Client/Auth/Digest.php | 8 +-- src/Http/Client/Auth/Oauth.php | 6 +-- .../TestCase/Network/Http/Auth/DigestTest.php | 24 ++++----- .../TestCase/Network/Http/Auth/OauthTest.php | 8 +-- tests/TestCase/Network/Http/ClientTest.php | 37 ++++++++++++++ tests/test_app/TestApp/Http/CompatAuth.php | 51 +++++++++++++++++++ 8 files changed, 122 insertions(+), 36 deletions(-) create mode 100644 tests/test_app/TestApp/Http/CompatAuth.php diff --git a/src/Http/Client.php b/src/Http/Client.php index eb5da7be05b..7f3c4cd2cb4 100644 --- a/src/Http/Client.php +++ b/src/Http/Client.php @@ -435,10 +435,10 @@ protected function _createRequest($method, $url, $data, $options) $request->cookie($options['cookies']); } if (isset($options['auth'])) { - $this->_addAuthentication($request, $options); + $request = $this->_addAuthentication($request, $options); } if (isset($options['proxy'])) { - $this->_addProxy($request, $options); + $request = $this->_addProxy($request, $options); } return $request; } @@ -480,13 +480,14 @@ protected function _typeHeaders($type) * * @param \Cake\Http\Client\Request $request The request to modify. * @param array $options Array of options containing the 'auth' key. - * @return void + * @return \Cake\Http\Client\Request The updated request object. */ protected function _addAuthentication(Request $request, $options) { $auth = $options['auth']; $adapter = $this->_createAuth($auth, $options); - $adapter->authentication($request, $options['auth']); + $result = $adapter->authentication($request, $options['auth']); + return $result ?: $request; } /** @@ -497,13 +498,14 @@ protected function _addAuthentication(Request $request, $options) * * @param \Cake\Http\Client\Request $request The request to modify. * @param array $options Array of options containing the 'proxy' key. - * @return void + * @return \Cake\Http\Client\Request The updated request object. */ protected function _addProxy(Request $request, $options) { $auth = $options['proxy']; $adapter = $this->_createAuth($auth, $options); - $adapter->proxyAuthentication($request, $options['proxy']); + $result = $adapter->proxyAuthentication($request, $options['proxy']); + return $result ?: $request; } /** diff --git a/src/Http/Client/Auth/Basic.php b/src/Http/Client/Auth/Basic.php index e7dd5285ed9..1021b4f75c6 100644 --- a/src/Http/Client/Auth/Basic.php +++ b/src/Http/Client/Auth/Basic.php @@ -29,15 +29,16 @@ class Basic * * @param \Cake\Network\Http\Request $request Request instance. * @param array $credentials Credentials. - * @return void + * @return \Cake\Network\Http\Request The updated request. * @see http://www.ietf.org/rfc/rfc2617.txt */ public function authentication(Request $request, array $credentials) { if (isset($credentials['username'], $credentials['password'])) { $value = $this->_generateHeader($credentials['username'], $credentials['password']); - $request->header('Authorization', $value); + $request = $request->withHeader('Authorization', $value); } + return $request; } /** @@ -45,15 +46,16 @@ public function authentication(Request $request, array $credentials) * * @param \Cake\Network\Http\Request $request Request instance. * @param array $credentials Credentials. - * @return void + * @return \Cake\Network\Http\Request The updated request. * @see http://www.ietf.org/rfc/rfc2617.txt */ public function proxyAuthentication(Request $request, array $credentials) { if (isset($credentials['username'], $credentials['password'])) { $value = $this->_generateHeader($credentials['username'], $credentials['password']); - $request->header('Proxy-Authorization', $value); + $request = $request->withHeader('Proxy-Authorization', $value); } + return $request; } /** diff --git a/src/Http/Client/Auth/Digest.php b/src/Http/Client/Auth/Digest.php index fc5c14fe90f..4cc3381498f 100644 --- a/src/Http/Client/Auth/Digest.php +++ b/src/Http/Client/Auth/Digest.php @@ -48,22 +48,22 @@ public function __construct(Client $client, $options = null) * * @param \Cake\Network\Http\Request $request The request object. * @param array $credentials Authentication credentials. - * @return void + * @return \Cake\Network\Http\Request The updated request. * @see http://www.ietf.org/rfc/rfc2617.txt */ public function authentication(Request $request, array $credentials) { if (!isset($credentials['username'], $credentials['password'])) { - return; + return $request; } if (!isset($credentials['realm'])) { $credentials = $this->_getServerInfo($request, $credentials); } if (!isset($credentials['realm'])) { - return; + return $request; } $value = $this->_generateHeader($request, $credentials); - $request->header('Authorization', $value); + return $request->withHeader('Authorization', $value); } /** diff --git a/src/Http/Client/Auth/Oauth.php b/src/Http/Client/Auth/Oauth.php index df4296bd1e4..7d1bbe86343 100644 --- a/src/Http/Client/Auth/Oauth.php +++ b/src/Http/Client/Auth/Oauth.php @@ -34,7 +34,7 @@ class Oauth * * @param \Cake\Network\Http\Request $request The request object. * @param array $credentials Authentication credentials. - * @return void + * @return \Cake\Network\Http\Request The updated request. * @throws \Cake\Core\Exception\Exception On invalid signature types. */ public function authentication(Request $request, array $credentials) @@ -46,7 +46,7 @@ public function authentication(Request $request, array $credentials) $credentials['tokenSecret'] ); if (!$hasKeys) { - return; + return $request; } if (empty($credentials['method'])) { $credentials['method'] = 'hmac-sha1'; @@ -64,7 +64,7 @@ public function authentication(Request $request, array $credentials) default: throw new Exception(sprintf('Unknown Oauth signature method %s', $credentials['method'])); } - $request->header('Authorization', $value); + return $request->withHeader('Authorization', $value); } /** diff --git a/tests/TestCase/Network/Http/Auth/DigestTest.php b/tests/TestCase/Network/Http/Auth/DigestTest.php index 9363ffd37a9..7199795fd1a 100644 --- a/tests/TestCase/Network/Http/Auth/DigestTest.php +++ b/tests/TestCase/Network/Http/Auth/DigestTest.php @@ -57,12 +57,10 @@ public function testRealmAndNonceFromExtraRequest() ->will($this->returnValue($response)); $auth = ['username' => 'admin', 'password' => '1234']; - $request = (new Request())->method(Request::METHOD_GET) - ->url('http://example.com/some/path'); + $request = new Request('http://example.com/some/path', Request::METHOD_GET); + $request = $this->auth->authentication($request, $auth); - $this->auth->authentication($request, $auth); - - $result = $request->header('Authorization'); + $result = $request->getHeaderLine('Authorization'); $this->assertContains('Digest', $result); $this->assertContains('realm="The batcave"', $result); $this->assertContains('nonce="4cded326c6c51"', $result); @@ -89,11 +87,9 @@ public function testQop() ->will($this->returnValue($response)); $auth = ['username' => 'admin', 'password' => '1234']; - $request = (new Request())->method(Request::METHOD_GET) - ->url('http://example.com/some/path'); - - $this->auth->authentication($request, $auth); - $result = $request->header('Authorization'); + $request = new Request('http://example.com/some/path', Request::METHOD_GET); + $request = $this->auth->authentication($request, $auth); + $result = $request->getHeaderLine('Authorization'); $this->assertContains('qop="auth"', $result); $this->assertContains('nc=00000001', $result); @@ -117,11 +113,9 @@ public function testOpaque() ->will($this->returnValue($response)); $auth = ['username' => 'admin', 'password' => '1234']; - $request = (new Request())->method(Request::METHOD_GET) - ->url('http://example.com/some/path'); - - $this->auth->authentication($request, $auth); - $result = $request->header('Authorization'); + $request = new Request('http://example.com/some/path', Request::METHOD_GET); + $request = $this->auth->authentication($request, $auth); + $result = $request->getHeaderLine('Authorization'); $this->assertContains('opaque="d8ea7aa61a1693024c4cc3a516f49b3c"', $result); } diff --git a/tests/TestCase/Network/Http/Auth/OauthTest.php b/tests/TestCase/Network/Http/Auth/OauthTest.php index 0b44e80ca2f..d6cb8fc6c14 100644 --- a/tests/TestCase/Network/Http/Auth/OauthTest.php +++ b/tests/TestCase/Network/Http/Auth/OauthTest.php @@ -56,9 +56,9 @@ public function testPlainTextSigning() 'method' => 'plaintext', ]; $request = new Request(); - $auth->authentication($request, $creds); + $request = $auth->authentication($request, $creds); - $result = $request->header('Authorization'); + $result = $request->getHeaderLine('Authorization'); $this->assertContains('OAuth', $result); $this->assertContains('oauth_version="1.0"', $result); $this->assertContains('oauth_token="a%20token%20value"', $result); @@ -204,9 +204,9 @@ public function testHmacSigning() 'timestamp' => '1191242096' ]; $auth = new Oauth(); - $auth->authentication($request, $options); + $request = $auth->authentication($request, $options); - $result = $request->header('Authorization'); + $result = $request->getHeaderLine('Authorization'); $expected = 'tR3+Ty81lMeYAr/Fid0kMTYa/WM='; $this->assertContains( 'oauth_signature="' . $expected . '"', diff --git a/tests/TestCase/Network/Http/ClientTest.php b/tests/TestCase/Network/Http/ClientTest.php index 62e25e2b47d..14bcb9c6533 100644 --- a/tests/TestCase/Network/Http/ClientTest.php +++ b/tests/TestCase/Network/Http/ClientTest.php @@ -13,6 +13,7 @@ */ namespace Cake\Test\TestCase\Network\Http; +use Cake\Core\Configure; use Cake\Network\Http\Client; use Cake\Network\Http\Request; use Cake\Network\Http\Response; @@ -344,6 +345,42 @@ public function testGetWithAuthenticationAndProxy() $this->assertSame($result, $response); } + /** + * Test authentication adapter that mutates request. + * + * @return void + */ + public function testAuthenticationWithMutation() + { + Configure::write('App.namespace', 'TestApp'); + $response = new Response(); + $mock = $this->getMock('Cake\Network\Http\Adapter\Stream', ['send']); + $headers = [ + 'Authorization' => 'Bearer abc123', + 'Proxy-Authorization' => 'Bearer abc123', + ]; + $mock->expects($this->once()) + ->method('send') + ->with($this->callback(function ($request) use ($headers) { + $this->assertEquals(Request::METHOD_GET, $request->getMethod()); + $this->assertEquals('http://cakephp.org/', '' . $request->getUri()); + $this->assertEquals($headers['Authorization'], $request->getHeaderLine('Authorization')); + $this->assertEquals($headers['Proxy-Authorization'], $request->getHeaderLine('Proxy-Authorization')); + return true; + })) + ->will($this->returnValue([$response])); + + $http = new Client([ + 'host' => 'cakephp.org', + 'adapter' => $mock + ]); + $result = $http->get('/', [], [ + 'auth' => ['type' => 'TestApp\Http\CompatAuth'], + 'proxy' => ['type' => 'TestApp\Http\CompatAuth'], + ]); + $this->assertSame($result, $response); + } + /** * Return a list of HTTP methods. * diff --git a/tests/test_app/TestApp/Http/CompatAuth.php b/tests/test_app/TestApp/Http/CompatAuth.php new file mode 100644 index 00000000000..a7708cdc3c1 --- /dev/null +++ b/tests/test_app/TestApp/Http/CompatAuth.php @@ -0,0 +1,51 @@ +header('Authorization', 'Bearer abc123'); + } + + /** + * Proxy Authentication added via in-place mutation methods. + * + * @param \Cake\Network\Http\Request $request Request instance. + * @param array $credentials Credentials. + * @return \Cake\Network\Http\Request The updated request. + */ + public function proxyAuthentication(Request $request, array $credentials) + { + $request->header('Proxy-Authorization', 'Bearer abc123'); + } + +} From a21902035bc479f0132a8b7207f1395f825b718c Mon Sep 17 00:00:00 2001 From: Mark Story Date: Wed, 18 May 2016 22:24:28 -0400 Subject: [PATCH 4/4] Fix PHPCS errors. --- src/Http/Client/Auth/Oauth.php | 2 +- src/Http/Client/Request.php | 2 +- tests/test_app/TestApp/Http/CompatAuth.php | 1 - 3 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/Http/Client/Auth/Oauth.php b/src/Http/Client/Auth/Oauth.php index 7d1bbe86343..a61f288cf0c 100644 --- a/src/Http/Client/Auth/Oauth.php +++ b/src/Http/Client/Auth/Oauth.php @@ -163,7 +163,7 @@ public function baseString($request, $oauthValues) * * Section 9.1.2. of the Oauth spec * - * @param Psr\Http\Message\UriInterface $url URL + * @param Psr\Http\Message\UriInterface $uri Uri object to build a normalized version of. * @return string Normalized URL */ protected function _normalizedUrl($uri) diff --git a/src/Http/Client/Request.php b/src/Http/Client/Request.php index 8b5b9625fa5..c67954f2a67 100644 --- a/src/Http/Client/Request.php +++ b/src/Http/Client/Request.php @@ -38,7 +38,7 @@ class Request extends Message implements RequestInterface * @param string $url The request URL * @param string $method The HTTP method to use. * @param array $headers The HTTP headers to set. - * @param array|string $body The request body to use. + * @param array|string $data The request body to use. */ public function __construct($url = '', $method = self::METHOD_GET, array $headers = [], $data = null) { diff --git a/tests/test_app/TestApp/Http/CompatAuth.php b/tests/test_app/TestApp/Http/CompatAuth.php index a7708cdc3c1..052a932d32f 100644 --- a/tests/test_app/TestApp/Http/CompatAuth.php +++ b/tests/test_app/TestApp/Http/CompatAuth.php @@ -47,5 +47,4 @@ public function proxyAuthentication(Request $request, array $credentials) { $request->header('Proxy-Authorization', 'Bearer abc123'); } - }