From a0fdfb9d4dd6e84703d155d9c1f3c72d52324814 Mon Sep 17 00:00:00 2001 From: Stephen J Day Date: Thu, 23 Jul 2015 19:39:56 -0700 Subject: [PATCH] Simplify auth.Challenge interface to SetHeaders This removes the erroneous http.Handler interface in favor a simple SetHeaders method that only operattes on the response. Several unnecessary uses of pointer types were also fixed up. Signed-off-by: Stephen J Day --- registry/auth/auth.go | 12 ++++++------ registry/auth/htpasswd/access.go | 10 ++++++---- registry/auth/htpasswd/access_test.go | 2 +- registry/auth/silly/access.go | 7 +++++-- registry/auth/silly/access_test.go | 2 +- registry/auth/token/accesscontroller.go | 20 ++++++++------------ registry/handlers/app.go | 2 +- 7 files changed, 28 insertions(+), 27 deletions(-) diff --git a/registry/auth/auth.go b/registry/auth/auth.go index 3107537e3..7ae2a157d 100644 --- a/registry/auth/auth.go +++ b/registry/auth/auth.go @@ -61,12 +61,12 @@ type Access struct { // header values based on the error. type Challenge interface { error - // ServeHTTP prepares the request to conduct the appropriate challenge - // response by adding the appropriate HTTP challenge header on the response - // message. Callers are expected to set the appropriate HTTP status code - // (e.g. 401) themselves. Because no body is written, users may write a - // custom body after calling ServeHTTP. - ServeHTTP(w http.ResponseWriter, r *http.Request) + + // SetHeaders prepares the request to conduct a challenge response by + // adding the an HTTP challenge header on the response message. Callers + // are expected to set the appropriate HTTP status code (e.g. 401) + // themselves. + SetHeaders(w http.ResponseWriter) } // AccessController controls access to registry resources based on a request diff --git a/registry/auth/htpasswd/access.go b/registry/auth/htpasswd/access.go index b8c4d41e4..bb153f4b5 100644 --- a/registry/auth/htpasswd/access.go +++ b/registry/auth/htpasswd/access.go @@ -87,12 +87,14 @@ type challenge struct { err error } -func (ch *challenge) ServeHTTP(w http.ResponseWriter, r *http.Request) { - header := fmt.Sprintf("Basic realm=%q", ch.realm) - w.Header().Set("WWW-Authenticate", header) +var _ auth.Challenge = challenge{} + +// SetHeaders sets the basic challenge header on the response. +func (ch challenge) SetHeaders(w http.ResponseWriter) { + w.Header().Set("WWW-Authenticate", fmt.Sprintf("Basic realm=%q", ch.realm)) } -func (ch *challenge) Error() string { +func (ch challenge) Error() string { return fmt.Sprintf("basic authentication challenge: %#v", ch) } diff --git a/registry/auth/htpasswd/access_test.go b/registry/auth/htpasswd/access_test.go index 79e9422ca..db0405475 100644 --- a/registry/auth/htpasswd/access_test.go +++ b/registry/auth/htpasswd/access_test.go @@ -48,7 +48,7 @@ func TestBasicAccessController(t *testing.T) { if err != nil { switch err := err.(type) { case auth.Challenge: - err.ServeHTTP(w, r) + err.SetHeaders(w) w.WriteHeader(http.StatusUnauthorized) return default: diff --git a/registry/auth/silly/access.go b/registry/auth/silly/access.go index 7ae43e25d..7d6efb079 100644 --- a/registry/auth/silly/access.go +++ b/registry/auth/silly/access.go @@ -75,7 +75,10 @@ type challenge struct { scope string } -func (ch *challenge) ServeHTTP(w http.ResponseWriter, r *http.Request) { +var _ auth.Challenge = challenge{} + +// SetHeaders sets a simple bearer challenge on the response. +func (ch challenge) SetHeaders(w http.ResponseWriter) { header := fmt.Sprintf("Bearer realm=%q,service=%q", ch.realm, ch.service) if ch.scope != "" { @@ -85,7 +88,7 @@ func (ch *challenge) ServeHTTP(w http.ResponseWriter, r *http.Request) { w.Header().Set("WWW-Authenticate", header) } -func (ch *challenge) Error() string { +func (ch challenge) Error() string { return fmt.Sprintf("silly authentication challenge: %#v", ch) } diff --git a/registry/auth/silly/access_test.go b/registry/auth/silly/access_test.go index 2fd160de9..8b5ecb801 100644 --- a/registry/auth/silly/access_test.go +++ b/registry/auth/silly/access_test.go @@ -21,7 +21,7 @@ func TestSillyAccessController(t *testing.T) { if err != nil { switch err := err.(type) { case auth.Challenge: - err.ServeHTTP(w, r) + err.SetHeaders(w) w.WriteHeader(http.StatusUnauthorized) return default: diff --git a/registry/auth/token/accesscontroller.go b/registry/auth/token/accesscontroller.go index c947b67df..0549f8eff 100644 --- a/registry/auth/token/accesscontroller.go +++ b/registry/auth/token/accesscontroller.go @@ -82,20 +82,22 @@ type authChallenge struct { accessSet accessSet } +var _ auth.Challenge = authChallenge{} + // Error returns the internal error string for this authChallenge. -func (ac *authChallenge) Error() string { +func (ac authChallenge) Error() string { return ac.err.Error() } // Status returns the HTTP Response Status Code for this authChallenge. -func (ac *authChallenge) Status() int { +func (ac authChallenge) Status() int { return http.StatusUnauthorized } // challengeParams constructs the value to be used in // the WWW-Authenticate response challenge header. // See https://tools.ietf.org/html/rfc6750#section-3 -func (ac *authChallenge) challengeParams() string { +func (ac authChallenge) challengeParams() string { str := fmt.Sprintf("Bearer realm=%q,service=%q", ac.realm, ac.service) if scope := ac.accessSet.scopeParam(); scope != "" { @@ -111,15 +113,9 @@ func (ac *authChallenge) challengeParams() string { return str } -// SetHeader sets the WWW-Authenticate value for the given header. -func (ac *authChallenge) SetHeader(header http.Header) { - header.Add("WWW-Authenticate", ac.challengeParams()) -} - -// ServeHttp handles writing the challenge response -// by setting the challenge header. -func (ac *authChallenge) ServeHTTP(w http.ResponseWriter, r *http.Request) { - ac.SetHeader(w.Header()) +// SetChallenge sets the WWW-Authenticate value for the response. +func (ac authChallenge) SetHeaders(w http.ResponseWriter) { + w.Header().Add("WWW-Authenticate", ac.challengeParams()) } // accessController implements the auth.AccessController interface. diff --git a/registry/handlers/app.go b/registry/handlers/app.go index f61b2c1e0..8395ea656 100644 --- a/registry/handlers/app.go +++ b/registry/handlers/app.go @@ -502,7 +502,7 @@ func (app *App) authorized(w http.ResponseWriter, r *http.Request, context *Cont switch err := err.(type) { case auth.Challenge: // Add the appropriate WWW-Auth header - err.ServeHTTP(w, r) + err.SetHeaders(w) if err := errcode.ServeJSON(w, v2.ErrorCodeUnauthorized.WithDetail(accessRecords)); err != nil { ctxu.GetLogger(context).Errorf("error serving error json: %v (from %v)", err, context.Errors)