diff --git a/.github/workflows/ecosystem-tools.yaml b/.github/workflows/ecosystem-tools.yaml index 27a81054..fbdeabdf 100644 --- a/.github/workflows/ecosystem-tools.yaml +++ b/.github/workflows/ecosystem-tools.yaml @@ -78,6 +78,9 @@ jobs: - name: Run push-pull tests run: | make test-push-pull + - name: Run push-pull-authn tests + run: | + make test-push-pull-authn - name: Run metrics tests run: | make test-bats-metrics diff --git a/Makefile b/Makefile index 4b6b4ae7..9e8d6c6b 100644 --- a/Makefile +++ b/Makefile @@ -373,6 +373,10 @@ test-push-pull-running-dedupe: check-linux binary check-skopeo $(BATS) $(REGCLIE test-push-pull-running-dedupe-verbose: check-linux binary check-skopeo $(BATS) $(REGCLIENT) $(ORAS) $(HELM) $(BATS) --trace --verbose-run --print-output-on-failure --show-output-of-passing-tests test/blackbox/pushpull_running_dedupe.bats +.PHONY: test-push-pull-authn +test-push-pull-authn: check-linux binary check-skopeo $(BATS) $(REGCLIENT) + $(BATS) --trace --print-output-on-failure test/blackbox/pushpull_authn.bats + .PHONY: test-sync-harness test-sync-harness: check-linux binary binary-minimal bench check-skopeo $(BATS) $(BATS) --trace --print-output-on-failure test/blackbox/sync_harness.bats @@ -401,11 +405,11 @@ test-bats-metadata: check-linux binary check-skopeo $(BATS) .PHONY: test-cloud-only test-cloud-only: check-linux binary check-skopeo $(BATS) - $(BATS) --trace --print-output-on-failure test/blackbox/cloud-only.bats + $(BATS) --trace --print-output-on-failure test/blackbox/cloud_only.bats .PHONY: test-cloud-only-verbose test-cloud-only-verbose: check-linux binary check-skopeo $(BATS) - $(BATS) --trace --verbose-run --print-output-on-failure --show-output-of-passing-tests test/blackbox/cloud-only.bats + $(BATS) --trace --verbose-run --print-output-on-failure --show-output-of-passing-tests test/blackbox/cloud_only.bats .PHONY: test-bats-sync test-bats-sync: BUILD_LABELS=sync diff --git a/pkg/api/authn.go b/pkg/api/authn.go index 3ffb6293..7b4ffb81 100644 --- a/pkg/api/authn.go +++ b/pkg/api/authn.go @@ -38,7 +38,7 @@ import ( apiErr "zotregistry.io/zot/pkg/api/errors" zcommon "zotregistry.io/zot/pkg/common" "zotregistry.io/zot/pkg/log" - localCtx "zotregistry.io/zot/pkg/requestcontext" + reqCtx "zotregistry.io/zot/pkg/requestcontext" storageConstants "zotregistry.io/zot/pkg/storage/constants" ) @@ -63,8 +63,8 @@ func AuthHandler(ctlr *Controller) mux.MiddlewareFunc { return authnMiddleware.TryAuthnHandlers(ctlr) } -func (amw *AuthnMiddleware) sessionAuthn(ctlr *Controller, response http.ResponseWriter, - request *http.Request, +func (amw *AuthnMiddleware) sessionAuthn(ctlr *Controller, userAc *reqCtx.UserAccessControl, + response http.ResponseWriter, request *http.Request, ) (bool, error) { identity, ok := GetAuthUserFromRequestSession(ctlr.CookieStore, request, ctlr.Log) if !ok { @@ -83,23 +83,24 @@ func (amw *AuthnMiddleware) sessionAuthn(ctlr *Controller, response http.Respons return false, nil } - ctx := getReqContextWithAuthorization(identity, []string{}, request) + userAc.SetUsername(identity) + userAc.SaveOnRequest(request) - groups, err := ctlr.MetaDB.GetUserGroups(ctx) + groups, err := ctlr.MetaDB.GetUserGroups(request.Context()) if err != nil { ctlr.Log.Err(err).Str("identity", identity).Msg("can not get user profile in DB") return false, err } - ctx = getReqContextWithAuthorization(identity, groups, request) - *request = *request.WithContext(ctx) + userAc.AddGroups(groups) + userAc.SaveOnRequest(request) return true, nil } -func (amw *AuthnMiddleware) basicAuthn(ctlr *Controller, response http.ResponseWriter, - request *http.Request, +func (amw *AuthnMiddleware) basicAuthn(ctlr *Controller, userAc *reqCtx.UserAccessControl, + response http.ResponseWriter, request *http.Request, ) (bool, error) { cookieStore := ctlr.CookieStore @@ -122,15 +123,17 @@ func (amw *AuthnMiddleware) basicAuthn(ctlr *Controller, response http.ResponseW groups = ac.getUserGroups(identity) } - ctx := getReqContextWithAuthorization(identity, groups, request) - *request = *request.WithContext(ctx) + userAc.SetUsername(identity) + userAc.AddGroups(groups) + userAc.SaveOnRequest(request) // saved logged session if err := saveUserLoggedSession(cookieStore, response, request, identity, ctlr.Log); err != nil { return false, err } - if err := ctlr.MetaDB.SetUserGroups(ctx, groups); err != nil { + // we have already populated the request context with userAc + if err := ctlr.MetaDB.SetUserGroups(request.Context(), groups); err != nil { ctlr.Log.Error().Err(err).Str("identity", identity).Msg("couldn't update user profile") return false, err @@ -156,14 +159,16 @@ func (amw *AuthnMiddleware) basicAuthn(ctlr *Controller, response http.ResponseW groups = append(groups, ldapgroups...) - ctx := getReqContextWithAuthorization(identity, groups, request) - *request = *request.WithContext(ctx) + userAc.SetUsername(identity) + userAc.AddGroups(groups) + userAc.SaveOnRequest(request) if err := saveUserLoggedSession(cookieStore, response, request, identity, ctlr.Log); err != nil { return false, err } - if err := ctlr.MetaDB.SetUserGroups(ctx, groups); err != nil { + // we have already populated the request context with userAc + if err := ctlr.MetaDB.SetUserGroups(request.Context(), groups); err != nil { ctlr.Log.Error().Err(err).Str("identity", identity).Msg("couldn't update user profile") return false, err @@ -201,10 +206,11 @@ func (amw *AuthnMiddleware) basicAuthn(ctlr *Controller, response http.ResponseW } if storedIdentity == identity { - ctx := getReqContextWithAuthorization(identity, []string{}, request) + userAc.SetUsername(identity) + userAc.SaveOnRequest(request) // check if api key expired - isExpired, err := ctlr.MetaDB.IsAPIKeyExpired(ctx, hashedKey) + isExpired, err := ctlr.MetaDB.IsAPIKeyExpired(request.Context(), hashedKey) if err != nil { ctlr.Log.Err(err).Str("identity", identity).Msg("can not verify if api key expired") @@ -215,22 +221,22 @@ func (amw *AuthnMiddleware) basicAuthn(ctlr *Controller, response http.ResponseW return false, nil } - err = ctlr.MetaDB.UpdateUserAPIKeyLastUsed(ctx, hashedKey) + err = ctlr.MetaDB.UpdateUserAPIKeyLastUsed(request.Context(), hashedKey) if err != nil { ctlr.Log.Err(err).Str("identity", identity).Msg("can not update user profile in DB") return false, err } - groups, err := ctlr.MetaDB.GetUserGroups(ctx) + groups, err := ctlr.MetaDB.GetUserGroups(request.Context()) if err != nil { ctlr.Log.Err(err).Str("identity", identity).Msg("can not get user's groups in DB") return false, err } - ctx = getReqContextWithAuthorization(identity, groups, request) - *request = *request.WithContext(ctx) + userAc.AddGroups(groups) + userAc.SaveOnRequest(request) return true, nil } @@ -242,7 +248,7 @@ func (amw *AuthnMiddleware) basicAuthn(ctlr *Controller, response http.ResponseW func (amw *AuthnMiddleware) TryAuthnHandlers(ctlr *Controller) mux.MiddlewareFunc { //nolint: gocyclo // no password based authN, if neither LDAP nor HTTP BASIC is enabled if !ctlr.Config.IsBasicAuthnEnabled() { - return noPasswdAuth(ctlr.Config) + return noPasswdAuth(ctlr) } amw.credMap = make(map[string]string) @@ -369,10 +375,15 @@ func (amw *AuthnMiddleware) TryAuthnHandlers(ctlr *Controller) mux.MiddlewareFun isMgmtRequested := request.RequestURI == constants.FullMgmt allowAnonymous := ctlr.Config.HTTP.AccessControl.AnonymousPolicyExists() + // build user access control info + userAc := reqCtx.NewUserAccessControl() + // if it will not be populated by authn handlers, this represents an anonymous user + userAc.SaveOnRequest(request) + // try basic auth if authorization header is given if !isAuthorizationHeaderEmpty(request) { //nolint: gocritic //nolint: contextcheck - authenticated, err := amw.basicAuthn(ctlr, response, request) + authenticated, err := amw.basicAuthn(ctlr, userAc, response, request) if err != nil { response.WriteHeader(http.StatusInternalServerError) @@ -387,7 +398,7 @@ func (amw *AuthnMiddleware) TryAuthnHandlers(ctlr *Controller) mux.MiddlewareFun } else if hasSessionHeader(request) { // try session auth //nolint: contextcheck - authenticated, err := amw.sessionAuthn(ctlr, response, request) + authenticated, err := amw.sessionAuthn(ctlr, userAc, response, request) if err != nil { if errors.Is(err, zerr.ErrUserDataNotFound) { ctlr.Log.Err(err).Msg("can not find user profile in DB") @@ -408,19 +419,12 @@ func (amw *AuthnMiddleware) TryAuthnHandlers(ctlr *Controller) mux.MiddlewareFun // the session header can be present also for anonymous calls if allowAnonymous || isMgmtRequested { - ctx := getReqContextWithAuthorization("", []string{}, request) - *request = *request.WithContext(ctx) //nolint:contextcheck - next.ServeHTTP(response, request) return } } else if allowAnonymous || isMgmtRequested { // try anonymous auth only if basic auth/session was not given - // we want to bypass auth for mgmt route - ctx := getReqContextWithAuthorization("", []string{}, request) - *request = *request.WithContext(ctx) //nolint:contextcheck - next.ServeHTTP(response, request) return @@ -495,7 +499,7 @@ func bearerAuthHandler(ctlr *Controller) mux.MiddlewareFunc { } } -func noPasswdAuth(config *config.Config) mux.MiddlewareFunc { +func noPasswdAuth(ctlr *Controller) mux.MiddlewareFunc { return func(next http.Handler) http.Handler { return http.HandlerFunc(func(response http.ResponseWriter, request *http.Request) { if request.Method == http.MethodOptions { @@ -505,8 +509,29 @@ func noPasswdAuth(config *config.Config) mux.MiddlewareFunc { return } - ctx := getReqContextWithAuthorization("", []string{}, request) - *request = *request.WithContext(ctx) //nolint:contextcheck + userAc := reqCtx.NewUserAccessControl() + + // if no basic auth enabled then try to get identity from mTLS auth + if request.TLS != nil { + verifiedChains := request.TLS.VerifiedChains + if len(verifiedChains) > 0 && len(verifiedChains[0]) > 0 { + for _, cert := range request.TLS.PeerCertificates { + identity := cert.Subject.CommonName + if identity != "" { + // assign identity to authz context, needed for extensions + userAc.SetUsername(identity) + } + } + } + } + + if ctlr.Config.IsMTLSAuthEnabled() && userAc.IsAnonymous() { + authFail(response, request, ctlr.Config.HTTP.Realm, ctlr.Config.HTTP.Auth.FailDelay) + + return + } + + userAc.SaveOnRequest(request) // Process request next.ServeHTTP(response, request) @@ -649,18 +674,6 @@ func getRelyingPartyArgs(cfg *config.Config, provider string) ( return issuer, clientID, clientSecret, redirectURI, scopes, options } -func getReqContextWithAuthorization(username string, groups []string, request *http.Request) context.Context { - acCtx := localCtx.AccessControlContext{ - Username: username, - Groups: groups, - } - - authzCtxKey := localCtx.GetContextKey() - ctx := context.WithValue(request.Context(), authzCtxKey, acCtx) - - return ctx -} - func authFail(w http.ResponseWriter, r *http.Request, realm string, delay int) { time.Sleep(time.Duration(delay) * time.Second) @@ -809,7 +822,10 @@ func OAuth2Callback(ctlr *Controller, w http.ResponseWriter, r *http.Request, st return "", zerr.ErrInvalidStateCookie } - ctx := getReqContextWithAuthorization(email, groups, r) + userAc := reqCtx.NewUserAccessControl() + userAc.SetUsername(email) + userAc.AddGroups(groups) + userAc.SaveOnRequest(r) // if this line has been reached, then a new session should be created // if the `session` key is already on the cookie, it's not a valid one @@ -817,7 +833,7 @@ func OAuth2Callback(ctlr *Controller, w http.ResponseWriter, r *http.Request, st return "", err } - if err := ctlr.MetaDB.SetUserGroups(ctx, groups); err != nil { + if err := ctlr.MetaDB.SetUserGroups(r.Context(), groups); err != nil { ctlr.Log.Error().Err(err).Str("identity", email).Msg("couldn't update the user profile") return "", err diff --git a/pkg/api/authn_test.go b/pkg/api/authn_test.go index 03cef7dc..10e92fb6 100644 --- a/pkg/api/authn_test.go +++ b/pkg/api/authn_test.go @@ -25,7 +25,7 @@ import ( extconf "zotregistry.io/zot/pkg/extensions/config" "zotregistry.io/zot/pkg/log" mTypes "zotregistry.io/zot/pkg/meta/types" - localCtx "zotregistry.io/zot/pkg/requestcontext" + reqCtx "zotregistry.io/zot/pkg/requestcontext" "zotregistry.io/zot/pkg/test" "zotregistry.io/zot/pkg/test/mocks" ) @@ -470,13 +470,9 @@ func TestAPIKeys(t *testing.T) { So(resp, ShouldNotBeNil) So(resp.StatusCode(), ShouldEqual, http.StatusUnauthorized) - authzCtxKey := localCtx.GetContextKey() - - acCtx := localCtx.AccessControlContext{ - Username: email, - } - - ctx := context.WithValue(context.Background(), authzCtxKey, acCtx) + userAc := reqCtx.NewUserAccessControl() + userAc.SetUsername(email) + ctx := userAc.DeriveContext(context.Background()) err = ctlr.MetaDB.DeleteUserData(ctx) So(err, ShouldBeNil) diff --git a/pkg/api/authz.go b/pkg/api/authz.go index 9bedef24..3228e8b0 100644 --- a/pkg/api/authz.go +++ b/pkg/api/authz.go @@ -8,22 +8,16 @@ import ( "github.com/gorilla/mux" "zotregistry.io/zot/pkg/api/config" + "zotregistry.io/zot/pkg/api/constants" "zotregistry.io/zot/pkg/common" "zotregistry.io/zot/pkg/log" - localCtx "zotregistry.io/zot/pkg/requestcontext" + reqCtx "zotregistry.io/zot/pkg/requestcontext" ) const ( - // method actions. - Create = "create" - Read = "read" - Update = "update" - Delete = "delete" - // behaviour actions. - DetectManifestCollision = "detectManifestCollision" - BASIC = "Basic" - BEARER = "Bearer" - OPENID = "OpenID" + BASIC = "Basic" + BEARER = "Bearer" + OPENID = "OpenID" ) // AccessController authorizes users to act on resources. @@ -90,7 +84,7 @@ func (ac *AccessController) getGlobPatterns(username string, groups []string, ac } // can verifies if a user can do action on repository. -func (ac *AccessController) can(ctx context.Context, username, action, repository string) bool { +func (ac *AccessController) can(userAc *reqCtx.UserAccessControl, action, repository string) bool { can := false var longestMatchedPattern string @@ -104,12 +98,8 @@ func (ac *AccessController) can(ctx context.Context, username, action, repositor } } - acCtx, err := localCtx.GetAccessControlContext(ctx) - if err != nil { - return false - } - - userGroups := acCtx.Groups + userGroups := userAc.GetGroups() + username := userAc.GetUsername() // check matched repo based policy pg, ok := ac.Config.Repositories[longestMatchedPattern] @@ -119,11 +109,7 @@ func (ac *AccessController) can(ctx context.Context, username, action, repositor // check admins based policy if !can { - if ac.isAdmin(username) && common.Contains(ac.Config.AdminPolicy.Actions, action) { - can = true - } - - if ac.isAnyGroupInAdminPolicy(userGroups) && common.Contains(ac.Config.AdminPolicy.Actions, action) { + if ac.isAdmin(username, userGroups) && common.Contains(ac.Config.AdminPolicy.Actions, action) { can = true } } @@ -132,8 +118,12 @@ func (ac *AccessController) can(ctx context.Context, username, action, repositor } // isAdmin . -func (ac *AccessController) isAdmin(username string) bool { - return common.Contains(ac.Config.AdminPolicy.Users, username) +func (ac *AccessController) isAdmin(username string, userGroups []string) bool { + if common.Contains(ac.Config.AdminPolicy.Users, username) || ac.isAnyGroupInAdminPolicy(userGroups) { + return true + } + + return false } func (ac *AccessController) isAnyGroupInAdminPolicy(userGroups []string) bool { @@ -161,33 +151,37 @@ func (ac *AccessController) getUserGroups(username string) []string { return groupNames } -// getContext updates an AccessControlContext for a user/anonymous and returns a context.Context containing it. -func (ac *AccessController) getContext(acCtx *localCtx.AccessControlContext, request *http.Request) context.Context { - readGlobPatterns := ac.getGlobPatterns(acCtx.Username, acCtx.Groups, Read) - dmcGlobPatterns := ac.getGlobPatterns(acCtx.Username, acCtx.Groups, DetectManifestCollision) +// getContext updates an UserAccessControl with admin status and specific permissions on repos. +func (ac *AccessController) updateUserAccessControl(userAc *reqCtx.UserAccessControl) { + identity := userAc.GetUsername() + groups := userAc.GetGroups() - acCtx.ReadGlobPatterns = readGlobPatterns - acCtx.DmcGlobPatterns = dmcGlobPatterns + readGlobPatterns := ac.getGlobPatterns(identity, groups, constants.ReadPermission) + createGlobPatterns := ac.getGlobPatterns(identity, groups, constants.CreatePermission) + updateGlobPatterns := ac.getGlobPatterns(identity, groups, constants.UpdatePermission) + deleteGlobPatterns := ac.getGlobPatterns(identity, groups, constants.DeletePermission) + dmcGlobPatterns := ac.getGlobPatterns(identity, groups, constants.DetectManifestCollisionPermission) - if ac.isAdmin(acCtx.Username) { - acCtx.IsAdmin = true + userAc.SetGlobPatterns(constants.ReadPermission, readGlobPatterns) + userAc.SetGlobPatterns(constants.CreatePermission, createGlobPatterns) + userAc.SetGlobPatterns(constants.UpdatePermission, updateGlobPatterns) + userAc.SetGlobPatterns(constants.DeletePermission, deleteGlobPatterns) + userAc.SetGlobPatterns(constants.DetectManifestCollisionPermission, dmcGlobPatterns) + + if ac.isAdmin(userAc.GetUsername(), userAc.GetGroups()) { + userAc.SetIsAdmin(true) } else { - acCtx.IsAdmin = false + userAc.SetIsAdmin(false) } - - authzCtxKey := localCtx.GetContextKey() - ctx := context.WithValue(request.Context(), authzCtxKey, *acCtx) - - return ctx } // getAuthnMiddlewareContext builds ac context(allowed to read repos and if user is admin) and returns it. func (ac *AccessController) getAuthnMiddlewareContext(authnType string, request *http.Request) context.Context { - amwCtx := localCtx.AuthnMiddlewareContext{ + amwCtx := reqCtx.AuthnMiddlewareContext{ AuthnType: authnType, } - amwCtxKey := localCtx.GetAuthnMiddlewareCtxKey() + amwCtxKey := reqCtx.GetAuthnMiddlewareCtxKey() ctx := context.WithValue(request.Context(), amwCtxKey, amwCtx) return ctx @@ -254,7 +248,7 @@ func BaseAuthzHandler(ctlr *Controller) mux.MiddlewareFunc { } // request comes from bearer authn, bypass it - authnMwCtx, err := localCtx.GetAuthnMiddlewareContext(request.Context()) + authnMwCtx, err := reqCtx.GetAuthnMiddlewareContext(request.Context()) if err != nil || (authnMwCtx != nil && authnMwCtx.AuthnType == BEARER) { next.ServeHTTP(response, request) @@ -268,51 +262,20 @@ func BaseAuthzHandler(ctlr *Controller) mux.MiddlewareFunc { return } - acCtrlr := NewAccessController(ctlr.Config) + aCtlr := NewAccessController(ctlr.Config) - var identity string + // get access control context made in authn.go + userAc, err := reqCtx.UserAcFromContext(request.Context()) + if err != nil { // should never happen + authFail(response, request, ctlr.Config.HTTP.Realm, ctlr.Config.HTTP.Auth.FailDelay) - // anonymous context - acCtx := &localCtx.AccessControlContext{} - - // get username from context made in authn.go - if ctlr.Config.IsBasicAuthnEnabled() { - // get access control context made in authn.go if authn is enabled - acCtx, err = localCtx.GetAccessControlContext(request.Context()) - if err != nil { // should never happen - authFail(response, request, ctlr.Config.HTTP.Realm, ctlr.Config.HTTP.Auth.FailDelay) - - return - } - - identity = acCtx.Username + return } - if request.TLS != nil { - verifiedChains := request.TLS.VerifiedChains - // still no identity, get it from TLS certs - if identity == "" && verifiedChains != nil && - len(verifiedChains) > 0 && len(verifiedChains[0]) > 0 { - for _, cert := range request.TLS.PeerCertificates { - identity = cert.Subject.CommonName - } + aCtlr.updateUserAccessControl(userAc) + userAc.SaveOnRequest(request) - // if we still don't have an identity - if identity == "" { - acCtrlr.Log.Info().Msg("couldn't get identity from TLS certificate") - authFail(response, request, ctlr.Config.HTTP.Realm, ctlr.Config.HTTP.Auth.FailDelay) - - return - } - - // assign identity to authz context, needed for extensions - acCtx.Username = identity - } - } - - ctx := acCtrlr.getContext(acCtx, request) - - next.ServeHTTP(response, request.WithContext(ctx)) //nolint:contextcheck + next.ServeHTTP(response, request) //nolint:contextcheck }) } } @@ -327,7 +290,7 @@ func DistSpecAuthzHandler(ctlr *Controller) mux.MiddlewareFunc { } // request comes from bearer authn, bypass it - authnMwCtx, err := localCtx.GetAuthnMiddlewareContext(request.Context()) + authnMwCtx, err := reqCtx.GetAuthnMiddlewareContext(request.Context()) if err != nil || (authnMwCtx != nil && authnMwCtx.AuthnType == BEARER) { next.ServeHTTP(response, request) @@ -340,45 +303,40 @@ func DistSpecAuthzHandler(ctlr *Controller) mux.MiddlewareFunc { acCtrlr := NewAccessController(ctlr.Config) - var identity string - - // get acCtx built in authn and previous authz middlewares - acCtx, err := localCtx.GetAccessControlContext(request.Context()) + // get userAc built in authn and previous authz middlewares + userAc, err := reqCtx.UserAcFromContext(request.Context()) if err != nil { // should never happen authFail(response, request, ctlr.Config.HTTP.Realm, ctlr.Config.HTTP.Auth.FailDelay) return } - // get username from context made in authn.go - identity = acCtx.Username - var action string if request.Method == http.MethodGet || request.Method == http.MethodHead { - action = Read + action = constants.ReadPermission } if request.Method == http.MethodPut || request.Method == http.MethodPatch || request.Method == http.MethodPost { // assume user wants to create - action = Create + action = constants.CreatePermission // if we get a reference (tag) if ok { is := ctlr.StoreController.GetImageStore(resource) tags, err := is.GetImageTags(resource) // if repo exists and request's tag exists then action is UPDATE if err == nil && common.Contains(tags, reference) && reference != "latest" { - action = Update + action = constants.UpdatePermission } } } if request.Method == http.MethodDelete { - action = Delete + action = constants.DeletePermission } - can := acCtrlr.can(request.Context(), identity, action, resource) //nolint:contextcheck + can := acCtrlr.can(userAc, action, resource) //nolint:contextcheck if !can { - common.AuthzFail(response, request, ctlr.Config.HTTP.Realm, ctlr.Config.HTTP.Auth.FailDelay) + common.AuthzFail(response, request, userAc.GetUsername(), ctlr.Config.HTTP.Realm, ctlr.Config.HTTP.Auth.FailDelay) } else { next.ServeHTTP(response, request) //nolint:contextcheck } diff --git a/pkg/api/config/config.go b/pkg/api/config/config.go index b22eadac..920a2058 100644 --- a/pkg/api/config/config.go +++ b/pkg/api/config/config.go @@ -249,6 +249,19 @@ func (c *Config) IsLdapAuthEnabled() bool { return false } +func (c *Config) IsMTLSAuthEnabled() bool { + if c.HTTP.TLS != nil && + c.HTTP.TLS.Key != "" && + c.HTTP.TLS.Cert != "" && + c.HTTP.TLS.CACert != "" && + !c.IsBasicAuthnEnabled() && + !c.HTTP.AccessControl.AnonymousPolicyExists() { + return true + } + + return false +} + func (c *Config) IsHtpasswdAuthEnabled() bool { if c.HTTP.Auth != nil && c.HTTP.Auth.HTPasswd.Path != "" { return true diff --git a/pkg/api/constants/consts.go b/pkg/api/constants/consts.go index a15b1bd9..93202eff 100644 --- a/pkg/api/constants/consts.go +++ b/pkg/api/constants/consts.go @@ -23,4 +23,12 @@ const ( APIKeysPrefix = "zak_" CallbackUIQueryParam = "callback_ui" APIKeyTimeFormat = time.RFC3339 + // authz permissions. + // method actions. + CreatePermission = "create" + ReadPermission = "read" + UpdatePermission = "update" + DeletePermission = "delete" + // behaviour actions. + DetectManifestCollisionPermission = "detectManifestCollision" ) diff --git a/pkg/api/controller.go b/pkg/api/controller.go index ebc9c202..b424457f 100644 --- a/pkg/api/controller.go +++ b/pkg/api/controller.go @@ -182,7 +182,7 @@ func (c *Controller) Run(reloadCtx context.Context) error { if c.Config.HTTP.TLS.CACert != "" { clientAuth := tls.VerifyClientCertIfGiven - if !c.Config.IsBasicAuthnEnabled() && !c.Config.HTTP.AccessControl.AnonymousPolicyExists() { + if c.Config.IsMTLSAuthEnabled() { clientAuth = tls.RequireAndVerifyClientCert } diff --git a/pkg/api/controller_test.go b/pkg/api/controller_test.go index c717759c..3d03ca82 100644 --- a/pkg/api/controller_test.go +++ b/pkg/api/controller_test.go @@ -1449,7 +1449,7 @@ func TestTLSWithBasicAuthAllowReadAccess(t *testing.T) { // without creds, writes should fail resp, err = resty.R().Post(secureBaseURL + "/v2/repo/blobs/uploads/") So(err, ShouldBeNil) - So(resp.StatusCode(), ShouldEqual, http.StatusForbidden) + So(resp.StatusCode(), ShouldEqual, http.StatusUnauthorized) }) } @@ -1552,8 +1552,6 @@ func TestMutualTLSAuthWithoutCN(t *testing.T) { So(err, ShouldBeNil) caCertPool := x509.NewCertPool() caCertPool.AppendCertsFromPEM(caCert) - htpasswdPath := test.MakeHtpasswdFile() - defer os.Remove(htpasswdPath) port := test.GetFreePort() secureBaseURL := test.GetSecureBaseURL(port) @@ -1721,7 +1719,7 @@ func TestTLSMutualAuthAllowReadAccess(t *testing.T) { // without creds, writes should fail resp, err = resty.R().Post(secureBaseURL + "/v2/repo/blobs/uploads/") So(err, ShouldBeNil) - So(resp.StatusCode(), ShouldEqual, http.StatusForbidden) + So(resp.StatusCode(), ShouldEqual, http.StatusUnauthorized) // setup TLS mutual auth cert, err := tls.LoadX509KeyPair("../../test/data/client.cert", "../../test/data/client.key") @@ -1899,7 +1897,7 @@ func TestTLSMutualAndBasicAuthAllowReadAccess(t *testing.T) { // with only client certs, writes should fail resp, err = resty.R().Post(secureBaseURL + "/v2/repo/blobs/uploads/") So(err, ShouldBeNil) - So(resp.StatusCode(), ShouldEqual, http.StatusForbidden) + So(resp.StatusCode(), ShouldEqual, http.StatusUnauthorized) // with client certs and creds, should get expected status code resp, _ = resty.R().SetBasicAuth(username, passphrase).Get(secureBaseURL) @@ -3755,11 +3753,11 @@ func TestAuthorizationWithOnlyAnonymousPolicy(t *testing.T) { err = json.Unmarshal(resp.Body(), &e) So(err, ShouldBeNil) - // should get 403 without create + // should get 401 without create resp, err = resty.R().Post(baseURL + "/v2/" + TestRepo + "/blobs/uploads/") So(err, ShouldBeNil) So(resp, ShouldNotBeNil) - So(resp.StatusCode(), ShouldEqual, http.StatusForbidden) + So(resp.StatusCode(), ShouldEqual, http.StatusUnauthorized) if entry, ok := conf.HTTP.AccessControl.Repositories[TestRepo]; ok { entry.AnonymousPolicy = []string{"create", "read"} @@ -3864,12 +3862,12 @@ func TestAuthorizationWithOnlyAnonymousPolicy(t *testing.T) { updatedManifestBlob, err := json.Marshal(updatedManifest) So(err, ShouldBeNil) - // update manifest should get 403 without update perm + // update manifest should get 401 without update perm resp, err = resty.R().SetBody(updatedManifestBlob). Put(baseURL + "/v2/" + TestRepo + "/manifests/0.0.2") So(err, ShouldBeNil) So(resp, ShouldNotBeNil) - So(resp.StatusCode(), ShouldEqual, http.StatusForbidden) + So(resp.StatusCode(), ShouldEqual, http.StatusUnauthorized) // get the manifest and check if it's the old one resp, err = resty.R(). @@ -6928,7 +6926,12 @@ func TestManifestCollision(t *testing.T) { conf.HTTP.AccessControl = &config.AccessControlConfig{ Repositories: config.Repositories{ AuthorizationAllRepos: config.PolicyGroup{ - AnonymousPolicy: []string{api.Read, api.Create, api.Delete, api.DetectManifestCollision}, + AnonymousPolicy: []string{ + constants.ReadPermission, + constants.CreatePermission, + constants.DeletePermission, + constants.DetectManifestCollisionPermission, + }, }, }, } diff --git a/pkg/api/routes.go b/pkg/api/routes.go index 0ccafcdf..d43ecf63 100644 --- a/pkg/api/routes.go +++ b/pkg/api/routes.go @@ -45,7 +45,7 @@ import ( "zotregistry.io/zot/pkg/meta" mTypes "zotregistry.io/zot/pkg/meta/types" zreg "zotregistry.io/zot/pkg/regexp" - localCtx "zotregistry.io/zot/pkg/requestcontext" + reqCtx "zotregistry.io/zot/pkg/requestcontext" storageCommon "zotregistry.io/zot/pkg/storage/common" storageTypes "zotregistry.io/zot/pkg/storage/types" "zotregistry.io/zot/pkg/test/inject" @@ -777,8 +777,8 @@ func (rh *RouteHandler) DeleteManifest(response http.ResponseWriter, request *ht return } - // authz request context (set in authz middleware) - acCtx, err := localCtx.GetAccessControlContext(request.Context()) + // user authz request context (set in authz middleware) + userAc, err := reqCtx.UserAcFromContext(request.Context()) if err != nil { response.WriteHeader(http.StatusInternalServerError) @@ -786,8 +786,8 @@ func (rh *RouteHandler) DeleteManifest(response http.ResponseWriter, request *ht } var detectCollision bool - if acCtx != nil { - detectCollision = acCtx.CanDetectManifestCollision(name) + if userAc != nil { + detectCollision = userAc.Can(constants.DetectManifestCollisionPermission, name) } manifestBlob, manifestDigest, mediaType, err := imgStore.GetImageManifest(name, reference) @@ -1151,6 +1151,7 @@ func (rh *RouteHandler) DeleteBlob(response http.ResponseWriter, request *http.R // @Success 202 {string} string "accepted" // @Header 202 {string} Location "/v2/{name}/blobs/uploads/{session_id}" // @Header 202 {string} Range "0-0" +// @Failure 401 {string} string "unauthorized" // @Failure 404 {string} string "not found" // @Failure 500 {string} string "internal server error" // @Router /v2/{name}/blobs/uploads [post]. @@ -1711,16 +1712,16 @@ func (rh *RouteHandler) ListRepositories(response http.ResponseWriter, request * repos := make([]string, 0) // authz context - acCtx, err := localCtx.GetAccessControlContext(request.Context()) + userAc, err := reqCtx.UserAcFromContext(request.Context()) if err != nil { response.WriteHeader(http.StatusInternalServerError) return } - if acCtx != nil { + if userAc != nil { for _, r := range combineRepoList { - if acCtx.IsAdmin || acCtx.CanReadRepo(r) { + if userAc.Can(constants.ReadPermission, r) { repos = append(repos, r) } } diff --git a/pkg/api/routes_test.go b/pkg/api/routes_test.go index 918f3272..d42a51e9 100644 --- a/pkg/api/routes_test.go +++ b/pkg/api/routes_test.go @@ -29,7 +29,7 @@ import ( "zotregistry.io/zot/pkg/api/constants" "zotregistry.io/zot/pkg/log" mTypes "zotregistry.io/zot/pkg/meta/types" - localCtx "zotregistry.io/zot/pkg/requestcontext" + reqCtx "zotregistry.io/zot/pkg/requestcontext" storageTypes "zotregistry.io/zot/pkg/storage/types" "zotregistry.io/zot/pkg/test" "zotregistry.io/zot/pkg/test/mocks" @@ -141,9 +141,8 @@ func TestRoutes(t *testing.T) { Convey("List repositories authz error", func() { var invalid struct{} - ctx := context.TODO() - key := localCtx.GetContextKey() - ctx = context.WithValue(ctx, key, invalid) + uacKey := reqCtx.GetContextKey() + ctx := context.WithValue(context.Background(), uacKey, invalid) request, _ := http.NewRequestWithContext(ctx, http.MethodGet, baseURL, nil) request = mux.SetURLVars(request, map[string]string{ @@ -164,9 +163,8 @@ func TestRoutes(t *testing.T) { Convey("Delete manifest authz error", func() { var invalid struct{} - ctx := context.TODO() - key := localCtx.GetContextKey() - ctx = context.WithValue(ctx, key, invalid) + uacKey := reqCtx.GetContextKey() + ctx := context.WithValue(context.Background(), uacKey, invalid) request, _ := http.NewRequestWithContext(ctx, http.MethodGet, baseURL, nil) request = mux.SetURLVars(request, map[string]string{ @@ -1419,9 +1417,8 @@ func TestRoutes(t *testing.T) { Convey("CreateAPIKey invalid access control context", func() { var invalid struct{} - ctx := context.TODO() - key := localCtx.GetContextKey() - ctx = context.WithValue(ctx, key, invalid) + uacKey := reqCtx.GetContextKey() + ctx := context.WithValue(context.Background(), uacKey, invalid) request, _ := http.NewRequestWithContext(ctx, http.MethodPost, baseURL, bytes.NewReader([]byte{})) response := httptest.NewRecorder() @@ -1443,13 +1440,9 @@ func TestRoutes(t *testing.T) { }) Convey("CreateAPIKey bad request body", func() { - acCtx := localCtx.AccessControlContext{ - Username: "test", - } - - ctx := context.TODO() - key := localCtx.GetContextKey() - ctx = context.WithValue(ctx, key, acCtx) + userAc := reqCtx.NewUserAccessControl() + userAc.SetUsername("test") + ctx := userAc.DeriveContext(context.Background()) request, _ := http.NewRequestWithContext(ctx, http.MethodPost, baseURL, bytes.NewReader([]byte{})) response := httptest.NewRecorder() @@ -1462,13 +1455,9 @@ func TestRoutes(t *testing.T) { }) Convey("CreateAPIKey error on AddUserAPIKey", func() { - acCtx := localCtx.AccessControlContext{ - Username: "test", - } - - ctx := context.TODO() - key := localCtx.GetContextKey() - ctx = context.WithValue(ctx, key, acCtx) + userAc := reqCtx.NewUserAccessControl() + userAc.SetUsername("test") + ctx := userAc.DeriveContext(context.Background()) payload := api.APIKeyPayload{ Label: "test", @@ -1494,13 +1483,9 @@ func TestRoutes(t *testing.T) { }) Convey("Revoke error on DeleteUserAPIKeyFn", func() { - acCtx := localCtx.AccessControlContext{ - Username: "test", - } - - ctx := context.TODO() - key := localCtx.GetContextKey() - ctx = context.WithValue(ctx, key, acCtx) + userAc := reqCtx.NewUserAccessControl() + userAc.SetUsername("test") + ctx := userAc.DeriveContext(context.Background()) request, _ := http.NewRequestWithContext(ctx, http.MethodDelete, baseURL, bytes.NewReader([]byte{})) response := httptest.NewRecorder() diff --git a/pkg/common/http_server.go b/pkg/common/http_server.go index c09d2911..254aa170 100644 --- a/pkg/common/http_server.go +++ b/pkg/common/http_server.go @@ -13,7 +13,7 @@ import ( "zotregistry.io/zot/pkg/api/config" "zotregistry.io/zot/pkg/api/constants" apiErr "zotregistry.io/zot/pkg/api/errors" - localCtx "zotregistry.io/zot/pkg/requestcontext" + reqCtx "zotregistry.io/zot/pkg/requestcontext" ) func AllowedMethods(methods ...string) []string { @@ -79,17 +79,17 @@ func AuthzOnlyAdminsMiddleware(conf *config.Config) mux.MiddlewareFunc { return } - // get acCtx built in previous authn/authz middlewares - acCtx, err := localCtx.GetAccessControlContext(request.Context()) + // get userAccessControl built in previous authn/authz middlewares + userAc, err := reqCtx.UserAcFromContext(request.Context()) if err != nil { // should not happen as this has been previously checked for errors - AuthzFail(response, request, conf.HTTP.Realm, conf.HTTP.Auth.FailDelay) + AuthzFail(response, request, userAc.GetUsername(), conf.HTTP.Realm, conf.HTTP.Auth.FailDelay) return } // reject non-admin access if authentication is enabled - if acCtx != nil && !acCtx.IsAdmin { - AuthzFail(response, request, conf.HTTP.Realm, conf.HTTP.Auth.FailDelay) + if userAc != nil && !userAc.IsAdmin() { + AuthzFail(response, request, userAc.GetUsername(), conf.HTTP.Realm, conf.HTTP.Auth.FailDelay) return } @@ -99,7 +99,7 @@ func AuthzOnlyAdminsMiddleware(conf *config.Config) mux.MiddlewareFunc { } } -func AuthzFail(w http.ResponseWriter, r *http.Request, realm string, delay int) { +func AuthzFail(w http.ResponseWriter, r *http.Request, identity, realm string, delay int) { time.Sleep(time.Duration(delay) * time.Second) // don't send auth headers if request is coming from UI @@ -114,7 +114,12 @@ func AuthzFail(w http.ResponseWriter, r *http.Request, realm string, delay int) } w.Header().Set("Content-Type", "application/json") - WriteJSON(w, http.StatusForbidden, apiErr.NewErrorList(apiErr.NewError(apiErr.DENIED))) + + if identity == "" { + WriteJSON(w, http.StatusUnauthorized, apiErr.NewErrorList(apiErr.NewError(apiErr.UNAUTHORIZED))) + } else { + WriteJSON(w, http.StatusForbidden, apiErr.NewErrorList(apiErr.NewError(apiErr.DENIED))) + } } func WriteJSON(response http.ResponseWriter, status int, data interface{}) { diff --git a/pkg/extensions/search/resolver.go b/pkg/extensions/search/resolver.go index ebeb3b59..94850de9 100644 --- a/pkg/extensions/search/resolver.go +++ b/pkg/extensions/search/resolver.go @@ -26,7 +26,7 @@ import ( "zotregistry.io/zot/pkg/extensions/search/pagination" "zotregistry.io/zot/pkg/log" mTypes "zotregistry.io/zot/pkg/meta/types" - localCtx "zotregistry.io/zot/pkg/requestcontext" + reqCtx "zotregistry.io/zot/pkg/requestcontext" "zotregistry.io/zot/pkg/storage" ) @@ -1108,7 +1108,7 @@ func deleteElementAt(slice []*string, i int) []*string { func expandedRepoInfo(ctx context.Context, repo string, metaDB mTypes.MetaDB, cveInfo cveinfo.CveInfo, log log.Logger, ) (*gql_generated.RepoInfo, error) { - if ok, err := localCtx.RepoIsUserAvailable(ctx, repo); !ok || err != nil { + if ok, err := reqCtx.RepoIsUserAvailable(ctx, repo); !ok || err != nil { log.Info().Err(err).Str("repository", repo).Bool("availability", ok).Msg("resolver: repo user availability") return &gql_generated.RepoInfo{}, nil //nolint:nilerr // don't give details to a potential attacker diff --git a/pkg/extensions/search/resolver_test.go b/pkg/extensions/search/resolver_test.go index 47684947..b052c387 100644 --- a/pkg/extensions/search/resolver_test.go +++ b/pkg/extensions/search/resolver_test.go @@ -22,7 +22,7 @@ import ( "zotregistry.io/zot/pkg/log" "zotregistry.io/zot/pkg/meta/boltdb" mTypes "zotregistry.io/zot/pkg/meta/types" - localCtx "zotregistry.io/zot/pkg/requestcontext" + reqCtx "zotregistry.io/zot/pkg/requestcontext" "zotregistry.io/zot/pkg/storage" "zotregistry.io/zot/pkg/test/mocks" ) @@ -3584,15 +3584,13 @@ func TestExpandedRepoInfo(t *testing.T) { }) Convey("Access error", t, func() { - authzCtxKey := localCtx.GetContextKey() - acCtxUser := localCtx.AccessControlContext{ - ReadGlobPatterns: map[string]bool{ - "repo": false, - }, - Username: "user", - } + userAc := reqCtx.NewUserAccessControl() + userAc.SetUsername("user") + userAc.SetGlobPatterns("read", map[string]bool{ + "repo": false, + }) - ctx := context.WithValue(context.Background(), authzCtxKey, acCtxUser) + ctx := userAc.DeriveContext(context.Background()) responseContext := graphql.WithResponseContext(ctx, graphql.DefaultErrorPresenter, graphql.DefaultRecover) diff --git a/pkg/meta/boltdb/boltdb.go b/pkg/meta/boltdb/boltdb.go index bc79da21..83b1043c 100644 --- a/pkg/meta/boltdb/boltdb.go +++ b/pkg/meta/boltdb/boltdb.go @@ -13,13 +13,14 @@ import ( "go.etcd.io/bbolt" zerr "zotregistry.io/zot/errors" + "zotregistry.io/zot/pkg/api/constants" zcommon "zotregistry.io/zot/pkg/common" "zotregistry.io/zot/pkg/extensions/imagetrust" "zotregistry.io/zot/pkg/log" "zotregistry.io/zot/pkg/meta/common" mTypes "zotregistry.io/zot/pkg/meta/types" "zotregistry.io/zot/pkg/meta/version" - localCtx "zotregistry.io/zot/pkg/requestcontext" + reqCtx "zotregistry.io/zot/pkg/requestcontext" ) type BoltDB struct { @@ -653,7 +654,7 @@ func (bdw *BoltDB) GetMultipleRepoMeta(ctx context.Context, filter func(repoMeta cursor := buck.Cursor() for repoName, repoMetaBlob := cursor.First(); repoName != nil; repoName, repoMetaBlob = cursor.Next() { - if ok, err := localCtx.RepoIsUserAvailable(ctx, string(repoName)); !ok || err != nil { + if ok, err := reqCtx.RepoIsUserAvailable(ctx, string(repoName)); !ok || err != nil { continue } @@ -971,7 +972,7 @@ func (bdw *BoltDB) SearchRepos(ctx context.Context, searchText string, cursor := repoBuck.Cursor() for repoName, repoMetaBlob := cursor.First(); repoName != nil; repoName, repoMetaBlob = cursor.Next() { - if ok, err := localCtx.RepoIsUserAvailable(ctx, string(repoName)); !ok || err != nil { + if ok, err := reqCtx.RepoIsUserAvailable(ctx, string(repoName)); !ok || err != nil { continue } @@ -1141,7 +1142,7 @@ func (bdw *BoltDB) FilterTags(ctx context.Context, filterFunc mTypes.FilterFunc, repoName, repoMetaBlob := cursor.First() for ; repoName != nil; repoName, repoMetaBlob = cursor.Next() { - if ok, err := localCtx.RepoIsUserAvailable(ctx, string(repoName)); !ok || err != nil { + if ok, err := reqCtx.RepoIsUserAvailable(ctx, string(repoName)); !ok || err != nil { continue } @@ -1251,7 +1252,7 @@ func (bdw *BoltDB) FilterRepos(ctx context.Context, filter mTypes.FilterRepoFunc ) for repoName, repoMetaBlob := cursor.First(); repoName != nil; repoName, repoMetaBlob = cursor.Next() { - if ok, err := localCtx.RepoIsUserAvailable(ctx, string(repoName)); !ok || err != nil { + if ok, err := reqCtx.RepoIsUserAvailable(ctx, string(repoName)); !ok || err != nil { continue } @@ -1310,7 +1311,7 @@ func (bdw *BoltDB) SearchTags(ctx context.Context, searchText string, return nil } - if ok, err := localCtx.RepoIsUserAvailable(ctx, string(repoName)); !ok || err != nil { + if ok, err := reqCtx.RepoIsUserAvailable(ctx, string(repoName)); !ok || err != nil { return err } @@ -1396,21 +1397,16 @@ func (bdw *BoltDB) SearchTags(ctx context.Context, searchText string, } func (bdw *BoltDB) ToggleStarRepo(ctx context.Context, repo string) (mTypes.ToggleState, error) { - acCtx, err := localCtx.GetAccessControlContext(ctx) + userAc, err := reqCtx.UserAcFromContext(ctx) if err != nil { return mTypes.NotChanged, err } - userid := localCtx.GetUsernameFromContext(acCtx) - - if userid == "" { - // empty user is anonymous + if userAc.IsAnonymous() || !userAc.Can(constants.ReadPermission, repo) { return mTypes.NotChanged, zerr.ErrUserDataNotAllowed } - if ok, err := localCtx.RepoIsUserAvailable(ctx, repo); !ok || err != nil { - return mTypes.NotChanged, zerr.ErrUserDataNotAllowed - } + userid := userAc.GetUsername() var res mTypes.ToggleState @@ -1486,20 +1482,16 @@ func (bdw *BoltDB) GetStarredRepos(ctx context.Context) ([]string, error) { } func (bdw *BoltDB) ToggleBookmarkRepo(ctx context.Context, repo string) (mTypes.ToggleState, error) { - acCtx, err := localCtx.GetAccessControlContext(ctx) + userAc, err := reqCtx.UserAcFromContext(ctx) if err != nil { return mTypes.NotChanged, err } - userid := localCtx.GetUsernameFromContext(acCtx) - if userid == "" { - // empty user is anonymous + if userAc.IsAnonymous() || !userAc.Can(constants.ReadPermission, repo) { return mTypes.NotChanged, zerr.ErrUserDataNotAllowed } - if ok, err := localCtx.RepoIsUserAvailable(ctx, repo); !ok || err != nil { - return mTypes.NotChanged, zerr.ErrUserDataNotAllowed - } + userid := userAc.GetUsername() var res mTypes.ToggleState @@ -1570,14 +1562,14 @@ func (bdw *BoltDB) PatchDB() error { } func getUserStars(ctx context.Context, transaction *bbolt.Tx) []string { - acCtx, err := localCtx.GetAccessControlContext(ctx) + userAc, err := reqCtx.UserAcFromContext(ctx) if err != nil { return []string{} } var ( userData mTypes.UserData - userid = localCtx.GetUsernameFromContext(acCtx) + userid = userAc.GetUsername() userdb = transaction.Bucket([]byte(UserDataBucket)) ) @@ -1598,14 +1590,14 @@ func getUserStars(ctx context.Context, transaction *bbolt.Tx) []string { } func getUserBookmarks(ctx context.Context, transaction *bbolt.Tx) []string { - acCtx, err := localCtx.GetAccessControlContext(ctx) + userAc, err := reqCtx.UserAcFromContext(ctx) if err != nil { return []string{} } var ( userData mTypes.UserData - userid = localCtx.GetUsernameFromContext(acCtx) + userid = userAc.GetUsername() userdb = transaction.Bucket([]byte(UserDataBucket)) ) @@ -1626,18 +1618,17 @@ func getUserBookmarks(ctx context.Context, transaction *bbolt.Tx) []string { } func (bdw *BoltDB) SetUserGroups(ctx context.Context, groups []string) error { - acCtx, err := localCtx.GetAccessControlContext(ctx) + userAc, err := reqCtx.UserAcFromContext(ctx) if err != nil { return err } - userid := localCtx.GetUsernameFromContext(acCtx) - - if userid == "" { - // empty user is anonymous + if userAc.IsAnonymous() { return zerr.ErrUserDataNotAllowed } + userid := userAc.GetUsername() + err = bdw.DB.Update(func(tx *bbolt.Tx) error { //nolint:varnamelen var userData mTypes.UserData @@ -1663,18 +1654,17 @@ func (bdw *BoltDB) GetUserGroups(ctx context.Context) ([]string, error) { } func (bdw *BoltDB) UpdateUserAPIKeyLastUsed(ctx context.Context, hashedKey string) error { - acCtx, err := localCtx.GetAccessControlContext(ctx) + userAc, err := reqCtx.UserAcFromContext(ctx) if err != nil { return err } - userid := localCtx.GetUsernameFromContext(acCtx) - - if userid == "" { - // empty user is anonymous + if userAc.IsAnonymous() { return zerr.ErrUserDataNotAllowed } + userid := userAc.GetUsername() + err = bdw.DB.Update(func(tx *bbolt.Tx) error { //nolint:varnamelen var userData mTypes.UserData @@ -1697,18 +1687,17 @@ func (bdw *BoltDB) UpdateUserAPIKeyLastUsed(ctx context.Context, hashedKey strin } func (bdw *BoltDB) IsAPIKeyExpired(ctx context.Context, hashedKey string) (bool, error) { - acCtx, err := localCtx.GetAccessControlContext(ctx) + userAc, err := reqCtx.UserAcFromContext(ctx) if err != nil { return false, err } - userid := localCtx.GetUsernameFromContext(acCtx) - - if userid == "" { - // empty user is anonymous + if userAc.IsAnonymous() { return false, zerr.ErrUserDataNotAllowed } + userid := userAc.GetUsername() + var isExpired bool err = bdw.DB.Update(func(tx *bbolt.Tx) error { //nolint:varnamelen @@ -1745,17 +1734,17 @@ func (bdw *BoltDB) IsAPIKeyExpired(ctx context.Context, hashedKey string) (bool, func (bdw *BoltDB) GetUserAPIKeys(ctx context.Context) ([]mTypes.APIKeyDetails, error) { apiKeys := make([]mTypes.APIKeyDetails, 0) - acCtx, err := localCtx.GetAccessControlContext(ctx) + userAc, err := reqCtx.UserAcFromContext(ctx) if err != nil { return nil, err } - userid := localCtx.GetUsernameFromContext(acCtx) - if userid == "" { - // empty user is anonymous + if userAc.IsAnonymous() { return nil, zerr.ErrUserDataNotAllowed } + userid := userAc.GetUsername() + err = bdw.DB.Update(func(transaction *bbolt.Tx) error { var userData mTypes.UserData @@ -1787,17 +1776,17 @@ func (bdw *BoltDB) GetUserAPIKeys(ctx context.Context) ([]mTypes.APIKeyDetails, } func (bdw *BoltDB) AddUserAPIKey(ctx context.Context, hashedKey string, apiKeyDetails *mTypes.APIKeyDetails) error { - acCtx, err := localCtx.GetAccessControlContext(ctx) + userAc, err := reqCtx.UserAcFromContext(ctx) if err != nil { return err } - userid := localCtx.GetUsernameFromContext(acCtx) - if userid == "" { - // empty user is anonymous + if userAc.IsAnonymous() { return zerr.ErrUserDataNotAllowed } + userid := userAc.GetUsername() + err = bdw.DB.Update(func(transaction *bbolt.Tx) error { var userData mTypes.UserData @@ -1831,17 +1820,17 @@ func (bdw *BoltDB) AddUserAPIKey(ctx context.Context, hashedKey string, apiKeyDe } func (bdw *BoltDB) DeleteUserAPIKey(ctx context.Context, keyID string) error { - acCtx, err := localCtx.GetAccessControlContext(ctx) + userAc, err := reqCtx.UserAcFromContext(ctx) if err != nil { return err } - userid := localCtx.GetUsernameFromContext(acCtx) - if userid == "" { - // empty user is anonymous + if userAc.IsAnonymous() { return zerr.ErrUserDataNotAllowed } + userid := userAc.GetUsername() + err = bdw.DB.Update(func(transaction *bbolt.Tx) error { var userData mTypes.UserData @@ -1896,17 +1885,17 @@ func (bdw *BoltDB) GetUserAPIKeyInfo(hashedKey string) (string, error) { func (bdw *BoltDB) GetUserData(ctx context.Context) (mTypes.UserData, error) { var userData mTypes.UserData - acCtx, err := localCtx.GetAccessControlContext(ctx) + userAc, err := reqCtx.UserAcFromContext(ctx) if err != nil { return userData, err } - userid := localCtx.GetUsernameFromContext(acCtx) - if userid == "" { - // empty user is anonymous + if userAc.IsAnonymous() { return userData, zerr.ErrUserDataNotAllowed } + userid := userAc.GetUsername() + err = bdw.DB.View(func(tx *bbolt.Tx) error { return bdw.getUserData(userid, tx, &userData) }) @@ -1935,17 +1924,17 @@ func (bdw *BoltDB) getUserData(userid string, transaction *bbolt.Tx, res *mTypes } func (bdw *BoltDB) SetUserData(ctx context.Context, userData mTypes.UserData) error { - acCtx, err := localCtx.GetAccessControlContext(ctx) + userAc, err := reqCtx.UserAcFromContext(ctx) if err != nil { return err } - userid := localCtx.GetUsernameFromContext(acCtx) - if userid == "" { - // empty user is anonymous + if userAc.IsAnonymous() { return zerr.ErrUserDataNotAllowed } + userid := userAc.GetUsername() + err = bdw.DB.Update(func(tx *bbolt.Tx) error { return bdw.setUserData(userid, tx, userData) }) @@ -1973,17 +1962,17 @@ func (bdw *BoltDB) setUserData(userid string, transaction *bbolt.Tx, userData mT } func (bdw *BoltDB) DeleteUserData(ctx context.Context) error { - acCtx, err := localCtx.GetAccessControlContext(ctx) + userAc, err := reqCtx.UserAcFromContext(ctx) if err != nil { return err } - userid := localCtx.GetUsernameFromContext(acCtx) - if userid == "" { - // empty user is anonymous + if userAc.IsAnonymous() { return zerr.ErrUserDataNotAllowed } + userid := userAc.GetUsername() + err = bdw.DB.Update(func(tx *bbolt.Tx) error { buck := tx.Bucket([]byte(UserDataBucket)) if buck == nil { diff --git a/pkg/meta/boltdb/boltdb_test.go b/pkg/meta/boltdb/boltdb_test.go index 1882322f..d0125b09 100644 --- a/pkg/meta/boltdb/boltdb_test.go +++ b/pkg/meta/boltdb/boltdb_test.go @@ -18,7 +18,7 @@ import ( "zotregistry.io/zot/pkg/log" "zotregistry.io/zot/pkg/meta/boltdb" mTypes "zotregistry.io/zot/pkg/meta/types" - localCtx "zotregistry.io/zot/pkg/requestcontext" + reqCtx "zotregistry.io/zot/pkg/requestcontext" "zotregistry.io/zot/pkg/test" ) @@ -43,21 +43,16 @@ func TestWrapperErrors(t *testing.T) { repoMetaBlob, err := json.Marshal(repoMeta) So(err, ShouldBeNil) - authzCtxKey := localCtx.GetContextKey() + userAc := reqCtx.NewUserAccessControl() + userAc.SetUsername("test") - acCtx := localCtx.AccessControlContext{ - Username: "test", - } - - ctx := context.WithValue(context.Background(), authzCtxKey, acCtx) + ctx := userAc.DeriveContext(context.Background()) Convey("AddUserAPIKey", func() { Convey("no userid found", func() { - acCtx := localCtx.AccessControlContext{ - Username: "", - } + userAc := reqCtx.NewUserAccessControl() + ctx := userAc.DeriveContext(context.Background()) - ctx := context.WithValue(context.Background(), authzCtxKey, acCtx) err = boltdbWrapper.AddUserAPIKey(ctx, "", &mTypes.APIKeyDetails{}) So(err, ShouldNotBeNil) }) @@ -87,11 +82,9 @@ func TestWrapperErrors(t *testing.T) { err = boltdbWrapper.UpdateUserAPIKeyLastUsed(ctx, "") So(err, ShouldNotBeNil) - acCtx := localCtx.AccessControlContext{ - Username: "", - } + userAc := reqCtx.NewUserAccessControl() + ctx := userAc.DeriveContext(context.Background()) - ctx := context.WithValue(context.Background(), authzCtxKey, acCtx) err = boltdbWrapper.UpdateUserAPIKeyLastUsed(ctx, "") //nolint: contextcheck So(err, ShouldNotBeNil) }) @@ -109,25 +102,19 @@ func TestWrapperErrors(t *testing.T) { }) So(err, ShouldBeNil) - authzCtxKey := localCtx.GetContextKey() - - acCtx := localCtx.AccessControlContext{ - Username: "test", - } - - ctx := context.WithValue(context.Background(), authzCtxKey, acCtx) + userAc := reqCtx.NewUserAccessControl() + userAc.SetUsername("test") + ctx := userAc.DeriveContext(context.Background()) err = boltdbWrapper.DeleteUserAPIKey(ctx, "") So(err, ShouldEqual, zerr.ErrBucketDoesNotExist) }) Convey("userdata not found", func() { - authzCtxKey := localCtx.GetContextKey() - acCtx := localCtx.AccessControlContext{ - Username: "test", - } + userAc := reqCtx.NewUserAccessControl() + userAc.SetUsername("test") + ctx := userAc.DeriveContext(context.Background()) - ctx := context.WithValue(context.Background(), authzCtxKey, acCtx) err := boltdbWrapper.DeleteUserData(ctx) So(err, ShouldBeNil) @@ -135,13 +122,8 @@ func TestWrapperErrors(t *testing.T) { So(err, ShouldNotBeNil) }) - authzCtxKey := localCtx.GetContextKey() - - acCtx := localCtx.AccessControlContext{ - Username: "", - } - - ctx := context.WithValue(context.Background(), authzCtxKey, acCtx) //nolint: contextcheck + userAc := reqCtx.NewUserAccessControl() + ctx := userAc.DeriveContext(context.Background()) err = boltdbWrapper.DeleteUserAPIKey(ctx, "test") //nolint: contextcheck So(err, ShouldNotBeNil) @@ -188,11 +170,8 @@ func TestWrapperErrors(t *testing.T) { }) Convey("SetUserData", func() { - acCtx = localCtx.AccessControlContext{ - Username: "", - } - - ctx = context.WithValue(context.Background(), authzCtxKey, acCtx) + userAc := reqCtx.NewUserAccessControl() + ctx := userAc.DeriveContext(context.Background()) err = boltdbWrapper.SetUserData(ctx, mTypes.UserData{}) So(err, ShouldNotBeNil) @@ -203,13 +182,9 @@ func TestWrapperErrors(t *testing.T) { longString := base64.RawURLEncoding.EncodeToString(buff) - authzCtxKey := localCtx.GetContextKey() - - acCtx := localCtx.AccessControlContext{ - Username: longString, - } - - ctx := context.WithValue(context.Background(), authzCtxKey, acCtx) + userAc = reqCtx.NewUserAccessControl() + userAc.SetUsername(longString) + ctx = userAc.DeriveContext(context.Background()) err = boltdbWrapper.SetUserData(ctx, mTypes.UserData{}) //nolint: contextcheck So(err, ShouldNotBeNil) @@ -219,22 +194,17 @@ func TestWrapperErrors(t *testing.T) { }) So(err, ShouldBeNil) - acCtx = localCtx.AccessControlContext{ - Username: "test", - } - - ctx = context.WithValue(context.Background(), authzCtxKey, acCtx) + userAc = reqCtx.NewUserAccessControl() + userAc.SetUsername("test") + ctx = userAc.DeriveContext(context.Background()) err = boltdbWrapper.SetUserData(ctx, mTypes.UserData{}) //nolint: contextcheck So(err, ShouldNotBeNil) }) Convey("DeleteUserData", func() { - acCtx = localCtx.AccessControlContext{ - Username: "", - } - - ctx = context.WithValue(context.Background(), authzCtxKey, acCtx) + userAc = reqCtx.NewUserAccessControl() + ctx = userAc.DeriveContext(context.Background()) err = boltdbWrapper.DeleteUserData(ctx) So(err, ShouldNotBeNil) @@ -244,22 +214,17 @@ func TestWrapperErrors(t *testing.T) { }) So(err, ShouldBeNil) - acCtx = localCtx.AccessControlContext{ - Username: "test", - } - - ctx = context.WithValue(context.Background(), authzCtxKey, acCtx) + userAc = reqCtx.NewUserAccessControl() + userAc.SetUsername("test") + ctx = userAc.DeriveContext(context.Background()) err = boltdbWrapper.DeleteUserData(ctx) So(err, ShouldNotBeNil) }) Convey("GetUserGroups and SetUserGroups", func() { - acCtx = localCtx.AccessControlContext{ - Username: "", - } - - ctx = context.WithValue(context.Background(), authzCtxKey, acCtx) + userAc = reqCtx.NewUserAccessControl() + ctx = userAc.DeriveContext(context.Background()) _, err := boltdbWrapper.GetUserGroups(ctx) So(err, ShouldNotBeNil) @@ -867,22 +832,20 @@ func TestWrapperErrors(t *testing.T) { }) Convey("ToggleStarRepo bad context errors", func() { - authzCtxKey := localCtx.GetContextKey() - ctx := context.WithValue(context.Background(), authzCtxKey, "bad context") + uacKey := reqCtx.GetContextKey() + ctx := context.WithValue(context.Background(), uacKey, "bad context") _, err := boltdbWrapper.ToggleStarRepo(ctx, "repo") So(err, ShouldNotBeNil) }) Convey("ToggleStarRepo, no repoMeta found", func() { - acCtx := localCtx.AccessControlContext{ - ReadGlobPatterns: map[string]bool{ - "repo": true, - }, - Username: "username", - } - authzCtxKey := localCtx.GetContextKey() - ctx := context.WithValue(context.Background(), authzCtxKey, acCtx) + userAc := reqCtx.NewUserAccessControl() + userAc.SetUsername("username") + userAc.SetGlobPatterns("read", map[string]bool{ + "repo": true, + }) + ctx := userAc.DeriveContext(context.Background()) err := boltdbWrapper.DB.Update(func(tx *bbolt.Tx) error { repoBuck := tx.Bucket([]byte(boltdb.RepoMetadataBucket)) @@ -899,38 +862,36 @@ func TestWrapperErrors(t *testing.T) { }) Convey("ToggleStarRepo, bad repoMeta found", func() { - acCtx := localCtx.AccessControlContext{ - ReadGlobPatterns: map[string]bool{ - "repo": true, - }, - Username: "username", - } - authzCtxKey := localCtx.GetContextKey() - ctx := context.WithValue(context.Background(), authzCtxKey, acCtx) + userAc := reqCtx.NewUserAccessControl() + userAc.SetUsername("username") + userAc.SetGlobPatterns("read", map[string]bool{ + "repo": true, + }) + ctx := userAc.DeriveContext(context.Background()) _, err = boltdbWrapper.ToggleStarRepo(ctx, "repo") So(err, ShouldNotBeNil) }) Convey("ToggleBookmarkRepo bad context errors", func() { - authzCtxKey := localCtx.GetContextKey() - ctx := context.WithValue(context.Background(), authzCtxKey, "bad context") + uacKey := reqCtx.GetContextKey() + ctx := context.WithValue(context.Background(), uacKey, "bad context") _, err := boltdbWrapper.ToggleBookmarkRepo(ctx, "repo") So(err, ShouldNotBeNil) }) Convey("GetUserData bad context errors", func() { - authzCtxKey := localCtx.GetContextKey() - ctx := context.WithValue(context.Background(), authzCtxKey, "bad context") + uacKey := reqCtx.GetContextKey() + ctx := context.WithValue(context.Background(), uacKey, "bad context") _, err := boltdbWrapper.GetUserData(ctx) So(err, ShouldNotBeNil) }) Convey("SetUserData bad context errors", func() { - authzCtxKey := localCtx.GetContextKey() - ctx := context.WithValue(context.Background(), authzCtxKey, "bad context") + uacKey := reqCtx.GetContextKey() + ctx := context.WithValue(context.Background(), uacKey, "bad context") err := boltdbWrapper.SetUserData(ctx, mTypes.UserData{}) So(err, ShouldNotBeNil) @@ -940,64 +901,64 @@ func TestWrapperErrors(t *testing.T) { _, err := boltdbWrapper.GetUserGroups(ctx) So(err, ShouldNotBeNil) - authzCtxKey := localCtx.GetContextKey() - ctx := context.WithValue(context.Background(), authzCtxKey, "bad context") + uacKey := reqCtx.GetContextKey() + ctx := context.WithValue(context.Background(), uacKey, "bad context") _, err = boltdbWrapper.GetUserGroups(ctx) //nolint: contextcheck So(err, ShouldNotBeNil) }) Convey("SetUserGroups bad context errors", func() { - authzCtxKey := localCtx.GetContextKey() - ctx := context.WithValue(context.Background(), authzCtxKey, "bad context") + uacKey := reqCtx.GetContextKey() + ctx := context.WithValue(context.Background(), uacKey, "bad context") err := boltdbWrapper.SetUserGroups(ctx, []string{}) So(err, ShouldNotBeNil) }) Convey("AddUserAPIKey bad context errors", func() { - authzCtxKey := localCtx.GetContextKey() - ctx := context.WithValue(context.Background(), authzCtxKey, "bad context") + uacKey := reqCtx.GetContextKey() + ctx := context.WithValue(context.Background(), uacKey, "bad context") err := boltdbWrapper.AddUserAPIKey(ctx, "", &mTypes.APIKeyDetails{}) So(err, ShouldNotBeNil) }) Convey("DeleteUserAPIKey bad context errors", func() { - authzCtxKey := localCtx.GetContextKey() - ctx := context.WithValue(context.Background(), authzCtxKey, "bad context") + uacKey := reqCtx.GetContextKey() + ctx := context.WithValue(context.Background(), uacKey, "bad context") err := boltdbWrapper.DeleteUserAPIKey(ctx, "") So(err, ShouldNotBeNil) }) Convey("UpdateUserAPIKeyLastUsed bad context errors", func() { - authzCtxKey := localCtx.GetContextKey() - ctx := context.WithValue(context.Background(), authzCtxKey, "bad context") + uacKey := reqCtx.GetContextKey() + ctx := context.WithValue(context.Background(), uacKey, "bad context") err := boltdbWrapper.UpdateUserAPIKeyLastUsed(ctx, "") So(err, ShouldNotBeNil) }) Convey("DeleteUserData bad context errors", func() { - authzCtxKey := localCtx.GetContextKey() - ctx := context.WithValue(context.Background(), authzCtxKey, "bad context") + uacKey := reqCtx.GetContextKey() + ctx := context.WithValue(context.Background(), uacKey, "bad context") err := boltdbWrapper.DeleteUserData(ctx) So(err, ShouldNotBeNil) }) Convey("GetStarredRepos bad context errors", func() { - authzCtxKey := localCtx.GetContextKey() - ctx := context.WithValue(context.Background(), authzCtxKey, "bad context") + uacKey := reqCtx.GetContextKey() + ctx := context.WithValue(context.Background(), uacKey, "bad context") _, err := boltdbWrapper.GetStarredRepos(ctx) So(err, ShouldNotBeNil) }) Convey("GetBookmarkedRepos bad context errors", func() { - authzCtxKey := localCtx.GetContextKey() - ctx := context.WithValue(context.Background(), authzCtxKey, "bad context") + uacKey := reqCtx.GetContextKey() + ctx := context.WithValue(context.Background(), uacKey, "bad context") _, err := boltdbWrapper.GetBookmarkedRepos(ctx) So(err, ShouldNotBeNil) @@ -1021,14 +982,12 @@ func TestWrapperErrors(t *testing.T) { }) Convey("GetUserRepoMeta unmarshal error", func() { - acCtx := localCtx.AccessControlContext{ - ReadGlobPatterns: map[string]bool{ - "repo": true, - }, - Username: "username", - } - authzCtxKey := localCtx.GetContextKey() - ctx := context.WithValue(context.Background(), authzCtxKey, acCtx) + userAc := reqCtx.NewUserAccessControl() + userAc.SetUsername("username") + userAc.SetGlobPatterns("read", map[string]bool{ + "repo": true, + }) + ctx := userAc.DeriveContext(context.Background()) err = boltdbWrapper.DB.Update(func(tx *bbolt.Tx) error { repoBuck := tx.Bucket([]byte(boltdb.RepoMetadataBucket)) diff --git a/pkg/meta/dynamodb/dynamodb.go b/pkg/meta/dynamodb/dynamodb.go index 632c464f..8dac9eb8 100644 --- a/pkg/meta/dynamodb/dynamodb.go +++ b/pkg/meta/dynamodb/dynamodb.go @@ -16,13 +16,14 @@ import ( ispec "github.com/opencontainers/image-spec/specs-go/v1" zerr "zotregistry.io/zot/errors" + "zotregistry.io/zot/pkg/api/constants" zcommon "zotregistry.io/zot/pkg/common" "zotregistry.io/zot/pkg/extensions/imagetrust" "zotregistry.io/zot/pkg/log" "zotregistry.io/zot/pkg/meta/common" mTypes "zotregistry.io/zot/pkg/meta/types" "zotregistry.io/zot/pkg/meta/version" - localCtx "zotregistry.io/zot/pkg/requestcontext" + reqCtx "zotregistry.io/zot/pkg/requestcontext" ) var errMetaDB = errors.New("metadb: error while constructing manifest meta") @@ -811,7 +812,7 @@ func (dwr *DynamoDB) GetMultipleRepoMeta(ctx context.Context, return []mTypes.RepoMetadata{}, err } - if ok, err := localCtx.RepoIsUserAvailable(ctx, repoMeta.Name); !ok || err != nil { + if ok, err := reqCtx.RepoIsUserAvailable(ctx, repoMeta.Name); !ok || err != nil { continue } @@ -855,7 +856,7 @@ func (dwr *DynamoDB) SearchRepos(ctx context.Context, searchText string, err } - if ok, err := localCtx.RepoIsUserAvailable(ctx, repoMeta.Name); !ok || err != nil { + if ok, err := reqCtx.RepoIsUserAvailable(ctx, repoMeta.Name); !ok || err != nil { continue } @@ -1011,7 +1012,7 @@ func (dwr *DynamoDB) FilterTags(ctx context.Context, filterFunc mTypes.FilterFun err } - if ok, err := localCtx.RepoIsUserAvailable(ctx, repoMeta.Name); !ok || err != nil { + if ok, err := reqCtx.RepoIsUserAvailable(ctx, repoMeta.Name); !ok || err != nil { continue } @@ -1133,7 +1134,7 @@ func (dwr *DynamoDB) FilterRepos(ctx context.Context, filter mTypes.FilterRepoFu err } - if ok, err := localCtx.RepoIsUserAvailable(ctx, repoMeta.Name); !ok || err != nil { + if ok, err := reqCtx.RepoIsUserAvailable(ctx, repoMeta.Name); !ok || err != nil { continue } @@ -1187,7 +1188,7 @@ func (dwr *DynamoDB) SearchTags(ctx context.Context, searchText string, err } - if ok, err := localCtx.RepoIsUserAvailable(ctx, repoMeta.Name); !ok || err != nil { + if ok, err := reqCtx.RepoIsUserAvailable(ctx, repoMeta.Name); !ok || err != nil { return []mTypes.RepoMetadata{}, map[string]mTypes.ManifestMetadata{}, map[string]mTypes.IndexData{}, err } @@ -1555,8 +1556,13 @@ func (dwr *DynamoDB) ToggleBookmarkRepo(ctx context.Context, repo string) ( ) { res := mTypes.NotChanged - if ok, err := localCtx.RepoIsUserAvailable(ctx, repo); !ok || err != nil { - return res, zerr.ErrUserDataNotAllowed + userAc, err := reqCtx.UserAcFromContext(ctx) + if err != nil { + return mTypes.NotChanged, err + } + + if userAc.IsAnonymous() || !userAc.Can(constants.ReadPermission, repo) { + return mTypes.NotChanged, zerr.ErrUserDataNotAllowed } userData, err := dwr.GetUserData(ctx) @@ -1604,21 +1610,16 @@ func (dwr *DynamoDB) ToggleStarRepo(ctx context.Context, repo string) ( ) { res := mTypes.NotChanged - acCtx, err := localCtx.GetAccessControlContext(ctx) + userAc, err := reqCtx.UserAcFromContext(ctx) if err != nil { - return res, err + return mTypes.NotChanged, err } - userid := localCtx.GetUsernameFromContext(acCtx) - - if userid == "" { - // empty user is anonymous, it has no data - return res, zerr.ErrUserDataNotAllowed + if userAc.IsAnonymous() || !userAc.Can(constants.ReadPermission, repo) { + return mTypes.NotChanged, zerr.ErrUserDataNotAllowed } - if ok, err := localCtx.RepoIsUserAvailable(ctx, repo); !ok || err != nil { - return res, zerr.ErrUserDataNotAllowed - } + userid := userAc.GetUsername() userData, err := dwr.GetUserData(ctx) if err != nil && !errors.Is(err, zerr.ErrUserDataNotFound) { @@ -1810,6 +1811,15 @@ func (dwr *DynamoDB) IsAPIKeyExpired(ctx context.Context, hashedKey string) (boo } func (dwr DynamoDB) UpdateUserAPIKeyLastUsed(ctx context.Context, hashedKey string) error { + userAc, err := reqCtx.UserAcFromContext(ctx) + if err != nil { + return err + } + + if userAc.IsAnonymous() { + return zerr.ErrUserDataNotAllowed + } + userData, err := dwr.GetUserData(ctx) if err != nil { return err @@ -1828,17 +1838,17 @@ func (dwr DynamoDB) UpdateUserAPIKeyLastUsed(ctx context.Context, hashedKey stri func (dwr DynamoDB) GetUserAPIKeys(ctx context.Context) ([]mTypes.APIKeyDetails, error) { apiKeys := make([]mTypes.APIKeyDetails, 0) - acCtx, err := localCtx.GetAccessControlContext(ctx) + userAc, err := reqCtx.UserAcFromContext(ctx) if err != nil { return nil, err } - userid := localCtx.GetUsernameFromContext(acCtx) - if userid == "" { - // empty user is anonymous + if userAc.IsAnonymous() { return nil, zerr.ErrUserDataNotAllowed } + userid := userAc.GetUsername() + userData, err := dwr.GetUserData(ctx) if err != nil && !errors.Is(err, zerr.ErrUserDataNotFound) { return nil, fmt.Errorf("metaDB: error while getting userData for identity %s %w", userid, err) @@ -1864,17 +1874,17 @@ func (dwr DynamoDB) GetUserAPIKeys(ctx context.Context) ([]mTypes.APIKeyDetails, } func (dwr DynamoDB) AddUserAPIKey(ctx context.Context, hashedKey string, apiKeyDetails *mTypes.APIKeyDetails) error { - acCtx, err := localCtx.GetAccessControlContext(ctx) + userAc, err := reqCtx.UserAcFromContext(ctx) if err != nil { return err } - userid := localCtx.GetUsernameFromContext(acCtx) - if userid == "" { - // empty user is anonymous + if userAc.IsAnonymous() { return zerr.ErrUserDataNotAllowed } + userid := userAc.GetUsername() + userData, err := dwr.GetUserData(ctx) if err != nil && !errors.Is(err, zerr.ErrUserDataNotFound) { return fmt.Errorf("metaDB: error while getting userData for identity %s %w", userid, err) @@ -1992,17 +2002,17 @@ func (dwr DynamoDB) GetUserAPIKeyInfo(hashedKey string) (string, error) { func (dwr DynamoDB) GetUserData(ctx context.Context) (mTypes.UserData, error) { var userData mTypes.UserData - acCtx, err := localCtx.GetAccessControlContext(ctx) + userAc, err := reqCtx.UserAcFromContext(ctx) if err != nil { return userData, err } - userid := localCtx.GetUsernameFromContext(acCtx) - if userid == "" { - // empty user is anonymous + if userAc.IsAnonymous() { return userData, zerr.ErrUserDataNotAllowed } + userid := userAc.GetUsername() + resp, err := dwr.Client.GetItem(ctx, &dynamodb.GetItemInput{ TableName: aws.String(dwr.UserDataTablename), Key: map[string]types.AttributeValue{ @@ -2026,17 +2036,17 @@ func (dwr DynamoDB) GetUserData(ctx context.Context) (mTypes.UserData, error) { } func (dwr DynamoDB) SetUserData(ctx context.Context, userData mTypes.UserData) error { - acCtx, err := localCtx.GetAccessControlContext(ctx) + userAc, err := reqCtx.UserAcFromContext(ctx) if err != nil { return err } - userid := localCtx.GetUsernameFromContext(acCtx) - if userid == "" { - // empty user is anonymous + if userAc.IsAnonymous() { return zerr.ErrUserDataNotAllowed } + userid := userAc.GetUsername() + userAttributeValue, err := attributevalue.Marshal(userData) if err != nil { return err @@ -2062,17 +2072,17 @@ func (dwr DynamoDB) SetUserData(ctx context.Context, userData mTypes.UserData) e } func (dwr DynamoDB) DeleteUserData(ctx context.Context) error { - acCtx, err := localCtx.GetAccessControlContext(ctx) + userAc, err := reqCtx.UserAcFromContext(ctx) if err != nil { return err } - userid := localCtx.GetUsernameFromContext(acCtx) - if userid == "" { - // empty user is anonymous + if userAc.IsAnonymous() { return zerr.ErrUserDataNotAllowed } + userid := userAc.GetUsername() + _, err = dwr.Client.DeleteItem(ctx, &dynamodb.DeleteItemInput{ TableName: aws.String(dwr.UserDataTablename), Key: map[string]types.AttributeValue{ diff --git a/pkg/meta/dynamodb/dynamodb_test.go b/pkg/meta/dynamodb/dynamodb_test.go index b66556e5..e547ca3e 100644 --- a/pkg/meta/dynamodb/dynamodb_test.go +++ b/pkg/meta/dynamodb/dynamodb_test.go @@ -20,7 +20,7 @@ import ( "zotregistry.io/zot/pkg/log" mdynamodb "zotregistry.io/zot/pkg/meta/dynamodb" mTypes "zotregistry.io/zot/pkg/meta/types" - localCtx "zotregistry.io/zot/pkg/requestcontext" + reqCtx "zotregistry.io/zot/pkg/requestcontext" "zotregistry.io/zot/pkg/test" ) @@ -170,13 +170,9 @@ func TestWrapperErrors(t *testing.T) { So(dynamoWrapper.ResetManifestDataTable(), ShouldBeNil) //nolint:contextcheck So(dynamoWrapper.ResetRepoMetaTable(), ShouldBeNil) //nolint:contextcheck - authzCtxKey := localCtx.GetContextKey() - - acCtx := localCtx.AccessControlContext{ - Username: "test", - } - - ctx := context.WithValue(context.Background(), authzCtxKey, acCtx) + userAc := reqCtx.NewUserAccessControl() + userAc.SetUsername("test") + ctx := userAc.DeriveContext(context.Background()) Convey("SetUserData", func() { hashKey := "id" @@ -197,13 +193,8 @@ func TestWrapperErrors(t *testing.T) { err := dynamoWrapper.SetUserData(ctx, userProfileSrc) So(err, ShouldBeNil) - authzCtxKey := localCtx.GetContextKey() - - acCtx := localCtx.AccessControlContext{ - Username: "", - } - - ctx := context.WithValue(context.Background(), authzCtxKey, acCtx) + userAc := reqCtx.NewUserAccessControl() + ctx := userAc.DeriveContext(context.Background()) err = dynamoWrapper.SetUserData(ctx, mTypes.UserData{}) //nolint: contextcheck So(err, ShouldNotBeNil) @@ -213,43 +204,32 @@ func TestWrapperErrors(t *testing.T) { err := dynamoWrapper.DeleteUserData(ctx) So(err, ShouldBeNil) - authzCtxKey := localCtx.GetContextKey() - - acCtx := localCtx.AccessControlContext{ - Username: "", - } - - ctx := context.WithValue(context.Background(), authzCtxKey, acCtx) + userAc := reqCtx.NewUserAccessControl() + ctx := userAc.DeriveContext(context.Background()) err = dynamoWrapper.DeleteUserData(ctx) //nolint: contextcheck So(err, ShouldNotBeNil) }) Convey("ToggleBookmarkRepo no access", func() { - acCtx := localCtx.AccessControlContext{ - ReadGlobPatterns: map[string]bool{ - "repo": false, - }, - Username: "username", - } - - authzCtxKey := localCtx.GetContextKey() - ctx := context.WithValue(context.Background(), authzCtxKey, acCtx) + userAc := reqCtx.NewUserAccessControl() + userAc.SetUsername("username") + userAc.SetGlobPatterns("read", map[string]bool{ + "repo": false, + }) + ctx := userAc.DeriveContext(context.Background()) _, err := dynamoWrapper.ToggleBookmarkRepo(ctx, "unaccesible") So(err, ShouldNotBeNil) }) Convey("ToggleBookmarkRepo GetUserMeta no user data", func() { - acCtx := localCtx.AccessControlContext{ - ReadGlobPatterns: map[string]bool{ - "repo": true, - }, - Username: "username", - } - - authzCtxKey := localCtx.GetContextKey() - ctx := context.WithValue(context.Background(), authzCtxKey, acCtx) + userAc := reqCtx.NewUserAccessControl() + userAc.SetUsername("username") + userAc.SetGlobPatterns("read", map[string]bool{ + "repo": true, + }) + ctx := userAc.DeriveContext(context.Background()) status, err := dynamoWrapper.ToggleBookmarkRepo(ctx, "repo") So(err, ShouldBeNil) @@ -257,15 +237,12 @@ func TestWrapperErrors(t *testing.T) { }) Convey("ToggleBookmarkRepo GetUserMeta client error", func() { - acCtx := localCtx.AccessControlContext{ - ReadGlobPatterns: map[string]bool{ - "repo": true, - }, - Username: "username", - } - - authzCtxKey := localCtx.GetContextKey() - ctx := context.WithValue(context.Background(), authzCtxKey, acCtx) + userAc := reqCtx.NewUserAccessControl() + userAc.SetUsername("username") + userAc.SetGlobPatterns("read", map[string]bool{ + "repo": false, + }) + ctx := userAc.DeriveContext(context.Background()) dynamoWrapper.UserDataTablename = badTablename @@ -275,15 +252,12 @@ func TestWrapperErrors(t *testing.T) { }) Convey("GetBookmarkedRepos", func() { - acCtx := localCtx.AccessControlContext{ - ReadGlobPatterns: map[string]bool{ - "repo": true, - }, - Username: "username", - } - - authzCtxKey := localCtx.GetContextKey() - ctx := context.WithValue(context.Background(), authzCtxKey, acCtx) + userAc := reqCtx.NewUserAccessControl() + userAc.SetUsername("username") + userAc.SetGlobPatterns("read", map[string]bool{ + "repo": false, + }) + ctx := userAc.DeriveContext(context.Background()) repos, err := dynamoWrapper.GetBookmarkedRepos(ctx) So(err, ShouldBeNil) @@ -291,38 +265,32 @@ func TestWrapperErrors(t *testing.T) { }) Convey("ToggleStarRepo GetUserMeta bad context", func() { - authzCtxKey := localCtx.GetContextKey() - ctx := context.WithValue(context.Background(), authzCtxKey, "some bad context") + uacKey := reqCtx.GetContextKey() + ctx := context.WithValue(context.Background(), uacKey, "bad context") _, err := dynamoWrapper.ToggleStarRepo(ctx, "repo") So(err, ShouldNotBeNil) }) Convey("ToggleStarRepo GetUserMeta no access", func() { - acCtx := localCtx.AccessControlContext{ - ReadGlobPatterns: map[string]bool{ - "repo": false, - }, - Username: "username", - } - - authzCtxKey := localCtx.GetContextKey() - ctx := context.WithValue(context.Background(), authzCtxKey, acCtx) + userAc := reqCtx.NewUserAccessControl() + userAc.SetUsername("username") + userAc.SetGlobPatterns("read", map[string]bool{ + "repo": false, + }) + ctx := userAc.DeriveContext(context.Background()) _, err := dynamoWrapper.ToggleStarRepo(ctx, "unaccesible") So(err, ShouldNotBeNil) }) Convey("ToggleStarRepo GetUserMeta error", func() { - acCtx := localCtx.AccessControlContext{ - ReadGlobPatterns: map[string]bool{ - "repo": false, - }, - Username: "username", - } - - authzCtxKey := localCtx.GetContextKey() - ctx := context.WithValue(context.Background(), authzCtxKey, acCtx) + userAc := reqCtx.NewUserAccessControl() + userAc.SetUsername("username") + userAc.SetGlobPatterns("read", map[string]bool{ + "repo": false, + }) + ctx := userAc.DeriveContext(context.Background()) dynamoWrapper.UserDataTablename = badTablename @@ -331,15 +299,12 @@ func TestWrapperErrors(t *testing.T) { }) Convey("ToggleStarRepo GetRepoMeta error", func() { - acCtx := localCtx.AccessControlContext{ - ReadGlobPatterns: map[string]bool{ - "repo": true, - }, - Username: "username", - } - - authzCtxKey := localCtx.GetContextKey() - ctx := context.WithValue(context.Background(), authzCtxKey, acCtx) + userAc := reqCtx.NewUserAccessControl() + userAc.SetUsername("username") + userAc.SetGlobPatterns("read", map[string]bool{ + "repo": true, + }) + ctx := userAc.DeriveContext(context.Background()) dynamoWrapper.RepoMetaTablename = badTablename @@ -348,8 +313,8 @@ func TestWrapperErrors(t *testing.T) { }) Convey("GetUserData bad context", func() { - authzCtxKey := localCtx.GetContextKey() - ctx := context.WithValue(context.Background(), authzCtxKey, "bad context") + uacKey := reqCtx.GetContextKey() + ctx := context.WithValue(context.Background(), uacKey, "bad context") userData, err := dynamoWrapper.GetUserData(ctx) So(err, ShouldNotBeNil) @@ -358,14 +323,12 @@ func TestWrapperErrors(t *testing.T) { }) Convey("GetUserData client error", func() { - acCtx := localCtx.AccessControlContext{ - ReadGlobPatterns: map[string]bool{ - "repo": true, - }, - Username: "username", - } - authzCtxKey := localCtx.GetContextKey() - ctx := context.WithValue(context.Background(), authzCtxKey, acCtx) + userAc := reqCtx.NewUserAccessControl() + userAc.SetUsername("username") + userAc.SetGlobPatterns("read", map[string]bool{ + "repo": true, + }) + ctx := userAc.DeriveContext(context.Background()) dynamoWrapper.UserDataTablename = badTablename @@ -374,16 +337,14 @@ func TestWrapperErrors(t *testing.T) { }) Convey("GetUserMeta unmarshal error, bad user data", func() { - acCtx := localCtx.AccessControlContext{ - ReadGlobPatterns: map[string]bool{ - "repo": true, - }, - Username: "username", - } - authzCtxKey := localCtx.GetContextKey() - ctx := context.WithValue(context.Background(), authzCtxKey, acCtx) + userAc := reqCtx.NewUserAccessControl() + userAc.SetUsername("username") + userAc.SetGlobPatterns("read", map[string]bool{ + "repo": true, + }) + ctx := userAc.DeriveContext(context.Background()) - err := setBadUserData(dynamoWrapper.Client, userDataTablename, acCtx.Username) + err := setBadUserData(dynamoWrapper.Client, userDataTablename, userAc.GetUsername()) So(err, ShouldBeNil) _, err = dynamoWrapper.GetUserData(ctx) @@ -391,69 +352,65 @@ func TestWrapperErrors(t *testing.T) { }) Convey("SetUserData bad context", func() { - authzCtxKey := localCtx.GetContextKey() - ctx := context.WithValue(context.Background(), authzCtxKey, "bad context") + uacKey := reqCtx.GetContextKey() + ctx := context.WithValue(context.Background(), uacKey, "bad context") err := dynamoWrapper.SetUserData(ctx, mTypes.UserData{}) So(err, ShouldNotBeNil) }) Convey("GetUserData bad context errors", func() { - authzCtxKey := localCtx.GetContextKey() - ctx := context.WithValue(context.Background(), authzCtxKey, "bad context") + uacKey := reqCtx.GetContextKey() + ctx := context.WithValue(context.Background(), uacKey, "bad context") _, err := dynamoWrapper.GetUserData(ctx) So(err, ShouldNotBeNil) }) Convey("SetUserData bad context errors", func() { - authzCtxKey := localCtx.GetContextKey() - ctx := context.WithValue(context.Background(), authzCtxKey, "bad context") + uacKey := reqCtx.GetContextKey() + ctx := context.WithValue(context.Background(), uacKey, "bad context") err := dynamoWrapper.SetUserData(ctx, mTypes.UserData{}) So(err, ShouldNotBeNil) }) Convey("AddUserAPIKey bad context errors", func() { - authzCtxKey := localCtx.GetContextKey() - ctx := context.WithValue(context.Background(), authzCtxKey, "bad context") + uacKey := reqCtx.GetContextKey() + ctx := context.WithValue(context.Background(), uacKey, "bad context") err := dynamoWrapper.AddUserAPIKey(ctx, "", &mTypes.APIKeyDetails{}) So(err, ShouldNotBeNil) }) Convey("DeleteUserAPIKey bad context errors", func() { - authzCtxKey := localCtx.GetContextKey() - ctx := context.WithValue(context.Background(), authzCtxKey, "bad context") + uacKey := reqCtx.GetContextKey() + ctx := context.WithValue(context.Background(), uacKey, "bad context") err := dynamoWrapper.DeleteUserAPIKey(ctx, "") So(err, ShouldNotBeNil) }) Convey("UpdateUserAPIKeyLastUsed bad context errors", func() { - authzCtxKey := localCtx.GetContextKey() - ctx := context.WithValue(context.Background(), authzCtxKey, "bad context") + uacKey := reqCtx.GetContextKey() + ctx := context.WithValue(context.Background(), uacKey, "bad context") err := dynamoWrapper.UpdateUserAPIKeyLastUsed(ctx, "") So(err, ShouldNotBeNil) }) Convey("DeleteUserData bad context errors", func() { - authzCtxKey := localCtx.GetContextKey() - ctx := context.WithValue(context.Background(), authzCtxKey, "bad context") + uacKey := reqCtx.GetContextKey() + ctx := context.WithValue(context.Background(), uacKey, "bad context") err := dynamoWrapper.DeleteUserData(ctx) So(err, ShouldNotBeNil) }) Convey("DeleteUserAPIKey returns nil", func() { - authzCtxKey := localCtx.GetContextKey() - - acCtx := localCtx.AccessControlContext{ - Username: "email", - } - - ctx := context.WithValue(context.Background(), authzCtxKey, acCtx) + userAc := reqCtx.NewUserAccessControl() + userAc.SetUsername("email") + ctx := userAc.DeriveContext(context.Background()) apiKeyDetails := make(map[string]mTypes.APIKeyDetails) apiKeyDetails["id"] = mTypes.APIKeyDetails{ @@ -471,24 +428,16 @@ func TestWrapperErrors(t *testing.T) { Convey("AddUserAPIKey", func() { Convey("no userid found", func() { - authzCtxKey := localCtx.GetContextKey() - - acCtx := localCtx.AccessControlContext{ - Username: "", - } - - ctx := context.WithValue(context.Background(), authzCtxKey, acCtx) + userAc := reqCtx.NewUserAccessControl() + ctx := userAc.DeriveContext(context.Background()) err = dynamoWrapper.AddUserAPIKey(ctx, "key", &mTypes.APIKeyDetails{}) So(err, ShouldNotBeNil) }) - authzCtxKey := localCtx.GetContextKey() - acCtx := localCtx.AccessControlContext{ - Username: "email", - } - - ctx := context.WithValue(context.Background(), authzCtxKey, acCtx) + userAc := reqCtx.NewUserAccessControl() + userAc.SetUsername("email") + ctx := userAc.DeriveContext(context.Background()) err := dynamoWrapper.AddUserAPIKey(ctx, "key", &mTypes.APIKeyDetails{}) So(err, ShouldBeNil) @@ -505,21 +454,15 @@ func TestWrapperErrors(t *testing.T) { }) Convey("GetUserData", func() { - authzCtxKey := localCtx.GetContextKey() + userAc := reqCtx.NewUserAccessControl() + ctx := userAc.DeriveContext(context.Background()) - acCtx := localCtx.AccessControlContext{ - Username: "", - } - - ctx := context.WithValue(context.Background(), authzCtxKey, acCtx) _, err := dynamoWrapper.GetUserData(ctx) So(err, ShouldNotBeNil) - acCtx = localCtx.AccessControlContext{ - Username: "email", - } - - ctx = context.WithValue(context.Background(), authzCtxKey, acCtx) + userAc = reqCtx.NewUserAccessControl() + userAc.SetUsername("email") + ctx = userAc.DeriveContext(context.Background()) dynamoWrapper.UserDataTablename = wrongTableName _, err = dynamoWrapper.GetUserData(ctx) @@ -1177,15 +1120,12 @@ func TestWrapperErrors(t *testing.T) { So(err, ShouldBeNil) dynamoWrapper.UserDataTablename = badTablename - acCtx := localCtx.AccessControlContext{ - ReadGlobPatterns: map[string]bool{ - "repo": true, - }, - Username: "username", - } - - authzCtxKey := localCtx.GetContextKey() - ctx := context.WithValue(context.Background(), authzCtxKey, acCtx) + userAc := reqCtx.NewUserAccessControl() + userAc.SetUsername("username") + userAc.SetGlobPatterns("read", map[string]bool{ + "repo": true, + }) + ctx := userAc.DeriveContext(context.Background()) _, err = dynamoWrapper.GetUserRepoMeta(ctx, "repo") So(err, ShouldNotBeNil) @@ -1195,15 +1135,12 @@ func TestWrapperErrors(t *testing.T) { err := setBadRepoMeta(dynamoWrapper.Client, repoMetaTablename, "repo") So(err, ShouldBeNil) - acCtx := localCtx.AccessControlContext{ - ReadGlobPatterns: map[string]bool{ - "repo": true, - }, - Username: "username", - } - - authzCtxKey := localCtx.GetContextKey() - ctx := context.WithValue(context.Background(), authzCtxKey, acCtx) + userAc := reqCtx.NewUserAccessControl() + userAc.SetUsername("username") + userAc.SetGlobPatterns("read", map[string]bool{ + "repo": true, + }) + ctx := userAc.DeriveContext(context.Background()) _, err = dynamoWrapper.GetUserRepoMeta(ctx, "repo") So(err, ShouldNotBeNil) diff --git a/pkg/meta/meta_test.go b/pkg/meta/meta_test.go index f10b6c23..4e76c9ca 100644 --- a/pkg/meta/meta_test.go +++ b/pkg/meta/meta_test.go @@ -30,7 +30,7 @@ import ( "zotregistry.io/zot/pkg/meta/common" mdynamodb "zotregistry.io/zot/pkg/meta/dynamodb" mTypes "zotregistry.io/zot/pkg/meta/types" - localCtx "zotregistry.io/zot/pkg/requestcontext" + reqCtx "zotregistry.io/zot/pkg/requestcontext" "zotregistry.io/zot/pkg/test" ) @@ -163,11 +163,10 @@ func RunMetaDBTests(t *testing.T, metaDB mTypes.MetaDB, preparationFuncs ...func hashKey2 := "key" label2 := "apiKey2" - authzCtxKey := localCtx.GetContextKey() - acCtx := localCtx.AccessControlContext{ - Username: "test", - } - ctx := context.WithValue(context.Background(), authzCtxKey, acCtx) + userAc := reqCtx.NewUserAccessControl() + userAc.SetUsername("test") + + ctx := userAc.DeriveContext(context.Background()) err := metaDB.AddUserAPIKey(ctx, hashKey1, &apiKeyDetails) So(err, ShouldBeNil) @@ -315,9 +314,8 @@ func RunMetaDBTests(t *testing.T, metaDB mTypes.MetaDB, preparationFuncs ...func Convey("Test API keys operations with invalid access control context", func() { var invalid struct{} - ctx := context.TODO() - key := localCtx.GetContextKey() - ctx = context.WithValue(ctx, key, invalid) + key := reqCtx.GetContextKey() + ctx := context.WithValue(context.Background(), key, invalid) _, err := metaDB.GetUserAPIKeys(ctx) So(err, ShouldNotBeNil) @@ -349,13 +347,10 @@ func RunMetaDBTests(t *testing.T, metaDB mTypes.MetaDB, preparationFuncs ...func }) Convey("Test API keys operations with empty userid", func() { - acCtx := localCtx.AccessControlContext{ - Username: "", - } + userAc := reqCtx.NewUserAccessControl() + userAc.SetUsername("") - ctx := context.TODO() - key := localCtx.GetContextKey() - ctx = context.WithValue(ctx, key, acCtx) + ctx := userAc.DeriveContext(context.Background()) _, err := metaDB.GetUserAPIKeys(ctx) So(err, ShouldNotBeNil) @@ -390,11 +385,10 @@ func RunMetaDBTests(t *testing.T, metaDB mTypes.MetaDB, preparationFuncs ...func expirationDate := time.Now().Add(500 * time.Millisecond).Local().Round(time.Millisecond) apiKeyDetails.ExpirationDate = expirationDate - authzCtxKey := localCtx.GetContextKey() - acCtx := localCtx.AccessControlContext{ - Username: "test", - } - ctx := context.WithValue(context.Background(), authzCtxKey, acCtx) + userAc := reqCtx.NewUserAccessControl() + userAc.SetUsername("test") + + ctx := userAc.DeriveContext(context.Background()) err := metaDB.AddUserAPIKey(ctx, hashKey1, &apiKeyDetails) So(err, ShouldBeNil) @@ -829,40 +823,34 @@ func RunMetaDBTests(t *testing.T, metaDB mTypes.MetaDB, preparationFuncs ...func repo2 = "repo2" ) - authzCtxKey := localCtx.GetContextKey() - - acCtx1 := localCtx.AccessControlContext{ - ReadGlobPatterns: map[string]bool{ - repo1: true, - repo2: true, - }, - Username: "user1", - } + userAc := reqCtx.NewUserAccessControl() + userAc.SetUsername("user1") + userAc.SetGlobPatterns("read", map[string]bool{ + repo1: true, + repo2: true, + }) // "user1" - ctx1 := context.WithValue(context.Background(), authzCtxKey, acCtx1) + ctx1 := userAc.DeriveContext(context.Background()) - acCtx2 := localCtx.AccessControlContext{ - ReadGlobPatterns: map[string]bool{ - repo1: true, - repo2: true, - }, - Username: "user2", - } + userAc = reqCtx.NewUserAccessControl() + userAc.SetUsername("user2") + userAc.SetGlobPatterns("read", map[string]bool{ + repo1: true, + repo2: true, + }) // "user2" - ctx2 := context.WithValue(context.Background(), authzCtxKey, acCtx2) + ctx2 := userAc.DeriveContext(context.Background()) - acCtx3 := localCtx.AccessControlContext{ - ReadGlobPatterns: map[string]bool{ - repo1: true, - repo2: true, - }, - Username: "", - } + userAc = reqCtx.NewUserAccessControl() + userAc.SetGlobPatterns("read", map[string]bool{ + repo1: true, + repo2: true, + }) - // anonymous - ctx3 := context.WithValue(context.Background(), authzCtxKey, acCtx3) + // anonymous user + ctx3 := userAc.DeriveContext(context.Background()) err := metaDB.SetRepoReference(repo1, tag1, manifestDigest1, ispec.MediaTypeImageManifest) So(err, ShouldBeNil) @@ -1072,40 +1060,34 @@ func RunMetaDBTests(t *testing.T, metaDB mTypes.MetaDB, preparationFuncs ...func repo2 = "repo2" ) - authzCtxKey := localCtx.GetContextKey() - - acCtx1 := localCtx.AccessControlContext{ - ReadGlobPatterns: map[string]bool{ - repo1: true, - repo2: true, - }, - Username: "user1", - } + userAc := reqCtx.NewUserAccessControl() + userAc.SetUsername("user1") + userAc.SetGlobPatterns("read", map[string]bool{ + repo1: true, + repo2: true, + }) // "user1" - ctx1 := context.WithValue(context.Background(), authzCtxKey, acCtx1) + ctx1 := userAc.DeriveContext(context.Background()) - acCtx2 := localCtx.AccessControlContext{ - ReadGlobPatterns: map[string]bool{ - repo1: true, - repo2: true, - }, - Username: "user2", - } + userAc = reqCtx.NewUserAccessControl() + userAc.SetUsername("user2") + userAc.SetGlobPatterns("read", map[string]bool{ + repo1: true, + repo2: true, + }) // "user2" - ctx2 := context.WithValue(context.Background(), authzCtxKey, acCtx2) + ctx2 := userAc.DeriveContext(context.Background()) - acCtx3 := localCtx.AccessControlContext{ - ReadGlobPatterns: map[string]bool{ - repo1: true, - repo2: true, - }, - Username: "", - } + userAc = reqCtx.NewUserAccessControl() + userAc.SetGlobPatterns("read", map[string]bool{ + repo1: true, + repo2: true, + }) - // anonymous - ctx3 := context.WithValue(context.Background(), authzCtxKey, acCtx3) + // anonymous user + ctx3 := userAc.DeriveContext(context.Background()) err := metaDB.SetRepoReference(repo1, tag1, manifestDigest1, ispec.MediaTypeImageManifest) So(err, ShouldBeNil) @@ -1651,15 +1633,14 @@ func RunMetaDBTests(t *testing.T, metaDB mTypes.MetaDB, preparationFuncs ...func err = metaDB.SetManifestMeta(repo3, manifestDigest1, emptyRepoMeta) So(err, ShouldBeNil) - acCtx := localCtx.AccessControlContext{ - ReadGlobPatterns: map[string]bool{ - repo1: true, - repo2: true, - }, - Username: "username", - } - authzCtxKey := localCtx.GetContextKey() - ctx := context.WithValue(context.Background(), authzCtxKey, acCtx) + userAc := reqCtx.NewUserAccessControl() + userAc.SetUsername("username") + userAc.SetGlobPatterns("read", map[string]bool{ + repo1: true, + repo2: true, + }) + + ctx := userAc.DeriveContext(context.Background()) repos, _, _, err := metaDB.SearchRepos(ctx, "repo") So(err, ShouldBeNil) @@ -1808,17 +1789,15 @@ func RunMetaDBTests(t *testing.T, metaDB mTypes.MetaDB, preparationFuncs ...func }) Convey("With no permision", func() { - acCtx1 := localCtx.AccessControlContext{ - ReadGlobPatterns: map[string]bool{ + userAc := reqCtx.NewUserAccessControl() + userAc.SetUsername("user1") + userAc.SetGlobPatterns("read", + map[string]bool{ repo1: false, repo2: false, }, - Username: "user1", - } - authzCtxKey := localCtx.GetContextKey() - - // "user1" - ctx1 := context.WithValue(context.Background(), authzCtxKey, acCtx1) + ) + ctx1 := userAc.DeriveContext(context.Background()) repos, _, _, err := metaDB.SearchTags(ctx1, "repo1:0.0.1") So(err, ShouldBeNil) @@ -1890,15 +1869,14 @@ func RunMetaDBTests(t *testing.T, metaDB mTypes.MetaDB, preparationFuncs ...func err = metaDB.SetManifestMeta(repo3, manifestDigest1, mTypes.ManifestMetadata{ConfigBlob: configBlob}) So(err, ShouldBeNil) - acCtx := localCtx.AccessControlContext{ - ReadGlobPatterns: map[string]bool{ - repo1: true, - repo2: false, - }, - Username: "username", - } - authzCtxKey := localCtx.GetContextKey() - ctx := context.WithValue(context.Background(), authzCtxKey, acCtx) + userAc := reqCtx.NewUserAccessControl() + userAc.SetUsername("username") + userAc.SetGlobPatterns("read", map[string]bool{ + repo1: true, + repo2: false, + }) + + ctx := userAc.DeriveContext(context.Background()) repos, _, _, err := metaDB.SearchTags(ctx, "repo1:") So(err, ShouldBeNil) @@ -2194,16 +2172,14 @@ func RunMetaDBTests(t *testing.T, metaDB mTypes.MetaDB, preparationFuncs ...func }) Convey("Search with access control", func() { - acCtx := localCtx.AccessControlContext{ - ReadGlobPatterns: map[string]bool{ - repo1: false, - repo2: true, - }, - Username: "username", - } + userAc := reqCtx.NewUserAccessControl() + userAc.SetUsername("username") + userAc.SetGlobPatterns("read", map[string]bool{ + repo1: false, + repo2: true, + }) - authzCtxKey := localCtx.GetContextKey() - ctx := context.WithValue(context.Background(), authzCtxKey, acCtx) + ctx := userAc.DeriveContext(context.Background()) repos, manifestMetaMap, _, err := metaDB.FilterTags(ctx, func(repoMeta mTypes.RepoMetadata, manifestMeta mTypes.ManifestMetadata) bool { @@ -2446,15 +2422,13 @@ func RunMetaDBTests(t *testing.T, metaDB mTypes.MetaDB, preparationFuncs ...func Convey("Test bookmarked/starred field present in returned RepoMeta", func() { repo99 := "repo99" - authzCtxKey := localCtx.GetContextKey() + userAc := reqCtx.NewUserAccessControl() + userAc.SetUsername("user1") + userAc.SetGlobPatterns("read", map[string]bool{ + repo99: true, + }) - acCtx := localCtx.AccessControlContext{ - ReadGlobPatterns: map[string]bool{ - repo99: true, - }, - Username: "user1", - } - ctx := context.WithValue(context.Background(), authzCtxKey, acCtx) + ctx := userAc.DeriveContext(context.Background()) manifestDigest := godigest.FromString("dig") err := metaDB.SetManifestData(manifestDigest, mTypes.ManifestData{ @@ -2528,15 +2502,13 @@ func RunMetaDBTests(t *testing.T, metaDB mTypes.MetaDB, preparationFuncs ...func }) Convey("Test GetUserRepoMeta", func() { - authzCtxKey := localCtx.GetContextKey() + userAc := reqCtx.NewUserAccessControl() + userAc.SetUsername("user1") + userAc.SetGlobPatterns("read", map[string]bool{ + "repo": true, + }) - acCtx := localCtx.AccessControlContext{ - ReadGlobPatterns: map[string]bool{ - "repo": true, - }, - Username: "user1", - } - ctx := context.WithValue(context.Background(), authzCtxKey, acCtx) + ctx := userAc.DeriveContext(context.Background()) digest := godigest.FromString("1") diff --git a/pkg/requestcontext/authn.go b/pkg/requestcontext/authn.go new file mode 100644 index 00000000..e155ed1f --- /dev/null +++ b/pkg/requestcontext/authn.go @@ -0,0 +1,33 @@ +package uac + +import ( + "context" + + "zotregistry.io/zot/errors" +) + +// request-local context key. +var amwCtxKey = Key(1) //nolint: gochecknoglobals + +// pointer needed for use in context.WithValue. +func GetAuthnMiddlewareCtxKey() *Key { + return &amwCtxKey +} + +type AuthnMiddlewareContext struct { + AuthnType string +} + +func GetAuthnMiddlewareContext(ctx context.Context) (*AuthnMiddlewareContext, error) { + authnMiddlewareCtxKey := GetAuthnMiddlewareCtxKey() + if authnMiddlewareCtx := ctx.Value(authnMiddlewareCtxKey); authnMiddlewareCtx != nil { + amCtx, ok := authnMiddlewareCtx.(AuthnMiddlewareContext) + if !ok { + return nil, errors.ErrBadType + } + + return &amCtx, nil + } + + return nil, nil //nolint: nilnil +} diff --git a/pkg/requestcontext/checkrepo.go b/pkg/requestcontext/checkrepo.go deleted file mode 100644 index de388109..00000000 --- a/pkg/requestcontext/checkrepo.go +++ /dev/null @@ -1,36 +0,0 @@ -package requestcontext - -import ( - "context" - - zerr "zotregistry.io/zot/errors" -) - -func RepoIsUserAvailable(ctx context.Context, repoName string) (bool, error) { - authzCtxKey := GetContextKey() - - if authCtx := ctx.Value(authzCtxKey); authCtx != nil { - acCtx, ok := authCtx.(AccessControlContext) - if !ok { - err := zerr.ErrBadCtxFormat - - return false, err - } - - if acCtx.IsAdmin || acCtx.CanReadRepo(repoName) { - return true, nil - } - - return false, nil - } - - return true, nil -} - -func GetUsernameFromContext(ctx *AccessControlContext) string { - if ctx == nil { - return "" - } - - return ctx.Username -} diff --git a/pkg/requestcontext/context.go b/pkg/requestcontext/context.go deleted file mode 100644 index e27a3e5c..00000000 --- a/pkg/requestcontext/context.go +++ /dev/null @@ -1,120 +0,0 @@ -package requestcontext - -import ( - "context" - - glob "github.com/bmatcuk/doublestar/v4" //nolint:gci - - zerr "zotregistry.io/zot/errors" -) - -type Key int - -// request-local context key. -var authzCtxKey = Key(0) //nolint: gochecknoglobals - -// pointer needed for use in context.WithValue. -func GetContextKey() *Key { - return &authzCtxKey -} - -// AccessControlContext - contains user authn/authz information. -type AccessControlContext struct { - // read method action - ReadGlobPatterns map[string]bool - // detectManifestCollision behaviour action - DmcGlobPatterns map[string]bool - IsAdmin bool - Username string - Groups []string -} - -/* - GetAccessControlContext returns an AccessControlContext struct made available on all http requests - (using context.Context values) by authz and authn middlewares. - -its methods and attributes can be used in http.Handlers to get user info for that specific request -(username, groups, if it's an admin, if it can access certain resources). -*/ -func GetAccessControlContext(ctx context.Context) (*AccessControlContext, error) { - authzCtxKey := GetContextKey() - if authCtx := ctx.Value(authzCtxKey); authCtx != nil { - acCtx, ok := authCtx.(AccessControlContext) - if !ok { - return nil, zerr.ErrBadType - } - - return &acCtx, nil - } - - return nil, nil //nolint: nilnil -} - -// returns whether or not the user/anonymous who made the request has read permission on 'repository'. -func (acCtx *AccessControlContext) CanReadRepo(repository string) bool { - if acCtx.ReadGlobPatterns != nil { - return acCtx.matchesRepo(acCtx.ReadGlobPatterns, repository) - } - - return true -} - -/* -returns whether or not the user/anonymous who made the request -has detectManifestCollision permission on 'repository'. -*/ -func (acCtx *AccessControlContext) CanDetectManifestCollision(repository string) bool { - if acCtx.DmcGlobPatterns != nil { - return acCtx.matchesRepo(acCtx.DmcGlobPatterns, repository) - } - - return false -} - -/* -returns whether or not 'repository' can be found in the list of patterns -on which the user who made the request has read permission on. -*/ -func (acCtx *AccessControlContext) matchesRepo(globPatterns map[string]bool, repository string) bool { - var longestMatchedPattern string - - // because of the longest path matching rule, we need to check all patterns from config - for pattern := range globPatterns { - matched, err := glob.Match(pattern, repository) - if err == nil { - if matched && len(pattern) > len(longestMatchedPattern) { - longestMatchedPattern = pattern - } - } - } - - allowed := globPatterns[longestMatchedPattern] - - return allowed -} - -// request-local context key. -var amwCtxKey = Key(1) //nolint: gochecknoglobals - -// pointer needed for use in context.WithValue. -func GetAuthnMiddlewareCtxKey() *Key { - return &amwCtxKey -} - -type AuthnMiddlewareContext struct { - AuthnType string -} - -func GetAuthnMiddlewareContext(ctx context.Context) (*AuthnMiddlewareContext, error) { - authnMiddlewareCtxKey := GetAuthnMiddlewareCtxKey() - if authnMiddlewareCtx := ctx.Value(authnMiddlewareCtxKey); authnMiddlewareCtx != nil { - amCtx, ok := authnMiddlewareCtx.(AuthnMiddlewareContext) - if !ok { - return nil, zerr.ErrBadType - } - - return &amCtx, nil - } - - return nil, nil //nolint: nilnil -} diff --git a/pkg/requestcontext/user_access_control.go b/pkg/requestcontext/user_access_control.go new file mode 100644 index 00000000..fee4ec37 --- /dev/null +++ b/pkg/requestcontext/user_access_control.go @@ -0,0 +1,255 @@ +package uac + +import ( + "context" + "net/http" + + glob "github.com/bmatcuk/doublestar/v4" //nolint:gci + + "zotregistry.io/zot/errors" + "zotregistry.io/zot/pkg/api/constants" +) + +type Key int + +// request-local context key. +var uacCtxKey = Key(0) //nolint: gochecknoglobals + +// pointer needed for use in context.WithValue. +func GetContextKey() *Key { + return &uacCtxKey +} + +type UserAccessControl struct { + authzInfo *UserAuthzInfo + authnInfo *UserAuthnInfo + methodActions []string + behaviourActions []string +} + +type UserAuthzInfo struct { + // {action: {repo: bool}} + globPatterns map[string]map[string]bool + isAdmin bool +} + +type UserAuthnInfo struct { + groups []string + username string +} + +func NewUserAccessControl() *UserAccessControl { + return &UserAccessControl{ + // authzInfo will be populated in authz.go middleware + // if no authz enabled on server this will be nil + authzInfo: nil, + // authnInfo will be populated in authn.go middleware + // if no authn enabled on server this will be nil + authnInfo: nil, + // actions type + behaviourActions: []string{constants.DetectManifestCollisionPermission}, + methodActions: []string{ + constants.ReadPermission, + constants.CreatePermission, + constants.UpdatePermission, + constants.DeletePermission, + }, + } +} + +func (uac *UserAccessControl) SetUsername(username string) { + if uac.authnInfo == nil { + uac.authnInfo = &UserAuthnInfo{} + } + + uac.authnInfo.username = username +} + +func (uac *UserAccessControl) GetUsername() string { + if uac.authnInfo == nil { + return "" + } + + return uac.authnInfo.username +} + +func (uac *UserAccessControl) AddGroups(groups []string) { + if uac.authnInfo == nil { + uac.authnInfo = &UserAuthnInfo{ + groups: []string{}, + } + } + + uac.authnInfo.groups = append(uac.authnInfo.groups, groups...) +} + +func (uac *UserAccessControl) GetGroups() []string { + if uac.authnInfo == nil { + return []string{} + } + + return uac.authnInfo.groups +} + +func (uac *UserAccessControl) IsAnonymous() bool { + if uac.authnInfo == nil { + return true + } + + return uac.authnInfo.username == "" +} + +func (uac *UserAccessControl) IsAdmin() bool { + // if isAdmin was not set in authz.go then everybody is admin + if uac.authzInfo == nil { + return true + } + + return uac.authzInfo.isAdmin +} + +func (uac *UserAccessControl) SetIsAdmin(isAdmin bool) { + if uac.authzInfo == nil { + uac.authzInfo = &UserAuthzInfo{} + } + + uac.authzInfo.isAdmin = isAdmin +} + +/* + UserAcFromContext returns an UserAccessControl struct made available on all http requests + (using context.Context values) by authz and authn middlewares. + + If no UserAccessControl is found on context, it will return an empty one. + +its methods and attributes can be used in http.Handlers to get user info for that specific request +(username, groups, if it's an admin, if it can access certain resources). +*/ +func UserAcFromContext(ctx context.Context) (*UserAccessControl, error) { + if uacValue := ctx.Value(GetContextKey()); uacValue != nil { + uac, ok := uacValue.(UserAccessControl) + if !ok { + return nil, errors.ErrBadType + } + + return &uac, nil + } + + return NewUserAccessControl(), nil +} + +func (uac *UserAccessControl) SetGlobPatterns(action string, patterns map[string]bool) { + if uac.authzInfo == nil { + uac.authzInfo = &UserAuthzInfo{ + globPatterns: make(map[string]map[string]bool), + } + } + + uac.authzInfo.globPatterns[action] = patterns +} + +/* +Can returns whether or not the user/anonymous who made the request has 'action' permission on 'repository'. +*/ +func (uac *UserAccessControl) Can(action, repository string) bool { + var defaultRet bool + if uac.isBehaviourAction(action) { + defaultRet = false + } else if uac.isMethodAction(action) { + defaultRet = true + } + + if uac.IsAdmin() { + return defaultRet + } + + // if glob patterns are not set then authz is not enabled, so everybody have access. + if !uac.areGlobPatternsSet() { + return defaultRet + } + + return uac.matchesRepo(uac.authzInfo.globPatterns[action], repository) +} + +func (uac *UserAccessControl) isBehaviourAction(action string) bool { + for _, behaviourAction := range uac.behaviourActions { + if action == behaviourAction { + return true + } + } + + return false +} + +func (uac *UserAccessControl) isMethodAction(action string) bool { + for _, methodAction := range uac.methodActions { + if action == methodAction { + return true + } + } + + return false +} + +// returns whether or not glob patterns have been set in authz.go. +func (uac *UserAccessControl) areGlobPatternsSet() bool { + notSet := uac.authzInfo == nil || uac.authzInfo.globPatterns == nil + + return !notSet +} + +/* +returns whether or not 'repository' can be found in the list of patterns +on which the user who made the request has read permission on. +*/ +func (uac *UserAccessControl) matchesRepo(globPatterns map[string]bool, repository string) bool { + var longestMatchedPattern string + + // because of the longest path matching rule, we need to check all patterns from config + for pattern := range globPatterns { + matched, err := glob.Match(pattern, repository) + if err == nil { + if matched && len(pattern) > len(longestMatchedPattern) { + longestMatchedPattern = pattern + } + } + } + + allowed := globPatterns[longestMatchedPattern] + + return allowed +} + +/* + SaveOnRequest saves UserAccessControl on the request's context. + +Later UserAcFromContext(request.Context()) can be used to obtain UserAccessControl that was saved on it. +*/ +func (uac *UserAccessControl) SaveOnRequest(request *http.Request) { + uacContext := context.WithValue(request.Context(), GetContextKey(), *uac) + + *request = *request.WithContext(uacContext) +} + +/* + DeriveContext takes a context(parent) and returns a derived context(child) containing this UserAccessControl. + +Later UserAcFromContext(ctx context.Context) can be used to obtain the UserAccessControl that was added on it. +*/ +func (uac *UserAccessControl) DeriveContext(ctx context.Context) context.Context { + return context.WithValue(ctx, GetContextKey(), *uac) +} + +func RepoIsUserAvailable(ctx context.Context, repoName string) (bool, error) { + uac, err := UserAcFromContext(ctx) + if err != nil { + return false, err + } + + // no authn/authz enabled on server + if uac == nil { + return true, nil + } + + return uac.Can("read", repoName), nil +} diff --git a/swagger/docs.go b/swagger/docs.go index d67aa650..bef36946 100644 --- a/swagger/docs.go +++ b/swagger/docs.go @@ -526,6 +526,12 @@ const docTemplate = `{ } } }, + "401": { + "description": "unauthorized", + "schema": { + "type": "string" + } + }, "404": { "description": "not found", "schema": { diff --git a/swagger/swagger.json b/swagger/swagger.json index d720a3b6..f77cfb36 100644 --- a/swagger/swagger.json +++ b/swagger/swagger.json @@ -517,6 +517,12 @@ } } }, + "401": { + "description": "unauthorized", + "schema": { + "type": "string" + } + }, "404": { "description": "not found", "schema": { diff --git a/swagger/swagger.yaml b/swagger/swagger.yaml index 77a66473..c5c3c6ad 100644 --- a/swagger/swagger.yaml +++ b/swagger/swagger.yaml @@ -650,6 +650,10 @@ paths: type: string schema: type: string + "401": + description: unauthorized + schema: + type: string "404": description: not found schema: diff --git a/test/blackbox/cloud-only.bats b/test/blackbox/cloud_only.bats similarity index 100% rename from test/blackbox/cloud-only.bats rename to test/blackbox/cloud_only.bats diff --git a/test/blackbox/pushpull_authn.bats b/test/blackbox/pushpull_authn.bats new file mode 100644 index 00000000..ca08417e --- /dev/null +++ b/test/blackbox/pushpull_authn.bats @@ -0,0 +1,158 @@ +load helpers_zot + +function verify_prerequisites { + if [ ! $(command -v curl) ]; then + echo "you need to install curl as a prerequisite to running the tests" >&3 + return 1 + fi + + if [ ! $(command -v jq) ]; then + echo "you need to install jq as a prerequisite to running the tests" >&3 + return 1 + fi + + if [ ! $(command -v htpasswd) ]; then + echo "you need to install htpasswd as a prerequisite to running the tests" >&3 + return 1 + fi + + return 0 +} + +function setup_file() { + # Verify prerequisites are available + if ! $(verify_prerequisites); then + exit 1 + fi + + # Setup zot server + local zot_root_dir=${BATS_FILE_TMPDIR}/zot + local zot_config_file=${BATS_FILE_TMPDIR}/zot_config.json + local zot_htpasswd_file=${BATS_FILE_TMPDIR}/zot_htpasswd + htpasswd -Bbn test test123 >> ${zot_htpasswd_file} + + echo ${zot_root_dir} >&3 + + mkdir -p ${zot_root_dir} + + cat > ${zot_config_file}<