Problem/Motivation

When masquerading, I receive the following error using version 4.0.0 of the module and Drupal 10.4.2 (note: I first noticed this while we were on Drupal 10.3.x, and I'm just now having time to address it).

Error: Call to undefined method Drupal\user\Entity\User::getAccount() in event_log_track_auth_user_logout() (line 93 of /var/www/html/docroot/modules/contrib/events_log_track/modules/event_log_track_auth/event_log_track_auth.module).

Backtrace:

#0 [internal function]: event_log_track_auth_user_logout()
#1 /var/www/html/docroot/core/lib/Drupal/Core/Extension/ModuleHandler.php(416): call_user_func_array()
#2 /var/www/html/docroot/core/lib/Drupal/Core/Extension/ModuleHandler.php(395): Drupal\Core\Extension\ModuleHandler->Drupal\Core\Extension\{closure}()
#3 /var/www/html/docroot/core/lib/Drupal/Core/Extension/ModuleHandler.php(415): Drupal\Core\Extension\ModuleHandler->invokeAllWith()
#4 /var/www/html/docroot/modules/contrib/masquerade/src/Masquerade.php(114): Drupal\Core\Extension\ModuleHandler->invokeAll()
#5 /var/www/html/docroot/modules/contrib/masquerade/src/Masquerade.php(170): Drupal\masquerade\Masquerade->switchUser()
#6 /var/www/html/docroot/modules/contrib/masquerade/src/Controller/SwitchController.php(82): Drupal\masquerade\Masquerade->switchTo()
#7 [internal function]: Drupal\masquerade\Controller\SwitchController->switchTo()
#8 /var/www/html/docroot/core/lib/Drupal/Core/EventSubscriber/EarlyRenderingControllerWrapperSubscriber.php(123): call_user_func_array()
#9 /var/www/html/docroot/core/lib/Drupal/Core/Render/Renderer.php(638): Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}()
#10 /var/www/html/docroot/core/lib/Drupal/Core/EventSubscriber/EarlyRenderingControllerWrapperSubscriber.php(121): Drupal\Core\Render\Renderer->executeInRenderContext()
#11 /var/www/html/docroot/core/lib/Drupal/Core/EventSubscriber/EarlyRenderingControllerWrapperSubscriber.php(97): Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->wrapControllerExecutionInRenderContext()
#12 /var/www/html/vendor/symfony/http-kernel/HttpKernel.php(181): Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}()
#13 /var/www/html/vendor/symfony/http-kernel/HttpKernel.php(76): Symfony\Component\HttpKernel\HttpKernel->handleRaw()
#14 /var/www/html/docroot/modules/contrib/redirect_after_login/src/RedirectMiddleware.php(44): Symfony\Component\HttpKernel\HttpKernel->handle()
#15 /var/www/html/docroot/core/lib/Drupal/Core/StackMiddleware/Session.php(53): Drupal\redirect_after_login\RedirectMiddleware->handle()
#16 /var/www/html/docroot/core/lib/Drupal/Core/StackMiddleware/KernelPreHandle.php(48): Drupal\Core\StackMiddleware\Session->handle()
#17 /var/www/html/docroot/core/lib/Drupal/Core/StackMiddleware/ContentLength.php(28): Drupal\Core\StackMiddleware\KernelPreHandle->handle()
#18 /var/www/html/docroot/core/modules/big_pipe/src/StackMiddleware/ContentLength.php(32): Drupal\Core\StackMiddleware\ContentLength->handle()
#19 /var/www/html/vendor/asm89/stack-cors/src/Cors.php(53): Drupal\big_pipe\StackMiddleware\ContentLength->handle()
#20 /var/www/html/docroot/core/lib/Drupal/Core/StackMiddleware/ReverseProxyMiddleware.php(48): Asm89\Stack\Cors->handle()
#21 /var/www/html/docroot/core/lib/Drupal/Core/StackMiddleware/NegotiationMiddleware.php(51): Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle()
#22 /var/www/html/docroot/core/lib/Drupal/Core/StackMiddleware/AjaxPageState.php(36): Drupal\Core\StackMiddleware\NegotiationMiddleware->handle()
#23 /var/www/html/docroot/core/lib/Drupal/Core/StackMiddleware/StackedHttpKernel.php(51): Drupal\Core\StackMiddleware\AjaxPageState->handle()
#24 /var/www/html/docroot/core/lib/Drupal/Core/DrupalKernel.php(741): Drupal\Core\StackMiddleware\StackedHttpKernel->handle()
#25 /var/www/html/docroot/index.php(19): Drupal\Core\DrupalKernel->handle()
#26 {main}

Steps to reproduce

  1. Install the latest recommended version of the Events Log Track, Events Log Track User Authentication, and Masquerade modules
  2. Go to /admin/people
  3. Click Masquerade to start masquerading as another user. The error will occur here.

Proposed resolution

On line 93 of event_log_track_auth.module change:

'%user' => $account->getAccount()->name,

to:

'%user' => $account->getAccountName(),

Issue fork events_log_track-3506232

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

jsutta created an issue. See original summary.

jsutta’s picture

jsutta commented

Added the change to the issue's fork.

jsutta’s picture

jsutta commented
Status: Active » Needs review

jsutta opened merge request !72

mingsong’s picture

mingsong
Location 🇦🇺
commented
Status: Needs review » Postponed (maintainer needs more info)

Will this error occur if the Masquerade module not installed?

luke.stewart’s picture

luke.stewart commented

I suspect this has something to do with what is passed to the the hook user_logout.

The API spec defines it as receiving an AccountInterface
https://api.drupal.org/api/drupal/core%21modules%21user%21user.api.php/f...
Which does not specify a method getAccount
https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Session%2...

However my guess is that without masquerade in play the object being passed in has that method. So this is perhaps how this has slipped through.

jsutta’s picture

jsutta commented

Hi all,

Sorry for the delay! I just had a chance to check and found that if I uninstall the masquerade module this issue doesn't occur when I log out (and the logout event is logged as expected).

Does that mean this is actually an issue with the masquerade module?

jsutta’s picture

jsutta commented
Status: Postponed (maintainer needs more info) » Needs review
mingsong’s picture

mingsong
Location 🇦🇺
commented

Does that mean this is actually an issue with the masquerade module?

I haven't had a chance to look closely. I can't complain other modules without knowing what exactly happened.

But it is difficult for us to reproduce this issue with our module.

Ideally, we would like to have a PHPUnit test to expose an issue when fixing it. That saying, we can't fix an issue which requires another module to trigger.

jsutta’s picture

jsutta commented

Just made a slight change... I added a new $user variable whose value is set based on whether or not the getAccountName method is available. If it's available, it's set to $account->getAccountName(), and if not it's set using the current method, $account->getAccount()->name.

The change still addresses an issue that's only there because of the masquerade module, but could be a good compromise.

mingsong’s picture

mingsong
Location 🇦🇺
commented

Thanks @Joe Sutta for working on this issue.

This issue actually brings in a very interesting question. What user name should be logged? The admin user's name or the name imitated?

In my opinion, the real (admin) user name should be logged.

mingsong’s picture

mingsong
Location 🇦🇺
commented
Status: Needs review » Needs work

Change to 'Need work' due to the outstanding question above.

jsutta’s picture

jsutta commented

I agree with that, since they're the one taking the action.

jsutta’s picture

jsutta commented
Status: Needs work » Needs review
muriqui’s picture

muriqui commented

Since the error message complains about the getAccount() function, I suggest checking if that function exists instead. For example,

$user_name = (method_exists($account, 'getAccount')) ? $account->getAccount()->name : $account->getAccountName();

The method_exists checks aren't really necessary. When you log out normally, the object that gets passed to hook_user_logout() is an AccountProxy, which is a wrapper around a User entity. The AccountProxyInterface is what defines the getAccount() method used by the current code, which returns the wrapped User. However, AccountProxyInterface also extends AccountInterface—which, as noted above, is all that's guaranteed by hook_user_logout()—and as such, an AccountProxy object also has the getAccountName method, which is implemented like so:

public function getAccountName() {
    return $this->getAccount()
        ->getAccountName();
}

And that getAccountName() method on the User is simply this:

public function getAccountName() {
    return $this->get('name')->value ?: '';
}

Thus, calling $account->getAccountName() on an AccountProxy does the same thing as calling $account->getAccount()->name, the only difference being that getAccountName is guaranteed and getAccount is not.

As for the error thrown when using Masquerade, that's because masquerading triggers a logout event for the admin before masquerading as the user. When it does so, the object that comes into hook_user_logout() isn't an AccountProxy, it's just a normal User entity for the admin, and thus it will fail if you try to call getAccount, but succeeds with getAccountName. (This also answers the question about which user will be logged: it's the admin.)

So the cleanest solution would be to revert the MR to the original commit f0f48bdab0cac3b9cbc54e07640637a54f8995a0 that just changed $account->getAccount()->name to $account->getAccountName().

smustgrave closed merge request !72

  • smustgrave committed 5636fc2c on 4.0.x 
    Issue #3506232 by jsutta, muriqui, mingsong: Error: Call to undefined...
smustgrave’s picture

smustgrave commented
Status: Needs review » Fixed

Change makes sense to me

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.