Skip to content

Commit

Permalink
Error on namespaced registries for credential helpers
Browse files Browse the repository at this point in the history 
We now error on login/logout if path based registries are used for other
credential helpers than the `AuthenticationFileHelper`. For that we
introduce a new error type API users can rely on.

The functions `SetCredentials` (for login) as well as
`RemoveAuthentication` (for logout) already feature support for path
based registries for the `AuthenticationFileHelper`. This patch adds
unit tests to ensure that the support will not break in the future.

Signed-off-by: Sascha Grunert <[email protected]>
  • Loading branch information
@saschagrunert
saschagrunert committed Jul 1, 2021
1 parent bca1df8 commit a12697b
Show file tree
Hide file tree
Showing 2 changed files with 216 additions and 14 deletions.
62 changes: 48 additions & 14 deletions pkg/docker/config/config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,17 @@ var (
ErrNotLoggedIn = errors.New("not logged in")
// ErrNotSupported is returned for unsupported methods
ErrNotSupported = errors.New("not supported")
// ErrNamespacedRegistry is returned if namespaced registries are not supported
// for the credential helper. Only the
// sysregistriesv2.AuthenticationFileHelper supports namespaced registries.
ErrNamespacedRegistry = errors.New("namespaced registry not supported for credential helper")
)

// SetCredentials stores the username and password in the credential helper or file
// and returns path to file or helper name in format (helper:%s).
// An ErrNamespacedRegistry will be part of the returned multierror if the
// provided registry is namespaced and a credentials helper other than the
// AuthenticationFileHelper is used.
// Returns a human-redable description of the location that was updated.
// NOTE: The return value is only intended to be read by humans; its form is not an API,
// it may change (or new forms can be added) any time.
Expand Down Expand Up @@ -83,7 +90,12 @@ func SetCredentials(sys *types.SystemContext, registry, username, password strin
// External helpers.
default:
desc = fmt.Sprintf("credential helper: %s", helper)
err = setAuthToCredHelper(helper, registry, username, password)

if isNamespaced(registry) {
err = ErrNamespacedRegistry
} else {
err = setAuthToCredHelper(helper, registry, username, password)
}
}
if err != nil {
multiErr = multierror.Append(multiErr, err)
Expand Down Expand Up @@ -328,6 +340,9 @@ func getAuthenticationWithHomeDir(sys *types.SystemContext, registry, homeDir st

// RemoveAuthentication removes credentials for `registry` from all possible
// sources such as credential helpers and auth files.
// An ErrNamespacedRegistry will be part of the returned multierror if the
// provided registry is namespaced and a credentials helper other than the
// AuthenticationFileHelper is used.
func RemoveAuthentication(sys *types.SystemContext, registry string) error {
helpers, err := sysregistriesv2.CredentialHelpers(sys)
if err != nil {
Expand All @@ -338,15 +353,19 @@ func RemoveAuthentication(sys *types.SystemContext, registry string) error {
isLoggedIn := false

removeFromCredHelper := func(helper string) {
err := deleteAuthFromCredHelper(helper, registry)
if err == nil {
logrus.Debugf("Credentials for %q were deleted from credential helper %s", registry, helper)
isLoggedIn = true
return
}
if credentials.IsErrCredentialsNotFoundMessage(err.Error()) {
logrus.Debugf("Not logged in to %s with credential helper %s", registry, helper)
return
if isNamespaced(registry) {
err = ErrNamespacedRegistry
} else {
err := deleteAuthFromCredHelper(helper, registry)
if err == nil {
logrus.Debugf("Credentials for %q were deleted from credential helper %s", registry, helper)
isLoggedIn = true
return
}
if credentials.IsErrCredentialsNotFoundMessage(err.Error()) {
logrus.Debugf("Not logged in to %s with credential helper %s", registry, helper)
return
}
}
multiErr = multierror.Append(multiErr, errors.Wrapf(err, "error removing credentials for %s from credential helper %s", registry, helper))
}
Expand Down Expand Up @@ -699,16 +718,20 @@ func decodeDockerAuth(conf dockerAuthConfig) (types.DockerAuthConfig, error) {
// to just an hostname.
// Copied from github.com/docker/docker/registry/auth.go
func convertToHostname(url string) string {
stripped := stripScheme(url)
nameParts := strings.SplitN(stripped, "/", 2)
return nameParts[0]
}

// stripScheme striped the http|https scheme from the provided URL.
func stripScheme(url string) string {
stripped := url
if strings.HasPrefix(url, "http://") {
stripped = strings.TrimPrefix(url, "http://")
} else if strings.HasPrefix(url, "https://") {
stripped = strings.TrimPrefix(url, "https://")
}

nameParts := strings.SplitN(stripped, "/", 2)

return nameParts[0]
return stripped
}

func normalizeRegistry(registry string) string {
Expand All @@ -719,3 +742,14 @@ func normalizeRegistry(registry string) string {
}
return normalized
}

// isNamespaced returns true if the provided registry contains one or more
// subpaths.
func isNamespaced(registry string) bool {
stripped := stripScheme(registry)

// don't take trailling slashes into account
stripped = strings.TrimSuffix(stripped, "/")

return len(strings.Split(stripped, "/")) > 1
}
168 changes: 168 additions & 0 deletions pkg/docker/config/config_test.go
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package config

import (
"encoding/json"
"fmt"
"io/ioutil"
"os"
Expand Down Expand Up @@ -675,3 +676,170 @@ func TestAuthKeysForRef(t *testing.T) {
require.Equal(t, tc.expected, result, tc.name)
}
}

func TestSetCredentials(t *testing.T) {
for _, tc := range []struct {
registries []string
assert func(dockerConfigFile)
}{
{
registries: []string{"quay.io"},
assert: func(auth dockerConfigFile) {
require.Len(t, auth.AuthConfigs, 1)
require.NotEmpty(t, auth.AuthConfigs["quay.io"].Auth)
},
},
{
registries: []string{"quay.io/a/b/c/d/image"},
assert: func(auth dockerConfigFile) {
require.Len(t, auth.AuthConfigs, 1)
require.NotEmpty(t, auth.AuthConfigs["quay.io/a/b/c/d/image"].Auth)
},
},
{
registries: []string{
"quay.io/a/b/c",
"quay.io/a/b",
"quay.io/a",
"quay.io",
"my-registry.local",
"my-registry.local",
},
assert: func(auth dockerConfigFile) {
require.Len(t, auth.AuthConfigs, 5)
require.NotEmpty(t, auth.AuthConfigs["quay.io/a/b/c"].Auth)
require.NotEmpty(t, auth.AuthConfigs["quay.io/a/b"].Auth)
require.NotEmpty(t, auth.AuthConfigs["quay.io/a"].Auth)
require.NotEmpty(t, auth.AuthConfigs["quay.io"].Auth)
require.NotEmpty(t, auth.AuthConfigs["my-registry.local"].Auth)
},
},
} {
tmpFile, err := ioutil.TempFile("", "auth.json.set")
require.NoError(t, err)
_, err = tmpFile.WriteString("{}")
require.NoError(t, err)
defer os.RemoveAll(tmpFile.Name())
sys := &types.SystemContext{AuthFilePath: tmpFile.Name()}

for _, registry := range tc.registries {
_, err := SetCredentials(sys, registry, "username", "password")
require.NoError(t, err)
}

auth, err := readJSONFile(tmpFile.Name(), false)
require.NoError(t, err)

tc.assert(auth)
}
}

func TestRemoveAuthentication(t *testing.T) {
testAuth := dockerAuthConfig{Auth: "ZXhhbXBsZTpvcmc="}
for _, tc := range []struct {
config dockerConfigFile
registries []string
assert func(dockerConfigFile)
}{
{
config: dockerConfigFile{
AuthConfigs: map[string]dockerAuthConfig{
"quay.io": testAuth,
},
},
registries: []string{"quay.io"},
assert: func(auth dockerConfigFile) {
require.Len(t, auth.AuthConfigs, 0)
},
},
{
config: dockerConfigFile{
AuthConfigs: map[string]dockerAuthConfig{
"quay.io": testAuth,
"my-registry.local": testAuth,
},
},
registries: []string{"my-registry.local"},
assert: func(auth dockerConfigFile) {
require.Len(t, auth.AuthConfigs, 1)
require.NotEmpty(t, auth.AuthConfigs["quay.io"].Auth)
},
},
{
config: dockerConfigFile{
AuthConfigs: map[string]dockerAuthConfig{
"quay.io/a/b/c": testAuth,
"quay.io/a/b": testAuth,
"quay.io/a": testAuth,
"quay.io": testAuth,
"my-registry.local": testAuth,
},
},
registries: []string{
"quay.io/a/b",
"quay.io",
"my-registry.local",
},
assert: func(auth dockerConfigFile) {
require.Len(t, auth.AuthConfigs, 2)
require.NotEmpty(t, auth.AuthConfigs["quay.io/a/b/c"].Auth)
require.NotEmpty(t, auth.AuthConfigs["quay.io/a"].Auth)
},
},
} {

content, err := json.Marshal(&tc.config)
require.NoError(t, err)

tmpFile, err := ioutil.TempFile("", "auth.json")
require.NoError(t, err)
defer os.RemoveAll(tmpFile.Name())

_, err = tmpFile.Write(content)
require.NoError(t, err)

sys := &types.SystemContext{AuthFilePath: tmpFile.Name()}

for _, registry := range tc.registries {
err := RemoveAuthentication(sys, registry)
require.NoError(t, err)
}

auth, err := readJSONFile(tmpFile.Name(), false)
require.NoError(t, err)

tc.assert(auth)
}
}

func TestIsNamespaced(t *testing.T) {
for _, tc := range []struct {
registry string
expected bool
}{
{
registry: "my-registry.local",
expected: false,
},
{
registry: "https://my-registry.local",
expected: false,
},
{
registry: "my-registry.local/path",
expected: true,
},
{
registry: "quay.io/a/b/c/d",
expected: true,
},
{
registry: "quay.io/",
expected: false,
},
} {

res := isNamespaced(tc.registry)
require.Equal(t, tc.expected, res)
}
}

0 comments on commit a12697b

Please sign in to comment.