Skip to content

fix(gui-client): don't say "signed in" without a connlib session - #9477

Merged
thomaseizinger merged 5 commits into
mainfrom
fix/bad-login-state
Jun 9, 2025
Merged

thomaseizinger merged 5 commits into
mainfrom
fix/bad-login-state

Conversation

@thomaseizinger

@thomaseizinger thomaseizinger commented Jun 8, 2025

Copy link
Copy Markdown
Member

With the introduction of the "connect on start" configuration option, we introduced a bug where the GUI client said "Signed in as ..." even though we did not have a connlib session. The tray-menu handles this state correctly and clicking sign out and sign in restores Firezone to a functional state.

This disparity happened because we assumed that having a token means we must have a session.

To fix this, we introduce a new SessionViewModel that combines the state of the auth session and the connlib state. Only if we have both do we infer that we are "signed in". This also requires us to introduce an intermediary state where we are "loading". This is represented as a spinner in the UI.

Last but not least, this also removes the automated hiding of the client window. In a prior design, the only job of this window was to show the "Sign in" button so it wasn't useful beyond clicking that. Now that we show more things in this window, automatically hiding it might confuse the user.

Here is what this new design looks like:

Screencast.From.2025-06-08.11-09-03.webm

As a result of other improvements around "zero-click sign-in", the user often doesn't even have to switch to the browser window because sign-in happens in the background. Unfortunately, the tab still remains open but that is outside of our control (at least on Linux).

@thomaseizinger
thomaseizinger requested review from Copilot and jamilbk June 8, 2025 09:15
@vercel

vercel Bot commented Jun 8, 2025

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
firezone ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 9, 2025 9:36am

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes a GUI issue where the client would display “Signed in” without a valid connlib session by introducing a unified SessionViewModel (with a new loading state) and replacing the separate signed_in/signed_out events with a single session_changed event. It also removes automatic window hiding and updates the front end to render a spinner for the loading state.

  • Add SessionViewModel enum to represent signed-in, signed-out, and loading states.
  • Consolidate GUI integration calls to emit session_changed and refresh both tray menu and GUI via refresh_ui_state.
  • Refactor React components to consume SessionViewModel, display a spinner when loading, and remove auto-hide behavior.

Reviewed Changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
rust/gui-client/src-tauri/src/view.rs Introduce SessionViewModel enum with TSLink annotation
rust/gui-client/src-tauri/src/gui.rs Switch GUI integration to session_changed event
rust/gui-client/src-tauri/src/controller.rs Implement build_ui_state & refresh_ui_state, remove tray-only refresh
rust/gui-client/src-tauri/src/auth.rs Remove obsolete TSLink on old Session struct
rust/gui-client/src-tauri/Cargo.toml Configure TSLink enum representation as “discriminated”
rust/gui-client/src-frontend/main.tsx Extend theme with spinner styling
rust/gui-client/src-frontend/components/OverviewPage.tsx Refactor to render based on SessionViewModel (including spinner)
rust/gui-client/src-frontend/components/App.tsx Update event listener to session_changed, drop old listeners
Comments suppressed due to low confidence (3)

rust/gui-client/src-tauri/src/view.rs:24

  • Annotate SessionViewModel with a serde tag (e.g., #[serde(tag = "type", content = "data")]) so its JSON output matches the TSLink discriminated enum representation.
#[derive(Clone, serde::Serialize)]

rust/gui-client/src-tauri/src/controller.rs:905

  • Add unit tests for build_ui_state to verify each Status branch maps to the correct SessionViewModel and ConnlibState.
fn build_ui_state(&self) -> (system_tray::ConnlibState, SessionViewModel) {

rust/gui-client/src-tauri/src/view.rs:25

  • [nitpick] Add doc comments to SessionViewModel to explain what each variant represents for maintainability and clarity.
pub enum SessionViewModel {

Comment thread rust/gui-client/src-frontend/components/OverviewPage.tsx

@jamilbk jamilbk left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Sleek! Does the browser take focus when Sign in is clicked? We had an issue on macOS where the browser would not take focus and it caused a lot of confusion until fixed.

@thomaseizinger

Copy link
Copy Markdown
Member Author

Sleek! Does the browser take focus when Sign in is clicked? We had an issue on macOS where the browser would not take focus and it caused a lot of confusion until fixed.

I doesn't on Linux but I am not sure if I can force focus on it? It does show a notification though!

I'll test Windows tomorrow.

@thomaseizinger
thomaseizinger added this pull request to the merge queue Jun 9, 2025
Merged via the queue into main with commit 9210ed2 Jun 9, 2025
@thomaseizinger
thomaseizinger deleted the fix/bad-login-state branch June 9, 2025 09:48
@thomaseizinger

Copy link
Copy Markdown
Member Author

Sleek! Does the browser take focus when Sign in is clicked? We had an issue on macOS where the browser would not take focus and it caused a lot of confusion until fixed.

Yep, on Windows the focus switches to the browser window.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants