Problem/Motivation

The function getUserTimeout assumes that the uid passed to it is a valid id.
There are somehow scenarios where this is not the case or the user has been removed.

Causing the following error:
Error: Call to a member function id() on null in Drupal\autologout\AutologoutManager->getUserTimeout()

CommentFileSizeAuthor
#10 autologout-checkifuserexists-3209127-10.patch413 byteskerasai
#6 autologout-checkifuserexists-3209127-6.patch2.16 KBbrentg

Issue fork autologout-3209127

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

brentgees created an issue. See original summary.

brentgees opened merge request !2

brentg’s picture

brentg
Primary language Dutch
Location Ghent
commented
Version: master » 8.x-1.x-dev

brentgees closed merge request !2

brentgees opened merge request !3

brentg’s picture

brentg
Primary language Dutch
Location Ghent
commented
StatusFileSize
new autologout-checkifuserexists-3209127-6.patch2.16 KB

Created a patch that works on older versions as well. (8.x-1.0)

brentg’s picture

brentg
Primary language Dutch
Location Ghent
commented
Assigned: brentg » Unassigned
brentg’s picture

brentg
Primary language Dutch
Location Ghent
commented
Status: Active » Needs review
gombi’s picture

gombi commented
Status: Needs review » Postponed (maintainer needs more info)

Hello brentg,

I'm unable to reproduce this issue by deleting a user, as the autologout.ajax_set_last route returns 403 - Forbidden.

Could you add the steps to reproduce?

kerasai’s picture

kerasai commented
Version: 8.x-1.x-dev » 2.x-dev
Status: Postponed (maintainer needs more info) » Needs review
StatusFileSize
new autologout-checkifuserexists-3209127-10.patch413 bytes

I'm running into this with an automated test suite that rapidly creates and removes users.

An early return to treat the missing user as an anonymous user solves this.

Patch attached for 2.x branch.

the_g_bomb’s picture

the_g_bomb commented

Left a comment on the MR, then spotted the patch @10, which is a similar approach to my suggested addition in the MR.

the_g_bomb’s picture

the_g_bomb commented
Status: Needs review » Needs work

This will need a rebase

deaom made their first commit to this issue’s fork.

deaom opened merge request !94

deaom’s picture

deaom commented
Status: Needs work » Needs review

Applied the patch to the 2.x branch and also updated gitlab-ci file to exclude the readme file for cspell. There are some eslint and phpstan error, but I think there is already an issue open to address those. Ready for review.

benstallings’s picture

benstallings commented
Status: Needs review » Reviewed & tested by the community

Claude Code says:

The branch is clean and focused. The core fix (null user guard) is correct and minimal. The ?? → ?: / direct-access changes in autologout.module are appropriate for the 2.x (Drupal 10+) target. No concerns — this looks ready to merge.

the_g_bomb’s picture

the_g_bomb commented

I think the cspell improvements are out of scope for this ticket. There will be a follow up ticket to re-address cspell, phpcs and phpstan etc

the_g_bomb’s picture

the_g_bomb commented

I think the same application can be applied to getUserRedirectUrl().

I suspect we can also do with a regression test to strengthen this. I'll add it to the ticket to address tests.

the_g_bomb’s picture

the_g_bomb commented
Status: Reviewed & tested by the community » Fixed

Merged, thanks all

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

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