Skip to content

Commit

Permalink
Use real logger interface for service name
Browse files Browse the repository at this point in the history
  • Loading branch information
tractorcow committed Aug 30, 2018
1 parent ba669b6 commit 700e0fc
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 24 deletions.
6 changes: 3 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -103,20 +103,20 @@ Throwing an exception from the `handleToken()` method will result in all other h

## Logging

By default, OAuth errors will be logged to PHP’s error log. You can set up your own logging by overriding the `Bigfork\SilverStripeOAuth\Client\Logger` Injector service definition. For example, to log errors to an `oauth.log` file:
By default, OAuth errors will be logged to PHP’s error log. You can set up your own logging by overriding the `Psr\Log\LoggerInterface.oauth:` Injector service definition. For example, to log errors to an `oauth.log` file:

```yml
---
After: silverstripe-oauth-logging
---
SilverStripe\Core\Injector\Injector:
Bigfork\SilverStripeOAuth\Client\Logger:
Psr\Log\LoggerInterface.oauth:
calls: null # Reset - without this, the below "calls" would be merged in instead of replacing the original
---
After: silverstripe-oauth-logging
---
SilverStripe\Core\Injector\Injector:
Bigfork\SilverStripeOAuth\Client\Logger:
Psr\Log\LoggerInterface.oauth:
calls:
pushDisplayErrorHandler: [ pushHandler, [ '%$OAuthLogFileHandler' ] ]
OAuthLogFileHandler:
Expand Down
4 changes: 3 additions & 1 deletion _config/logging.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,12 @@
Name: silverstripe-oauth-logging
---
SilverStripe\Core\Injector\Injector:
Bigfork\SilverStripeOAuth\Client\Logger:
Psr\Log\LoggerInterface.oauth:
type: singleton
class: Monolog\Logger
constructor:
- "oauth-error-log"
calls:
pushDisplayErrorHandler: [ pushHandler, [ '%$Monolog\Handler\ErrorLogHandler' ] ]
# Alias for old service name
Bigfork\SilverStripeOAuth\Client\Logger: '%$Psr\Log\LoggerInterface.oauth'
39 changes: 21 additions & 18 deletions src/Control/Controller.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,14 @@
namespace Bigfork\SilverStripeOAuth\Client\Control;

use Bigfork\SilverStripeOAuth\Client\Factory\ProviderFactory;
use Bigfork\SilverStripeOAuth\Client\Logger;
use Exception;
use League\OAuth2\Client\Provider\Exception\IdentityProviderException;
use Psr\Log\LoggerInterface;
use SilverStripe\Control\Controller as SilverStripeController;
use SilverStripe\Control\Director;
use SilverStripe\Control\HTTPRequest;
use SilverStripe\Control\HTTPResponse;
use SilverStripe\Control\Controller as SilverStripeController;
use SilverStripe\Control\HTTPResponse_Exception;
use SilverStripe\Core\Injector\Injector;

class Controller extends SilverStripeController
Expand Down Expand Up @@ -43,6 +43,8 @@ protected function findBackUrl(HTTPRequest $request)
$backUrl = $request->getHeader('X-Backurl');
} elseif ($request->getHeader('Referer')) {
$backUrl = $request->getHeader('Referer');
} else {
$backUrl = null;
}

if (!$backUrl || !Director::is_site_url($backUrl)) {
Expand Down Expand Up @@ -82,7 +84,7 @@ public function AbsoluteLink()
* @todo allow whitelisting of scopes (per provider)?
* @param HTTPRequest $request
* @return HTTPResponse
* @throws \SilverStripe\Control\HTTPResponse_Exception
* @throws HTTPResponse_Exception
*/
public function authenticate(HTTPRequest $request)
{
Expand All @@ -93,16 +95,12 @@ public function authenticate(HTTPRequest $request)
// Missing or invalid data means we can't proceed
if (!$providerName || !is_array($scope)) {
$this->httpError(404);
return;
}

$provider = Injector::inst()->get(ProviderFactory::class)->getProvider($providerName);

// Ensure we always have scope to work with
if (empty($scope)) {
$scope = $provider->getDefaultScopes();
return null;
}

/** @var ProviderFactory $providerFactory */
$providerFactory = Injector::inst()->get(ProviderFactory::class);
$provider = $providerFactory->getProvider($providerName);
$url = $provider->getAuthorizationUrl(['scope' => $scope]);

$request->getSession()->set('oauth2', [
Expand All @@ -120,7 +118,8 @@ public function authenticate(HTTPRequest $request)
* The return endpoint after the user has authenticated with a provider
*
* @param HTTPRequest $request
* @return mixed
* @return HTTPResponse
* @throws HTTPResponse_Exception
*/
public function callback(HTTPRequest $request)
{
Expand All @@ -129,11 +128,13 @@ public function callback(HTTPRequest $request)
if (!$this->validateState($request)) {
$session->clear('oauth2');
$this->httpError(400, 'Invalid session state.');
return;
return null;
}

$providerName = $session->get('oauth2.provider');
$provider = Injector::inst()->get(ProviderFactory::class)->getProvider($providerName);
/** @var ProviderFactory $providerFactory */
$providerFactory = Injector::inst()->get(ProviderFactory::class);
$provider = $providerFactory->getProvider($providerName);
$returnUrl = $this->getReturnUrl();

try {
Expand All @@ -158,15 +159,17 @@ public function callback(HTTPRequest $request)
}
}
} catch (IdentityProviderException $e) {
$logger = Injector::inst()->get(Logger::class);
/** @var LoggerInterface $logger */
$logger = Injector::inst()->get(LoggerInterface::class . '.oauth');
$logger->error('OAuth IdentityProviderException: ' . $e->getMessage());
$this->httpError(400, 'Invalid access token.');
return;
return null;
} catch (Exception $e) {
$logger = Injector::inst()->get(Logger::class);
/** @var LoggerInterface $logger */
$logger = Injector::inst()->get(LoggerInterface::class . '.oauth');
$logger->error('OAuth Exception: ' . $e->getMessage());
$this->httpError(400, $e->getMessage());
return;
return null;
} finally {
$session->clear('oauth2');
}
Expand Down
4 changes: 2 additions & 2 deletions src/Factory/ProviderFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@

namespace Bigfork\SilverStripeOAuth\Client\Factory;

use Bigfork\SilverStripeOAuth\Client\Authenticator\Authenticator;
use InvalidArgumentException;
use League\OAuth2\Client\Provider\AbstractProvider;

class ProviderFactory
{
Expand All @@ -29,7 +29,7 @@ public function getProviders()

/**
* @param string $name
* @return League\OAuth2\Client\Provider\AbstractProvider
* @return AbstractProvider
*/
public function getProvider($name)
{
Expand Down

0 comments on commit 700e0fc

Please sign in to comment.