Skip to content

Commit

Permalink
Improved authentication prompt with the header provided by VPN server
Browse files Browse the repository at this point in the history
  • Loading branch information
ancwrd1 committed Feb 14, 2025
1 parent 0bd5647 commit 240c2ae
Show file tree
Hide file tree
Showing 7 changed files with 58 additions and 37 deletions.
18 changes: 13 additions & 5 deletions snx-rs-gui/src/prompt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,15 @@ use gtk::{
prelude::{BoxExt, DialogExt, EntryExt, GtkWindowExt, WidgetExt},
Align, Orientation, ResponseType, WindowPosition,
};

use snxcore::model::AuthPrompt;
use snxcore::prompt::SecurePrompt;

use crate::dbus::send_notification;

pub struct GtkPrompt;

impl GtkPrompt {
fn get_input(&self, prompt: &str, secure: bool) -> anyhow::Result<String> {
fn get_input(&self, prompt: &AuthPrompt, secure: bool) -> anyhow::Result<String> {
let (tx, rx) = mpsc::channel();

let prompt = prompt.to_owned();
Expand All @@ -34,8 +34,16 @@ impl GtkPrompt {
let content = dialog.content_area();
let inner = gtk::Box::builder().orientation(Orientation::Vertical).margin(6).build();

if !prompt.header.is_empty() {
inner.pack_start(
&gtk::Label::builder().label(&prompt.header).halign(Align::Start).build(),
false,
true,
12,
);
}
inner.pack_start(
&gtk::Label::builder().label(&prompt).halign(Align::Start).build(),
&gtk::Label::builder().label(&prompt.prompt).halign(Align::Start).build(),
false,
true,
6,
Expand Down Expand Up @@ -68,11 +76,11 @@ impl GtkPrompt {
}

impl SecurePrompt for GtkPrompt {
fn get_secure_input(&self, prompt: &str) -> anyhow::Result<String> {
fn get_secure_input(&self, prompt: &AuthPrompt) -> anyhow::Result<String> {
self.get_input(prompt, true)
}

fn get_plain_input(&self, prompt: &str) -> anyhow::Result<String> {
fn get_plain_input(&self, prompt: &AuthPrompt) -> anyhow::Result<String> {
self.get_input(prompt, false)
}

Expand Down
9 changes: 5 additions & 4 deletions snx-rs/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use tracing::{debug, metadata::LevelFilter, warn};

use crate::cmdline::CmdlineParams;
use snxcore::model::params::TunnelType;
use snxcore::model::LoginPrompt;
use snxcore::model::AuthPrompt;
use snxcore::{
browser::spawn_otp_listener,
ccc::CccHttpClient,
Expand Down Expand Up @@ -146,13 +146,13 @@ async fn main_standalone(params: TunnelParams) -> anyhow::Result<()> {
MfaType::PasswordInput => {
let prompt = mfa_prompts
.pop_front()
.unwrap_or_else(|| LoginPrompt::new_password(&challenge.prompt));
.unwrap_or_else(|| AuthPrompt::new_password(&challenge.prompt));

let input = if !params.password.is_empty() && first_mfa && prompt.is_password() {
first_mfa = false;
Ok(params.password.clone())
} else {
TtyPrompt.get_secure_input(&prompt.prompt)
TtyPrompt.get_secure_input(&prompt)
};

match input {
Expand All @@ -172,7 +172,8 @@ async fn main_standalone(params: TunnelParams) -> anyhow::Result<()> {
session = connector.challenge_code(session, &otp).await?;
}
MfaType::UserNameInput => {
let input = TtyPrompt.get_plain_input(&challenge.prompt)?;
let prompt = AuthPrompt::new("", "username", &challenge.prompt);
let input = TtyPrompt.get_plain_input(&prompt)?;
session = connector.challenge_code(session, &input).await?;
}
}
Expand Down
15 changes: 8 additions & 7 deletions snxcore/src/controller.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use crate::{
browser::{spawn_otp_listener, BrowserController},
ccc::CccHttpClient,
model::{
params::TunnelParams, ConnectionStatus, LoginPrompt, MfaChallenge, MfaType, TunnelServiceRequest,
params::TunnelParams, AuthPrompt, ConnectionStatus, MfaChallenge, MfaType, TunnelServiceRequest,
TunnelServiceResponse,
},
platform::{self, UdpSocketExt},
Expand Down Expand Up @@ -45,7 +45,7 @@ impl FromStr for ServiceCommand {
pub struct ServiceController<B, P> {
pub params: Arc<TunnelParams>,
prompt: P,
mfa_prompts: Option<VecDeque<LoginPrompt>>,
mfa_prompts: Option<VecDeque<AuthPrompt>>,
password_from_keychain: String,
username: String,
first_mfa: bool,
Expand Down Expand Up @@ -134,7 +134,7 @@ where
.mfa_prompts
.as_mut()
.and_then(|p| p.pop_front())
.unwrap_or_else(|| LoginPrompt::new_password(&mfa.prompt));
.unwrap_or_else(|| AuthPrompt::new_password(&mfa.prompt));

if !self.params.password.is_empty() && self.first_mfa && prompt.is_password() {
self.first_mfa = false;
Expand All @@ -144,11 +144,11 @@ where
Ok(self.password_from_keychain.clone())
} else {
let prompt = if self.params.server_prompt {
&prompt.prompt
prompt
} else {
&mfa.prompt
AuthPrompt::new_password(&mfa.prompt)
};
let input = self.prompt.get_secure_input(prompt)?;
let input = self.prompt.get_secure_input(&prompt)?;
Ok(input)
}
}
Expand All @@ -169,7 +169,8 @@ where
}
}
MfaType::UserNameInput => {
let input = self.prompt.get_plain_input(&mfa.prompt)?;
let prompt = AuthPrompt::new("", "username", &mfa.prompt);
let input = self.prompt.get_plain_input(&prompt)?;
self.username = input.clone();

if !self.username.is_empty() && !self.params.no_keychain && self.params.password.is_empty() {
Expand Down
10 changes: 7 additions & 3 deletions snxcore/src/model.rs
Original file line number Diff line number Diff line change
Expand Up @@ -134,25 +134,29 @@ pub enum TunnelServiceResponse {
}

#[derive(Debug, Clone, PartialEq)]
pub struct LoginPrompt {
pub struct AuthPrompt {
pub header: String,
pub factor_type: String,
pub prompt: String,
}

impl LoginPrompt {
pub fn new<F, S>(factor_type: F, prompt: S) -> Self
impl AuthPrompt {
pub fn new<H, F, S>(header: H, factor_type: F, prompt: S) -> Self
where
H: AsRef<str>,
F: AsRef<str>,
S: AsRef<str>,
{
Self {
header: header.as_ref().to_owned(),
factor_type: factor_type.as_ref().to_owned(),
prompt: prompt.as_ref().to_owned(),
}
}

pub fn new_password<S: AsRef<str>>(prompt: S) -> Self {
Self {
header: String::new(),
factor_type: "password".to_owned(),
prompt: prompt.as_ref().to_owned(),
}
Expand Down
7 changes: 0 additions & 7 deletions snxcore/src/model/proto.rs
Original file line number Diff line number Diff line change
Expand Up @@ -339,13 +339,6 @@ pub enum LoginDisplayLabelSelect {
Empty(String),
}

#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
pub struct LoginDisplayLabel {
pub header: String,
pub username: Option<String>,
pub password: Option<String>,
}

#[derive(Default, Debug, Clone, PartialEq, Serialize, Deserialize)]
pub struct AuthenticationRealm {
#[serde(rename = "clientType")]
Expand Down
22 changes: 16 additions & 6 deletions snxcore/src/prompt.rs
Original file line number Diff line number Diff line change
@@ -1,29 +1,39 @@
use crate::model::AuthPrompt;
use anyhow::anyhow;
use std::io::Write;
use std::io::{stderr, stdin, IsTerminal};

pub trait SecurePrompt {
fn get_secure_input(&self, prompt: &str) -> anyhow::Result<String>;
fn get_secure_input(&self, prompt: &AuthPrompt) -> anyhow::Result<String>;

fn get_plain_input(&self, prompt: &str) -> anyhow::Result<String>;
fn get_plain_input(&self, prompt: &AuthPrompt) -> anyhow::Result<String>;

fn show_notification(&self, summary: &str, message: &str) -> anyhow::Result<()>;
}

pub struct TtyPrompt;

impl SecurePrompt for TtyPrompt {
fn get_secure_input(&self, prompt: &str) -> anyhow::Result<String> {
fn get_secure_input(&self, prompt: &AuthPrompt) -> anyhow::Result<String> {
if stdin().is_terminal() && stderr().is_terminal() {
Ok(passterm::prompt_password_stdin(Some(prompt), passterm::Stream::Stderr)?)
if !prompt.header.is_empty() {
println!("{}", prompt.header);
}
Ok(passterm::prompt_password_stdin(
Some(&prompt.prompt),
passterm::Stream::Stderr,
)?)
} else {
Err(anyhow!("No attached TTY to get user input!"))
}
}

fn get_plain_input(&self, prompt: &str) -> anyhow::Result<String> {
fn get_plain_input(&self, prompt: &AuthPrompt) -> anyhow::Result<String> {
if stdin().is_terminal() && stderr().is_terminal() {
eprint!("{}", prompt);
if !prompt.header.is_empty() {
println!("{}", prompt.header);
}
eprint!("{}", prompt.prompt);
stderr().flush()?;
let mut line = String::new();
stdin().read_line(&mut line)?;
Expand Down
14 changes: 9 additions & 5 deletions snxcore/src/server_info.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use crate::model::proto::LoginDisplayLabelSelect;
use crate::model::LoginPrompt;
use crate::model::AuthPrompt;
use crate::{
ccc::CccHttpClient,
model::{
Expand All @@ -23,15 +23,19 @@ pub async fn get(params: &TunnelParams) -> anyhow::Result<ServerInfoResponse> {
.try_into()
}

pub async fn get_mfa_prompts(params: &TunnelParams) -> anyhow::Result<VecDeque<LoginPrompt>> {
pub async fn get_mfa_prompts(params: &TunnelParams) -> anyhow::Result<VecDeque<AuthPrompt>> {
let factors = get_login_factors(params).await?;

let result = factors
.into_iter()
.filter_map(|factor| match factor.custom_display_labels {
LoginDisplayLabelSelect::LoginDisplayLabel(map) => map
.get("password")
.map(|label| LoginPrompt::new(factor.factor_type, format!("{}: ", label))),
LoginDisplayLabelSelect::LoginDisplayLabel(map) => map.get("password").map(|label| {
AuthPrompt::new(
map.get("header").map(ToOwned::to_owned).unwrap_or_default(),
factor.factor_type,
format!("{}: ", label),
)
}),
LoginDisplayLabelSelect::Empty(_) => None,
})
.collect();
Expand Down

0 comments on commit 240c2ae

Please sign in to comment.