Skip to content

Feature for #27518, users can revoke active sessions again.

Jesse Hall requested to merge jessehall3/gitlab-ee:27518-revoke-active-sessions into master

This MR would resolve #27518 (closed), restoring a user's ability to revoke active sessions.

Details

As discussed in #27518 (closed), the ability to revoke active sessions was temporarily removed for security reasons.

The Revoke button and associated controller action were removed in commit 038d5305.

Also, impersonated sessions were filtered from a user's list of active sessions in commit 44c4aad9.

This MR is a work in progress, as I am still writing the tests. The changes here simply reuse the code that was previously removed with one exception -- session IDs are no longer used within a webpage, or within a route, for revoking/destroying sessions. Instead, a new "public session ID" is used, see L26 in app/models/active_session.rb. The intent here is to completely avoid exposing session IDs.

As there are a number of UX and security implications that go along with these changes, I'm not expecting this MR is be merge-ready without further feedback.

Special Notes

Impersonated Sessions

Because the exposure of session IDs for impersonated sessions was the focal point of the security issue from before (as I read in #27518 (closed)), I made no attempts here to guess on how to handle the process of revoking impersonated sessions. That said, I explicitly prohibited impersonated sessions from being revoked through the destroy method called by the active sessions controller, see L107 in app/models/active_session.rb. I figured those decisions could be discussed in another issue.

I'm Not A Security Expert

My assumption here is that the lingering security concern was the continued exposure of session IDs to the user. By creating a UUID independent of the session id, and only exposing that UUID to the user, I believe that problem is resolved. I cannot foresee how this new UUID exposes any information about the session ID, but I will leave it to the security experts to provide a definitive answer, and perhaps an alternative solution.

Conformity

Performance and Testing

Security

If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:

  • Label as security and @ mention @gitlab-com/gl-security/appsec
  • The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
  • Security reports checked/validated by a reviewer from the AppSec team

Closes #27518 (closed)

Edited by 🤖 GitLab Bot 🤖

Merge request reports