From 99946ad2afef3a1237be8e5337544464e1655384 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Mon, 10 Mar 2025 21:48:03 +0100 Subject: [PATCH 1/2] WIP: don't require IndexInfo Lots to do here; too many wrappers everywhere, which may become easier when content trust is removed (which adds another level of abstraction) Signed-off-by: Sebastiaan van Stijn --- cli/command/registry.go | 47 ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 46 insertions(+), 1 deletion(-) diff --git a/cli/command/registry.go b/cli/command/registry.go index 71d0b680d3ac..745dbe85e4fd 100644 --- a/cli/command/registry.go +++ b/cli/command/registry.go @@ -35,6 +35,44 @@ const ( // [registry.IndexServer]: https://pkg.go.dev/github.com/docker/docker@v28.3.3+incompatible/registry#IndexServer const authConfigKey = "https://index.docker.io/v1/" +// NewAuthRequester returns a RequestPrivilegeFunc for the specified registry +// and the given cmdName (used as informational message to the user). +// +// The returned function is a [registrytypes.RequestAuthConfig] to prompt the user +// for credentials if needed. It is called as fallback if the credentials (if any) +// used for the initial operation did not work. +// +// TODO(thaJeztah): cli Cli could be a Streams if it was not for cli.SetIn to be needed? +// TODO(thaJeztah): ideally, this would accept reposName / imageRef as a regular string (we can parse it if needed!), or .. maybe generics and accept either? +func NewAuthRequester(cli Cli, indexServer string, promptMsg string) registrytypes.RequestAuthConfig { + configKey := getAuthConfigKey(indexServer) + return newPrivilegeFunc(cli, configKey, promptMsg) +} + +func newPrivilegeFunc(cli Cli, indexServer string, promptMsg string) registrytypes.RequestAuthConfig { + return func(ctx context.Context) (string, error) { + // TODO(thaJeztah): can we make the prompt an argument? ("prompt func()" or "prompt func()? + _, _ = fmt.Fprint(cli.Out(), "\n"+promptMsg+"\n") + isDefaultRegistry := indexServer == authConfigKey + authConfig, err := GetDefaultAuthConfig(cli.ConfigFile(), true, indexServer, isDefaultRegistry) + if err != nil { + _, _ = fmt.Fprintf(cli.Err(), "Unable to retrieve stored credentials for %s, error: %s.\n", indexServer, err) + } + + select { + case <-ctx.Done(): + return "", ctx.Err() + default: + } + + authConfig, err = PromptUserForCredentials(ctx, cli, "", "", authConfig.Username, indexServer) + if err != nil { + return "", err + } + return registrytypes.EncodeAuthConfig(authConfig) + } +} + // ResolveAuthConfig returns auth-config for the given registry from the // credential-store. It returns an empty AuthConfig if no credentials were // found. @@ -44,7 +82,8 @@ const authConfigKey = "https://index.docker.io/v1/" // // [registry.ResolveAuthConfig]: https://pkg.go.dev/github.com/docker/docker@v28.3.3+incompatible/registry#ResolveAuthConfig func ResolveAuthConfig(cfg *configfile.ConfigFile, index *registrytypes.IndexInfo) registrytypes.AuthConfig { - configKey := index.Name + indexServer := index.Name + configKey := getAuthConfigKey(indexServer) if index.Official { configKey = authConfigKey } @@ -66,6 +105,7 @@ func ResolveAuthConfig(cfg *configfile.ConfigFile, index *registrytypes.IndexInf // If credentials for given serverAddress exists in the credential store, the configuration will be populated with values in it func GetDefaultAuthConfig(cfg *configfile.ConfigFile, checkCredStore bool, serverAddress string, isDefaultRegistry bool) (registrytypes.AuthConfig, error) { if !isDefaultRegistry { + // FIXME(thaJeztah): should the same logic be used for getAuthConfigKey ?? Looks like we're normalizing things here, but not elsewhere? why? serverAddress = credentials.ConvertToHostname(serverAddress) } authCfg := configtypes.AuthConfig{} @@ -100,6 +140,8 @@ func GetDefaultAuthConfig(cfg *configfile.ConfigFile, checkCredStore bool, serve // If defaultUsername is not empty, the username prompt includes that username // and the user can hit enter without inputting a username to use that default // username. +// +// TODO(thaJeztah): cli Cli could be a Streams if it was not for cli.SetIn to be needed? func PromptUserForCredentials(ctx context.Context, cli Cli, argUser, argPassword, defaultUsername, serverAddress string) (registrytypes.AuthConfig, error) { // On Windows, force the use of the regular OS stdin stream. // @@ -191,6 +233,9 @@ func PromptUserForCredentials(ctx context.Context, cli Cli, argUser, argPassword // base64url encoded ([RFC 4648, Section 5]) JSON string for sending through // the "X-Registry-Auth" header. // +// FIXME(thaJeztah): do we need a variant like this, but with "indexServer" (domainName) as input? +// TODO(thaJeztah): should this accept an image ref-type, and use instead of ResolveAuthConfig +// // [RFC 4648, Section 5]: https://tools.ietf.org/html/rfc4648#section-5 func RetrieveAuthTokenFromImage(cfg *configfile.ConfigFile, image string) (string, error) { registryRef, err := reference.ParseNormalizedNamed(image) From c2671f23408d6d88ab3c64adc40da842e3586148 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Tue, 22 Jul 2025 22:36:29 +0200 Subject: [PATCH 2/2] cli/command: remove NewAuthRequester as its now unused Signed-off-by: Sebastiaan van Stijn --- cli/command/registry.go | 38 -------------------------------------- 1 file changed, 38 deletions(-) diff --git a/cli/command/registry.go b/cli/command/registry.go index 745dbe85e4fd..f94e0e86d4fb 100644 --- a/cli/command/registry.go +++ b/cli/command/registry.go @@ -35,44 +35,6 @@ const ( // [registry.IndexServer]: https://pkg.go.dev/github.com/docker/docker@v28.3.3+incompatible/registry#IndexServer const authConfigKey = "https://index.docker.io/v1/" -// NewAuthRequester returns a RequestPrivilegeFunc for the specified registry -// and the given cmdName (used as informational message to the user). -// -// The returned function is a [registrytypes.RequestAuthConfig] to prompt the user -// for credentials if needed. It is called as fallback if the credentials (if any) -// used for the initial operation did not work. -// -// TODO(thaJeztah): cli Cli could be a Streams if it was not for cli.SetIn to be needed? -// TODO(thaJeztah): ideally, this would accept reposName / imageRef as a regular string (we can parse it if needed!), or .. maybe generics and accept either? -func NewAuthRequester(cli Cli, indexServer string, promptMsg string) registrytypes.RequestAuthConfig { - configKey := getAuthConfigKey(indexServer) - return newPrivilegeFunc(cli, configKey, promptMsg) -} - -func newPrivilegeFunc(cli Cli, indexServer string, promptMsg string) registrytypes.RequestAuthConfig { - return func(ctx context.Context) (string, error) { - // TODO(thaJeztah): can we make the prompt an argument? ("prompt func()" or "prompt func()? - _, _ = fmt.Fprint(cli.Out(), "\n"+promptMsg+"\n") - isDefaultRegistry := indexServer == authConfigKey - authConfig, err := GetDefaultAuthConfig(cli.ConfigFile(), true, indexServer, isDefaultRegistry) - if err != nil { - _, _ = fmt.Fprintf(cli.Err(), "Unable to retrieve stored credentials for %s, error: %s.\n", indexServer, err) - } - - select { - case <-ctx.Done(): - return "", ctx.Err() - default: - } - - authConfig, err = PromptUserForCredentials(ctx, cli, "", "", authConfig.Username, indexServer) - if err != nil { - return "", err - } - return registrytypes.EncodeAuthConfig(authConfig) - } -} - // ResolveAuthConfig returns auth-config for the given registry from the // credential-store. It returns an empty AuthConfig if no credentials were // found.