Skip to content

Conversation

@OneBlue
Copy link
Collaborator

@OneBlue OneBlue commented Jan 15, 2026

Summary of the Pull Request

This change redesigns our per user session management logic into a global SessionManager.

This allows sessions to be interacted across Windows users.

PR Checklist

  • Closes: Link to issue #xxx
  • Communication: I've discussed this with core contributors already. If work hasn't been agreed, this work might be rejected
  • Tests: Added/updated if needed and all pass
  • Localization: All end user facing strings can be localized
  • Dev docs: Added/updated if needed
  • Documentation updated: If checked, please file a pull request on our docs repo and link it here: #xxx

Detailed Description of the Pull Request / Additional comments

Validation Steps Performed

@OneBlue OneBlue changed the title User/oneblue/session manager Redesign WSLA Session management Jan 15, 2026
@OneBlue OneBlue marked this pull request as ready for review January 16, 2026 02:18
@OneBlue OneBlue requested a review from a team as a code owner January 16, 2026 02:18
Copilot AI review requested due to automatic review settings January 16, 2026 02:18
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR redesigns WSLA session management from a per-user session model to a global SessionManager. The change enables sessions to be accessed across Windows users with proper access control, replacing WSLAUserSession with WSLASessionManager throughout the codebase.

Changes:

  • Replaced per-user WSLAUserSession with global WSLASessionManager for centralized session management
  • Added access control logic to prevent non-elevated tokens from accessing elevated sessions
  • Introduced COMImplClass helper template to standardize COM-to-implementation forwarding
  • Updated GetNonElevatedToken to accept a TOKEN_TYPE parameter for flexible token generation

Reviewed changes

Copilot reviewed 26 out of 26 changed files in this pull request and generated 8 comments.

Show a summary per file
File Description
src/windows/wslaservice/inc/wslaservice.idl Renamed IWSLAUserSession to IWSLASessionManager, added GetId() method to IWSLASession, and added Sid field to session information
src/windows/wslaservice/exe/WSLASessionManager.h/.cpp New global session manager implementation with token-based access control
src/windows/wslaservice/exe/WSLASessionManagerFactory.h/.cpp Factory for singleton session manager instance
src/windows/wslaservice/exe/WSLASession.h/.cpp Added session creator tracking (PID, SID, elevation status)
src/windows/wslaservice/exe/COMImplClass.h New helper template for COM-to-implementation forwarding pattern
src/windows/wslaservice/exe/WSLAContainer.h/.cpp Refactored to use COMImplClass base template
test/windows/Common.h/.cpp Updated GetNonElevatedToken to accept TOKEN_TYPE parameter
test/windows/WSLATests.cpp Updated tests for new session manager API and added elevation access control tests
src/windows/common/WslClient.cpp Updated to use IWSLASessionManager
src/windows/wsladiag/wsladiag.cpp Updated to use IWSLASessionManager

{
wil::com_ptr<IWSLAUserSession> userSession;
VERIFY_SUCCEEDED(CoCreateInstance(__uuidof(WSLAUserSession), nullptr, CLSCTX_LOCAL_SERVER, IID_PPV_ARGS(&userSession)));
wil::com_ptr<IWSLASessionManager> userSession;
Copy link

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The variable is named userSession but now represents a global session manager, not a per-user session. Rename to manager or sessionManager for clarity and consistency with the rest of the test.

Copilot uses AI. Check for mistakes.
{
wil::com_ptr<IWSLAUserSession> userSession;
VERIFY_SUCCEEDED(CoCreateInstance(__uuidof(WSLAUserSession), nullptr, CLSCTX_LOCAL_SERVER, IID_PPV_ARGS(&userSession)));
wil::com_ptr<IWSLASessionManager> userSession;
Copy link

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The variable is named userSession but now represents a global session manager, not a per-user session. Rename to manager or sessionManager for clarity and consistency with the rest of the test.

Copilot uses AI. Check for mistakes.

wil::com_ptr<IWSLAUserSession> userSession;
VERIFY_SUCCEEDED(CoCreateInstance(__uuidof(WSLAUserSession), nullptr, CLSCTX_LOCAL_SERVER, IID_PPV_ARGS(&userSession)));
wil::com_ptr<IWSLASessionManager> userSession;
Copy link

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The variable is named userSession but now represents a global session manager, not a per-user session. Rename to manager or sessionManager for clarity and consistency with the test at line 2361.

Copilot uses AI. Check for mistakes.

wil::com_ptr<IWSLAUserSession> userSession;
VERIFY_SUCCEEDED(CoCreateInstance(__uuidof(WSLAUserSession), nullptr, CLSCTX_LOCAL_SERVER, IID_PPV_ARGS(&userSession)));
wil::com_ptr<IWSLASessionManager> userSession;
Copy link

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The variable is named userSession but now represents a global session manager, not a per-user session. Rename to manager or sessionManager for clarity and consistency with the test at line 2361.

Copilot uses AI. Check for mistakes.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
}

wil::unique_handle GetNonElevatedToken()
wil::unique_handle GetNonElevatedToken(TOKEN_TYPE Type)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Big thanks to @johnstep for his help in getting this right

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants