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

Make /user/register redirect optional #85

Open
wants to merge 15 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
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
5 changes: 3 additions & 2 deletions behat.yml.dist
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,9 @@ default:
user registration: '/user/register'
extensions:
Drupal\MinkExtension:
goutte: ~
selenium2: ~
goutte:
guzzle_parameters:
verify: false
pfrenssen marked this conversation as resolved.
Show resolved Hide resolved
ajax_timeout: 10
browser_name: 'chrome'
javascript_session: 'selenium2'
Expand Down
1 change: 1 addition & 0 deletions config/install/oe_authentication.settings.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,4 @@ register_path: 'eim/external/register.cgi'
validation_path: 'TicketValidationService'
assurance_level: TOP
ticket_types: SERVICE,PROXY
redirect_user_register_route: true
pfrenssen marked this conversation as resolved.
Show resolved Hide resolved
5 changes: 4 additions & 1 deletion config/schema/oe_authentication.schema.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,7 @@ oe_authentication.settings:
label: 'Application assurance levels'
ticket_types:
type: string
label: 'Application available ticket types'
label: 'Application available ticket types'
redirect_user_register_route:
pfrenssen marked this conversation as resolved.
Show resolved Hide resolved
type: boolean
label: 'Redirect Drupal user registration route to EU Login'
8 changes: 7 additions & 1 deletion oe_authentication.services.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,17 @@ services:
tags:
- { name: access_check, applies_to: _external_user_access_check }
oe_authentication.route_subscriber:
class: \Drupal\oe_authentication\Routing\RouteSubscriber
class: Drupal\oe_authentication\Routing\RouteSubscriber
arguments: ['@config.factory']
tags:
- { name: event_subscriber }
oe_authentication.event_subscriber:
class: \Drupal\oe_authentication\Event\EuLoginEventSubscriber
tags:
- { name: event_subscriber }
arguments: ['@config.factory', '@messenger']
oe_authentication.config_subscriber:
class: Drupal\oe_authentication\Event\UserRegisterRouteRedirectConfigSubscriber
arguments: ['@router.builder']
tags:
- { name: event_subscriber }
5 changes: 3 additions & 2 deletions src/Controller/RegisterController.php
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,9 @@ public function register(): TrustedRedirectResponse {
$url = $this->getRegisterUrl()->toString();
$response = new TrustedRedirectResponse($url);

$cache = new CacheableMetadata();
$cache->addCacheContexts(['user.roles:anonymous']);
$cache = (new CacheableMetadata())
->addCacheContexts(['user.roles:anonymous'])
->setCacheTags(['config:oe_authentication.settings']);
pfrenssen marked this conversation as resolved.
Show resolved Hide resolved
$response->addCacheableDependency($cache);

return $response;
Expand Down
57 changes: 57 additions & 0 deletions src/Event/UserRegisterRouteRedirectConfigSubscriber.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
<?php

declare(strict_types = 1);

namespace Drupal\oe_authentication\Event;

use Drupal\Core\Config\ConfigCrudEvent;
use Drupal\Core\Config\ConfigEvents;
use Drupal\Core\Routing\RouteBuilderInterface;
use Symfony\Component\EventDispatcher\EventSubscriberInterface;

/**
* Acts when oe_authentication.settings:redirect_user_register_route changes.
*/
class UserRegisterRouteRedirectConfigSubscriber implements EventSubscriberInterface {

/**
* The route builder service.
*
* @var \Drupal\Core\Routing\RouteBuilderInterface
*/
protected $routeBuilder;

/**
* Constructs a new event subscriber instance.
*
* @param \Drupal\Core\Routing\RouteBuilderInterface $route_builder
* The route builder service.
*/
public function __construct(RouteBuilderInterface $route_builder) {
$this->routeBuilder = $route_builder;
}

/**
* {@inheritdoc}
*/
public static function getSubscribedEvents(): array {
return [
ConfigEvents::SAVE => 'onConfigSave',
];
}

/**
* Acts when oe_authentication.settings:redirect_user_register_route changes.
*
pfrenssen marked this conversation as resolved.
Show resolved Hide resolved
* @param \Drupal\Core\Config\ConfigCrudEvent $event
* The config CRUD event.
*/
public function onConfigSave(ConfigCrudEvent $event): void {
if ($event->getConfig()->getName() === 'oe_authentication.settings') {
if ($event->isChanged('redirect_user_register_route')) {
$this->routeBuilder->rebuild();
}
}
}

}
39 changes: 39 additions & 0 deletions src/Form/AuthenticationSettingsForm.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,11 @@

namespace Drupal\oe_authentication\Form;

use Drupal\Core\Config\ConfigFactoryInterface;
use Drupal\Core\Form\ConfigFormBase;
use Drupal\Core\Form\FormStateInterface;
use Drupal\Core\Routing\RouteBuilderInterface;
use Symfony\Component\DependencyInjection\ContainerInterface;

/**
* Settings form for module.
Expand All @@ -17,6 +20,36 @@ class AuthenticationSettingsForm extends ConfigFormBase {
*/
const CONFIG_NAME = 'oe_authentication.settings';

/**
* The route builder service.
*
* @var \Drupal\Core\Routing\RouteBuilderInterface
*/
protected $routeBuilder;

/**
* Constructs a new form instance.
*
* @param \Drupal\Core\Config\ConfigFactoryInterface $config_factory
* The factory for configuration objects.
* @param \Drupal\Core\Routing\RouteBuilderInterface $route_builder
* The route builder service.
*/
public function __construct(ConfigFactoryInterface $config_factory, RouteBuilderInterface $route_builder) {
parent::__construct($config_factory);
$this->routeBuilder = $route_builder;
}

/**
* {@inheritdoc}
*/
public static function create(ContainerInterface $container) {
return new static(
$container->get('config.factory'),
$container->get('router.builder')
);
}

/**
* {@inheritdoc}
*/
Expand Down Expand Up @@ -53,6 +86,11 @@ public function buildForm(array $form, FormStateInterface $form_state) {
'#title' => $this->t('Application available ticket types'),
'#default_value' => $this->config(static::CONFIG_NAME)->get('ticket_types'),
];
$form['redirect_user_register_route'] = [
'#type' => 'checkbox',
'#title' => $this->t('Redirect user registration route to EU Login'),
'#default_value' => $this->config(static::CONFIG_NAME)->get('redirect_user_register_route'),
];
return parent::buildForm($form, $form_state);
}

Expand All @@ -66,6 +104,7 @@ public function submitForm(array &$form, FormStateInterface $form_state) {
->set('validation_path', $form_state->getValue('validation_path'))
->set('assurance_level', $form_state->getValue('assurance_level'))
->set('ticket_types', $form_state->getValue('ticket_types'))
->set('redirect_user_register_route', $form_state->getValue('redirect_user_register_route'))
->save();
parent::submitForm($form, $form_state);
}
Expand Down
33 changes: 27 additions & 6 deletions src/Routing/RouteSubscriber.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

namespace Drupal\oe_authentication\Routing;

use Drupal\Core\Config\ConfigFactoryInterface;
use Drupal\Core\Routing\RouteSubscriberBase;
use Symfony\Component\Routing\Route;
use Symfony\Component\Routing\RouteCollection;
Expand All @@ -13,6 +14,23 @@
*/
class RouteSubscriber extends RouteSubscriberBase {

/**
* The config factory service.
*
* @var \Drupal\Core\Config\ConfigFactoryInterface
*/
protected $configFactory;

/**
* Constructs a new route event subscriber.
*
* @param \Drupal\Core\Config\ConfigFactoryInterface $config_factory
* The config factory service.
*/
public function __construct(ConfigFactoryInterface $config_factory) {
$this->configFactory = $config_factory;
}

/**
* {@inheritdoc}
*/
Expand All @@ -33,12 +51,15 @@ protected function alterRoutes(RouteCollection $collection): void {
}

// Replace the core register route controller.
$route = $collection->get('user.register');
if ($route instanceof Route) {
$defaults = $route->getDefaults();
unset($defaults['_form']);
$defaults['_controller'] = '\Drupal\oe_authentication\Controller\RegisterController::register';
$route->setDefaults($defaults);
$config = $this->configFactory->get('oe_authentication.settings');
if ($config->get('redirect_user_register_route')) {
pfrenssen marked this conversation as resolved.
Show resolved Hide resolved
$route = $collection->get('user.register');
if ($route instanceof Route) {
pfrenssen marked this conversation as resolved.
Show resolved Hide resolved
$defaults = $route->getDefaults();
unset($defaults['_form']);
$defaults['_controller'] = '\Drupal\oe_authentication\Controller\RegisterController::register';
$route->setDefaults($defaults);
}
}

// Replace the cas callback route controller.
Expand Down
24 changes: 23 additions & 1 deletion tests/Behat/AuthenticationContext.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@

namespace Drupal\Tests\oe_authentication\Behat;

use Drupal\DrupalExtension\Context\ConfigContext;
use Drupal\user\Entity\User;
use Behat\Behat\Hook\Scope\BeforeScenarioScope;
use Drupal\DrupalExtension\Context\RawDrupalContext;
Expand Down Expand Up @@ -105,6 +104,29 @@ public function backupCasConfigs(): void {
}
}

/**
* Act after reverting the config.
*
* Some tests are changing the oe_authentication:redirect_user_register_route
* config value. Changing this value, requires router rebuilding. This is
* covered by UserRegisterRouteRedirectConfigSubscriber event subscriber. The
* subscriber detects if the value of redirect_user_register_route has been
* changed and rebuilds the routes. However, because Behat tests are running
* in a single request, often the static cache is not invalidated, resulting
* in a failure to detect the config value changes, when the configs are
* reverted, in ConfigContext::cleanConfig(). Thus, we're manually rebuilding
* the routes at the end of the scenario for tests that requesting it, by
* using the @RebuildRouter tag.
*
* @see \Drupal\oe_authentication\Event\UserRegisterRouteRedirectConfigSubscriber
* @see \Drupal\DrupalExtension\Context\ConfigContext::cleanConfig()
*
* @AfterScenario @RebuildRouter
*/
public function rebuildRoutes(): void {
\Drupal::service('router.builder')->rebuild();
pfrenssen marked this conversation as resolved.
Show resolved Hide resolved
}

/**
* Navigates to the current user's page.
*
Expand Down
12 changes: 11 additions & 1 deletion tests/features/ecas-register.feature
Original file line number Diff line number Diff line change
@@ -1,12 +1,22 @@
@javascript @ecas-login
@api @ecas-login
Feature: Register through OE Authentication
In order to be able to have new users
As an anonymous user of the system
I need to be able to go to the registration URL

@DrupalLogin @BackupAuthConfigs @RebuildRouter
pfrenssen marked this conversation as resolved.
Show resolved Hide resolved
Scenario: Register
Given I am an anonymous user
When I visit "the user registration page"
# Redirected to the Ecas mockup server.
And I click "External"
Then I should see "Create an account"

Given I am logged in as a user with the "administer authentication configuration" permission
When I am on "the Authentication configuration page"
And I uncheck "Redirect user registration route to EU Login"
Then I press "Save configuration"

Given I am an anonymous user
When I visit "the user registration page"
Then I should see "Create new account"
pfrenssen marked this conversation as resolved.
Show resolved Hide resolved