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.
  • Loading branch information
samcoe committed Jul 12, 2023
1 parent 8c7935e commit 83ec7b5
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
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

0 comments on commit 83ec7b5

Please sign in to comment.