Avoid auth on cross-origin redirects

Author: huynhtrungcsc

github/github.go

@@ -1981,6 +1981,20 @@ func setCredentialsAsHeaders(req *http.Request, id, secret string) *http.Request
 	return &convertedRequest
 }
 
+func isCrossOriginRedirect(req *http.Request) bool {
+	if req == nil || req.URL == nil || req.Response == nil || req.Response.Request == nil || req.Response.Request.URL == nil {
+		return false
+	}
+	return !sameRedirectOrigin(req.URL, req.Response.Request.URL)
+}
+
+func sameRedirectOrigin(a, b *url.URL) bool {
+	if a == nil || b == nil {
+		return false
+	}
+	return strings.EqualFold(a.Scheme, b.Scheme) && strings.EqualFold(a.Host, b.Host)
+}
+
 /*
 UnauthenticatedRateLimitedTransport allows you to make unauthenticated calls
 that need to use a higher rate limit associated with your OAuth application.
@@ -2021,6 +2035,10 @@ func (t *UnauthenticatedRateLimitedTransport) RoundTrip(req *http.Request) (*htt
 		return nil, errors.New("t.ClientSecret is empty")
 	}
 
+	if isCrossOriginRedirect(req) {
+		return t.transport().RoundTrip(req)
+	}
+
 	req2 := setCredentialsAsHeaders(req, t.ClientID, t.ClientSecret)
 	// Make the HTTP request.
 	return t.transport().RoundTrip(req2)
@@ -2055,6 +2073,10 @@ type BasicAuthTransport struct {
 
 // RoundTrip implements the RoundTripper interface.
 func (t *BasicAuthTransport) RoundTrip(req *http.Request) (*http.Response, error) {
+	if isCrossOriginRedirect(req) {
+		return t.transport().RoundTrip(req)
+	}
+
 	req2 := setCredentialsAsHeaders(req, t.Username, t.Password)
 	if t.OTP != "" {
 		req2.Header.Set(headerOTP, t.OTP)

github/github_test.go

@@ -4296,6 +4296,42 @@ func TestUnauthenticatedRateLimitedTransport_transport(t *testing.T) {
 	}
 }
 
+func TestUnauthenticatedRateLimitedTransport_doesNotAuthorizeCrossOriginRedirect(t *testing.T) {
+	t.Parallel()
+
+	gotAuth := make(chan string, 1)
+	attacker := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		gotAuth <- r.Header.Get("Authorization")
+		w.WriteHeader(http.StatusOK)
+		_, _ = w.Write([]byte(`{}`))
+	}))
+	defer attacker.Close()
+
+	trusted := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		http.Redirect(w, r, attacker.URL+"/steal", http.StatusFound)
+	}))
+	defer trusted.Close()
+
+	tp := &UnauthenticatedRateLimitedTransport{
+		ClientID:     "id",
+		ClientSecret: "secret",
+	}
+	c := mustNewClient(t, WithHTTPClient(tp.Client()), WithURLs(Ptr(trusted.URL+"/"), nil))
+	req, err := c.NewRequest(t.Context(), "GET", "anything", nil)
+	if err != nil {
+		t.Fatalf("NewRequest returned unexpected error: %v", err)
+	}
+	resp, err := c.Do(req, nil)
+	if err != nil {
+		t.Fatalf("Do returned unexpected error: %v", err)
+	}
+	resp.Body.Close()
+
+	if got := <-gotAuth; got != "" {
+		t.Errorf("Authorization on cross-origin redirect = %q, want empty", got)
+	}
+}
+
 func TestBasicAuthTransport(t *testing.T) {
 	t.Parallel()
 	client, mux, _ := setup(t)
@@ -4330,6 +4366,48 @@ func TestBasicAuthTransport(t *testing.T) {
 	assertNilError(t, err)
 }
 
+func TestBasicAuthTransport_doesNotAuthorizeCrossOriginRedirect(t *testing.T) {
+	t.Parallel()
+
+	gotAuth := make(chan string, 1)
+	gotOTP := make(chan string, 1)
+	attacker := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		gotAuth <- r.Header.Get("Authorization")
+		gotOTP <- r.Header.Get(headerOTP)
+		w.WriteHeader(http.StatusOK)
+		_, _ = w.Write([]byte(`{}`))
+	}))
+	defer attacker.Close()
+
+	trusted := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		http.Redirect(w, r, attacker.URL+"/steal", http.StatusFound)
+	}))
+	defer trusted.Close()
+
+	tp := &BasicAuthTransport{
+		Username: "u",
+		Password: "p",
+		OTP:      "123456",
+	}
+	c := mustNewClient(t, WithHTTPClient(tp.Client()), WithURLs(Ptr(trusted.URL+"/"), nil))
+	req, err := c.NewRequest(t.Context(), "GET", "anything", nil)
+	if err != nil {
+		t.Fatalf("NewRequest returned unexpected error: %v", err)
+	}
+	resp, err := c.Do(req, nil)
+	if err != nil {
+		t.Fatalf("Do returned unexpected error: %v", err)
+	}
+	resp.Body.Close()
+
+	if got := <-gotAuth; got != "" {
+		t.Errorf("Authorization on cross-origin redirect = %q, want empty", got)
+	}
+	if got := <-gotOTP; got != "" {
+		t.Errorf("OTP on cross-origin redirect = %q, want empty", got)
+	}
+}
+
 func TestBasicAuthTransport_transport(t *testing.T) {
 	t.Parallel()
 	// default transport