diff --git a/registry/auth/auth.go b/registry/auth/auth.go index 1f28ea85e..0bda67f6c 100644 --- a/registry/auth/auth.go +++ b/registry/auth/auth.go @@ -76,6 +76,12 @@ type Access struct { Action string } +// Grant describes the permitted level of access for an authorized request. +type Grant struct { + User UserInfo // The authenticated user for the request. + Resources []Resource // The list of resources which have been authorized for the request. +} + // Challenge is a special error type which is used for HTTP 401 Unauthorized // responses and is able to write the response with WWW-Authenticate challenge // header values based on the error. @@ -93,15 +99,15 @@ 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 a nil error if the request is granted access and - // returns a new authorized context. If one or more Access structs are - // provided, the requested access will be compared with what is available - // to the request. Access is denied if the error is non-nil. 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. The returned context object should be derived - // from r.Context() and have a "auth.user" value set to a UserInfo struct. - Authorized(r *http.Request, access ...Access) (context.Context, error) + // Authorized determines 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. + // + // Return a Grant to grant the request access. Return an error to deny + // access. 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(r *http.Request, access ...Access) (*Grant, error) } // CredentialAuthenticator is an object which is able to authenticate credentials diff --git a/registry/auth/htpasswd/access.go b/registry/auth/htpasswd/access.go index a5c89a42d..c8c432653 100644 --- a/registry/auth/htpasswd/access.go +++ b/registry/auth/htpasswd/access.go @@ -49,7 +49,7 @@ func newAccessController(options map[string]interface{}) (auth.AccessController, return &accessController{realm: realm.(string), path: path}, nil } -func (ac *accessController) Authorized(req *http.Request, accessRecords ...auth.Access) (context.Context, error) { +func (ac *accessController) Authorized(req *http.Request, accessRecords ...auth.Access) (*auth.Grant, error) { username, password, ok := req.BasicAuth() if !ok { return nil, &challenge{ @@ -94,7 +94,7 @@ func (ac *accessController) Authorized(req *http.Request, accessRecords ...auth. } } - return auth.WithUser(req.Context(), auth.UserInfo{Name: username}), nil + return &auth.Grant{User: auth.UserInfo{Name: username}}, nil } // challenge implements the auth.Challenge interface. diff --git a/registry/auth/htpasswd/access_test.go b/registry/auth/htpasswd/access_test.go index ad5e7f70c..01f2ac9d7 100644 --- a/registry/auth/htpasswd/access_test.go +++ b/registry/auth/htpasswd/access_test.go @@ -43,7 +43,7 @@ func TestBasicAccessController(t *testing.T) { userNumber := 0 server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - authCtx, err := accessController.Authorized(r) + grant, err := accessController.Authorized(r) if err != nil { switch err := err.(type) { case auth.Challenge: @@ -55,13 +55,12 @@ func TestBasicAccessController(t *testing.T) { } } - userInfo, ok := authCtx.Value(auth.UserKey).(auth.UserInfo) - if !ok { - t.Fatal("basic accessController did not set auth.user context") + if grant == nil { + t.Fatal("basic accessController did not return auth grant") } - if userInfo.Name != testUsers[userNumber] { - t.Fatalf("expected user name %q, got %q", testUsers[userNumber], userInfo.Name) + if grant.User.Name != testUsers[userNumber] { + t.Fatalf("expected user name %q, got %q", testUsers[userNumber], grant.User.Name) } w.WriteHeader(http.StatusNoContent) diff --git a/registry/auth/silly/access.go b/registry/auth/silly/access.go index c8f383e23..1984ba20d 100644 --- a/registry/auth/silly/access.go +++ b/registry/auth/silly/access.go @@ -8,12 +8,10 @@ package silly import ( - "context" "fmt" "net/http" "strings" - "github.com/distribution/distribution/v3/internal/dcontext" "github.com/distribution/distribution/v3/registry/auth" ) @@ -43,7 +41,7 @@ func newAccessController(options map[string]interface{}) (auth.AccessController, // Authorized simply checks for the existence of the authorization header, // responding with a bearer challenge if it doesn't exist. -func (ac *accessController) Authorized(req *http.Request, accessRecords ...auth.Access) (context.Context, error) { +func (ac *accessController) Authorized(req *http.Request, accessRecords ...auth.Access) (*auth.Grant, error) { if req.Header.Get("Authorization") == "" { challenge := challenge{ realm: ac.realm, @@ -61,10 +59,7 @@ func (ac *accessController) Authorized(req *http.Request, accessRecords ...auth. return nil, &challenge } - ctx := auth.WithUser(req.Context(), auth.UserInfo{Name: "silly"}) - ctx = dcontext.WithLogger(ctx, dcontext.GetLogger(ctx, auth.UserNameKey, auth.UserKey)) - - return ctx, nil + return &auth.Grant{User: auth.UserInfo{Name: "silly"}}, nil } type challenge struct { diff --git a/registry/auth/silly/access_test.go b/registry/auth/silly/access_test.go index 1a137c715..506af0bde 100644 --- a/registry/auth/silly/access_test.go +++ b/registry/auth/silly/access_test.go @@ -15,7 +15,7 @@ func TestSillyAccessController(t *testing.T) { } server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - authCtx, err := ac.Authorized(r) + grant, err := ac.Authorized(r) if err != nil { switch err := err.(type) { case auth.Challenge: @@ -27,13 +27,12 @@ func TestSillyAccessController(t *testing.T) { } } - userInfo, ok := authCtx.Value(auth.UserKey).(auth.UserInfo) - if !ok { - t.Fatal("silly accessController did not set auth.user context") + if grant == nil { + t.Fatal("silly accessController did not return auth grant") } - if userInfo.Name != "silly" { - t.Fatalf("expected user name %q, got %q", "silly", userInfo.Name) + if grant.User.Name != "silly" { + t.Fatalf("expected user name %q, got %q", "silly", grant.User.Name) } w.WriteHeader(http.StatusNoContent) diff --git a/registry/auth/token/accesscontroller.go b/registry/auth/token/accesscontroller.go index e019d0f51..bed4c827d 100644 --- a/registry/auth/token/accesscontroller.go +++ b/registry/auth/token/accesscontroller.go @@ -1,7 +1,6 @@ package token import ( - "context" "crypto" "crypto/x509" "encoding/json" @@ -291,7 +290,7 @@ func newAccessController(options map[string]interface{}) (auth.AccessController, // Authorized handles checking whether the given request is authorized // for actions on resources described by the given access items. -func (ac *accessController) Authorized(req *http.Request, accessItems ...auth.Access) (context.Context, error) { +func (ac *accessController) Authorized(req *http.Request, accessItems ...auth.Access) (*auth.Grant, error) { challenge := &authChallenge{ realm: ac.realm, autoRedirect: ac.autoRedirect, @@ -332,9 +331,10 @@ func (ac *accessController) Authorized(req *http.Request, accessItems ...auth.Ac } } - ctx := auth.WithResources(req.Context(), claims.resources()) - - return auth.WithUser(ctx, auth.UserInfo{Name: claims.Subject}), nil + return &auth.Grant{ + User: auth.UserInfo{Name: claims.Subject}, + Resources: claims.resources(), + }, nil } // init handles registering the token auth backend. diff --git a/registry/auth/token/token_test.go b/registry/auth/token/token_test.go index 52d34a70f..a96546af3 100644 --- a/registry/auth/token/token_test.go +++ b/registry/auth/token/token_test.go @@ -465,7 +465,7 @@ func TestAccessController(t *testing.T) { Action: "baz", } - authCtx, err := accessController.Authorized(req, testAccess) + grant, err := accessController.Authorized(req, testAccess) challenge, ok := err.(auth.Challenge) if !ok { t.Fatal("accessController did not return a challenge") @@ -475,8 +475,8 @@ func TestAccessController(t *testing.T) { t.Fatalf("accessControler did not get expected error - got %s - expected %s", challenge, ErrTokenRequired) } - if authCtx != nil { - t.Fatalf("expected nil auth context but got %s", authCtx) + if grant != nil { + t.Fatalf("expected nil auth grant but got %#v", grant) } // 2. Supply an invalid token. @@ -500,7 +500,7 @@ func TestAccessController(t *testing.T) { req.Header.Set("Authorization", fmt.Sprintf("Bearer %s", token.Raw)) - authCtx, err = accessController.Authorized(req, testAccess) + grant, err = accessController.Authorized(req, testAccess) challenge, ok = err.(auth.Challenge) if !ok { t.Fatal("accessController did not return a challenge") @@ -510,8 +510,8 @@ func TestAccessController(t *testing.T) { t.Fatalf("accessControler did not get expected error - got %s - expected %s", challenge, ErrTokenRequired) } - if authCtx != nil { - t.Fatalf("expected nil auth context but got %s", authCtx) + if grant != nil { + t.Fatalf("expected nil auth grant but got %#v", grant) } // create a valid jwk @@ -532,7 +532,7 @@ func TestAccessController(t *testing.T) { req.Header.Set("Authorization", fmt.Sprintf("Bearer %s", token.Raw)) - authCtx, err = accessController.Authorized(req, testAccess) + grant, err = accessController.Authorized(req, testAccess) challenge, ok = err.(auth.Challenge) if !ok { t.Fatal("accessController did not return a challenge") @@ -542,8 +542,8 @@ func TestAccessController(t *testing.T) { t.Fatalf("accessControler did not get expected error - got %s - expected %s", challenge, ErrInsufficientScope) } - if authCtx != nil { - t.Fatalf("expected nil auth context but got %s", authCtx) + if grant != nil { + t.Fatalf("expected nil auth grant but got %#v", grant) } // 4. Supply the token we need, or deserve, or whatever. @@ -562,18 +562,13 @@ func TestAccessController(t *testing.T) { req.Header.Set("Authorization", fmt.Sprintf("Bearer %s", token.Raw)) - authCtx, err = accessController.Authorized(req, testAccess) + grant, err = accessController.Authorized(req, testAccess) if err != nil { t.Fatalf("accessController returned unexpected error: %s", err) } - userInfo, ok := authCtx.Value(auth.UserKey).(auth.UserInfo) - if !ok { - t.Fatal("token accessController did not set auth.user context") - } - - if userInfo.Name != "foo" { - t.Fatalf("expected user name %q, got %q", "foo", userInfo.Name) + if grant.User.Name != "foo" { + t.Fatalf("expected user name %q, got %q", "foo", grant.User.Name) } // 5. Supply a token with full admin rights, which is represented as "*". diff --git a/registry/handlers/app.go b/registry/handlers/app.go index 8bb5bbbcd..471f7c169 100644 --- a/registry/handlers/app.go +++ b/registry/handlers/app.go @@ -797,7 +797,7 @@ func (app *App) authorized(w http.ResponseWriter, r *http.Request, context *Cont accessRecords = appendCatalogAccessRecord(accessRecords, r) } - ctx, err := app.accessController.Authorized(r.WithContext(context.Context), accessRecords...) + grant, err := app.accessController.Authorized(r.WithContext(context.Context), accessRecords...) if err != nil { switch err := err.(type) { case auth.Challenge: @@ -818,6 +818,12 @@ func (app *App) authorized(w http.ResponseWriter, r *http.Request, context *Cont return err } + if grant == nil { + return fmt.Errorf("access controller returned neither an access grant nor an error") + } + + ctx := auth.WithUser(context.Context, grant.User) + ctx = auth.WithResources(ctx, grant.Resources) dcontext.GetLogger(ctx, auth.UserNameKey).Info("authorized request") // TODO(stevvooe): This pattern needs to be cleaned up a bit. One context