Fix repeated Keychain access during download retries

cmd/download.go

@@ -40,20 +40,18 @@ func downloadCmdWithAppStore(appStore func() appstore.AppStore) *cobra.Command {
 				return err
 			}
 
+			store := appStore()
+			infoResult, err := store.AccountInfo()
+			if err != nil {
+				return err
+			}
+
+			acc := infoResult.Account
 			var lastErr error
-			var acc appstore.Account
 			purchaseRequired := false
 			purchased := false
 
 			return retry.Do(func() error {
-				store := appStore()
-				infoResult, err := store.AccountInfo()
-				if err != nil {
-					return err
-				}
-
-				acc = infoResult.Account
-
 				if errors.Is(lastErr, appstore.ErrPasswordTokenExpired) {
 					loginResult, err := store.Login(appstore.LoginInput{
 						Email:    acc.Email,

cmd/download_test.go

@@ -77,6 +77,7 @@ var _ = Describe("Download command", func() {
 		cmd.SetContext(context.WithValue(context.Background(), interactiveKey, false))
 
 		Expect(cmd.Execute()).To(Succeed())
+		Expect(store.accountInfoCalls).To(Equal(1))
 		Expect(store.purchaseInputs).To(HaveLen(1))
 		Expect(store.purchaseInputs[0].Platform).To(Equal(appstore.PlatformMacOS))
 		Expect(store.purchaseInputs[0].App).To(Equal(appstore.App{ID: 42}))
@@ -99,12 +100,74 @@ var _ = Describe("Download command", func() {
 		cmd.SetContext(context.WithValue(context.Background(), interactiveKey, false))
 
 		Expect(cmd.Execute()).To(Succeed())
+		Expect(store.accountInfoCalls).To(Equal(1))
 		Expect(store.loginInputs).To(HaveLen(1))
 		Expect(store.purchaseInputs).To(HaveLen(2))
 		Expect(store.purchaseInputs[0].Platform).To(Equal(appstore.PlatformMacOS))
 		Expect(store.purchaseInputs[1].Platform).To(Equal(appstore.PlatformMacOS))
 		Expect(store.downloadInputs).To(HaveLen(2))
 	})
+
+	DescribeTable("reuses the account throughout download retries", func(downloadErrors []error, purchaseCount int) {
+		account := appstore.Account{
+			Email:         "user@example.com",
+			Password:      "password",
+			PasswordToken: "expired-token",
+			StoreFront:    "143441",
+		}
+		refreshedAccount := account
+		refreshedAccount.PasswordToken = "refreshed-token"
+		store := &fakeDownloadAppStore{
+			account:        account,
+			loginAccount:   refreshedAccount,
+			downloadErrors: downloadErrors,
+		}
+		previousDependencies := dependencies
+		DeferCleanup(func() { dependencies = previousDependencies })
+		dependencies.Logger = log.NewLogger(log.Args{})
+
+		cmd := downloadCmdWithAppStore(func() appstore.AppStore { return store })
+		cmd.SetArgs([]string{"--bundle-identifier", "com.example.app", "--purchase"})
+		cmd.SetContext(context.WithValue(context.Background(), interactiveKey, false))
+
+		Expect(cmd.Execute()).To(Succeed())
+		Expect(store.accountInfoCalls).To(Equal(1))
+		Expect(store.loginInputs).To(Equal([]appstore.LoginInput{{Email: account.Email, Password: account.Password}}))
+		Expect(store.lookupInputs).To(HaveLen(len(downloadErrors)))
+		Expect(store.lookupInputs[0].Account).To(Equal(account))
+		Expect(store.downloadInputs).To(HaveLen(len(downloadErrors)))
+		Expect(store.downloadInputs[0].Account).To(Equal(account))
+
+		for _, input := range store.lookupInputs[1:] {
+			Expect(input.Account).To(Equal(refreshedAccount))
+		}
+
+		for _, input := range store.downloadInputs[1:] {
+			Expect(input.Account).To(Equal(refreshedAccount))
+		}
+
+		Expect(store.purchaseInputs).To(HaveLen(purchaseCount))
+
+		for _, input := range store.purchaseInputs {
+			Expect(input.Account).To(Equal(refreshedAccount))
+		}
+	},
+		Entry("refreshes an expired token", []error{appstore.ErrPasswordTokenExpired, nil}, 0),
+		Entry("retains the refreshed token for a subsequent purchase", []error{appstore.ErrPasswordTokenExpired, appstore.ErrLicenseRequired, nil}, 1),
+	)
+
+	It("returns an account read error before attempting a download", func() {
+		store := &fakeDownloadAppStore{accountInfoError: errors.New("account unavailable")}
+		cmd := downloadCmdWithAppStore(func() appstore.AppStore { return store })
+		cmd.SetArgs([]string{"--app-id", "42"})
+
+		Expect(cmd.Execute()).To(MatchError(store.accountInfoError))
+		Expect(store.accountInfoCalls).To(Equal(1))
+		Expect(store.loginInputs).To(BeEmpty())
+		Expect(store.lookupInputs).To(BeEmpty())
+		Expect(store.purchaseInputs).To(BeEmpty())
+		Expect(store.downloadInputs).To(BeEmpty())
+	})
 })
 
 type fakeSinfReplicator struct {
@@ -121,26 +184,35 @@ func (f *fakeSinfReplicator) ReplicateSinf(input appstore.ReplicateSinfInput) er
 }
 
 type fakeDownloadAppStore struct {
-	downloadErrors []error
-	downloadInputs []appstore.DownloadInput
-	loginInputs    []appstore.LoginInput
-	purchaseErrors []error
-	purchaseInputs []appstore.PurchaseInput
+	account          appstore.Account
+	accountInfoCalls int
+	accountInfoError error
+	downloadErrors   []error
+	downloadInputs   []appstore.DownloadInput
+	loginAccount     appstore.Account
+	loginInputs      []appstore.LoginInput
+	lookupInputs     []appstore.LookupInput
+	purchaseErrors   []error
+	purchaseInputs   []appstore.PurchaseInput
 }
 
 func (f *fakeDownloadAppStore) Login(input appstore.LoginInput) (appstore.LoginOutput, error) {
 	f.loginInputs = append(f.loginInputs, input)
 
-	return appstore.LoginOutput{Account: appstore.Account{StoreFront: "143441"}}, nil
+	return appstore.LoginOutput{Account: f.loginAccount}, nil
 }
 
-func (*fakeDownloadAppStore) AccountInfo() (appstore.AccountInfoOutput, error) {
-	return appstore.AccountInfoOutput{Account: appstore.Account{StoreFront: "143441"}}, nil
+func (f *fakeDownloadAppStore) AccountInfo() (appstore.AccountInfoOutput, error) {
+	f.accountInfoCalls++
+
+	return appstore.AccountInfoOutput{Account: f.account}, f.accountInfoError
 }
 
 func (*fakeDownloadAppStore) Revoke() error { return nil }
 
-func (*fakeDownloadAppStore) Lookup(input appstore.LookupInput) (appstore.LookupOutput, error) {
+func (f *fakeDownloadAppStore) Lookup(input appstore.LookupInput) (appstore.LookupOutput, error) {
+	f.lookupInputs = append(f.lookupInputs, input)
+
 	return appstore.LookupOutput{}, nil
 }