diff --git a/client/internal/auth/device_flow.go b/client/internal/auth/device_flow.go index 9dec7cf53..3592e589d 100644 --- a/client/internal/auth/device_flow.go +++ b/client/internal/auth/device_flow.go @@ -304,6 +304,16 @@ func (d *DeviceAuthorizationFlow) WaitToken(ctx context.Context, info AuthFlowIn return TokenInfo{}, fmt.Errorf("validate access token failed with error: %v", err) } + // Same as the PKCE flow: the account the token belongs to is what + // callers store to send back as the login_hint. Without it a client + // driven through the device flow — Android TV and tvOS — never binds + // an account to its profile and every later login goes out blind. + if email, err := parseEmailFromIDToken(tokenInfo.IDToken); err != nil { + log.Warnf("failed to parse email from ID token: %v", err) + } else { + tokenInfo.Email = email + } + log.Infof("device flow: user authorization confirmed after %d polls in %s", polls, time.Since(start).Round(time.Second)) return tokenInfo, err } diff --git a/client/ios/NetBirdSDK/login.go b/client/ios/NetBirdSDK/login.go index 42a575359..cf7aa6730 100644 --- a/client/ios/NetBirdSDK/login.go +++ b/client/ios/NetBirdSDK/login.go @@ -11,6 +11,7 @@ import ( "github.com/netbirdio/netbird/client/internal/auth" "github.com/netbirdio/netbird/client/internal/profilemanager" + "github.com/netbirdio/netbird/client/mobile" "github.com/netbirdio/netbird/client/system" ) @@ -284,12 +285,14 @@ func (a *Auth) login(urlOpener URLOpener, forceDeviceAuth bool, deviceName strin } jwtToken := "" + email := "" if needsLogin { tokenInfo, err := a.foregroundGetTokenInfo(authClient, urlOpener, forceDeviceAuth) if err != nil { return fmt.Errorf("interactive sso login failed: %v", err) } jwtToken = tokenInfo.GetTokenToUse() + email = tokenInfo.Email } err, isAuthError := authClient.Login(ctx, "", jwtToken) @@ -301,6 +304,14 @@ func (a *Auth) login(urlOpener URLOpener, forceDeviceAuth bool, deviceName strin 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 := mobile.WriteProfileEmail(a.cfgPath, email); err != nil { + log.Warnf("failed to store profile account email: %v", err) + } + } + // Save the config before notifying success to ensure persistence completes // before the callback potentially triggers teardown on the Swift side. // Note: This differs from Android which doesn't save config after login. @@ -320,10 +331,24 @@ func (a *Auth) login(urlOpener URLOpener, forceDeviceAuth bool, deviceName strin return nil } +// profileLoginHint returns the stored account email for the profile at cfgPath, +// so a re-login targets the account the profile already belongs to instead of +// whatever session the shared browser cookie jar happens to hold. +// +// An empty hint is deliberate, not a fallback: a fresh profile leaves the +// choice to the IdP. Switching accounts is done by switching or removing +// profiles, not by logging out — logout keeps the email. +func profileLoginHint(cfgPath string) string { + if cfgPath == "" { + return "" + } + return mobile.ReadProfileEmail(cfgPath) +} + const authInfoRequestTimeout = 30 * time.Second func (a *Auth) foregroundGetTokenInfo(authClient *auth.Auth, urlOpener URLOpener, forceDeviceAuth bool) (*auth.TokenInfo, error) { - oAuthFlow, err := authClient.GetOAuthFlow(a.ctx, forceDeviceAuth, "") + oAuthFlow, err := authClient.GetOAuthFlow(a.ctx, forceDeviceAuth, profileLoginHint(a.cfgPath)) if err != nil { return nil, fmt.Errorf("failed to get OAuth flow: %v", err) } diff --git a/client/mobile/profile_state.go b/client/mobile/profile_state.go index bb983ec1d..ad05801f8 100644 --- a/client/mobile/profile_state.go +++ b/client/mobile/profile_state.go @@ -78,8 +78,14 @@ func WriteProfileEmail(configPath string, email string) error { return fmt.Errorf("resolve profile account path: %w", err) } + // DirectWriteJson, not the atomic writers: those create a temp file and + // rename it over the target, which the tvOS App Group sandbox blocks. It is + // the same reason the config next to this file goes through + // DirectWriteOutConfig. The file is rewritten whole from one key, so losing + // atomicity costs nothing beyond a torn write on a crash mid-write, which + // reads back as "no email" and is recovered by the next login. state := profilemanager.ProfileState{Email: email} - if err := util.WriteJsonWithRestrictedPermission(context.Background(), accountPath, state); err != nil { + if err := util.DirectWriteJson(context.Background(), accountPath, state); err != nil { return fmt.Errorf("write profile account: %w", err) } diff --git a/util/file.go b/util/file.go index 926904f9f..52eb91c0f 100644 --- a/util/file.go +++ b/util/file.go @@ -56,9 +56,9 @@ func WriteJson(ctx context.Context, file string, obj interface{}) error { } // DirectWriteJson writes JSON config object to a file creating parent directories if required without creating a temporary file -func DirectWriteJson(ctx context.Context, file string, obj interface{}) error { +func DirectWriteJson(ctx context.Context, file string, obj interface{}) (err error) { - _, _, err := prepareConfigFileDir(file) + _, _, err = prepareConfigFileDir(file) if err != nil { return err } @@ -68,11 +68,24 @@ func DirectWriteJson(ctx context.Context, file string, obj interface{}) error { return err } + // Named return so a failed Close is reported rather than logged and + // swallowed: the write is only durable once the file closes cleanly, and a + // caller told "written" would carry on with data that never landed. defer func() { - err = targetFile.Close() - if err != nil { - log.Errorf("failed to close file %s: %v", file, err) + cerr := targetFile.Close() + if cerr == nil { + return } + if err == nil { + // Returned, not logged: the caller reports it once. + err = cerr + return + } + // The body already failed and that error is the one the caller gets, so + // it is the one that explains the failure. This is then the only place + // the close failure can surface — at debug, per the logging rules for + // close errors on writes. + log.Debugf("failed to close file %s after %v: %v", file, err, cerr) }() // make it pretty