Change API authentication to allow asset downloads

mislav · mislav · commit 33fd6b1149bc · 2020-08-21T18:00:52.000+02:00
We install an HTTP middleware that adds the "Authorization" header on
every HTTP request. However, our asset download process might redirect
to a 3rd-party host (Amazon S3) and we want to allow those requests but
not require that they are authenticated.

Furthermore, we need the ability to specify the `Accept` request header
without it being overwritten by middleware, so now middleware only adds
headers that are not present in a request.
diff --git a/api/client.go b/api/client.go
@@ -45,7 +45,9 @@ func NewClientFromHTTP(httpClient *http.Client) *Client {
 func AddHeader(name, value string) ClientOption {
 	return func(tr http.RoundTripper) http.RoundTripper {
 		return &funcTripper{roundTrip: func(req *http.Request) (*http.Response, error) {
-			req.Header.Add(name, value)
+			if len(req.Header.Values(name)) == 0 {
+				req.Header.Add(name, value)
+			}
 			return tr.RoundTrip(req)
 		}}
 	}
@@ -55,11 +57,16 @@ func AddHeader(name, value string) ClientOption {
 func AddHeaderFunc(name string, getValue func(*http.Request) (string, error)) ClientOption {
 	return func(tr http.RoundTripper) http.RoundTripper {
 		return &funcTripper{roundTrip: func(req *http.Request) (*http.Response, error) {
+			if len(req.Header.Values(name)) > 0 {
+				return tr.RoundTrip(req)
+			}
 			value, err := getValue(req)
 			if err != nil {
 				return nil, err
 			}
-			req.Header.Add(name, value)
+			if value != "" {
+				req.Header.Add(name, value)
+			}
 			return tr.RoundTrip(req)
 		}}
 	}
diff --git a/pkg/cmd/factory/http.go b/pkg/cmd/factory/http.go
@@ -1,7 +1,6 @@
 package factory
 
 import (
-	"errors"
 	"fmt"
 	"net/http"
 	"os"
@@ -30,20 +29,8 @@ func httpClient(io *iostreams.IOStreams, cfg config.Config, appVersion string, s
 
 			hostname := ghinstance.NormalizeHostname(req.URL.Hostname())
 			token, err := cfg.Get(hostname, "oauth_token")
-			if token == "" {
-				var notFound *config.NotFoundError
-				// TODO: check if stdout is TTY too
-				if errors.As(err, &notFound) && io.IsStdinTTY() {
-					// interactive OAuth flow
-					token, err = config.AuthFlowWithConfig(cfg, hostname, "Notice: authentication required", nil)
-				}
-				if err != nil {
-					return "", err
-				}
-				if token == "" {
-					// TODO: instruct user how to manually authenticate
-					return "", fmt.Errorf("authentication required for %s", hostname)
-				}
+			if err != nil || token == "" {
+				return "", nil
 			}
 
 			return fmt.Sprintf("token %s", token), nil
