fix!: Scope auth transports to allowed origins

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,157 @@ func TestUnauthenticatedRateLimitedTransport_transport(t *testing.T) {
 	}
 }
 
+func TestIsCrossOriginRedirect(t *testing.T) {
+	t.Parallel()
+
+	req := func(from, to string) *http.Request {
+		r := &http.Request{URL: mustParseURL(t, to)}
+		r.Response = &http.Response{Request: &http.Request{URL: mustParseURL(t, from)}}
+		return r
+	}
+
+	tests := []struct {
+		name string
+		req  *http.Request
+		want bool
+	}{
+		{
+			name: "nil request",
+		},
+		{
+			name: "nil url",
+			req:  &http.Request{},
+		},
+		{
+			name: "nil response",
+			req:  &http.Request{URL: mustParseURL(t, "https://api.github.com/")},
+		},
+		{
+			name: "nil response request",
+			req: &http.Request{
+				URL:      mustParseURL(t, "https://api.github.com/"),
+				Response: &http.Response{},
+			},
+		},
+		{
+			name: "nil response request url",
+			req: &http.Request{
+				URL:      mustParseURL(t, "https://api.github.com/"),
+				Response: &http.Response{Request: &http.Request{}},
+			},
+		},
+		{
+			name: "same origin",
+			req:  req("https://api.github.com/repos", "https://api.github.com/orgs"),
+		},
+		{
+			name: "same origin case insensitive",
+			req:  req("HTTPS://API.GITHUB.COM/repos", "https://api.github.com/orgs"),
+		},
+		{
+			name: "different scheme",
+			req:  req("https://api.github.com/repos", "http://api.github.com/repos"),
+			want: true,
+		},
+		{
+			name: "different host",
+			req:  req("https://api.github.com/repos", "https://example.com/repos"),
+			want: true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+			if got := isCrossOriginRedirect(tt.req); got != tt.want {
+				t.Errorf("isCrossOriginRedirect() = %t, want %t", got, tt.want)
+			}
+		})
+	}
+}
+
+func TestSameRedirectOrigin(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name string
+		a    *url.URL
+		b    *url.URL
+		want bool
+	}{
+		{
+			name: "nil url",
+		},
+		{
+			name: "same origin",
+			a:    mustParseURL(t, "https://api.github.test/repos"),
+			b:    mustParseURL(t, "https://api.github.test/orgs"),
+			want: true,
+		},
+		{
+			name: "same origin case insensitive",
+			a:    mustParseURL(t, "HTTPS://API.GITHUB.TEST/repos"),
+			b:    mustParseURL(t, "https://api.github.test/orgs"),
+			want: true,
+		},
+		{
+			name: "different scheme",
+			a:    mustParseURL(t, "http://api.github.test/repos"),
+			b:    mustParseURL(t, "https://api.github.test/repos"),
+		},
+		{
+			name: "different host",
+			a:    mustParseURL(t, "https://api.github.test/repos"),
+			b:    mustParseURL(t, "https://example.test/repos"),
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+			if got := sameRedirectOrigin(tt.a, tt.b); got != tt.want {
+				t.Errorf("sameRedirectOrigin() = %t, want %t", got, tt.want)
+			}
+		})
+	}
+}
+
+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 +4481,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