Skip to content

Commit e529741

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]> (cherry picked from commit 51c9ffe) Signed-off-by: Derek McGowan <[email protected]>
1 parent ca45b92 commit e529741

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
@@ -194,15 +194,15 @@ func (a *dockerAuthorizer) AddResponses(ctx context.Context, responses []*http.R
194194
return err
195195
}
196196

197-
if username != "" && secret != "" {
198-
common := auth.TokenOptions{
199-
Username: username,
200-
Secret: secret,
201-
}
202-
203-
a.handlers[host] = newAuthHandler(a.client, a.header, c.Scheme, common)
204-
return nil
197+
if username == "" || secret == "" {
198+
return fmt.Errorf("%w: no basic auth credentials", ErrInvalidAuthorization)
205199
}
200+
201+
a.handlers[host] = newAuthHandler(a.client, a.header, c.Scheme, auth.TokenOptions{
202+
Username: username,
203+
Secret: secret,
204+
})
205+
return nil
206206
}
207207
}
208208
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
@@ -34,6 +34,7 @@ import (
3434

3535
"github.com/containerd/containerd/remotes"
3636
"github.com/containerd/containerd/remotes/docker/auth"
37+
remoteerrors "github.com/containerd/containerd/remotes/errors"
3738
digest "github.com/opencontainers/go-digest"
3839
specs "github.com/opencontainers/image-spec/specs-go"
3940
ocispec "github.com/opencontainers/image-spec/specs-go/v1"
@@ -210,6 +211,14 @@ func TestPostBasicAuthTokenResolver(t *testing.T) {
210211
runBasicTest(t, "testname", withTokenServer(th, creds))
211212
}
212213

214+
func TestBasicAuthResolver(t *testing.T) {
215+
creds := func(string) (string, string, error) {
216+
return "totallyvaliduser", "totallyvalidpassword", nil
217+
}
218+
219+
runBasicTest(t, "testname", withBasicAuthServer(creds))
220+
}
221+
213222
func TestBadTokenResolver(t *testing.T) {
214223
th := http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
215224
if r.Method != http.MethodPost {
@@ -231,7 +240,7 @@ func TestBadTokenResolver(t *testing.T) {
231240
defer close()
232241

233242
resolver := NewResolver(ro)
234-
image := fmt.Sprintf("%s/doesntmatter:sometatg", base)
243+
image := fmt.Sprintf("%s/doesntmatter:sometag", base)
235244

236245
_, _, err := resolver.Resolve(ctx, image)
237246
if err == nil {
@@ -242,6 +251,59 @@ func TestBadTokenResolver(t *testing.T) {
242251
}
243252
}
244253

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

608+
func withBasicAuthServer(creds func(string) (string, string, error)) func(h http.Handler) (string, ResolverOptions, func()) {
609+
return func(h http.Handler) (string, ResolverOptions, func()) {
610+
// Wrap with basic auth
611+
wrapped := http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
612+
user, password, ok := r.BasicAuth()
613+
if ok {
614+
if user != "totallyvaliduser" || password != "totallyvalidpassword" {
615+
rw.WriteHeader(http.StatusForbidden)
616+
rw.Write([]byte(`{"errors":[{"code":"DENIED"}]}`))
617+
return
618+
}
619+
} else {
620+
authHeader := "Basic realm=\"testserver\""
621+
rw.Header().Set("WWW-Authenticate", authHeader)
622+
rw.WriteHeader(http.StatusUnauthorized)
623+
return
624+
}
625+
h.ServeHTTP(rw, r)
626+
})
627+
628+
base, options, close := tlsServer(wrapped)
629+
options.Hosts = ConfigureDefaultRegistries(
630+
WithClient(options.Client),
631+
WithAuthorizer(NewDockerAuthorizer(
632+
WithAuthCreds(creds),
633+
)),
634+
)
635+
return base, options, close
636+
}
637+
}
638+
546639
func tlsServer(h http.Handler) (string, ResolverOptions, func()) {
547640
s := httptest.NewUnstartedServer(h)
548641
s.StartTLS()
@@ -558,6 +651,7 @@ func tlsServer(h http.Handler) (string, ResolverOptions, func()) {
558651
},
559652
},
560653
}
654+
561655
options := ResolverOptions{
562656
Hosts: ConfigureDefaultRegistries(WithClient(client)),
563657
// Set deprecated field for tests to use for configuration

0 commit comments

Comments
 (0)