Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add OAUTH_RESOURCE_MAPPING_REFRESH event, add members.firewall_name parameter #197

Merged
merged 3 commits into from
Feb 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/codeception.yml
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ jobs:
run: |
nohup $CHROMEWEBDRIVER/chromedriver --url-base=/wd/hub /dev/null 2>&1 &

- name: Start Webserver and Chrome
- name: Start Symfony Server
run: |
curl -sS https://get.symfony.com/cli/installer | bash -s -- --install-dir=$HOME/.symfony/bin
~/.symfony/bin/symfony server:start --port=8080 --dir=public --allow-http --no-tls --daemon
Expand Down
4 changes: 4 additions & 0 deletions UPGRADE.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
# Upgrade Notes

### 5.0.1
- **[IMPROVEMENT]**: Introduce `OAUTH_RESOURCE_MAPPING_REFRESH` Event
- **[IMPROVEMENT]**: Configurable Firewall Name via container parameter `members.firewall_name`

## Migrating from Version 4.x to Version 5.0

### Global Changes
Expand Down
4 changes: 4 additions & 0 deletions config/packages/security_auth_manager.yaml
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
# if you're using a different firewall name, you need to enable this parameter
# parameters:
# members.firewall_name: 'your_fw_name'

security:

# symfony default is set to "true".
Expand Down
2 changes: 2 additions & 0 deletions config/services/event.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ services:

# event: check auth
MembersBundle\EventListener\AuthenticationListener:
arguments:
$firewallName: '%members.firewall_name%'
tags:
- { name: kernel.event_subscriber }

Expand Down
13 changes: 11 additions & 2 deletions docs/40_Events.md
Original file line number Diff line number Diff line change
Expand Up @@ -306,7 +306,7 @@
| Type | Reference |
|:--- |:--- |
| **const** | `\MembersEvent:OAUTH_RESOURCE_MAPPING_PROFILE` |
| **name** | `members.oauth.connection.success` |
| **name** | `members.oauth.resource_mapping.profile` |
| **class** | `\MembersBundle\Event\OAuth\OAuthResourceEvent` |
| **description** | The OAUTH_RESOURCE_MAPPING_PROFILE event occurs before a sso identity gets assigned to given user profile. This event allows you to map resource data (e.g. google) to your user identity. |

Expand All @@ -315,10 +315,19 @@
| Type | Reference |
|:--- |:--- |
| **const** | `\MembersEvent:OAUTH_RESOURCE_MAPPING_REGISTRATION` |
| **name** | `members.oauth.connection.success` |
| **name** | `members.oauth.resource_mapping.registration` |
| **class** | `\MembersBundle\Event\OAuth\OAuthResourceEvent` |
| **description** | The OAUTH_RESOURCE_MAPPING_REGISTRATION event occurs before the registration form gets rendered. This event allows you to map resource data (e.g. google) to your registration form. |

### members.oauth.resource_mapping.refresh

| Type | Reference |
|:--- |:-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
| **const** | `\MembersEvent:OAUTH_RESOURCE_MAPPING_REFRESH` |
| **name** | `members.oauth.resource_mapping.refresh` |
| **class** | `\MembersBundle\Event\OAuth\OAuthResourceRefreshEvent` |
| **description** | TThe OAUTH_RESOURCE_MAPPING_REFRESH event occurs after an existing sso identity has been found. This event allows you to map resource data (e.g. google) to your existing user identity. |

***

### members.oauth.identity_status.profile_completion
Expand Down
15 changes: 14 additions & 1 deletion docs/SSO/12_ResourceMapping.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,8 @@ class MembersResourceMappingListener implements EventSubscriberInterface
{
return [
MembersEvents::OAUTH_RESOURCE_MAPPING_PROFILE => 'onProfileMapping',
MembersEvents::OAUTH_RESOURCE_MAPPING_REGISTRATION => 'onRegistrationMapping'
MembersEvents::OAUTH_RESOURCE_MAPPING_REGISTRATION => 'onRegistrationMapping',
MembersEvents::OAUTH_RESOURCE_MAPPING_REFRESH => 'onRegistrationRefresh',
];
}

Expand All @@ -62,6 +63,18 @@ class MembersResourceMappingListener implements EventSubscriberInterface
$this->mapData($user, $ownerDetails);
}

public function onRegistrationRefresh(OAuthResourceRefreshEvent $event): void
{
$user = $event->getUser();
$resourceOwner = $event->getResourceOwner();
$ownerDetails = $resourceOwner->toArray();

$user->setUserName($ownerDetails['name']);

// ATTENTION! You need to inform event about changes!
$event->setHasChanged(true);
}

protected function mapData(UserInterface $user, array $ownerDetails): void
{
if (empty($user->getEmail()) && isset($ownerDetails['email'])) {
Expand Down
8 changes: 8 additions & 0 deletions docs/SSO/20_Installation.md
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,14 @@ members:
activation_type: 'complete_profile' # choose between "complete_profile" and "instant"
```

## Configure Firewall
If your using a different name for your firewall than `members_fe` you need to configure the container parameter:

```yaml
parameters:
members.firewall_name: your_fw_name
```

## Configure Client
Every provider comes with its own configuration.
In this example, we're going to setup the google client:
Expand Down
9 changes: 8 additions & 1 deletion src/DependencyInjection/MembersExtension.php
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,11 @@ public function prepend(ContainerBuilder $container): void
$configs = $container->getExtensionConfig($this->getAlias());
$config = $this->processConfiguration($this->getConfiguration([], $container), $configs);

/** @phpstan-ignore-next-line */
if (!$container->hasParameter('members.firewall_name')) {
$container->setParameter('members.firewall_name', 'members_fe');
}

$oauthEnabled = false;
if ($container->hasExtension('security') === true && $config['oauth']['enabled'] === true) {
$oauthEnabled = true;
Expand Down Expand Up @@ -114,6 +119,8 @@ protected function enableOauth(ContainerBuilder $container, array $config): void

protected function extendPimcoreSecurityConfiguration(ContainerBuilder $container, bool $oauthEnabled): void
{
$firewallName = $container->getParameter('members.firewall_name');

$container->loadFromExtension('pimcore', [
'security' => [
'password_hasher_factories' => [
Expand All @@ -125,7 +132,7 @@ protected function extendPimcoreSecurityConfiguration(ContainerBuilder $containe
if ($oauthEnabled === true) {
$container->loadFromExtension('security', [
'firewalls' => [
'members_fe' => [
$firewallName => [
'custom_authenticators' => [
OAuthIdentityAuthenticator::class
]
Expand Down
18 changes: 18 additions & 0 deletions src/Event/OAuth/OAuthResourceRefreshEvent.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
<?php

namespace MembersBundle\Event\OAuth;

class OAuthResourceRefreshEvent extends OAuthResourceEvent
{
protected bool $hasChanged = false;

public function hasChanged(): bool
{
return $this->hasChanged === true;
}

public function setHasChanged(bool $hasChanged): void
{
$this->hasChanged = $hasChanged;
}
}
7 changes: 7 additions & 0 deletions src/Exception/EntityNotRefreshedException.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<?php

namespace MembersBundle\Exception;

final class EntityNotRefreshedException extends \Exception
{
}
9 changes: 9 additions & 0 deletions src/MembersEvents.php
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,15 @@ final class MembersEvents
*/
public const OAUTH_RESOURCE_MAPPING_REGISTRATION = 'members.oauth.resource_mapping.registration';

/**
* The OAUTH_RESOURCE_MAPPING_REFRESH event occurs after an existing sso identity has been found.
*
* This event allows you to map resource data (e.g. google) to your existing user identity.
*
* @Event("MembersBundle\Event\OAuth\OAuthResourceRefreshEvent")
*/
public const OAUTH_RESOURCE_MAPPING_REFRESH = 'members.oauth.resource_mapping.refresh';

/**
* The OAUTH_IDENTITY_STATUS_PROFILE_COMPLETION event occurs before a user enters the profile completion step.
*
Expand Down
7 changes: 4 additions & 3 deletions src/Security/Authenticator/OAuthIdentityAuthenticator.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@
use KnpU\OAuth2ClientBundle\Client\OAuth2ClientInterface;
use KnpU\OAuth2ClientBundle\Security\Authenticator\OAuth2Authenticator;
use MembersBundle\Adapter\User\UserInterface;
use MembersBundle\Manager\SsoIdentityManagerInterface;
use MembersBundle\Security\OAuth\Dispatcher\Router\DispatchRouter;
use MembersBundle\Security\OAuth\Exception\AccountNotLinkedException;
use MembersBundle\Security\OAuth\OAuthRegistrationHandler;
use MembersBundle\Security\OAuth\OAuthResponse;
use Symfony\Component\HttpFoundation\RedirectResponse;
use Symfony\Component\HttpFoundation\Request;
Expand All @@ -29,7 +29,7 @@ class OAuthIdentityAuthenticator extends OAuth2Authenticator implements Authenti
public function __construct(
protected UrlGeneratorInterface $router,
protected ClientRegistry $clientRegistry,
protected SsoIdentityManagerInterface $ssoIdentityManager,
protected OAuthRegistrationHandler $oAuthRegistrationHandler,
protected DispatchRouter $dispatchRouter
) {
}
Expand Down Expand Up @@ -67,9 +67,10 @@ public function authenticate(Request $request): Passport

return new SelfValidatingPassport(
new UserBadge($accessToken->getToken(), function () use ($accessToken, $client, $provider, $type, $parameter) {

$user = $client->fetchUserFromToken($accessToken);
$oAuthResponse = new OAuthResponse($provider, $accessToken, $user, $parameter);
$memberUser = $this->ssoIdentityManager->getUserBySsoIdentity($oAuthResponse->getProvider(), $oAuthResponse->getResourceOwner()->getId());
$memberUser = $this->oAuthRegistrationHandler->getRefreshedUserFromUserResponse($oAuthResponse);

if ($memberUser instanceof UserInterface) {
return $memberUser;
Expand Down
13 changes: 13 additions & 0 deletions src/Security/OAuth/AccountConnector.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

use MembersBundle\Adapter\Sso\SsoIdentityInterface;
use MembersBundle\Adapter\User\UserInterface as MembersUserInterface;
use MembersBundle\Exception\EntityNotRefreshedException;
use MembersBundle\Manager\SsoIdentityManagerInterface;
use MembersBundle\Service\ResourceMappingService;
use Symfony\Component\Security\Core\User\UserInterface;
Expand Down Expand Up @@ -55,6 +56,18 @@ public function connectToSsoIdentity(UserInterface $user, OAuthResponseInterface
return $ssoIdentity;
}

/**
* @throws EntityNotRefreshedException
*/
public function refreshSsoIdentityUser(UserInterface $user, OAuthResponseInterface $oAuthResponse): void
{
if (!$user instanceof MembersUserInterface) {
throw new \InvalidArgumentException('User is not supported');
}

$this->resourceMappingService->mapResourceData($user, $oAuthResponse->getResourceOwner(), ResourceMappingService::MAP_FOR_REFRESH);
}

protected function applyCredentialsToSsoIdentity(SsoIdentityInterface $ssoIdentity, OAuthResponseInterface $oAuthResponse): void
{
$token = $oAuthResponse->getAccessToken();
Expand Down
2 changes: 2 additions & 0 deletions src/Security/OAuth/AccountConnectorInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,6 @@
interface AccountConnectorInterface
{
public function connectToSsoIdentity(UserInterface $user, OAuthResponseInterface $oAuthResponse): SsoIdentityInterface;

public function refreshSsoIdentityUser(UserInterface $user, OAuthResponseInterface $oAuthResponse): void;
}
21 changes: 21 additions & 0 deletions src/Security/OAuth/OAuthRegistrationHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

use MembersBundle\Adapter\Sso\SsoIdentityInterface;
use MembersBundle\Adapter\User\UserInterface;
use MembersBundle\Exception\EntityNotRefreshedException;
use MembersBundle\Manager\SsoIdentityManagerInterface;
use MembersBundle\Manager\UserManagerInterface;
use MembersBundle\Service\RequestPropertiesForUserExtractorServiceInterface;
Expand All @@ -24,6 +25,26 @@ public function getUserFromUserResponse(OAuthResponseInterface $OAuthResponse):
return $this->ssoIdentityManager->getUserBySsoIdentity($OAuthResponse->getProvider(), $OAuthResponse->getResourceOwner()->getId());
}

public function getRefreshedUserFromUserResponse(OAuthResponseInterface $oAuthResponse): ?UserInterface
{
$user = $this->ssoIdentityManager->getUserBySsoIdentity($oAuthResponse->getProvider(), $oAuthResponse->getResourceOwner()->getId());

if (!$user instanceof UserInterface) {
return null;
}

try {
$this->accountConnector->refreshSsoIdentityUser($user, $oAuthResponse);
} catch (EntityNotRefreshedException $e) {
// entity hasn't changed. return
return $user;
}

$this->userManager->updateUser($user);

return $user;
}

/**
* @throws \Exception
*/
Expand Down
21 changes: 19 additions & 2 deletions src/Service/ResourceMappingService.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,20 @@

namespace MembersBundle\Service;

use League\OAuth2\Client\Provider\ResourceOwnerInterface;
use MembersBundle\Adapter\User\UserInterface;
use MembersBundle\Event\OAuth\OAuthResourceEvent;
use MembersBundle\Event\OAuth\OAuthResourceRefreshEvent;
use MembersBundle\Exception\EntityNotRefreshedException;
use MembersBundle\MembersEvents;
use League\OAuth2\Client\Provider\ResourceOwnerInterface;
use Symfony\Contracts\EventDispatcher\EventDispatcherInterface;
use Symfony\Component\EventDispatcher\EventDispatcherInterface as ComponentEventDispatcherInterface;

class ResourceMappingService
{
public const MAP_FOR_PROFILE = 'profile';
public const MAP_FOR_REGISTRATION = 'registration';
public const MAP_FOR_REFRESH = 'refresh';

public function __construct(
protected string $authIdentifier,
Expand All @@ -22,6 +25,7 @@ public function __construct(

/**
* @throws \Exception
* @throws EntityNotRefreshedException
*/
public function mapResourceData(UserInterface $user, ResourceOwnerInterface $resourceOwner, string $type): void
{
Expand All @@ -39,11 +43,24 @@ public function mapResourceData(UserInterface $user, ResourceOwnerInterface $res
return;
}

$this->eventDispatcher->dispatch(new OAuthResourceEvent($user, $resourceOwner), $eventName);
$eventClass = $type === self::MAP_FOR_REFRESH ? OAuthResourceRefreshEvent::class : OAuthResourceEvent::class;
$event = new $eventClass($user, $resourceOwner);

$this->eventDispatcher->dispatch($event, $eventName);

if ($event instanceof OAuthResourceRefreshEvent && $event->hasChanged() === false) {
throw new EntityNotRefreshedException(sprintf('entity %d has not changed', $user->getId()));
}

}

public function addDefaults(UserInterface $user, ResourceOwnerInterface $resourceOwner, string $type): void
{
// do not add default values in refresh mode
if ($type === self::MAP_FOR_REFRESH) {
return;
}

$ownerDetails = $resourceOwner->toArray();
$disallowedProperties = ['lastLogin', 'password', 'confirmationToken', 'passwordRequestedAt', 'groups', 'ssoIdentities'];

Expand Down
4 changes: 2 additions & 2 deletions tests/_envs/github.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ modules:
wait: 1
window_size: 1280x1024
capabilities:
chromeOptions:
args: ['--headless', '--disable-gpu']
'goog:chromeOptions':
args: ['--no-sandbox', '--disable-extensions', '--headless', '--disable-gpu', '--disable-dev-shm-usage', '--window-size=1280,1024']
prefs:
download.default_directory: '%TEST_BUNDLE_TEST_DIR%/_data/downloads'
1 change: 1 addition & 0 deletions tests/_etc/config/app/config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ pimcore_admin:
framework:

session:
gc_probability: 0
storage_factory_id: session.storage.factory.native

profiler:
Expand Down
2 changes: 1 addition & 1 deletion tests/_etc/config/app/system_settings.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ pimcore:
steps: 10
email:
debug:
email_addresses: shagspiel@dachcom.ch
email_addresses: development@dachcom.ch
pimcore_admin:
assets:
hide_edit_image: true
Expand Down
Loading