🇬🇧United Kingdom @philipnorton42

Created proposed change as a merge request.

Change committed and released as 1.0.5 ( https://www.drupal.org/project/session_inspector/releases/1.0.5 → )

Thanks!

Thanks for the bug report!

It looks like the "php" session.serialize_handler doesn't work correctly. It is needed to read the data held in other sessions and pick out data like the user agent and masqurade settings.

Looking at the strings in the session data is doesn't look like using string positions is the best way to go about things. I think a better way is to use the session_decode() method since that is built into PHP and should be able to read the session data correctly. I needed to get around the fact that this method alters the current session and use a temporary session to grab hold of the data.

Change incoming :)

Thanks for creating and maintaining this module by the way. It protects my site from quite a bit of random spam!

philipnorton42 → created an issue.

Added a merge request for the changes proposed.

Thanks for the work on the module!

philipnorton42 → created an issue.

philipnorton42 → created an issue.

Added to release 1.0.2.

Everything working as expected here, GitLab CI tests are green https://git.drupalcode.org/project/session_inspector_hostname_geocoder/-....

Whilst I'm doing this I will also update the required version of Geocoder to the latest stable version (4.18).

philipnorton42 → created an issue.

philipnorton42 → created an issue.

Added to release 1.0.4.

Just clearing out some old issues for this module. I'm happy to discuss adding this feature if you really want it, but for the time being I'm closing this issue.

Thanks very much for reporting this issue.

Unfortunately, there is no action that delete's all of the sessions from a site.

I think the route you are thinking about is "session_inspector.delete_all", but this is designed to delete all _other_ sessions for a single user. Essentially, this will clear out the sessions that is not currently active for a single user.

I deliberately didn't add functionality to clear out all sessions for all users since this can easily break the user experience. There are also other ways to do this (like truncating the sessions table for example).

Someone did suggest adding a drush command to clear out sessions (https://drupal.org/project/session_inspector/issues/3337515), but that issue hasn't been integrated.

GitLab file added, all tests are passing https://git.drupalcode.org/project/session_inspector/-/pipelines/68073.

philipnorton42 → created an issue.

I came across this issue the other day and found this patch, worked for my situation.

Added the test to check that this patch fixes the issue.

It's interesting you should say that the code in Bert.php doesn't seem to be needed. I wonder if it's a side effect of broken configuration that I've just worked around on my local setup?

Let me do some investigation and check that.

I was doing some testing on this today and realised that if a bundle with an entity is added to the field then the page crashes, I've pushed a fix for that so it works with non-bundle and bundle entities.

I found the same problem with the module and after some debugging I identified the same line of code being at fault.

The $form_op is the lock that needs to be broken, so if this is called from the content admin page then it contains a "*", which means to "break all locks". This string makes no sense to the database query since we are trying to delete any operations that are called "*" for this entity.

I think this solution to detect for the presence of the "*" and ignore the condition works and should be merged. The patch also passes coding standards checks.

Hi!

Thanks very much for creating this module, I really like the idea and there's a lot of work been done already on it.

I had a go with the fix you added here, and although it prevented the crash it did still have one or two issues. I've fixed these and added a small fix to the src/Plugin/Field/FieldWidget/Bert.php class that was uncovered after fixing the original issue.

I have pushed the fix to the fork that you created, so I've added a merge request as well. The fork was a little out of date with the main 2.x branch so I merged that in too.

The site I am working with now has user autocomplete fields working nicely for both the core fields and extra fields added.

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

I'm not sure what you're talking about ressa.

The last image I posted on my site that made it to Drupal planet was over 9 months ago (it was the DrupalCon Prague 2023 article). There are no current posts that feature images in the RSS feed and my older posts have been removed from the site.

As a precaution, I just double checked the output of my RSS feeds and that image had an absolute reference.

My website _is_ producing relative links in the markup, but not in the RSS feed. If you want to raise an issue with the content of my site then please submit via the contact form at https://www.hashbangcode.com/contact-us and I'd be happy to accommodate you :)

In the meantime, I'm going to close this ticket since I don't feel that this is an issue with my feed to Drupal Planet.

Thanks!

I know this is closed, but I just wanted to thank you very much for releasing a new version of the module so quickly. I really appreciate it.

Hi @zcht,

I have updated the MR with an event subscriber approach to this issue. As this removes the need for the ShyController I have removed that from the codebase.

I have never used the passwordless login module before, but I'm assuming that these changes would also negate the need for that controller. I've left it alone for now, but I'd be interested to hear what you think.

That's interesting, thanks for the update @zcht.

In that case there are two options:
- We copy and paste the entire codebase from the UserController module into the ShyController controller.
- We select another mechanism to perform this check. I'm thinking that instead of using the controller to return an access denied we do this through an event listener.

I was looking at event listeners the other day and one to perform this action should be pretty simple to write (I think!).

I'll revert that code and have another go at it.

Created an MR with the proposed change.

Marking as ready for review.

I see this problem too.

The above patch does solve the issue, but the code contains other issues that cause more crashes. For example, the `$this->userStorage` property is called, but that property is never set so the page crashes if the user reaches that code.

I wonder if a better approach might be to decorate or extend the original UserController here? Most of the code in the ShyController is using code from that controller and that would mean that the ShyController only needs to reject the request or call the parent UserController resetPass action.

I'll give this a go.

Thanks for the changes, I've merged them in.

Thanks for the patch! Merged.

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

Thanks for the addition, I've merged that in.

All sorted and released into version 1.0.3.

Thanks for reporting and fixing :)

I found this problem as well, but I found that it was connected to a different problem, with a few moving parts.

By default the path auto system will return a non-aliased path if the language isn't defined (i.e. the language is "und"). This means that this code will return the path "/node/123".

\Drupal::service('path_alias.manager')->getAliasByPath('/node/1', 'und');`

To get the correct path the language needs to be stipulated. This code will return the correct aliased path of the node.

\Drupal::service('path_alias.manager')->getAliasByPath('/node/1', 'end');`

I'm running a non-multilingual site and I was using the output on the page /admin/content/messages to test the templates. They would always show as having the incorrect path of "/node/123" due to this issue. The problem is that the page at /admin/content/messages does not pass a language to the getText() method when rendering the message entity. When this happens the getText() method just uses the default of "und", which introduces this issue.

When I created a view to print out the messages everything worked correctly.

I can see two avenues to correct this, but I'm not sure which one is best as I'm not exactly familiar with the module.

1) Alter MessageListBuilder::buildRow()

When calling getText(), pass in the language of the message entity. Something like this:

$text = $entity->getText($entity->get('langcode')->value);

2) Alter Message::getText()

During the running of getText() it looks to see if the language has been set, this could be changed to force the language to come from the message entity itself.

    if (!$langcode) {
      $langcode = $this->get('langcode')->value;
    }

As I said, I'm not sure which is the best approach here. I'm thinking that altering getText() might have some far reaching consequences. If anyone has an opinion then let me know and I'll create a patch :)

Ok, I figured out how to do change the target to 10.1.x and created another MR, this means we have two for this request:

9.5.x - https://git.drupalcode.org/project/drupal/-/merge_requests/2801
10.1.x - https://git.drupalcode.org/project/drupal/-/merge_requests/3423

I found these instructions that managed to perform the desired effect - https://www.drupal.org/docs/develop/git/using-git-to-contribute-to-drupa... →

I hope this is correct. Let me know if I've done anything wrong here. I'll pop this back to "needs review".

Hi tim-deils,

Thanks for taking an interest in session inspector!

Your changes make sense, but I'm spotting a couple of problems when I try to run things. I'll go through the bugs I found here, rather than add them to the merge request as comments.

1) Running the drush command without any parameters throws the following error.

```
$ drush session-inspector:delete-all-sessions

In Tables.php line 369:

'root' not found
```

This is because the command runs line 71 of the SessionInspectorCommands class with the $property of 'root' and the $value being the path of my Drupal installation.

This line:
```
$accounts = $this->userStorage->loadByProperties([$property => $value]);
```

I think there's more in the $options variable than you were expecting and that's causing some problems with the loadByProperties() method.

I'm running Drush 11.4.0 if that helps?

2) No session data is found

When looking at the session page for a user it displays no information. This is because the getSessions() method now uses an array, but the use of array_keys() within this method means that the array must be indexed when being passed in.

I changed the code to be this and it fixed the problem.

```
$sessions = $this->sessionInspector->getSessions([$user->id() => $user]);
```

I think, however, that it might be better to have a getSessionForUser() and getAllSessions() methods so that they can be treated differently and not have to code a single method to adapt to both situations.

3) Protection on the drush command

The drush command to delete everyones session needs to have a "are you sure?" check on it. Might help prevent mistakes with such a destructive command.

One of my clients has over a million users and things would get pretty heated if I accidentally logged everyone out! :)

4) Unit tests

It would be great to have some unit tests for the command. I created the following that checks that a user's session is killed off when the drush command is run.

```
<?php

namespace Drupal\Tests\session_inspector\Functional;

use Drupal\Tests\BrowserTestBase;
use Drush\TestTraits\DrushTestTrait;

/**
* Test the functionality of the session inspector module drush commands.
*
* @group session_inspector
*/
class SessionInspectorCommandsTest extends BrowserTestBase {

use DrushTestTrait;

/**
* Modules to enable.
*
* @var array
*/
protected static $modules = [
'session_inspector',
];

/**
* The theme to install as the default for testing.
*
* @var string
*/
public $defaultTheme = 'stark';

/**
* Test that a user is logged out via drush session .
*/
public function testDrushClearSession() {
$user = $this->createUser(['inspect own user sessions']);
$this->drupalLogin($user);

$this->drush('session-inspector:delete-all-sessions');

$this->drupalGet('user/' . $user->id() . '/sessions');
$this->assertSession()->responseContains('Access denied');
}

}
```

Due to the problems discussed above I haven't actually managed to get this test to run correctly, but it should help you spot problems on your end.

Thanks again for taking the time to make these changes. I really appreciate it.

Let me know if you have any questions.
Phil

Great work ressa, thanks very much!