Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add autoredirect auth config #2711

Merged
merged 3 commits into from
Nov 27, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't have to be in this change but I'd prefer we use a url.URL object to build this out.

} 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