From 9775c0076bfb6c24908de29ae198bd2207d809d5 Mon Sep 17 00:00:00 2001
From: Ian Littman <ian@cloudyhills.com>
Date: Tue, 21 Jun 2016 21:08:38 -0500
Subject: [PATCH] Look at Authorization header directly for HTTP Basic auth
 check

Should allow for better compatibility with server implementations that aren't sitting on top of a standard SAPI (e.g. persistent web servers building a PSR-7 compatible request from a socket-received message).

One catch here is that I've seen Apache hijack the HTTP Authorization header in the past, though that would probably impact the other aspects of the server just as much as it would this, so I think that risk is manageable.

Added tests to cover all paths through the new code, so the AbstractGrant type still has 100% coverage :)

Did notice that, as of the latest versions of PHPUnit, the mock creation method is deprecated. Maybe that needs to be updated? Haven't checked to see whether the replacements are PHPUnit 4.8 compatible though, so maybe they need to stay in order to test on older PHP versions?
---
 src/Grant/AbstractGrant.php       | 46 +++++++++++++++-----
 tests/Grant/AbstractGrantTest.php | 70 +++++++++++++++++++++++++++++++
 2 files changed, 106 insertions(+), 10 deletions(-)

diff --git a/src/Grant/AbstractGrant.php b/src/Grant/AbstractGrant.php
index 9c72a9a1..547ab213 100644
--- a/src/Grant/AbstractGrant.php
+++ b/src/Grant/AbstractGrant.php
@@ -137,21 +137,15 @@ abstract class AbstractGrant implements GrantTypeInterface
      */
     protected function validateClient(ServerRequestInterface $request)
     {
-        $clientId = $this->getRequestParameter(
-            'client_id',
-            $request,
-            $this->getServerParameter('PHP_AUTH_USER', $request)
-        );
+        list($basicAuthUser, $basicAuthPassword) = $this->getBasicAuthCredentials($request);
+
+        $clientId = $this->getRequestParameter('client_id', $request, $basicAuthUser);
         if (is_null($clientId)) {
             throw OAuthServerException::invalidRequest('client_id');
         }
 
         // If the client is confidential require the client secret
-        $clientSecret = $this->getRequestParameter(
-            'client_secret',
-            $request,
-            $this->getServerParameter('PHP_AUTH_PW', $request)
-        );
+        $clientSecret = $this->getRequestParameter('client_secret', $request, $basicAuthPassword);
 
         $client = $this->clientRepository->getClientEntity(
             $clientId,
@@ -237,6 +231,38 @@ abstract class AbstractGrant implements GrantTypeInterface
         return isset($requestParameters[$parameter]) ? $requestParameters[$parameter] : $default;
     }
 
+    /**
+     * Retrieve HTTP Basic Auth credentials with the Authorization header
+     * of a request. First index of the returned array is the username,
+     * second is the password (so list() will work). If the header does
+     * not exist, or is otherwise an invalid HTTP Basic header, return
+     * [null, null].
+     *
+     * @param \Psr\Http\Message\ServerRequestInterface $request
+     * @return string[]|null[]
+     */
+    protected function getBasicAuthCredentials(ServerRequestInterface $request)
+    {
+        if (!$request->hasHeader('Authorization')) {
+            return [null, null];
+        }
+
+        $header = $request->getHeader('Authorization')[0];
+        if (strpos($header, 'Basic ') !== 0) {
+            return [null, null];
+        }
+
+        if (!($decoded = base64_decode(substr($header, 6)))) {
+            return [null, null];
+        }
+
+        if (strpos($decoded, ':') === false) {
+            return [null, null]; // HTTP Basic header without colon isn't valid
+        }
+
+        return explode(':', $decoded, 2);
+    }
+
     /**
      * Retrieve query string parameter.
      *
diff --git a/tests/Grant/AbstractGrantTest.php b/tests/Grant/AbstractGrantTest.php
index 6e516409..deafbdcd 100644
--- a/tests/Grant/AbstractGrantTest.php
+++ b/tests/Grant/AbstractGrantTest.php
@@ -32,6 +32,76 @@ class AbstractGrantTest extends \PHPUnit_Framework_TestCase
         $grantMock->setEmitter(new Emitter());
     }
 
+    public function testHttpBasicWithPassword()
+    {
+        /** @var AbstractGrant $grantMock */
+        $grantMock = $this->getMockForAbstractClass(AbstractGrant::class);
+        $abstractGrantReflection = new \ReflectionClass($grantMock);
+
+        $serverRequest = new ServerRequest();
+        $serverRequest = $serverRequest->withHeader('Authorization', 'Basic ' . base64_encode('Open:Sesame'));
+        $basicAuthMethod = $abstractGrantReflection->getMethod('getBasicAuthCredentials');
+        $basicAuthMethod->setAccessible(true);
+
+        $this->assertSame(['Open', 'Sesame'], $basicAuthMethod->invoke($grantMock, $serverRequest));
+    }
+
+    public function testHttpBasicNoPassword()
+    {
+        /** @var AbstractGrant $grantMock */
+        $grantMock = $this->getMockForAbstractClass(AbstractGrant::class);
+        $abstractGrantReflection = new \ReflectionClass($grantMock);
+
+        $serverRequest = new ServerRequest();
+        $serverRequest = $serverRequest->withHeader('Authorization', 'Basic ' . base64_encode('Open:'));
+        $basicAuthMethod = $abstractGrantReflection->getMethod('getBasicAuthCredentials');
+        $basicAuthMethod->setAccessible(true);
+
+        $this->assertSame(['Open', ''], $basicAuthMethod->invoke($grantMock, $serverRequest));
+    }
+
+    public function testHttpBasicNotBasic()
+    {
+        /** @var AbstractGrant $grantMock */
+        $grantMock = $this->getMockForAbstractClass(AbstractGrant::class);
+        $abstractGrantReflection = new \ReflectionClass($grantMock);
+
+        $serverRequest = new ServerRequest();
+        $serverRequest = $serverRequest->withHeader('Authorization', 'Foo ' . base64_encode('Open:Sesame'));
+        $basicAuthMethod = $abstractGrantReflection->getMethod('getBasicAuthCredentials');
+        $basicAuthMethod->setAccessible(true);
+
+        $this->assertSame([null, null], $basicAuthMethod->invoke($grantMock, $serverRequest));
+    }
+
+    public function testHttpBasicNotBase64()
+    {
+        /** @var AbstractGrant $grantMock */
+        $grantMock = $this->getMockForAbstractClass(AbstractGrant::class);
+        $abstractGrantReflection = new \ReflectionClass($grantMock);
+
+        $serverRequest = new ServerRequest();
+        $serverRequest = $serverRequest->withHeader('Authorization', 'Basic ||');
+        $basicAuthMethod = $abstractGrantReflection->getMethod('getBasicAuthCredentials');
+        $basicAuthMethod->setAccessible(true);
+
+        $this->assertSame([null, null], $basicAuthMethod->invoke($grantMock, $serverRequest));
+    }
+
+    public function testHttpBasicNoColon()
+    {
+        /** @var AbstractGrant $grantMock */
+        $grantMock = $this->getMockForAbstractClass(AbstractGrant::class);
+        $abstractGrantReflection = new \ReflectionClass($grantMock);
+
+        $serverRequest = new ServerRequest();
+        $serverRequest = $serverRequest->withHeader('Authorization', 'Basic ' . base64_encode('OpenSesame'));
+        $basicAuthMethod = $abstractGrantReflection->getMethod('getBasicAuthCredentials');
+        $basicAuthMethod->setAccessible(true);
+
+        $this->assertSame([null, null], $basicAuthMethod->invoke($grantMock, $serverRequest));
+    }
+
     public function testValidateClientPublic()
     {
         $client = new ClientEntity();