Skip to content

Commit 6cd2cc4

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 5e21abb commit 6cd2cc4

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
@@ -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"
@@ -214,6 +215,14 @@ func TestPostBasicAuthTokenResolver(t *testing.T) {
214215
runBasicTest(t, "testname", withTokenServer(th, creds))
215216
}
216217

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

237246
resolver := NewResolver(ro)
238-
image := fmt.Sprintf("%s/doesntmatter:sometatg", base)
247+
image := fmt.Sprintf("%s/doesntmatter:sometag", base)
239248

240249
_, _, err := resolver.Resolve(ctx, image)
241250
if err == nil {
@@ -246,6 +255,59 @@ func TestBadTokenResolver(t *testing.T) {
246255
}
247256
}
248257

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

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

0 commit comments

Comments
 (0)