Fix login/logout for docker.io/vendor

go.mod

@@ -4,7 +4,7 @@ go 1.15
 
 require (
 	github.com/BurntSushi/toml v0.4.1
-	github.com/containers/image/v5 v5.17.0
+	github.com/containers/image/v5 v5.17.1-0.20211201214147-603ec1341d58
 	github.com/containers/ocicrypt v1.1.2
 	github.com/containers/storage v1.37.1-0.20211119174841-bf170b3ddac0
 	github.com/disiqueira/gotree/v3 v3.0.2

go.sum

@@ -235,8 +235,8 @@ github.com/containernetworking/cni v0.8.0/go.mod h1:LGwApLUm2FpoOfxTDEeq8T9ipbpZ
 github.com/containernetworking/cni v0.8.1/go.mod h1:LGwApLUm2FpoOfxTDEeq8T9ipbpZ61X79hmU3w8FmsY=
 github.com/containernetworking/plugins v0.8.6/go.mod h1:qnw5mN19D8fIwkqW7oHHYDHVlzhJpcY6TQxn/fUyDDM=
 github.com/containernetworking/plugins v0.9.1/go.mod h1:xP/idU2ldlzN6m4p5LmGiwRDjeJr6FLK6vuiUwoH7P8=
-github.com/containers/image/v5 v5.17.0 h1:KS5pro80CCsSp5qDBTMmSAWQo+xcBX19zUPExmYX2OQ=
-github.com/containers/image/v5 v5.17.0/go.mod h1:GnYVusVRFPMMTAAUkrcS8NNSpBp8oyrjOUe04AAmRr4=
+github.com/containers/image/v5 v5.17.1-0.20211201214147-603ec1341d58 h1:DI6d+6aRBC14mbfnh0eYlHeFBSZQ4adDykrS8F/Awrg=
+github.com/containers/image/v5 v5.17.1-0.20211201214147-603ec1341d58/go.mod h1:iUA6fv9NnqIhEaP3+dqo22nKMNkSWCj8d5o8Dju0j1Q=
 github.com/containers/libtrust v0.0.0-20190913040956-14b96171aa3b h1:Q8ePgVfHDplZ7U33NwHZkrVELsZP5fYj9pM5WBZB2GE=
 github.com/containers/libtrust v0.0.0-20190913040956-14b96171aa3b/go.mod h1:9rfv8iPl1ZP7aqh9YA68wnZv2NUDbXdcdPHVz0pFbPY=
 github.com/containers/ocicrypt v1.0.1/go.mod h1:MeJDzk1RJHv89LjsH0Sp5KTY3ZYkjXO/C+bKAeWFIrc=
@@ -653,7 +653,6 @@ github.com/opencontainers/go-digest v1.0.0 h1:apOUWs51W5PlhuyGyz9FCeeBIOUDA/6nW8
 github.com/opencontainers/go-digest v1.0.0/go.mod h1:0JzlMkj0TRzQZfJkVvzbP0HBR3IKzErnv2BNG4W4MAM=
 github.com/opencontainers/image-spec v1.0.0/go.mod h1:BtxoFyWECRxE4U/7sNtV5W15zMzWCbyJoFRP3s7yZA0=
 github.com/opencontainers/image-spec v1.0.1/go.mod h1:BtxoFyWECRxE4U/7sNtV5W15zMzWCbyJoFRP3s7yZA0=
-github.com/opencontainers/image-spec v1.0.2-0.20210819154149-5ad6f50d6283/go.mod h1:BtxoFyWECRxE4U/7sNtV5W15zMzWCbyJoFRP3s7yZA0=
 github.com/opencontainers/image-spec v1.0.2-0.20211123152302-43a7dee1ec31 h1:Wh4aR2I6JFwySre9m3iHJYuMnvUFE/HT6qAXozRWi/E=
 github.com/opencontainers/image-spec v1.0.2-0.20211123152302-43a7dee1ec31/go.mod h1:BtxoFyWECRxE4U/7sNtV5W15zMzWCbyJoFRP3s7yZA0=
 github.com/opencontainers/runc v0.0.0-20190115041553-12f6a991201f/go.mod h1:qT5XzbpPznkRYVz/mWwUaVBUv2rmF59PVA73FjuZG0U=

pkg/auth/auth.go

@@ -70,13 +70,10 @@ func Login(ctx context.Context, systemContext *types.SystemContext, opts *LoginO
 	systemContext = systemContextWithOptions(systemContext, opts.AuthFile, opts.CertDir)
 
 	var (
-		authConfig    types.DockerAuthConfig
 		key, registry string
-		ref           reference.Named
 		err           error
 	)
-	l := len(args)
-	switch l {
+	switch len(args) {
 	case 0:
 		if !opts.AcceptUnspecifiedRegistry {
 			return errors.New("please provide a registry to login to")
@@ -88,26 +85,18 @@ func Login(ctx context.Context, systemContext *types.SystemContext, opts *LoginO
 		logrus.Debugf("registry not specified, default to the first registry %q from registries.conf", key)
 
 	case 1:
-		key, registry, ref, err = parseRegistryArgument(args[0], opts.AcceptRepositories)
+		key, registry, err = parseCredentialsKey(args[0], opts.AcceptRepositories)
 		if err != nil {
 			return err
 		}
 
 	default:
 		return errors.New("login accepts only one registry to login to")
-
 	}
 
-	if ref != nil {
-		authConfig, err = config.GetCredentialsForRef(systemContext, ref)
-		if err != nil {
-			return errors.Wrap(err, "get credentials for repository")
-		}
-	} else {
-		authConfig, err = config.GetCredentials(systemContext, registry)
-		if err != nil {
-			return errors.Wrap(err, "get credentials")
-		}
+	authConfig, err := config.GetCredentials(systemContext, key)
+	if err != nil {
+		return errors.Wrap(err, "get credentials")
 	}
 
 	if opts.GetLoginSet {
@@ -173,39 +162,44 @@ func Login(ctx context.Context, systemContext *types.SystemContext, opts *LoginO
 	return errors.Wrapf(err, "authenticating creds for %q", key)
 }
 
-// parseRegistryArgument verifies the provided arg depending if we accept
-// repositories or not.
-func parseRegistryArgument(arg string, acceptRepositories bool) (key, registry string, maybeRef reference.Named, err error) {
+// parseCredentialsKey turns the provided argument into a valid credential key
+// and computes the registry part.
+func parseCredentialsKey(arg string, acceptRepositories bool) (key, registry string, err error) {
 	if !acceptRepositories {
 		registry = getRegistryName(arg)
 		key = registry
-		return key, registry, maybeRef, nil
+		return key, registry, nil
 	}
 
 	key = trimScheme(arg)
 	if key != arg {
-		return key, registry, nil, errors.New("credentials key has https[s]:// prefix")
+		return "", "", errors.New("credentials key has https[s]:// prefix")
 	}
 
 	registry = getRegistryName(key)
 	if registry == key {
-		// We cannot parse a reference from a registry, so we stop here
-		return key, registry, nil, nil
+		// The key is not namespaced
+		return key, registry, nil
 	}
 
-	ref, parseErr := reference.ParseNamed(key)
-	if parseErr != nil {
-		return key, registry, nil, errors.Wrapf(parseErr, "parse reference from %q", key)
+	// Sanity-check that the key looks reasonable (e.g. doesn't use invalid characters),
+	// and does not contain a tag or digest.
+	// WARNING: ref.Named() MUST NOT be used to compute key, because
+	// reference.ParseNormalizedNamed() turns docker.io/vendor to docker.io/library/vendor
+	// Ideally c/image should provide dedicated validation functionality.
+	ref, err := reference.ParseNormalizedNamed(key)
+	if err != nil {
+		return "", "", errors.Wrapf(err, "parse reference from %q", key)
 	}
-
 	if !reference.IsNameOnly(ref) {
-		return key, registry, nil, errors.Errorf("reference %q contains tag or digest", ref.String())
+		return "", "", errors.Errorf("reference %q contains tag or digest", ref.String())
+	}
+	refRegistry := reference.Domain(ref)
+	if refRegistry != registry { // This should never happen, check just to make sure
+		return "", "", fmt.Errorf("internal error: key %q registry mismatch, %q vs. %q", key, ref, refRegistry)
 	}
 
-	maybeRef = ref
-	registry = reference.Domain(ref)
-
-	return key, registry, maybeRef, nil
+	return key, registry, nil
 }
 
 // getRegistryName scrubs and parses the input to get the server name
@@ -271,15 +265,23 @@ func Logout(systemContext *types.SystemContext, opts *LogoutOptions, args []stri
 	}
 	systemContext = systemContextWithOptions(systemContext, opts.AuthFile, "")
 
+	if opts.All {
+		if len(args) != 0 {
+			return errors.New("--all takes no arguments")
+		}
+		if err := config.RemoveAllAuthentication(systemContext); err != nil {
+			return err
+		}
+		fmt.Fprintln(opts.Stdout, "Removed login credentials for all registries")
+		return nil
+	}
+
 	var (
 		key, registry string
-		ref           reference.Named
 		err           error
 	)
-	if len(args) > 1 {
-		return errors.New("logout accepts only one registry to logout from")
-	}
-	if len(args) == 0 && !opts.All {
+	switch len(args) {
+	case 0:
 		if !opts.AcceptUnspecifiedRegistry {
 			return errors.New("please provide a registry to logout from")
 		}
@@ -288,23 +290,15 @@ func Logout(systemContext *types.SystemContext, opts *LogoutOptions, args []stri
 		}
 		registry = key
 		logrus.Debugf("registry not specified, default to the first registry %q from registries.conf", key)
-	}
-	if len(args) != 0 {
-		if opts.All {
-			return errors.New("--all takes no arguments")
-		}
-		key, registry, ref, err = parseRegistryArgument(args[0], opts.AcceptRepositories)
+
+	case 1:
+		key, registry, err = parseCredentialsKey(args[0], opts.AcceptRepositories)
 		if err != nil {
 			return err
 		}
-	}
 
-	if opts.All {
-		if err := config.RemoveAllAuthentication(systemContext); err != nil {
-			return err
-		}
-		fmt.Fprintln(opts.Stdout, "Removed login credentials for all registries")
-		return nil
+	default:
+		return errors.New("logout accepts only one registry to logout from")
 	}
 
 	err = config.RemoveAuthentication(systemContext, key)
@@ -313,17 +307,9 @@ func Logout(systemContext *types.SystemContext, opts *LogoutOptions, args []stri
 		fmt.Fprintf(opts.Stdout, "Removed login credentials for %s\n", key)
 		return nil
 	case config.ErrNotLoggedIn:
-		var authConfig types.DockerAuthConfig
-		if ref != nil {
-			authConfig, err = config.GetCredentialsForRef(systemContext, ref)
-			if err != nil {
-				return errors.Wrap(err, "get credentials for repository")
-			}
-		} else {
-			authConfig, err = config.GetCredentials(systemContext, registry)
-			if err != nil {
-				return errors.Wrap(err, "get credentials")
-			}
+		authConfig, err := config.GetCredentials(systemContext, key)
+		if err != nil {
+			return errors.Wrap(err, "get credentials")
 		}
 
 		authInvalid := docker.CheckAuth(context.Background(), systemContext, authConfig.Username, authConfig.Password, registry)

pkg/auth/auth_test.go

@@ -5,10 +5,10 @@ import (
 	"os"
 	"testing"
 
-	"github.com/containers/image/v5/docker/reference"
 	. "github.com/onsi/ginkgo"
 	"github.com/onsi/gomega"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 var _ = Describe("Config", func() {
@@ -66,73 +66,76 @@ var _ = Describe("Config", func() {
 	})
 })
 
-func TestParseRegistryArgument(t *testing.T) {
+func TestParseCredentialsKey(t *testing.T) {
 	t.Parallel()
 	for _, tc := range []struct {
 		name               string
 		arg                string
 		acceptRepositories bool
-		shouldErr          bool
-		expect             func(key, registry string, ref reference.Named)
+		expectedKey        string // or "" if we expect failure
+		expectedRegistry   string
 	}{
 		{
 			name:               "success repository",
 			arg:                "quay.io/user",
 			acceptRepositories: true,
-			shouldErr:          false,
-			expect: func(key, registry string, ref reference.Named) {
-				assert.Equal(t, "quay.io/user", key)
-				assert.Equal(t, "quay.io", registry)
-				assert.NotNil(t, ref)
-			},
+			expectedKey:        "quay.io/user",
+			expectedRegistry:   "quay.io",
 		},
 		{
 			name:               "success no repository",
 			arg:                "quay.io",
 			acceptRepositories: true,
-			shouldErr:          false,
-			expect: func(key, registry string, ref reference.Named) {
-				assert.Equal(t, "quay.io", key)
-				assert.Equal(t, "quay.io", registry)
-				assert.Nil(t, ref)
-			},
+			expectedKey:        "quay.io",
+			expectedRegistry:   "quay.io",
+		},
+		{
+			name:               "a docker.io top-level namespace",
+			arg:                "docker.io/user",
+			acceptRepositories: true,
+			expectedKey:        "docker.io/user",
+			expectedRegistry:   "docker.io",
+		},
+		{
+			name:               "a single docker.io/library repo",
+			arg:                "docker.io/library/user",
+			acceptRepositories: true,
+			expectedKey:        "docker.io/library/user",
+			expectedRegistry:   "docker.io",
 		},
 		{
 			name:               "with http[s] prefix",
 			arg:                "https://quay.io",
 			acceptRepositories: true,
-			shouldErr:          true,
+			expectedKey:        "",
 		},
 		{
 			name:               "failure with tag",
 			arg:                "quay.io/username/image:tag",
 			acceptRepositories: true,
-			shouldErr:          true,
+			expectedKey:        "",
 		},
 		{
 			name:               "failure parse reference",
 			arg:                "quay.io/:tag",
 			acceptRepositories: true,
-			shouldErr:          true,
+			expectedKey:        "",
 		},
 		{
 			name:               "success accept no repository",
 			arg:                "https://quay.io/user",
 			acceptRepositories: false,
-			shouldErr:          false,
-			expect: func(key, registry string, ref reference.Named) {
-				assert.Equal(t, "quay.io", key)
-				assert.Equal(t, "quay.io", registry)
-				assert.Nil(t, ref)
-			},
+			expectedKey:        "quay.io",
+			expectedRegistry:   "quay.io",
 		},
 	} {
-		key, registry, ref, err := parseRegistryArgument(tc.arg, tc.acceptRepositories)
-		if tc.shouldErr {
+		key, registry, err := parseCredentialsKey(tc.arg, tc.acceptRepositories)
+		if tc.expectedKey == "" {
 			assert.Error(t, err, tc.name)
 		} else {
-			assert.NoError(t, err, tc.name)
-			tc.expect(key, registry, ref)
+			require.NoError(t, err, tc.name)
+			assert.Equal(t, tc.expectedKey, key, tc.name)
+			assert.Equal(t, tc.expectedRegistry, registry)
 		}
 	}
 }