Skip to content

Commit 8caf9be

Browse files
refactor(register): improve code quality across registration flow
- Validate URL scheme in normalize_custom_homeserver (reject ftp://, file://, etc.) - Extract update_homeserver_option_buttons() to consolidate radio button state - Initialize selected_homeserver via LiveHook::after_new_from_doc instead of lazy init in handle_actions - Suppress redundant popup for TokenRequired errors (token input field is sufficient feedback) - Stop storing username/password in Cli struct; use RegisterRequest directly - Improve token error detection with structured UIAA error checking before falling back to string matching - Reduce spawn_sso_server complexity by extracting post_pending/post_status/ post_failure closures, eliminating 6 duplicated if-else branches
1 parent 5c835fe commit 8caf9be

3 files changed

Lines changed: 121 additions & 101 deletions

File tree

src/register/register_screen.rs

Lines changed: 38 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -409,7 +409,7 @@ live_design! {
409409
}
410410
}
411411

412-
#[derive(Live, LiveHook, Widget)]
412+
#[derive(Live, Widget)]
413413
pub struct RegisterScreen {
414414
#[deref]
415415
view: View,
@@ -427,6 +427,12 @@ pub struct RegisterScreen {
427427
registration_token_required: bool,
428428
}
429429

430+
impl LiveHook for RegisterScreen {
431+
fn after_new_from_doc(&mut self, _cx: &mut Cx) {
432+
self.selected_homeserver = "matrix.org".to_string();
433+
}
434+
}
435+
430436
impl RegisterScreen {
431437
fn toggle_homeserver_options(&mut self, cx: &mut Cx) {
432438
self.is_homeserver_editing = !self.is_homeserver_editing;
@@ -436,6 +442,20 @@ impl RegisterScreen {
436442
self.redraw(cx);
437443
}
438444

445+
/// Updates the radio-style homeserver option buttons to reflect the current selection.
446+
fn update_homeserver_option_buttons(&self, cx: &mut Cx) {
447+
let is_matrix_org = !self.pending_custom_homeserver;
448+
let matrix_btn = self.view.button(ids!(matrix_option));
449+
let other_btn = self.view.button(ids!(other_option));
450+
if is_matrix_org {
451+
matrix_btn.set_text(cx, "● matrix.org");
452+
other_btn.set_text(cx, "○ Other homeserver");
453+
} else {
454+
matrix_btn.set_text(cx, "○ matrix.org");
455+
other_btn.set_text(cx, "● Other homeserver");
456+
}
457+
}
458+
439459
fn show_warning(&self, message: &str) {
440460
use crate::shared::popup_list::{enqueue_popup_notification, PopupItem, PopupKind};
441461
enqueue_popup_notification(PopupItem {
@@ -521,10 +541,7 @@ impl RegisterScreen {
521541
.set_visible(cx, false);
522542

523543
// Reset homeserver option buttons
524-
let matrix_option_button = self.view.button(ids!(matrix_option));
525-
let other_option_button = self.view.button(ids!(other_option));
526-
matrix_option_button.set_text(cx, "● matrix.org");
527-
other_option_button.set_text(cx, "○ Other homeserver");
544+
self.update_homeserver_option_buttons(cx);
528545

529546
// Clear input fields
530547
self.view.text_input(ids!(username_input)).set_text(cx, "");
@@ -574,12 +591,6 @@ impl MatchEvent for RegisterScreen {
574591
let edit_button = self.view.button(ids!(edit_button));
575592
let sso_button = self.view.button(ids!(sso_button));
576593

577-
// Initialize selected_homeserver if empty
578-
if self.selected_homeserver.is_empty() {
579-
self.selected_homeserver = "matrix.org".to_string();
580-
self.pending_custom_homeserver = false;
581-
}
582-
583594
if login_button.clicked(actions) {
584595
cx.action(RegisterAction::NavigateToLogin);
585596
}
@@ -596,10 +607,10 @@ impl MatchEvent for RegisterScreen {
596607
self.update_button_mask(&sso_button, cx, 1.0);
597608

598609
// Show SSO registration modal immediately
599-
let status_label = self.view.label(ids!(status_modal_inner.status));
600-
status_label.set_text(cx, "Opening your browser...\n\nPlease complete registration in your browser, then return to Robrix.");
601-
let cancel_button = self.view.button(ids!(status_modal_inner.cancel_button));
602-
cancel_button.set_text(cx, "Cancel");
610+
self.view.label(ids!(status_modal_inner.status))
611+
.set_text(cx, "Opening your browser...\n\nPlease complete registration in your browser, then return to Robrix.");
612+
self.view.button(ids!(status_modal_inner.cancel_button))
613+
.set_text(cx, "Cancel");
603614
self.view.modal(ids!(status_modal)).open(cx);
604615
self.redraw(cx);
605616

@@ -629,8 +640,7 @@ impl MatchEvent for RegisterScreen {
629640
.set_visible(cx, false);
630641

631642
// Update button styles to show selection
632-
matrix_option_button.set_text(cx, "● matrix.org");
633-
other_option_button.set_text(cx, "○ Other homeserver");
643+
self.update_homeserver_option_buttons(cx);
634644

635645
self.is_homeserver_editing = false;
636646
self.view
@@ -650,8 +660,7 @@ impl MatchEvent for RegisterScreen {
650660
.set_text(cx, "Custom homeserver (required)");
651661

652662
// Update button styles to show selection
653-
matrix_option_button.set_text(cx, "○ matrix.org");
654-
other_option_button.set_text(cx, "● Other homeserver");
663+
self.update_homeserver_option_buttons(cx);
655664
self.update_registration_mode(cx);
656665
}
657666

@@ -748,12 +757,12 @@ impl MatchEvent for RegisterScreen {
748757
self.update_button_mask(&register_button, cx, 1.0);
749758

750759
// Show registration status modal with appropriate text for password registration
751-
let status_label = self.view.label(ids!(status_modal_inner.status));
752-
status_label.set_text(cx, "Registering account, please wait...");
753-
let title_label = self.view.label(ids!(status_modal_inner.title));
754-
title_label.set_text(cx, "Registration Status");
755-
let cancel_button = self.view.button(ids!(status_modal_inner.cancel_button));
756-
cancel_button.set_text(cx, "Cancel");
760+
self.view.label(ids!(status_modal_inner.status))
761+
.set_text(cx, "Registering account, please wait...");
762+
self.view.label(ids!(status_modal_inner.title))
763+
.set_text(cx, "Registration Status");
764+
self.view.button(ids!(status_modal_inner.cancel_button))
765+
.set_text(cx, "Cancel");
757766
self.view.modal(ids!(status_modal)).open(cx);
758767
self.redraw(cx);
759768

@@ -827,11 +836,10 @@ impl MatchEvent for RegisterScreen {
827836
RegisterAction::SsoRegistrationStatus { status } => {
828837
// Update SSO status in modal (only if our modal is already open)
829838
if self.sso_pending {
830-
let status_label = self.view.label(ids!(status_modal_inner.status));
831-
status_label.set_text(cx, status);
832-
let cancel_button =
833-
self.view.button(ids!(status_modal_inner.cancel_button));
834-
cancel_button.set_text(cx, "Cancel");
839+
self.view.label(ids!(status_modal_inner.status))
840+
.set_text(cx, status);
841+
self.view.button(ids!(status_modal_inner.cancel_button))
842+
.set_text(cx, "Cancel");
835843
self.redraw(cx);
836844
}
837845
}

src/register/validation.rs

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,8 @@ pub(super) fn normalize_custom_homeserver(raw: &str) -> Option<String> {
4242
format!("https://{trimmed}")
4343
};
4444

45-
Url::parse(&normalized).ok().map(|_| normalized)
45+
let url = Url::parse(&normalized).ok()?;
46+
matches!(url.scheme(), "http" | "https").then_some(normalized)
4647
}
4748

4849
pub(super) fn needs_custom_homeserver_input(
@@ -88,6 +89,16 @@ mod tests {
8889
);
8990
}
9091

92+
#[test]
93+
fn normalize_custom_homeserver_rejects_non_http_schemes() {
94+
assert_eq!(normalize_custom_homeserver("ftp://evil.com"), None);
95+
assert_eq!(normalize_custom_homeserver("file:///etc/passwd"), None);
96+
assert_eq!(normalize_custom_homeserver("javascript://alert(1)"), None);
97+
// http and https are accepted
98+
assert!(normalize_custom_homeserver("http://my-server.example").is_some());
99+
assert!(normalize_custom_homeserver("https://my-server.example").is_some());
100+
}
101+
91102
#[test]
92103
fn choosing_other_requires_explicit_custom_value_before_submit() {
93104
assert!(needs_custom_homeserver_input(true, "matrix.org"));

0 commit comments

Comments
 (0)