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.

Sign up for GitHub

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

don't try passwordless on Linux #493

Merged
merged 3 commits into from
Oct 23, 2024
Merged

don't try passwordless on Linux #493

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5a0ab79
don't try passwordless on Linux
cfbao Oct 23, 2024
f1b0d7e
exclude linux earlier
cfbao Oct 23, 2024
086519c
comment
cfbao Oct 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 2 additions & 7 deletions src/D2L.Bmx/BrowserLauncher.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,6 @@ internal class BrowserLauncher : IBrowserLauncher {
"/Applications/Google Chrome.app/Contents/MacOS/Google Chrome",
"/Applications/Microsoft Edge.app/Contents/MacOS/Microsoft Edge",
];
private static readonly string[] LinuxPaths = [
"/opt/google/chrome/chrome",
"/opt/microsoft/msedge/msedge",
];

async Task<IBrowser> IBrowserLauncher.LaunchAsync( string browserPath ) {
var launchOptions = new LaunchOptions {
Expand Down Expand Up @@ -59,10 +55,9 @@ bool IBrowserLauncher.TryGetPathToBrowser( [NotNullWhen( returnValue: true )] ou
} else if( OperatingSystem.IsMacOS() ) {
path = Array.Find( MacPaths, File.Exists );
return path is not null;
} else if( OperatingSystem.IsLinux() ) {
path = Array.Find( LinuxPaths, File.Exists );
return path is not null;
}
// Okta DSSO is only supported for Windows and Mac. There's no point for us to support Linux.
// Chromium is finicky on Linux anyway.
return false;
}
}
7 changes: 6 additions & 1 deletion src/D2L.Bmx/OktaAuthenticator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,12 @@ bool ignoreCache
return new( Org: org, User: user, Client: oktaAuthenticated );
}

if( browserLauncher.TryGetPathToBrowser( out string? browserPath ) ) {
if(
// `TryGetPathToBrowser` does return `false` for Linux, but excluding Linux earlier here helps
// the compiler trim more unused code (e.g. all of PuppeteerSharp)
!OperatingSystem.IsLinux()
Comment on lines +63 to +65
Copy link
Member

Choose a reason for hiding this comment

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

I get how that works in theory, how the compiler can suss out that during build time that "hey this check with a static will always be false here, no need to include code only used in this branch"

It is still mind blowing :P

Not relevant here but, do you know if there is any ways to guarantee that certain things are trimmed or else to throw errors etc...? AFAIK its a bit of a problem with code that relies on compiler optimizations via clever syntax, where it would silently start using the slower path after relatively innocuous looking changes.

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't know of a way to do that in general.

In the specific case of OS dependent code though, using preprocessor directives is a common way to do it, e.g.
https://stackoverflow.com/questions/30153797/c-sharp-preprocessor-differentiate-between-operating-systems
The .NET runtime and core libraries themselves use this a lot.

I tried this a bit in the early days of .NET BMX, but couldn't get the dev experience I wanted.

&& browserLauncher.TryGetPathToBrowser( out string? browserPath )
) {
if( !nonInteractive ) {
Console.Error.WriteLine( "Attempting Okta passwordless authentication..." );
}
Expand Down
Loading