Repository: google/go-containerregistry (go)

Issue:
Implement password grant_type for oauth
For [the oauth flow](https://docs.docker.com/registry/spec/auth/oauth/), we currently just pass through IdentityToken if it's set. In order to fix azure support, I believe we also need to implement the username/password flow for oauth.

Pull request:
Fall back to oauth flow with username/password
Fixes #565

Patch:
diff --git a/pkg/v1/remote/transport/bearer.go b/pkg/v1/remote/transport/bearer.go
index adaccd6f..74e59706 100644
--- a/pkg/v1/remote/transport/bearer.go
+++ b/pkg/v1/remote/transport/bearer.go
@@ -98,20 +98,33 @@ func (bt *bearerTransport) RoundTrip(in *http.Request) (*http.Response, error) {
 	return res, err
 }
 
-// TODO(jonjohnsonjr): Can we just use this?
-// https://github.com/docker/distribution/blob/master/registry/client/auth/session.go
+// It's unclear which authentication flow to use based purely on the protocol,
+// so we rely on heuristics and fallbacks to support as many registries as possible.
+// The basic token exchange is attempted first, falling back to the oauth flow.
+// If the IdentityToken is set, this indicates that we should start with the oauth flow.
 func (bt *bearerTransport) refresh() error {
+	first, second := bt.refreshBasic, bt.refreshOauth
+
+	auth, err := bt.basic.Authorization()
+	if err != nil {
+		return err
+	}
+	if auth.IdentityToken != "" {
+		// If the secret being stored is an identity token,
+		// the Username should be set to <token>, which indicates
+		// we are using an oauth flow.
+		first, second = bt.refreshOauth, bt.refreshBasic
+	}
+
 	content, err := func() ([]byte, error) {
-		// If the secret being stored is an identity token, the Username should be set to <token>.
-		// https://github.com/docker/cli/blob/0f337f1dfe574eb12eab8bb102a24f714cc79d86/docs/reference/commandline/login.md#credential-helper-protocol
-		auth, err := bt.basic.Authorization()
+		b, err := first()
 		if err != nil {
-			return nil, err
-		}
-		if auth.IdentityToken != "" {
-			return bt.refreshOauth(auth)
+			b, err = second()
+			if err != nil {
+				return nil, err
+			}
 		}
-		return bt.refreshBasic()
+		return b, err
 	}()
 	if err != nil {
 		return err
@@ -119,8 +132,10 @@ func (bt *bearerTransport) refresh() error {
 
 	// Some registries don't have "token" in the response. See #54.
 	type tokenResponse struct {
-		Token       string `json:"token"`
-		AccessToken string `json:"access_token"`
+		Token        string `json:"token"`
+		AccessToken  string `json:"access_token"`
+		RefreshToken string `json:"refresh_token"`
+		// TODO: handle expiry?
 	}
 
 	var response tokenResponse
@@ -128,16 +143,26 @@ func (bt *bearerTransport) refresh() error {
 		return err
 	}
 
+	// Some registries set access_token instead of token.
+	if response.AccessToken != "" {
+		response.Token = response.AccessToken
+	}
+
 	// Find a token to turn into a Bearer authenticator
 	var bearer authn.Bearer
 	if response.Token != "" {
 		bearer = authn.Bearer{Token: response.Token}
-	} else if response.AccessToken != "" {
-		bearer = authn.Bearer{Token: response.AccessToken}
 	} else {
 		return fmt.Errorf("no token in bearer response:\n%s", content)
 	}
 
+	// If we obtained a refresh token from the oauth flow, use that for refresh() now.
+	if response.RefreshToken != "" {
+		bt.basic = authn.FromConfig(authn.AuthConfig{
+			IdentityToken: response.RefreshToken,
+		})
+	}
+
 	// Replace our old bearer authenticator (if we had one) with our newly refreshed authenticator.
 	bt.bearer = &bearer
 	return nil
@@ -169,19 +194,30 @@ func (bt *bearerTransport) canonicalAddress(host string) (address string) {
 }
 
 // https://docs.docker.com/registry/spec/auth/oauth/
-func (bt *bearerTransport) refreshOauth(auth *authn.AuthConfig) ([]byte, error) {
-	u, err := url.Parse(bt.realm)
+func (bt *bearerTransport) refreshOauth() ([]byte, error) {
+	auth, err := bt.basic.Authorization()
 	if err != nil {
 		return nil, err
 	}
 
-	v := url.Values{
-		"scope": bt.scopes,
+	u, err := url.Parse(bt.realm)
+	if err != nil {
+		return nil, err
 	}
+
+	v := url.Values{}
+	v.Set("scope", strings.Join(bt.scopes, " "))
 	v.Set("service", bt.service)
 	v.Set("client_id", transportName)
-	v.Set("grant_type", "refresh_token")
-	v.Set("refresh_token", auth.IdentityToken)
+	if auth.IdentityToken != "" {
+		v.Set("grant_type", "refresh_token")
+		v.Set("refresh_token", auth.IdentityToken)
+	} else if auth.Username != "" && auth.Password != "" {
+		v.Set("grant_type", "password")
+		v.Set("username", auth.Username)
+		v.Set("password", auth.Password)
+		v.Set("access_type", "offline")
+	}
 
 	client := http.Client{Transport: bt.inner}
 	resp, err := client.PostForm(u.String(), v)
@@ -190,17 +226,10 @@ func (bt *bearerTransport) refreshOauth(auth *authn.AuthConfig) ([]byte, error)
 	}
 	defer resp.Body.Close()
 
-	if err := CheckError(resp, http.StatusOK, http.StatusNotFound); err != nil {
+	if err := CheckError(resp, http.StatusOK); err != nil {
 		return nil, err
 	}
 
-	// > Not all token servers implement oauth2. If the request to the endpoint
-	// > returns 404 using the HTTP POST method, refer to Token Documentation for
-	// > using the HTTP GET method supported by all token servers.
-	if resp.StatusCode == http.StatusNotFound {
-		return bt.refreshBasic()
-	}
-
 	return ioutil.ReadAll(resp.Body)
 }
 


Test patch:
diff --git a/pkg/v1/remote/transport/bearer_test.go b/pkg/v1/remote/transport/bearer_test.go
index 6e21ef3e..ba575024 100644
--- a/pkg/v1/remote/transport/bearer_test.go
+++ b/pkg/v1/remote/transport/bearer_test.go
@@ -259,6 +259,70 @@ func TestBearerTransportOauthRefresh(t *testing.T) {
 	}
 }
 
+func TestBearerTransportOauthRefreshToken(t *testing.T) {
+	initialToken := "initial_token"
+	accessToken := "access_token"
+	refreshToken := "refresh_token"
+
+	server := httptest.NewServer(
+		http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			if r.Method == http.MethodPost {
+				b, err := ioutil.ReadAll(r.Body)
+				if err != nil {
+					t.Fatal(err)
+				}
+				if len(b) == 0 {
+					t.Errorf("got empty request body for POST")
+				}
+				w.WriteHeader(http.StatusOK)
+				w.Write([]byte(fmt.Sprintf(`{"access_token": %q, "refresh_token": %q}`, accessToken, refreshToken)))
+				return
+			}
+
+			hdr := r.Header.Get("Authorization")
+			if hdr == "Bearer "+accessToken {
+				w.WriteHeader(http.StatusOK)
+				return
+			}
+
+			w.WriteHeader(http.StatusUnauthorized)
+		}))
+	defer server.Close()
+
+	u, err := url.Parse(server.URL)
+	if err != nil {
+		t.Fatal(err)
+	}
+	registry, err := name.NewRegistry(u.Host, name.WeakValidation)
+	if err != nil {
+		t.Errorf("Unexpected error during NewRegistry: %v", err)
+	}
+
+	bearer := &authn.Bearer{Token: initialToken}
+	transport := &bearerTransport{
+		inner:    http.DefaultTransport,
+		basic:    authn.FromConfig(authn.AuthConfig{Username: "user", Password: "pass"}),
+		bearer:   bearer,
+		registry: registry,
+		realm:    server.URL,
+		scheme:   "http",
+		scopes:   []string{"myscope"},
+		service:  u.Host,
+	}
+	client := http.Client{Transport: transport}
+
+	res, err := client.Get(fmt.Sprintf("http://%s/v2/foo/bar/blobs/blah", u.Host))
+	if err != nil {
+		t.Fatalf("Unexpected error during client.Get: %v", err)
+	}
+	if res.StatusCode != http.StatusOK {
+		t.Errorf("client.Get final StatusCode got %v, want: %v", res.StatusCode, http.StatusOK)
+	}
+	if transport.bearer.Token != accessToken {
+		t.Errorf("Expected Bearer token to be refreshed, got %v, want %v", bearer.Token, accessToken)
+	}
+}
+
 type recorder struct {
 	reqs []*http.Request
 	resp *http.Response