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

remove --experimental-bypass-browser-security #488

Merged
merged 1 commit into from
Oct 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 4 additions & 2 deletions src/D2L.Bmx/Browser.cs
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,12 @@ public static class Browser {
"/opt/microsoft/msedge/msedge",
];

public static async Task<IBrowser> LaunchBrowserAsync( string browserPath, bool noSandbox = false ) {
public static async Task<IBrowser> LaunchBrowserAsync( string browserPath ) {
var launchOptions = new LaunchOptions {
ExecutablePath = browserPath,
Args = noSandbox ? ["--no-sandbox"] : []
// For whatever reason, with an elevated user, Chromium cannot launch in headless mode without --no-sandbox.
// This isn't a big concern for BMX, because it only visits the org Okta homepage, which should be trusted.
Args = UserPrivileges.HasElevatedPermissions() ? ["--no-sandbox"] : []
};

return await Puppeteer.LaunchAsync( launchOptions );
Expand Down
6 changes: 2 additions & 4 deletions src/D2L.Bmx/LoginHandler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,7 @@ OktaAuthenticator oktaAuth
) {
public async Task HandleAsync(
string? org,
string? user,
bool bypassBrowserSecurity
string? user
) {
if( !File.Exists( BmxPaths.CONFIG_FILE_NAME ) ) {
throw new BmxException(
Expand All @@ -17,8 +16,7 @@ await oktaAuth.AuthenticateAsync(
org,
user,
nonInteractive: false,
ignoreCache: true,
bypassBrowserSecurity: bypassBrowserSecurity
ignoreCache: true
);
Console.WriteLine( "Successfully logged in and Okta session has been cached." );
}
Expand Down
72 changes: 12 additions & 60 deletions src/D2L.Bmx/OktaAuthenticator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,7 @@ public async Task<OktaAuthenticatedContext> AuthenticateAsync(
string? org,
string? user,
bool nonInteractive,
bool ignoreCache,
bool bypassBrowserSecurity
bool ignoreCache
) {
var orgSource = ParameterSource.CliArg;
if( string.IsNullOrEmpty( org ) && !string.IsNullOrEmpty( config.Org ) ) {
Expand Down Expand Up @@ -59,21 +58,25 @@ bool bypassBrowserSecurity
return new( Org: org, User: user, Client: oktaAuthenticated );
}

if( TryGetDssoAuthOptions( nonInteractive, bypassBrowserSecurity, out var dssoAuthOptions ) ) {
if( Browser.TryGetPathToBrowser( out string? browserPath ) ) {
if( !nonInteractive ) {
Console.Error.WriteLine( "Attempting Okta passwordless authentication..." );
}
oktaAuthenticated = await GetDssoAuthenticatedClientAsync(
orgUrl,
user,
dssoAuthOptions
browserPath
);
if( oktaAuthenticated is not null ) {
return new( Org: org, User: user, Client: oktaAuthenticated );
}
if( !nonInteractive ) {
Console.Error.WriteLine( "Falling back to Okta password authentication..." );
}
} else if( BmxEnvironment.IsDebug ) {
consoleWriter.WriteWarning(
"No suitable browser found for Okta passwordless authentication"
);
}

if( nonInteractive ) {
Expand Down Expand Up @@ -105,63 +108,12 @@ private bool TryAuthenticateFromCache(
return true;
}

private record DssoAuthOptions( string BrowserPath, bool BypassBrowserSecurity, bool PrintDebugMessage );

private bool TryGetDssoAuthOptions(
bool nonInteractive,
bool bypassBrowserSecurity,
[NotNullWhen( returnValue: true )] out DssoAuthOptions? options
) {
options = null;

bool printDebugMessage =
BmxEnvironment.IsDebug
|| (
// if the user passed --experimental-bypass-browser-security, they've explicitly expressed interest in the browser/DSSO process
bypassBrowserSecurity
// but we still shouldn't print messages in non-interactive mode (it may break credential process)
&& !nonInteractive
);

bool hasElevatedPermissions = UserPrivileges.HasElevatedPermissions();
if( hasElevatedPermissions && !bypassBrowserSecurity ) {
if( printDebugMessage ) {
consoleWriter.WriteWarning(
"BMX is being run with elevated privileges and is unable to use Okta passwordless authentication."
);
}
return false;
}
if( !hasElevatedPermissions && bypassBrowserSecurity ) {
// We want to avoid providing '--no-sandbox' to chromium unless absolutely necessary.
bypassBrowserSecurity = false;
if( printDebugMessage ) {
consoleWriter.WriteWarning(
"Ignoring '--experimental-bypass-browser-security', "
+ "as it's only needed when BMX is run with elevated privileges."
);
}
}

if( !Browser.TryGetPathToBrowser( out string? browserPath ) ) {
if( printDebugMessage ) {
consoleWriter.WriteWarning(
"No suitable browser found for Okta passwordless authentication"
);
}
return false;
}

options = new( browserPath, bypassBrowserSecurity, printDebugMessage );
return true;
}

private async Task<IOktaAuthenticatedClient?> GetDssoAuthenticatedClientAsync(
Uri orgUrl,
string user,
DssoAuthOptions options
string browserPath
) {
await using var browser = await Browser.LaunchBrowserAsync( options.BrowserPath, options.BypassBrowserSecurity );
await using var browser = await Browser.LaunchBrowserAsync( browserPath );

using var cancellationTokenSource = new CancellationTokenSource( TimeSpan.FromSeconds( 15 ) );
var sessionIdTcs = new TaskCompletionSource<string?>( TaskCreationOptions.RunContinuationsAsynchronously );
Expand All @@ -186,7 +138,7 @@ async Task GetSessionCookieAsync() {
attempt++;
await page.GoToAsync( orgUrl.AbsoluteUri ).WaitAsync( cancellationTokenSource.Token );
} else {
if( options.PrintDebugMessage ) {
if( BmxEnvironment.IsDebug ) {
if( url.AbsolutePath == "/" ) {
consoleWriter.WriteWarning(
"Okta passwordless authentication is not available."
Expand All @@ -206,11 +158,11 @@ async Task GetSessionCookieAsync() {
}
}
} catch( TaskCanceledException ) {
if( options.PrintDebugMessage ) {
if( BmxEnvironment.IsDebug ) {
consoleWriter.WriteWarning( "Okta passwordless authentication timed out." );
}
} catch( Exception ) {
if( options.PrintDebugMessage ) {
if( BmxEnvironment.IsDebug ) {
consoleWriter.WriteWarning( "Unknown error occurred while trying Okta passwordless authentication." );
}
}
Expand Down
3 changes: 0 additions & 3 deletions src/D2L.Bmx/ParameterDescriptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,4 @@ internal static class ParameterDescriptions {
Write BMX command to AWS profile, so that AWS tools & SDKs using the profile will source credentials from BMX.
See https://docs.aws.amazon.com/cli/latest/userguide/cli-configure-sourcing-external.html.
""";

public const string ExperimentalBypassBrowserSecurity
= "Disable Chromium sandbox when using Okta passwordless authentication";
}
6 changes: 2 additions & 4 deletions src/D2L.Bmx/PrintHandler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,13 @@ public async Task HandleAsync(
int? duration,
bool nonInteractive,
string? format,
bool cacheAwsCredentials,
bool bypassBrowserSecurity
bool cacheAwsCredentials
) {
var oktaContext = await oktaAuth.AuthenticateAsync(
org: org,
user: user,
nonInteractive: nonInteractive,
ignoreCache: false,
bypassBrowserSecurity: bypassBrowserSecurity
ignoreCache: false
);
var awsCreds = ( await awsCredsCreator.CreateAwsCredsAsync(
okta: oktaContext,
Expand Down
21 changes: 5 additions & 16 deletions src/D2L.Bmx/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -18,16 +18,10 @@
name: "--user",
description: ParameterDescriptions.User );

// allow no-sandbox argument for Chromium to for passwordless auth with elevated permissions
var bypassBrowserSecurityOption = new Option<bool>(
name: "--experimental-bypass-browser-security",
description: ParameterDescriptions.ExperimentalBypassBrowserSecurity );

// bmx login
var loginCommand = new Command( "login", "Log into Okta and save an Okta session" ){
var loginCommand = new Command( "login", "Log into Okta and save an Okta session" ) {
orgOption,
userOption,
bypassBrowserSecurityOption,
};
loginCommand.SetHandler( ( InvocationContext context ) => {
var consoleWriter = new ConsoleWriter();
Expand All @@ -41,8 +35,7 @@
) );
return handler.HandleAsync(
org: context.ParseResult.GetValueForOption( orgOption ),
user: context.ParseResult.GetValueForOption( userOption ),
bypassBrowserSecurity: context.ParseResult.GetValueForOption( bypassBrowserSecurityOption )
user: context.ParseResult.GetValueForOption( userOption )
);
} );

Expand Down Expand Up @@ -126,7 +119,6 @@
userOption,
nonInteractiveOption,
cacheAwsCredentialsOption,
bypassBrowserSecurityOption,
};

printCommand.SetHandler( ( InvocationContext context ) => {
Expand Down Expand Up @@ -155,8 +147,7 @@
duration: context.ParseResult.GetValueForOption( durationOption ),
nonInteractive: context.ParseResult.GetValueForOption( nonInteractiveOption ),
format: context.ParseResult.GetValueForOption( formatOption ),
cacheAwsCredentials: context.ParseResult.GetValueForOption( cacheAwsCredentialsOption ),
bypassBrowserSecurity: context.ParseResult.GetValueForOption( bypassBrowserSecurityOption )
cacheAwsCredentials: context.ParseResult.GetValueForOption( cacheAwsCredentialsOption )
);
} );

Expand All @@ -182,7 +173,6 @@
nonInteractiveOption,
cacheAwsCredentialsOption,
useCredentialProcessOption,
bypassBrowserSecurityOption,
};

writeCommand.SetHandler( ( InvocationContext context ) => {
Expand Down Expand Up @@ -217,9 +207,8 @@
output: context.ParseResult.GetValueForOption( outputOption ),
profile: context.ParseResult.GetValueForOption( profileOption ),
cacheAwsCredentials: context.ParseResult.GetValueForOption( cacheAwsCredentialsOption ),
useCredentialProcess: context.ParseResult.GetValueForOption( useCredentialProcessOption ),
bypassBrowserSecurity: context.ParseResult.GetValueForOption( bypassBrowserSecurityOption )
);
useCredentialProcess: context.ParseResult.GetValueForOption( useCredentialProcessOption )
);
} );

var updateCommand = new Command( "update", "Updates BMX to the latest version" );
Expand Down
6 changes: 2 additions & 4 deletions src/D2L.Bmx/WriteHandler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -27,17 +27,15 @@ public async Task HandleAsync(
string? output,
string? profile,
bool cacheAwsCredentials,
bool useCredentialProcess,
bool bypassBrowserSecurity
bool useCredentialProcess
) {
cacheAwsCredentials = cacheAwsCredentials || useCredentialProcess;

var oktaContext = await oktaAuth.AuthenticateAsync(
org: org,
user: user,
nonInteractive: nonInteractive,
ignoreCache: false,
bypassBrowserSecurity: bypassBrowserSecurity
ignoreCache: false
);
var awsCredsInfo = await awsCredsCreator.CreateAwsCredsAsync(
okta: oktaContext,
Expand Down
Loading