Added Sso Login fixes #114

[alanpoon]

Fixes #114
Fixes #114
https://github.com/user-attachments/assets/f29eaee3-2a8f-4a74-8700-25ea41b64408

[kevinaboos]

Great start, thanks for implementing this!

I just left a few small comments.

One other thing (which you can save for the next follow-up PR) is that we actually need to first query which login types are supported rather than just assuming that a given homeserver supports all SSO providers that you've listed here.
Note that I already do this with the username+password login flow, but you'll need to use the same function to do so for SSO login.

Once you add that, you'll also see that it includes the provider's ID, name, icon, and brand ID, so at that point you won't need the images of each login provider's logo to be a part of the repo any more.

[alanpoon]

Great start, thanks for implementing this!

I just left a few small comments.

One other thing (which you can save for the next follow-up PR) is that we actually need to first query which login types are supported rather than just assuming that a given homeserver supports all SSO providers that you've listed here. Note that I already do this with the username+password login flow, but you'll need to use the same function to do so for SSO login.

Once you add that, you'll also see that it includes the provider's ID, name, icon, and brand ID, so at that point you won't need the images of each login provider's logo to be a part of the repo any more.

I can add the get_login_types but fetching the logo images needs authentication. I've checked element.io, they are serving the logo assets on their own server.

[kevinaboos]

Great start, thanks for implementing this!
I just left a few small comments.
One other thing (which you can save for the next follow-up PR) is that we actually need to first query which login types are supported rather than just assuming that a given homeserver supports all SSO providers that you've listed here. Note that I already do this with the username+password login flow, but you'll need to use the same function to do so for SSO login.
Once you add that, you'll also see that it includes the provider's ID, name, icon, and brand ID, so at that point you won't need the images of each login provider's logo to be a part of the repo any more.

I can add the get_login_types but fetching the logo images needs authentication. I've checked element.io, they are serving the logo assets on their own server.

Ah, very interesting. Good find, this is probably an oversight on their part due to the hasty (but necessary) migration to authenticated media that just happened last month. I will raise it with the matrix team.

In that case, let's proceed as you suggested by including the icons in the app but still using the proper functions to get the set of supported login types. 👍

[kevinaboos]

there are several places in the code that use unwrap(). We should not be panicking unless the error is completely unrecoverable, and none of the cases that use unwrap in this PR are unrecoverable. Most of them just exhibit generally improper usage of iterators; see my comments here.

[ZhangHanDong]

@alanpoon

Issue 113 requirements have not completed the following parts:

Username format support

• ⚠️ The code does not explicitly handle Matrix UserId format validation
• ⚠️ The logic of automatically supplementing the homeserver domain when only a partial UserId is entered is not implemented

Sliding Sync warning

• ❌ The function of displaying a sliding sync support warning when the user modifies the default homeserver is not implemented

CLI parameter support

• ⚠️ There is a structure definition for the AutofillInfo action, but the actual implementation is todo!()

Code refactoring suggestions：

• ⚠️ Currently, all login operations are concentrated in a single handle_action, which is not conducive to future maintenance. I suggest that you split it into smaller functions based on business logic, making the code structure clearer and easier to maintain. You can refer to the following code structure (do not copy and paste, use it only as a reference).
• ⚠️ The code in sliding_sync.rs also follows this approach. Please separate out the code snippets that can be independent methods with business semantics.

// Constants
static MATRIX_SIGN_UP_URL: &str = "https://matrix.org/docs/chat_basics/matrix-for-im/#creating-a-matrix-account";
const COLOR_DANGER_RED: Vec4 = Vec4 { x: 220f32/255f32, y: 0f32, z: 5f32/255f32, w: 1f32 };
const COLOR_ACCEPT_GREEN: Vec4 = Vec4 { x: 19f32/255f32, y: 136f32/255f32, z: 8f32/255f32, w: 1f32 };
const MESSAGE_TEXT_COLOR: Vec4 = Vec4 { x: 68f32/255f32, y: 68f32/255f32, z: 68f32/255f32, w: 1f32 };

// SSO Handler
#[derive(Debug)]
struct SsoHandler {
    buttons: Vec<(&'static str, &'static [LiveId])>,
    identity_providers: Vec<IdentityProvider>,
    pending: bool,
}

impl SsoHandler {
    fn new() -> Self {
        Self {
            buttons: vec![
                ("apple", ids!(apple_button)),
                ("facebook", ids!(facebook_button)),
                ("github", ids!(github_button)),
                ("gitlab", ids!(gitlab_button)),
                ("google", ids!(google_button)),
            ],
            identity_providers: Vec::new(),
            pending: false,
        }
    }

    fn update_providers(&mut self, providers: Vec<IdentityProvider>) {
        self.identity_providers = providers;
    }

    fn set_pending(&mut self, pending: bool) {
        self.pending = pending;
    }

    fn update_buttons(&self, cx: &mut Cx, view: &mut View) {
        for (brand, button_id) in &self.buttons {
            let button = view.view_get(button_id).borrow_mut();
            if let Some(mut button) = button {
                let visible = self.identity_providers
                    .iter()
                    .any(|ip| ip.id.contains(brand));
                
                button.set_visible(visible);
                button.apply_over(cx, live! {
                    cursor: if self.pending { NotAllowed } else { Hand },
                    image = {
                        draw_bg: { mask: if self.pending { 1.0 } else { 0.0 } }
                    }
                });
            }
        }
    }

    fn handle_button_clicks(&self, actions: &Actions, view: &View, homeserver: String) {
        if self.pending {
            return;
        }

        for (brand, button_id) in &self.buttons {
            if let Some(button) = view.view_get(button_id).borrow() {
                if button.finger_up(actions).is_some() {
                    if let Some(ip) = self.identity_providers
                        .iter()
                        .find(|ip| ip.id.contains(brand))
                    {
                        submit_async_request(MatrixRequest::SpawnSSOServer {
                            id: ip.id.clone(),
                            homeserver_url: homeserver,
                        });
                        break;
                    }
                }
            }
        }
    }
}

// Main Login Screen
#[derive(Live, LiveHook, Widget)]
pub struct LoginScreen {
    #[deref] view: View,
    #[rust] sso_handler: SsoHandler,
    #[rust] prev_homeserver_url: Option<String>,
}

impl Default for LoginScreen {
    fn default() -> Self {
        Self {
            view: View::default(),
            sso_handler: SsoHandler::new(),
            prev_homeserver_url: None,
        }
    }
}

impl Widget for LoginScreen {
    fn handle_event(&mut self, cx: &mut Cx, event: &Event, scope: &mut Scope) {
        self.view.handle_event(cx, event, scope);
        self.match_event(cx, event);
    }

    fn draw_walk(&mut self, cx: &mut Cx2d, scope: &mut Scope, walk: Walk) -> DrawStep {
        self.view.draw_walk(cx, scope, walk)
    }
}

impl MatchEvent for LoginScreen {
    fn handle_actions(&mut self, cx: &mut Cx, actions: &Actions) {
        // 获取UI元素引用
        let status_label = self.view.label(id!(status_label));
        let login_button = self.view.button(id!(login_button));
        let signup_button = self.view.button(id!(signup_button));
        let user_id_input = self.view.text_input(id!(user_id_input));
        let password_input = self.view.text_input(id!(password_input));
        let homeserver_input = self.view.text_input(id!(homeserver_input));
        let sso_search_button = self.view.button(id!(sso_search_button));

        // 处理注册按钮点击
        if signup_button.clicked(actions) {
            log!("Opening URL \"{}\"", MATRIX_SIGN_UP_URL);
            let _ = robius_open::Uri::new(MATRIX_SIGN_UP_URL).open();
        }

        // 处理登录按钮点击或输入框回车
        if login_button.clicked(actions) 
            || user_id_input.returned(actions).is_some()
            || password_input.returned(actions).is_some() 
            || homeserver_input.returned(actions).is_some()
        {
            self.handle_login_attempt(
                cx,
                &user_id_input,
                &password_input,
                &homeserver_input,
                &status_label
            );
        }

        // 处理各种登录相关的Action
        for action in actions {
            match action.downcast_ref() {
                Some(LoginAction::AutofillInfo { user_id, password, homeserver }) => {
                    user_id_input.set_text(user_id);
                    password_input.set_text(password);
                    if let Some(server) = homeserver {
                        homeserver_input.set_text(server);
                    }
                }
                Some(LoginAction::Status(status)) => {
                    status_label.set_text(status);
                    status_label.apply_over(cx, live!{
                        draw_text: { color: (MESSAGE_TEXT_COLOR) }
                    });
                    sso_search_button.set_enabled(true);
                }
                Some(LoginAction::LoginSuccess) => {
                    self.handle_login_success(cx, &user_id_input, &password_input, &homeserver_input, &status_label);
                }
                Some(LoginAction::LoginFailure(error)) => {
                    self.handle_login_failure(cx, error, &status_label);
                }
                Some(LoginAction::SsoPending(pending)) => {
                    self.sso_handler.set_pending(*pending);
                    self.sso_handler.update_buttons(cx, &mut self.view);
                }
                Some(LoginAction::IdentityProvider(providers)) => {
                    self.sso_handler.update_providers(providers.clone());
                    self.sso_handler.update_buttons(cx, &mut self.view);
                    sso_search_button.set_enabled(true);
                    status_label.set_text("");
                }
                _ => {}
            }
        }

        // 处理SSO搜索按钮
        if sso_search_button.clicked(actions) && self.prev_homeserver_url != Some(homeserver_input.text()) {
            self.handle_sso_search(cx, &homeserver_input, &status_label, &sso_search_button);
        }

        // 处理SSO按钮点击
        self.sso_handler.handle_button_clicks(actions, &self.view, homeserver_input.text());
    }
}

impl LoginScreen {
    fn handle_login_attempt(
        &self,
        cx: &mut Cx,
        user_id_input: &TextInput,
        password_input: &TextInput,
        homeserver_input: &TextInput,
        status_label: &Label,
    ) {
        let user_id = user_id_input.text();
        let password = password_input.text();
        let homeserver = homeserver_input.text();

        if user_id.is_empty() || password.is_empty() {
            status_label.apply_over(cx, live!{
                draw_text: { color: (COLOR_DANGER_RED) }
            });
            status_label.set_text("Please enter both User ID and Password.");
        } else {
            status_label.apply_over(cx, live!{
                draw_text: { color: (MESSAGE_TEXT_COLOR) }
            });
            status_label.set_text("Waiting for login response...");
            submit_async_request(MatrixRequest::Login(LoginRequest::LoginByPassword(
                LoginByPassword {
                    user_id,
                    password,
                    homeserver: homeserver.is_empty().not().then(|| homeserver),
                }
            )));
        }
    }

    fn handle_login_success(
        &self,
        cx: &mut Cx,
        user_id_input: &TextInput,
        password_input: &TextInput,
        homeserver_input: &TextInput,
        status_label: &Label,
    ) {
        user_id_input.set_text("");
        password_input.set_text("");
        homeserver_input.set_text("");
        status_label.set_text("Login successful!");
        status_label.apply_over(cx, live!{
            draw_text: { color: (COLOR_ACCEPT_GREEN) }
        });
    }

    fn handle_login_failure(&self, cx: &mut Cx, error: &str, status_label: &Label) {
        status_label.set_text(error);
        status_label.apply_over(cx, live!{
            draw_text: { color: (COLOR_DANGER_RED) }
        });
    }

    fn handle_sso_search(
        &mut self,
        cx: &mut Cx,
        homeserver_input: &TextInput,
        status_label: &Label,
        sso_search_button: &Button,
    ) {
        self.prev_homeserver_url = Some(homeserver_input.text());
        status_label.set_text("Fetching support login types from the homeserver...");
        submit_async_request(MatrixRequest::Login(
            LoginRequest::HomeserverLoginTypesQuery(homeserver_input.text())
        ));
        sso_search_button.set_enabled(false);
    }
}

#[derive(Clone, DefaultNone, Debug)]
pub enum LoginAction {
    LoginSuccess,
    LoginFailure(String),
    Status(String),
    AutofillInfo {
        user_id: String,
        password: String,
        homeserver: Option<String>,
    },
    SsoPending(bool),
    IdentityProvider(Vec<IdentityProvider>),
    None,
}

[CodiumAI-Agent]

Persistent review updated to latest commit 0a5a97b

[kevinaboos]

Code is looking great! Just a few small issues left, should be very quick to address.

One general thing: when you're copy-pasting code segments or comments, please ensure that the code comments are correct, up-to-date, and not misleading.

nit: you use vec![] pretty often; it'd be clearer and more canonical if you just use Vec::new() instead.

Also, you'll need to merge in the main branch and resolve merge conflicts once again, as there have been some major changes merged in recently.

[kevinaboos]

Nice job, everything looks good. Thanks!

[CodiumAI-Agent]

Persistent review updated to latest commit 6c2971b