Prevent invite redirect in combination with tfa redirect functionality.

Created on 3 January 2024, 12 months ago
Updated 16 January 2024, 11 months ago

Problem/Motivation

We are using this `disable_user_view` module in combination with the `tfa` module.
With that `tfa` module we use a patch that enforces the TFA activation for each user.
That enforcement is done by a redirect.

Basically that tfa redirect triggers the following code in an event subscriber on KernelEvents::REQUEST

$tfa_overview_url = Url::fromRoute('tfa.overview', ['user' => $this->user->id()]);
$event->setResponse(new RedirectResponse($tfa_overview_url->toString()));

While in disable_user_view there is an access False set in the RouteSubscriber.php
$route->setRequirement('_access', 'FALSE');

That combination of the TFA redirect and the disable_user_view access False results in an infinite loop.

Steps to reproduce

Install disable_user_view
Install tfa with the patch from issue: 3223327

Currently that is: tfa version 1.3
With patch #28: https://www.drupal.org/files/issues/2023-10-10/3223327-implement-force_t...

Enable "Force TFA Setup" in the tfa configuration.
Set "Sip validation" to 0 in the tfa configuration
Enable the roles for testing in tfa configuration

Try to login with a user.

This will result in the infinite loop.

Proposed resolution

As a quick resolution I suggest to remove the acccess False from the RouteSubscriber.php
I will add a patch for that.

As a result `/user/{uid}` will not be redirected to a 403 but will redirect to `/user/{uid}/edit`.
Depending on your usecase that should not be a huge problem. Because `/user/{uid}/edit` is still controlled by it's own access settings. So when you've no access to that page you will also end up on a 403. But this time with the only difference the `/edit` part in the url.

A better solution might be a dynamic approach for the access False in the UserEditController.php.
We might look into that solution as well and provide patch, although it depend on choices and priorities we make in the project.

Remaining tasks

Providing a dynamic solution.

User interface changes

Not really, only the visible user edit url and redirecting to that url when you have access to that user page.

API changes

No.

Data model changes

No.

🐛 Bug report
Status

Active

Version

2.0

Component

Code

Created by

🇳🇱Netherlands Floris Vedder

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

  • Issue created by @Floris Vedder
  • 🇳🇱Netherlands Floris Vedder

    Patch for the simple (quick & dirty) solution.

    Note that it changes the behaviour of the disable_user_view module.

    With this patch `/user/{uid}` will not result in a 403 but will redirect to `/user/{uid}/edit`.
    That should not be a problem for security because `/user/{uid}/edit` is still repsonsible for it's own access check.
    But for some use cases that might not be the solution where you're after.

    As suggested a nicer solution might be possible in UserEditController.php

  • 🇦🇺Australia marc.groth

    +1 for the patch. Without it (and without any other modules) the /user/[uid] page results in a 403 access denied. This means that whenever a user logs in, this is the first page that they see.

    It would be a lot better if this just redirected the user (which is what the patch achieves).

Production build 0.71.5 2024