persisted identity and account correctness

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

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")
+}

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
 }

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