diff --git a/account/client.go b/account/client.go index c8aff73a..72a192d7 100644 --- a/account/client.go +++ b/account/client.go @@ -38,7 +38,9 @@ type Client struct { salt []byte saltPath string - mu sync.RWMutex + // generation advances whenever the active account may change; guarded by mu. + generation uint64 + mu sync.RWMutex } // NewClient returns a Client that caches its salt under dataDir. A nil logger @@ -72,6 +74,25 @@ func (a *Client) setSalt(salt []byte) { a.salt = salt } +func (a *Client) storeSalt(salt []byte) error { + a.mu.Lock() + defer a.mu.Unlock() + return a.storeSaltLocked(salt) +} + +// storeSaltLocked caches salt before persisting it, so a failed write never leaves a stale salt +// cached. It requires a.mu to be held. +func (a *Client) storeSaltLocked(salt []byte) error { + a.salt = salt + return writeSalt(salt, a.saltPath) +} + +func (a *Client) identityGeneration() uint64 { + a.mu.RLock() + defer a.mu.RUnlock() + return a.generation +} + func (a *Client) proBaseURL() string { if a.proURL != "" { return a.proURL @@ -177,7 +198,8 @@ func (a *Client) sendRequest( // sendProRequest sends a request to the Pro server, automatically adding the required headers, // including the device ID, user ID, and Pro token from settings, if available. If the URL is relative, -// the Pro server base URL will be prepended. +// the Pro server base URL will be prepended. A 2xx response carrying a non-empty error envelope +// is returned as an error. func (a *Client) sendProRequest( ctx context.Context, method, url string, @@ -197,7 +219,17 @@ func (a *Client) sendProRequest( headers[common.UserIDHeader] = uid } maps.Copy(headers, additionalheaders) - return a.sendRequest(ctx, method, url, queryParams, headers, body) + resp, err := a.sendRequest(ctx, method, url, queryParams, headers, body) + if err != nil { + return nil, err + } + var envelope struct { + Error string `json:"error"` + } + if json.Unmarshal(resp, &envelope) == nil && envelope.Error != "" { + return nil, fmt.Errorf("received bad response: %s", envelope.Error) + } + return resp, nil } // curlFromRequest renders req as a curl command, replacing req.Body with an diff --git a/account/client_test.go b/account/client_test.go index 2212daa6..a48d7827 100644 --- a/account/client_test.go +++ b/account/client_test.go @@ -45,3 +45,13 @@ func TestErrorResponseBodyOmittedFromLog(t *testing.T) { assert.Contains(t, buf.String(), "status=400", "error log dropped the status") assert.Contains(t, err.Error(), "invalid_code", "returned error dropped the server error code") } + +func TestProErrorEnvelope(t *testing.T) { + a, _ := newTestClient(t) + useProServer(t, a, serveJSON(map[string]string{"error": "device not found"})) + + _, err := a.RemoveDevice(context.Background(), "device") + assert.ErrorContains(t, err, "device not found") + _, err = a.ActivationCode(context.Background(), "me@example.com", "code") + assert.ErrorContains(t, err, "device not found") +} diff --git a/account/subscription.go b/account/subscription.go index ca2af2f6..4bc61c31 100644 --- a/account/subscription.go +++ b/account/subscription.go @@ -108,11 +108,6 @@ func (a *Client) SubscriptionPlans(ctx context.Context, channel string) (string, if err := json.Unmarshal(resp, &plans); err != nil { return "", traces.RecordError(ctx, fmt.Errorf("unmarshaling plans response: %w", err)) } - if plans.BaseResponse != nil && plans.Error != "" { - err = fmt.Errorf("received bad response: %s", plans.Error) - a.logger.Error("retrieving plans", "error", err) - return "", traces.RecordError(ctx, err) - } return string(resp), nil } @@ -146,6 +141,9 @@ type VerifySubscriptionResponse struct { // should contain the information required by service to verify the subscription, such as the // purchase token for Google Play or the receipt for Apple. The status and subscription ID are returned // along with any error that occurred during the verification process. +// +// When the purchase belongs to a different account, the client switches to that account before +// returning, unless the active account may have changed while the request was in flight. func (a *Client) VerifySubscription(ctx context.Context, service SubscriptionService, data map[string]string) (string, error) { ctx, span := otel.Tracer(tracerName).Start(ctx, "verify_subscription") defer span.End() @@ -161,13 +159,46 @@ func (a *Client) VerifySubscription(ctx context.Context, service SubscriptionSer return "", traces.RecordError(ctx, fmt.Errorf("unsupported service: %s", service)) } + generation := a.identityGeneration() resp, err := a.sendProRequest(ctx, "POST", path, nil, nil, data) if err != nil { a.logger.Error("verifying subscription", "error", err) return "", traces.RecordError(ctx, fmt.Errorf("verifying subscription: %w", err)) } + var result VerifySubscriptionResponse + if err := json.Unmarshal(resp, &result); err != nil { + a.logger.Error("unmarshaling verify subscription response", "error", err) + } else { + a.switchToPurchaser(ctx, generation, result.ActualUserID, result.ActualUserToken) + } return string(resp), nil +} +// switchToPurchaser switches to the purchasing account unless the account may have changed since +// generation was captured. It logs rather than returns failures, because the purchase already succeeded. +func (a *Client) switchToPurchaser(ctx context.Context, generation uint64, id int64, token string) { + if id == 0 || token == "" { + return + } + a.mu.Lock() + defer a.mu.Unlock() + if a.generation != generation { + a.logger.Warn("not switching to the purchasing account", "error", traces.RecordError(ctx, ErrAccountChanged)) + return + } + switching := id != settings.GetInt64(settings.UserIDKey) + if !switching && token == settings.GetString(settings.TokenKey) { + return + } + a.generation++ + var extra settings.Settings + if switching { + // The previous account's OAuth sign-in doesn't carry over to the purchaser. + extra = settings.Settings{settings.OAuthLoginKey: false, settings.OAuthProviderKey: ""} + } + if err := a.storeIdentityLocked(id, token, extra); err != nil { + a.logger.Error("saving the purchasing account", "error", traces.RecordError(ctx, err)) + } } type RestoreSubscriptionResponse struct { @@ -181,6 +212,9 @@ type RestoreSubscriptionResponse struct { // data should contain the fields required by the backend for the chosen service (e.g. // "purchaseToken" for Google, "receipt" for Apple); callers are responsible for populating // it. Services other than [GoogleService] and [AppleService] are rejected. +// +// When the subscription belongs to a different account, the client switches to that account +// before returning, unless the active account may have changed while the request was in flight. func (a *Client) RestoreSubscription(ctx context.Context, service SubscriptionService, data map[string]string) (*RestoreSubscriptionResponse, error) { ctx, span := otel.Tracer(tracerName).Start(ctx, "restore_subscription") defer span.End() @@ -195,6 +229,7 @@ func (a *Client) RestoreSubscription(ctx context.Context, service SubscriptionSe return nil, traces.RecordError(ctx, fmt.Errorf("unsupported service: %s", service)) } + generation := a.identityGeneration() resp, err := a.sendProRequest(ctx, "POST", path, nil, nil, data) if err != nil { a.logger.Error("restoring subscription", "error", err) @@ -204,6 +239,7 @@ func (a *Client) RestoreSubscription(ctx context.Context, service SubscriptionSe if err := json.Unmarshal(resp, &result); err != nil { return nil, traces.RecordError(ctx, fmt.Errorf("unmarshaling restore subscription response: %w", err)) } + a.switchToPurchaser(ctx, generation, result.ActualUserID, result.ActualUserToken) return &result, nil } @@ -344,9 +380,5 @@ func (a *Client) ActivationCode(ctx context.Context, email, resellerCode string) if err := json.Unmarshal(resp, &purchase); err != nil { return nil, traces.RecordError(ctx, fmt.Errorf("unmarshaling purchase response: %w", err)) } - if purchase.BaseResponse != nil && purchase.Error != "" { - a.logger.Error("retrieving subscription status", "error", purchase.Error) - return nil, traces.RecordError(ctx, fmt.Errorf("received bad response: %s", purchase.Error)) - } return &purchase, nil } diff --git a/account/subscription_test.go b/account/subscription_test.go index 4e1a2434..3e2fb5aa 100644 --- a/account/subscription_test.go +++ b/account/subscription_test.go @@ -11,8 +11,10 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/getlantern/radiance/account/protos" "github.com/getlantern/radiance/common" "github.com/getlantern/radiance/common/settings" + "github.com/getlantern/radiance/events" "github.com/getlantern/radiance/log" ) @@ -148,12 +150,36 @@ func TestRestoreSubscription(t *testing.T) { assert.Equal(t, "token", resp.ActualUserToken) }) - t.Run("google", func(t *testing.T) { + t.Run("google switches to the purchasing account", func(t *testing.T) { ac, _ := newTestClient(t) + ac.setData(&UserData{ + LegacyID: 1, + LegacyToken: "old-token", + Token: "old-jwt", + LegacyUserData: &protos.LoginResponse_UserData{UserId: 1, Email: "old@example.com", UserLevel: "pro"}, + }, nil) + setOAuthFlags(t) + changes := make(chan struct{}, 4) + sub := events.Subscribe(func(UserChangeEvent) { changes <- struct{}{} }) + defer sub.Unsubscribe() + resp, err := ac.RestoreSubscription(context.Background(), GoogleService, map[string]string{"purchaseToken": "t"}) require.NoError(t, err) require.NotNil(t, resp) assert.Equal(t, "active", resp.Status) + + assert.Equal(t, int64(1234), settings.GetInt64(settings.UserIDKey)) + assert.Equal(t, "token", settings.GetString(settings.TokenKey)) + assert.Empty(t, settings.GetString(settings.JwtTokenKey)) + assert.Empty(t, settings.GetString(settings.EmailKey)) + assert.False(t, settings.IsPro()) + assert.False(t, settings.GetBool(settings.OAuthLoginKey)) + assert.Empty(t, settings.GetString(settings.OAuthProviderKey)) + select { + case <-changes: + case <-time.After(3 * time.Second): + t.Fatal("account switch not reported") + } }) t.Run("unsupported", func(t *testing.T) { @@ -166,7 +192,7 @@ func TestRestoreSubscription(t *testing.T) { func TestPlans(t *testing.T) { ac, _ := newTestClient(t) - require.NoError(t, ac.storeIdentity(123, "token")) + require.NoError(t, ac.storeIdentity(123, "token", nil)) resp, err := ac.SubscriptionPlans(context.Background(), "store") require.NoError(t, err) assert.NotEmpty(t, resp) @@ -191,7 +217,7 @@ func TestPlansWaitsForNewUser(t *testing.T) { case <-time.After(100 * time.Millisecond): } - require.NoError(t, ac.storeIdentity(123, "token")) + require.NoError(t, ac.storeIdentity(123, "token", nil)) select { case r := <-done: require.NoError(t, r.err) @@ -226,6 +252,89 @@ func TestNewStripeSubscriptionOmitsEmptyCouponCode(t *testing.T) { assert.NotEmpty(t, resp) assert.False(t, ts.stripeSubscriptionHasCouponCode) } + +func TestVerifySwitchesToPurchaser(t *testing.T) { + a, _ := newTestClient(t) + a.setData(&UserData{LegacyID: 1, LegacyToken: "old-token"}, nil) + require.NoError(t, a.storeSalt([]byte("previous-salt"))) + useProServer(t, a, serveJSON(VerifySubscriptionResponse{Status: "active", ActualUserID: 99, ActualUserToken: "purchaser"})) + + _, err := a.VerifySubscription(context.Background(), AppleService, map[string]string{"receipt": "r"}) + require.NoError(t, err) + + assert.Equal(t, int64(99), settings.GetInt64(settings.UserIDKey)) + assert.Equal(t, "purchaser", settings.GetString(settings.TokenKey)) + assert.Nil(t, a.getSaltCached(), "the previous account's salt must not carry over") + saved, err := readSalt(a.saltPath) + require.NoError(t, err) + assert.Nil(t, saved) +} + +func TestPurchaseSwitchNotSavedKeepsAccount(t *testing.T) { + a, _ := newTestClient(t) + require.NoError(t, a.setData(&UserData{LegacyID: 1, LegacyToken: "old-token"}, nil)) + require.NoError(t, a.storeSalt([]byte("previous-salt"))) + useProServer(t, a, serveJSON(VerifySubscriptionResponse{Status: "active", ActualUserID: 99, ActualUserToken: "purchaser"})) + defer failSettingsSaves(t)() + + _, err := a.VerifySubscription(context.Background(), AppleService, map[string]string{"receipt": "r"}) + + assert.NoError(t, err, "the purchase itself succeeded") + assert.Equal(t, int64(1), settings.GetInt64(settings.UserIDKey)) + assert.Equal(t, []byte("previous-salt"), a.getSaltCached()) + salt, err := readSalt(a.saltPath) + require.NoError(t, err) + assert.Nil(t, salt) +} + +func TestLatePurchaseResponseDoesNotSwitch(t *testing.T) { + a, _ := newTestClient(t) + a.setData(&UserData{LegacyID: 1, LegacyToken: "a"}, nil) + handler, requested, release := heldJSON(VerifySubscriptionResponse{Status: "active", ActualUserID: 99, ActualUserToken: "purchaser"}) + useProServer(t, a, handler) + + errc := make(chan error, 1) + go func() { + _, err := a.VerifySubscription(context.Background(), AppleService, map[string]string{"receipt": "r"}) + errc <- err + }() + <-requested + a.setData(&UserData{LegacyID: 2, LegacyToken: "b"}, nil) + close(release) + + assert.NoError(t, <-errc, "the purchase itself succeeded") + assert.Equal(t, int64(2), settings.GetInt64(settings.UserIDKey)) + assert.Equal(t, "b", settings.GetString(settings.TokenKey)) +} + +func TestSameAccountPurchaseKeepsSaltAndOAuth(t *testing.T) { + a, _ := newTestClient(t) + a.setData(&UserData{LegacyID: 99, LegacyToken: "old-token"}, nil) + require.NoError(t, a.storeSalt([]byte("my-salt"))) + setOAuthFlags(t) + useProServer(t, a, serveJSON(VerifySubscriptionResponse{Status: "active", ActualUserID: 99, ActualUserToken: "new-token"})) + + _, err := a.VerifySubscription(context.Background(), AppleService, map[string]string{"receipt": "r"}) + require.NoError(t, err) + + assert.Equal(t, "new-token", settings.GetString(settings.TokenKey)) + assert.Equal(t, []byte("my-salt"), a.getSaltCached()) + saved, err := readSalt(a.saltPath) + require.NoError(t, err) + assert.Equal(t, []byte("my-salt"), saved) + assert.True(t, settings.GetBool(settings.OAuthLoginKey)) + assert.Equal(t, "google", settings.GetString(settings.OAuthProviderKey)) +} + +func TestVerifySucceedsWithAnUnexpectedBody(t *testing.T) { + a, _ := newTestClient(t) + useProServer(t, a, http.HandlerFunc(func(http.ResponseWriter, *http.Request) {})) + + _, err := a.VerifySubscription(context.Background(), AppleService, map[string]string{"receipt": "r"}) + + assert.NoError(t, err) +} + func TestPurchaseResponseUnmarshalPlan(t *testing.T) { var fromString PurchaseResponse require.NoError(t, json.Unmarshal([]byte(`{"status":"ok","provider":"web-license","plan":"1y-usd-10","isPro":true}`), &fromString)) diff --git a/account/user.go b/account/user.go index 20e2954b..d64736dc 100644 --- a/account/user.go +++ b/account/user.go @@ -6,7 +6,11 @@ import ( "errors" "fmt" "io/fs" + "maps" + "math" "net/url" + "slices" + "strconv" "strings" "go.opentelemetry.io/otel" @@ -36,6 +40,7 @@ func (a *Client) NewUser(ctx context.Context) (*UserData, error) { ctx, span := otel.Tracer(tracerName).Start(ctx, "new_user") defer span.End() + generation := a.identityGeneration() resp, err := a.sendProRequest(ctx, "POST", "/user-create", nil, nil, nil) if err != nil { a.logger.Error("creating new user", "error", err) @@ -45,32 +50,36 @@ func (a *Client) NewUser(ctx context.Context) (*UserData, error) { if err := json.Unmarshal(resp, &userResp); err != nil { return nil, traces.RecordError(ctx, fmt.Errorf("error unmarshalling new user response: %w", err)) } - userData, err := a.storeData(ctx, userResp) - if err != nil { - return nil, err - } - return userData, nil + return a.storeData(ctx, userResp, generation) } // FetchUserData fetches user data from the server. func (a *Client) FetchUserData(ctx context.Context) (*UserData, error) { ctx, span := otel.Tracer(tracerName).Start(ctx, "fetch_user_data") defer span.End() - return a.fetchUserData(ctx) + generation := a.identityGeneration() + resp, err := a.requestUserData(ctx, nil) + if err != nil { + return nil, err + } + return a.storeData(ctx, resp, generation) } -// fetchUserData fetches the user's account data and stores it. -func (a *Client) fetchUserData(ctx context.Context) (*UserData, error) { - resp, err := a.sendProRequest(ctx, "GET", "/user-data", nil, nil, nil) +func (a *Client) requestUserData(ctx context.Context, headers map[string]string) (UserDataResponse, error) { + resp, err := a.sendProRequest(ctx, "GET", "/user-data", nil, headers, nil) if err != nil { a.logger.Error("user data", "error", err) - return nil, traces.RecordError(ctx, fmt.Errorf("getting user data: %w", err)) + return UserDataResponse{}, traces.RecordError(ctx, fmt.Errorf("getting user data: %w", err)) } var userResp UserDataResponse if err := json.Unmarshal(resp, &userResp); err != nil { - return nil, traces.RecordError(ctx, fmt.Errorf("error unmarshalling new user response: %w", err)) + return UserDataResponse{}, traces.RecordError(ctx, fmt.Errorf("unmarshalling user data response: %w", err)) + } + if userResp.LoginResponse_UserData == nil { + return UserDataResponse{}, traces.RecordError(ctx, errors.New("no user data in response")) } - return a.storeData(ctx, userResp) + userResp.UserLevel = normalizeLegacyUserLevel(userResp.UserLevel) + return userResp, nil } func normalizeLegacyUserLevel(level string) string { @@ -81,12 +90,13 @@ func normalizeLegacyUserLevel(level string) string { return level } -func (a *Client) storeData(ctx context.Context, resp UserDataResponse) (*UserData, error) { - if resp.BaseResponse != nil && resp.Error != "" { - err := fmt.Errorf("received bad response: %s", resp.Error) - a.logger.Error("user data", "error", err) - return nil, traces.RecordError(ctx, err) - } +// ErrAccountChanged is returned when the account may have changed while the request was in flight; +// the response was discarded, and a retry acts on the current account. +var ErrAccountChanged = errors.New("account changed during request") + +// storeData stores resp unless the account changed after generation was captured, in which case it +// returns [ErrAccountChanged]. +func (a *Client) storeData(ctx context.Context, resp UserDataResponse, generation uint64) (*UserData, error) { if resp.LoginResponse_UserData == nil { a.logger.Error("user data", "error", "no user data in response") return nil, traces.RecordError(ctx, fmt.Errorf("no user data in response")) @@ -95,8 +105,14 @@ func (a *Client) storeData(ctx context.Context, resp UserDataResponse) (*UserDat resp.DeviceID = settings.GetString(settings.DeviceIDKey) a.mu.Lock() defer a.mu.Unlock() - - if resp.Token == "" && resp.UserId != 0 && resp.UserId == settings.GetInt64(settings.UserIDKey) { + if a.generation != generation { + return nil, traces.RecordError(ctx, ErrAccountChanged) + } + sameUser := resp.UserId != 0 && resp.UserId == settings.GetInt64(settings.UserIDKey) + if resp.UserId != 0 && !sameUser { + a.generation++ + } + if resp.Token == "" && sameUser { resp.Token = settings.GetString(settings.TokenKey) } login := &UserData{ @@ -104,7 +120,11 @@ func (a *Client) storeData(ctx context.Context, resp UserDataResponse) (*UserDat LegacyToken: resp.Token, LegacyUserData: resp.LoginResponse_UserData, } - return a.setDataLocked(login, true), nil + data, err := a.setDataLocked(login, true, nil) + if err != nil { + return nil, traces.RecordError(ctx, fmt.Errorf("saving user data: %w", err)) + } + return data, nil } type DataCapInfo struct { @@ -185,18 +205,26 @@ func (a *Client) SignUp(ctx context.Context, email, password string) ([]byte, *p if err != nil { return nil, nil, traces.RecordError(ctx, err) } - a.setSalt(salt) var signupData protos.SignupResponse if err := proto.Unmarshal(resp, &signupData); err != nil { return nil, nil, traces.RecordError(ctx, fmt.Errorf("error unmarshalling sign up response: %w", err)) } - if err := a.storeIdentity(signupData.LegacyID, signupData.ProToken); err != nil { + a.mu.Lock() + a.generation++ + err = a.storeIdentityLocked(signupData.LegacyID, signupData.ProToken, settings.Settings{ + settings.JwtTokenKey: signupData.Token, + settings.OAuthLoginKey: false, + settings.OAuthProviderKey: "", + }) + if err != nil { + a.mu.Unlock() return nil, nil, traces.RecordError(ctx, fmt.Errorf("saving signup identity: %w", err)) } - jwtTokenErr := settings.Set(settings.JwtTokenKey, signupData.Token) - if jwtTokenErr != nil { - return nil, nil, traces.RecordError(ctx, fmt.Errorf("could not save JWT token: %w", jwtTokenErr)) + err = a.storeSaltLocked(salt) + a.mu.Unlock() + if err != nil { + return nil, nil, traces.RecordError(ctx, fmt.Errorf("saving signup salt: %w", err)) } return salt, &signupData, nil @@ -293,13 +321,16 @@ func (a *Client) Login(ctx context.Context, email, password string) (*UserData, // regardless of state we need to save login information // We have device flow limit on login - a.setData(&loginResp) - a.setSalt(salt) - if saltErr := writeSalt(salt, a.saltPath); saltErr != nil { - return nil, traces.RecordError(ctx, saltErr) + oauth := settings.Settings{settings.OAuthLoginKey: false, settings.OAuthProviderKey: ""} + a.mu.Lock() + defer a.mu.Unlock() + a.generation++ + if _, err := a.setDataLocked(&loginResp, false, oauth); err != nil { + return nil, traces.RecordError(ctx, fmt.Errorf("saving login: %w", err)) + } + if err := a.storeSaltLocked(salt); err != nil { + return nil, traces.RecordError(ctx, err) } - settings.Set(settings.OAuthLoginKey, false) - settings.Set(settings.OAuthProviderKey, "") return &loginResp, nil } @@ -323,11 +354,10 @@ func (a *Client) Logout(ctx context.Context, email string) (*UserData, error) { if err != nil { return nil, traces.RecordError(ctx, fmt.Errorf("logging out: %w", err)) } - a.ClearUser() - a.setSalt(nil) - settings.Set(settings.OAuthLoginKey, false) - settings.Set(settings.OAuthProviderKey, "") - if err := writeSalt(nil, a.saltPath); err != nil { + if err := a.ClearUser(); err != nil { + return nil, traces.RecordError(ctx, fmt.Errorf("logged out on the server, but clearing the local account failed: %w", err)) + } + if err := a.storeSalt(nil); err != nil { return nil, traces.RecordError(ctx, fmt.Errorf("writing salt after logout: %w", err)) } return a.NewUser(ctx) @@ -371,8 +401,13 @@ func (a *Client) CompleteRecoveryByEmail(ctx context.Context, email, newPassword if err != nil { return traces.RecordError(ctx, fmt.Errorf("failed to complete recovery by email: %w", err)) } - if err = writeSalt(newSalt, a.saltPath); err != nil { - return traces.RecordError(ctx, fmt.Errorf("failed to write new salt: %w", err)) + a.mu.Lock() + defer a.mu.Unlock() + // The salt cache isn't keyed by email, so only the current account's new salt belongs in it. + if strings.EqualFold(settings.GetString(settings.EmailKey), lowerCaseEmail) { + if err = a.storeSaltLocked(newSalt); err != nil { + return traces.RecordError(ctx, fmt.Errorf("failed to write new salt: %w", err)) + } } return nil } @@ -438,8 +473,9 @@ func (a *Client) StartChangeEmail(ctx context.Context, newEmail, password string return traces.RecordError(ctx, err) } -// CompleteChangeEmail completes a change of the email address associated with this user account, -// using the code received via email. +// CompleteChangeEmail changes the account's email using the code received via email. +// If another account is active when the response arrives, only the server is updated and an error +// is returned. func (a *Client) CompleteChangeEmail(ctx context.Context, newEmail, password, code string) error { ctx, span := otel.Tracer(tracerName).Start(ctx, "complete_change_email") defer span.End() @@ -459,6 +495,7 @@ func (a *Client) CompleteChangeEmail(ctx context.Context, newEmail, password, co return traces.RecordError(ctx, err) } + userID := settings.GetInt64(settings.UserIDKey) data := &protos.CompleteChangeEmailRequest{ OldEmail: settings.GetString(settings.EmailKey), NewEmail: newEmail, @@ -470,14 +507,18 @@ func (a *Client) CompleteChangeEmail(ctx context.Context, newEmail, password, co if err != nil { return traces.RecordError(ctx, err) } - if err := writeSalt(newSalt, a.saltPath); err != nil { + a.mu.Lock() + defer a.mu.Unlock() + if settings.GetInt64(settings.UserIDKey) != userID { + return traces.RecordError(ctx, errors.New("email changed on the server, but another account is now active locally")) + } + a.generation++ + if err := a.storeSaltLocked(newSalt); err != nil { return traces.RecordError(ctx, err) } - if err := settings.Set(settings.EmailKey, newEmail); err != nil { + if err := a.storeIdentityLocked(0, "", settings.Settings{settings.EmailKey: newEmail}); err != nil { return traces.RecordError(ctx, err) } - - a.setSalt(newSalt) return nil } @@ -514,9 +555,10 @@ func (a *Client) DeleteAccount(ctx context.Context, email, password string) (*Us return nil, traces.RecordError(ctx, err) } - a.ClearUser() - a.setSalt(nil) - if err := writeSalt(nil, a.saltPath); err != nil { + if err := a.ClearUser(); err != nil { + return nil, traces.RecordError(ctx, fmt.Errorf("account deleted on the server, but clearing the local account failed: %w", err)) + } + if err := a.storeSalt(nil); err != nil { return nil, traces.RecordError(ctx, fmt.Errorf("failed to write salt during account deletion cleanup: %w", err)) } @@ -546,38 +588,57 @@ func (a *Client) OAuthLoginURL(ctx context.Context, provider string) (string, er return loginURL.String(), nil } +// OAuthLoginCallback signs in with the token from an OAuth callback. It returns [ErrAccountChanged] +// if the account changes before the sign-in is saved. func (a *Client) OAuthLoginCallback(ctx context.Context, oAuthToken string) (*UserData, error) { a.logger.Debug("Getting OAuth login callback") jwtUserInfo, err := decodeJWT(oAuthToken) if err != nil { - return nil, fmt.Errorf("error decoding JWT: %w", err) + return nil, fmt.Errorf("%w: error decoding JWT: %w", ErrInvalidToken, err) } - - // Seed the identity first so the fetchUserData call below can authenticate. - login := &UserData{ - LegacyID: jwtUserInfo.LegacyUserID, - LegacyToken: jwtUserInfo.LegacyToken, - LegacyUserData: &protos.LoginResponse_UserData{ - UserId: jwtUserInfo.LegacyUserID, - Token: jwtUserInfo.LegacyToken, - DeviceID: jwtUserInfo.DeviceID, - Email: jwtUserInfo.Email, - }, + if jwtUserInfo.LegacyUserID == 0 || jwtUserInfo.LegacyToken == "" { + return nil, fmt.Errorf("%w: login token is missing the account identity", ErrInvalidToken) } - a.setData(login) - user, err := a.fetchUserData(ctx) + + generation := a.identityGeneration() + resp, err := a.requestUserData(ctx, map[string]string{ + common.UserIDHeader: strconv.FormatInt(jwtUserInfo.LegacyUserID, 10), + common.ProTokenHeader: jwtUserInfo.LegacyToken, + }) if err != nil { - return nil, fmt.Errorf("error getting user data: %w", err) + return nil, fmt.Errorf("getting OAuth user data: %w", err) } - - if err := settings.Set(settings.JwtTokenKey, oAuthToken); err != nil { - a.logger.Error("Failed to persist JWT token", "error", err) - return nil, fmt.Errorf("failed to persist JWT token: %w", err) + if resp.UserId == 0 { + return nil, errors.New("OAuth user data is missing the account identity") + } + if resp.UserId == jwtUserInfo.LegacyUserID { + if resp.Token == "" { + resp.Token = jwtUserInfo.LegacyToken + } + if resp.Email == "" { + resp.Email = jwtUserInfo.Email + } + } + resp.DeviceID = settings.GetString(settings.DeviceIDKey) + user := &UserData{ + LegacyID: resp.UserId, + LegacyToken: resp.Token, + LegacyUserData: resp.LoginResponse_UserData, + Id: jwtUserInfo.Email, + EmailConfirmed: true, + Token: oAuthToken, + } + a.mu.Lock() + defer a.mu.Unlock() + if a.generation != generation { + return nil, ErrAccountChanged + } + a.generation++ + if _, err := a.setDataLocked(user, false, settings.Settings{ + settings.OAuthLoginKey: true, + }); err != nil { + return nil, fmt.Errorf("saving OAuth user data: %w", err) } - settings.Set(settings.OAuthLoginKey, true) - user.Id = jwtUserInfo.Email - user.EmailConfirmed = true - a.setData(user) return user, nil } @@ -592,7 +653,7 @@ func (a *Client) OAuthDeviceLimitCallback(ctx context.Context, oAuthToken string if jwtUserInfo.LegacyUserID == 0 || jwtUserInfo.LegacyToken == "" { return fmt.Errorf("%w: device-limit token is missing the account identity", ErrInvalidToken) } - return a.storeIdentity(jwtUserInfo.LegacyUserID, jwtUserInfo.LegacyToken) + return a.storeIdentity(jwtUserInfo.LegacyUserID, jwtUserInfo.LegacyToken, nil) } // ErrInvalidToken distinguishes an unusable OAuth callback token from a @@ -620,9 +681,6 @@ func (a *Client) RemoveDevice(ctx context.Context, deviceID string) (*LinkRespon if err := json.Unmarshal(resp, &link); err != nil { return nil, traces.RecordError(ctx, fmt.Errorf("error unmarshalling remove device response: %w", err)) } - if link.BaseResponse != nil && link.BaseResponse.Error != "" { - return nil, traces.RecordError(ctx, fmt.Errorf("failed to remove device: %s", link.BaseResponse.Error)) - } return &link, nil } @@ -657,12 +715,9 @@ func (a *Client) referralAttachV1(ctx context.Context, code string) (*ReferralAt return nil, traces.RecordError(ctx, err) } var baseResp protos.BaseResponse - if err := proto.Unmarshal(resp, &baseResp); err != nil { + if err := json.Unmarshal(resp, &baseResp); err != nil { return nil, traces.RecordError(ctx, fmt.Errorf("error unmarshalling referral attach response: %w", err)) } - if baseResp.Error != "" { - return nil, traces.RecordError(ctx, errors.New(baseResp.Error)) - } return &ReferralAttachResponse{ BaseResponse: &baseResp, }, nil @@ -686,9 +741,6 @@ func (a *Client) referralAttachV2(ctx context.Context, code, channel string) (*R if err := json.Unmarshal(resp, &referral); err != nil { return nil, traces.RecordError(ctx, fmt.Errorf("error unmarshalling referral attach v2 response: %w", err)) } - if referral.BaseResponse != nil && referral.Error != "" { - return nil, traces.RecordError(ctx, errors.New(referral.Error)) - } return &referral, nil } @@ -697,23 +749,78 @@ type UserChangeEvent struct { events.Event } -func (a *Client) storeIdentity(id int64, token string) error { +func (a *Client) storeIdentity(id int64, token string, extra settings.Settings) error { a.mu.Lock() defer a.mu.Unlock() - changed, err := a.storeIdentityLocked(id, token) - if changed { - events.Emit(UserChangeEvent{}) + a.generation++ + return a.storeIdentityLocked(id, token, extra) +} + +// PatchAccountSettings saves updates as a change to the active account. If any value differs from +// its stored setting, user-data and purchase responses requested before the call are discarded; +// otherwise the call does nothing. +// A different user ID clears the previous account's credentials, cached data, salt and OAuth flags. +// A supplied user ID must be a positive int64-compatible number or base-10 string. +// A changed account emits a [UserChangeEvent]; on error the active identity remains unchanged. +func (a *Client) PatchAccountSettings(updates settings.Settings) error { + a.mu.Lock() + defer a.mu.Unlock() + var id int64 + if value, ok := updates[settings.UserIDKey]; ok { + var err error + id, err = parseUserID(value) + if err != nil { + return err + } } - return err + var extra settings.Settings + if id != 0 && id != settings.GetInt64(settings.UserIDKey) { + extra = settings.Settings{settings.OAuthLoginKey: false, settings.OAuthProviderKey: ""} + maps.Copy(extra, updates) + } else { + // Diff also compares types, so the user ID, already matched numerically, is left out. + extra = updates.Diff(settings.GetAllFor(slices.Collect(maps.Keys(updates))...)) + delete(extra, settings.UserIDKey) + if len(extra) == 0 { + return nil + } + } + a.generation++ + return a.storeIdentityLocked(id, "", extra) } -// storeIdentityLocked requires a.mu to be held. -func (a *Client) storeIdentityLocked(id int64, token string) (bool, error) { - previousID := settings.GetInt64(settings.UserIDKey) - previousToken := settings.GetString(settings.TokenKey) - updates := settings.Settings{} +func parseUserID(v any) (int64, error) { + value := fmt.Sprint(v) + if f, ok := v.(float64); ok { + if math.Trunc(f) != f { + return 0, fmt.Errorf("invalid user ID %v: expected an integer", f) + } + value = strconv.FormatFloat(f, 'f', 0, 64) + } + id, err := strconv.ParseInt(value, 10, 64) + if err != nil || id <= 0 { + return 0, fmt.Errorf("invalid user ID %q: expected a positive int64", value) + } + return id, nil +} + +// storeIdentityLocked saves nonzero id and nonempty token together with extra, emitting a +// [UserChangeEvent] if the account changed. Switching accounts clears credentials, cached data +// and salt, but keeps OAuth flags because sign-in records its provider before switching. +// A failed settings write leaves the identity unchanged but may discard the persisted salt. +// It requires a.mu to be held. +func (a *Client) storeIdentityLocked(id int64, token string, extra settings.Settings) error { + before := currentAccount() + defer func() { + if currentAccount() != before { + events.Emit(UserChangeEvent{}) + } + }() // The cached response can lag behind signup or OAuth, so compare the active identity. - if id != 0 && id != previousID { + switching := id != 0 && id != settings.GetInt64(settings.UserIDKey) + updates := settings.Settings{} + if switching { + // Overwrite rather than delete, so unlocked readers never see the account without an id. updates[settings.TokenKey] = "" updates[settings.JwtTokenKey] = "" updates[settings.EmailKey] = "" @@ -721,6 +828,7 @@ func (a *Client) storeIdentityLocked(id int64, token string) (bool, error) { updates[settings.DevicesKey] = []settings.Device{} updates[settings.UserDataKey] = nil } + maps.Copy(updates, extra) if id != 0 { updates[settings.UserIDKey] = id } @@ -728,61 +836,59 @@ func (a *Client) storeIdentityLocked(id int64, token string) (bool, error) { updates[settings.TokenKey] = token } if len(updates) == 0 { - return false, nil + return nil + } + if switching { + // Invalidate the unkeyed salt on disk before publishing another identity. + if err := writeSalt(nil, a.saltPath); err != nil { + return fmt.Errorf("clearing the previous account's salt: %w", err) + } + } + if err := settings.Patch(updates); err != nil { + return err } - err := settings.Patch(updates) - changed := settings.GetInt64(settings.UserIDKey) != previousID || settings.GetString(settings.TokenKey) != previousToken - return changed, err + if switching { + a.salt = nil + } + return nil } -func (a *Client) setData(data *UserData) { - if data == nil { - a.ClearUser() - return - } - a.mu.Lock() - defer a.mu.Unlock() - a.setDataLocked(data, false) +// accountState holds the settings whose change is reported as a [UserChangeEvent]. +type accountState struct { + id int64 + token, level, email, jwt string } -// setDataLocked requires a.mu to be held and data to be non-nil. -func (a *Client) setDataLocked(data *UserData, preserveLoginFields bool) *UserData { - changed, err := a.storeIdentityLocked(data.LegacyID, data.LegacyToken) - if err != nil { - a.logger.Error("failed to store account identity", "error", err) - } - defer func() { - if changed { - events.Emit(UserChangeEvent{}) - } - }() - if data.LegacyUserData == nil { - a.logger.Info("no user data to set, storing id and token only") - return data +func currentAccount() accountState { + return accountState{ + id: settings.GetInt64(settings.UserIDKey), + token: settings.GetString(settings.TokenKey), + level: settings.GetString(settings.UserLevelKey), + email: settings.GetString(settings.EmailKey), + jwt: settings.GetString(settings.JwtTokenKey), } +} - if data.LegacyUserData.UserLevel != "" { - oldUserLevel := settings.GetString(settings.UserLevelKey) - changed = changed || oldUserLevel != data.LegacyUserData.UserLevel - if err := settings.Set(settings.UserLevelKey, data.LegacyUserData.UserLevel); err != nil { - a.logger.Error("failed to set user level in settings", "error", err) - } - } - if data.LegacyUserData.Email != "" { - oldEmail := settings.GetString(settings.EmailKey) - changed = changed || oldEmail != data.LegacyUserData.Email - if err := settings.Set(settings.EmailKey, data.LegacyUserData.Email); err != nil { - a.logger.Error("failed to set email in settings", "error", err) - } - } - if data.Token != "" { - oldJwtToken := settings.GetString(settings.JwtTokenKey) - changed = changed || oldJwtToken != data.Token - if err := settings.Set(settings.JwtTokenKey, data.Token); err != nil { - a.logger.Error("failed to set JWT token in settings", "error", err) - } +// setData stores data together with extra, or clears the account when data is nil. +func (a *Client) setData(data *UserData, extra settings.Settings) error { + if data == nil { + return a.ClearUser() } + a.mu.Lock() + defer a.mu.Unlock() + a.generation++ + _, err := a.setDataLocked(data, false, extra) + return err +} +// setDataLocked stores a copy of data together with extra, first merging data with the cached +// response of the same account when preserveLoginFields is set. Data without user data stores only +// its id, token and devices. It returns data, possibly merged, which the caller may modify without +// affecting settings. It requires a.mu to be held and data to be non-nil. +func (a *Client) setDataLocked(data *UserData, preserveLoginFields bool, extra settings.Settings) (*UserData, error) { + updates := settings.Settings{} + // A device-limited login carries no user data, but its device list is what the user needs to + // pick a device to remove. if len(data.Devices) > 0 { devices := []settings.Device{} for _, d := range data.Devices { @@ -791,47 +897,51 @@ func (a *Client) setDataLocked(data *UserData, preserveLoginFields bool) *UserDa ID: d.Id, }) } - if err := settings.Set(settings.DevicesKey, devices); err != nil { - a.logger.Error("failed to set devices in settings", "error", err) - } + updates[settings.DevicesKey] = devices } - - if preserveLoginFields { - // Merge only the cached response so old login fields cannot overwrite newer settings. - cached := new(UserData) - if err := settings.GetStruct(settings.UserDataKey, cached); err != nil { - a.logger.Warn("ignoring invalid cached user data", "error", err) - } else if data.LegacyID != 0 && cached.LegacyID == data.LegacyID { - cached.LegacyToken = data.LegacyToken - cached.LegacyUserData = data.LegacyUserData - data = cached + if data.LegacyUserData == nil { + a.logger.Info("no user data to set, storing id, token and devices only") + } else { + if data.LegacyUserData.UserLevel != "" { + updates[settings.UserLevelKey] = data.LegacyUserData.UserLevel + } + if data.LegacyUserData.Email != "" { + updates[settings.EmailKey] = data.LegacyUserData.Email + } + if data.Token != "" { + updates[settings.JwtTokenKey] = data.Token + } + if preserveLoginFields { + // Merge only the cached response so old login fields cannot overwrite newer settings. + cached := new(UserData) + if err := settings.GetStruct(settings.UserDataKey, cached); err != nil { + a.logger.Warn("ignoring invalid cached user data", "error", err) + } else if data.LegacyID != 0 && cached.LegacyID == data.LegacyID { + cached.LegacyToken = data.LegacyToken + cached.LegacyUserData = data.LegacyUserData + data = cached + } } + updates[settings.UserDataKey] = proto.Clone(data) } - if err := settings.Set(settings.UserDataKey, data); err != nil { - a.logger.Error("failed to set login response in settings", "error", err) + maps.Copy(updates, extra) + if err := a.storeIdentityLocked(data.LegacyID, data.LegacyToken, updates); err != nil { + return nil, err } - - return data + return data, nil } -// ClearUser removes the cached account and its credentials. -func (a *Client) ClearUser() { +// ClearUser removes the cached account and its credentials. On error the account remains. +func (a *Client) ClearUser() error { a.mu.Lock() defer a.mu.Unlock() + a.generation++ hadIdentity := settings.GetInt64(settings.UserIDKey) != 0 || settings.GetString(settings.TokenKey) != "" - err := settings.Clear( - settings.UserIDKey, - settings.TokenKey, - settings.UserLevelKey, - settings.EmailKey, - settings.DevicesKey, - settings.JwtTokenKey, - settings.UserDataKey, - ) - if err != nil { - a.logger.Warn("failed to clear user info", "error", err) + if err := settings.Clear(settings.UserKeys...); err != nil { + return fmt.Errorf("clearing user info: %w", err) } if hadIdentity { events.Emit(UserChangeEvent{}) } + return nil } diff --git a/account/user_change_test.go b/account/user_change_test.go index 714f6c53..b9055b83 100644 --- a/account/user_change_test.go +++ b/account/user_change_test.go @@ -40,13 +40,13 @@ func TestAccountChangesNotifyWithoutBackendWrappers(t *testing.T) { _, err = a.FetchUserData(ctx) require.NoError(t, err) expect(0) - a.setData(&UserData{LegacyUserData: &protos.LoginResponse_UserData{UserLevel: "pro"}}) + a.setData(&UserData{LegacyUserData: &protos.LoginResponse_UserData{UserLevel: "pro"}}, nil) expect(1) - a.setData(&UserData{LegacyUserData: &protos.LoginResponse_UserData{UserLevel: "pro"}}) + a.setData(&UserData{LegacyUserData: &protos.LoginResponse_UserData{UserLevel: "pro"}}, nil) expect(0) - a.setData(&UserData{LegacyID: 456, LegacyToken: "new-token"}) + a.setData(&UserData{LegacyID: 456, LegacyToken: "new-token"}, nil) expect(1) - a.setData(&UserData{LegacyID: 456, LegacyToken: "new-token"}) + a.setData(&UserData{LegacyID: 456, LegacyToken: "new-token"}, nil) expect(0) _, err = a.Logout(ctx, "test@example.com") require.NoError(t, err) @@ -56,6 +56,13 @@ func TestAccountChangesNotifyWithoutBackendWrappers(t *testing.T) { a.ClearUser() expect(0) + oauthToken := mockJWT(t, map[string]any{ + "legacy_user_id": 123, "legacy_token": "oauth-token", "email": "test@example.com", + }) + _, err = a.OAuthLoginCallback(ctx, oauthToken) + require.NoError(t, err) + expect(1) + token := mockJWT(t, map[string]any{"legacy_user_id": 789, "legacy_token": "oauth-token"}) require.NoError(t, a.OAuthDeviceLimitCallback(ctx, token)) expect(1) diff --git a/account/user_refresh_test.go b/account/user_refresh_test.go index 48db068c..f9904383 100644 --- a/account/user_refresh_test.go +++ b/account/user_refresh_test.go @@ -3,8 +3,11 @@ package account import ( "context" "fmt" + "math" "net/http" "net/http/httptest" + "os" + "path/filepath" "testing" "github.com/stretchr/testify/assert" @@ -93,7 +96,7 @@ func TestFetchUserDataWithoutMatchingLogin(t *testing.T) { LegacyUserData: &protos.LoginResponse_UserData{ UserId: tt.cachedID, Email: "old@example.com", UserLevel: "pro", }, - }) + }, nil) } got, err := ac.FetchUserData(context.Background()) require.NoError(t, err) @@ -123,7 +126,7 @@ func TestFetchUserDataWithoutToken(t *testing.T) { ac.setData(&UserData{ LegacyID: 123, LegacyToken: "cached-token", Id: "account-id", LegacyUserData: &protos.LoginResponse_UserData{UserId: 123, Token: "cached-token"}, - }) + }, nil) require.NoError(t, settings.Set(settings.TokenKey, "current-token")) server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { writeJSONResponse(w, UserDataResponse{ @@ -159,7 +162,7 @@ func TestSignupSwitchesAccountBeforeRefresh(t *testing.T) { LegacyID: 456, LegacyToken: "old-token", Token: "old-jwt", Devices: []*protos.LoginResponse_Device{{Id: "old-device"}}, LegacyUserData: &protos.LoginResponse_UserData{UserId: 456}, - }) + }, nil) server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { writeProtoResponse(w, &protos.SignupResponse{LegacyID: 123, ProToken: "signup-token", Token: "signup-jwt"}) })) @@ -185,7 +188,7 @@ func TestLoginReplacesCachedFields(t *testing.T) { Token: "previous-jwt", Devices: []*protos.LoginResponse_Device{{Id: "old-device"}}, LegacyUserData: &protos.LoginResponse_UserData{UserId: 123}, - }) + }, nil) server.loginResponse = &UserData{ LegacyID: 123, LegacyUserData: &protos.LoginResponse_UserData{UserId: 123}, @@ -236,7 +239,7 @@ func TestSetDataNilClearsCachedUser(t *testing.T) { ac, _ := newTestClient(t) _, err := ac.NewUser(context.Background()) require.NoError(t, err) - ac.setData(nil) + ac.setData(nil, nil) assert.False(t, settings.Exists(settings.UserDataKey)) assert.Empty(t, settings.GetString(settings.TokenKey)) } @@ -248,7 +251,7 @@ func TestFetchUserDataDoesNotReplayLoginSettings(t *testing.T) { Token: "old-jwt", Devices: []*protos.LoginResponse_Device{{Id: "old-device"}}, LegacyUserData: &protos.LoginResponse_UserData{UserId: 123}, - }) + }, nil) devices := []settings.Device{{ID: "new-device", Name: "Laptop"}} require.NoError(t, settings.Patch(settings.Settings{ settings.JwtTokenKey: "new-jwt", @@ -271,12 +274,263 @@ func TestFetchUserDataDoesNotReplayLoginSettings(t *testing.T) { assert.Equal(t, devices, storedDevices) } +func TestStaleUserDataIsDiscarded(t *testing.T) { + for _, tt := range []struct { + name string + change func(a *Client) + }{ + {"A to B", func(a *Client) { + a.setData(&UserData{LegacyID: 2, LegacyToken: "b"}, nil) + }}, + {"A to B to A", func(a *Client) { + a.setData(&UserData{LegacyID: 2, LegacyToken: "b"}, nil) + a.setData(&UserData{LegacyID: 1, LegacyToken: "a"}, nil) + }}, + {"logout", func(a *Client) { a.ClearUser() }}, + {"settings patch", func(a *Client) { + a.PatchAccountSettings(settings.Settings{settings.UserIDKey: int64(2), settings.TokenKey: "b"}) + }}, + } { + t.Run(tt.name, func(t *testing.T) { + a, _ := newTestClient(t) + a.setData(&UserData{LegacyID: 1, LegacyToken: "a"}, nil) + handler, requested, release := heldJSON(UserDataResponse{ + LoginResponse_UserData: &protos.LoginResponse_UserData{UserId: 1, Token: "a", UserLevel: "pro"}, + }) + useProServer(t, a, handler) + + errc := make(chan error, 1) + go func() { + _, err := a.FetchUserData(context.Background()) + errc <- err + }() + <-requested + tt.change(a) + after := currentAccount() + close(release) + + assert.ErrorIs(t, <-errc, ErrAccountChanged) + assert.Equal(t, after, currentAccount(), "a stale response must not change the account") + assert.False(t, settings.IsPro()) + }) + } +} + +func TestNewUserDiscardsInFlightUserData(t *testing.T) { + a, _ := newTestClient(t) + a.setData(&UserData{LegacyID: 1, LegacyToken: "a"}, nil) + userData, requested, release := heldJSON(UserDataResponse{ + LoginResponse_UserData: &protos.LoginResponse_UserData{UserId: 1, Token: "a"}, + }) + mux := http.NewServeMux() + mux.Handle("/user-data", userData) + mux.Handle("/user-create", serveJSON(UserDataResponse{ + LoginResponse_UserData: &protos.LoginResponse_UserData{UserId: 2, Token: "b"}, + })) + useProServer(t, a, mux) + + errc := make(chan error, 1) + go func() { + _, err := a.FetchUserData(context.Background()) + errc <- err + }() + <-requested + _, err := a.NewUser(context.Background()) + require.NoError(t, err) + close(release) + + assert.ErrorIs(t, <-errc, ErrAccountChanged) + assert.Equal(t, int64(2), settings.GetInt64(settings.UserIDKey)) +} + +func TestReturnedUserDataIsACopy(t *testing.T) { + a, _ := newTestClient(t) + got, err := a.FetchUserData(context.Background()) + require.NoError(t, err) + + got.LegacyUserData.UserLevel = "pro" + got.Id = "mutated" + + var stored UserData + require.NoError(t, settings.GetStruct(settings.UserDataKey, &stored)) + assert.Empty(t, stored.Id) + assert.Equal(t, "free", stored.LegacyUserData.UserLevel) +} + +func TestAccountSwitchNeverExposesAnEmptyIdentity(t *testing.T) { + a, _ := newTestClient(t) + a.setData(&UserData{LegacyID: 1, LegacyToken: "a"}, nil) + stop := make(chan struct{}) + sawEmpty := make(chan bool, 1) + go func() { + empty := false + for { + select { + case <-stop: + sawEmpty <- empty + return + default: + empty = empty || settings.GetInt64(settings.UserIDKey) == 0 || settings.GetString(settings.TokenKey) == "" + } + } + }() + for i := range 50 { + a.setData(&UserData{LegacyID: int64(2 + i%2), LegacyToken: "t"}, nil) + } + close(stop) + assert.False(t, <-sawEmpty, "readers must never see an account without an id or token") +} + +func TestPatchAccountSwitchClearsOld(t *testing.T) { + for _, id := range []any{ + int(2), int8(2), int16(2), int32(2), int64(2), + uint(2), uint8(2), uint16(2), uint32(2), uint64(2), + float64(2), "2", + } { + t.Run(fmt.Sprintf("%T", id), func(t *testing.T) { + a, _ := newTestClient(t) + require.NoError(t, a.setData(&UserData{ + LegacyID: 1, + LegacyToken: "a", + Token: "a-jwt", + LegacyUserData: &protos.LoginResponse_UserData{UserId: 1, Email: "a@example.com", UserLevel: "pro"}, + }, nil)) + require.NoError(t, a.storeSalt([]byte("a-salt"))) + setOAuthFlags(t) + updates := settings.Settings{settings.UserIDKey: id, settings.TokenKey: "b"} + + require.NoError(t, a.PatchAccountSettings(updates)) + + assert.Equal(t, id, updates[settings.UserIDKey]) + assert.Equal(t, int64(2), settings.Get(settings.UserIDKey)) + require.NoError(t, settings.Reload()) + assert.Equal(t, int64(2), settings.GetInt64(settings.UserIDKey)) + assert.Equal(t, "b", settings.GetString(settings.TokenKey)) + assert.Empty(t, settings.GetString(settings.JwtTokenKey)) + assert.Empty(t, settings.GetString(settings.EmailKey)) + assert.Empty(t, settings.Get(settings.UserDataKey)) + assert.False(t, settings.IsPro()) + assert.False(t, settings.GetBool(settings.OAuthLoginKey)) + assert.Empty(t, settings.GetString(settings.OAuthProviderKey)) + assert.Nil(t, a.getSaltCached()) + salt, err := readSalt(a.saltPath) + require.NoError(t, err) + assert.Nil(t, salt) + }) + } +} + +func TestPatchAccountRejectsBadID(t *testing.T) { + for _, id := range []any{ + nil, true, "", "invalid", "2.5", "9223372036854775808", + 0, -1, uint64(math.MaxInt64) + 1, 2.5, float32(2.5), + float64(math.MaxInt64), math.Inf(1), math.NaN(), + } { + t.Run(fmt.Sprintf("%T/%v", id, id), func(t *testing.T) { + a, _ := newTestClient(t) + require.NoError(t, a.PatchAccountSettings(settings.Settings{ + settings.UserIDKey: int64(1), settings.TokenKey: "a", + })) + before := settings.GetAll() + generation := a.identityGeneration() + + err := a.PatchAccountSettings(settings.Settings{settings.UserIDKey: id, settings.TokenKey: "b"}) + + require.ErrorContains(t, err, "invalid user ID") + assert.Equal(t, before, settings.GetAll()) + assert.Equal(t, generation, a.identityGeneration()) + }) + } +} + +func TestParseUserIDLarge(t *testing.T) { + for _, tt := range []struct { + value any + want int64 + }{ + {float64(1e9), 1e9}, + {float64(1<<60) + 256, 1152921504606847232}, + {float64(math.MaxInt64) - 1024, 9223372036854774784}, + {int64(math.MaxInt64), math.MaxInt64}, + {uint64(math.MaxInt64), math.MaxInt64}, + {"9223372036854775807", math.MaxInt64}, + } { + got, err := parseUserID(tt.value) + require.NoError(t, err) + assert.Equal(t, tt.want, got) + } +} + +func TestSwitchNeedsSaltCleared(t *testing.T) { + a, _ := newTestClient(t) + require.NoError(t, a.PatchAccountSettings(settings.Settings{ + settings.UserIDKey: int64(1), settings.TokenKey: "a", + })) + require.NoError(t, a.storeSalt([]byte("a-salt"))) + before := settings.GetAll() + dir := filepath.Dir(a.saltPath) + require.NoError(t, os.Rename(dir, dir+".aside")) + t.Cleanup(func() { require.NoError(t, os.Rename(dir+".aside", dir)) }) + + err := a.PatchAccountSettings(settings.Settings{settings.UserIDKey: int64(2), settings.TokenKey: "b"}) + + require.ErrorContains(t, err, "clearing the previous account's salt") + assert.Equal(t, before, settings.GetAll()) + assert.Equal(t, []byte("a-salt"), a.getSaltCached()) + salt, err := readSalt(filepath.Join(dir+".aside", saltFileName)) + require.NoError(t, err) + assert.Equal(t, []byte("a-salt"), salt) +} + +func TestPatchAccountNoopKeepsPending(t *testing.T) { + a, _ := newTestClient(t) + require.NoError(t, a.PatchAccountSettings(settings.Settings{ + settings.UserIDKey: int64(1), settings.TokenKey: "a", settings.OAuthLoginKey: true, + })) + require.NoError(t, settings.Reload()) + handler, requested, release := heldJSON(UserDataResponse{ + LoginResponse_UserData: &protos.LoginResponse_UserData{UserId: 1, Token: "a", UserLevel: "pro"}, + }) + useProServer(t, a, handler) + errc := make(chan error, 1) + go func() { + _, err := a.FetchUserData(context.Background()) + errc <- err + }() + <-requested + + require.NoError(t, a.PatchAccountSettings(settings.Settings{ + settings.UserIDKey: "1", settings.TokenKey: "a", settings.OAuthLoginKey: true, + })) + close(release) + + require.NoError(t, <-errc) + assert.True(t, settings.IsPro()) +} + +func TestPatchAccountSameKeepsFields(t *testing.T) { + a, _ := newTestClient(t) + require.NoError(t, a.setData(&UserData{ + LegacyID: 1, + LegacyToken: "a", + Token: "a-jwt", + LegacyUserData: &protos.LoginResponse_UserData{UserId: 1, Email: "a@example.com"}, + }, nil)) + require.NoError(t, a.storeSalt([]byte("a-salt"))) + + require.NoError(t, a.PatchAccountSettings(settings.Settings{settings.EmailKey: "new@example.com"})) + + assert.Equal(t, "new@example.com", settings.GetString(settings.EmailKey)) + assert.Equal(t, "a-jwt", settings.GetString(settings.JwtTokenKey)) + assert.Equal(t, []byte("a-salt"), a.getSaltCached()) +} + func TestFetchUserDataKeepsFreeLevel(t *testing.T) { ac, _ := newTestClient(t) ac.setData(&UserData{ LegacyID: 123, LegacyToken: "test-token", LegacyUserData: &protos.LoginResponse_UserData{UserId: 123, UserLevel: "free"}, - }) + }, nil) data, err := ac.FetchUserData(context.Background()) require.NoError(t, err) assert.Equal(t, "free", data.LegacyUserData.UserLevel) diff --git a/account/user_test.go b/account/user_test.go index e0b5d961..77100d93 100644 --- a/account/user_test.go +++ b/account/user_test.go @@ -11,6 +11,7 @@ import ( "math/big" "net/http" "net/http/httptest" + "os" "path/filepath" "testing" @@ -195,7 +196,7 @@ func newTestServer(t *testing.T) (*httptest.Server, *testServer) { } json.NewDecoder(r.Body).Decode(&body) state.referralAttachV1Code = body.Code - writeProtoResponse(w, &protos.BaseResponse{Error: state.referralAttachV1Error}) + writeJSONResponse(w, &protos.BaseResponse{Error: state.referralAttachV1Error}) }) mux.HandleFunc("/referral-attach-v2", func(w http.ResponseWriter, r *http.Request) { @@ -295,7 +296,14 @@ func newTestServer(t *testing.T) (*httptest.Server, *testServer) { func newTestClient(t *testing.T) (*Client, *testServer) { ts, state := newTestServer(t) - settings.InitSettings(t.TempDir()) + // Legacy settings discovery reads these roots; empty ones keep the host's own files out. + for _, v := range []string{"HOME", "XDG_CONFIG_HOME", "APPDATA", "PUBLIC"} { + t.Setenv(v, t.TempDir()) + } + settings.Reset() + dataDir := t.TempDir() + require.NoError(t, settings.InitSettings(dataDir)) + require.NoError(t, settings.Set(settings.DataPathKey, dataDir)) t.Cleanup(settings.Reset) return &Client{ httpClient: ts.Client(), @@ -306,6 +314,15 @@ func newTestClient(t *testing.T) (*Client, *testServer) { }, state } +// failSettingsSaves makes settings saves fail until the returned func restores the data directory. +func failSettingsSaves(t *testing.T) (restore func()) { + t.Helper() + dir := settings.GetString(settings.DataPathKey) + aside := dir + ".aside" + require.NoError(t, os.Rename(dir, aside)) + return func() { require.NoError(t, os.Rename(aside, dir)) } +} + // newTestClientWithSRP creates a test client and pre-registers an email/password on the mock server. func newTestClientWithSRP(t *testing.T, email, password string) (*Client, *testServer) { ac, state := newTestClient(t) @@ -327,6 +344,36 @@ func newTestClientWithSRP(t *testing.T, email, password string) (*Client, *testS return ac, state } +// useProServer points a's pro-server requests at handler for the rest of the test. +func useProServer(t *testing.T, a *Client, handler http.Handler) { + t.Helper() + server := httptest.NewServer(handler) + t.Cleanup(server.Close) + a.proURL = server.URL +} + +func serveJSON(v any) http.HandlerFunc { + return func(w http.ResponseWriter, r *http.Request) { writeJSONResponse(w, v) } +} + +// heldJSON writes v only once release is closed, closing requested when the request arrives. +func heldJSON(v any) (handler http.HandlerFunc, requested, release chan struct{}) { + requested, release = make(chan struct{}), make(chan struct{}) + return func(w http.ResponseWriter, r *http.Request) { + close(requested) + <-release + writeJSONResponse(w, v) + }, requested, release +} + +func setOAuthFlags(t *testing.T) { + t.Helper() + require.NoError(t, settings.Patch(settings.Settings{ + settings.OAuthLoginKey: true, + settings.OAuthProviderKey: "google", + })) +} + func TestSignUp(t *testing.T) { ac, _ := newTestClient(t) settings.Set(settings.TokenKey, "test-token") @@ -335,6 +382,21 @@ func TestSignUp(t *testing.T) { assert.NoError(t, err) assert.NotNil(t, salt) assert.NotNil(t, signupResponse) + saved, err := readSalt(ac.saltPath) + require.NoError(t, err) + assert.Equal(t, salt, saved, "the signup salt must survive a restart") +} + +func TestSignUpKeepsSaltWhenSaveFails(t *testing.T) { + ac, _ := newTestClient(t) + require.NoError(t, settings.Patch(settings.Settings{settings.TokenKey: "t", settings.UserIDKey: "123"})) + ac.salt = []byte("account-salt") + defer failSettingsSaves(t)() + + _, _, err := ac.SignUp(context.Background(), "new@example.com", "password") + + assert.ErrorContains(t, err, "saving signup identity") + assert.Equal(t, []byte("account-salt"), ac.getSaltCached()) } func TestSignupEmailResendCode(t *testing.T) { @@ -359,6 +421,43 @@ func TestLogin(t *testing.T) { assert.NoError(t, err) } +func TestDeviceLimitedLoginStoresDevices(t *testing.T) { + for _, tt := range []struct { + name string + existing *UserData + }{ + {name: "fresh identity"}, + {name: "existing identity", existing: &UserData{ + LegacyID: 7, + LegacyToken: "tok", + Devices: []*protos.LoginResponse_Device{{Id: "stale", Name: "Old"}}, + LegacyUserData: &protos.LoginResponse_UserData{UserId: 7, UserLevel: "free"}, + }}, + } { + t.Run(tt.name, func(t *testing.T) { + a, server := newTestClientWithSRP(t, "test@example.com", "password") + if tt.existing != nil { + a.setData(tt.existing, nil) + } + userDataBefore := settings.Exists(settings.UserDataKey) + server.loginResponse = &UserData{ + Devices: []*protos.LoginResponse_Device{ + {Id: "phone", Name: "Phone"}, + {Id: "laptop", Name: "Laptop"}, + }, + } + + _, err := a.Login(context.Background(), "test@example.com", "password") + require.NoError(t, err) + + devices, err := settings.Devices() + require.NoError(t, err) + assert.Equal(t, []settings.Device{{ID: "phone", Name: "Phone"}, {ID: "laptop", Name: "Laptop"}}, devices) + assert.Equal(t, userDataBefore, settings.Exists(settings.UserDataKey), + "a device-limited login is not a successful login") + }) + } +} func TestLogout(t *testing.T) { ac, _ := newTestClient(t) settings.Set(settings.DeviceIDKey, "deviceId") @@ -366,6 +465,36 @@ func TestLogout(t *testing.T) { assert.NoError(t, err) } +func TestLogoutKeepsAccountWhenSaveFails(t *testing.T) { + ac, _ := newTestClient(t) + require.NoError(t, ac.setData(&UserData{LegacyID: 7, LegacyToken: "tok"}, nil)) + defer failSettingsSaves(t)() + + _, err := ac.Logout(context.Background(), "test@example.com") + + assert.ErrorContains(t, err, "logged out on the server") + assert.Equal(t, int64(7), settings.GetInt64(settings.UserIDKey)) + assert.Equal(t, "tok", settings.GetString(settings.TokenKey)) +} + +func TestLoginKeepsAccountWhenSaveFails(t *testing.T) { + ac, server := newTestClientWithSRP(t, "test@example.com", "password") + require.NoError(t, ac.setData(&UserData{LegacyID: 7, LegacyToken: "tok"}, nil)) + server.loginResponse = &UserData{ + LegacyID: 8, + LegacyToken: "new", + LegacyUserData: &protos.LoginResponse_UserData{UserId: 8, UserLevel: "pro"}, + } + defer failSettingsSaves(t)() + + _, err := ac.Login(context.Background(), "test@example.com", "password") + + assert.ErrorContains(t, err, "saving login") + assert.Equal(t, int64(7), settings.GetInt64(settings.UserIDKey)) + assert.Equal(t, "tok", settings.GetString(settings.TokenKey)) + assert.False(t, settings.IsPro()) +} + func TestLogoutOmitsCredentials(t *testing.T) { const ( legacyToken = "legacy-token-value" @@ -399,6 +528,50 @@ func TestCompleteRecoveryByEmail(t *testing.T) { assert.NoError(t, err) } +func TestRecoverySalt(t *testing.T) { + completeRecovery := func(t *testing.T, a *Client, email string) error { + t.Helper() + return a.CompleteRecoveryByEmail(context.Background(), email, "new-password", "code") + } + + t.Run("current account caches and saves the new salt", func(t *testing.T) { + a, _ := newTestClient(t) + require.NoError(t, settings.Set(settings.EmailKey, "me@example.com")) + a.salt = []byte("old-salt") + + require.NoError(t, completeRecovery(t, a, "Me@example.com")) + + saved, err := readSalt(a.saltPath) + require.NoError(t, err) + assert.NotEqual(t, []byte("old-salt"), a.getSaltCached()) + assert.Equal(t, a.getSaltCached(), saved) + }) + + t.Run("a failed save still replaces the cached salt", func(t *testing.T) { + a, _ := newTestClient(t) + require.NoError(t, settings.Set(settings.EmailKey, "me@example.com")) + a.salt = []byte("old-salt") + a.saltPath = filepath.Join(t.TempDir(), "missing", saltFileName) + + assert.Error(t, completeRecovery(t, a, "me@example.com")) + assert.NotEqual(t, []byte("old-salt"), a.getSaltCached()) + }) + + t.Run("another account's salt is not stored", func(t *testing.T) { + a, _ := newTestClient(t) + require.NoError(t, settings.Set(settings.EmailKey, "me@example.com")) + a.salt = []byte("my-salt") + require.NoError(t, writeSalt([]byte("my-salt"), a.saltPath)) + + require.NoError(t, completeRecovery(t, a, "other@example.com")) + + assert.Equal(t, []byte("my-salt"), a.getSaltCached()) + saved, err := os.ReadFile(a.saltPath) + require.NoError(t, err) + assert.Equal(t, []byte("my-salt"), saved) + }) +} + func TestValidateEmailRecoveryCode(t *testing.T) { ac, _ := newTestClient(t) err := ac.ValidateEmailRecoveryCode(context.Background(), "test@example.com", "code") @@ -436,6 +609,72 @@ func TestCompleteChangeEmail(t *testing.T) { assert.NoError(t, err) } +func TestChangeEmailDiscardsStale(t *testing.T) { + ac, _ := newTestClient(t) + require.NoError(t, ac.setData(&UserData{ + LegacyID: 1, + LegacyToken: "a", + LegacyUserData: &protos.LoginResponse_UserData{UserId: 1, Email: "old@example.com"}, + }, nil)) + handler, requested, release := heldJSON(UserDataResponse{ + LoginResponse_UserData: &protos.LoginResponse_UserData{UserId: 1, Token: "a", Email: "old@example.com"}, + }) + useProServer(t, ac, handler) + errc := make(chan error, 1) + go func() { + _, err := ac.FetchUserData(context.Background()) + errc <- err + }() + <-requested + + require.NoError(t, ac.CompleteChangeEmail(context.Background(), "new@example.com", "password", "code")) + close(release) + + assert.ErrorIs(t, <-errc, ErrAccountChanged) + assert.Equal(t, "new@example.com", settings.GetString(settings.EmailKey)) +} + +func TestChangeEmailSurvivesUpdate(t *testing.T) { + ac, _ := newTestClient(t) + require.NoError(t, ac.setData(&UserData{ + LegacyID: 1, + LegacyToken: "a", + LegacyUserData: &protos.LoginResponse_UserData{UserId: 1, Email: "old@example.com"}, + }, nil)) + ac.salt = []byte("old-salt") + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + assert.NoError(t, ac.PatchAccountSettings(settings.Settings{settings.UserLevelKey: "pro"})) + writeProtoResponse(w, &protos.EmptyResponse{}) + })) + defer server.Close() + ac.authURL = server.URL + + require.NoError(t, ac.CompleteChangeEmail(context.Background(), "new@example.com", "password", "code")) + + assert.Equal(t, "new@example.com", settings.GetString(settings.EmailKey)) + assert.NotEqual(t, []byte("old-salt"), ac.getSaltCached()) +} + +func TestChangeEmailSkipsOtherAccount(t *testing.T) { + ac, _ := newTestClient(t) + require.NoError(t, ac.setData(&UserData{ + LegacyID: 1, + LegacyToken: "a", + LegacyUserData: &protos.LoginResponse_UserData{UserId: 1, Email: "old@example.com"}, + }, nil)) + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + assert.NoError(t, ac.PatchAccountSettings(settings.Settings{settings.UserIDKey: int64(2), settings.TokenKey: "b"})) + writeProtoResponse(w, &protos.EmptyResponse{}) + })) + defer server.Close() + ac.authURL = server.URL + + assert.Error(t, ac.CompleteChangeEmail(context.Background(), "new@example.com", "password", "code")) + + assert.Equal(t, int64(2), settings.GetInt64(settings.UserIDKey)) + assert.Empty(t, settings.GetString(settings.EmailKey)) +} + func TestDeleteAccount(t *testing.T) { email := "test@example.com" ac, _ := newTestClientWithSRP(t, email, "password") @@ -444,6 +683,18 @@ func TestDeleteAccount(t *testing.T) { assert.NoError(t, err) } +func TestDeleteAccountKeepsAccountWhenSaveFails(t *testing.T) { + email := "test@example.com" + ac, _ := newTestClientWithSRP(t, email, "password") + require.NoError(t, ac.setData(&UserData{LegacyID: 7, LegacyToken: "tok"}, nil)) + defer failSettingsSaves(t)() + + _, err := ac.DeleteAccount(context.Background(), email, "password") + + assert.ErrorContains(t, err, "account deleted on the server") + assert.Equal(t, int64(7), settings.GetInt64(settings.UserIDKey)) +} + func TestOAuthLoginUrl(t *testing.T) { ac, _ := newTestClient(t) url, err := ac.OAuthLoginURL(context.Background(), "google") @@ -458,7 +709,7 @@ func TestOAuthLoginCallback(t *testing.T) { LegacyID: 456, Token: "old-jwt", Devices: []*protos.LoginResponse_Device{{Id: "old-device"}}, LegacyUserData: &protos.LoginResponse_UserData{UserId: 456}, - }) + }, nil) // Mock JWT with unverified signature — decodeJWT uses ParseUnverified so this succeeds. mockToken := "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJlbWFpbCI6InRlc3RAZXhhbXBsZS5jb20iLCJsZWdhY3lfdXNlcl9pZCI6MTIzNDUsImxlZ2FjeV90b2tlbiI6InRlc3QtdG9rZW4ifQ.test" @@ -472,14 +723,139 @@ func TestOAuthLoginCallback(t *testing.T) { assert.Empty(t, devices) } +func TestOAuthLoginDiscardedOnSwitch(t *testing.T) { + ac, _ := newTestClient(t) + useProServer(t, ac, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + assert.NoError(t, ac.PatchAccountSettings(settings.Settings{settings.UserIDKey: int64(99), settings.TokenKey: "other"})) + writeJSONResponse(w, UserDataResponse{ + LoginResponse_UserData: &protos.LoginResponse_UserData{UserId: 12345, Token: "test-token"}, + }) + })) + mockToken := "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJlbWFpbCI6InRlc3RAZXhhbXBsZS5jb20iLCJsZWdhY3lfdXNlcl9pZCI6MTIzNDUsImxlZ2FjeV90b2tlbiI6InRlc3QtdG9rZW4ifQ.test" + + _, err := ac.OAuthLoginCallback(context.Background(), mockToken) + + assert.ErrorIs(t, err, ErrAccountChanged) + assert.Equal(t, int64(99), settings.GetInt64(settings.UserIDKey)) + assert.Empty(t, settings.GetString(settings.JwtTokenKey)) + assert.False(t, settings.GetBool(settings.OAuthLoginKey)) +} + +func TestOAuthLoginSavesAccount(t *testing.T) { + ac, _ := newTestClient(t) + require.NoError(t, ac.setData(&UserData{ + LegacyID: 1, LegacyToken: "old-token", Token: "old-jwt", + LegacyUserData: &protos.LoginResponse_UserData{UserId: 1, Email: "old@example.com"}, + }, nil)) + require.NoError(t, ac.storeSalt([]byte("old-salt"))) + require.NoError(t, settings.Set(settings.DeviceIDKey, "device")) + require.NoError(t, settings.Set(settings.OAuthProviderKey, "google")) + before := currentAccount() + generation := ac.identityGeneration() + useProServer(t, ac, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + assert.Equal(t, "2", r.Header.Get(common.UserIDHeader)) + assert.Equal(t, "oauth-token", r.Header.Get(common.ProTokenHeader)) + assert.Equal(t, "device", r.Header.Get(common.DeviceIDHeader)) + assert.Equal(t, before, currentAccount()) + assert.Equal(t, []byte("old-salt"), ac.getSaltCached()) + writeJSONResponse(w, UserDataResponse{ + LoginResponse_UserData: &protos.LoginResponse_UserData{UserId: 2, UserLevel: "pro"}, + }) + })) + token := mockJWT(t, map[string]any{ + "legacy_user_id": 2, "legacy_token": "oauth-token", "email": "new@example.com", + }) + + user, err := ac.OAuthLoginCallback(context.Background(), token) + + require.NoError(t, err) + require.NotNil(t, user) + assert.Equal(t, generation+1, ac.identityGeneration()) + assert.Equal(t, "new@example.com", user.Id) + assert.True(t, user.EmailConfirmed) + assert.Equal(t, token, user.Token) + assert.Equal(t, "oauth-token", user.LegacyToken) + assert.Equal(t, "new@example.com", user.LegacyUserData.Email) + assert.Equal(t, "device", user.LegacyUserData.DeviceID) + require.NoError(t, settings.Reload()) + assert.Equal(t, int64(2), settings.GetInt64(settings.UserIDKey)) + assert.Equal(t, "oauth-token", settings.GetString(settings.TokenKey)) + assert.Equal(t, token, settings.GetString(settings.JwtTokenKey)) + assert.Equal(t, "new@example.com", settings.GetString(settings.EmailKey)) + assert.True(t, settings.GetBool(settings.OAuthLoginKey)) + assert.Equal(t, "google", settings.GetString(settings.OAuthProviderKey)) + assert.True(t, settings.IsPro()) + var cached UserData + require.NoError(t, settings.GetStruct(settings.UserDataKey, &cached)) + assert.True(t, proto.Equal(user, &cached)) + assert.Nil(t, ac.getSaltCached()) +} + +func TestOAuthLoginFailureKeepsAccount(t *testing.T) { + for _, tt := range []struct { + name string + status int + body string + failSave bool + }{ + {name: "request", status: http.StatusServiceUnavailable, body: "unavailable"}, + {name: "json", status: http.StatusOK, body: "invalid"}, + {name: "missing user data", status: http.StatusOK, body: "{}"}, + {name: "missing identity", status: http.StatusOK, body: `{"email":"new@example.com"}`}, + {name: "save", status: http.StatusOK, body: `{"userId":2,"token":"oauth-token"}`, failSave: true}, + } { + t.Run(tt.name, func(t *testing.T) { + ac, _ := newTestClient(t) + require.NoError(t, ac.setData(&UserData{ + LegacyID: 1, LegacyToken: "old-token", Token: "old-jwt", + LegacyUserData: &protos.LoginResponse_UserData{UserId: 1, Email: "old@example.com"}, + }, nil)) + require.NoError(t, ac.storeSalt([]byte("old-salt"))) + before := settings.GetAll() + useProServer(t, ac, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(tt.status) + _, _ = io.WriteString(w, tt.body) + })) + if tt.failSave { + defer failSettingsSaves(t)() + } + token := mockJWT(t, map[string]any{"legacy_user_id": 2, "legacy_token": "oauth-token"}) + + user, err := ac.OAuthLoginCallback(context.Background(), token) + + require.Error(t, err) + assert.Nil(t, user) + assert.Equal(t, before, settings.GetAll()) + assert.Equal(t, []byte("old-salt"), ac.getSaltCached()) + }) + } +} + func TestOAuthLoginCallback_InvalidToken(t *testing.T) { ac, _ := newTestClient(t) _, err := ac.OAuthLoginCallback(context.Background(), "invalid-token") - assert.Error(t, err) + assert.ErrorIs(t, err, ErrInvalidToken) assert.Contains(t, err.Error(), "error decoding JWT") } +func TestOAuthLoginRejectsNoIdentity(t *testing.T) { + for _, claims := range []map[string]any{ + {"legacy_token": "oauth-token"}, + {"legacy_user_id": 2}, + } { + ac, _ := newTestClient(t) + useProServer(t, ac, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + t.Error("no request should be sent for a token without an account identity") + })) + + _, err := ac.OAuthLoginCallback(context.Background(), mockJWT(t, claims)) + + assert.ErrorIs(t, err, ErrInvalidToken) + } +} + // mockJWT builds an unsigned JWT from the given claims; decodeJWT uses // ParseUnverified, so the fake signature is never checked. func mockJWT(t *testing.T, claims map[string]any) string { @@ -560,3 +936,23 @@ func TestReferralAttach_V2Error(t *testing.T) { assert.Nil(t, resp) assert.Contains(t, err.Error(), "invalid code") } + +func TestOAuthFlagsCleared(t *testing.T) { + t.Run("ClearUser", func(t *testing.T) { + a, _ := newTestClient(t) + setOAuthFlags(t) + a.ClearUser() + assert.False(t, settings.GetBool(settings.OAuthLoginKey)) + assert.Empty(t, settings.GetString(settings.OAuthProviderKey)) + }) + + t.Run("SignUp", func(t *testing.T) { + a, _ := newTestClient(t) + require.NoError(t, settings.Patch(settings.Settings{settings.TokenKey: "t", settings.UserIDKey: "123"})) + setOAuthFlags(t) + _, _, err := a.SignUp(context.Background(), "new@example.com", "password") + require.NoError(t, err) + assert.False(t, settings.GetBool(settings.OAuthLoginKey)) + assert.Empty(t, settings.GetString(settings.OAuthProviderKey)) + }) +} diff --git a/backend/radiance.go b/backend/radiance.go index d01e9ca9..0ec973a2 100644 --- a/backend/radiance.go +++ b/backend/radiance.go @@ -142,8 +142,8 @@ func NewLocalBackend(ctx context.Context, opts Options) (*LocalBackend, error) { slog.Warn("Failed to apply some env overrides", "error", envOverrideErrs) } dataDir := settings.GetString(settings.DataPathKey) - if err := settings.Patch(startupSettings(opts)); err != nil { - slog.Warn("Persisting startup settings", "error", err) + if err := settings.Apply(startupSettings(opts)); err != nil { + slog.Warn("Failed to save startup settings", "error", err) } platformDeviceID := settings.GetString(settings.DeviceIDKey) @@ -715,12 +715,20 @@ func (r *LocalBackend) PatchSettings(updates settings.Settings) error { diff = maps.Clone(diff) delete(diff, settings.PeerShareEnabledKey) } + patch := settings.Patch + for _, key := range settings.UserKeys { + if value, ok := updates[key]; ok { + // Account fields must be applied together against the identity active at commit time. + diff[key] = value + patch = r.accountClient.PatchAccountSettings + } + } // The rest of the patch is persisted before the toggle applies, so a // peer it starts registers with this patch's peer settings, such as // peer_serve_pool, rather than the previous ones. var patchErr error if len(diff) > 0 { - patchErr = settings.Patch(diff) + patchErr = patch(diff) } if peerRequested { if err := r.applyPeerShare(peerOn); err != nil { @@ -749,7 +757,7 @@ func (r *LocalBackend) PatchSettings(updates settings.Settings) error { // vpn settings // - // settings.Patch above already persisted the whole diff, so an early return + // The patch above already persisted the whole diff, so an early return // on a handler error would leave a persisted key that no runtime state // matches — the divergence applyPeerShare's rollback exists to prevent. Run // every handler and join their errors so the caller sees write failures. @@ -768,7 +776,7 @@ func (r *LocalBackend) PatchSettings(updates settings.Settings) error { } // Drive the Unbounded widget proxy off the toggle change immediately // rather than waiting for the next NewConfigEvent to re-evaluate. - // settings.Patch above has already persisted the new value, so go + // The patch above has already persisted the new value, so go // straight to Apply() — SetEnabled would short-circuit on the // already-matching persisted value and never re-evaluate the // manager. Apply re-checks the three-condition predicate against diff --git a/backend/user_messages_test.go b/backend/user_messages_test.go index 0b44f24a..ef03e797 100644 --- a/backend/user_messages_test.go +++ b/backend/user_messages_test.go @@ -30,6 +30,62 @@ type accountTransportFunc func(*http.Request) (*http.Response, error) func (f accountTransportFunc) RoundTrip(req *http.Request) (*http.Response, error) { return f(req) } +func TestPatchSettingsAccountKeysDiscardStaleUserData(t *testing.T) { + settings.Reset() + t.Cleanup(settings.Reset) + require.NoError(t, settings.InitSettings(t.TempDir())) + require.NoError(t, settings.Patch(settings.Settings{settings.UserIDKey: int64(1), settings.TokenKey: "a"})) + requested, release := make(chan struct{}), make(chan struct{}) + a := account.NewClient(&http.Client{Transport: accountTransportFunc(func(req *http.Request) (*http.Response, error) { + close(requested) + <-release + return &http.Response{ + StatusCode: http.StatusOK, + Header: make(http.Header), + Body: io.NopCloser(strings.NewReader(`{"userId":1,"token":"a","userLevel":"pro"}`)), + Request: req, + }, nil + })}, t.TempDir(), log.NoOpLogger()) + r := &LocalBackend{accountClient: a} + errc := make(chan error, 1) + go func() { + _, err := a.FetchUserData(context.Background()) + errc <- err + }() + <-requested + + require.NoError(t, r.PatchSettings(settings.Settings{settings.UserIDKey: int64(2), settings.TokenKey: "b"})) + close(release) + + require.ErrorIs(t, <-errc, account.ErrAccountChanged) + require.Equal(t, int64(2), settings.GetInt64(settings.UserIDKey)) + require.False(t, settings.IsPro()) +} + +func TestPatchSettingsSwitchKeepsMatches(t *testing.T) { + settings.Reset() + t.Cleanup(settings.Reset) + require.NoError(t, settings.InitSettings(t.TempDir())) + require.NoError(t, settings.Patch(settings.Settings{ + settings.UserIDKey: int64(1), + settings.TokenKey: "a", + settings.UserLevelKey: "pro", + settings.OAuthLoginKey: true, + })) + r := &LocalBackend{accountClient: account.NewClient(http.DefaultClient, t.TempDir(), log.NoOpLogger())} + + require.NoError(t, r.PatchSettings(settings.Settings{ + settings.UserIDKey: int64(2), + settings.TokenKey: "b", + settings.UserLevelKey: "pro", + settings.OAuthLoginKey: true, + })) + + require.Equal(t, int64(2), settings.GetInt64(settings.UserIDKey)) + require.True(t, settings.IsPro()) + require.True(t, settings.GetBool(settings.OAuthLoginKey)) +} + func TestUserMessagesFollowAccountEvents(t *testing.T) { settings.Reset() t.Cleanup(settings.Reset) diff --git a/common/init.go b/common/init.go index 1cce1d0d..a741d480 100644 --- a/common/init.go +++ b/common/init.go @@ -81,7 +81,9 @@ func Init(dataDir, logDir, logLevel string) (err error) { return fmt.Errorf("failed to initialize settings: %w", err) } - settings.Patch(settings.Settings{settings.DataPathKey: data, settings.LogPathKey: logs}) + if err := settings.Apply(settings.Settings{settings.DataPathKey: data, settings.LogPathKey: logs}); err != nil { + slog.Warn("Failed to save startup settings", "error", err) + } seedLogLevel(logLevel) logger := log.NewLogger(log.Config{ @@ -228,14 +230,8 @@ func setupDirectories(data, logs string) (dataDir, logDir string, err error) { } else if logs == "" { logs = internal.DefaultLogPath() } - // Honor the caller's path as-is. A previous version of this function - // unconditionally appended /data and /logs suffixes here even when the - // caller passed a fully-resolved path (e.g. Android passes - // /.lantern). That broke upgrade continuity: v9.0.x had - // written settings.json under /, while v9.1.x reads from - // /data/, so every existing install lost its persisted - // user_id, device_id, jwt token, and user_level on upgrade — surfacing - // as "Pro is suddenly expired after the update." + // Use the caller's path as-is; earlier versions' settings live directly under it, so a + // suffix hides them from migration. data, _ = filepath.Abs(data) logs, _ = filepath.Abs(logs) for _, path := range []string{data, logs} { diff --git a/common/settings/legacy_crossdir.go b/common/settings/legacy_crossdir.go index ed2e7392..51db2b63 100644 --- a/common/settings/legacy_crossdir.go +++ b/common/settings/legacy_crossdir.go @@ -2,7 +2,6 @@ package settings import ( "errors" - "fmt" "io/fs" "log/slog" "os" @@ -15,15 +14,10 @@ import ( // lookup to a temp dir without touching the host's %PUBLIC%. var windowsCrossDirCandidatesFn = windowsCrossDirCandidates -// windowsCrossDirCandidates returns settings candidates from the v9.0.x -// Windows data directory (${PUBLIC}\Lantern\data), which the same-dir -// migration doesn't cover: radiance's data dir moved to -// ${ProgramData}\Lantern and the two directories don't share a parent, so -// the same-dir candidates never see the v9.0.x file. -// -// Returns nil on non-Windows hosts or when %PUBLIC% is unset. +// windowsCrossDirCandidates returns the v9.0.x local.json from ${PUBLIC}\Lantern\data, which the +// same-dir candidates miss because radiance's data dir moved to ${ProgramData}\Lantern. func windowsCrossDirCandidates(fileDir string) []candidateSource { - if runtime.GOOS != "windows" { + if runtime.GOOS != "windows" || !readsHostProfile() { return nil } pub := os.Getenv("PUBLIC") @@ -57,34 +51,22 @@ func samePathWindows(a, b string) bool { // readWindowsCrossDirCandidates is split out so tests can drive the path // resolution directly without needing to spoof %PUBLIC% / GOOS. func readWindowsCrossDirCandidates(v90xDir string) []candidateSource { - // legacySettingsFileName is tried first because it's the actual - // v9.0.x name; settingsFileName is included as a defensive - // fallback for users whose v9.0.x file got renamed by a partial / - // failed earlier upgrade attempt. - specs := []struct { - name, label string - }{ - {legacySettingsFileName, fmt.Sprintf("v9.0.x Windows %s", filepath.Join(v90xDir, legacySettingsFileName))}, - {settingsFileName, fmt.Sprintf("v9.0.x Windows %s", filepath.Join(v90xDir, settingsFileName))}, + // settings.json is deliberately not read: in this directory it is the live UI's settings, not a + // v9.0.x artifact, so adopting it would import whatever the UI last wrote. + full := filepath.Join(v90xDir, legacySettingsFileName) + label := "v9.0.x Windows " + full + b, err := os.ReadFile(full) + if errors.Is(err, fs.ErrNotExist) { + return nil } - var out []candidateSource - for _, s := range specs { - full := filepath.Join(v90xDir, s.name) - b, err := os.ReadFile(full) - switch { - case err == nil: - out = append(out, candidateSource{ - path: full, - contents: b, - exists: true, - label: s.label, - }) - case errors.Is(err, fs.ErrNotExist): - // Expected — fresh install or that filename never existed for this user. - default: - slog.Warn("v9.0.x Windows cross-dir read failed", - "path", full, "error", err) - } + if err != nil { + slog.Warn("legacy settings migration: read failed", "path", full, "error", err) + return nil } - return out + return []candidateSource{{ + path: full, + contents: b, + exists: true, + label: label, + }} } diff --git a/common/settings/legacy_yaml.go b/common/settings/legacy_yaml.go index 9e38b4a2..668e4d1c 100644 --- a/common/settings/legacy_yaml.go +++ b/common/settings/legacy_yaml.go @@ -26,11 +26,13 @@ func legacyYAMLCandidate(fileDir string) candidateSource { if path == "" { return candidateSource{} } + label := fmt.Sprintf("pre-9.x %s yaml", layout) raw, err := os.ReadFile(path) + if errors.Is(err, fs.ErrNotExist) { + return candidateSource{} + } if err != nil { - if !errors.Is(err, fs.ErrNotExist) { - slog.Warn("pre-9.x yaml read failed", "path", path, "error", err) - } + slog.Warn("legacy settings migration: read failed", "path", path, "error", err) return candidateSource{} } translated, err := translateLegacyYAML(raw, layout) @@ -42,17 +44,23 @@ func legacyYAMLCandidate(fileDir string) candidateSource { path: path, contents: translated, exists: true, - label: fmt.Sprintf("pre-9.x %s yaml", layout), + label: label, } } func legacyYAMLPath(fileDir string) (path, layout string) { switch runtime.GOOS { case "darwin", "windows": + if !readsHostProfile() { + break + } if cfg, err := os.UserConfigDir(); err == nil { return filepath.Join(cfg, "Lantern", "settings.yaml"), "desktop" } case "linux": + if !readsHostProfile() { + break + } // Pre-9.x appdir lowercased the app name on linux only. if cfg, err := os.UserConfigDir(); err == nil { return filepath.Join(cfg, "lantern", "settings.yaml"), "desktop" diff --git a/common/settings/legacy_yaml_test.go b/common/settings/legacy_yaml_test.go index 03a591a2..6430416a 100644 --- a/common/settings/legacy_yaml_test.go +++ b/common/settings/legacy_yaml_test.go @@ -11,8 +11,7 @@ import ( func TestTranslateLegacyYAML_Desktop(t *testing.T) { t.Run("pro user with all fields", func(t *testing.T) { // Shape mirrors what the pre-9.x desktop client (flashlight + - // lantern-client) wrote into ~/.lantern/settings.yaml / - // %APPDATA%\Lantern\settings.yaml / ~/.config/lantern/settings.yaml. + // lantern-client) wrote to the paths legacyYAMLPath resolves. yaml := []byte(`userID: 3580849 deviceID: 84e9c7b2-2a54-44f3-9ec6-276086017e49 userPro: true @@ -42,7 +41,7 @@ userToken: tok `) out, err := translateLegacyYAML(yaml, "desktop") require.NoError(t, err) - assert.Equal(t, "free", userLevelInJSON(out), + assert.Equal(t, "free", userLevelIn(t, out), "a known but non-pro user should translate to user_level=free") }) @@ -52,7 +51,7 @@ userToken: "" `) out, err := translateLegacyYAML(yaml, "desktop") require.NoError(t, err) - assert.Equal(t, "", userLevelInJSON(out), + assert.Equal(t, "", userLevelIn(t, out), "no user_id means we shouldn't claim 'free'; let next login decide") }) @@ -62,6 +61,15 @@ userToken: "" }) } +func userLevelIn(t *testing.T, contents []byte) string { + t.Helper() + var got struct { + UserLevel string `json:"user_level"` + } + require.NoError(t, json.Unmarshal(contents, &got)) + return got.UserLevel +} + func TestTranslateLegacyYAML_iOS(t *testing.T) { yaml := []byte(`AppName: lantern DeviceID: ios-device-9000 diff --git a/common/settings/settings.go b/common/settings/settings.go index 802ee996..4bc5587d 100644 --- a/common/settings/settings.go +++ b/common/settings/settings.go @@ -2,6 +2,7 @@ package settings import ( + "bytes" "context" jsonpkg "encoding/json" "errors" @@ -10,6 +11,7 @@ import ( "log/slog" "os" "path/filepath" + "reflect" "strconv" "strings" "sync" @@ -79,15 +81,15 @@ const ( PreferredLocationKey _key = "preferred_location" // [common.PreferredLocation] settingsFileName = "settings.json" - // legacySettingsFileName is what v9.0.x called the same file (it was - // renamed). On upgrade from v9.0.x, the user's - // persisted user_id / token / user_level live at /local.json; - // migrateLegacySettingsIfNeeded reads it from there so Pro state - // survives the rename. + // legacySettingsFileName is v9.0.x's name for the settings file. legacySettingsFileName = "local.json" settingsInvalidFileName = "settings.invalid.json" + // migrationMarkerFileName lives beside settings.json rather than inside it, so losing the + // settings file doesn't re-run a migration that already adopted an identity. + migrationMarkerFileName = ".settings-migrated" + // Development only keys ConfigFetchDisabledKey _key = "config_fetch_disabled" // bool FeatureOverridesKey _key = "feature_overrides" // string @@ -111,13 +113,12 @@ type settings struct { initialized bool filePath string mu sync.Mutex - // pending holds writes that have not reached the file, reapplied on top - // of it until one persists, so a later write cannot resurrect the state - // they replaced. Guarded by mu. + // pending holds [Apply] writes that have not reached the file, reapplied on top of it until one + // persists, so a later write cannot resurrect the state they replaced. Guarded by mu. pending map[string]pendingWrite } -// pendingWrite is one unpersisted write; deleted marks a tombstone. +// pendingWrite is one key's write; deleted removes the key. type pendingWrite struct { value any deleted bool @@ -151,7 +152,8 @@ func clone(src *koanf.Koanf) (*koanf.Koanf, error) { return next, nil } -// InitSettings initializes the config for user settings. +// InitSettings initializes the config for user settings. Before it succeeds, writes are held in +// memory only, and the file's values take precedence over them when it loads. func InitSettings(fileDir string) error { k.mu.Lock() defer k.mu.Unlock() @@ -162,41 +164,80 @@ func InitSettings(fileDir string) error { return fmt.Errorf("failed to create data directory: %v", err) } k.filePath = filepath.Join(fileDir, settingsFileName) - // Migration candidates reach outside fileDir — the pre-9.x YAML lives in - // the user's real Lantern profile — so under test it would import the - // developer's live account, pro token included, into the test's store. - if !testing.Testing() && !env.GetBool(env.Testing) { - migrateLegacySettingsIfNeeded(fileDir, k.filePath) + defer func() { + if !k.initialized { + k.filePath = "" + } + }() + markerPath := filepath.Join(fileDir, migrationMarkerFileName) + migrate := migrationPending(markerPath) + var identity map[string]any + if migrate { + identity = legacyIdentity(fileDir, k.filePath) } next, err := clone(snapshot()) if err != nil { return err } + fresh := false switch err := loadInto(next, k.filePath); { case errors.Is(err, fs.ErrNotExist): slog.Warn("settings file not found", "path", k.filePath) // file may not have been created yet - if err := save(next); err != nil { - return err - } + fresh = true case errors.Is(err, errParseSettings): - // An invalid settings file must not be fatal: quarantine it, then start - // from in-memory defaults and overwrite the bad file with them. + // An invalid settings file must not be fatal; quarantine it before it's overwritten. slog.Error("Settings file is invalid; starting from defaults", "path", k.filePath, "error", err) quarantineInvalidSettings(k.filePath) + fresh = true + case err != nil: + return fmt.Errorf("loading settings: %w", err) + } + if identity != nil { + adopted, err := clone(next) + if err == nil { + err = adoptIdentity(adopted, identity) + } + if err == nil { + err = save(adopted) + } + switch { + case err == nil: + next = adopted + fresh = false + case fresh: + return err + default: + slog.Warn("legacy settings migration: saving adopted identity failed; will retry", "error", err) + migrate = false + } + } + if fresh { if err := save(next); err != nil { return err } - case err != nil: - return fmt.Errorf("loading settings: %w", err) + } + if migrate { + if err := atomicfile.WriteFile(markerPath, nil, fileperm.File); err != nil { + slog.Warn("legacy settings migration: writing marker failed; will retry", "path", markerPath, "error", err) + } } k.k.Store(next) k.initialized = true return nil } -// candidateSource is one possible location of persisted user state. -// contents is always canonical JSON — direct for v9.x, translated for -// pre-9.x YAML. +// migrationPending reports whether the legacy migration has yet to complete. An unreadable marker +// counts as complete, because re-adopting a legacy identity can undo a logout. +func migrationPending(markerPath string) bool { + _, err := os.Stat(markerPath) + if err != nil && !errors.Is(err, fs.ErrNotExist) { + slog.Warn("legacy settings migration: marker unreadable; skipping", "path", markerPath, "error", err) + } + return errors.Is(err, fs.ErrNotExist) +} + +// candidateSource is one possible location of persisted user state. contents is v9.x JSON, or +// pre-9.x YAML translated to JSON. type candidateSource struct { path string contents []byte @@ -204,19 +245,30 @@ type candidateSource struct { label string } -// migrateLegacySettingsIfNeeded preserves valid canonical settings and recovers -// missing or invalid settings from older client versions, preferring Pro accounts. -func migrateLegacySettingsIfNeeded(fileDir, canonicalPath string) { - raw, err := os.ReadFile(canonicalPath) - canonicalExists := err == nil - switch { +// legacyIdentity returns the identity keys to adopt from settings written by older client +// versions, or nil when nothing should be adopted or canonicalPath can't be read. A legacy candidate +// that can't be read counts as missing. +// +// Legacy candidates in priority order: +// +// 1. /local.json — v9.0.x (renamed) +// 2. ${PUBLIC}\Lantern\data\local.json — v9.0.x cross-dir, Windows only +// 3. pre-9.x platform-specific YAML +// 4. /data/settings.json — v9.1.x (misplaced under data/) +// +// A canonical file holding a JSON object always wins, even without an account, because it is +// state this client already accepted. Otherwise a legacy candidate counts only if it holds a JSON +// object: pick the highest-priority one with user_level=="pro", or if none is pro, the +// highest-priority one with a non-zero user_id, so the device registration is kept. +func legacyIdentity(fileDir, canonicalPath string) map[string]any { + switch raw, err := os.ReadFile(canonicalPath); { case err == nil: - if validSettingsJSON(raw) { - return + if p, err := (settingsJSON{}).Unmarshal(raw); err == nil && p != nil { + return nil } case !errors.Is(err, fs.ErrNotExist): - slog.Warn("legacy settings migration: read failed", "path", canonicalPath, "error", err) - return + // InitSettings fails on the same read before marking the migration complete. + return nil } candidates := []candidateSource{ {path: filepath.Join(fileDir, legacySettingsFileName), label: "v9.0.x local.json"}, @@ -226,90 +278,95 @@ func migrateLegacySettingsIfNeeded(fileDir, canonicalPath string) { b, err := os.ReadFile(candidates[i].path) switch { case err == nil: - if validSettingsJSON(b) { - candidates[i].contents = b - candidates[i].exists = true - } + candidates[i].contents = b + candidates[i].exists = true case errors.Is(err, fs.ErrNotExist): - // Expected — file just isn't there. Treat as not-present. default: - slog.Warn("legacy settings migration: read failed", - "path", candidates[i].path, "error", err) + slog.Warn("legacy settings migration: read failed", "path", candidates[i].path, "error", err) } } + // Optional candidates go after local.json, inserted oldest-first so newer ones rank higher. if yc := legacyYAMLCandidate(fileDir); yc.exists { candidates = append(candidates[:1], append([]candidateSource{yc}, candidates[1:]...)...) } - // Windows v9.0.x cross-dir candidates (${PUBLIC}\Lantern\data) are the - // same generation of state as the same-dir local.json, just stored - // under a different filesystem root because lanternd's data dir moved - // to ${ProgramData}\Lantern. On every other GOOS / when the - // env is unset windowsCrossDirCandidatesFn returns nil and this is a - // no-op. + // Cross-dir candidates are the same generation as local.json. if winExtras := windowsCrossDirCandidatesFn(fileDir); len(winExtras) > 0 { candidates = append(candidates[:1], append(winExtras, candidates[1:]...)...) } - - // Pick: highest-priority file with user_level=="pro"; if none has pro, - // highest-priority file that exists at all (with non-empty contents). - pickIdx := -1 + parsed := make([]map[string]any, len(candidates)) for i, c := range candidates { - if c.exists && userLevelInJSON(c.contents) == "pro" { + if !c.exists { + continue + } + p, err := (settingsJSON{}).Unmarshal(c.contents) + if err != nil || p == nil { + slog.Warn("legacy settings migration: ignoring invalid candidate", "path", c.path, "error", err) + continue + } + parsed[i] = p + } + pickIdx := -1 + for i, p := range parsed { + if p != nil && p[UserLevelKey.String()] == "pro" { pickIdx = i break } } if pickIdx == -1 { - for i, c := range candidates { - if c.exists && validSettingsJSON(c.contents) { + for i, p := range parsed { + if hasAccount(p) { pickIdx = i break } } } if pickIdx == -1 { - // Nothing on disk yet — fresh install, normal path. No-op. - return + return nil } - if canonicalExists { - quarantineInvalidSettings(canonicalPath) + identity := make(map[string]any) + for _, key := range identityKeys { + if v, ok := parsed[pickIdx][key.String()]; ok { + identity[key.String()] = v + } } - writeMigrated(canonicalPath, candidates[pickIdx].contents, candidates[pickIdx].label) + slog.Info("legacy settings migration: recovering persisted identity", "source", candidates[pickIdx].label) + return identity } -func validSettingsJSON(contents []byte) bool { - var values map[string]jsonpkg.RawMessage - return jsonpkg.Unmarshal(contents, &values) == nil && values != nil +func hasAccount(settings map[string]any) bool { + id := toString(settings[UserIDKey.String()]) + return id != "" && id != "0" } -// writeMigrated overwrites the canonical settings file with the recovered -// contents and logs the outcome. Uses atomicfile.WriteFile (the same -// mechanism the normal save path uses) so a crash mid-write can't leave -// a half-written settings.json on disk. Errors are logged-and-swallowed: -// if the write fails the caller falls through to the fresh-install path, -// which is a worse UX but not a corruption risk. -func writeMigrated(canonicalPath string, contents []byte, source string) { - if err := atomicfile.WriteFile(canonicalPath, contents, fileperm.File); err != nil { - slog.Warn("legacy settings migration: write failed", - "dst", canonicalPath, "source", source, "error", err) - return - } - slog.Info("legacy settings migration: recovered persisted state", - "dst", canonicalPath, "source", source, "bytes", len(contents)) +var identityKeys = []_key{ + UserIDKey, TokenKey, UserLevelKey, EmailKey, DeviceIDKey, + JwtTokenKey, OAuthLoginKey, OAuthProviderKey, +} + +// UserKeys are the settings that belong to the active user rather than the device. +var UserKeys = []_key{ + UserIDKey, TokenKey, UserLevelKey, EmailKey, + JwtTokenKey, DevicesKey, UserDataKey, OAuthLoginKey, OAuthProviderKey, } -// userLevelInJSON returns the value of the "user_level" key from a JSON -// settings blob, or "" if the key is missing / the blob is malformed. -// Lightweight extraction so the migration doesn't need to load the full -// koanf state machine before we've decided which file to read. -func userLevelInJSON(contents []byte) string { - var s struct { - UserLevel string `json:"user_level"` +// readsHostProfile reports whether the migration may read legacy files outside the data dir. Under +// test they would import the developer's live Lantern account into the test's store. +func readsHostProfile() bool { + return !testing.Testing() && !env.GetBool(env.Testing) +} + +func adoptIdentity(kk *koanf.Koanf, identity map[string]any) error { + if toString(kk.Get(UserIDKey.String())) != toString(identity[UserIDKey.String()]) { + for _, key := range UserKeys { + kk.Delete(key.String()) + } } - if err := jsonpkg.Unmarshal(contents, &s); err != nil { - return "" + for key, v := range identity { + if err := kk.Set(key, v); err != nil { + return fmt.Errorf("could not set key %s: %w", key, err) + } } - return s.UserLevel + return nil } // loadInto merges the settings file at path into target. Koanf parses before @@ -347,13 +404,16 @@ func Get(key _key) any { } func GetString(key _key) string { - value := Get(key) + return toString(Get(key)) +} + +func toString(value any) string { if value == nil { return "" } switch v := value.(type) { case float64: - // Direct callers can still supply floating-point IDs. + // Callers can store float64 IDs, which %v would render in scientific notation. return strconv.FormatInt(int64(v), 10) case string: return v @@ -410,21 +470,27 @@ func Exists(key _key) bool { return snapshot().Exists(key.String()) } -// mutate applies the writes returned by produce on top of the file's current -// contents, not this process's snapshot, so a write can't clobber keys another -// process changed; produce sees that same state and can decide against it. The -// next snapshot is published even when persisting fails, so the new value is -// visible in this process. +// mutate applies the writes returned by produce on top of the file's current contents, not this +// process's snapshot, so a write can't clobber keys another process changed; produce sees that +// same state and can decide against it. The writes take effect only once saved; on error the +// previous settings remain. func mutate(produce func(next *koanf.Koanf) (map[string]pendingWrite, error)) error { + return write(produce, false) +} + +// write is [mutate], except that with keepUnsaved the writes take effect even when the file can't +// be read or saved, and are journaled until a save succeeds. +func write(produce func(next *koanf.Koanf) (map[string]pendingWrite, error), keepUnsaved bool) error { k.mu.Lock() defer k.mu.Unlock() unlock := lockSettingsFile(k.filePath) defer unlock() next, baseErr := writeBase() if baseErr != nil { - // An unreadable file must not void the write locally — a logout's - // tombstones in particular: decide and journal against the live - // snapshot, and leave the file alone. + if !keepUnsaved { + return baseErr + } + // Decide against the live snapshot instead of the unreadable file. var err error if next, err = clone(snapshot()); err != nil { return errors.Join(baseErr, err) @@ -445,6 +511,9 @@ func mutate(produce func(next *koanf.Koanf) (map[string]pendingWrite, error)) er saveErr = save(next) } if saveErr != nil { + if !keepUnsaved { + return saveErr + } if k.pending == nil { k.pending = make(map[string]pendingWrite) } @@ -538,6 +607,7 @@ func lockSettingsFile(path string) func() { } } +// Set stores value under key. On error the previous value remains. func Set(key _key, value any) error { return mutate(writesProducer(map[string]pendingWrite{key.String(): {value: value}})) } @@ -564,6 +634,7 @@ func ReplaceString(key _key, from, to string) error { }) } +// Clear removes keys. On error the previous values remain. func Clear(keys ..._key) error { updates := make(map[string]pendingWrite, len(keys)) for _, key := range keys { @@ -574,10 +645,11 @@ func Clear(keys ..._key) error { type Settings map[_key]any +// Diff returns the entries of s that are absent from s2 or hold a different value there. func (s Settings) Diff(s2 Settings) Settings { diff := make(Settings) for k, v1 := range s { - if v2, ok := s2[k]; !ok || v1 != v2 { + if v2, ok := s2[k]; !ok || !reflect.DeepEqual(v1, v2) { diff[k] = v1 } } @@ -592,6 +664,8 @@ func GetAll() Settings { return s } +// GetAllFor returns the values of keys that are set, omitting unset keys; with no keys it returns +// all settings. func GetAllFor(keys ..._key) Settings { if len(keys) == 0 { return GetAll() @@ -599,39 +673,59 @@ func GetAllFor(keys ..._key) Settings { cur := snapshot() s := make(Settings) for _, key := range keys { - s[key] = cur.Get(key.String()) + if cur.Exists(key.String()) { + s[key] = cur.Get(key.String()) + } } return s } -// Patch takes a map of settings to update and applies them all at once. +// Patch applies updates all at once. On error the previous values remain. func Patch(updates Settings) error { + return mutate(writesProducer(valueWrites(updates))) +} + +// Apply is like [Patch], except the updates take effect even when the settings file can't be read +// or saved, and that error is returned. Unsaved updates are reapplied over the file, including by +// [Reload], until a later write saves them. +func Apply(updates Settings) error { + return write(writesProducer(valueWrites(updates)), true) +} + +func valueWrites(updates Settings) map[string]pendingWrite { writes := make(map[string]pendingWrite, len(updates)) for key, value := range updates { writes[key.String()] = pendingWrite{value: value} } - return mutate(writesProducer(writes)) + return writes } +// save writes next to the settings file unless the file already holds it. It does nothing until +// [InitSettings] sets the file path, and requires k.mu to be held. func save(next *koanf.Koanf) error { + if k.filePath == "" { + return nil + } out, err := next.Marshal(json.Parser()) if err != nil { return fmt.Errorf("could not marshal koanf file: %w", err) } - - err = atomicfile.WriteFile(k.filePath, out, fileperm.File) - if err != nil { + if current, err := atomicfile.ReadFile(k.filePath); err == nil && bytes.Equal(out, current) { + return nil + } + if err := atomicfile.WriteFile(k.filePath, out, fileperm.File); err != nil { return fmt.Errorf("could not write koanf file: %w", err) } return nil } -// Reset clears the current settings in memory primarily for testing purposes. +// Reset restores the default, uninitialized settings, primarily for testing purposes. func Reset() { k.mu.Lock() defer k.mu.Unlock() - k.k.Store(koanf.New(".")) + k.k.Store(defaultSnapshot()) k.pending = nil + k.filePath = "" k.initialized = false } diff --git a/common/settings/settings_test.go b/common/settings/settings_test.go index a5208460..66975eca 100644 --- a/common/settings/settings_test.go +++ b/common/settings/settings_test.go @@ -45,20 +45,15 @@ func TestWindowsCrossDirCandidates(t *testing.T) { assert.Contains(t, cs[0].label, "v9.0.x Windows", "label must identify the cross-dir source") }) - t.Run("readWindowsCrossDirCandidates picks up both local.json and settings.json", func(t *testing.T) { + t.Run("readWindowsCrossDirCandidates ignores settings.json", func(t *testing.T) { dir := t.TempDir() local := []byte(`{"user_level":"pro","user_id":1}`) - canon := []byte(`{"user_level":"expired","user_id":2}`) require.NoError(t, os.WriteFile(filepath.Join(dir, legacySettingsFileName), local, 0o644)) - require.NoError(t, os.WriteFile(filepath.Join(dir, settingsFileName), canon, 0o644)) + require.NoError(t, os.WriteFile(filepath.Join(dir, settingsFileName), []byte(`{"user_id":2}`), 0o644)) cs := readWindowsCrossDirCandidates(dir) - require.Len(t, cs, 2, "both filenames should be returned when both exist") - // local.json must come first — it's the actual v9.0.x name; the - // settings.json fallback is for users who got renamed by a - // partial earlier upgrade. - assert.Equal(t, local, cs[0].contents, "local.json must be first") - assert.Equal(t, canon, cs[1].contents, "settings.json fallback must be second") + require.Len(t, cs, 1) + assert.Equal(t, local, cs[0].contents) }) t.Run("readWindowsCrossDirCandidates returns empty when dir is empty", func(t *testing.T) { @@ -91,14 +86,57 @@ func TestWindowsCrossDirCandidates(t *testing.T) { }) } +// isolateDiscovery stops InitSettings from reading the host's own legacy settings files. +func isolateDiscovery(t *testing.T) { + t.Helper() + prevYAML, prevWin := legacyYAMLPathFn, windowsCrossDirCandidatesFn + legacyYAMLPathFn = func(string) (string, string) { return "", "" } + windowsCrossDirCandidatesFn = func(string) []candidateSource { return nil } + t.Cleanup(func() { + legacyYAMLPathFn, windowsCrossDirCandidatesFn = prevYAML, prevWin + }) +} + +func initAt(t *testing.T, dir string) { + t.Helper() + Reset() + t.Cleanup(Reset) + require.NoError(t, InitSettings(dir)) +} + +// readOnlyDir makes dir readable but not writable until the returned func restores it, skipping the +// test where that cannot be enforced. +func readOnlyDir(t *testing.T, dir string) (restore func()) { + t.Helper() + if runtime.GOOS == "windows" { + t.Skip("directory permissions differ on windows") + } + if os.Geteuid() == 0 { + t.Skip("root writes regardless of mode") + } + require.NoError(t, os.Chmod(dir, 0o555)) + t.Cleanup(func() { _ = os.Chmod(dir, 0o755) }) + return func() { require.NoError(t, os.Chmod(dir, 0o755)) } +} + +// moveAside makes dir unavailable until the returned func moves it back. +func moveAside(t *testing.T, dir string) (restore func()) { + t.Helper() + aside := dir + ".aside" + require.NoError(t, os.Rename(dir, aside)) + return func() { require.NoError(t, os.Rename(aside, dir)) } +} + func TestInitSettings(t *testing.T) { + isolateDiscovery(t) + t.Run("existing valid config file", func(t *testing.T) { tempDir := t.TempDir() path := filepath.Join(tempDir, settingsFileName) content := []byte(`{"locale": "en-US", "country_code": "US"}`) require.NoError(t, os.WriteFile(path, content, 0644), "failed to create test config file") - require.NoError(t, InitSettings(tempDir), "failed to initialize settings") + initAt(t, tempDir) assert.Equal(t, "en-US", Get(LocaleKey)) assert.Equal(t, "US", Get(CountryCodeKey)) }) @@ -111,463 +149,656 @@ func TestInitSettings(t *testing.T) { }) t.Run("invalid config file recovers", func(t *testing.T) { - Reset() - t.Cleanup(Reset) tempDir := t.TempDir() path := filepath.Join(tempDir, settingsFileName) require.NoError(t, os.WriteFile(path, []byte(`{invalid json}`), 0644)) - require.NoError(t, InitSettings(tempDir), "an invalid settings file must not block initialization") + initAt(t, tempDir) invalidPath := filepath.Join(tempDir, settingsInvalidFileName) assert.FileExists(t, invalidPath, "the invalid file must be preserved for diagnostics") assert.NoError(t, loadInto(koanf.New("."), path), "settings.json must be rewritten with a parseable default") }) -} -func TestMigrateLegacySettingsIfNeeded(t *testing.T) { - // Redirect the OS-specific pre-9.x YAML lookup to nowhere by - // default so individual tests don't pick up the host machine's - // actual ~/Library/Application Support/Lantern/settings.yaml or - // equivalent. Sub-tests that exercise the YAML path opt in by - // pointing the function at their tempDir. - prevYAMLPath := legacyYAMLPathFn - legacyYAMLPathFn = func(string) (string, string) { return "", "" } - t.Cleanup(func() { legacyYAMLPathFn = prevYAMLPath }) + t.Run("changes before InitSettings stay in memory", func(t *testing.T) { + Reset() + t.Cleanup(Reset) - // Same treatment for the Windows v9.0.x cross-dir lookup: default - // to nothing so tests don't accidentally pick up a real - // %PUBLIC%\Lantern\data on a developer's Windows machine. Tests that - // exercise this path opt in. - prevWinCrossDir := windowsCrossDirCandidatesFn - windowsCrossDirCandidatesFn = func(string) []candidateSource { return nil } - t.Cleanup(func() { windowsCrossDirCandidatesFn = prevWinCrossDir }) + require.NoError(t, Set(LocaleKey, "en-US")) + + assert.Equal(t, "en-US", GetString(LocaleKey)) + }) + + t.Run("Reset restores defaults", func(t *testing.T) { + initAt(t, t.TempDir()) + require.NoError(t, Set(UserLevelKey, "pro")) + Reset() + assert.Equal(t, "free", GetString(UserLevelKey)) + assert.False(t, Exists(LocaleKey)) + }) +} - writeNested := func(t *testing.T, dir string, contents []byte) { +func TestLegacyMigration(t *testing.T) { + isolateDiscovery(t) + + writeNested := func(t *testing.T, dir string, contents string) { t.Helper() nd := filepath.Join(dir, "data") require.NoError(t, os.MkdirAll(nd, 0o755)) - require.NoError(t, os.WriteFile(filepath.Join(nd, settingsFileName), contents, 0o644)) + require.NoError(t, os.WriteFile(filepath.Join(nd, settingsFileName), []byte(contents), 0o644)) + } + writeLegacy := func(t *testing.T, dir string, contents string) { + t.Helper() + require.NoError(t, os.WriteFile(filepath.Join(dir, legacySettingsFileName), []byte(contents), 0o644)) } - writeLegacy := func(t *testing.T, dir string, contents []byte) { + writeCanonical := func(t *testing.T, dir string, contents string) { t.Helper() - require.NoError(t, os.WriteFile(filepath.Join(dir, legacySettingsFileName), contents, 0o644)) + require.NoError(t, os.WriteFile(filepath.Join(dir, settingsFileName), []byte(contents), 0o644)) + } + useCrossDir := func(t *testing.T, v90xDir string) { + t.Helper() + prev := windowsCrossDirCandidatesFn + windowsCrossDirCandidatesFn = func(string) []candidateSource { + return readWindowsCrossDirCandidates(v90xDir) + } + t.Cleanup(func() { windowsCrossDirCandidatesFn = prev }) + } + useYAML := func(t *testing.T, dir, contents string) { + t.Helper() + yamlPath := filepath.Join(dir, "fake-pre9x-settings.yaml") + require.NoError(t, os.WriteFile(yamlPath, []byte(contents), 0o644)) + prev := legacyYAMLPathFn + legacyYAMLPathFn = func(string) (string, string) { return yamlPath, "desktop" } + t.Cleanup(func() { legacyYAMLPathFn = prev }) + } + // onDisk reads back the saved settings, so assertions cover what the next start sees. + onDisk := func(t *testing.T, dir string) map[string]any { + t.Helper() + b, err := os.ReadFile(filepath.Join(dir, settingsFileName)) + require.NoError(t, err) + var got map[string]any + require.NoError(t, json.Unmarshal(b, &got)) + return got } - t.Run("v9.0.x local.json recovered when canonical is missing (Derek's failing case)", func(t *testing.T) { - // User upgraded from v9.0.x straight to the fixed build. v9.0.x wrote - // to /local.json; canonical settings.json doesn't exist; - // no v9.1.x nested file. The fix must read local.json so Pro survives. + t.Run("v9.0.x local.json recovered when canonical is missing", func(t *testing.T) { tempDir := t.TempDir() - want := []byte(`{"user_id": 3580849, "user_level": "pro", "token": "abc"}`) - writeLegacy(t, tempDir, want) + writeLegacy(t, tempDir, `{"user_id": 3580849, "user_level": "pro", "token": "abc"}`) - canonical := filepath.Join(tempDir, settingsFileName) - migrateLegacySettingsIfNeeded(tempDir, canonical) + initAt(t, tempDir) - got, err := os.ReadFile(canonical) - require.NoError(t, err) - assert.Equal(t, want, got, "v9.0.x local.json should be migrated to canonical") + assert.Equal(t, "3580849", GetString(UserIDKey)) + assert.Equal(t, "pro", GetString(UserLevelKey)) + assert.Equal(t, "abc", GetString(TokenKey)) + assert.Equal(t, "abc", onDisk(t, tempDir)["token"], "adopted identity must be saved") + assert.FileExists(t, filepath.Join(tempDir, migrationMarkerFileName)) + }) + + t.Run("a user id above 2^53 is adopted exactly", func(t *testing.T) { + tempDir := t.TempDir() + writeLegacy(t, tempDir, `{"user_id": 9007199254740993, "token": "abc"}`) + + initAt(t, tempDir) + + assert.Equal(t, int64(9007199254740993), GetInt64(UserIDKey)) }) t.Run("v9.1.x nested file recovered when canonical is missing", func(t *testing.T) { tempDir := t.TempDir() - want := []byte(`{"user_id": 135809562, "user_level": "pro", "device_id": "abc"}`) - writeNested(t, tempDir, want) + writeNested(t, tempDir, `{"user_id": 135809562, "user_level": "pro", "device_id": "abc"}`) - canonical := filepath.Join(tempDir, settingsFileName) - migrateLegacySettingsIfNeeded(tempDir, canonical) + initAt(t, tempDir) - got, err := os.ReadFile(canonical) - require.NoError(t, err) - assert.Equal(t, want, got, "v9.1.x nested file should be migrated to canonical") + assert.Equal(t, "135809562", GetString(UserIDKey)) + assert.Equal(t, "abc", GetString(DeviceIDKey)) }) t.Run("v9.0.x local.json wins over v9.1.x expired nested", func(t *testing.T) { - // Upgrade chain v9.0.x → v9.1.x → fix: legacy has pro, nested has - // expired (because v9.1.x lost the user_id). Migration must pick - // legacy so Pro survives. tempDir := t.TempDir() - canonical := filepath.Join(tempDir, settingsFileName) - legacyPro := []byte(`{"user_id": 1, "user_level": "pro"}`) - writeLegacy(t, tempDir, legacyPro) - writeNested(t, tempDir, []byte(`{"user_id": 999, "user_level": "expired"}`)) + writeLegacy(t, tempDir, `{"user_id": 1, "user_level": "pro"}`) + writeNested(t, tempDir, `{"user_id": 999, "user_level": "expired"}`) - migrateLegacySettingsIfNeeded(tempDir, canonical) + initAt(t, tempDir) - got, err := os.ReadFile(canonical) - require.NoError(t, err) - assert.Equal(t, legacyPro, got, "legacy local.json with pro should beat nested expired") + assert.Equal(t, "1", GetString(UserIDKey)) + assert.True(t, IsPro()) }) t.Run("canonical-pro wins over nested-expired", func(t *testing.T) { tempDir := t.TempDir() - canonical := filepath.Join(tempDir, settingsFileName) - canonicalPro := []byte(`{"user_id": 1, "user_level": "pro"}`) - require.NoError(t, os.WriteFile(canonical, canonicalPro, 0o644)) - writeNested(t, tempDir, []byte(`{"user_id": 999, "user_level": "expired"}`)) + writeCanonical(t, tempDir, `{"user_id": 1, "user_level": "pro"}`) + writeNested(t, tempDir, `{"user_id": 999, "user_level": "expired"}`) - migrateLegacySettingsIfNeeded(tempDir, canonical) + initAt(t, tempDir) - got, err := os.ReadFile(canonical) - require.NoError(t, err) - assert.Equal(t, canonicalPro, got, "canonical-pro should survive") + assert.Equal(t, "1", GetString(UserIDKey)) + assert.True(t, IsPro()) }) t.Run("canonical expired account survives nested Pro recovery", func(t *testing.T) { tempDir := t.TempDir() - canonical := filepath.Join(tempDir, settingsFileName) - canonicalContents := []byte(`{"user_id": 1, "user_level": "expired"}`) - require.NoError(t, os.WriteFile(canonical, canonicalContents, 0o644)) - writeLegacy(t, tempDir, []byte(`{"user_id": 1, "user_level": "expired"}`)) - nestedPro := []byte(`{"user_id": 2, "user_level": "pro"}`) - writeNested(t, tempDir, nestedPro) + writeCanonical(t, tempDir, `{"user_id": 1, "user_level": "expired", "token": "t1", "jwt_token": "j1"}`) + writeLegacy(t, tempDir, `{"user_id": 1, "user_level": "expired"}`) + writeNested(t, tempDir, `{"user_id": 2, "user_level": "pro"}`) - migrateLegacySettingsIfNeeded(tempDir, canonical) + initAt(t, tempDir) - got, err := os.ReadFile(canonical) - require.NoError(t, err) - assert.Equal(t, canonicalContents, got) + assert.Equal(t, "1", GetString(UserIDKey)) + assert.False(t, IsPro()) + assert.Equal(t, "t1", GetString(TokenKey)) + assert.Equal(t, "j1", GetString(JwtTokenKey)) + assert.FileExists(t, filepath.Join(tempDir, migrationMarkerFileName)) }) t.Run("all-pro: canonical wins (most recent deliberate state)", func(t *testing.T) { tempDir := t.TempDir() - canonical := filepath.Join(tempDir, settingsFileName) - canonicalContents := []byte(`{"user_id": 1, "user_level": "pro"}`) - require.NoError(t, os.WriteFile(canonical, canonicalContents, 0o644)) - writeLegacy(t, tempDir, []byte(`{"user_id": 2, "user_level": "pro"}`)) - writeNested(t, tempDir, []byte(`{"user_id": 3, "user_level": "pro"}`)) + writeCanonical(t, tempDir, `{"user_id": 1, "user_level": "pro"}`) + writeLegacy(t, tempDir, `{"user_id": 2, "user_level": "pro"}`) + writeNested(t, tempDir, `{"user_id": 3, "user_level": "pro"}`) - migrateLegacySettingsIfNeeded(tempDir, canonical) + initAt(t, tempDir) - got, err := os.ReadFile(canonical) - require.NoError(t, err) - assert.Equal(t, canonicalContents, got, "canonical preferred when all have pro") + assert.Equal(t, "1", GetString(UserIDKey)) }) t.Run("canonical free account survives stale Pro recovery files", func(t *testing.T) { - dir := t.TempDir() - canonical := filepath.Join(dir, settingsFileName) - adopted := []byte(`{"user_id":42,"user_level":"free","token":"adopted-token"}`) - require.NoError(t, os.WriteFile(canonical, adopted, 0600)) - stalePro := []byte(`{"user_id":1,"user_level":"pro","token":"stale-token"}`) - writeLegacy(t, dir, stalePro) - writeNested(t, dir, stalePro) - - migrateLegacySettingsIfNeeded(dir, canonical) - - got, err := os.ReadFile(canonical) - require.NoError(t, err) - assert.Equal(t, adopted, got) - }) - - t.Run("logged out canonical settings survive stale Pro recovery files", func(t *testing.T) { - dir := t.TempDir() - canonical := filepath.Join(dir, settingsFileName) - require.NoError(t, os.WriteFile(canonical, []byte(`{}`), 0600)) - writeLegacy(t, dir, []byte(`{"user_id":1,"user_level":"pro","token":"stale-token"}`)) + tempDir := t.TempDir() + writeCanonical(t, tempDir, `{"user_id":42,"user_level":"free","token":"adopted-token"}`) + stalePro := `{"user_id":1,"user_level":"pro","token":"stale-token"}` + writeLegacy(t, tempDir, stalePro) + writeNested(t, tempDir, stalePro) - migrateLegacySettingsIfNeeded(dir, canonical) + initAt(t, tempDir) - got, err := os.ReadFile(canonical) - require.NoError(t, err) - assert.JSONEq(t, `{}`, string(got)) + assert.Equal(t, "42", GetString(UserIDKey)) + assert.Equal(t, "adopted-token", GetString(TokenKey)) + assert.False(t, IsPro()) }) t.Run("invalid canonical settings recover a valid free account", func(t *testing.T) { for _, invalid := range []string{`{invalid}`, `null`, `[]`, `{} {}`} { t.Run(invalid, func(t *testing.T) { - dir := t.TempDir() - canonical := filepath.Join(dir, settingsFileName) - require.NoError(t, os.WriteFile(canonical, []byte(invalid), 0600)) - legacy := []byte(`{"user_id":42,"user_level":"free","token":"current-token"}`) - writeLegacy(t, dir, legacy) + tempDir := t.TempDir() + writeCanonical(t, tempDir, invalid) + writeLegacy(t, tempDir, `{"user_id":42,"user_level":"free","token":"current-token"}`) - migrateLegacySettingsIfNeeded(dir, canonical) + initAt(t, tempDir) - got, err := os.ReadFile(canonical) - require.NoError(t, err) - assert.Equal(t, legacy, got) + assert.Equal(t, "42", GetString(UserIDKey)) + assert.Equal(t, "current-token", GetString(TokenKey)) + assert.Equal(t, "current-token", onDisk(t, tempDir)["token"]) }) } }) - t.Run("malformed legacy settings do not hide a valid nested account", func(t *testing.T) { - dir := t.TempDir() - canonical := filepath.Join(dir, settingsFileName) - writeLegacy(t, dir, []byte(`{invalid}`)) - nested := []byte(`{"user_id":42,"user_level":"free"}`) - writeNested(t, dir, nested) + t.Run("none have pro: legacy wins over nested when canonical missing", func(t *testing.T) { + // User identifiers must survive even when Pro state is non-pro, + // to keep the device registration intact server-side. + tempDir := t.TempDir() + writeLegacy(t, tempDir, `{"user_id": 1, "user_level": "free", "token": "abc"}`) + writeNested(t, tempDir, `{"user_id": 2, "user_level": "free", "token": "xyz"}`) - migrateLegacySettingsIfNeeded(dir, canonical) + initAt(t, tempDir) - got, err := os.ReadFile(canonical) - require.NoError(t, err) - assert.Equal(t, nested, got) + assert.Equal(t, "1", GetString(UserIDKey)) + assert.Equal(t, "abc", GetString(TokenKey)) }) - t.Run("none have pro: legacy wins over nested when canonical missing", func(t *testing.T) { - // User identifiers must survive even when Pro state is non-pro, - // to keep the device registration intact server-side. + t.Run("adopts the recovered account's OAuth state", func(t *testing.T) { tempDir := t.TempDir() - canonical := filepath.Join(tempDir, settingsFileName) - legacyContents := []byte(`{"user_id": 1, "user_level": "free", "token": "abc"}`) - writeLegacy(t, tempDir, legacyContents) - writeNested(t, tempDir, []byte(`{"user_id": 2, "user_level": "free", "token": "xyz"}`)) + writeNested(t, tempDir, `{"user_id": 3, "user_level": "pro", "jwt_token": "jwt", "oauth_login": true, "oauth_provider": "apple"}`) - migrateLegacySettingsIfNeeded(tempDir, canonical) + initAt(t, tempDir) - got, err := os.ReadFile(canonical) - require.NoError(t, err) - assert.Equal(t, legacyContents, got, "legacy preferred over nested when canonical missing and neither has pro") + assert.Equal(t, "3", GetString(UserIDKey)) + assert.Equal(t, "jwt", GetString(JwtTokenKey)) + assert.True(t, GetBool(OAuthLoginKey)) + assert.Equal(t, "apple", GetString(OAuthProviderKey)) }) - t.Run("nothing on disk is a no-op", func(t *testing.T) { + t.Run("logged out canonical settings survive stale Pro recovery files", func(t *testing.T) { tempDir := t.TempDir() - canonical := filepath.Join(tempDir, settingsFileName) + writeCanonical(t, tempDir, `{"locale": "en-US", "device_id": "dev"}`) + writeLegacy(t, tempDir, `{"user_id": 7, "user_level": "pro", "token": "stale-token"}`) + + initAt(t, tempDir) + + assert.False(t, Exists(UserIDKey)) + assert.False(t, Exists(TokenKey)) + assert.Equal(t, "en-US", GetString(LocaleKey)) + assert.Equal(t, "dev", GetString(DeviceIDKey)) + }) + + t.Run("an unreadable legacy file counts as missing", func(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("permission semantics differ on windows") + } + if os.Geteuid() == 0 { + t.Skip("root reads files regardless of mode") + } + tempDir := t.TempDir() + writeLegacy(t, tempDir, `{"user_id": 7, "user_level": "pro", "token": "tok"}`) + legacy := filepath.Join(tempDir, legacySettingsFileName) + require.NoError(t, os.Chmod(legacy, 0o000)) + t.Cleanup(func() { _ = os.Chmod(legacy, 0o644) }) + writeNested(t, tempDir, `{"user_id": 5, "user_level": "free"}`) + + initAt(t, tempDir) + + assert.Equal(t, "5", GetString(UserIDKey)) + assert.FileExists(t, filepath.Join(tempDir, migrationMarkerFileName)) + }) + + t.Run("adopts identity keys only", func(t *testing.T) { + tempDir := t.TempDir() + writeLegacy(t, tempDir, `{"user_id": 1, "user_level": "pro", "peer_share_enabled": true, "locale": "ru-RU"}`) + + initAt(t, tempDir) + + assert.Equal(t, "1", GetString(UserIDKey)) + assert.False(t, Exists(PeerShareEnabledKey)) + assert.False(t, Exists(LocaleKey)) + }) + + t.Run("fresh install writes defaults and the marker", func(t *testing.T) { + tempDir := t.TempDir() + + initAt(t, tempDir) + + assert.Equal(t, "free", GetString(UserLevelKey)) + assert.FileExists(t, filepath.Join(tempDir, settingsFileName)) + assert.FileExists(t, filepath.Join(tempDir, migrationMarkerFileName)) + }) + + t.Run("a completed migration never resurrects a cleared account", func(t *testing.T) { + tempDir := t.TempDir() + writeLegacy(t, tempDir, `{"user_id": 111, "user_level": "pro", "token": "OLD-PRO-TOKEN", "email": "old@example.com"}`) + initAt(t, tempDir) + require.Equal(t, "111", GetString(UserIDKey)) + + require.NoError(t, Clear(UserIDKey, TokenKey, UserLevelKey, EmailKey)) + require.NoError(t, Patch(Settings{UserIDKey: 222, UserLevelKey: "free"})) + initAt(t, tempDir) + + assert.Equal(t, "222", GetString(UserIDKey)) + assert.Equal(t, "free", GetString(UserLevelKey)) + assert.False(t, Exists(TokenKey)) + + require.NoError(t, Set(UserLevelKey, "expired")) + initAt(t, tempDir) + assert.Equal(t, "expired", GetString(UserLevelKey)) + }) + + t.Run("a missing canonical file after migration starts fresh", func(t *testing.T) { + tempDir := t.TempDir() + writeLegacy(t, tempDir, `{"user_id": 111, "user_level": "pro"}`) + initAt(t, tempDir) + require.NoError(t, os.Remove(filepath.Join(tempDir, settingsFileName))) + + initAt(t, tempDir) + + assert.False(t, Exists(UserIDKey)) + }) + + t.Run("malformed canonical loses to a valid legacy file", func(t *testing.T) { + tempDir := t.TempDir() + writeCanonical(t, tempDir, `{invalid json}`) + writeLegacy(t, tempDir, `{"user_id": 7, "user_level": "free", "token": "tok"}`) + + initAt(t, tempDir) + + assert.Equal(t, "7", GetString(UserIDKey)) + assert.Equal(t, "tok", GetString(TokenKey)) + assert.FileExists(t, filepath.Join(tempDir, settingsInvalidFileName)) + }) + + t.Run("malformed legacy candidate is ignored", func(t *testing.T) { + tempDir := t.TempDir() + writeLegacy(t, tempDir, `{"user_level": "pro"`) + writeNested(t, tempDir, `{"user_id": 5, "user_level": "free"}`) + + initAt(t, tempDir) + + assert.Equal(t, "5", GetString(UserIDKey)) + }) + + t.Run("a failed save is retried on the next start", func(t *testing.T) { + tempDir := t.TempDir() + // null loads as empty settings but isn't a JSON object, so the legacy identity is adopted. + writeCanonical(t, tempDir, `null`) + writeLegacy(t, tempDir, `{"user_id": 2, "user_level": "pro"}`) + restore := readOnlyDir(t, tempDir) + + initAt(t, tempDir) + assert.False(t, Exists(UserIDKey), "an unsaved adoption must not take effect") + assert.NoFileExists(t, filepath.Join(tempDir, migrationMarkerFileName)) + + restore() + initAt(t, tempDir) + assert.Equal(t, "2", GetString(UserIDKey)) + assert.FileExists(t, filepath.Join(tempDir, migrationMarkerFileName)) + }) + + t.Run("a new canonical file carries the adopted identity", func(t *testing.T) { + tempDir := t.TempDir() + writeLegacy(t, tempDir, `{"user_id": 7, "user_level": "free", "token": "tok"}`) + + initAt(t, tempDir) + + assert.Equal(t, float64(7), onDisk(t, tempDir)["user_id"]) + }) - migrateLegacySettingsIfNeeded(tempDir, canonical) + t.Run("a failed first write leaves a non-pro legacy identity recoverable", func(t *testing.T) { + tempDir := t.TempDir() + writeLegacy(t, tempDir, `{"user_id": 7, "user_level": "free", "token": "tok"}`) + restore := readOnlyDir(t, tempDir) - _, err := os.Stat(canonical) - assert.True(t, os.IsNotExist(err), "no migration when no source files exist") + Reset() + t.Cleanup(Reset) + require.Error(t, InitSettings(tempDir)) + assert.NoFileExists(t, filepath.Join(tempDir, settingsFileName)) + + restore() + initAt(t, tempDir) + assert.Equal(t, "7", GetString(UserIDKey)) }) t.Run("pre-9.x desktop YAML recovered when no JSON candidates exist", func(t *testing.T) { - // Redirect the YAML lookup at a tempDir-local file so the test - // is portable across OSes. tempDir := t.TempDir() - yamlPath := filepath.Join(tempDir, "fake-pre9x-settings.yaml") - require.NoError(t, os.WriteFile(yamlPath, []byte(`userID: 3580849 + useYAML(t, tempDir, `userID: 3580849 deviceID: legacy-device-id userPro: true userToken: legacy-token emailAddress: derek@example.com -`), 0o644)) - legacyYAMLPathFn = func(string) (string, string) { return yamlPath, "desktop" } - t.Cleanup(func() { legacyYAMLPathFn = func(string) (string, string) { return "", "" } }) +`) - canonical := filepath.Join(tempDir, settingsFileName) - migrateLegacySettingsIfNeeded(tempDir, canonical) + initAt(t, tempDir) - got, err := os.ReadFile(canonical) - require.NoError(t, err) - gotStr := string(got) - assert.Contains(t, gotStr, `"user_id":3580849`) - assert.Contains(t, gotStr, `"device_id":"legacy-device-id"`) - assert.Contains(t, gotStr, `"user_level":"pro"`) - assert.Contains(t, gotStr, `"token":"legacy-token"`) - assert.Contains(t, gotStr, `"email":"derek@example.com"`) + assert.Equal(t, "3580849", GetString(UserIDKey)) + assert.Equal(t, "legacy-device-id", GetString(DeviceIDKey)) + assert.Equal(t, "pro", GetString(UserLevelKey)) + assert.Equal(t, "legacy-token", GetString(TokenKey)) + assert.Equal(t, "derek@example.com", GetString(EmailKey)) }) t.Run("v9.0.x local.json beats pre-9.x YAML", func(t *testing.T) { - // Both exist with pro state. local.json is the higher-priority - // (more recent) source, so it should win. tempDir := t.TempDir() - yamlPath := filepath.Join(tempDir, "fake-pre9x-settings.yaml") - require.NoError(t, os.WriteFile(yamlPath, []byte(`userID: 1 + useYAML(t, tempDir, `userID: 1 userPro: true userToken: legacy-token -`), 0o644)) - legacyYAMLPathFn = func(string) (string, string) { return yamlPath, "desktop" } - t.Cleanup(func() { legacyYAMLPathFn = func(string) (string, string) { return "", "" } }) +`) + writeLegacy(t, tempDir, `{"user_id": 2, "user_level": "pro", "token": "v9.0-token"}`) - writeLegacy(t, tempDir, []byte(`{"user_id": 2, "user_level": "pro", "token": "v9.0-token"}`)) + initAt(t, tempDir) - canonical := filepath.Join(tempDir, settingsFileName) - migrateLegacySettingsIfNeeded(tempDir, canonical) - - got, err := os.ReadFile(canonical) - require.NoError(t, err) - assert.Contains(t, string(got), `"user_id": 2`, - "v9.0.x local.json should win over pre-9.x YAML when both have pro") - assert.Contains(t, string(got), `"v9.0-token"`) + assert.Equal(t, "2", GetString(UserIDKey)) + assert.Equal(t, "v9.0-token", GetString(TokenKey)) }) t.Run("pre-9.x YAML beats v9.1.x bugged nested file", func(t *testing.T) { - // Pre-9.x has pro; v9.1.x nested has expired (the bugged case). - // Pre-9.x must win. tempDir := t.TempDir() - yamlPath := filepath.Join(tempDir, "fake-pre9x-settings.yaml") - require.NoError(t, os.WriteFile(yamlPath, []byte(`userID: 1 + useYAML(t, tempDir, `userID: 1 userPro: true userToken: legacy-token -`), 0o644)) - legacyYAMLPathFn = func(string) (string, string) { return yamlPath, "desktop" } - t.Cleanup(func() { legacyYAMLPathFn = func(string) (string, string) { return "", "" } }) - - writeNested(t, tempDir, []byte(`{"user_id": 999, "user_level": "expired"}`)) +`) + writeNested(t, tempDir, `{"user_id": 999, "user_level": "expired"}`) - canonical := filepath.Join(tempDir, settingsFileName) - migrateLegacySettingsIfNeeded(tempDir, canonical) + initAt(t, tempDir) - got, err := os.ReadFile(canonical) - require.NoError(t, err) - assert.Contains(t, string(got), `"user_level":"pro"`, - "pre-9.x YAML with pro should win over v9.1.x nested expired") - assert.Contains(t, string(got), `"legacy-token"`) + assert.Equal(t, "pro", GetString(UserLevelKey)) + assert.Equal(t, "legacy-token", GetString(TokenKey)) }) t.Run("iOS userconfig.yaml recovered when canonical is missing", func(t *testing.T) { - // On iOS the legacy YAML is sandbox-relative — it lives next to - // where settings.json now lives, so legacyYAMLCandidate reads - // from fileDir directly and we can exercise it from a test - // without monkeypatching $HOME or $APPDATA. (Desktop legacy - // paths are covered via translateLegacyYAML's unit tests, which - // don't depend on the OS-specific path resolution.) if runtime.GOOS != "ios" { t.Skip("iOS-only path: legacy YAML elsewhere is OS-specific") } + prev := legacyYAMLPathFn + legacyYAMLPathFn = legacyYAMLPath + t.Cleanup(func() { legacyYAMLPathFn = prev }) tempDir := t.TempDir() - yamlPath := filepath.Join(tempDir, "userconfig.yaml") - require.NoError(t, os.WriteFile(yamlPath, []byte(`UserID: 7777 + require.NoError(t, os.WriteFile(filepath.Join(tempDir, "userconfig.yaml"), []byte(`UserID: 7777 DeviceID: ios-device Token: tok `), 0o644)) - canonical := filepath.Join(tempDir, settingsFileName) - migrateLegacySettingsIfNeeded(tempDir, canonical) - got, err := os.ReadFile(canonical) - require.NoError(t, err) - assert.Contains(t, string(got), `"user_id":7777`) - assert.Contains(t, string(got), `"device_id":"ios-device"`) + initAt(t, tempDir) + + assert.Equal(t, "7777", GetString(UserIDKey)) + assert.Equal(t, "ios-device", GetString(DeviceIDKey)) }) t.Run("Windows v9.0.x cross-dir local.json recovered when canonical/same-dir/nested all missing", func(t *testing.T) { - // FD #174606 / engineering#3460: PR #370 moved the Windows daemon - // from \Lantern\data (where v9.0.x Flutter+FFI wrote - // local.json) to \Lantern. The same-dir scan can't - // see across that filesystem boundary, so cross-dir candidates - // are spliced in by windowsCrossDirCandidatesFn. Verify recovery. tempDir := t.TempDir() v90xDir := filepath.Join(tempDir, "windows-v90x") require.NoError(t, os.MkdirAll(v90xDir, 0o755)) - want := []byte(`{"user_id": 195646669, "user_level": "pro", "token": "preserved"}`) - require.NoError(t, os.WriteFile(filepath.Join(v90xDir, legacySettingsFileName), want, 0o644)) - - windowsCrossDirCandidatesFn = func(string) []candidateSource { - return readWindowsCrossDirCandidates(v90xDir) - } - t.Cleanup(func() { - windowsCrossDirCandidatesFn = func(string) []candidateSource { return nil } - }) + require.NoError(t, os.WriteFile(filepath.Join(v90xDir, legacySettingsFileName), + []byte(`{"user_id": 195646669, "user_level": "pro", "token": "preserved"}`), 0o644)) + useCrossDir(t, v90xDir) - canonical := filepath.Join(tempDir, settingsFileName) - migrateLegacySettingsIfNeeded(tempDir, canonical) + initAt(t, tempDir) - got, err := os.ReadFile(canonical) - require.NoError(t, err) - assert.Equal(t, want, got, "v9.0.x cross-dir local.json must be migrated to canonical") + assert.Equal(t, "195646669", GetString(UserIDKey)) + assert.Equal(t, "preserved", GetString(TokenKey)) }) - t.Run("Windows v9.0.x cross-dir falls back to settings.json when local.json missing", func(t *testing.T) { - // Defensive case: a user whose v9.0.x file got renamed by an - // earlier partial upgrade attempt. The cross-dir scan should - // still pick up settings.json under ${PUBLIC}\Lantern\data. + t.Run("Windows cross-dir settings.json is never adopted", func(t *testing.T) { tempDir := t.TempDir() v90xDir := filepath.Join(tempDir, "windows-v90x") require.NoError(t, os.MkdirAll(v90xDir, 0o755)) - want := []byte(`{"user_id": 42, "user_level": "pro"}`) - require.NoError(t, os.WriteFile(filepath.Join(v90xDir, settingsFileName), want, 0o644)) - - windowsCrossDirCandidatesFn = func(string) []candidateSource { - return readWindowsCrossDirCandidates(v90xDir) - } - t.Cleanup(func() { - windowsCrossDirCandidatesFn = func(string) []candidateSource { return nil } - }) + require.NoError(t, os.WriteFile(filepath.Join(v90xDir, settingsFileName), + []byte(`{"user_id": 42, "user_level": "pro", "peer_share_enabled": true}`), 0o644)) + useCrossDir(t, v90xDir) - canonical := filepath.Join(tempDir, settingsFileName) - migrateLegacySettingsIfNeeded(tempDir, canonical) + initAt(t, tempDir) - got, err := os.ReadFile(canonical) - require.NoError(t, err) - assert.Equal(t, want, got, "v9.0.x cross-dir settings.json must be migrated when local.json absent") + assert.False(t, Exists(UserIDKey)) + assert.False(t, Exists(PeerShareEnabledKey)) }) t.Run("Windows v9.0.x cross-dir loses to same-dir v9.0.x local.json", func(t *testing.T) { - // If both exist, same-dir wins — it's strictly more recent state. - // (Cross-dir is only a thing because Windows users skipped through - // the v9.0.x → v9.1.x transition; same-dir local.json means the - // daemon already migrated once and the cross-dir copy is stale.) tempDir := t.TempDir() v90xDir := filepath.Join(tempDir, "windows-v90x") require.NoError(t, os.MkdirAll(v90xDir, 0o755)) require.NoError(t, os.WriteFile(filepath.Join(v90xDir, legacySettingsFileName), []byte(`{"user_id": 999, "user_level": "pro"}`), 0o644)) - sameDirPro := []byte(`{"user_id": 1, "user_level": "pro", "token": "same-dir"}`) - require.NoError(t, os.WriteFile(filepath.Join(tempDir, legacySettingsFileName), sameDirPro, 0o644)) + writeLegacy(t, tempDir, `{"user_id": 1, "user_level": "pro", "token": "same-dir"}`) + useCrossDir(t, v90xDir) - windowsCrossDirCandidatesFn = func(string) []candidateSource { - return readWindowsCrossDirCandidates(v90xDir) - } - t.Cleanup(func() { - windowsCrossDirCandidatesFn = func(string) []candidateSource { return nil } - }) + initAt(t, tempDir) - canonical := filepath.Join(tempDir, settingsFileName) - migrateLegacySettingsIfNeeded(tempDir, canonical) - - got, err := os.ReadFile(canonical) - require.NoError(t, err) - assert.Equal(t, sameDirPro, got, "same-dir v9.0.x must outrank cross-dir v9.0.x") + assert.Equal(t, "1", GetString(UserIDKey)) + assert.Equal(t, "same-dir", GetString(TokenKey)) }) t.Run("Windows v9.0.x cross-dir-pro beats v9.1.x nested-expired", func(t *testing.T) { - // The headline case: user upgrades from v9.0.x Windows (Pro state - // at ${PUBLIC}\Lantern\data\local.json) through a buggy v9.1.x - // build (which wrote an expired record at \Lantern\data\settings.json) - // to the fix. The cross-dir Pro must win over the nested expired. tempDir := t.TempDir() v90xDir := filepath.Join(tempDir, "windows-v90x") require.NoError(t, os.MkdirAll(v90xDir, 0o755)) - crossDirPro := []byte(`{"user_id": 195646669, "user_level": "pro"}`) - require.NoError(t, os.WriteFile(filepath.Join(v90xDir, legacySettingsFileName), crossDirPro, 0o644)) - writeNested(t, tempDir, []byte(`{"user_id": 99, "user_level": "expired"}`)) - - windowsCrossDirCandidatesFn = func(string) []candidateSource { - return readWindowsCrossDirCandidates(v90xDir) - } - t.Cleanup(func() { - windowsCrossDirCandidatesFn = func(string) []candidateSource { return nil } - }) + require.NoError(t, os.WriteFile(filepath.Join(v90xDir, legacySettingsFileName), + []byte(`{"user_id": 195646669, "user_level": "pro"}`), 0o644)) + writeNested(t, tempDir, `{"user_id": 99, "user_level": "expired"}`) + useCrossDir(t, v90xDir) - canonical := filepath.Join(tempDir, settingsFileName) - migrateLegacySettingsIfNeeded(tempDir, canonical) + initAt(t, tempDir) - got, err := os.ReadFile(canonical) - require.NoError(t, err) - assert.Equal(t, crossDirPro, got, "cross-dir pro must beat nested expired (engineering#3460)") + assert.Equal(t, "195646669", GetString(UserIDKey)) + assert.True(t, IsPro()) }) t.Run("unreadable canonical (non-ENOENT) skips migration", func(t *testing.T) { - // Permission error on the canonical path: don't fall through and - // overwrite a file we couldn't read. unix only — windows handles - // permissions differently and chmod wouldn't reproduce this. if runtime.GOOS == "windows" { t.Skip("permission semantics differ on windows") } + if os.Geteuid() == 0 { + t.Skip("root reads files regardless of mode") + } tempDir := t.TempDir() canonical := filepath.Join(tempDir, settingsFileName) - require.NoError(t, os.WriteFile(canonical, []byte(`{"user_level": "expired"}`), 0o644)) - // Make the file unreadable. + writeCanonical(t, tempDir, `{"user_level": "expired"}`) require.NoError(t, os.Chmod(canonical, 0o000)) t.Cleanup(func() { _ = os.Chmod(canonical, 0o644) }) - // Stage a legacy-pro candidate that would otherwise win. - writeLegacy(t, tempDir, []byte(`{"user_id": 1, "user_level": "pro"}`)) + writeLegacy(t, tempDir, `{"user_id": 1, "user_level": "pro"}`) - migrateLegacySettingsIfNeeded(tempDir, canonical) + Reset() + t.Cleanup(Reset) + assert.Error(t, InitSettings(tempDir)) - // Restore readability and confirm the canonical contents are - // unchanged (still the expired body, not the legacy-pro body). require.NoError(t, os.Chmod(canonical, 0o644)) got, err := os.ReadFile(canonical) require.NoError(t, err) assert.Equal(t, `{"user_level": "expired"}`, string(got), "canonical with non-ENOENT read error should be left alone") + assert.NoFileExists(t, filepath.Join(tempDir, migrationMarkerFileName)) }) } +func TestDiff(t *testing.T) { + curr := Settings{ + DevicesKey: []any{map[string]any{"ID": "a"}}, + UserDataKey: nil, + LocaleKey: "en-US", + FeatureOverridesKey: map[string]any{"x": true}, + } + updates := Settings{ + DevicesKey: []any{map[string]any{"ID": "a"}}, + UserDataKey: nil, + LocaleKey: "fa-IR", + FeatureOverridesKey: map[string]any{"x": false}, + EmailKey: nil, + } + + diff := updates.Diff(curr) + + assert.Equal(t, Settings{ + LocaleKey: "fa-IR", + FeatureOverridesKey: map[string]any{"x": false}, + EmailKey: nil, + }, diff, "equal slices and nulls drop out; a null for an absent key stays") +} + +func TestGetAllForOmitsAbsentKeys(t *testing.T) { + isolateDiscovery(t) + initAt(t, t.TempDir()) + require.NoError(t, Set(UserDataKey, nil)) + + got := GetAllFor(UserDataKey, EmailKey) + + assert.Equal(t, Settings{UserDataKey: nil}, got) +} + +func TestSaveFailure(t *testing.T) { + isolateDiscovery(t) + + t.Run("Set and Patch take effect only once saved", func(t *testing.T) { + dir := t.TempDir() + initAt(t, dir) + require.NoError(t, Set(LocaleKey, "en-US")) + before, err := os.ReadFile(filepath.Join(dir, settingsFileName)) + require.NoError(t, err) + restore := moveAside(t, dir) + + assert.Error(t, Set(LocaleKey, "ru-RU")) + assert.Error(t, Patch(Settings{LocaleKey: "ru-RU", CountryCodeKey: "RU"})) + assert.Equal(t, "en-US", GetString(LocaleKey)) + assert.False(t, Exists(CountryCodeKey)) + restore() + after, err := os.ReadFile(filepath.Join(dir, settingsFileName)) + require.NoError(t, err) + assert.Equal(t, before, after, "the file keeps its previous contents") + assert.Equal(t, Settings{LocaleKey: "ru-RU"}, Settings{LocaleKey: "ru-RU"}.Diff(GetAllFor(LocaleKey)), + "a retry still sees the change") + + require.NoError(t, Patch(Settings{LocaleKey: "ru-RU", CountryCodeKey: "RU"})) + assert.Equal(t, "ru-RU", GetString(LocaleKey)) + assert.Equal(t, "RU", GetString(CountryCodeKey)) + }) + + t.Run("Apply takes effect even when the save fails", func(t *testing.T) { + dir := t.TempDir() + initAt(t, dir) + require.NoError(t, Patch(Settings{TelemetryKey: true})) + defer moveAside(t, dir)() + + assert.Error(t, Apply(Settings{TelemetryKey: false, LocaleKey: "en-US"})) + + assert.False(t, GetBool(TelemetryKey)) + assert.Equal(t, "en-US", GetString(LocaleKey)) + }) + + t.Run("Clear takes effect only once saved", func(t *testing.T) { + dir := t.TempDir() + initAt(t, dir) + require.NoError(t, Patch(Settings{TokenKey: "secret", UserIDKey: 1})) + restore := moveAside(t, dir) + + assert.Error(t, Clear(TokenKey, UserIDKey)) + assert.Equal(t, "secret", GetString(TokenKey)) + assert.Equal(t, "1", GetString(UserIDKey)) + + restore() + require.NoError(t, Clear(TokenKey, UserIDKey)) + assert.False(t, Exists(TokenKey)) + assert.False(t, Exists(UserIDKey)) + }) +} + +func TestPatchSkipsUnchangedWrites(t *testing.T) { + isolateDiscovery(t) + dir := t.TempDir() + initAt(t, dir) + path := filepath.Join(dir, settingsFileName) + // On Windows, os.Stat defers the file ID lookup to os.SameFile; a handle's Stat records it now. + stat := func() os.FileInfo { + t.Helper() + f, err := os.Open(path) + require.NoError(t, err) + defer f.Close() + info, err := f.Stat() + require.NoError(t, err) + return info + } + initial := stat() + + require.NoError(t, Patch(Settings{LocaleKey: "en-US", CountryCodeKey: "US"})) + changed := stat() + require.NoError(t, Patch(Settings{LocaleKey: "en-US"})) + require.NoError(t, Set(CountryCodeKey, "US")) + + // atomicfile renames a new file into place, so a file with the same identity was never rewritten. + assert.False(t, os.SameFile(initial, changed), "a real change is written") + assert.True(t, os.SameFile(changed, stat()), "unchanged values are not rewritten") +} + +func TestPatchRewritesAFileChangedElsewhere(t *testing.T) { + isolateDiscovery(t) + dir := t.TempDir() + initAt(t, dir) + require.NoError(t, Set(LocaleKey, "en-US")) + path := filepath.Join(dir, settingsFileName) + require.NoError(t, os.WriteFile(path, []byte(`{"locale": "ru-RU"}`), 0o644)) + + require.NoError(t, Set(LocaleKey, "en-US")) + + require.NoError(t, Reload()) + assert.Equal(t, "en-US", GetString(LocaleKey)) +} + +func TestFailedInitKeepsChangesInMemory(t *testing.T) { + isolateDiscovery(t) + Reset() + t.Cleanup(Reset) + dir := t.TempDir() + readOnlyDir(t, dir) + require.Error(t, InitSettings(dir)) + + require.NoError(t, Set(LocaleKey, "en-US")) + + assert.Equal(t, "en-US", GetString(LocaleKey)) +} + func TestReload(t *testing.T) { + isolateDiscovery(t) initSettingsAt := func(t *testing.T, contents string) string { t.Helper() Reset() @@ -635,6 +866,7 @@ func TestReload(t *testing.T) { } func TestReloadRacesNothing(t *testing.T) { + isolateDiscovery(t) Reset() t.Cleanup(Reset) dir := t.TempDir() @@ -722,7 +954,8 @@ func TestWriteAdoptsOtherProcessState(t *testing.T) { got, err := os.ReadFile(path) require.NoError(t, err) - assert.True(t, validSettingsJSON(got), "the corrupt file must be overwritten with valid JSON") + var onDisk map[string]any + assert.NoError(t, json.Unmarshal(got, &onDisk), "the corrupt file must be overwritten with valid JSON") assert.Equal(t, "en-US", GetString(LocaleKey)) }) @@ -737,31 +970,28 @@ func TestWriteAdoptsOtherProcessState(t *testing.T) { }) } -func TestFailedSaveIsNotRevertedByLaterWrites(t *testing.T) { - if runtime.GOOS == "windows" { - t.Skip("uses a read-only directory to force save failures") - } +func TestFailedApplyIsNotRevertedByLaterWrites(t *testing.T) { Reset() t.Cleanup(Reset) dir := t.TempDir() require.NoError(t, InitSettings(dir)) - require.NoError(t, Set(TokenKey, "live-token")) + require.NoError(t, Set(DataPathKey, "old")) - require.NoError(t, os.Chmod(dir, 0o500)) - t.Cleanup(func() { os.Chmod(dir, 0o755) }) - require.Error(t, Clear(TokenKey), "save must fail in a read-only directory") - assert.False(t, Exists(TokenKey), "the deletion is visible in this process despite the failed save") + restore := readOnlyDir(t, dir) + require.Error(t, Apply(Settings{DataPathKey: "new"}), "save must fail in a read-only directory") + assert.Equal(t, "new", GetString(DataPathKey), "Apply takes effect despite the failed save") + require.Error(t, Set(LocaleKey, "en-US")) + assert.Equal(t, "new", GetString(DataPathKey), "a failed later write must not revert it") - require.NoError(t, os.Chmod(dir, 0o755)) + restore() require.NoError(t, Set(LocaleKey, "fa-IR")) - assert.False(t, Exists(TokenKey), "a later write must not resurrect the cleared token") + assert.Equal(t, "new", GetString(DataPathKey), "a later write must not revert the applied value") got, err := os.ReadFile(filepath.Join(dir, settingsFileName)) require.NoError(t, err) var onDisk map[string]any require.NoError(t, json.Unmarshal(got, &onDisk)) - _, tokenOnDisk := onDisk["token"] - assert.False(t, tokenOnDisk, "the pending deletion must reach the file with the next save") + assert.Equal(t, "new", onDisk["data_path"], "the journaled value must reach the file with the next save") assert.Equal(t, "fa-IR", onDisk["locale"]) } @@ -836,10 +1066,13 @@ func TestConditionalWritesDecideAgainstFile(t *testing.T) { }) } -func TestUnreadableFileStillAppliesWritesLocally(t *testing.T) { +func TestUnreadableFileLeavesSettings(t *testing.T) { if runtime.GOOS == "windows" { t.Skip("uses file permissions to force read failures") } + if os.Geteuid() == 0 { + t.Skip("root reads files regardless of mode") + } Reset() t.Cleanup(Reset) dir := t.TempDir() @@ -850,35 +1083,20 @@ func TestUnreadableFileStillAppliesWritesLocally(t *testing.T) { require.NoError(t, os.Chmod(path, 0o000)) t.Cleanup(func() { os.Chmod(path, 0o644) }) require.Error(t, Clear(TokenKey), "an unreadable file must surface as an error") - assert.False(t, Exists(TokenKey), "logout must still take effect in this process") - require.NoError(t, os.Chmod(path, 0o000)) // confirm the failed write left the file alone - got, err := os.ReadFile(path) - require.Error(t, err, "the unreadable file must not be overwritten") - _ = got + assert.Equal(t, "live-token", GetString(TokenKey), "a write that wasn't saved must not take effect") require.NoError(t, os.Chmod(path, 0o644)) - require.NoError(t, Set(LocaleKey, "en-US")) - + require.NoError(t, Clear(TokenKey)) assert.False(t, Exists(TokenKey)) - raw, err := os.ReadFile(path) - require.NoError(t, err) - var onDisk map[string]any - require.NoError(t, json.Unmarshal(raw, &onDisk)) - _, tokenOnDisk := onDisk["token"] - assert.False(t, tokenOnDisk, "the journaled tombstone must reach the file once it is writable") } -func TestInitSettingsSkipsMigrationDespiteMalformedTestingEnv(t *testing.T) { +func TestTestsDontReadTheHostProfile(t *testing.T) { t.Setenv(env.Testing.String(), "yes") - Reset() - t.Cleanup(Reset) dir := t.TempDir() - legacy := []byte(`{"user_level":"pro","token":"leaked"}`) - require.NoError(t, os.WriteFile(filepath.Join(dir, legacySettingsFileName), legacy, 0o644)) - require.NoError(t, InitSettings(dir)) - - assert.NotEqual(t, "pro", GetString(UserLevelKey), - "a malformed RADIANCE_TESTING value must not re-enable the migration in a test binary") - assert.False(t, Exists(TokenKey)) + path, _ := legacyYAMLPath(dir) + if runtime.GOOS != "ios" { + assert.Empty(t, path, "a malformed RADIANCE_TESTING value must not enable host lookups in a test binary") + } + assert.Nil(t, windowsCrossDirCandidates(dir)) } diff --git a/ipc/server.go b/ipc/server.go index 6e27c9c0..89d60a46 100644 --- a/ipc/server.go +++ b/ipc/server.go @@ -1313,7 +1313,11 @@ func (s *localapi) accountOAuthHandler(w http.ResponseWriter, r *http.Request) { } userData, err := s.backend(r.Context()).OAuthLoginCallback(r.Context(), req.OAuthToken) if err != nil { - http.Error(w, err.Error(), http.StatusInternalServerError) + status := http.StatusInternalServerError + if errors.Is(err, account.ErrInvalidToken) { + status = http.StatusBadRequest + } + http.Error(w, err.Error(), status) return } writeJSON(w, http.StatusOK, userData)