Merge pull request #696 from stevenmaguire/sm-add-guarded-attribute-s…

…upport

Add support to blacklist mass assignment of specific provider properties

src/Provider/AbstractProvider.php

@@ -25,6 +25,7 @@
 use League\OAuth2\Client\Token\AccessToken;
 use League\OAuth2\Client\Token\AccessTokenInterface;
 use League\OAuth2\Client\Tool\ArrayAccessorTrait;
+use League\OAuth2\Client\Tool\GuardedPropertyTrait;
 use League\OAuth2\Client\Tool\QueryBuilderTrait;
 use League\OAuth2\Client\Tool\RequestFactory;
 use Psr\Http\Message\RequestInterface;
@@ -39,6 +40,7 @@
 abstract class AbstractProvider
 {
     use ArrayAccessorTrait;
+    use GuardedPropertyTrait;
     use QueryBuilderTrait;
 
     /**
@@ -109,11 +111,9 @@ abstract class AbstractProvider
      */
     public function __construct(array $options = [], array $collaborators = [])
     {
-        foreach ($options as $option => $value) {
-            if (property_exists($this, $option)) {
-                $this->{$option} = $value;
-            }
-        }
+        // We'll let the GuardedPropertyTrait handle mass assignment of incoming
+        // options, skipping any blacklisted properties defined in the provider
+        $this->fillProperties($options);
 
         if (empty($collaborators['grantFactory'])) {
             $collaborators['grantFactory'] = new GrantFactory();

src/Tool/GuardedPropertyTrait.php

@@ -0,0 +1,70 @@
+<?php
+/**
+ * This file is part of the league/oauth2-client library
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ *
+ * @copyright Copyright (c) Alex Bilbie <[email protected]>
+ * @license http://opensource.org/licenses/MIT MIT
+ * @link http://thephpleague.com/oauth2-client/ Documentation
+ * @link https://packagist.org/packages/league/oauth2-client Packagist
+ * @link https://github.com/thephpleague/oauth2-client GitHub
+ */
+
+namespace League\OAuth2\Client\Tool;
+
+/**
+ * Provides support for blacklisting explicit properties from the
+ * mass assignment behavior.
+ */
+trait GuardedPropertyTrait
+{
+    /**
+     * The properties that aren't mass assignable.
+     *
+     * @var array
+     */
+    protected $guarded = [];
+
+    /**
+     * Attempts to mass assign the given options to explicitly defined properties,
+     * skipping over any properties that are defined in the guarded array.
+     *
+     * @param array $options
+     * @return mixed
+     */
+    protected function fillProperties(array $options = [])
+    {
+        if (isset($options['guarded'])) {
+            unset($options['guarded']);
+        }
+
+        foreach ($options as $option => $value) {
+            if (property_exists($this, $option) && !$this->isGuarded($option)) {
+                $this->{$option} = $value;
+            }
+        }
+    }
+
+    /**
+     * Returns current guarded properties.
+     *
+     * @return array
+     */
+    public function getGuarded()
+    {
+        return $this->guarded;
+    }
+
+    /**
+     * Determines if the given property is guarded.
+     *
+     * @param  string  $property
+     * @return bool
+     */
+    public function isGuarded($property)
+    {
+        return in_array($property, $this->getGuarded());
+    }
+}

test/src/Provider/AbstractProviderTest.php

@@ -2,6 +2,7 @@
 
 namespace League\OAuth2\Client\Test\Provider;
 
+use UnexpectedValueException;
 use Eloquent\Liberator\Liberator;
 use Eloquent\Phony\Phpunit\Phony;
 use GuzzleHttp\Exception\BadResponseException;
@@ -10,6 +11,7 @@
 use League\OAuth2\Client\Test\Provider\Fake as MockProvider;
 use League\OAuth2\Client\Grant\AbstractGrant;
 use League\OAuth2\Client\Grant\GrantFactory;
+use League\OAuth2\Client\Grant\Exception\InvalidGrantException;
 use League\OAuth2\Client\Token\AccessToken;
 use League\OAuth2\Client\Token\AccessTokenInterface;
 use League\OAuth2\Client\Tool\RequestFactory;
@@ -35,19 +37,15 @@ protected function setUp()
         ]);
     }
 
-    /**
-     * @expectedException League\OAuth2\Client\Grant\Exception\InvalidGrantException
-     */
     public function testInvalidGrantString()
     {
+        $this->expectException(InvalidGrantException::class);
         $this->provider->getAccessToken('invalid_grant', ['invalid_parameter' => 'none']);
     }
 
-    /**
-     * @expectedException League\OAuth2\Client\Grant\Exception\InvalidGrantException
-     */
     public function testInvalidGrantObject()
     {
+        $this->expectException(InvalidGrantException::class);
         $grant = new \StdClass();
         $this->provider->getAccessToken($grant, ['invalid_parameter' => 'none']);
     }
@@ -398,11 +396,9 @@ public function testErrorResponsesCanBeCustomizedAtTheProvider()
         );
     }
 
-    /**
-     * @expectedException \League\OAuth2\Client\Provider\Exception\IdentityProviderException
-     */
     public function testClientErrorTriggersProviderException()
     {
+        $this->expectException(IdentityProviderException::class);
         $provider = new MockProvider([
           'clientId' => 'mock_client_id',
           'clientSecret' => 'mock_secret',
@@ -600,29 +596,34 @@ public function parseResponseProvider()
     /**
      * @dataProvider parseResponseProvider
      */
-    public function testParseResponse($body, $type, $parsed)
+    public function testParseResponse($body, $type, $parsed, $statusCode = 200)
     {
         $stream = Phony::mock(StreamInterface::class);
         $stream->__toString->returns($body);
 
         $response = Phony::mock(ResponseInterface::class);
         $response->getBody->returns($stream->get());
         $response->getHeader->with('content-type')->returns($type);
+        $response->getStatusCode->returns($statusCode);
 
         $method = $this->getMethod(AbstractProvider::class, 'parseResponse');
         $result = $method->invoke($this->provider, $response->get());
 
         $this->assertEquals($parsed, $result);
     }
 
-    /**
-     * @expectedException UnexpectedValueException
-     */
     public function testParseResponseJsonFailure()
     {
+        $this->expectException(UnexpectedValueException::class);
         $this->testParseResponse('{a: 1}', 'application/json', null);
     }
 
+    public function testParseResponseNonJsonFailure()
+    {
+        $this->expectException(UnexpectedValueException::class);
+        $this->testParseResponse('<xml></xml>', 'application/xml', null, 500);
+    }
+
     public function getAppendQueryProvider()
     {
         return [
@@ -676,6 +677,31 @@ public function testDefaultPrepareAccessTokenResponse()
         $this->assertEquals($result['user_id'], $newResult['resource_owner_id']);
     }
 
+    public function testGuardedProperties()
+    {
+        $options = [
+            'clientId' => 'mock_client_id',
+            'clientSecret' => 'mock_secret',
+            'redirectUri' => 'none',
+            'skipMeDuringMassAssignment' => 'bar',
+            'guarded' => 'foo'
+        ];
+
+        $provider = new Fake\ProviderWithGuardedProperties($options);
+
+        $this->assertAttributeNotEquals(
+            $options['skipMeDuringMassAssignment'],
+            'skipMeDuringMassAssignment',
+            $provider
+        );
+
+        $this->assertAttributeNotEquals(
+            $options['guarded'],
+            'guarded',
+            $provider
+        );
+    }
+
     public function testPrepareAccessTokenResponseWithDotNotation()
     {
         $provider = Phony::partialMock(Fake\ProviderWithAccessTokenResourceOwnerId::class);

test/src/Provider/Fake/ProviderWithGuardedProperties.php

@@ -0,0 +1,22 @@
+<?php
+
+namespace League\OAuth2\Client\Test\Provider\Fake;
+
+use League\OAuth2\Client\Test\Provider\Fake as MockProvider;
+
+class ProviderWithGuardedProperties extends MockProvider
+{
+    /**
+     * The properties that aren't mass assignable.
+     *
+     * @var array
+     */
+    protected $guarded = ['skipMeDuringMassAssignment'];
+
+    /**
+     * Throwaway property that shouldn't be mass assigned.
+     *
+     * @var string
+     */
+    protected $skipMeDuringMassAssignment = 'foo';
+}