From 83d91854afa1c0280425e3266354f7a82a5b3dc6 Mon Sep 17 00:00:00 2001 From: Richard Bairwell Date: Wed, 20 Apr 2016 10:16:54 +0000 Subject: [PATCH] Add handling of origin port to reply if set --- CHANGELOG.md | 2 + src/MiddlewareCors/Traits/Parse.php | 63 +++++++---- tests/MiddlewareCors/PreflightTest.php | 56 +++++----- tests/MiddlewareCorsTest.php | 145 +++++++++++++++++++------ 4 files changed, 182 insertions(+), 84 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 44a25a3..07431b7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,5 @@ +v0.3.6 - 20th Apr 2016 + Add handling of origin port to reply if sent. v0.3.5 - 19th Apr 2016 Add handling of origin protocol scheme to reply if sent. v0.3.0 - 13th Apr 2016 diff --git a/src/MiddlewareCors/Traits/Parse.php b/src/MiddlewareCors/Traits/Parse.php index e2477e7..1162ddd 100644 --- a/src/MiddlewareCors/Traits/Parse.php +++ b/src/MiddlewareCors/Traits/Parse.php @@ -170,26 +170,17 @@ protected function parseOrigin(ServerRequestInterface $request) : string $this->addLog('Processing origin of "'.$origin.'"'); // lowercase the user provided origin for comparison purposes. - $origin = strtolower($origin); - $parsed = parse_url($origin); - $protocol = ''; + $origin = strtolower($origin); + $parsed = parse_url($origin); + $originHost = $origin; if (true === is_array($parsed)) { if (true === isset($parsed['host'])) { $this->addLog('Parsed a hostname from origin: '.$parsed['host']); - $origin = $parsed['host']; - } else { - $this->addLog('Unable to parse hostname from origin'); - } - if (true === isset($parsed['scheme'])) { - $this->addLog('Parsed a protocol from origin: '.$parsed['scheme']); - $protocol = $parsed['scheme'].'://'; - } else { - $this->addLog('Unable to parse protocol/scheme from origin'); + $originHost = $parsed['host']; } } else { - $this->addLog('Unable to parse URL from origin of '.$origin); + $parsed = []; } - // read the current origin setting $originSetting = $this->settings['origin']; @@ -209,11 +200,11 @@ protected function parseOrigin(ServerRequestInterface $request) : string foreach ($originSetting as $item) { // see if the origin matches (the parseOriginMatch function supports // wildcards) - $matched = $this->parseOriginMatch($item, $origin); + $matched = $this->parseOriginMatch($item, $originHost); // if anything else but '' was returned, then we have a valid match. if ('' !== $matched) { $this->addLog('Iterator found a matched origin of '.$matched); - $matched = $this->addProtocolIfNeeded($protocol, $matched); + $matched = $this->addProtocolPortIfNeeded($matched, $parsed); return $matched; } } @@ -223,31 +214,59 @@ protected function parseOrigin(ServerRequestInterface $request) : string // is to try to match it as a string (if applicable) if ('' === $matched && true === is_string($originSetting)) { $this->addLog('Attempting to match origin as string'); - $matched = $this->parseOriginMatch($originSetting, $origin); + $matched = $this->parseOriginMatch($originSetting, $originHost); } // return the matched setting (may be '' to indicate nothing matched) - $matched = $this->addProtocolIfNeeded($protocol, $matched); + $matched = $this->addProtocolPortIfNeeded($matched, $parsed); return $matched; }//end parseOrigin() /** * Returns the protocol if needed. * - * @param string $protocol Protocol to add if matched is not empty or *. - * @param string $matched The matched host. + * @param string $matched The matched host. + * @param array $parsed The results of parse_url. * * @return string */ - protected function addProtocolIfNeeded(string $protocol, string $matched) : string + protected function addProtocolPortIfNeeded(string $matched, array $parsed) : string { if ('' === $matched || '*' === $matched) { $return = $matched; + + return $return; + } + $protocol = 'https://'; + $port = 0; + if (true === isset($parsed['scheme'])) { + $this->addLog('Parsed a protocol from origin: '.$parsed['scheme']); + $protocol = $parsed['scheme'].'://'; + } else { + $this->addLog('Unable to parse protocol/scheme from origin'); + } + if (true === isset($parsed['port'])) { + $this->addLog('Parsed a port from origin: '.$parsed['port']); + $port = (int) $parsed['port']; } else { + $this->addLog('Unable to parse port from origin'); + } + + if (0 === $port) { + if ('https://' === $protocol) { + $port = 443; + } else { + $port = 80; + } + } + + if (('http://' === $protocol && 80 === $port) || ('https://' === $protocol && 443 === $port)) { $return = $protocol.$matched; + } else { + $return = $protocol.$matched.':'.$port; } return $return; - }//end addProtocolIfNeeded() + }//end addProtocolPortIfNeeded() /** * Check to see if an origin string matches an item (wildcarded or not). diff --git a/tests/MiddlewareCors/PreflightTest.php b/tests/MiddlewareCors/PreflightTest.php index 671bddd..9ff0893 100644 --- a/tests/MiddlewareCors/PreflightTest.php +++ b/tests/MiddlewareCors/PreflightTest.php @@ -359,35 +359,35 @@ public function testInvokerPreflightValidAcrmValidAcrh() */ public function testInvokerPreflightAllTheThings() { - $results = $this->runInvoke( - [ - 'method' => 'OPTIONS', - 'setHeaders' => [ - 'origin' => 'example.com', - 'access-control-request-method' => 'put', - 'access-control-request-headers' => 'x-jeff, x-smith, x-jones' - ], - 'configuration' => [ - 'allowMethods' => ['PUT', 'POST'], - 'allowHeaders' => 'x-jeff,x-smith,x-jones', - 'maxAge' => 300, - 'allowCredentials' => true, - 'origin' => 'example.com' + $results = $this->runInvoke( + [ + 'method' => 'OPTIONS', + 'setHeaders' => [ + 'origin' => 'http://example.com', + 'access-control-request-method' => 'put', + 'access-control-request-headers' => 'x-jeff, x-smith, x-jones' + ], + 'configuration' => [ + 'allowMethods' => ['PUT', 'POST'], + 'allowHeaders' => 'x-jeff,x-smith,x-jones', + 'maxAge' => 300, + 'allowCredentials' => true, + 'origin' => 'example.com' + ] ] - ] - ); - $expected = [ - 'withHeader:Access-Control-Allow-Origin' => 'example.com', - 'withHeader:Access-Control-Allow-Credentials' => 'true', - 'withHeader:Access-Control-Allow-Methods' => 'PUT, POST', - 'withHeader:Access-Control-Allow-Headers' => 'x-jeff, x-smith, x-jones', - 'withHeader:Access-Control-Max-Age' => 300, - 'withAddedHeader:Vary' => 'Origin', - 'withStatus' => '204:No Content', - 'withoutHeader:Content-Type' => true, - 'withoutHeader:Content-Length' => true - ]; - $this->arraysAreSimilar($expected, $results); + ); + $expected = [ + 'withHeader:Access-Control-Allow-Origin' => 'http://example.com', + 'withHeader:Access-Control-Allow-Credentials' => 'true', + 'withHeader:Access-Control-Allow-Methods' => 'PUT, POST', + 'withHeader:Access-Control-Allow-Headers' => 'x-jeff, x-smith, x-jones', + 'withHeader:Access-Control-Max-Age' => 300, + 'withAddedHeader:Vary' => 'Origin', + 'withStatus' => '204:No Content', + 'withoutHeader:Content-Type' => true, + 'withoutHeader:Content-Length' => true + ]; + $this->arraysAreSimilar($expected, $results); }//end testInvokerPreflightAllTheThings() /** diff --git a/tests/MiddlewareCorsTest.php b/tests/MiddlewareCorsTest.php index 36b64db..5ed0e98 100644 --- a/tests/MiddlewareCorsTest.php +++ b/tests/MiddlewareCorsTest.php @@ -176,14 +176,12 @@ public function testInvokerWithOriginHeader() 'configuration' => [] ] ); - $expected = ['withHeader:Access-Control-Allow-Origin' => '*', 'calledNext' => 'called']; - $this->arraysAreSimilar($results, $expected); // check logs + $expectedLogs=[ 'Request has an origin setting and is being treated like a CORs request', 'Processing origin of "http://example.com"', 'Parsed a hostname from origin: example.com', - 'Parsed a protocol from origin: http', 'Attempting to match origin as string', 'Checking configuration origin of "*" against user "example.com"', 'Origin is either an empty string or wildcarded star. Returning *', @@ -192,7 +190,9 @@ public function testInvokerWithOriginHeader() ]; $logEntries=$this->getLoggerStrings(); $this->assertEquals($expectedLogs,$logEntries); - + // check output + $expected = ['withHeader:Access-Control-Allow-Origin' => '*', 'calledNext' => 'called']; + $this->arraysAreSimilar($results, $expected); }//end testInvokerWithOriginHeader() /** @@ -226,7 +226,6 @@ public function testInvokerWithFullyQualifiedOriginHeader() 'Request has an origin setting and is being treated like a CORs request', 'Processing origin of "http://example.com:83/text.html"', 'Parsed a hostname from origin: example.com', - 'Parsed a protocol from origin: http', 'Attempting to match origin as string', 'Checking configuration origin of "*" against user "example.com"', 'Origin is either an empty string or wildcarded star. Returning *', @@ -261,24 +260,111 @@ public function testInvokerWithFullyQualifiedOriginHeaderAndProtocol() 'configuration' => ['origin' => 'example.com'] ] ); - $expected = ['withHeader:Access-Control-Allow-Origin' => 'https://example.com', 'calledNext' => 'called']; + $expected = ['withHeader:Access-Control-Allow-Origin' => 'https://example.com:83', 'calledNext' => 'called']; $this->arraysAreSimilar($results, $expected); // check logs $expectedLogs=[ 'Request has an origin setting and is being treated like a CORs request', 'Processing origin of "https://example.com:83/text.html"', 'Parsed a hostname from origin: example.com', + 'Attempting to match origin as string', + 'Checking configuration origin of "example.com" against user "example.com"', + 'Origin is an exact case insensitive match', 'Parsed a protocol from origin: https', + 'Parsed a port from origin: 83', + 'Processing with origin of "https://example.com:83"', + 'Calling next bit of middleware' + ]; + $logEntries=$this->getLoggerStrings(); + $this->assertEquals($expectedLogs,$logEntries); + + }//end testInvokerWithOriginHeaderAndProtocol() + /** + * Runs a test based on this having: + * - Method: GET + * - * allowed origin (default) + * - Origin set to example.com (matching wildcard) + * should get + * Access-Control-Allow-Origin + * and next called. + * + * @test + * @covers \Bairwell\MiddlewareCors::__construct + * @covers \Bairwell\MiddlewareCors::__invoke + * @covers \Bairwell\MiddlewareCors\Traits\Parse::parseOriginMatch + * @covers \Bairwell\MiddlewareCors\Traits\Parse::parseOrigin + */ + public function testInvokerWithFullyQualifiedOriginHeaderAndProtocolStandardPort() + { + $results = $this->runInvoke( + [ + 'method' => 'GET', + 'setHeaders' => ['origin' => 'https://example.com:443/text.html'], + 'configuration' => ['origin' => 'example.com'] + ] + ); + $expected = ['withHeader:Access-Control-Allow-Origin' => 'https://example.com', 'calledNext' => 'called']; + $this->arraysAreSimilar($results, $expected); + // check logs + $expectedLogs=[ + 'Request has an origin setting and is being treated like a CORs request', + 'Processing origin of "https://example.com:443/text.html"', + 'Parsed a hostname from origin: example.com', 'Attempting to match origin as string', 'Checking configuration origin of "example.com" against user "example.com"', 'Origin is an exact case insensitive match', + 'Parsed a protocol from origin: https', + 'Parsed a port from origin: 443', 'Processing with origin of "https://example.com"', 'Calling next bit of middleware' ]; $logEntries=$this->getLoggerStrings(); $this->assertEquals($expectedLogs,$logEntries); - }//end testInvokerWithOriginHeaderAndProtocol() + }//end testInvokerWithOriginHeaderAndProtocolStandardPort() + /** + * Runs a test based on this having: + * - Method: GET + * - * allowed origin (default) + * - Origin set to example.com (matching wildcard) + * should get + * Access-Control-Allow-Origin + * and next called. + * + * @test + * @covers \Bairwell\MiddlewareCors::__construct + * @covers \Bairwell\MiddlewareCors::__invoke + * @covers \Bairwell\MiddlewareCors\Traits\Parse::parseOriginMatch + * @covers \Bairwell\MiddlewareCors\Traits\Parse::parseOrigin + */ + public function testInvokerWithFullyQualifiedOriginHeaderAndProtocolNoPort() + { + $results = $this->runInvoke( + [ + 'method' => 'GET', + 'setHeaders' => ['origin' => 'https://testing.com/text.html'], + 'configuration' => ['origin' => 'testing.com'] + ] + ); + $expected = ['withHeader:Access-Control-Allow-Origin' => 'https://testing.com', 'calledNext' => 'called']; + $this->arraysAreSimilar($results, $expected); + // check logs + $expectedLogs=[ + 'Request has an origin setting and is being treated like a CORs request', + 'Processing origin of "https://testing.com/text.html"', + 'Parsed a hostname from origin: testing.com', + 'Attempting to match origin as string', + 'Checking configuration origin of "testing.com" against user "testing.com"', + 'Origin is an exact case insensitive match', + 'Parsed a protocol from origin: https', + 'Unable to parse port from origin', + 'Processing with origin of "https://testing.com"', + 'Calling next bit of middleware' + ]; + $logEntries=$this->getLoggerStrings(); + $this->assertEquals($expectedLogs,$logEntries); + + }//end testInvokerWithOriginHeaderAndProtocolNoPort() /** * Runs a test based on this having: * - Method: GET @@ -303,22 +389,25 @@ public function testInvokerWithCustomOriginHeader() 'configuration' => ['origin' => 'example.com'] ] ); - $expected = ['withHeader:Access-Control-Allow-Origin' => 'http://example.com', 'calledNext' => 'called']; - $this->arraysAreSimilar($results, $expected); + // check logs $expectedLogs=[ 'Request has an origin setting and is being treated like a CORs request', 'Processing origin of "http://example.com"', 'Parsed a hostname from origin: example.com', - 'Parsed a protocol from origin: http', 'Attempting to match origin as string', 'Checking configuration origin of "example.com" against user "example.com"', 'Origin is an exact case insensitive match', + 'Parsed a protocol from origin: http', + 'Unable to parse port from origin', 'Processing with origin of "http://example.com"', 'Calling next bit of middleware' ]; $logEntries=$this->getLoggerStrings(); $this->assertEquals($expectedLogs,$logEntries); + // + $expected = ['withHeader:Access-Control-Allow-Origin' => 'http://example.com', 'calledNext' => 'called']; + $this->arraysAreSimilar($results, $expected); }//end testInvokerWithCustomOriginHeader() /** @@ -356,8 +445,6 @@ public function testInvokerWithCustomOriginHeaderInvalid() $expectedLogs=[ 'Request has an origin setting and is being treated like a CORs request', 'Processing origin of "dummy.com"', - 'Unable to parse hostname from origin', - 'Unable to parse protocol/scheme from origin', 'Attempting to match origin as string', 'Checking configuration origin of "example.com" against user "dummy.com"', 'Unable to match "example.com" against user "dummy.com"' @@ -432,8 +519,6 @@ public function testInvokerWithCustomOriginHeaderDummyCallback() $expectedLogs=[ 'Request has an origin setting and is being treated like a CORs request', 'Processing origin of "dummy.com"', - 'Unable to parse hostname from origin', - 'Unable to parse protocol/scheme from origin', 'Attempting to match origin as string', 'Checking configuration origin of "example.com" against user "dummy.com"', 'Unable to match "example.com" against user "dummy.com"' @@ -479,8 +564,6 @@ public function testInvokerWithCustomOriginHeaderCustomCallbacks() $expectedLogs=[ 'Request has an origin setting and is being treated like a CORs request', 'Processing origin of "dummy.com"', - 'Unable to parse hostname from origin', - 'Unable to parse protocol/scheme from origin', 'Origin server request is being passed to callback', 'Attempting to match origin as string', 'Checking configuration origin of "hello" against user "dummy.com"', @@ -517,19 +600,19 @@ public function testInvokerWithCustomOriginHeaderCustomAllowedCallbacks() 'configuration' => ['origin' => $originCallback] ] ); - $expected = ['withHeader:Access-Control-Allow-Origin' => 'dummy.com', 'calledNext' => 'called']; + $expected = ['withHeader:Access-Control-Allow-Origin' => 'https://dummy.com', 'calledNext' => 'called']; $this->arraysAreSimilar($results, $expected); // check logs $expectedLogs=[ 'Request has an origin setting and is being treated like a CORs request', 'Processing origin of "dummy.com"', - 'Unable to parse hostname from origin', - 'Unable to parse protocol/scheme from origin', 'Origin server request is being passed to callback', 'Attempting to match origin as string', 'Checking configuration origin of "dummy.com" against user "dummy.com"', 'Origin is an exact case insensitive match', - 'Processing with origin of "dummy.com"', + 'Unable to parse protocol/scheme from origin', + 'Unable to parse port from origin', + 'Processing with origin of "https://dummy.com"', 'Calling next bit of middleware' ]; $logEntries=$this->getLoggerStrings(); @@ -560,21 +643,21 @@ public function testInvokerWithOriginArray() 'configuration' => ['origin' => ['example.com', 'dummy.com']] ] ); - $expected = ['withHeader:Access-Control-Allow-Origin' => 'dummy.com', 'calledNext' => 'called']; + $expected = ['withHeader:Access-Control-Allow-Origin' => 'https://dummy.com', 'calledNext' => 'called']; $this->arraysAreSimilar($results, $expected); // check logs $expectedLogs=[ 'Request has an origin setting and is being treated like a CORs request', 'Processing origin of "dummy.com"', - 'Unable to parse hostname from origin', - 'Unable to parse protocol/scheme from origin', 'Iterating through Origin array', 'Checking configuration origin of "example.com" against user "dummy.com"', 'Unable to match "example.com" against user "dummy.com"', 'Checking configuration origin of "dummy.com" against user "dummy.com"', 'Origin is an exact case insensitive match', 'Iterator found a matched origin of dummy.com', - 'Processing with origin of "dummy.com"', + 'Unable to parse protocol/scheme from origin', + 'Unable to parse port from origin', + 'Processing with origin of "https://dummy.com"', 'Calling next bit of middleware' ]; $logEntries=$this->getLoggerStrings(); @@ -605,21 +688,21 @@ public function testInvokerWithOriginArrayWildcard() 'configuration' => ['origin' => ['example.com', '*.dummy.com']] ] ); - $expected = ['withHeader:Access-Control-Allow-Origin' => 'www.dummy.com', 'calledNext' => 'called']; + $expected = ['withHeader:Access-Control-Allow-Origin' => 'https://www.dummy.com', 'calledNext' => 'called']; $this->arraysAreSimilar($results, $expected); // check logs $expectedLogs=[ 'Request has an origin setting and is being treated like a CORs request', 'Processing origin of "www.dummy.com"', - 'Unable to parse hostname from origin', - 'Unable to parse protocol/scheme from origin', 'Iterating through Origin array', 'Checking configuration origin of "example.com" against user "www.dummy.com"', 'Unable to match "example.com" against user "www.dummy.com"', 'Checking configuration origin of "*.dummy.com" against user "www.dummy.com"', 'Wildcarded origin match with www.dummy.com', 'Iterator found a matched origin of www.dummy.com', - 'Processing with origin of "www.dummy.com"', + 'Unable to parse protocol/scheme from origin', + 'Unable to parse port from origin', + 'Processing with origin of "https://www.dummy.com"', 'Calling next bit of middleware' ]; $logEntries=$this->getLoggerStrings(); @@ -661,8 +744,6 @@ public function testInvokerWithOriginArrayInvalid() $expectedLogs=[ 'Request has an origin setting and is being treated like a CORs request', 'Processing origin of "bbc.co.uk"', - 'Unable to parse hostname from origin', - 'Unable to parse protocol/scheme from origin', 'Iterating through Origin array', 'Checking configuration origin of "example.com" against user "bbc.co.uk"', 'Unable to match "example.com" against user "bbc.co.uk"', @@ -707,8 +788,6 @@ public function testInvokerWithOriginHeaderAndCredentials() $expectedLogs=[ 'Request has an origin setting and is being treated like a CORs request', 'Processing origin of "example.com"', - 'Unable to parse hostname from origin', - 'Unable to parse protocol/scheme from origin', 'Attempting to match origin as string', 'Checking configuration origin of "*" against user "example.com"', 'Origin is either an empty string or wildcarded star. Returning *', @@ -756,8 +835,6 @@ public function testInvokerWithOriginHeaderAndCredentialsWithHeaders() $expectedLogs=[ 'Request has an origin setting and is being treated like a CORs request', 'Processing origin of "example.com"', - 'Unable to parse hostname from origin', - 'Unable to parse protocol/scheme from origin', 'Attempting to match origin as string', 'Checking configuration origin of "*" against user "example.com"', 'Origin is either an empty string or wildcarded star. Returning *',