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

Enable broker support on Linux platform #5086

Merged
merged 151 commits into from
Feb 24, 2025
Merged

Enable broker support on Linux platform #5086

merged 151 commits into from
Feb 24, 2025

Conversation

xinyuxu1026
Copy link
Contributor

@xinyuxu1026 xinyuxu1026 commented Jan 16, 2025

Fixes #

Changes proposed in this request

  • Add broker support on Linux and WSL platforms
    • Linux platform doesn't require a new one to be registered, it should be able to use the same redirect_uri as Windows platform.
    • For parent window, on Linux we import libX11 to get the window handle, and this applies to both Linux and WSL since they are both Linux environments
    • Linux broker doesn't support POP
  • Update msalruntime to version 0.18.0

Testing

  • Microsoft.Identity.Test.Integration.NetCore tests are enabled in the CI for Linux platform
  • Manual testing: test Msalruntime flow in WSL(Windows Subsystem for Linux)

Performance impact

Documentation

  • All relevant documentation is updated.

@xinyuxu1026 xinyuxu1026 requested a review from a team as a code owner January 16, 2025 00:05
@gladjohn gladjohn requested a review from Copilot January 16, 2025 05:09

Choose a reason for hiding this comment

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

Copilot reviewed 5 out of 10 changed files in this pull request and generated no comments.

Files not reviewed (5)
  • Directory.Packages.props: Language not supported
  • NuGet.Config: Language not supported
  • src/client/Microsoft.Identity.Client.Broker/Microsoft.Identity.Client.Broker.csproj: Language not supported
  • tests/devapps/WAM/NetWSLWam/Properties/launchSettings.json: Language not supported
  • tests/devapps/WAM/NetWSLWam/test.csproj: Language not supported
Comments suppressed due to low confidence (2)

src/client/Microsoft.Identity.Client/PlatformsCommon/Shared/DesktopOsHelper.cs:59

  • [nitpick] The method name 'IsRunningOnWsl' could be more descriptive. Consider renaming it to 'IsRunningOnWindowsSubsystemForLinux'.
public static bool IsRunningOnWsl()

src/client/Microsoft.Identity.Client.Broker/RuntimeBroker.cs:129

  • Replace the debug log statement with a proper logging mechanism: _logger.Info("Runtime Broker AcquireTokenInteractiveAsync");
Console.WriteLine("Runtime Broker AcquireTokenInteractiveAsync");
Copy link
Contributor

@gladjohn gladjohn left a comment

Choose a reason for hiding this comment

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

good work @xinyuxu1026, some comments

  • few tests have asserts commented out
  • really like to understand the CI variables check on product code
  • still no dll checks on the pipeline after building the test project
  • linux file check may need a exception handler

@xinyuxu1026 xinyuxu1026 merged commit b994b69 into main Feb 24, 2025
7 checks passed
@xinyuxu1026 xinyuxu1026 deleted the xinyu/wsl-support branch February 24, 2025 05:28
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.

4 participants