Skip to content

Commit

Permalink
Do not add auth token to redirect requests which do not have the same…
Browse files Browse the repository at this point in the history
… host as the inital request. (#7692)
  • Loading branch information
samcoe committed Jul 13, 2023
1 parent 8c7935e commit 2a4160a
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 4 deletions.
16 changes: 12 additions & 4 deletions api/http_client.go
Expand Up @@ -91,9 +91,17 @@ func AddAuthTokenHeader(rt http.RoundTripper, cfg tokenGetter) http.RoundTripper
return &funcTripper{roundTrip: func(req *http.Request) (*http.Response, error) {
// If the header is already set in the request, don't overwrite it.
if req.Header.Get(authorization) == "" {
hostname := ghinstance.NormalizeHostname(getHost(req))
if token, _ := cfg.Token(hostname); token != "" {
req.Header.Set(authorization, fmt.Sprintf("token %s", token))
var redirectHostnameChange bool
if req.Response != nil && req.Response.Request != nil {
redirectHostnameChange = getHost(req) != getHost(req.Response.Request)
}
// Only set header if an initial request or redirect request to the same host as the initial request.
// If the host has changed during a redirect do not add the authentication token header.
if !redirectHostnameChange {
hostname := ghinstance.NormalizeHostname(getHost(req))
if token, _ := cfg.Token(hostname); token != "" {
req.Header.Set(authorization, fmt.Sprintf("token %s", token))
}
}
}
return rt.RoundTrip(req)
Expand Down Expand Up @@ -128,5 +136,5 @@ func getHost(r *http.Request) string {
if r.Host != "" {
return r.Host
}
return r.URL.Hostname()
return r.URL.Host
}
35 changes: 35 additions & 0 deletions api/http_client_test.go
Expand Up @@ -6,6 +6,7 @@ import (
"net/http/httptest"
"os"
"regexp"
"strings"
"testing"

"github.com/MakeNowJust/heredoc"
Expand Down Expand Up @@ -200,6 +201,40 @@ func TestNewHTTPClient(t *testing.T) {
}
}

func TestHTTPClientRedirectAuthenticationHeaderHandling(t *testing.T) {
var request *http.Request
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
request = r

This comment was marked as spam.

Copy link
@mj-collab

mj-collab Aug 9, 2023

Unique

This comment was marked as spam.

w.WriteHeader(http.StatusNoContent)
}))
defer server.Close()

var redirectRequest *http.Request
redirectServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
redirectRequest = r
http.Redirect(w, r, server.URL, http.StatusFound)
}))
defer redirectServer.Close()

client, err := NewHTTPClient(HTTPClientOptions{
Config: tinyConfig{
fmt.Sprintf("%s:oauth_token", strings.TrimPrefix(redirectServer.URL, "http://")): "REDIRECT-TOKEN",
fmt.Sprintf("%s:oauth_token", strings.TrimPrefix(server.URL, "http://")): "TOKEN",
},
})
require.NoError(t, err)

req, err := http.NewRequest("GET", redirectServer.URL, nil)
require.NoError(t, err)

res, err := client.Do(req)
require.NoError(t, err)

assert.Equal(t, "token REDIRECT-TOKEN", redirectRequest.Header.Get(authorization))
assert.Equal(t, "", request.Header.Get(authorization))
assert.Equal(t, 204, res.StatusCode)
}

type tinyConfig map[string]string

func (c tinyConfig) Token(host string) (string, string) {
Expand Down

2 comments on commit 2a4160a

@mj-collab

This comment was marked as spam.

@mj-collab

This comment was marked as spam.

Please sign in to comment.