remove --experimental-bypass-browser-security

Brightspace · Oct 21, 2024 · 7391cf9 · 7391cf9
1 parent e8c5942
commit 7391cf9
Show file tree

Hide file tree

Showing 7 changed files with 27 additions and 93 deletions.
diff --git a/src/D2L.Bmx/Browser.cs b/src/D2L.Bmx/Browser.cs
@@ -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 );

diff --git a/src/D2L.Bmx/LoginHandler.cs b/src/D2L.Bmx/LoginHandler.cs
@@ -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(
@@ -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." );
 	}

diff --git a/src/D2L.Bmx/OktaAuthenticator.cs b/src/D2L.Bmx/OktaAuthenticator.cs
@@ -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 ) ) {
@@ -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 ) {
@@ -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 );
@@ -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."
@@ -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." );
 			}
 		}

diff --git a/src/D2L.Bmx/ParameterDescriptions.cs b/src/D2L.Bmx/ParameterDescriptions.cs
@@ -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";
 }
diff --git a/src/D2L.Bmx/PrintHandler.cs b/src/D2L.Bmx/PrintHandler.cs
@@ -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,

diff --git a/src/D2L.Bmx/Program.cs b/src/D2L.Bmx/Program.cs
@@ -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();
@@ -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 )
 	);
 } );
 
@@ -126,7 +119,6 @@
 	userOption,
 	nonInteractiveOption,
 	cacheAwsCredentialsOption,
-	bypassBrowserSecurityOption,
 };
 
 printCommand.SetHandler( ( InvocationContext context ) => {
@@ -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 )
 	);
 } );
 
@@ -182,7 +173,6 @@
 	nonInteractiveOption,
 	cacheAwsCredentialsOption,
 	useCredentialProcessOption,
-	bypassBrowserSecurityOption,
 };
 
 writeCommand.SetHandler( ( InvocationContext context ) => {
@@ -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" );

diff --git a/src/D2L.Bmx/WriteHandler.cs b/src/D2L.Bmx/WriteHandler.cs
@@ -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,