fix(authz): get username from authn.go request context (#1383)

Signed-off-by: Petu Eusebiu <peusebiu@cisco.com>

pkg/api/authz.go

@@ -155,20 +155,15 @@ func (ac *AccessController) getUserGroups(username string) []string {
 	return groupNames
 }
 
-// getContext builds ac context(allowed to read repos and if user is admin) and returns it.
+// getContext updates an AccessControlContext for a user/anonymous and returns a context.Context containing it.
-func (ac *AccessController) getContext(username string, request *http.Request) context.Context {
+func (ac *AccessController) getContext(acCtx *localCtx.AccessControlContext, request *http.Request) context.Context {
-	acCtx, err := localCtx.GetAccessControlContext(request.Context())
+	readGlobPatterns := ac.getGlobPatterns(acCtx.Username, acCtx.Groups, Read)
-	if err != nil {
+	dmcGlobPatterns := ac.getGlobPatterns(acCtx.Username, acCtx.Groups, DetectManifestCollision)
-		return nil
-	}
-
-	readGlobPatterns := ac.getGlobPatterns(username, acCtx.Groups, Read)
-	dmcGlobPatterns := ac.getGlobPatterns(username, acCtx.Groups, DetectManifestCollision)
 
 	acCtx.ReadGlobPatterns = readGlobPatterns
 	acCtx.DmcGlobPatterns = dmcGlobPatterns
 
-	if ac.isAdmin(username) {
+	if ac.isAdmin(acCtx.Username) {
 		acCtx.IsAdmin = true
 	} else {
 		acCtx.IsAdmin = false
@@ -243,16 +238,23 @@ func AuthzHandler(ctlr *Controller) mux.MiddlewareFunc {
 			acCtrlr := NewAccessController(ctlr.Config)
 
 			var identity string
+
 			var err error
 
-			// allow anonymous authz if no authn present and only default policies are present
+			// anonymous context
-			identity = ""
+			acCtx := &localCtx.AccessControlContext{}
-			if isAuthnEnabled(ctlr.Config) && request.Header.Get("Authorization") != "" {
-				identity, _, err = getUsernamePasswordBasicAuth(request)
 
-				if err != nil {
+			// get username from context made in authn.go
+			if isAuthnEnabled(ctlr.Config) {
+				// 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, ctlr.Config.HTTP.Realm, ctlr.Config.HTTP.Auth.FailDelay)
+
+					return
 				}
+
+				identity = acCtx.Username
 			}
 
 			if request.TLS != nil {
@@ -268,14 +270,19 @@ func AuthzHandler(ctlr *Controller) mux.MiddlewareFunc {
 					if identity == "" {
 						acCtrlr.Log.Info().Msg("couldn't get identity from TLS certificate")
 						authFail(response, ctlr.Config.HTTP.Realm, ctlr.Config.HTTP.Auth.FailDelay)
+
+						return
 					}
 				}
 			}
 
-			ctx := acCtrlr.getContext(identity, request)
+			ctx := acCtrlr.getContext(acCtx, request)
 
-			// for extensions, we only need to know the username, whether the user is an admin, and what repositories
+			/* Notes:
-			// they can read. So, run next()
+			 	- since we only do READ actions in extensions, we can bypass authz for them
+				only need to know the username, whether the user is an admin, or what repos he can read.
+				let each extension to apply	authorization on them using localCtx.AccessControlContext{}
+			*/
 			if isExtensionURI(request.RequestURI) {
 				next.ServeHTTP(response, request.WithContext(ctx)) //nolint:contextcheck
 

pkg/requestcontext/context.go

@@ -18,7 +18,7 @@ func GetContextKey() *Key {
 	return &authzCtxKey
 }
 
-// AccessControlContext context passed down to http.Handlers.
+// AccessControlContext - contains user authn/authz information.
 type AccessControlContext struct {
 	// read method action
 	ReadGlobPatterns map[string]bool
@@ -29,6 +29,13 @@ type AccessControlContext struct {
 	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 {
@@ -43,7 +50,31 @@ func GetAccessControlContext(ctx context.Context) (*AccessControlContext, error)
 	return nil, nil //nolint: nilnil
 }
 
-// returns either a user has or not rights on 'repository'.
+// 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
 
@@ -61,21 +92,3 @@ func (acCtx *AccessControlContext) matchesRepo(globPatterns map[string]bool, rep
 
 	return allowed
 }
-
-// returns either a user has or not read rights on 'repository'.
-func (acCtx *AccessControlContext) CanReadRepo(repository string) bool {
-	if acCtx.ReadGlobPatterns != nil {
-		return acCtx.matchesRepo(acCtx.ReadGlobPatterns, repository)
-	}
-
-	return true
-}
-
-// returns either a user has or not detectManifestCollision rights on 'repository'.
-func (acCtx *AccessControlContext) CanDetectManifestCollision(repository string) bool {
-	if acCtx.DmcGlobPatterns != nil {
-		return acCtx.matchesRepo(acCtx.DmcGlobPatterns, repository)
-	}
-
-	return false
-}