Skip to content

Commit

Permalink
refactor(authz): use a struct for user access control info operations
Browse files Browse the repository at this point in the history 
fix(authz): fix isAdmin not using groups to determine if a user is admin.

Signed-off-by: Petu Eusebiu <[email protected]>
  • Loading branch information
@eusebiu-constantin-petu-dbk
eusebiu-constantin-petu-dbk committed Aug 8, 2023
1 parent ed90e3b commit 6bdf1c7
Show file tree
Hide file tree
Showing 17 changed files with 665 additions and 782 deletions.
86 changes: 40 additions & 46 deletions pkg/api/authn.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ import (
apiErr "zotregistry.io/zot/pkg/api/errors"
"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"
)

Expand All @@ -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 {
Expand All @@ -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

Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -201,24 +206,25 @@ func (amw *AuthnMiddleware) basicAuthn(ctlr *Controller, response http.ResponseW
}

if storedIdentity == identity {
ctx := getReqContextWithAuthorization(identity, []string{}, request)
userAc.SetUsername(identity)
userAc.SaveOnRequest(request)

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
}
Expand Down Expand Up @@ -357,10 +363,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)

Expand All @@ -375,7 +386,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")
Expand All @@ -396,19 +407,11 @@ 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
Expand Down Expand Up @@ -494,8 +497,8 @@ func noPasswdAuth(config *config.Config) mux.MiddlewareFunc {
return
}

ctx := getReqContextWithAuthorization("", []string{}, request)
*request = *request.WithContext(ctx) //nolint:contextcheck
userAc := reqCtx.NewUserAccessControl()
userAc.SaveOnRequest(request)

// Process request
next.ServeHTTP(response, request)
Expand Down Expand Up @@ -626,18 +629,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)

Expand Down Expand Up @@ -786,15 +777,18 @@ 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
if err := saveUserLoggedSession(ctlr.CookieStore, w, r, email, ctlr.Log); err != nil {
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
Expand Down
12 changes: 4 additions & 8 deletions pkg/api/authn_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,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"
)
Expand Down Expand Up @@ -376,13 +376,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.SaveOnContext(context.Background())

err = ctlr.MetaDB.DeleteUserData(ctx)
So(err, ShouldBeNil)
Expand Down
Loading

0 comments on commit 6bdf1c7

Please sign in to comment.