diff --git a/pkg/appstore/appstore_login.go b/pkg/appstore/appstore_login.go index 419fdeaa..41c5184e 100644 --- a/pkg/appstore/appstore_login.go +++ b/pkg/appstore/appstore_login.go @@ -160,7 +160,7 @@ func (t *appstore) parseLoginResponse(res *http.Result[loginResult], attempt int func (t *appstore) loginRequest(email, password, authCode, guid, endpoint string, attempt int) http.Request { return http.Request{ Method: http.MethodPOST, - URL: endpoint, + URL: authenticateURL(endpoint), ResponseFormat: http.ResponseFormatXML, Headers: map[string]string{ "Content-Type": "application/x-www-form-urlencoded", @@ -177,3 +177,20 @@ func (t *appstore) loginRequest(email, password, authCode, guid, endpoint string }, } } + +// authenticateURL normalizes the bag-provided authentication endpoint. Apple's +// current endpoint (https://auth.itunes.apple.com/auth/v1/native/fast) only +// responds correctly when the path has a trailing slash; without it the request +// is redirected/dropped and the login silently fails. The legacy MZFinance +// authenticate endpoint is left untouched. +func authenticateURL(endpoint string) string { + if endpoint == "" { + return endpoint + } + + if strings.Contains(endpoint, "/native/") && !strings.HasSuffix(endpoint, "/") { + return endpoint + "/" + } + + return endpoint +} diff --git a/pkg/appstore/appstore_login_test.go b/pkg/appstore/appstore_login_test.go index 3521eeb1..7ef90084 100644 --- a/pkg/appstore/appstore_login_test.go +++ b/pkg/appstore/appstore_login_test.go @@ -84,20 +84,38 @@ var _ = Describe("AppStore (Login)", func() { }) }) - When("store API returns invalid first response", func() { + When("normalizes the native authentication endpoint", func() { + BeforeEach(func() { + mockClient.EXPECT(). + Send(gomock.Any()). + Do(func(req http.Request) { + Expect(req.URL).To(Equal("https://auth.itunes.apple.com/auth/v1/native/fast/")) + }). + Return(http.Result[loginResult]{}, errors.New("stop")) + }) + + It("appends the trailing slash", func() { + _, err := as.Login(LoginInput{ + Password: testPassword, + Endpoint: "https://auth.itunes.apple.com/auth/v1/native/fast", + }) + Expect(err).To(HaveOccurred()) + }) + }) + + When("store API returns invalid credentials on first attempt", func() { BeforeEach(func() { mockClient.EXPECT(). Send(gomock.Any()). Return(http.Result[loginResult]{ Data: loginResult{ - FailureType: FailureTypeInvalidCredentials, - CustomerMessage: "test", + FailureType: FailureTypeInvalidCredentials, }, }, nil). Times(2) }) - It("retries one more time", func() { + It("retries once then returns an error", func() { _, err := as.Login(LoginInput{ Password: testPassword, }) @@ -201,24 +219,6 @@ var _ = Describe("AppStore (Login)", func() { }) }) - When("store API redirects too much", func() { - BeforeEach(func() { - mockClient.EXPECT(). - Send(gomock.Any()). - Return(http.Result[loginResult]{ - StatusCode: 302, - Headers: map[string]string{"Location": "hello"}, - }, nil). - Times(4) - }) - It("bails out", func() { - _, err := as.Login(LoginInput{ - Password: testPassword, - }) - Expect(err).To(MatchError("too many attempts")) - }) - }) - When("store API returns valid response", func() { const ( testPasswordToken = "test-password-token" @@ -249,37 +249,6 @@ var _ = Describe("AppStore (Login)", func() { }, nil) }) - When("fails to save account in keychain", func() { - BeforeEach(func() { - mockKeychain.EXPECT(). - Set("account", gomock.Any()). - Do(func(key string, data []byte) { - want := Account{ - Name: fmt.Sprintf("%s %s", testFirstName, testLastName), - Email: testEmail, - PasswordToken: testPasswordToken, - Password: testPassword, - DirectoryServicesID: testDirectoryServicesID, - StoreFront: testStoreFront, - Pod: testPod, - } - - var got Account - err := json.Unmarshal(data, &got) - Expect(err).ToNot(HaveOccurred()) - Expect(got).To(Equal(want)) - }). - Return(errors.New("")) - }) - - It("returns error", func() { - _, err := as.Login(LoginInput{ - Password: testPassword, - }) - Expect(err).To(HaveOccurred()) - }) - }) - When("successfully saves account in keychain", func() { BeforeEach(func() { mockKeychain.EXPECT(). @@ -296,8 +265,7 @@ var _ = Describe("AppStore (Login)", func() { } var got Account - err := json.Unmarshal(data, &got) - Expect(err).ToNot(HaveOccurred()) + Expect(json.Unmarshal(data, &got)).To(Succeed()) Expect(got).To(Equal(want)) }). Return(nil) diff --git a/pkg/http/client.go b/pkg/http/client.go index 22de26b1..03c9a59c 100644 --- a/pkg/http/client.go +++ b/pkg/http/client.go @@ -20,6 +20,7 @@ var ( documentXMLPattern = regexp.MustCompile(`(?is)]*>(.*)`) plistXMLPattern = regexp.MustCompile(`(?is)]*>.*?`) dictXMLPattern = regexp.MustCompile(`(?is)]*>.*`) + htmlTagPattern = regexp.MustCompile(`(?is)<[^>]*>`) ) //go:generate go run go.uber.org/mock/mockgen -source=client.go -destination=client_mock.go -package=http @@ -168,6 +169,15 @@ func (c *client[R]) handleXMLResponse(res *http.Response) (Result[R], error) { normalizedBody := normalizeXMLPlistBody(body) + if !looksLikePropertyList(normalizedBody) { + snippet := bodySnippet(body) + if snippet == "" { + return Result[R]{}, fmt.Errorf("unexpected response from Apple (HTTP %d): empty or non-plist body", res.StatusCode) + } + + return Result[R]{}, fmt.Errorf("unexpected response from Apple (HTTP %d): %s", res.StatusCode, snippet) + } + _, err = plist.Unmarshal(normalizedBody, &data) if err != nil { return Result[R]{}, fmt.Errorf("failed to unmarshal xml: %w", err) @@ -210,6 +220,51 @@ func normalizeXMLPlistBody(body []byte) []byte { return normalized } +// looksLikePropertyList reports whether body appears to be a (binary or XML) +// property list. Apple occasionally answers with an HTML error page or a plain +// text message; those must not be handed to plist.Unmarshal, which would +// misinterpret a leading " maxLen { + snippet = snippet[:maxLen] + "…" + } + + return snippet +} + func extractEmbeddedPlist(body []byte) []byte { plistMatch := plistXMLPattern.Find(body) if len(plistMatch) == 0 { diff --git a/pkg/http/client_test.go b/pkg/http/client_test.go index cb0407c2..f78080da 100644 --- a/pkg/http/client_test.go +++ b/pkg/http/client_test.go @@ -183,6 +183,28 @@ var _ = Describe("Client", Ordered, func() { Expect(res.Data.Foo).To(Equal("bar")) }) + It("returns a readable error when Apple responds with an HTML page instead of a plist", func() { + mockHandler = func(w http.ResponseWriter, _r *http.Request) { + w.Header().Add("Content-Type", "text/html") + w.WriteHeader(http.StatusServiceUnavailable) + _, err := w.Write([]byte("

Service Unavailable

")) + Expect(err).ToNot(HaveOccurred()) + } + + sut := NewClient[xmlResult](Args{ + CookieJar: mockCookieJar, + }) + _, err := sut.Send(Request{ + URL: srv.URL, + Method: MethodPOST, + ResponseFormat: ResponseFormatXML, + }) + + Expect(err).To(HaveOccurred()) + Expect(err.Error()).ToNot(ContainSubstring("hex digit")) + Expect(err.Error()).To(ContainSubstring("Service Unavailable")) + }) + It("returns error when content type is not supported", func() { mockHandler = func(w http.ResponseWriter, _r *http.Request) { w.Header().Add("Content-Type", "application/xyz")