sendnrw/netbird
2
0
Fork 0
mirror of https://github.com/netbirdio/netbird.git synced 2026-08-28 18:41:30 +02:00
Code Issues Packages Projects Releases Wiki Activity

[client] Keep the account email backing the SSO login hint correct (#6986)

Browse Source 
## Describe your changes

Three fixes to how the desktop GUI keeps the account email that backs
the SSO `login_hint`. Each is independent and reviewable on its own.

**1. Store the email after a GUI SSO login**

The daemon returns the authenticated user's email from `WaitSSOLogin`
but cannot persist it: it runs as root while the per-profile state file
is user-owned. The CLI's `handleSSOLogin` writes it after its own
`WaitSSOLogin`; the GUI path read the value and dropped it.

The profile was therefore left with no email, so `Profiles.List` showed
no account for it, and later logins and session extends went out with no
`login_hint` — leaving the IdP to pick an account instead of reusing the
one the profile belongs to. Mirror the CLI and store it, next to the
`Logout` path that already clears the same file for the same reason.

**2. File the email against the profile the login ran for**

`SetActiveProfileState` resolves the target itself, so it writes to
whichever profile is active when it is called. A GUI SSO login spans
seconds of user interaction in the browser, and the tray stays clickable
throughout: switching profiles in that window left the email filed under
the profile that happened to be active when the flow returned. The wrong
profile then advertised an account it does not own, and offered it as
the `login_hint` next time.

Adds `SetProfileState(id, state)`, the write-side counterpart of the
existing `GetProfileState(id)`, and keeps `SetActiveProfileState` as a
wrapper for callers with no particular profile in mind. `Login` now
reports the profile it resolved so the frontend can hand it back with
the SSO wait, which closes the window.

**3. Delete the email when a profile is removed**

Removing a profile left its state file behind: the daemon deletes what
it owns, but the email file is user-owned and out of reach for a root
daemon — the same split that already puts the `Logout` cleanup on the UI
side.

Beyond the stray file, legacy profiles are keyed by name rather than by
a generated ID, so recreating a profile under a removed one's name
inherited its email — shown as the account in the profile list and sent
as the `login_hint` on the next login.

## 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

- **New Features**
- SSO login details can now be saved to the specific profile selected
during sign-in.
  - Profile state can be managed independently for different profiles.

- **Bug Fixes**
  - Removing a profile now also cleans up its associated saved state.
- Cleanup issues no longer prevent successful profile removal and are
handled gracefully.
  - Existing active-profile behavior remains unchanged.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
Zoltan Papp
2026-08-03 14:21:40 +02:00
committed by GitHub
parent f9b412228e
commit 2f721ec0d5
4 changed files with 96 additions and 17 deletions
Download Patch File Download Diff File Expand all files Collapse all files

38
client/internal/profilemanager/state.go
View File

@@ -45,12 +45,35 @@ func (pm *ProfileManager) GetProfileState(id ID) (*ProfileState, error) {
return &state, nil
}
func (pm *ProfileManager) SetActiveProfileState(state *ProfileState) error {
// SetProfileState writes the state file of the profile identified by id. Prefer
// it over SetActiveProfileState whenever the caller knows which profile the data
// belongs to: an SSO login spans seconds of user interaction, and the active
// profile can change during it, which would file the account email under
// whichever profile happened to be active when the flow returned.
func (pm *ProfileManager) SetProfileState(id ID, state *ProfileState) error {
configDir, err := getConfigDir()
if err != nil {
return fmt.Errorf("get config directory: %w", err)
}
if id == "" {
return fmt.Errorf("empty profile ID")
}
if id != defaultProfileName && !IsValidProfileFilenameStem(id) {
return fmt.Errorf("invalid profile ID: %q", id)
}
stateFile := filepath.Join(configDir, id.String()+".state.json")
if err := util.WriteJsonWithRestrictedPermission(context.Background(), stateFile, state); err != nil {
return fmt.Errorf("write profile state: %w", err)
}
return nil
}
// SetActiveProfileState writes the state file of whichever profile is active at
// call time. Use SetProfileState when the target profile is known.
func (pm *ProfileManager) SetActiveProfileState(state *ProfileState) error {
activeProf, err := pm.GetActiveProfile()
if err != nil {
if errors.Is(err, ErrNoActiveProfile) {
@@ -59,18 +82,7 @@ func (pm *ProfileManager) SetActiveProfileState(state *ProfileState) error {
return fmt.Errorf("get active profile: %w", err)
}
id := activeProf.ID
if id != defaultProfileName && !IsValidProfileFilenameStem(id) {
return fmt.Errorf("invalid active profile ID: %q", id)
}
stateFile := filepath.Join(configDir, id.String()+".state.json")
err = util.WriteJsonWithRestrictedPermission(context.Background(), stateFile, state)
if err != nil {
return fmt.Errorf("write profile state: %w", err)
}
return nil
return pm.SetProfileState(activeProf.ID, state)
}
// RemoveProfileState deletes the per-profile state file (which holds the

9
client/ui/frontend/src/lib/connection.ts
View File

@@ -43,7 +43,12 @@ function buildSsoCancelPromise(state: SsoState, signal?: AbortSignal): Promise<v
}
async function runSsoLogin(
result: { verificationUri: string; verificationUriComplete: string; userCode: string },
result: {
verificationUri: string;
verificationUriComplete: string;
userCode: string;
profileId: string;
},
state: SsoState,
signal?: AbortSignal,
): Promise<void> {
@@ -56,7 +61,7 @@ async function runSsoLogin(
// suspended, so a frontend-driven Up (a promise continuation) would not
// fire until the user woke the window (e.g. hovering the tray icon).
const waitPromise = Connection.WaitSSOLoginAndUp(
{ userCode: result.userCode, hostname: "" },
{ userCode: result.userCode, hostname: "", profileId: result.profileId },
{ profileName: "", username: "" },
);

40
client/ui/services/connection.go
View File

@@ -33,12 +33,21 @@ type LoginResult struct {
UserCode string `json:"userCode"`
VerificationURI string `json:"verificationUri"`
VerificationURIComplete string `json:"verificationUriComplete"`
// ProfileID is the ID of the profile this login ran against, or "" when the
// caller named the profile itself and no ID was resolved. Pass it back in
// WaitSSOParams so the account email lands on this profile even if the
// active one changes during SSO.
ProfileID string `json:"profileId"`
}
// WaitSSOParams are the inputs to waitSSOLogin.
type WaitSSOParams struct {
UserCode string `json:"userCode"`
Hostname string `json:"hostname"`
// ProfileID is the profile the login was started for, used to file the
// account email against it rather than against whichever profile is active
// when the flow returns. Optional: empty falls back to the active profile.
ProfileID string `json:"profileId"`
}
// UpParams selects the profile to bring up.
@@ -77,11 +86,16 @@ func (s *Connection) Login(ctx context.Context, p LoginParams) (LoginResult, err
// Fall back to the daemon's active profile and the current OS user.
profileName := p.ProfileName
username := p.Username
// Only set when the daemon told us the ID. A caller-supplied ProfileName is
// a handle — a display name or an ID prefix resolve too — and the state file
// is named after the ID, so passing a handle on would name the wrong file.
profileID := ""
if profileName == "" {
if active, aerr := cli.GetActiveProfile(ctx, &proto.GetActiveProfileRequest{}); aerr == nil {
// Address the active profile by ID (the daemon resolves it as a
// handle); names can collide, the ID cannot.
profileName = active.GetId()
profileID = profileName
if username == "" {
username = active.GetUsername()
}
@@ -122,6 +136,7 @@ func (s *Connection) Login(ctx context.Context, p LoginParams) (LoginResult, err
UserCode: resp.GetUserCode(),
VerificationURI: resp.GetVerificationURI(),
VerificationURIComplete: resp.GetVerificationURIComplete(),
ProfileID: profileID,
}, nil
}
@@ -242,6 +257,31 @@ func (s *Connection) waitSSOLogin(ctx context.Context, p WaitSSOParams) (string,
return "", s.classifyDaemonError(err)
}
log.Infof("SSO login completed, daemon reported success")
// Persist the account email the same way the CLI does after its own
// WaitSSOLogin: the daemon returns it but cannot store it, since it runs as
// root and the per-profile state file is user-owned (see Logout below).
// Without this the profile has no email, so Profiles.List shows no account
// and later logins and session extends go out without a login_hint —
// leaving the IdP to guess which account was meant.
if email := resp.GetEmail(); email != "" {
state := &profilemanager.ProfileState{Email: email}
pm := profilemanager.NewProfileManager()
// Against the profile the login was started for: SSO spans seconds of
// user interaction, and a profile switch in that window would otherwise
// file the email under the wrong profile.
if p.ProfileID != "" {
err = pm.SetProfileState(profilemanager.ID(p.ProfileID), state)
} else {
err = pm.SetActiveProfileState(state)
}
if err != nil {
// Non-fatal: the login itself succeeded.
log.Warnf("failed to store account email: %v", err)
}
}
return resp.GetEmail(), nil
}

26
client/ui/services/profile.go
View File

@@ -6,6 +6,8 @@ import (
"context"
"os/user"
log "github.com/sirupsen/logrus"
"github.com/netbirdio/netbird/client/internal/profilemanager"
"github.com/netbirdio/netbird/client/proto"
)
@@ -151,11 +153,31 @@ func (s *Profiles) Remove(ctx context.Context, p ProfileRef) error {
if err != nil {
return err
}
_, err = cli.RemoveProfile(ctx, &proto.RemoveProfileRequest{
resp, err := cli.RemoveProfile(ctx, &proto.RemoveProfileRequest{
ProfileName: p.ProfileName,
Username: p.Username,
})
return err
if err != nil {
return err
}
// The daemon deletes what it owns but runs as root, so it leaves the
// user-owned state file holding the account email behind (same split as
// Connection.Logout). Legacy profiles are keyed by name rather than by a
// generated ID, so a recreated profile of the same name would inherit the
// deleted one's email and offer it as the login_hint.
//
// Keyed on the ID the daemon resolved, not on the request handle: that may
// have been a display name or an ID prefix, which would name a different
// file (or none).
if id := resp.GetId(); id != "" {
if err := profilemanager.NewProfileManager().RemoveProfileState(id); err != nil {
// Non-fatal: the profile itself is gone.
log.Warnf("failed to remove profile state for %s: %v", id, err)
}
}
return nil
}
// Rename changes a profile's display name. The on-disk ID is unaffected, so