Skip to content

Commit 51c9ffe

Browse files
dmcgowandmcgowan
committed
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 <[email protected]>
1 parent 15bf23d commit 51c9ffe

2 files changed

Lines changed: 103 additions & 9 deletions

File tree

‎remotes/docker/authorizer.go‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -186,15 +186,15 @@ func (a *dockerAuthorizer) AddResponses(ctx context.Context, responses []*http.R
186186
return err
187187
}
188188

189-
if username != "" && secret != "" {
190-
common := auth.TokenOptions{
191-
Username: username,
192-
Secret: secret,
193-
}
194-
195-
a.handlers[host] = newAuthHandler(a.client, a.header, c.Scheme, common)
196-
return nil
189+
if username == "" || secret == "" {
190+
return fmt.Errorf("%w: no basic auth credentials", ErrInvalidAuthorization)
197191
}
192+
193+
a.handlers[host] = newAuthHandler(a.client, a.header, c.Scheme, auth.TokenOptions{
194+
Username: username,
195+
Secret: secret,
196+
})
197+
return nil
198198
}
199199
}
200200
return fmt.Errorf("failed to find supported auth scheme: %w", errdefs.ErrNotImplemented)

‎remotes/docker/resolver_test.go‎

Lines changed: 95 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ import (
3535
"github.com/containerd/containerd/errdefs"
3636
"github.com/containerd/containerd/remotes"
3737
"github.com/containerd/containerd/remotes/docker/auth"
38+
remoteerrors "github.com/containerd/containerd/remotes/errors"
3839
digest "github.com/opencontainers/go-digest"
3940
specs "github.com/opencontainers/image-spec/specs-go"
4041
ocispec "github.com/opencontainers/image-spec/specs-go/v1"
@@ -215,6 +216,14 @@ func TestPostBasicAuthTokenResolver(t *testing.T) {
215216
runBasicTest(t, "testname", withTokenServer(th, creds))
216217
}
217218

219+
func TestBasicAuthResolver(t *testing.T) {
220+
creds := func(string) (string, string, error) {
221+
return "totallyvaliduser", "totallyvalidpassword", nil
222+
}
223+
224+
runBasicTest(t, "testname", withBasicAuthServer(creds))
225+
}
226+
218227
func TestBadTokenResolver(t *testing.T) {
219228
th := http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
220229
if r.Method != http.MethodPost {
@@ -236,7 +245,7 @@ func TestBadTokenResolver(t *testing.T) {
236245
defer close()
237246

238247
resolver := NewResolver(ro)
239-
image := fmt.Sprintf("%s/doesntmatter:sometatg", base)
248+
image := fmt.Sprintf("%s/doesntmatter:sometag", base)
240249

241250
_, _, err := resolver.Resolve(ctx, image)
242251
if err == nil {
@@ -247,6 +256,59 @@ func TestBadTokenResolver(t *testing.T) {
247256
}
248257
}
249258

259+
func TestMissingBasicAuthResolver(t *testing.T) {
260+
creds := func(string) (string, string, error) {
261+
return "", "", nil
262+
}
263+
264+
ctx := context.Background()
265+
h := newContent(ocispec.MediaTypeImageManifest, []byte("not anything parse-able"))
266+
267+
base, ro, close := withBasicAuthServer(creds)(logHandler{t, h})
268+
defer close()
269+
270+
resolver := NewResolver(ro)
271+
image := fmt.Sprintf("%s/doesntmatter:sometag", base)
272+
273+
_, _, err := resolver.Resolve(ctx, image)
274+
if err == nil {
275+
t.Fatal("Expected error getting token with inssufficient scope")
276+
}
277+
if !errors.Is(err, ErrInvalidAuthorization) {
278+
t.Fatal(err)
279+
}
280+
if !strings.Contains(err.Error(), "no basic auth credentials") {
281+
t.Fatalf("expected \"no basic auth credentials\" message, got %s", err.Error())
282+
}
283+
}
284+
285+
func TestWrongBasicAuthResolver(t *testing.T) {
286+
creds := func(string) (string, string, error) {
287+
return "totallyvaliduser", "definitelythewrongpassword", nil
288+
}
289+
290+
ctx := context.Background()
291+
h := newContent(ocispec.MediaTypeImageManifest, []byte("not anything parse-able"))
292+
293+
base, ro, close := withBasicAuthServer(creds)(logHandler{t, h})
294+
defer close()
295+
296+
resolver := NewResolver(ro)
297+
image := fmt.Sprintf("%s/doesntmatter:sometag", base)
298+
299+
_, _, err := resolver.Resolve(ctx, image)
300+
if err == nil {
301+
t.Fatal("Expected error getting token with inssufficient scope")
302+
}
303+
var rerr remoteerrors.ErrUnexpectedStatus
304+
if !errors.As(err, &rerr) {
305+
t.Fatal(err)
306+
}
307+
if rerr.StatusCode != 403 {
308+
t.Fatalf("expected 403 status code, got %d", rerr.StatusCode)
309+
}
310+
}
311+
250312
func TestHostFailureFallbackResolver(t *testing.T) {
251313
sf := func(h http.Handler) (string, ResolverOptions, func()) {
252314
s := httptest.NewServer(h)
@@ -549,6 +611,37 @@ func withTokenServer(th http.Handler, creds func(string) (string, string, error)
549611
}
550612
}
551613

614+
func withBasicAuthServer(creds func(string) (string, string, error)) func(h http.Handler) (string, ResolverOptions, func()) {
615+
return func(h http.Handler) (string, ResolverOptions, func()) {
616+
// Wrap with basic auth
617+
wrapped := http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
618+
user, password, ok := r.BasicAuth()
619+
if ok {
620+
if user != "totallyvaliduser" || password != "totallyvalidpassword" {
621+
rw.WriteHeader(http.StatusForbidden)
622+
rw.Write([]byte(`{"errors":[{"code":"DENIED"}]}`))
623+
return
624+
}
625+
} else {
626+
authHeader := "Basic realm=\"testserver\""
627+
rw.Header().Set("WWW-Authenticate", authHeader)
628+
rw.WriteHeader(http.StatusUnauthorized)
629+
return
630+
}
631+
h.ServeHTTP(rw, r)
632+
})
633+
634+
base, options, close := tlsServer(wrapped)
635+
options.Hosts = ConfigureDefaultRegistries(
636+
WithClient(options.Client),
637+
WithAuthorizer(NewDockerAuthorizer(
638+
WithAuthCreds(creds),
639+
)),
640+
)
641+
return base, options, close
642+
}
643+
}
644+
552645
func tlsServer(h http.Handler) (string, ResolverOptions, func()) {
553646
s := httptest.NewUnstartedServer(h)
554647
s.StartTLS()
@@ -564,6 +657,7 @@ func tlsServer(h http.Handler) (string, ResolverOptions, func()) {
564657
},
565658
},
566659
}
660+
567661
options := ResolverOptions{
568662
Hosts: ConfigureDefaultRegistries(WithClient(client)),
569663
// Set deprecated field for tests to use for configuration

0 commit comments

Comments
 (0)