Finish refactoring Injector services

bigfork · Jun 23, 2016 · 9259bf2 · 9259bf2
1 parent e813c47
commit 9259bf2
Show file tree

Hide file tree

Showing 6 changed files with 101 additions and 99 deletions.
diff --git a/README.md b/README.md
@@ -21,23 +21,27 @@ This module must be installed with composer. Run `composer require bigfork/silve
 
 ## Configuration
 
-Providers are configured using SilverStripe’s YAML configuration, mapping an “internal” name (passed around in URLs and stored in the database) to a PHP class that’s an instance of the PHP League’s `League\OAuth2\Client\Provider\AbstractProvider` class.
-
-Each provider can have a `constructor_options` array that will be passed to the constructor for the given provider class.
+Providers are registered as `Injector` services using SilverStripe’s YAML configuration. This allows you to specify an “internal” name (passed around in URLs and stored in the database), a PHP class for the provider (that extends `League\OAuth2\Client\Provider\AbstractProvider`), and constructor parameters & class properties.
 
 For example, to setup Facebook as a provider, first install the [Facebook OAuth2 package](https://github.com/thephpleague/oauth2-facebook), and then add the following to your YAML config:
 
 ```yml
-Bigfork\SilverStripeOAuth\Client\Factory\ProviderFactory:
-  providers:
-    'Facebook': # "Internal" name
-      class: 'League\OAuth2\Client\Provider\Facebook' # PHP class name
-      constructor_options: # Array to be passed as 1st arg to League\OAuth2\Client\Provider\Facebook::__construct()
+Injector:
+  ProviderFactory:
+    properties:
+      providers:
+        'Facebook': '%$FacebookProvider'
+  FacebookProvider:
+    class: 'League\OAuth2\Client\Provider\Facebook'
+    constructor:
+      Options:
         clientId: '12345678987654321'
         clientSecret: 'geisjgoesingoi3h1521onnro12rin'
         graphApiVersion: 'v2.6'
 ```
 
+Note that in the above example, the required `redirectUri` constructor argument is missing. This module will automatically update the service configuration to add this argument to all providers, to save having to update the URL when moving between environments/domain names. If the `redirectUri` argument is present, it will not be overridden.
+
 ---
 
 ## Concepts
@@ -70,7 +74,7 @@ The module includes one extra controller, `Bigfork\SilverStripeOAuth\Client\Cont
 
 ### Helper
 
-A simple class to help build an authentication request URL to create an access token.
+A simple class to help build an authentication request URL to create an access token. Also responsible for ensuring the `redirectUri` option is set in each provider’s service configuration.
 
 ---
 
@@ -124,7 +128,5 @@ if ($facebookToken->isExpired()) {
 ## Todo
 
 - Unit tests!
-- Investigate swapping `class` and `constructor_options` to instead be a service registered via `Injector`. Currently this is limited by the fact that we need to override the constructor arguments to add a `redirectUri` key to the first argument to `League\OAuth2\Client\Provider\AbstractProvider::__construct()`
-- Add in support for the `$collaborators` argument for `League\OAuth2\Client\Provider\AbstractProvider::__construct()`. This may depend on the outcome of the above item
 - Make the default behaviour of only allowing one access token per provider on each member optional, or just remove it
-- Better passing-around of redirect Uris, it's currently a bit messy
+- Allow controller extensions to better influence request/response flow?
diff --git a/_config.php b/_config.php
@@ -1,3 +1,3 @@
 <?php
 
-Bigfork\SilverStripeOAuth\Client\Helper\ConfigHelper::addRedirectUriToConfigs();
+Bigfork\SilverStripeOAuth\Client\Helper\Helper::addRedirectUriToConfigs();
diff --git a/src/Control/Controller.php b/src/Control/Controller.php
@@ -10,6 +10,7 @@
 use OAuthScope;
 use Session;
 use SS_HTTPRequest;
+use SS_HTTPResponse;
 
 class Controller extends SilverStripeController
 {
@@ -49,7 +50,6 @@ public function authenticate(SS_HTTPRequest $request)
     {
         $providerName = $request->getVar('provider');
         $scope = $request->getVar('scope');
-        $redirectUri = $request->getVar('redirectUri');
 
         // Missing or invalid data means we can't proceed
         if (!$providerName || !is_array($scope)) {
@@ -59,7 +59,7 @@ public function authenticate(SS_HTTPRequest $request)
         $provider = Injector::inst()->get('ProviderFactory')->getProvider($providerName);
 
         $scope = empty($scope) ? $provider->getDefaultScopes() : $scope;
-        $url = $provider->getAuthorizationUrl(['scope' => $scope, 'redirectUri' => $redirectUri]);
+        $url = $provider->getAuthorizationUrl(['scope' => $scope]);
 
         // Copied from \Controler::redirectBack()
         if ($request->requestVar('BackURL')) {
@@ -95,15 +95,27 @@ public function callback(SS_HTTPRequest $request)
         }
 
         $providerName = Session::get('oauth2.provider');
-        $redirectUri = Controller::join_links(Director::absoluteBaseURL(), $this->owner->Link(), 'callback/');
-        $provider = Injector::inst()->get('ProviderFactory')
-            ->getProvider($providerName, $redirectUri);
+        $provider = Injector::inst()->get('ProviderFactory')->getProvider($providerName);
 
         try {
+            $results = $this->extend('beforeGetAccessToken', $provider, $providerName, $request);
+            foreach ($results as $result) {
+                if ($result instanceof SS_HTTPResponse) {
+                    return $result;
+                }
+            }
+
             $token = $provider->getAccessToken('authorization_code', [
                 'code' => $request->getVar('code')
             ]);
 
+            $results = $this->extend('afterGetAccessToken', $provider, $token, $providerName, $request);
+            foreach ($results as $result) {
+                if ($result instanceof SS_HTTPResponse) {
+                    return $result;
+                }
+            }
+
             $member = Member::currentUser();
             $existingToken = $member->AccessTokens()->filter(['Provider' => $providerName])->first();
 

diff --git a/src/Helper/ConfigHelper.php b/src/Helper/ConfigHelper.php
diff --git a/src/Helper/Helper.php b/src/Helper/Helper.php
@@ -2,25 +2,29 @@
 
 namespace Bigfork\SilverStripeOAuth\Client\Helper;
 
+use Config;
 use Controller;
 use Director;
 use Injector;
 
 class Helper
 {
+    /**
+     * @var string
+     */
+    private static $default_redirect_uri;
+
     /**
      * @param string $provider
      * @param array $scopes
-     * @param string $redirectUri
      * @return string
      */
-    public static function buildAuthorisationUrl($provider, array $scopes = [], $redirectUri = '')
+    public static function buildAuthorisationUrl($provider, array $scopes = [])
     {
         $controller = Injector::inst()->get('Bigfork\SilverStripeOAuth\Client\Control\Controller');
         $data = [
             'provider' => $provider,
-            'scope' => $scopes,
-            'redirectUri' => $redirectUri
+            'scope' => $scopes
         ];
 
         return Controller::join_links(
@@ -29,4 +33,64 @@ public static function buildAuthorisationUrl($provider, array $scopes = [], $red
             'authenticate/?' . http_build_query($data)
         );
     }
+
+    /**
+     * Adds the redirectUri option to each of the configured provider's service
+     * configs: the redirectUri is required on construction
+     */
+    public static function addRedirectUriToConfigs()
+    {
+        $factoryConfig = Config::inst()->get('Injector', 'ProviderFactory');
+        $providers = $factoryConfig['properties']['providers'];
+
+        foreach ($providers as $name => $spec) {
+            // If this is not a service definition, skip it
+            if (strpos($spec, '%$') !== 0) {
+                continue;
+            }
+
+            // Trim %$ServiceName to ServiceName
+            $serviceName = substr($spec, 2);
+            $serviceConfig = (array)Config::inst()->get('Injector', $serviceName);
+
+            if (!empty($serviceConfig)) {
+                $serviceConfig = static::addRedirectUriToServiceConfig($serviceConfig);
+                Config::inst()->update('Injector', $serviceName, $serviceConfig);
+                Injector::inst()->load(array($serviceName => $serviceConfig));
+            }
+        }
+    }
+
+    /**
+     * Add in the redirectUri option to this service's constructor options
+     * 
+     * @param array $config
+     * @return array
+     */
+    protected static function addRedirectUriToServiceConfig(array $config)
+    {
+        if (!empty($config['constructor']) && is_array($config['constructor'])) {
+            $key = key($config['constructor']); // Key may be non-numeric
+
+            if (!isset($config['constructor'][$key]['redirectUri'])) {
+                $config['constructor'][$key]['redirectUri'] = static::getRedirectUri();
+            }
+        }
+
+        return $config;
+    }
+
+    /**
+     * @return string
+     */
+    protected static function getRedirectUri()
+    {
+        $configUri = Config::inst()->get(__CLASS__, 'default_redirect_uri');
+        if ($configUri) {
+            return $configUri;
+        }
+
+        $controller = Injector::inst()->get('Bigfork\SilverStripeOAuth\Client\Control\Controller');
+        return Controller::join_links($controller->AbsoluteLink(), 'callback/');
+    }
 }
diff --git a/tests/src/Helper/HelperTest.php b/tests/src/Helper/HelperTest.php
@@ -15,11 +15,10 @@ public function testBuildAuthorisationUrl()
         $controller = new HelperTest_Controller;
         Injector::inst()->registerService($controller, 'Bigfork\SilverStripeOAuth\Client\Control\Controller');
 
-        $url = Helper::buildAuthorisationUrl('ProviderName', ['test_scope'], 'http://mysite.com/callback');
+        $url = Helper::buildAuthorisationUrl('ProviderName', ['test_scope']);
 
         $expected = Director::absoluteBaseURL() . 'helpertest/authenticate/';
         $expected .= '?provider=ProviderName&scope%5B0%5D=test_scope';
-        $expected .= '&redirectUri=' . urlencode('http://mysite.com/callback');
 
         $this->assertEquals($expected, $url);
     }