Do not use a user_id provided in a query string

The practice leads to the ability for an attacker to authenticate as one user,
but then spoof as another user on later, authenticated requests.

config/module.config.php

@@ -51,10 +51,7 @@
     ),
     'service_manager' => array(
         'aliases' => array(
-            'ZF\OAuth2\Provider\UserId' => 'ZF\OAuth2\Provider\UserId\Request',
-        ),
-        'invokables' => array(
-            'ZF\OAuth2\Provider\UserId\Request' => 'ZF\OAuth2\Provider\UserId\Request',
+            'ZF\OAuth2\Provider\UserId' => 'ZF\OAuth2\Provider\UserId\AuthenticationService',
         ),
         'factories' => array(
             'ZF\OAuth2\Adapter\PdoAdapter'    => 'ZF\OAuth2\Factory\PdoAdapterFactory',
@@ -98,7 +95,7 @@
          *
          * If true, client errors are returned using the
          * application/problem+json content type,
-         * otherwise in the format described in the oauth2 specification 
+         * otherwise in the format described in the oauth2 specification
          * (default: true)
          */
         'api_problem_error_response' => true,

src/Provider/UserId/AuthenticationService.php

@@ -16,24 +16,57 @@ class AuthenticationService implements UserIdProviderInterface
      */
     private $authenticationService;
 
+    /**
+     * @var string
+     */
+    private $userId = 'id';
+
     /**
      *  Set authentication service
      *
      * @param ZendAuthenticationService $service
+     * @param array $config
      */
-    public function __construct(ZendAuthenticationService $service)
+    public function __construct(ZendAuthenticationService $service = null, $config = array())
     {
         $this->authenticationService = $service;
+
+        if (isset($config['zf-oauth2']['user_id'])) {
+            $this->userId = $config['zf-oauth2']['user_id'];
+        }
     }
 
     /**
      * Use Zend\Authentication\AuthenticationService to fetch the identity.
      *
-     * @param RequestInterface $request
+     * @param  RequestInterface $request
      * @return mixed
      */
     public function __invoke(RequestInterface $request)
     {
-        return $this->authenticationService->getIdentity();
+        if (empty($this->authenticationService)) {
+            return null;
+        }
+
+        $identity = $this->authenticationService->getIdentity();
+
+        if (is_object($identity)) {
+            if (property_exists($identity, $this->userId)) {
+                return $identity->{$this->$userId};
+            }
+
+            $method = "get" . ucfirst($this->userId);
+            if (method_exists($identity, $method)) {
+                return $identity->$method();
+            }
+
+            return null;
+        }
+
+        if (is_array($identity) && isset($identity[$this->userId])) {
+            return $identity[$this->userId];
+        }
+
+        return null;
     }
 }

src/Provider/UserId/AuthenticationServiceFactory.php

@@ -17,6 +17,15 @@ class AuthenticationServiceFactory implements FactoryInterface
      */
     public function createService(ServiceLocatorInterface $services)
     {
-        return new AuthenticationService($services->get('Zend\Authentication\AuthenticationService'));
+        $config = $services->get('Config');
+
+        if ($services->has('Zend\Authentication\AuthenticationService')) {
+            return new AuthenticationService(
+                $services->get('Zend\Authentication\AuthenticationService'),
+                $config
+            );
+        }
+
+        return new AuthenticationService(null, $config);
     }
 }

test/Controller/AuthControllerTest.php

@@ -172,7 +172,6 @@ public function testAuthorizeCode()
             'response_type' => 'code',
             'client_id'     => 'testclient',
             'state'         => 'xyz',
-            'user_id'       => 123,
             'redirect_uri'  => '/oauth/receivecode',
         )));
         $request->setPost(new Parameters(array(
@@ -199,7 +198,7 @@ public function testAuthorizeCode()
 
         $selectString = $sql->getSqlStringForSqlObject($select);
         $results = $adapter->query($selectString, $adapter::QUERY_MODE_EXECUTE)->toArray();
-        $this->assertEquals('123', $results[0]['user_id']);
+        $this->assertEquals(null, $results[0]['user_id']);
 
         // test get token from authorized code
         $request = $this->getRequest();

test/Controller/AuthControllerWithZendAuthenticationServiceTest.php

@@ -97,7 +97,7 @@ public function testAuthorizeCode()
             ->query($query)
             ->fetch();
 
-        $this->assertEquals('123', $row['user_id']);
+        $this->assertEquals(null, $row['user_id']);
 
         // test get token from authorized code
         $request = $this->getRequest();