From 97d69f37e98920dd3cff47ec4c52a127b0445ff4 Mon Sep 17 00:00:00 2001 From: Frederic Laing Date: Wed, 12 Nov 2025 20:35:10 +0100 Subject: [PATCH] implement a more consistent login mask with a stable layout and improved error messages --- examples/server.rs | 12 ++- i18n/en/cosmic_greeter.ftl | 8 ++ src/greeter.rs | 200 ++++++++++++++++++++++++++++--------- src/greeter/ipc.rs | 36 ++++++- src/locker.rs | 174 ++++++++++++++++++++++++++++---- 5 files changed, 352 insertions(+), 78 deletions(-) diff --git a/examples/server.rs b/examples/server.rs index b9f4ae6..6fbfe5f 100644 --- a/examples/server.rs +++ b/examples/server.rs @@ -54,10 +54,14 @@ fn main() { Request::PostAuthMessageResponse { response } => { match response.as_deref() { Some("password") => Response::Success, - _ => Response::Error { - error_type: ErrorType::AuthError, - description: "AUTH_ERR".to_string(), - }, + _ => { + // Add 1 second delay to simulate real PAM authentication failure behavior + tokio::time::sleep(std::time::Duration::from_secs(1)).await; + Response::Error { + error_type: ErrorType::AuthError, + description: "AUTH_ERR".to_string(), + } + } } } Request::StartSession { .. } => Response::Success, diff --git a/i18n/en/cosmic_greeter.ftl b/i18n/en/cosmic_greeter.ftl index 122fe04..2dc8677 100644 --- a/i18n/en/cosmic_greeter.ftl +++ b/i18n/en/cosmic_greeter.ftl @@ -3,6 +3,7 @@ accessibility = Accessibility .magnifier = Magnifier .high-contrast = High contrast .invert-colors = Invert Colors +authenticating = Authenticating... cancel = Cancel caps-lock = Caps Lock is active. enter-user = Enter name manually... @@ -25,3 +26,10 @@ shutdown-timeout = The system will shut down automatically in } suspend = Suspend user = User + +# Authentication errors +auth-error-default = Authentication failed. Please try again. +auth-error-credentials = Incorrect password. Please check your keyboard layout and try again. +auth-error-denied = Access denied. +auth-error-maxtries = Too many failed authentication attempts. +auth-error-account = Account is unavailable or disabled. diff --git a/src/greeter.rs b/src/greeter.rs index c1cfb12..f66642c 100644 --- a/src/greeter.rs +++ b/src/greeter.rs @@ -410,6 +410,7 @@ pub enum Message { HighContrast(bool), InvertColors(bool), WaylandUpdate(WaylandUpdate), + SpinnerTick, } impl From for Message { @@ -438,6 +439,9 @@ pub struct App { randr_list: Option, accessibility: Accessibility, + authenticating: bool, + spinner_rotation: f32, + spinner_handle: Option, } #[derive(Default)] @@ -617,7 +621,7 @@ impl App { } let item_cnt = items.len(); let menu_button = widget::menu::menu_button(vec![ - Element::from(widget::Space::with_width(Length::Fixed(25.0))), + Element::from(widget::Space::with_width(Length::Fixed(10.0))), widget::text(fl!("enter-user")) .align_x(iced::alignment::Horizontal::Left) .into(), @@ -756,6 +760,21 @@ impl App { .spacing(12.0) .max_width(280.0); + let military_time = self + .selected_username + .data_idx + .and_then(|i| self.flags.user_datas.get(i)) + .map(|user_data| user_data.time_applet_config.military_time) + .unwrap_or_default(); + let space_height = match military_time { + true => 63.0, + false => 10.0, + }; + + // Add top spacing for better visual appearance + // Bottom of the password text input field should align with bottom of time widget + column = column.push(widget::Space::with_height(Length::Fixed(space_height))); + match &self.socket_state { SocketState::Pending => { column = column.push(widget::text("Opening GREETD_SOCK")); @@ -769,16 +788,32 @@ impl App { { if !self.entering_name && user_data.name == self.selected_username.username { - if let Some(icon) = user_icon { + // Display user icon or empty transparent box + if let Some(icon_handle) = user_icon { column = column.push( widget::container( - widget::image(icon) + widget::image(icon_handle) .width(Length::Fixed(78.0)) - .height(Length::Fixed(78.0)), + .height(Length::Fixed(78.0)) + .content_fit(iced::ContentFit::Fill), ) + .padding(0.0) .width(Length::Fill) + .height(Length::Fixed(78.0)) .align_x(Alignment::Center), - ) + ); + } else { + // Empty transparent box for users without icons + column = column.push( + widget::container(widget::Space::new( + Length::Fixed(78.0), + Length::Fixed(78.0), + )) + .padding(0.0) + .width(Length::Fill) + .height(Length::Fixed(78.0)) + .align_x(Alignment::Center), + ); } column = column.push( widget::container(widget::text::title4(&user_data.full_name)) @@ -801,52 +836,59 @@ impl App { if let Some((prompt, secret, value_opt)) = &self.common.prompt_opt { match value_opt { Some(value) => { - let text_input_id = self - .common - .surface_names - .get(&id) - .and_then(|id| self.common.text_input_ids.get(id)) - .cloned() - .unwrap_or_else(|| cosmic::widget::Id::new("text_input")); - let mut text_input = widget::secure_input( - prompt.clone(), - value.as_str(), - Some( + // Only show password input when not authenticating + if !self.authenticating { + let text_input_id = self + .common + .surface_names + .get(&id) + .and_then(|id| self.common.text_input_ids.get(id)) + .cloned() + .unwrap_or_else(|| cosmic::widget::Id::new("text_input")); + let mut text_input = widget::secure_input( + prompt.clone(), + value.as_str(), + Some( + common::Message::Prompt( + prompt.clone(), + !*secret, + Some(value.clone()), + ) + .into(), + ), + *secret, + ) + .id(text_input_id) + .on_input(|input| { common::Message::Prompt( prompt.clone(), - !*secret, - Some(value.clone()), + *secret, + Some(input), ) - .into(), - ), - *secret, - ) - .id(text_input_id) - .on_input(|input| { - common::Message::Prompt(prompt.clone(), *secret, Some(input)) .into() - }) - .on_submit(|v| Message::Auth(Some(v))); + }) + .on_submit(|v| Message::Auth(Some(v))); - if let Some(text_input_id) = self - .common - .surface_names - .get(&id) - .and_then(|id| self.common.text_input_ids.get(id)) - { - text_input = text_input.id(text_input_id.clone()); - } + if let Some(text_input_id) = self + .common + .surface_names + .get(&id) + .and_then(|id| self.common.text_input_ids.get(id)) + { + text_input = text_input.id(text_input_id.clone()); + } - if *secret { - text_input = text_input.password() - } + if *secret { + text_input = text_input.password() + } - column = column.push(text_input); + column = column.push(text_input); - if self.common.caps_lock { - column = column.push(widget::text(fl!("caps-lock"))); - } else if self.common.error_opt.is_none() { - column = column.push(widget::text("")); + if self.common.caps_lock { + column = column.push(widget::text(fl!("caps-lock"))); + } else if self.common.error_opt.is_none() { + column = column.push(widget::text("")); + } } } None => { @@ -868,7 +910,27 @@ impl App { } } - if let Some(error) = &self.common.error_opt { + // Show either authenticating message or error message in the same location + if self.authenticating { + column = column.push( + widget::container( + widget::row::with_capacity(2) + .spacing(8.0) + .align_y(Alignment::Center) + .push( + widget::icon::from_name("process-working-symbolic") + .size(16) + .icon() + .rotation(iced::Rotation::Floating(iced::Radians( + self.spinner_rotation.to_radians(), + ))), + ) + .push(widget::text(fl!("authenticating"))), + ) + .width(Length::Fill) + .align_x(Alignment::Center), + ); + } else if let Some(error) = &self.common.error_opt { column = column.push( widget::text(error) .class(theme::Text::Color(iced::Color::from_rgb(1.0, 0.0, 0.0))), @@ -885,9 +947,8 @@ impl App { .width(Length::Fill) }; let menu = widget::container(widget::column::with_children(vec![ - widget::Space::with_height(Length::FillPortion(1)).into(), widget::layer_container( - iced::widget::row![left_element, right_element].align_y(Alignment::Center), + iced::widget::row![left_element, right_element].align_y(Alignment::Start), ) .layer(cosmic::cosmic_theme::Layer::Background) .padding(16) @@ -905,7 +966,7 @@ impl App { .class(cosmic::theme::Container::Background) .width(Length::Fixed(800.0)) .into(), - widget::Space::with_height(Length::FillPortion(4)).into(), + widget::Space::with_height(Length::Fill).into(), ])) .width(Length::Fill) .height(Length::Fill) @@ -1114,6 +1175,9 @@ impl cosmic::Application for App { theme_builder: Default::default(), randr_list: None, surface_id_pairs: Vec::new(), + authenticating: false, + spinner_rotation: 0.0, + spinner_handle: None, }; (app, Task::batch(tasks)) } @@ -1297,6 +1361,7 @@ impl cosmic::Application for App { } if self.entering_name || username != self.selected_username.username { self.entering_name = false; + self.authenticating = false; let data_idx = self .flags .user_datas @@ -1408,13 +1473,40 @@ impl cosmic::Application for App { } } Message::Auth(response) => { - self.common.prompt_opt = None; self.common.error_opt = None; + self.authenticating = true; self.send_request(Request::PostAuthMessageResponse { response }); + + // Start spinner animation if not already running + if self.spinner_handle.is_none() { + let (spinner_task, handle) = cosmic::task::stream( + cosmic::iced_futures::stream::channel(1, |mut msg_tx| async move { + let mut interval = time::interval(Duration::from_millis(16)); // ~60fps + loop { + msg_tx + .send(cosmic::Action::App(Message::SpinnerTick)) + .await + .unwrap(); + interval.tick().await; + } + }), + ) + .abortable(); + self.spinner_handle = Some(handle); + return spinner_task; + } } Message::Login => { self.common.prompt_opt = None; self.common.error_opt = None; + self.authenticating = false; + + // Stop spinner animation + if let Some(handle) = self.spinner_handle.take() { + handle.abort(); + } + self.spinner_rotation = 0.0; + match self.flags.sessions.get(&self.selected_session).cloned() { Some((cmd, env)) => { self.send_request(Request::StartSession { cmd, env }); @@ -1425,6 +1517,14 @@ impl cosmic::Application for App { } Message::Error(error) => { self.common.error_opt = Some(error); + self.authenticating = false; + + // Stop spinner animation + if let Some(handle) = self.spinner_handle.take() { + handle.abort(); + } + self.spinner_rotation = 0.0; + self.send_request(Request::CancelSession); } Message::Reconnect => { @@ -1754,6 +1854,10 @@ impl cosmic::Application for App { }; return reposition_subsurface(*subsurface_id, loc.x as i32, loc.y as i32); } + Message::SpinnerTick => { + // Update spinner rotation angle (360 degrees per second = 6 degrees per frame at 60fps) + self.spinner_rotation = (self.spinner_rotation + 6.0) % 360.0; + } } Task::none() } diff --git a/src/greeter/ipc.rs b/src/greeter/ipc.rs index 103f98a..c6e070d 100644 --- a/src/greeter/ipc.rs +++ b/src/greeter/ipc.rs @@ -10,7 +10,31 @@ use std::time::Duration; use tokio::net::UnixStream; use tokio::sync::mpsc; -use crate::common; +use crate::{common, fl}; + +/// Convert greetd error descriptions to user-friendly localized messages +fn greetd_error_to_message(error_type: greetd_ipc::ErrorType, description: &str) -> String { + use greetd_ipc::ErrorType; + + match error_type { + ErrorType::AuthError => { + // For authentication errors, check description for specific error types + if description.contains("PERM_DENIED") { + fl!("auth-error-denied") + } else if description.contains("MAXTRIES") { + fl!("auth-error-maxtries") + } else if description.contains("ACCT_EXPIRED") || description.contains("USER_UNKNOWN") { + fl!("auth-error-account") + } else { + fl!("auth-error-credentials") + } + } + ErrorType::Error => { + // For generic errors, show a generic message + fl!("auth-error-default") + } + } +} pub fn subscription() -> Subscription { struct GreetdSubscription; @@ -89,10 +113,9 @@ pub fn subscription() -> Subscription { } }, greetd_ipc::Response::Error { - error_type: _, + error_type, description, } => { - //TODO: use error_type? match request { greetd_ipc::Request::CancelSession => { // Do not send errors for cancel session to gui @@ -105,7 +128,12 @@ pub fn subscription() -> Subscription { break; } _ => { - _ = sender.send(Message::Error(description)).await; + _ = sender + .send(Message::Error(greetd_error_to_message( + error_type, + &description, + ))) + .await; } } } diff --git a/src/locker.rs b/src/locker.rs index 8474bba..fa34f99 100644 --- a/src/locker.rs +++ b/src/locker.rs @@ -103,6 +103,31 @@ pub fn main(user: pwd::Passwd) -> Result<(), Box> { Ok(()) } +/// Convert PAM errors to user-friendly localized messages +fn pam_error_to_message(error: &pam_client::Error) -> String { + use pam_client::ErrorCode; + + // Use the structured error code instead of string matching for reliability + match error.code() { + ErrorCode::AUTH_ERR | ErrorCode::CRED_INSUFFICIENT => { + fl!("auth-error-credentials") + } + ErrorCode::PERM_DENIED => { + fl!("auth-error-denied") + } + ErrorCode::MAXTRIES => { + fl!("auth-error-maxtries") + } + ErrorCode::ACCT_EXPIRED | ErrorCode::USER_UNKNOWN => { + fl!("auth-error-account") + } + _ => { + // For any other error, show a generic message + fl!("auth-error-default") + } + } +} + pub fn pam_thread(username: String, conversation: Conversation) -> Result<(), pam_client::Error> { //TODO: send errors to GUI, restart process @@ -241,6 +266,7 @@ pub enum Message { Error(String), Lock, Unlock, + SpinnerTick, } impl From for Message { @@ -277,6 +303,9 @@ pub struct App { dropdown_opt: Option, inhibit_opt: Option>, value_tx_opt: Option>, + authenticating: bool, + spinner_rotation: f32, + spinner_handle: Option, } impl App { @@ -422,16 +451,39 @@ impl App { .spacing(12.0) .max_width(280.0); - if let Some(icon) = &self.flags.user_icon { + let military_time = self.flags.user_data.time_applet_config.military_time; + let space_height = match military_time { + true => 63.0, + false => 10.0, + }; + + // Add top spacing for better visual appearance + // Bottom of the password text input field should align with bottom of time widget + column = column.push(widget::Space::with_height(Length::Fixed(space_height))); + + // Display user icon or empty transparent box + if let Some(icon_handle) = &self.flags.user_icon { column = column.push( widget::container( - widget::image(icon) + widget::image(icon_handle) .width(Length::Fixed(78.0)) - .height(Length::Fixed(78.0)), + .height(Length::Fixed(78.0)) + .content_fit(iced::ContentFit::Fill), ) + .padding(0.0) .width(Length::Fill) + .height(Length::Fixed(78.0)) .align_x(Alignment::Center), - ) + ); + } else { + // Empty transparent box for users without icons + column = column.push( + widget::container(widget::Space::new(Length::Fixed(78.0), Length::Fixed(78.0))) + .padding(0.0) + .width(Length::Fill) + .height(Length::Fixed(78.0)) + .align_x(Alignment::Center), + ); } column = column.push( @@ -464,11 +516,17 @@ impl App { ), *secret, ) - .id(text_input_id) - .on_input(|input| { - common::Message::Prompt(prompt.clone(), *secret, Some(input)).into() - }) - .on_submit(Message::Submit); + .id(text_input_id); + + // Don't allow input when authenticating + if !self.authenticating { + text_input = text_input + .on_input(|input| { + common::Message::Prompt(prompt.clone(), *secret, Some(input)) + .into() + }) + .on_submit(Message::Submit); + } if *secret { text_input = text_input.password() @@ -476,7 +534,7 @@ impl App { column = column.push(text_input); - if self.common.caps_lock { + if self.common.caps_lock && !self.authenticating { column = column.push(widget::text(fl!("caps-lock"))); } else if self.common.error_opt.is_none() { column = column.push(widget::text("")); @@ -488,7 +546,27 @@ impl App { } } - if let Some(error) = &self.common.error_opt { + // Show either authenticating message or error message in the same location + if self.authenticating { + column = column.push( + widget::container( + widget::row::with_capacity(2) + .spacing(8.0) + .align_y(Alignment::Center) + .push( + widget::icon::from_name("process-working-symbolic") + .size(16) + .icon() + .rotation(iced::Rotation::Floating(iced::Radians( + self.spinner_rotation.to_radians(), + ))), + ) + .push(widget::text(fl!("authenticating"))), + ) + .width(Length::Fill) + .align_x(Alignment::Center), + ); + } else if let Some(error) = &self.common.error_opt { column = column.push( widget::text(error) .class(theme::Text::Color(iced::Color::from_rgb(1.0, 0.0, 0.0))), @@ -506,9 +584,8 @@ impl App { }; widget::container(widget::column::with_children(vec![ - widget::Space::with_height(Length::FillPortion(1)).into(), widget::layer_container( - iced::widget::row![left_element, right_element].align_y(Alignment::Center), + iced::widget::row![left_element, right_element].align_y(Alignment::Start), ) .layer(cosmic::cosmic_theme::Layer::Background) .padding(16) @@ -526,7 +603,7 @@ impl App { .width(Length::Fill) .height(Length::Shrink) .into(), - widget::Space::with_height(Length::FillPortion(4)).into(), + widget::Space::with_height(Length::Fill).into(), ])) .width(Length::Fill) .height(Length::Fill) @@ -579,6 +656,9 @@ impl cosmic::Application for App { dropdown_opt: None, inhibit_opt: None, value_tx_opt: None, + authenticating: false, + spinner_rotation: 0.0, + spinner_handle: None, }; let task = if cfg!(feature = "logind") { @@ -817,7 +897,7 @@ impl cosmic::Application for App { tracing::warn!("authentication error: {}", err); msg_tx .send(cosmic::Action::App(Message::Error( - err.to_string(), + pam_error_to_message(&err), ))) .await .unwrap(); @@ -941,16 +1021,41 @@ impl cosmic::Application for App { } } Message::Submit(value) => { - self.common.prompt_opt = None; self.common.error_opt = None; + self.authenticating = true; match self.value_tx_opt.take() { Some(value_tx) => { - // Clear errors - self.common.error_opt = None; - return cosmic::task::future(async move { - value_tx.send(value).await.unwrap(); - Message::Channel(value_tx) - }); + // Start spinner animation if not already running + if self.spinner_handle.is_none() { + let (spinner_task, handle) = cosmic::task::stream( + cosmic::iced_futures::stream::channel(1, |mut msg_tx| async move { + let mut interval = + tokio::time::interval(Duration::from_millis(16)); // ~60fps + loop { + msg_tx + .send(cosmic::Action::App(Message::SpinnerTick)) + .await + .unwrap(); + interval.tick().await; + } + }), + ) + .abortable(); + self.spinner_handle = Some(handle); + + return Task::batch([ + spinner_task, + cosmic::task::future(async move { + value_tx.send(value).await.unwrap(); + Message::Channel(value_tx) + }), + ]); + } else { + return cosmic::task::future(async move { + value_tx.send(value).await.unwrap(); + Message::Channel(value_tx) + }); + } } None => tracing::warn!("tried to submit when value_tx_opt not set"), } @@ -968,6 +1073,17 @@ impl cosmic::Application for App { } Message::Error(error) => { self.common.error_opt = Some(error); + self.authenticating = false; + + // Stop spinner animation + if let Some(handle) = self.spinner_handle.take() { + handle.abort(); + } + self.spinner_rotation = 0.0; + } + Message::SpinnerTick => { + // Update spinner rotation angle (360 degrees per second = 6 degrees per frame at 60fps) + self.spinner_rotation = (self.spinner_rotation + 6.0) % 360.0; } Message::Lock => match self.state { State::Unlocked => { @@ -977,6 +1093,12 @@ impl cosmic::Application for App { self.common.error_opt = None; // Clear value_tx self.value_tx_opt = None; + // Reset authenticating state + self.authenticating = false; + if let Some(handle) = self.spinner_handle.take() { + handle.abort(); + } + self.spinner_rotation = 0.0; // Try to create lockfile when locking if let Some(ref lockfile) = self.flags.lockfile_opt { if let Err(err) = fs::File::create(lockfile) { @@ -1002,6 +1124,14 @@ impl cosmic::Application for App { self.common.error_opt = None; // Clear value_tx self.value_tx_opt = None; + // Stop authenticating + self.authenticating = false; + + // Stop spinner animation + if let Some(handle) = self.spinner_handle.take() { + handle.abort(); + } + self.spinner_rotation = 0.0; // Try to delete lockfile when unlocking if let Some(ref lockfile) = self.flags.lockfile_opt { if let Err(err) = fs::remove_file(lockfile) {