The session_user token does not respect switched user accounts on runtime

Created on 22 April 2025, 21 days ago

Problem/Motivation

When switching the user multiple times on runtime, for example using the AccountSwitcher service, ECA does not accordingly switch the session user.

The session_user token may be set by the Drupal\eca\EventSubscriber\EcaExecutionSwitchAccountSubscriber in the following way:

public function initializeUser(ConfigFactoryInterface $config_factory, AccountInterface $current_user): void {
    // ...
    if ($user) {
      self::$modelUser = $user;
      self::$sessionUser = $storage->load($current_user->id());
    }
    // ...
  }

https://git.drupalcode.org/project/eca/-/blob/2.1.x/src/EventSubscriber/...

The method initializeUser is called once on service instantiation, which is fine. However, the way the user object as sessionUser property is being set is not quite right, because it's then always referencing to one entity.

Steps to reproduce

Proposed resolution

Instead of using an entity as sessionUser property, use an AccountProxyInterface that would be able to automatically switch to the "real" current user.

Remaining tasks

User interface changes

API changes

Data model changes

🐛 Bug report
Status

Active

Version

3.0

Component

Code

Created by

🇩🇪Germany mxh Offenburg

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

Comments & Activities

  • Issue created by @mxh
  • 🇩🇪Germany jurgenhaas Gottmadingen

    the way the user object as sessionUser property is being set is not quite right, because it's then always referencing to one entity.

    I'm not sure, I do understand the problem. Would it be possible to write a test which demonstrates how this can go wrong?

  • 🇩🇪Germany mxh Offenburg

    This is setting a static property once, using an entity object that won't get exchanged once the current user is being switched:

    self::$sessionUser = $storage->load($current_user->id());
    

    On the other hand, the purpose of an AccountProxyInterface object would automatically use the currently logged in user.

  • 🇩🇪Germany jurgenhaas Gottmadingen

    The sessionUser is the one user that's currently logged in. That should never change, so isn't what you're describing on purpose?

  • 🇩🇪Germany mxh Offenburg

    The "session user" is a representation of the current state of the "current user". And the "current user" can be switched using core's account_switcher service. That may happen, otherwise that service wouldn't exist. I'm using it for a access report page, where I check access for various user accounts and show the results on one page. But that report currently does not produce correct results when ECA is involved in any way.

  • 🇩🇪Germany jurgenhaas Gottmadingen

    I don't think, that's correct. The description of that token reads like this:

    The user account that dispatched the event, regardless if ECA is processing models under a different account. This is only available if ECA is configured to always run under a specific account.

    So, that token holds the user at the point when ECA starts processing, before being switched to the user defined in the settings to always process under a different user account.

  • 🇩🇪Germany mxh Offenburg

    My explanations are correct, especially as analyzed in #3: It's a static property, being initialized once for the whole runtime. When switching the current user with account_switcher service, an AccountProxyInterface object automatically refers to the correct current user.

  • 🇩🇪Germany jurgenhaas Gottmadingen

    My explanations are correct, especially as analyzed in #3: It's a static property, being initialized once for the whole runtime.

    Yes, that's how it should be. The session user never changes. At least, that's how I understand the purpose of it. No matter how often the user gets switched by ECA, the session user has to stay what the runtime session has been at the very beginning.

    I probably find myself in a deep dark tunnel? I'm not seeing the issue, sorry.

  • 🇩🇪Germany mxh Offenburg

    I wouldn't have created an issue if I wouldn't have a problem. I took the time to analyze this, give an explanation and re-reading my explanations I think they makes it obvious to make sense what the problem is about.

    Also, the very first ECA execution on runtime may happen within a place where a component may have switched to a different current user on purpose. Creating an overview for access reports is a good example for that where you may check access for a privileged user and another time for a non-privileged user.

  • 🇩🇪Germany mxh Offenburg

    Maybe the following code part makes it more obvious: https://git.drupalcode.org/project/eca/-/blob/2.1.x/src/EventSubscriber/...

    public function initializeUser(ConfigFactoryInterface $config_factory, AccountInterface $current_user): void {
        if (self::$initialized) {
          // Already initialized.
          return;
        }
        self::$initialized = TRUE;
    
    // ...
    }
    

    So this logic is applied only once for the whole runtime. Given my possible case in #9 this may even be dangerous as the current user at the very first execution may be a privileged user and this would remain the same one on all subsequent processes on the same runtime.

  • 🇩🇪Germany jurgenhaas Gottmadingen

    I trust you, of course, I would never expect you to open an issue for no good reason.

    But I still don't understand the problem. The session_user token was created to only be available, if a global user is defined in eca.settings; otherwise that token will not be defined. At it gets initialized by the event subscriber, I don't see how anything else could switch to a different user before this gets initialized with that user of the current PHP process.

    I understand what you're describing what you want to achieve, but that's most likely the problem with that global user to run ECA processes, that it just doesn't allow any sort of proper permission checks. We've discussed this recently at various places, and that's why we recommend to no longer use this global user.

    If you still think I'm not getting the point, then please provide a model that demonstrates where this is going wrong.

    It's not about the user switch being done by ECA, it's about anything else that may switch the user on purpose.

    Also, the very first ECA execution on runtime may happen within a place where a component may have switched to a different current user on purpose. Creating an overview for access reports is a good example for that where you may check access for a privileged user and another time for a non-privileged user.

    What "else" would switch the user account on purpose?

  • 🇩🇪Germany mxh Offenburg

    What "else" would switch the user account on purpose?

    Access Policy API exactly does this within Drupal\Core\Session\AccessPolicyProcessor::processAccessPolicies:
    https://git.drupalcode.org/project/drupal/-/blob/11.x/core/lib/Drupal/Co...

  • 🇩🇪Germany jurgenhaas Gottmadingen

    OK, that's a new thing. And how would be ever get a chance to get to the original user account in that context? Because that's what the documented purpose of the session_user token is: knowing the original session user. Does Access Policy API expose that?

  • 🇩🇪Germany jurgenhaas Gottmadingen

    I just realized, there is an AccountEvents::SET_USER which is dispatched when the current user is being set. We may want to subscribe to that event and safely grab the session user from there, which should be happening very early before anything else like Access Policy API can alter anything.

  • 🇩🇪Germany jurgenhaas Gottmadingen

    I've tested the AccountEvents::SET_USER event, and it gets dispatches early in the process. This may be more reliable to set the static variable \Drupal\eca\EventSubscriber\EcaExecutionSwitchAccountSubscriber::$sessionUser to the real authenticated user, which is what this variable was designed to be there for.

    @mxh would you please test that in your scenario together with the Access Policy API to see if you then get the correct session user?

  • 🇩🇪Germany mxh Offenburg

    This issue is clearly a bug, I'm wondering why it got changed to a feature request.

    Because that's what the documented purpose of the session_user token is: knowing the original session user. Does Access Policy API expose that?

    This may be more reliable to set the static variable \Drupal\eca\EventSubscriber\EcaExecutionSwitchAccountSubscriber::$sessionUser to the real authenticated user, which is what this variable was designed to be there for.

    The only thing ECA can ever know, is the currently set user before ECA itself is executing anything. Which includes switching the user inside of ECA process execution. There is no safe way to always guarantee that we have the "real session user" at that point.

    For continuation I try to summarize this issue so far:

    The current behavior of ECA is:
    - When ECA is being executed the very first time, it sets the current user's entity object as session_user token. On every ECA execution at the same runtime later on, the same instance of that initially determined session_user is being used.
    This behavior is wrong as described in #9.

    To resolve this, my previous assumption to just use the AccountProxy object was wrong. Because the AccountProxy may then of course hold the "current user" set by ECA, which is not what we want to be stored there.

    I think the correct behavior would be:
    - When ECA is being executed on a root-level process (= execution is not a nested subprocess), it sets the current user's entity object as session_user token. When the last ECA process on a root level finished execution, the session_user token may get removed from the token variables environment to avoid further confusion.

  • 🇩🇪Germany jurgenhaas Gottmadingen

    The only thing ECA can ever know, is the currently set user before ECA itself is executing anything. Which includes switching the user inside of ECA process execution. There is no safe way to always guarantee that we have the "real session user" at that point.

    The \Drupal\eca\EventSubscriber\EcaExecutionSwitchAccountSubscriber::initializeUser method is called when the event subscribers get registered. So, that's not bound to the switch user action. Or am I wrong in that understanding?

    Regardless, AccountEvents::SET_USER seems the better approach, as it will register the session user right when the current user session is being set by core. And that's what documentation tells, what this token is supposed to provide. Or do you disagree with that statement?

  • 🇩🇪Germany mxh Offenburg

    Regardless, AccountEvents::SET_USER seems the better approach, as it will register the session user right when the current user session is being set by core.

    Whichever component in the system comes first, would "win". We have no guaranteed way that this is always the very first thing happening when setting the current user.

    And that's what documentation tells, what this token is supposed to provide. Or do you disagree with that statement?

    I don't necessarily disagree with it. I'd also like to have the "real session user" all the time if we could get it in a guaranteed manner. But we simply don't have it as described above.

    Any other component could decide else what is supposed to be the "session user". Example: The Masquerade module

    I think ECA should do what's possible within its scope, and I think the described way in #16 is still the "most" safe one I can currently think of. To recap it's the following way:

    When ECA is being executed on a root-level process (= execution is not a nested subprocess), it sets the current user's entity object as session_user token. When the last ECA process on a root level finished execution, the session_user token may get removed from the token variables environment to avoid further confusion.

    But I'd be happy to discover better available options.

  • 🇩🇪Germany jurgenhaas Gottmadingen

    Whichever component in the system comes first, would "win". We have no guaranteed way that this is always the very first thing happening when setting the current user.

    OK, we have to analyse this. As the current_user service is being used almost everywhere, it must be set very early in the process by Drupal core. Here is where this happens \Drupal\Core\EventSubscriber\AuthenticationSubscriber::getSubscribedEvents:

        // The priority for authentication must be higher than the highest event
        // subscriber accessing the current user. Especially it must be higher than
        // LanguageRequestSubscriber as LanguageManager accesses the current user if
        // the language module is enabled.
        $events[KernelEvents::REQUEST][] = ['onKernelRequestAuthenticate', 300];
    

    Now, do we know anything that beats that and comes sooner?

Production build 0.71.5 2024