Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Back channel logout #246

Open
wants to merge 7 commits into
base: develop
Choose a base branch
from

Conversation

upachler
Copy link

All Submissions:

Changes proposed in this Pull Request:

Aims to implement OIDC Backchannel Logout and Keycloak legacy BCL feature.

Screnario:

  • User logs into Wordpress via OIDC's OP
  • User then moves to other site, where she/he logs out of the OP, terminating all user sessions
  • When user moves back to Wordpress, the user is expected to be logged out

This change implements the last point in the screnario; previously, the user remained logged in as long as the WP session itself didn't time out (which is quite long compared to an access_token lifetime in typical OIDC configurations). Not being logged out is particularly strange for users who, in an SSO context, log out of a running OP session and then log in with a different user. If WP does not recognize the logout, the WP session will keep running with the original user, which breaks the seamlessness promised in SSO contexts.

Closes #205 .

How to test the changes in this Pull Request:

  1. Connect against OIDC BCL compliant OP or Keycloak < 12.0.0 (tested in 4.8)
  • For OIDC BCL, set https://my-wordpress-site.com/wp-admin/admin-ajax.php?action=openid-connect-backchannel-logout as BCL URL in WP's client config
  • For Keycloak Legacy BCL, you need to enable it first on the settings page, then set https://my-wordpress-site.com/ as the Admin URL in WP's client config
  1. Login in WP using OIDC, change over to non WP-site. Logout from there, so that WP doesn't see the logout directly
  2. Change back to WP
  3. WP session should be logged out.

BCL logouts are logged via the logging feature.

Other information:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes, as applicable?
  • Have you successfully run tests with your changes locally?
    -> Yes, in the Keycloak Legacy configuration, which is my main use case. I have no access to a BCL compliant OIDC provider

Changelog entry

Implement OpenID Connect Backchannel Logout
Implement Keycloak Backchannel Logout (k_logout, for Keycloak before v12.0.0)

…ranch was developed against 3.8.0; as this one is against dev)
@upachler upachler changed the title re-applied changes from feature/backchannel-logout branch (that old b… Back channel logout Oct 26, 2020
@upachler
Copy link
Author

Fixed all linter bugs in Travis CI, however don't know how to fix the 'Minimum Requirements' and 'Bleeding Edge' ones - but I'm wondering how those issues are related to my changes

@timnolte
Copy link
Collaborator

@upachler if you are using PHP functions that are only available in a newer PHP version then that is the problem. The checks should list where the problem is, looking.

@timnolte
Copy link
Collaborator

@upachler I checked the failed build and it's actually failing on a dependency supposedly. I'm going to have to run another dev build and see if something has changed with the package somehow. Unless it is just reporting the wrong source for the failure.

@timnolte
Copy link
Collaborator

@upachler I'm a bit concerned about this functionality being centered around Keycloak. The core functionality should be generalized with hooks provided for specific IDP customization. I'd like to see this changed to a core back channel logout that is according to spec. Something like the custom plugin setting can be added via the current hooks provided and then used in the hooks as needed in the generic back channel logout hooks.

@upachler
Copy link
Author

@timnolte I appreciate the desire to decouple the Keycloak specifics from the rest of the plugin, even more so because with the upcoming Keycloak 12.0.0, this feature is obsolete as Keycloak will support standard OIDC BCL (however, the newest commercial RH SSO will be based on a pre v12 Keycloak for quite a while longer, which why I developed Keycloak BCL support in the first place).
I'm wondering how a decoupled solution would look like though (keep in mind, this is both my first time developing in PHP as well as for Wordpress). Would it be a separate plugin, just for extending this one and supplying keycloak specific hooks? In this scenario, we wouldn't need Keycloak specific configuration, because enabling the Keycloak plugin would be all that's required.

However, the downside with new hooks is that they need to be supported in the future, which would weigh against the disadvantages of having keycloak specific functionality in the OIDC plugin.

Those are the major points where I see we'd need hooks for:

  • The main obstacle is the need to generate a repacement session ID for Keycloak, because it demands that being sent before calling the token endpoint to exchange the code for the access_token/id_token. However, at that point, there seems to be no session in Wordpress yet; that only exists after logging the user in, which can be only done once the token exchange was performed.
    So for Keycloak, I ended up generating a UUID-like random number that I can store as a session ID. This generated ID needs to be passed through to the point where the login happens, so that that part of the code is able to store the generated session ID (without Keycloak, it simply pulls that from the id_token).
    Maybe there is an alternative solution for a session ID, but I couldn't find anything so far that seemed reliable enough.
  • The other point is that Keycloak uses a fixed endpoint '/k_logout', which uses a different Content-Type (text/plain instead of application/json). Probably that's quite easy to do.
  • Last but not least the logout tokens differ in their formats, so we'd need separate token parsers that return a normalized claim set. Maybe the solution there would be for the KC hook to supply a faked id_token, that could also include the generated session id as 'sid' claim

@timnolte timnolte self-assigned this Jan 20, 2021
@timnolte timnolte added enhancement Issues & PRs related to new features. status: needs review PR that needs review. labels Jan 20, 2021
@timnolte timnolte deleted the branch oidc-wp:develop December 23, 2023 00:56
@timnolte timnolte closed this Dec 23, 2023
@timnolte timnolte reopened this Dec 23, 2023
@timnolte timnolte changed the base branch from dev to develop December 23, 2023 01:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Issues & PRs related to new features. status: needs review PR that needs review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Back channel logout
3 participants