easier to understand messages / prompts for user (#361)

### Why Making it easier to update prompt text and not showing exception messages users likely won't understand Swapped the order of commands and options for most useful to likely least Chenfeng had an existing branch for this and I have just fixed the merge issues --------- Co-authored-by: Chenfeng Bao <[email protected]>
Brightspace · Jul 5, 2023 · 4094b59 · 4094b59
1 parent 0991ee3
commit 4094b59
Show file tree

Hide file tree

Showing 9 changed files with 139 additions and 100 deletions.
diff --git a/src/D2L.Bmx/BmxException.cs b/src/D2L.Bmx/BmxException.cs
@@ -1,6 +1,10 @@
 namespace D2L.Bmx;
 
+/// <remarks>
+/// This exception is used to display an error message to the user.
+/// Do not include internal implementation details or very technical info in the exception message.
+/// If there's no user facing info to convey, use a different exception type.
+/// </remarks>
 internal class BmxException : Exception {
 	public BmxException( string message ) : base( message ) { }
-	public BmxException( string message, Exception innerException ) : base( message, innerException ) { }
 }
diff --git a/src/D2L.Bmx/ConsolePrompter.cs b/src/D2L.Bmx/ConsolePrompter.cs
@@ -34,8 +34,7 @@ internal class ConsolePrompter : IConsolePrompter {
 		) );
 
 	string IConsolePrompter.PromptOrg( bool allowEmptyInput ) {
-		Console.Error.Write(
-			$"The tenant name or full domain name of the Okta organization {( allowEmptyInput ? "(Optional): " : ": " )}" );
+		Console.Error.Write( $"{ParameterDescriptions.Org}{( allowEmptyInput ? "(Optional): " : ": " )}" );
 		string? org = _stdinReader.ReadLine();
 		if( org is null || ( string.IsNullOrWhiteSpace( org ) && !allowEmptyInput ) ) {
 			throw new BmxException( "Invalid org input" );
@@ -45,7 +44,7 @@ string IConsolePrompter.PromptOrg( bool allowEmptyInput ) {
 	}
 
 	string IConsolePrompter.PromptProfile() {
-		Console.Error.Write( "AWS profile: " );
+		Console.Error.Write( $"{ParameterDescriptions.Profile}: " );
 		string? profile = _stdinReader.ReadLine();
 		if( string.IsNullOrEmpty( profile ) ) {
 			throw new BmxException( "Invalid profile input" );
@@ -55,7 +54,7 @@ string IConsolePrompter.PromptProfile() {
 	}
 
 	string IConsolePrompter.PromptUser( bool allowEmptyInput ) {
-		Console.Error.Write( $"Okta Username {( allowEmptyInput ? " (Optional): " : ": " )}" );
+		Console.Error.Write( $"{ParameterDescriptions.User}{( allowEmptyInput ? " (Optional): " : ": " )}" );
 		string? user = _stdinReader.ReadLine();
 		if( user is null || ( string.IsNullOrWhiteSpace( user ) && !allowEmptyInput ) ) {
 			throw new BmxException( "Invalid user input" );
@@ -85,7 +84,7 @@ Input to BMX is redirected. Password input may be displayed on screen!
 			readKey = () => (char)_stdinReader.Read();
 		}
 
-		Console.Error.Write( "Okta Password: " );
+		Console.Error.Write( $"{ParameterDescriptions.Password}: " );
 
 		string? originalTerminalSettings = null;
 		try {
@@ -116,7 +115,7 @@ Input to BMX is redirected. Password input may be displayed on screen!
 					Console.Error.Write( moveLeftString + emptyString + moveLeftString );
 					passwordBuilder.Clear();
 				} else
-				// The backsapce key is received as the DEL character in raw mode
+				// The backspace key is received as the DEL character in raw mode
 				if( ( key == '\b' || key == DEL ) && passwordBuilder.Length > 0 ) {
 					Console.Error.Write( "\b \b" );
 					passwordBuilder.Length--;
@@ -133,7 +132,7 @@ Input to BMX is redirected. Password input may be displayed on screen!
 	}
 
 	int? IConsolePrompter.PromptDuration() {
-		Console.Error.Write( "Duration of AWS sessions in minutes (optional, default: 60): " );
+		Console.Error.Write( $"{ParameterDescriptions.Duration} (optional, default: 60): " );
 		string? input = _stdinReader.ReadLine();
 		if( input is null || !int.TryParse( input, out int duration ) || duration <= 0 ) {
 			return null;
@@ -175,16 +174,16 @@ OktaMfaFactor IConsolePrompter.SelectMfa( OktaMfaFactor[] mfaOptions ) {
 		}
 
 		if( mfaOptions.Length == 1 ) {
-			Console.Error.WriteLine( $"MFA method: {mfaOptions[0].Provider}: {mfaOptions[0].FactorType}" );
+			Console.Error.WriteLine( $"MFA method: {mfaOptions[0].Provider}-{mfaOptions[0].FactorType}" );
 			return mfaOptions[0];
 		}
 
 		for( int i = 0; i < mfaOptions.Length; i++ ) {
-			Console.Error.WriteLine( $"[{i + 1}] {mfaOptions[i].Provider}: {mfaOptions[i].FactorType}" );
+			Console.Error.WriteLine( $"[{i + 1}] {mfaOptions[i].Provider}-{mfaOptions[i].FactorType}" );
 		}
 		Console.Error.Write( "Select an available MFA option: " );
 		if( !int.TryParse( _stdinReader.ReadLine(), out int index ) || index > mfaOptions.Length || index < 1 ) {
-			throw new BmxException( "Invalid account selection" );
+			throw new BmxException( "Invalid MFA selection" );
 		}
 		return mfaOptions[index - 1];
 	}
@@ -196,7 +195,7 @@ string IConsolePrompter.GetMfaResponse( string mfaInputPrompt ) {
 		if( mfaInput is not null ) {
 			return mfaInput;
 		}
-		throw new BmxException( "Invalid Mfa Input" );
+		throw new BmxException( "Invalid MFA Input" );
 	}
 
 	private static string GetCurrentTerminalSettings() {

diff --git a/src/D2L.Bmx/LoginHandler.cs b/src/D2L.Bmx/LoginHandler.cs
@@ -9,7 +9,8 @@ public async Task HandleAsync(
 	) {
 		if( !File.Exists( BmxPaths.CONFIG_FILE_NAME ) ) {
 			throw new BmxException(
-				"No Config file found! Okta sessions are not cached without a config file. Please run bmx configure first." );
+				"BMX global config file not found. Okta sessions will not be saved. Please run `bmx configure` first."
+			);
 		}
 		await oktaAuth.AuthenticateAsync( org, user, nonInteractive: false, ignoreCache: true );
 		Console.WriteLine( "Successfully logged in and Okta session has been cached." );

diff --git a/src/D2L.Bmx/Okta/OktaApi.cs b/src/D2L.Bmx/Okta/OktaApi.cs
@@ -44,7 +44,7 @@ void IOktaApi.AddSession( string sessionId ) {
 		if( _httpClient.BaseAddress is not null ) {
 			_cookieContainer.Add( new Cookie( "sid", sessionId, "/", _httpClient.BaseAddress.Host ) );
 		} else {
-			throw new BmxException( "Error adding session: http client base address is not defined" );
+			throw new InvalidOperationException( "Error adding session: http client base address is not defined" );
 		}
 	}
 
@@ -73,7 +73,7 @@ await resp.Content.ReadAsStreamAsync(),
 				authnResponse.Embedded.Factors
 			);
 		}
-		throw new BmxException( "Error authenticating Okta" );
+		throw new BmxException( "Error authenticating to Okta" );
 	}
 
 	async Task IOktaApi.IssueMfaChallengeAsync( string stateToken, string factorId ) {
@@ -107,7 +107,7 @@ await resp.Content.ReadAsStreamAsync(),
 		if( authnResponse?.SessionToken is not null ) {
 			return new AuthenticateResponse.Success( authnResponse.SessionToken );
 		}
-		throw new BmxException( "Error authenticating Okta challenge MFA" );
+		throw new BmxException( "Error verifying Okta MFA challenge response" );
 	}
 
 	async Task<OktaSession> IOktaApi.CreateSessionAsync( string sessionToken ) {

diff --git a/src/D2L.Bmx/OktaAuthenticator.cs b/src/D2L.Bmx/OktaAuthenticator.cs
@@ -41,7 +41,7 @@ bool ignoreCache
 			return oktaApi;
 		}
 		if( nonInteractive ) {
-			throw new BmxException( "Authentication failed. No cached session" );
+			throw new BmxException( "Okta authentication failed. Please run `bmx login` first." );
 		}
 
 		string password = consolePrompter.PromptPassword();
@@ -52,7 +52,7 @@ bool ignoreCache
 			OktaMfaFactor mfaFactor = consolePrompter.SelectMfa( mfaInfo.Factors );
 
 			if( !IsMfaFactorTypeSupported( mfaFactor.FactorType ) ) {
-				throw new BmxException( "Selected MFA not supported by BMX." );
+				throw new BmxException( "Selected MFA not supported by BMX" );
 			}
 
 			// TODO: Handle retry
@@ -71,13 +71,16 @@ bool ignoreCache
 			if( File.Exists( BmxPaths.CONFIG_FILE_NAME ) ) {
 				CacheOktaSession( user, org, sessionResp.Id, sessionResp.ExpiresAt );
 			} else {
+				Console.ResetColor();
+				Console.ForegroundColor = ConsoleColor.Yellow;
 				Console.Error.WriteLine( "No config file found. Your Okta session will not be cached. " +
 				"Consider running `bmx configure` if you own this machine." );
+				Console.ResetColor();
 			}
 			return oktaApi;
 		}
 
-		throw new BmxException( "Authentication Failed" );
+		throw new BmxException( "Okta authentication failed" );
 	}
 
 	private async Task<bool> TryAuthenticateFromCacheAsync(

diff --git a/src/D2L.Bmx/ParameterDescriptions.cs b/src/D2L.Bmx/ParameterDescriptions.cs
@@ -0,0 +1,14 @@
+namespace D2L.Bmx;
+
+internal static class ParameterDescriptions {
+	public const string Org = "Okta org short name or domain name";
+	public const string User = "Okta username";
+	public const string Password = "Okta password";
+	public const string Account = "AWS account name";
+	public const string Role = "AWS role name";
+	public const string Duration = "AWS session duration in minutes";
+	public const string Profile = "AWS profile name";
+	public const string Output = "Custom path to the AWS credentials file";
+	public const string Format = "Output format of AWS credentials";
+	public const string NonInteractive = "Run non-interactively without showing any prompts";
+}
diff --git a/src/D2L.Bmx/PrintFormat.cs b/src/D2L.Bmx/PrintFormat.cs
@@ -0,0 +1,12 @@
+namespace D2L.Bmx;
+
+internal static class PrintFormat {
+	public static readonly HashSet<string> All = new( StringComparer.OrdinalIgnoreCase ) {
+		Bash,
+		PowerShell,
+		Json,
+	};
+	public const string Bash = "Bash";
+	public const string PowerShell = "PowerShell";
+	public const string Json = "JSON";
+}
diff --git a/src/D2L.Bmx/PrintHandler.cs b/src/D2L.Bmx/PrintHandler.cs
@@ -13,16 +13,16 @@ public async Task HandleAsync(
 		string? role,
 		int? duration,
 		bool nonInteractive,
-		string? output
+		string? format
 	) {
 		var oktaApi = await oktaAuth.AuthenticateAsync( org, user, nonInteractive, ignoreCache: false );
 		var awsCreds = await awsCredsCreator.CreateAwsCredsAsync( oktaApi, account, role, duration, nonInteractive );
 
-		if( string.Equals( output, "bash", StringComparison.OrdinalIgnoreCase ) ) {
+		if( string.Equals( format, PrintFormat.Bash, StringComparison.OrdinalIgnoreCase ) ) {
 			PrintBash( awsCreds );
-		} else if( string.Equals( output, "powershell", StringComparison.OrdinalIgnoreCase ) ) {
+		} else if( string.Equals( format, PrintFormat.PowerShell, StringComparison.OrdinalIgnoreCase ) ) {
 			PrintPowershell( awsCreds );
-		} else if( string.Equals( output, "json", StringComparison.OrdinalIgnoreCase ) ) {
+		} else if( string.Equals( format, PrintFormat.Json, StringComparison.OrdinalIgnoreCase ) ) {
 			PrintJson( awsCreds );
 		} else {
 			string? procName = null;