Skip to content

Commit

Permalink
Merge pull request #2711 from davidswu/autoredirect
Browse files Browse the repository at this point in the history
add autoredirect auth config
  • Loading branch information
dmcgowan authored Nov 27, 2018
2 parents dd36fd3 + 2e1e630 commit aa985ba
Show file tree
Hide file tree
Showing 9 changed files with 48 additions and 29 deletions.
2 changes: 1 addition & 1 deletion contrib/token-server/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,7 @@ func (ts *tokenServer) getToken(ctx context.Context, w http.ResponseWriter, r *h
// Get response context.
ctx, w = dcontext.WithResponseWriter(ctx, w)

challenge.SetHeaders(w)
challenge.SetHeaders(r, w)
handleError(ctx, errcode.ErrorCodeUnauthorized.WithDetail(challenge.Error()), w)

dcontext.GetResponseLogger(ctx).Info("get token authentication challenge")
Expand Down
4 changes: 2 additions & 2 deletions registry/auth/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
// if ctx, err := accessController.Authorized(ctx, access); err != nil {
// if challenge, ok := err.(auth.Challenge) {
// // Let the challenge write the response.
// challenge.SetHeaders(w)
// challenge.SetHeaders(r, w)
// w.WriteHeader(http.StatusUnauthorized)
// return
// } else {
Expand Down Expand Up @@ -87,7 +87,7 @@ type Challenge interface {
// 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)
SetHeaders(r *http.Request, w http.ResponseWriter)
}

// AccessController controls access to registry resources based on a request
Expand Down
2 changes: 1 addition & 1 deletion registry/auth/htpasswd/access.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ type challenge struct {
var _ auth.Challenge = challenge{}

// SetHeaders sets the basic challenge header on the response.
func (ch challenge) SetHeaders(w http.ResponseWriter) {
func (ch challenge) SetHeaders(r *http.Request, w http.ResponseWriter) {
w.Header().Set("WWW-Authenticate", fmt.Sprintf("Basic realm=%q", ch.realm))
}

Expand Down
2 changes: 1 addition & 1 deletion registry/auth/htpasswd/access_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ func TestBasicAccessController(t *testing.T) {
if err != nil {
switch err := err.(type) {
case auth.Challenge:
err.SetHeaders(w)
err.SetHeaders(r, w)
w.WriteHeader(http.StatusUnauthorized)
return
default:
Expand Down
2 changes: 1 addition & 1 deletion registry/auth/silly/access.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ type challenge struct {
var _ auth.Challenge = challenge{}

// SetHeaders sets a simple bearer challenge on the response.
func (ch challenge) SetHeaders(w http.ResponseWriter) {
func (ch challenge) SetHeaders(r *http.Request, w http.ResponseWriter) {
header := fmt.Sprintf("Bearer realm=%q,service=%q", ch.realm, ch.service)

if ch.scope != "" {
Expand Down
2 changes: 1 addition & 1 deletion registry/auth/silly/access_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ func TestSillyAccessController(t *testing.T) {
if err != nil {
switch err := err.(type) {
case auth.Challenge:
err.SetHeaders(w)
err.SetHeaders(r, w)
w.WriteHeader(http.StatusUnauthorized)
return
default:
Expand Down
59 changes: 38 additions & 21 deletions registry/auth/token/accesscontroller.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,10 +76,11 @@ var (

// authChallenge implements the auth.Challenge interface.
type authChallenge struct {
err error
realm string
service string
accessSet accessSet
err error
realm string
autoRedirect bool
service string
accessSet accessSet
}

var _ auth.Challenge = authChallenge{}
Expand All @@ -97,8 +98,14 @@ func (ac authChallenge) Status() int {
// 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 {
str := fmt.Sprintf("Bearer realm=%q,service=%q", ac.realm, ac.service)
func (ac authChallenge) challengeParams(r *http.Request) string {
var realm string
if ac.autoRedirect {
realm = fmt.Sprintf("https://%s/auth/token", r.Host)
} else {
realm = ac.realm
}
str := fmt.Sprintf("Bearer realm=%q,service=%q", realm, ac.service)

if scope := ac.accessSet.scopeParam(); scope != "" {
str = fmt.Sprintf("%s,scope=%q", str, scope)
Expand All @@ -114,23 +121,25 @@ func (ac authChallenge) challengeParams() string {
}

// SetChallenge sets the WWW-Authenticate value for the response.
func (ac authChallenge) SetHeaders(w http.ResponseWriter) {
w.Header().Add("WWW-Authenticate", ac.challengeParams())
func (ac authChallenge) SetHeaders(r *http.Request, w http.ResponseWriter) {
w.Header().Add("WWW-Authenticate", ac.challengeParams(r))
}

// accessController implements the auth.AccessController interface.
type accessController struct {
realm string
issuer string
service string
rootCerts *x509.CertPool
trustedKeys map[string]libtrust.PublicKey
realm string
autoRedirect bool
issuer string
service string
rootCerts *x509.CertPool
trustedKeys map[string]libtrust.PublicKey
}

// tokenAccessOptions is a convenience type for handling
// options to the contstructor of an accessController.
type tokenAccessOptions struct {
realm string
autoRedirect bool
issuer string
service string
rootCertBundle string
Expand All @@ -153,6 +162,12 @@ func checkOptions(options map[string]interface{}) (tokenAccessOptions, error) {

opts.realm, opts.issuer, opts.service, opts.rootCertBundle = vals[0], vals[1], vals[2], vals[3]

autoRedirect, ok := options["autoredirect"].(bool)
if !ok {
return opts, fmt.Errorf("token auth requires a valid option bool: autoredirect")
}
opts.autoRedirect = autoRedirect

return opts, nil
}

Expand Down Expand Up @@ -205,21 +220,23 @@ func newAccessController(options map[string]interface{}) (auth.AccessController,
}

return &accessController{
realm: config.realm,
issuer: config.issuer,
service: config.service,
rootCerts: rootPool,
trustedKeys: trustedKeys,
realm: config.realm,
autoRedirect: config.autoRedirect,
issuer: config.issuer,
service: config.service,
rootCerts: rootPool,
trustedKeys: trustedKeys,
}, nil
}

// Authorized handles checking whether the given request is authorized
// for actions on resources described by the given access items.
func (ac *accessController) Authorized(ctx context.Context, accessItems ...auth.Access) (context.Context, error) {
challenge := &authChallenge{
realm: ac.realm,
service: ac.service,
accessSet: newAccessSet(accessItems...),
realm: ac.realm,
autoRedirect: ac.autoRedirect,
service: ac.service,
accessSet: newAccessSet(accessItems...),
}

req, err := dcontext.GetRequest(ctx)
Expand Down
2 changes: 2 additions & 0 deletions registry/auth/token/token_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -333,6 +333,7 @@ func TestAccessController(t *testing.T) {
"issuer": issuer,
"service": service,
"rootcertbundle": rootCertBundleFilename,
"autoredirect": false,
}

accessController, err := newAccessController(options)
Expand Down Expand Up @@ -518,6 +519,7 @@ func TestNewAccessControllerPemBlock(t *testing.T) {
"issuer": issuer,
"service": service,
"rootcertbundle": rootCertBundleFilename,
"autoredirect": false,
}

ac, err := newAccessController(options)
Expand Down
2 changes: 1 addition & 1 deletion registry/handlers/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -847,7 +847,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.SetHeaders(w)
err.SetHeaders(r, w)

if err := errcode.ServeJSON(w, errcode.ErrorCodeUnauthorized.WithDetail(accessRecords)); err != nil {
dcontext.GetLogger(context).Errorf("error serving error json: %v (from %v)", err, context.Errors)
Expand Down

0 comments on commit aa985ba

Please sign in to comment.