WIP

Signed-off-by: Laura Brehm <[email protected]>

cli/command/cli.go

@@ -21,7 +21,9 @@ import (
 	"github.com/docker/cli/cli/context/store"
 	"github.com/docker/cli/cli/debug"
 	cliflags "github.com/docker/cli/cli/flags"
+	"github.com/docker/cli/cli/internal/oauth/manager"
 	manifeststore "github.com/docker/cli/cli/manifest/store"
+	"github.com/docker/cli/cli/oauth"
 	registryclient "github.com/docker/cli/cli/registry/client"
 	"github.com/docker/cli/cli/streams"
 	"github.com/docker/cli/cli/trust"
@@ -66,6 +68,7 @@ type Cli interface {
 	CurrentContext() string
 	DockerEndpoint() docker.Endpoint
 	TelemetryClient
+	OAuthManager() oauth.Manager
 }
 
 // DockerCli is an instance the docker command line client.
@@ -86,6 +89,7 @@ type DockerCli struct {
 	dockerEndpoint     docker.Endpoint
 	contextStoreConfig store.Config
 	initTimeout        time.Duration
+	oauthManager       oauth.Manager
 	res                telemetryResource
 
 	// baseCtx is the base context used for internal operations. In the future
@@ -96,6 +100,10 @@ type DockerCli struct {
 	enableGlobalMeter, enableGlobalTracer bool
 }
 
+func (cli *DockerCli) OAuthManager() oauth.Manager {
+	return cli.oauthManager
+}
+
 // DefaultVersion returns api.defaultVersion.
 func (cli *DockerCli) DefaultVersion() string {
 	return api.DefaultVersion
@@ -293,6 +301,8 @@ func (cli *DockerCli) Initialize(opts *cliflags.ClientOptions, ops ...CLIOption)
 		cli.createGlobalTracerProvider(cli.baseCtx)
 	}
 
+	cli.oauthManager = manager.NewManager()
+
 	return nil
 }
 

cli/command/registry/login.go

@@ -121,9 +121,39 @@ func runLogin(ctx context.Context, dockerCli command.Cli, opts loginOptions) err
 		response, err = loginWithCredStoreCreds(ctx, dockerCli, &authConfig)
 	}
 	if err != nil || authConfig.Username == "" || authConfig.Password == "" {
-		err = command.ConfigureAuth(ctx, dockerCli, opts.user, opts.password, &authConfig, isDefaultRegistry)
-		if err != nil {
-			return err
+		if isDefaultRegistry && opts.user == "" && opts.password == "" {
+			// todo(laurazard: clean this up
+			tokenRes, err := dockerCli.OAuthManager().LoginDevice(ctx, dockerCli.Err())
+			if err != nil {
+				return err
+			}
+			authConfig.Username = tokenRes.Claims.Domain.Username
+			authConfig.Password = tokenRes.AccessToken
+			authConfig.Email = tokenRes.Claims.Domain.Email
+			authConfig.ServerAddress = serverAddress
+
+			response, err = clnt.RegistryLogin(ctx, authConfig)
+			if err != nil && client.IsErrConnectionFailed(err) {
+				// If the server isn't responding (yet) attempt to login purely client side
+				response, err = loginClientSide(ctx, authConfig)
+			}
+			// If we (still) have an error, give up
+			if err != nil {
+				return err
+			}
+
+			authConfig.Password = authConfig.Password + ".." + tokenRes.RefreshToken
+
+			creds := dockerCli.ConfigFile().GetCredentialsStore(serverAddress)
+			if err := creds.Store(configtypes.AuthConfig(authConfig)); err != nil {
+				return errors.Errorf("Error saving credentials: %v", err)
+			}
+			return nil
+		} else {
+			err = command.ConfigureAuth(ctx, dockerCli, opts.user, opts.password, &authConfig, isDefaultRegistry)
+			if err != nil {
+				return err
+			}
 		}
 
 		response, err = clnt.RegistryLogin(ctx, authConfig)

cli/config/configfile/file.go

@@ -10,6 +10,7 @@ import (
 
 	"github.com/docker/cli/cli/config/credentials"
 	"github.com/docker/cli/cli/config/types"
+	"github.com/docker/cli/cli/internal/oauth/manager"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
 )
@@ -260,7 +261,7 @@ func (configFile *ConfigFile) GetCredentialsStore(registryHostname string) crede
 	} else {
 		credsStore = credentials.NewFileStore(configFile)
 	}
-	return credentials.NewOAuthStore(credsStore)
+	return credentials.NewOAuthStore(credsStore, manager.NewManager())
 }
 
 // var for unit testing.

cli/config/credentials/oauth_store.go

@@ -3,11 +3,9 @@ package credentials
 import (
 	"context"
 	"errors"
-	"os"
 	"strings"
 	"time"
 
-	"github.com/docker/cli/cli/config/credentials/internal/oauth/manager"
 	"github.com/docker/cli/cli/config/types"
 	"github.com/docker/cli/cli/oauth"
 	"github.com/docker/docker/registry"
@@ -22,11 +20,10 @@ type oauthStore struct {
 }
 
 // NewOAuthStore creates a new oauthStore backed by the provided store.
-func NewOAuthStore(backingStore Store) Store {
-	m, _ := manager.NewManager()
+func NewOAuthStore(backingStore Store, manager oauth.Manager) Store {
 	return &oauthStore{
 		backingStore: backingStore,
-		manager:      m,
+		manager:      manager,
 	}
 }
 
@@ -51,31 +48,20 @@ func (c *oauthStore) Get(serverAddress string) (types.AuthConfig, error) {
 		// store itself. This should be propagated up.
 		return types.AuthConfig{}, err
 	}
+
 	tokenRes, err := c.parseToken(auth.Password)
-	if err != nil && auth.Password != "" {
-		return types.AuthConfig{
-			Username:      auth.Username,
-			Password:      auth.Password,
-			Email:         auth.Email,
-			ServerAddress: registry.IndexServer,
-		}, nil
+	// if the password is not a token, return the auth config as is
+	if err != nil {
+		return auth, nil
 	}
 
-	var failedRefresh bool
 	// if the access token is valid for less than 50 minutes, refresh it
 	if tokenRes.RefreshToken != "" && tokenRes.Claims.Expiry.Time().Before(time.Now().Add(minimumTokenLifetime)) {
 		refreshRes, err := c.manager.RefreshToken(context.TODO(), tokenRes.RefreshToken)
-		if err != nil {
-			failedRefresh = true
-		}
-		tokenRes = refreshRes
-	}
-
-	if tokenRes.AccessToken == "" || failedRefresh {
-		tokenRes, err = c.manager.LoginDevice(context.TODO(), os.Stderr)
 		if err != nil {
 			return types.AuthConfig{}, err
 		}
+		tokenRes = refreshRes
 	}
 
 	err = c.storeInBackingStore(tokenRes)
@@ -121,19 +107,9 @@ func (c *oauthStore) Erase(serverAddress string) error {
 	return c.backingStore.Erase(serverAddress)
 }
 
-// Store stores the provided credentials in the backing credential store,
-// except when the credentials are for the official registry, in which case
-// no action is taken because the credentials retrieved/stored during Get.
+// Store stores the provided credentials in the backing store, without any
+// additional processing.
 func (c *oauthStore) Store(auth types.AuthConfig) error {
-	if auth.ServerAddress != registry.IndexServer {
-		return c.backingStore.Store(auth)
-	}
-
-	_, err := c.parseToken(auth.Password)
-	if err == nil {
-		return nil
-	}
-
 	return c.backingStore.Store(auth)
 }
 

cli/config/credentials/oauth_store_test.go

@@ -48,43 +48,18 @@ func TestOAuthStoreGet(t *testing.T) {
 			})
 		})
 
-		t.Run("no credentials - login", func(t *testing.T) {
+		t.Run("no credentials - return", func(t *testing.T) {
 			auths := map[string]types.AuthConfig{}
 			f := newStore(auths)
-			manager := &testManager{
-				loginDevice: func() (oauth.TokenResult, error) {
-					return oauth.TokenResult{
-						AccessToken:  "abcd1234",
-						RefreshToken: "efgh5678",
-						Claims: oauth.Claims{
-							Claims: jwt.Claims{
-								Expiry: jwt.NewNumericDate(time.Now().Add(1 * time.Hour)),
-							},
-							Domain: oauth.DomainClaims{Username: "bork!", Email: "[email protected]"},
-						},
-					}, nil
-				},
-			}
 			s := &oauthStore{
 				backingStore: NewFileStore(f),
-				manager:      manager,
 			}
 
 			auth, err := s.Get(registry.IndexServer)
 			assert.NilError(t, err)
 
-			assert.DeepEqual(t, auth, types.AuthConfig{
-				Username:      "bork!",
-				Password:      "abcd1234",
-				Email:         "[email protected]",
-				ServerAddress: registry.IndexServer,
-			})
-			assert.DeepEqual(t, auths[registry.IndexServer], types.AuthConfig{
-				Username:      "bork!",
-				Password:      "abcd1234..efgh5678",
-				Email:         "[email protected]",
-				ServerAddress: registry.IndexServer,
-			})
+			assert.DeepEqual(t, auth, types.AuthConfig{})
+			assert.Equal(t, len(auths), 0)
 		})
 
 		t.Run("expired credentials - refresh", func(t *testing.T) {
@@ -135,7 +110,7 @@ func TestOAuthStoreGet(t *testing.T) {
 			})
 		})
 
-		t.Run("expired credentials - refresh fails - login", func(t *testing.T) {
+		t.Run("expired credentials - refresh fails - return error", func(t *testing.T) {
 			f := newStore(map[string]types.AuthConfig{
 				registry.IndexServer: {
 					Username:      "bork!",
@@ -144,46 +119,22 @@ func TestOAuthStoreGet(t *testing.T) {
 					ServerAddress: registry.IndexServer,
 				},
 			})
-			var loginCalled bool
+			var refreshCalled bool
 			manager := &testManager{
 				refresh: func(_ string) (oauth.TokenResult, error) {
-					return oauth.TokenResult{}, errors.New("program failed")
-				},
-				loginDevice: func() (oauth.TokenResult, error) {
-					loginCalled = true
-					return oauth.TokenResult{
-						AccessToken:  "abcd1234",
-						RefreshToken: "efgh5678",
-						Claims: oauth.Claims{
-							Claims: jwt.Claims{
-								Expiry: jwt.NewNumericDate(time.Now().Add(1 * time.Hour)),
-							},
-							Domain: oauth.DomainClaims{Username: "bork!", Email: "[email protected]"},
-						},
-					}, nil
+					refreshCalled = true
+					return oauth.TokenResult{}, errors.New("refresh failed")
 				},
 			}
 			s := &oauthStore{
 				backingStore: NewFileStore(f),
 				manager:      manager,
 			}
 
-			auth, err := s.Get(registry.IndexServer)
-			assert.NilError(t, err)
+			_, err := s.Get(registry.IndexServer)
+			assert.ErrorContains(t, err, "refresh failed")
 
-			assert.Check(t, loginCalled)
-			assert.DeepEqual(t, auth, types.AuthConfig{
-				Username:      "bork!",
-				Password:      "abcd1234",
-				Email:         "[email protected]",
-				ServerAddress: registry.IndexServer,
-			})
-			assert.DeepEqual(t, f.GetAuthConfigs()[registry.IndexServer], types.AuthConfig{
-				Username:      "bork!",
-				Password:      "abcd1234..efgh5678",
-				Email:         "[email protected]",
-				ServerAddress: registry.IndexServer,
-			})
+			assert.Check(t, refreshCalled)
 		})
 
 		t.Run("old non-access token credentials", func(t *testing.T) {