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

reword user-facing messages to say "passwordless" #485

Merged
merged 1 commit into from
Oct 16, 2024
+14 −26
Merged

reword user-facing messages to say "passwordless" #485

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
36 changes: 12 additions & 24 deletions src/D2L.Bmx/OktaAuthenticator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,14 +150,14 @@ bool bypassBrowserSecurity
bool hasElevatedPermissions = UserPrivileges.HasElevatedPermissions();
if( hasElevatedPermissions && !bypassBrowserSecurity ) {
consoleWriter.WriteWarning( $"""
BMX is being run with elevated privileges and is unable to automatically sign in to Okta.
If you want to automatically sign in, and aren't concerned with the security of {orgUrl.Host},
consider using '--experimental-bypass-browser-security' flag.
BMX is being run with elevated privileges and is unable to use Okta passwordless authentication.
If you want passwordless authentication, and aren't concerned with the security of {orgUrl.Host},
consider using the '--experimental-bypass-browser-security' flag.
"""
);
return null;
} else if( !hasElevatedPermissions && bypassBrowserSecurity ) {
// We want to avoid providing '--no-sandbox' to chromium unless absolutely neccessary.
// We want to avoid providing '--no-sandbox' to chromium unless absolutely necessary.
bypassBrowserSecurity = false;
}

Expand All @@ -167,7 +167,7 @@ BMX is being run with elevated privileges and is unable to automatically sign in
}

if( !nonInteractive ) {
Console.Error.WriteLine( "Attempting to automatically sign in to Okta." );
Console.Error.WriteLine( "Attempting Okta passwordless authentication." );
}
using var cancellationTokenSource = new CancellationTokenSource( TimeSpan.FromSeconds( 15 ) );
var sessionIdTcs = new TaskCompletionSource<string?>( TaskCreationOptions.RunContinuationsAsynchronously );
Expand All @@ -193,7 +193,7 @@ async Task GetSessionCookieAsync() {
await page.GoToAsync( orgUrl.AbsoluteUri ).WaitAsync( cancellationTokenSource.Token );
} else {
consoleWriter.WriteWarning(
"Failed to authenticate with Okta when trying to automatically sign in" );
"Okta passwordless authentication failed" );
sessionIdTcs.SetResult( null );
}
return;
Expand All @@ -205,22 +205,9 @@ async Task GetSessionCookieAsync() {
}
}
} catch( TaskCanceledException ) {
consoleWriter.WriteWarning( $"""
Timed out when trying to automatically sign in to Okta. Check if the org '{orgUrl}' is correct.
If you have to run BMX with elevated privileges, and aren't concerned with the security of {orgUrl.Host},
consider running the command again with the '--experimental-bypass-browser-security' flag.
"""
);
} catch( TargetClosedException ) {
consoleWriter.WriteWarning( """
Failed to automatically sign in to Okta as BMX is likely being run with elevated privileges.
If you have to run BMX with elevated privileges, and aren't concerned with the security of {orgUrl.Host},
consider running the command again with the '--experimental-bypass-browser-security' flag.
Comment on lines -210 to -218
Copy link
Member Author

Choose a reason for hiding this comment

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

since #482, we check for elevated permission beforehand and short circuit the logic, so messages here are unnecessary

"""
);
consoleWriter.WriteWarning( "Okta passwordless authentication timed out." );
} catch( Exception ) {
consoleWriter.WriteWarning(
"Unknown error occurred while trying to automatically sign in with Okta." );
consoleWriter.WriteWarning( "Unknown error occurred while trying Okta passwordless authentication." );
}

if( sessionId is null ) {
Expand All @@ -232,9 +219,10 @@ consider running the command again with the '--experimental-bypass-browser-secur
string sessionLogin = oktaSession.Login.Split( "@" )[0];
string providedLogin = user.Split( "@" )[0];
if( !sessionLogin.Equals( providedLogin, StringComparison.OrdinalIgnoreCase ) ) {
consoleWriter.WriteWarning(
"Could not automatically sign in to Okta as provided Okta user "
+ $"'{sessionLogin}' does not match user '{providedLogin}'." );
consoleWriter.WriteWarning( $"""
Okta passwordless authentication failed.
The provided Okta user '{providedLogin}' does not match the system configured passwordless user '{sessionLogin}'.
""" );
return null;
}

Expand Down
2 changes: 1 addition & 1 deletion src/D2L.Bmx/ParameterDescriptions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,5 +20,5 @@ internal static class ParameterDescriptions {
""";

public const string ExperimentalBypassBrowserSecurity
= "Disable Chromium sandbox when automatically signing into Okta";
= "Disable Chromium sandbox when using Okta passwordless authentication";
}
2 changes: 1 addition & 1 deletion src/D2L.Bmx/Program.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
name: "--user",
description: ParameterDescriptions.User );

// allow no-sandbox argument for chromium to for passwordless auth with elevated permissions
// allow no-sandbox argument for Chromium to for passwordless auth with elevated permissions
Copy link
Member

Choose a reason for hiding this comment

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

This message seems a bit weirdly written?

Probably should say something along the lines of how: we need to set explicitly acknowledge we have no sandboxing in elevated permissions by passing this flag

Copy link
Member Author

Choose a reason for hiding this comment

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

ha, I was actually trying to fix this comment, that's why I touched "Chromium" here, but didn't end up completing the job.
will leave it be for now.

var bypassBrowserSecurityOption = new Option<bool>(
name: "--experimental-bypass-browser-security",
description: ParameterDescriptions.ExperimentalBypassBrowserSecurity );
Expand Down
Loading