diff --git a/client/android/client.go b/client/android/client.go index 1a8dd7d09c4..e171af5c965 100644 --- a/client/android/client.go +++ b/client/android/client.go @@ -76,6 +76,8 @@ type Client struct { connectClient *internal.ConnectClient config *profilemanager.Config cacheDir string + // Identifies the running profile for the SSO login hint; see profile_state.go. + cfgPath string stateChangeMu sync.Mutex stateChangeSubID string @@ -96,11 +98,12 @@ type Client struct { extendCancel context.CancelFunc } -func (c *Client) setState(cfg *profilemanager.Config, cacheDir string, cc *internal.ConnectClient) { +func (c *Client) setState(cfg *profilemanager.Config, cacheDir string, cfgPath string, cc *internal.ConnectClient) { c.stateMu.Lock() defer c.stateMu.Unlock() c.config = cfg c.cacheDir = cacheDir + c.cfgPath = cfgPath c.connectClient = cc } @@ -110,6 +113,16 @@ func (c *Client) stateSnapshot() (*profilemanager.Config, string, *internal.Conn return c.config, c.cacheDir, c.connectClient } +// authSnapshot returns the config together with the path it was loaded from, in +// one lock: the path identifies the profile whose account email backs the login +// hint, so reading it separately could pair one profile's config with another's +// hint when a profile switch lands in between. +func (c *Client) authSnapshot() (*profilemanager.Config, string, *internal.ConnectClient) { + c.stateMu.RLock() + defer c.stateMu.RUnlock() + return c.config, c.cfgPath, c.connectClient +} + func (c *Client) getConnectClient() *internal.ConnectClient { c.stateMu.RLock() defer c.stateMu.RUnlock() @@ -162,7 +175,7 @@ func (c *Client) Run(platformFiles PlatformFiles, urlOpener URLOpener, isAndroid defer c.ctxCancel() c.ctxCancelLock.Unlock() - auth := NewAuthWithConfig(ctx, cfg) + auth := NewAuthWithConfig(ctx, cfg, cfgFile) err = auth.login(urlOpener, isAndroidTV) if err != nil { return err @@ -170,7 +183,7 @@ func (c *Client) Run(platformFiles PlatformFiles, urlOpener URLOpener, isAndroid // todo do not throw error in case of cancelled context ctx = internal.CtxInitState(ctx) connectClient := internal.NewConnectClient(ctx, cfg, c.recorder) - c.setState(cfg, cacheDir, connectClient) + c.setState(cfg, cacheDir, cfgFile, connectClient) // This path runs the interactive SSO flow, so reaching here means the peer // is authenticated again — release the latch Status() reports from. Clear // only once the fresh connect client is installed: until then Status() @@ -211,7 +224,7 @@ func (c *Client) RunWithoutLogin(platformFiles PlatformFiles, dns *DNSList, dnsR // todo do not throw error in case of cancelled context ctx = internal.CtxInitState(ctx) connectClient := internal.NewConnectClient(ctx, cfg, c.recorder) - c.setState(cfg, cacheDir, connectClient) + c.setState(cfg, cacheDir, cfgFile, connectClient) return connectClient.RunOnAndroid(c.tunAdapter, c.iFaceDiscover, c.networkChangeListener, slices.Clone(dns.items), dnsReadyListener, stateFile, cacheDir) } diff --git a/client/android/login.go b/client/android/login.go index a9422cdbfde..f8576330e04 100644 --- a/client/android/login.go +++ b/client/android/login.go @@ -4,6 +4,8 @@ import ( "context" "fmt" + log "github.com/sirupsen/logrus" + "github.com/netbirdio/netbird/client/internal/auth" "github.com/netbirdio/netbird/client/internal/profilemanager" "github.com/netbirdio/netbird/client/system" @@ -53,11 +55,14 @@ func NewAuth(cfgPath string, mgmURL string) (*Auth, error) { }, nil } -// NewAuthWithConfig instantiate Auth based on existing config -func NewAuthWithConfig(ctx context.Context, config *profilemanager.Config) *Auth { +// NewAuthWithConfig instantiate Auth based on existing config. cfgPath is the +// file the config was loaded from; it identifies the profile whose account email +// backs the login_hint. +func NewAuthWithConfig(ctx context.Context, config *profilemanager.Config, cfgPath string) *Auth { return &Auth{ - ctx: ctx, - config: config, + ctx: ctx, + config: config, + cfgPath: cfgPath, } } @@ -150,12 +155,14 @@ func (a *Auth) login(urlOpener URLOpener, isAndroidTV bool) error { } jwtToken := "" + email := "" if needsLogin { tokenInfo, err := a.foregroundGetTokenInfo(authClient, urlOpener, isAndroidTV) if err != nil { return fmt.Errorf("interactive sso login failed: %v", err) } jwtToken = tokenInfo.GetTokenToUse() + email = tokenInfo.Email } err, _ = authClient.Login(a.ctx, "", jwtToken) @@ -163,17 +170,42 @@ func (a *Auth) login(urlOpener URLOpener, isAndroidTV bool) error { return fmt.Errorf("login failed: %v", err) } + // Stored after Login, not before: a rejected token must not leave a hint + // pointing at an account that cannot be used. + if email != "" && a.cfgPath != "" { + if err := writeProfileEmail(a.cfgPath, email); err != nil { + log.Warnf("failed to store profile account email: %v", err) + } + } + go urlOpener.OnLoginSuccess() return nil } +// loginHintSetter is implemented by both concrete flows (PKCE and device code) +// but absent from the OAuthFlow interface, hence the assertion below — the same +// way internal/auth wires it in authenticateWithPKCEFlow. +type loginHintSetter interface { + SetLoginHint(hint string) +} + func (a *Auth) foregroundGetTokenInfo(authClient *auth.Auth, urlOpener URLOpener, isAndroidTV bool) (*auth.TokenInfo, error) { oAuthFlow, err := authClient.GetOAuthFlow(a.ctx, isAndroidTV) if err != nil { return nil, fmt.Errorf("failed to get OAuth flow: %v", err) } + // An empty hint is deliberate, not a fallback: a fresh or logged-out profile + // leaves the choice to the IdP, which is how accounts get switched. + if a.cfgPath != "" { + if hint := readProfileEmail(a.cfgPath); hint != "" { + if setter, ok := oAuthFlow.(loginHintSetter); ok { + setter.SetLoginHint(hint) + } + } + } + flowInfo, err := oAuthFlow.RequestAuthInfo(context.TODO()) if err != nil { return nil, fmt.Errorf("getting a request OAuth flow info failed: %v", err) diff --git a/client/android/profile_manager.go b/client/android/profile_manager.go index 9a051137cc8..3197124d7a4 100644 --- a/client/android/profile_manager.go +++ b/client/android/profile_manager.go @@ -13,18 +13,17 @@ import ( ) const ( - // Android-specific config filename (different from desktop default.json) - defaultConfigFilename = "netbird.cfg" - // Subdirectory for non-default profiles (must match Java Preferences.java) - profilesSubdir = "profiles" // Android uses a single user context per app (non-empty username required by ServiceManager) androidUsername = "android" ) // Profile represents a profile for gomobile type Profile struct { - ID string - Name string + ID string + Name string + // Email is the account this profile last logged in with, "" if it never + // completed an SSO login or was logged out. See profile_state.go. + Email string IsActive bool } @@ -101,6 +100,7 @@ func (pm *ProfileManager) ListProfiles() (*ProfileArray, error) { profiles = append(profiles, &Profile{ ID: p.ID.String(), Name: p.Name, + Email: pm.profileEmail(p.ID.String()), IsActive: p.IsActive, }) } @@ -123,7 +123,22 @@ func (pm *ProfileManager) GetActiveProfile() (*Profile, error) { if err != nil { return nil, fmt.Errorf("failed to resolve active profile %q: %w", activeState.ID, err) } - return &Profile{ID: prof.ID.String(), Name: prof.Name, IsActive: true}, nil + return &Profile{ + ID: prof.ID.String(), + Name: prof.Name, + Email: pm.profileEmail(prof.ID.String()), + IsActive: true, + }, nil +} + +// profileEmail returns the account email recorded for a profile. Display-only, so +// an unresolvable path degrades to "" rather than an error. +func (pm *ProfileManager) profileEmail(id string) string { + configPath, err := pm.getProfileConfigPath(id) + if err != nil { + return "" + } + return readProfileEmail(configPath) } // SwitchProfile switches to a different profile @@ -185,6 +200,11 @@ func (pm *ProfileManager) LogoutProfile(id string) error { return fmt.Errorf("failed to save config: %w", err) } + // Not fatal: a stale hint costs an account switch, not the logout itself. + if err := removeProfileEmail(configPath); err != nil { + log.Warnf("failed to clear stored account email for profile %s: %v", id, err) + } + log.Infof("logged out from profile: %s", id) return nil } diff --git a/client/android/profile_state.go b/client/android/profile_state.go new file mode 100644 index 00000000000..3f0a0970143 --- /dev/null +++ b/client/android/profile_state.go @@ -0,0 +1,108 @@ +package android + +import ( + "context" + "fmt" + "os" + "path/filepath" + "strings" + + log "github.com/sirupsen/logrus" + + "github.com/netbirdio/netbird/client/internal/profilemanager" + "github.com/netbirdio/netbird/util" +) + +const ( + // Android-specific config filename (different from desktop default.json) + defaultConfigFilename = "netbird.cfg" + // Subdirectory for non-default profiles (must match Java Preferences.java) + profilesSubdir = "profiles" + // profileAccountSuffix names the file holding the profile's account email. + // Deliberately not ".state.json", which desktop uses for the same data: + // there the email and the engine's state manager live in different + // directories, but on Android both resolve under files/, so sharing the name + // would have the two overwrite each other — the state manager rewrites the + // whole file from its own keys (see statemanager.Manager.PersistState), and + // this package's writer does the same in reverse. + profileAccountSuffix = ".account.json" +) + +// profileAccountPathFor derives the account file path from a profile's config +// path: netbird.cfg -> netbird.account.json, .json -> .account.json. +// +// Deriving from the config path rather than resolving the active profile keeps +// the write on the profile the login actually ran for: Auth.login runs in a +// goroutine, so the active profile can change under a flow already in flight. +func profileAccountPathFor(configPath string) (string, error) { + if configPath == "" { + return "", fmt.Errorf("empty config path") + } + + base := filepath.Base(configPath) + stem := strings.TrimSuffix(base, filepath.Ext(base)) + if stem == "" || stem == "." { + return "", fmt.Errorf("config path %q has no filename stem", configPath) + } + + return filepath.Join(filepath.Dir(configPath), stem+profileAccountSuffix), nil +} + +// readProfileEmail returns the account email stored for the profile whose config +// lives at configPath. A missing or unreadable file yields "", which leaves the +// account choice to the IdP. +func readProfileEmail(configPath string) string { + accountPath, err := profileAccountPathFor(configPath) + if err != nil { + log.Debugf("no profile account path for login hint: %v", err) + return "" + } + + var state profilemanager.ProfileState + if _, err := util.ReadJson(accountPath, &state); err != nil { + if !os.IsNotExist(err) { + log.Debugf("failed to read profile account for login hint: %v", err) + } + return "" + } + + return state.Email +} + +// writeProfileEmail records the account email for the profile whose config lives +// at configPath, so later logins can pass it as an OIDC login_hint. An empty +// email is ignored rather than blanking what is already stored. +func writeProfileEmail(configPath string, email string) error { + if email == "" { + return nil + } + + accountPath, err := profileAccountPathFor(configPath) + if err != nil { + return fmt.Errorf("resolve profile account path: %w", err) + } + + state := profilemanager.ProfileState{Email: email} + if err := util.WriteJsonWithRestrictedPermission(context.Background(), accountPath, state); err != nil { + return fmt.Errorf("write profile account: %w", err) + } + + return nil +} + +// removeProfileEmail drops the stored account email. Called on logout: while the +// email is on disk it goes out as a login_hint, which would steer the next login +// straight back into the account just logged out of. Mirrors the desktop UI's +// RemoveProfileState call. +func removeProfileEmail(configPath string) error { + accountPath, err := profileAccountPathFor(configPath) + if err != nil { + return fmt.Errorf("resolve profile account path: %w", err) + } + + if err := os.Remove(accountPath); err != nil && !os.IsNotExist(err) { + return fmt.Errorf("remove profile account: %w", err) + } + + return nil +} diff --git a/client/android/profile_state_test.go b/client/android/profile_state_test.go new file mode 100644 index 00000000000..412435bc384 --- /dev/null +++ b/client/android/profile_state_test.go @@ -0,0 +1,161 @@ +package android + +import ( + "os" + "path/filepath" + "testing" +) + +func TestProfileAccountPathFor(t *testing.T) { + tests := []struct { + name string + configPath string + want string + wantErr bool + }{ + { + name: "default profile", + configPath: "/data/data/io.netbird.client/files/netbird.cfg", + want: "/data/data/io.netbird.client/files/netbird.account.json", + }, + { + name: "id profile", + configPath: "/data/data/io.netbird.client/files/profiles/4c5f5c8198c3989cffb5b5394f5a7ae0.json", + want: "/data/data/io.netbird.client/files/profiles/4c5f5c8198c3989cffb5b5394f5a7ae0.account.json", + }, + { + name: "legacy name-keyed profile is handled the same way", + configPath: "/data/data/io.netbird.client/files/profiles/work.json", + want: "/data/data/io.netbird.client/files/profiles/work.account.json", + }, + { + name: "empty path is rejected", + configPath: "", + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := profileAccountPathFor(tt.configPath) + if tt.wantErr { + if err == nil { + t.Fatalf("expected an error, got path %q", got) + } + return + } + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got != tt.want { + t.Errorf("got %q, want %q", got, tt.want) + } + }) + } +} + +func TestProfileAccountPathForDefaultDoesNotCollide(t *testing.T) { + root := "/data/data/io.netbird.client/files" + + defaultAccount, err := profileAccountPathFor(filepath.Join(root, defaultConfigFilename)) + if err != nil { + t.Fatalf("default profile: %v", err) + } + + idAccount, err := profileAccountPathFor(filepath.Join(root, profilesSubdir, "abc123.json")) + if err != nil { + t.Fatalf("id profile: %v", err) + } + + if defaultAccount == idAccount { + t.Fatalf("default and id profile share an account file: %q", defaultAccount) + } +} + +// The account file must never land on the engine state file: on Android both +// resolve under files/, and the state manager rewrites the whole file from its +// own keys, so sharing a path would have the two overwrite each other. The +// expected names here mirror ProfileManager.GetStateFilePath. +func TestProfileAccountPathAvoidsEngineStateFile(t *testing.T) { + root := "/data/data/io.netbird.client/files" + + cases := []struct { + configPath string + engineState string + }{ + { + configPath: filepath.Join(root, defaultConfigFilename), + engineState: filepath.Join(root, "state.json"), + }, + { + configPath: filepath.Join(root, profilesSubdir, "abc123.json"), + engineState: filepath.Join(root, profilesSubdir, "abc123.state.json"), + }, + } + + for _, c := range cases { + account, err := profileAccountPathFor(c.configPath) + if err != nil { + t.Fatalf("%s: %v", c.configPath, err) + } + if account == c.engineState { + t.Errorf("account file collides with the engine state file: %q", account) + } + } +} + +func TestWriteThenReadProfileEmail(t *testing.T) { + configPath := filepath.Join(t.TempDir(), "profiles", "abc123.json") + if err := ensureDirFor(t, configPath); err != nil { + t.Fatalf("prepare dir: %v", err) + } + + if got := readProfileEmail(configPath); got != "" { + t.Errorf("expected no email before a login, got %q", got) + } + + const email = "user@example.com" + if err := writeProfileEmail(configPath, email); err != nil { + t.Fatalf("write: %v", err) + } + + if got := readProfileEmail(configPath); got != email { + t.Errorf("got %q, want %q", got, email) + } + + if err := removeProfileEmail(configPath); err != nil { + t.Fatalf("remove: %v", err) + } + if got := readProfileEmail(configPath); got != "" { + t.Errorf("expected no email after logout, got %q", got) + } + + // Logout may run on a never-logged-in profile, so a second remove must pass. + if err := removeProfileEmail(configPath); err != nil { + t.Fatalf("second remove should be a no-op: %v", err) + } +} + +func TestWriteProfileEmailIgnoresEmpty(t *testing.T) { + configPath := filepath.Join(t.TempDir(), "profiles", "abc123.json") + if err := ensureDirFor(t, configPath); err != nil { + t.Fatalf("prepare dir: %v", err) + } + + const email = "user@example.com" + if err := writeProfileEmail(configPath, email); err != nil { + t.Fatalf("write: %v", err) + } + if err := writeProfileEmail(configPath, ""); err != nil { + t.Fatalf("write empty: %v", err) + } + + if got := readProfileEmail(configPath); got != email { + t.Errorf("empty write clobbered the stored email: got %q, want %q", got, email) + } +} + +func ensureDirFor(t *testing.T, path string) error { + t.Helper() + return os.MkdirAll(filepath.Dir(path), 0o700) +} diff --git a/client/android/session.go b/client/android/session.go index 961d5252859..d5da09c930d 100644 --- a/client/android/session.go +++ b/client/android/session.go @@ -278,7 +278,7 @@ func (c *Client) endExtend() { } func (c *Client) extendAuthSession(ctx context.Context, urlOpener URLOpener, isAndroidTV bool) error { - cfg, _, cc := c.stateSnapshot() + cfg, cfgPath, cc := c.authSnapshot() if cfg == nil || cc == nil { return fmt.Errorf("engine is not running") } @@ -293,7 +293,10 @@ func (c *Client) extendAuthSession(ctx context.Context, urlOpener URLOpener, isA } defer authClient.Close() - a := &Auth{ctx: ctx, config: cfg} + // Passing the config path makes the flow pick up the login_hint: an extend + // renews the session of the account already signed in, so it must not stop to + // offer a choice. + a := NewAuthWithConfig(ctx, cfg, cfgPath) tokenInfo, err := a.foregroundGetTokenInfo(authClient, urlOpener, isAndroidTV) if err != nil { return fmt.Errorf("interactive sso login failed: %v", err)