Warning: Undefined array key 1 in Drupal\autologout\EventSubscriber\AutologoutSubscriber->onRequest()

Here's a PR that resolves the warning message. There seems to be a new concept of "destinations[]" now and I've not handled this for now.

I tried the patch in a Drupal 9.5.10 site with autologout 8.1.4 and php 8.1.21.
The warnings are gone.

This seems to be a duplicate of https://www.drupal.org/project/autologout/issues/3301642 🐛 Throws Notice on Login when destination is set Needs work

Test failed. Need work.

The tests that are failing seem to stem from a core issue 🌱 [META] Serialization issues in Migration tests Active . So either this issue can be blocked until the core issue is fixed, or based on the reviews that it's working can be merged. Leaving it up to maintainers and leaving the status as is.

It also seems these are duplicates 🐛 Throws Notice on Login when destination is set Needs work and 🐛 Autologout does not handle multiple destinations Needs work .

Corrected the CS issue for missing doc comment for the new getDestination function, testing is now done via GitLab CI, so test are passing now. Changing status to needs review so it can be re-tested.

Hi @DeaOm, your MR is failing. Please check

Cleaned up the code a bit, since it's partially related to this issue, I think it's better to just fix it now. Otherwise, everything works as expected, and looking at the other two related issues, this one is cleaner and should probably be merged. Leaving the other two issues open so maintainers can credit them.

Hiding the issue patch file, since the test there will always fail now that we are switching to GitlabCI. @sandipta this is probably why you thought that the MR was falling, it's not. Just integration with GitlabCI is different and doesn't display passing and failing tests the same way DrupalCI did.

Leaving the issue to "needs review", because of the code changes I made.

Great. Thanks for the info @admirlju

The getDestination() function which was getting the destination from referrer was always returning an empty string, which means the if block was always skipped. It did solve the warning but also broke the functionality of redirecting user back to its own profile (if for example user/2 was logged in and then logged out and user/3 logged in after, the user/3 was redirected to the user/2 profile, because of the destination). Adjusted the condition to basically always redirect to its own user profile, so we do not need to explicitly check if user and user id match based on destination, getting ride of a chunk of code. Also added a test that checks that the destination is set and after user/2 is logged out and user/3 is logged in, the user/3 is redirected to the correct user profile. Ready for review.

Tests were passing over gitlabCI but were not passing locally. This was down to /web/ being used in path checking for the destination query parameter. Changed it so it now dynamically gets the correct path depending on the Drupal configuration. Otherwise, the fix works, testing by hand if I get logged out on an account with UID 2 and login using an account with UID 3 it redirects me to the currently logged-in user profile.

All that said, there is an issue if this should even be the functionality in the first place, for now, this should be merged since it fixes a bug in the current implementation of the module. But it would be nice if people contributed their opinions on this issue #3301938: Consider Backing out User Destination Manipulation (3101732) → .

Because of the change to test, this still needs review.

Tests are passing and I agree with the change to get the URL of user login page via route as this then guarantees tests are working everywhere.

Can we copy over credit from 🐛 Throws Notice on Login when destination is set Needs work please?

I have applied a small change on the MR in order to improve the detection of the logic that should happen when destination parameter is found in the URL.

Instead of using strpos (that is too generic), I have changed the implementation to use a regex.

I have done that because I found a problem with the following situation:

- Create a page with the following alias /user/my-user-info
- Login with the following URL /user/login?destination=/user/my-user-info
- After login, Drupal redirects to /user/ instead of /user/my-user-info because `strpos` detects the "user" word in the destination parameter

i am getting the same warnings; hope there will be release with fix soon (:

claudiu.cristea → made their first commit to this issue’s fork.

Rerolled after 1.5.0

Moving to 2.x but I have no permission to change the base branch in MR

Since it seems the MR patch url isn't working correctly (it seems outdated), here's a patch file that applies to both 1.x and 2.x branches. It's the same as the MR.

Patch is not working with 8.x-1.4, so generated new patch file (no logical change, only generated compatible patch) .
Here is patch.

+1

please release

Previous patches are not working with 8.x-1.5, so generated new patch file.

also +1 to release this fix... thank you!