mirror of
https://github.com/netbirdio/netbird.git
synced 2026-09-04 22:11:29 +02:00
[client] Fix browser dialog not closing on renew session flow (#6745)
## Describe your changes ## Issue ticket number and link ## Stack <!-- branch-stack --> ### Checklist - [x] Is it a bug fix - [ ] Is a typo/documentation fix - [ ] Is a feature enhancement - [ ] It is a refactor - [ ] Created tests that fail without the change (if possible) - [ ] This change does **not** modify the public API, gRPC protocols, functionality behavior, CLI / service flags, or introduce a new feature — **OR** I have discussed it with the NetBird team beforehand (link the issue / Slack thread in the description). See [CONTRIBUTING.md](https://github.com/netbirdio/netbird/blob/main/CONTRIBUTING.md#discuss-changes-with-the-netbird-team-first). > By submitting this pull request, you confirm that you have read and agree to the terms of the [Contributor License Agreement](https://github.com/netbirdio/netbird/blob/main/CONTRIBUTOR_LICENSE_AGREEMENT.md). ## Documentation Select exactly one: - [ ] I added/updated documentation for this change - [x] Documentation is **not needed** for this change (explain why) ### Docs PR URL (required if "docs added" is checked) Paste the PR link from https://github.com/netbirdio/docs here: https://github.com/netbirdio/docs/pull/__ <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * SSO browser-login popups now open centered on the display where the cursor is located, and they recenter correctly on subsequent opens. * Programmatic cleanup no longer triggers “login canceled” behavior; cancel is emitted only when the user closes the active popup. * **New Features** * Added a streamlined “close renewal flow” action that tears down the session-renewal UI by closing both the login and session-expiration popups. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -73,6 +73,13 @@ export default function SessionExpirationDialog() {
|
||||
|
||||
let offCancel: (() => void) | undefined;
|
||||
|
||||
// Return the dialog to its interactive state and dismiss the browser popup
|
||||
const resetDialog = () => {
|
||||
offCancel?.();
|
||||
WindowManager.CloseBrowserLogin().catch(console.error);
|
||||
setBusy(false);
|
||||
};
|
||||
|
||||
try {
|
||||
const start = await Session.RequestExtend({ hint: "" });
|
||||
const uri = start.verificationUriComplete || start.verificationUri;
|
||||
@@ -105,25 +112,22 @@ export default function SessionExpirationDialog() {
|
||||
if (outcome.kind === "cancel") {
|
||||
waitPromise.cancel?.();
|
||||
waitPromise.catch(() => {});
|
||||
resetDialog();
|
||||
return;
|
||||
}
|
||||
|
||||
// Another surface owns this flow; keep the dialog open to retry.
|
||||
if (outcome.result.preempted) {
|
||||
resetDialog();
|
||||
return;
|
||||
}
|
||||
|
||||
// Close before the popup so the restore can't flash this window back.
|
||||
WindowManager.CloseSessionExpiration().catch(console.error);
|
||||
WindowManager.CloseRenewFlow().catch(console.error);
|
||||
} catch (e) {
|
||||
resetDialog();
|
||||
await errorDialog({
|
||||
Title: t("sessionExpiration.extendFailedTitle"),
|
||||
Message: formatErrorMessage(e),
|
||||
});
|
||||
} finally {
|
||||
offCancel?.();
|
||||
WindowManager.CloseBrowserLogin().catch(console.error);
|
||||
setBusy(false);
|
||||
}
|
||||
}, [busy, t]);
|
||||
|
||||
@@ -139,12 +143,11 @@ export default function SessionExpirationDialog() {
|
||||
});
|
||||
WindowManager.CloseSessionExpiration().catch(console.error);
|
||||
} catch (e) {
|
||||
setBusy(false);
|
||||
await errorDialog({
|
||||
Title: t("sessionExpiration.logoutFailedTitle"),
|
||||
Message: formatErrorMessage(e),
|
||||
});
|
||||
} finally {
|
||||
setBusy(false);
|
||||
}
|
||||
}, [busy, t]);
|
||||
|
||||
|
||||
@@ -185,37 +185,38 @@ func (s *WindowManager) OpenBrowserLogin(uri string) {
|
||||
startURL = "/#/dialog/browser-login?uri=" + url.QueryEscape(uri)
|
||||
}
|
||||
s.hideOtherWindowsLocked("browser-login")
|
||||
// Prefer the main window's screen (multi-monitor); falls back to OS-default centering.
|
||||
var screen *application.Screen
|
||||
if s.mainWindow != nil {
|
||||
if sc, err := s.mainWindow.GetScreen(); err == nil {
|
||||
screen = sc
|
||||
}
|
||||
}
|
||||
opts := DialogWindowOptions("browser-login", s.title("window.title.signIn"), startURL, s.linuxIcon)
|
||||
// Not always-on-top: it would obscure the browser tab the user logs in through.
|
||||
opts.AlwaysOnTop = false
|
||||
opts.InitialPosition = application.WindowCentered
|
||||
opts.Screen = screen
|
||||
// Open on the active (where users cursor is) display, like the session-expiration dialog.
|
||||
opts.Screen = s.getScreenBasedOnCursorPosition()
|
||||
s.browserLogin = s.app.Window.NewWithOptions(opts)
|
||||
bl := s.browserLogin
|
||||
// Red-X close means cancel: emit the event so startLogin() tears down the SSO wait.
|
||||
bl.OnWindowEvent(events.Common.WindowClosing, func(_ *application.WindowEvent) {
|
||||
s.app.Event.Emit(EventBrowserLoginCancel)
|
||||
s.mu.Lock()
|
||||
s.browserLogin = nil
|
||||
s.restoreHiddenWindowsLocked()
|
||||
// Only a live user red-X still has this registered; programmatic closers
|
||||
// nil s.browserLogin first and clean up themselves. Guarding here stops a
|
||||
// stale close event from wiping a replacement popup's state.
|
||||
userClosed := s.browserLogin == bl
|
||||
if userClosed {
|
||||
s.browserLogin = nil
|
||||
s.restoreHiddenWindowsLocked()
|
||||
}
|
||||
s.mu.Unlock()
|
||||
if userClosed {
|
||||
s.app.Event.Emit(EventBrowserLoginCancel)
|
||||
}
|
||||
})
|
||||
s.centerWhenReady(s.browserLogin)
|
||||
s.centerOnCursorScreen(s.browserLogin)
|
||||
return
|
||||
}
|
||||
if uri != "" {
|
||||
s.browserLogin.SetURL("/#/dialog/browser-login?uri=" + url.QueryEscape(uri))
|
||||
}
|
||||
s.centerOnCursorScreen(s.browserLogin)
|
||||
s.browserLogin.Show()
|
||||
s.browserLogin.Focus()
|
||||
s.centerWhenReady(s.browserLogin)
|
||||
}
|
||||
|
||||
// BrowserLoginWindow returns the live SSO popup, or nil. While non-nil it is the
|
||||
@@ -238,6 +239,15 @@ func (s *WindowManager) CloseBrowserLogin() {
|
||||
s.mu.Lock()
|
||||
w := s.browserLogin
|
||||
s.browserLogin = nil
|
||||
// The WindowClosing hook no-ops on a programmatic close, so restore here —
|
||||
// but only if a popup was actually open. The frontend calls this even when no
|
||||
// popup was ever shown (e.g. resetDialog() after an early RequestExtend failure,
|
||||
// or connection.ts's catch path), and hiddenForLogin is shared with
|
||||
// OpenInstallProgress, so an unconditional restore could re-show windows a
|
||||
// still-running install-progress is hiding.
|
||||
if w != nil {
|
||||
s.restoreHiddenWindowsLocked()
|
||||
}
|
||||
s.mu.Unlock()
|
||||
if w != nil {
|
||||
w.Close()
|
||||
@@ -279,6 +289,35 @@ func (s *WindowManager) CloseSessionExpiration() {
|
||||
}
|
||||
}
|
||||
|
||||
// CloseRenewFlow tears down the SSO session-renewal UI in a single call: it
|
||||
// closes the browser-login popup and the session-expiration window together.
|
||||
func (s *WindowManager) CloseRenewFlow() {
|
||||
s.mu.Lock()
|
||||
bl := s.browserLogin
|
||||
se := s.sessionExpiration
|
||||
s.browserLogin = nil
|
||||
s.sessionExpiration = nil
|
||||
if se != nil {
|
||||
kept := s.hiddenForLogin[:0]
|
||||
for _, w := range s.hiddenForLogin {
|
||||
if w != se {
|
||||
kept = append(kept, w)
|
||||
}
|
||||
}
|
||||
s.hiddenForLogin = kept
|
||||
}
|
||||
s.restoreHiddenWindowsLocked()
|
||||
s.mu.Unlock()
|
||||
|
||||
// Close after unlock so the re-entrant handlers can take s.mu.
|
||||
if bl != nil {
|
||||
bl.Close()
|
||||
}
|
||||
if se != nil {
|
||||
se.Close()
|
||||
}
|
||||
}
|
||||
|
||||
// OpenInstallProgress shows the install-progress window and hides the rest for the duration
|
||||
// (restored on close). It owns its own result polling since the daemon restarts mid-install.
|
||||
func (s *WindowManager) OpenInstallProgress(version string) {
|
||||
|
||||
Reference in New Issue
Block a user