From d30a8321d8d2561dda41769a5e612462338d60c1 Mon Sep 17 00:00:00 2001 From: Josh Hawn Date: Wed, 17 Dec 2014 11:35:35 -0800 Subject: [PATCH] Address auth package comments from stevvooe Docker-DCO-1.1-Signed-off-by: Josh Hawn (github: jlhawn) --- auth/auth.go | 51 +++++++++++++++++++++++++++------- auth/token/accesscontroller.go | 2 +- auth/token/token.go | 11 ++------ auth/token/token_test.go | 3 -- 4 files changed, 44 insertions(+), 23 deletions(-) diff --git a/auth/auth.go b/auth/auth.go index bd31c56a..eb7332e7 100644 --- a/auth/auth.go +++ b/auth/auth.go @@ -1,3 +1,33 @@ +// Package auth defines a standard interface for request access controllers. +// +// An access controller has a simple interface with a single `Authorized` +// method which checks that a given request is authorized to perform one or +// more actions on one or more resources. This method should return a non-nil +// error if the requset is not authorized. +// +// An implementation registers its access controller by name with a constructor +// which accepts an options map for configuring the access controller. +// +// options := map[string]interface{}{"sillySecret": "whysosilly?"} +// accessController, _ := auth.GetAccessController("silly", options) +// +// This `accessController` can then be used in a request handler like so: +// +// func updateOrder(w http.ResponseWriter, r *http.Request) { +// orderNumber := r.FormValue("orderNumber") +// resource := auth.Resource{Type: "customerOrder", Name: orderNumber} +// access := auth.Access{Resource: resource, Action: "update"} +// +// if err := accessController.Authorized(r, access); err != nil { +// if challenge, ok := err.(auth.Challenge) { +// // Let the challenge write the response. +// challenge.ServeHTTP(w, r) +// } else { +// // Some other error. +// } +// } +// } +// package auth import ( @@ -23,8 +53,11 @@ type Access struct { // header values based on the error. type Challenge interface { error - Status() int - SetHeader(header http.Header) + // ServeHTTP prepares the request to conduct the appropriate challenge + // response. For most implementations, simply calling ServeHTTP should be + // sufficient. Because no body is written, users may write a custom body after + // calling ServeHTTP, but any headers must be written before the call and may + // be overwritten. ServeHTTP(w http.ResponseWriter, r *http.Request) } @@ -32,14 +65,12 @@ type Challenge interface { // and required access levels for a request. Implementations can support both // complete denial and http authorization challenges. type AccessController interface { - // Authorized returns non-nil if the request is granted the request - // access. If the error is non-nil, access should always be denied. The - // error may be of type Challenge, in which case the caller may have the - // Challenge handle the request or choose what action to take based on the - // Challenge header or response status. - // - // In the future, other error types, besides Challenge, may be added to - // support more complex authorization flows. + // Authorized returns non-nil if the request is granted access. If one or + // more Access structs are provided, the requested access will be compared + // with what is available to the request. If the error is non-nil, access + // should always be denied. The error may be of type Challenge, in which + // case the caller may have the Challenge handle the request or choose + // what action to take based on the Challenge header or response status. Authorized(req *http.Request, access ...Access) error } diff --git a/auth/token/accesscontroller.go b/auth/token/accesscontroller.go index 66bf1c31..09d78a71 100644 --- a/auth/token/accesscontroller.go +++ b/auth/token/accesscontroller.go @@ -114,7 +114,7 @@ func (ac *authChallenge) challengeParams() string { // SetHeader sets the WWW-Authenticate value for the given header. func (ac *authChallenge) SetHeader(header http.Header) { - header.Add(http.CanonicalHeaderKey("WWW-Authenticate"), ac.challengeParams()) + header.Add("WWW-Authenticate", ac.challengeParams()) } // ServeHttp handles writing the challenge response diff --git a/auth/token/token.go b/auth/token/token.go index d1b0d67a..2c1114a6 100644 --- a/auth/token/token.go +++ b/auth/token/token.go @@ -80,7 +80,6 @@ type Token struct { Header *Header Claims *ClaimSet Signature []byte - Valid bool } // VerifyOptions is used to specify @@ -150,11 +149,6 @@ func NewToken(rawToken string) (*Token, error) { // Verify attempts to verify this token using the given options. // Returns a nil error if the token is valid. func (t *Token) Verify(verifyOpts VerifyOptions) error { - if t.Valid { - // Token was already verified. - return nil - } - // Verify that the Issuer claim is a trusted authority. if !verifyOpts.TrustedIssuers.Contains(t.Claims.Issuer) { log.Errorf("token from untrusted issuer: %q", t.Claims.Issuer) @@ -203,8 +197,8 @@ func (t *Token) Verify(verifyOpts VerifyOptions) error { // Next, check if the signing key is one of the trusted keys. if _, isTrustedKey := verifyOpts.TrustedKeys[signingKey.KeyID()]; isTrustedKey { - // We're done! The token was signed by a trusted key and has been verified! - t.Valid = true + // We're done! The token was signed by + // a trusted key and has been verified! return nil } @@ -301,7 +295,6 @@ func (t *Token) verifyCertificateChain(leafKey libtrust.PublicKey, roots *x509.C } // The signing key's x509 chain is valid! - t.Valid = true return nil } diff --git a/auth/token/token_test.go b/auth/token/token_test.go index da466dde..c1e0d2ad 100644 --- a/auth/token/token_test.go +++ b/auth/token/token_test.go @@ -206,9 +206,6 @@ func TestTokenVerify(t *testing.T) { if err := token.Verify(verifyOps); err != nil { t.Fatal(err) } - if !token.Valid { - t.Fatal("token not marked as Valid") - } } }