remotes: add handling for missing basic auth credentials

When a credential handler is provided but no basic auth credentials
are provided, handle the error specifically rather than treating
the credentials as not implemented. This allows a clearer error to
be provided to users rather than a confusing not implemented error
or generic unauthorized error.

Add unit tests for the basic auth case.

Signed-off-by: Derek McGowan <derek@mcg.dev>

remotes/docker/authorizer.go

@@ -186,15 +186,15 @@ func (a *dockerAuthorizer) AddResponses(ctx context.Context, responses []*http.R
 				return err
 			}
 
-			if username != "" && secret != "" {
-				common := auth.TokenOptions{
-					Username: username,
-					Secret:   secret,
-				}
-
-				a.handlers[host] = newAuthHandler(a.client, a.header, c.Scheme, common)
-				return nil
+			if username == "" || secret == "" {
+				return fmt.Errorf("%w: no basic auth credentials", ErrInvalidAuthorization)
 			}
+
+			a.handlers[host] = newAuthHandler(a.client, a.header, c.Scheme, auth.TokenOptions{
+				Username: username,
+				Secret:   secret,
+			})
+			return nil
 		}
 	}
 	return fmt.Errorf("failed to find supported auth scheme: %w", errdefs.ErrNotImplemented)

remotes/docker/resolver_test.go

@@ -35,6 +35,7 @@ import (
 	"github.com/containerd/containerd/errdefs"
 	"github.com/containerd/containerd/remotes"
 	"github.com/containerd/containerd/remotes/docker/auth"
+	remoteerrors "github.com/containerd/containerd/remotes/errors"
 	digest "github.com/opencontainers/go-digest"
 	specs "github.com/opencontainers/image-spec/specs-go"
 	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
@@ -215,6 +216,14 @@ func TestPostBasicAuthTokenResolver(t *testing.T) {
 	runBasicTest(t, "testname", withTokenServer(th, creds))
 }
 
+func TestBasicAuthResolver(t *testing.T) {
+	creds := func(string) (string, string, error) {
+		return "totallyvaliduser", "totallyvalidpassword", nil
+	}
+
+	runBasicTest(t, "testname", withBasicAuthServer(creds))
+}
+
 func TestBadTokenResolver(t *testing.T) {
 	th := http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
 		if r.Method != http.MethodPost {
@@ -236,7 +245,7 @@ func TestBadTokenResolver(t *testing.T) {
 	defer close()
 
 	resolver := NewResolver(ro)
-	image := fmt.Sprintf("%s/doesntmatter:sometatg", base)
+	image := fmt.Sprintf("%s/doesntmatter:sometag", base)
 
 	_, _, err := resolver.Resolve(ctx, image)
 	if err == nil {
@@ -247,6 +256,59 @@ func TestBadTokenResolver(t *testing.T) {
 	}
 }
 
+func TestMissingBasicAuthResolver(t *testing.T) {
+	creds := func(string) (string, string, error) {
+		return "", "", nil
+	}
+
+	ctx := context.Background()
+	h := newContent(ocispec.MediaTypeImageManifest, []byte("not anything parse-able"))
+
+	base, ro, close := withBasicAuthServer(creds)(logHandler{t, h})
+	defer close()
+
+	resolver := NewResolver(ro)
+	image := fmt.Sprintf("%s/doesntmatter:sometag", base)
+
+	_, _, err := resolver.Resolve(ctx, image)
+	if err == nil {
+		t.Fatal("Expected error getting token with inssufficient scope")
+	}
+	if !errors.Is(err, ErrInvalidAuthorization) {
+		t.Fatal(err)
+	}
+	if !strings.Contains(err.Error(), "no basic auth credentials") {
+		t.Fatalf("expected \"no basic auth credentials\" message, got %s", err.Error())
+	}
+}
+
+func TestWrongBasicAuthResolver(t *testing.T) {
+	creds := func(string) (string, string, error) {
+		return "totallyvaliduser", "definitelythewrongpassword", nil
+	}
+
+	ctx := context.Background()
+	h := newContent(ocispec.MediaTypeImageManifest, []byte("not anything parse-able"))
+
+	base, ro, close := withBasicAuthServer(creds)(logHandler{t, h})
+	defer close()
+
+	resolver := NewResolver(ro)
+	image := fmt.Sprintf("%s/doesntmatter:sometag", base)
+
+	_, _, err := resolver.Resolve(ctx, image)
+	if err == nil {
+		t.Fatal("Expected error getting token with inssufficient scope")
+	}
+	var rerr remoteerrors.ErrUnexpectedStatus
+	if !errors.As(err, &rerr) {
+		t.Fatal(err)
+	}
+	if rerr.StatusCode != 403 {
+		t.Fatalf("expected 403 status code, got %d", rerr.StatusCode)
+	}
+}
+
 func TestHostFailureFallbackResolver(t *testing.T) {
 	sf := func(h http.Handler) (string, ResolverOptions, func()) {
 		s := httptest.NewServer(h)
@@ -549,6 +611,37 @@ func withTokenServer(th http.Handler, creds func(string) (string, string, error)
 	}
 }
 
+func withBasicAuthServer(creds func(string) (string, string, error)) func(h http.Handler) (string, ResolverOptions, func()) {
+	return func(h http.Handler) (string, ResolverOptions, func()) {
+		// Wrap with basic auth
+		wrapped := http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
+			user, password, ok := r.BasicAuth()
+			if ok {
+				if user != "totallyvaliduser" || password != "totallyvalidpassword" {
+					rw.WriteHeader(http.StatusForbidden)
+					rw.Write([]byte(`{"errors":[{"code":"DENIED"}]}`))
+					return
+				}
+			} else {
+				authHeader := "Basic realm=\"testserver\""
+				rw.Header().Set("WWW-Authenticate", authHeader)
+				rw.WriteHeader(http.StatusUnauthorized)
+				return
+			}
+			h.ServeHTTP(rw, r)
+		})
+
+		base, options, close := tlsServer(wrapped)
+		options.Hosts = ConfigureDefaultRegistries(
+			WithClient(options.Client),
+			WithAuthorizer(NewDockerAuthorizer(
+				WithAuthCreds(creds),
+			)),
+		)
+		return base, options, close
+	}
+}
+
 func tlsServer(h http.Handler) (string, ResolverOptions, func()) {
 	s := httptest.NewUnstartedServer(h)
 	s.StartTLS()
@@ -564,6 +657,7 @@ func tlsServer(h http.Handler) (string, ResolverOptions, func()) {
 			},
 		},
 	}
+
 	options := ResolverOptions{
 		Hosts: ConfigureDefaultRegistries(WithClient(client)),
 		// Set deprecated field for tests to use for configuration