Handle multiple www-authenticate headers (#1075)

* Additional check added upon receiving multiple challenges

It might happen there are multiple `www-authenticate` headers returned from a registry, e.g. `Negotiate` and `Basic`. In the current code, in such situation the first challenge would be picked with no further checks, which would result eventually in `unrecognized challenge` error failing the whole build, even though `Basic` challenge could be used instead. `pickFromMultipleChallenges` function was added to loop through the challenges in search for a supported one.

* Added TestPingMultipleChallenges unit test

* updated ping.go and unit test added

* fix gofmt issue

pkg/v1/remote/transport/ping.go

@@ -108,8 +108,8 @@ func ping(ctx context.Context, reg name.Registry, t http.RoundTripper) (*pingRes
 			}, nil
 		case http.StatusUnauthorized:
 			if challenges := authchallenge.ResponseChallenges(resp); len(challenges) != 0 {
-				// If we hit more than one, I'm not even sure what to do.
-				wac := challenges[0]
+				// If we hit more than one, let's try to find one that we know how to handle.
+				wac := pickFromMultipleChallenges(challenges)
 				return &pingResp{
 					challenge:  challenge(wac.Scheme).Canonical(),
 					parameters: wac.Parameters,
@@ -127,3 +127,22 @@ func ping(ctx context.Context, reg name.Registry, t http.RoundTripper) (*pingRes
 	}
 	return nil, errors.New(strings.Join(errs, "; "))
 }
+
+func pickFromMultipleChallenges(challenges []authchallenge.Challenge) authchallenge.Challenge {
+
+	// It might happen there are multiple www-authenticate headers, e.g. `Negotiate` and `Basic`.
+	// Picking simply the first one could result eventually in `unrecognized challenge` error,
+	// that's why we're looping through the challenges in search for one that can be handled.
+	allowedSchemes := []string{"basic", "bearer"}
+
+	for _, wac := range challenges {
+		currentScheme := strings.ToLower(wac.Scheme)
+		for _, allowed := range allowedSchemes {
+			if allowed == currentScheme {
+				return wac
+			}
+		}
+	}
+
+	return challenges[0]
+}

pkg/v1/remote/transport/ping_test.go

@@ -147,6 +147,55 @@ func TestPingBearerChallengeWithParams(t *testing.T) {
 	}
 }
 
+func TestPingMultipleChallenges(t *testing.T) {
+	server := httptest.NewServer(
+		http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			w.Header().Add("WWW-Authenticate", "Negotiate")
+			w.Header().Add("WWW-Authenticate", `Basic realm="http://auth.example.com/token"`)
+			http.Error(w, "Unauthorized", http.StatusUnauthorized)
+		}))
+	defer server.Close()
+	tprt := &http.Transport{
+		Proxy: func(req *http.Request) (*url.URL, error) {
+			return url.Parse(server.URL)
+		},
+	}
+
+	pr, err := ping(context.Background(), testRegistry, tprt)
+	if err != nil {
+		t.Errorf("ping() = %v", err)
+	}
+	if pr.challenge != basic {
+		t.Errorf("ping(); got %v, want %v", pr.challenge, basic)
+	}
+	if got, want := len(pr.parameters), 1; got != want {
+		t.Errorf("ping(); got %v, want %v", got, want)
+	}
+}
+
+func TestPingMultipleNotSupportedChallenges(t *testing.T) {
+	server := httptest.NewServer(
+		http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			w.Header().Add("WWW-Authenticate", "Negotiate")
+			w.Header().Add("WWW-Authenticate", "Digest")
+			http.Error(w, "Unauthorized", http.StatusUnauthorized)
+		}))
+	defer server.Close()
+	tprt := &http.Transport{
+		Proxy: func(req *http.Request) (*url.URL, error) {
+			return url.Parse(server.URL)
+		},
+	}
+
+	pr, err := ping(context.Background(), testRegistry, tprt)
+	if err != nil {
+		t.Errorf("ping() = %v", err)
+	}
+	if pr.challenge != "negotiate" {
+		t.Errorf("ping(); got %v, want %v", pr.challenge, "negotiate")
+	}
+}
+
 func TestUnsupportedStatus(t *testing.T) {
 	server := httptest.NewServer(
 		http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {