- Issue created by @zewebmaster
- 🇫🇷France Thony
Had reproduced the issue on my own project, I can confirm the patch fixed it, thanks
- Status changed to Needs review
7 months ago 7:56am 19 April 2024
When a user connects with a keycloak plugin, session information is not stored in the private tempstore.
Without this information, it is not possible to use the refreshToken to check the validity of the user's session.
$keycloak = \Drupal::service('keycloak.keycloak');
$sessionInfo = $keycloak->getSessionInfo();
dump($sessionInfo);
[
"access_token" => null
"refresh_token" => null
"id_token" => null
"client_id" => null
"session_id" => null
]
This problem has two distinct sources in the code.
The first problem comes from the keycloak_openid_connect_post_authorize() hook, which does not correctly check the openid_connect plugin used.
if (empty($plugin_id) || $plugin_id !== 'keycloak') {
// Nothing to do. Bail out.
return;
}
At this point, the hook performs a check on the plugin identifier, which corresponds to 'keycloak'.
But in reality, this id corresponds to the machine name of the plugin, which is not necessarily 'keycloak'.
To improve this check, we could base it on the plugin class corresponding to this plugin id:
Once the hook has run successfully, processing ends with the 'keycloak.keycloak' service storing the session information.
// Whether the user is not authenticated or the Keycloak client disabled.
if (!$this->currentUser->isAuthenticated() || !$this->isEnabled()) {
return FALSE;
}
At this point, the service checks whether the keycloak plugin is enabled in the configuration file.
The correction can be made in two ways:
1. Remove the check on the active status of the plugin.
I would opt for this solution for two reason:
First, I'm not sure that this check is really necessary. In fact, the service is called in a hook that is necessarily triggered by an active Keycloak plugin - whatever the instance.
Second, There is no need to correct the function KeycloakService::isCheckSessionEnabled(), which would cause an error when the keycloak_page_attachments_alter() is triggered. (This hook crashed on the first condition, which checks the tempstore data, and now it would crash on the second condition, which checks whether the plugin is active).
2. Allow the KeycloakService::setSessionInfo() function to accept the plugin_id as a parameter, so that the corresponding configuration file can be dynamically loaded into the service and checked.
I propose a patch that makes a correction in line with the above analysis.
There are still a few problems in the module linked to this configuration file, which is hard-coded in the service.
We still need to implement a method for checking sessions that no longer pass through an iFrame.
Needs review
2.2
Code
Had reproduced the issue on my own project, I can confirm the patch fixed it, thanks