refactor: create interfaces for AuthN and AuthZ (#1286)

This change defines interfaces for AuthN and AuthZ, which helps modularize the auth components in a way that each package takes care of its own business and has minimal knowledge of the implementation details of dependencies. The goal is to hide complexity and reduce engineer cognitive load.
Signed-off-by: Keran Yang <yangkr920208@gmail.com>

server/apis/interface.go

@@ -19,6 +19,7 @@ package apis
 import "github.com/gin-gonic/gin"
 
 type Handler interface {
+	AuthInfo(c *gin.Context)
 	ListNamespaces(c *gin.Context)
 	GetClusterSummary(c *gin.Context)
 	CreatePipeline(c *gin.Context)

server/apis/v1/dexauth.go → server/apis/v1/dexauthn.go

@@ -98,8 +98,24 @@ func (d *DexObject) oauth2Config(scopes []string) (*oauth2.Config, error) {
 	}, nil
 }
 
-// Verify is used to validate the user ID token.
-func (d *DexObject) Verify(ctx context.Context, rawIDToken string) (*oidc.IDToken, error) {
+func (d *DexObject) Authenticate(c *gin.Context) (*authn.UserInfo, error) {
+	var userInfo authn.UserInfo
+	userIdentityTokenStr, err := c.Cookie(common.UserIdentityCookieName)
+	if err != nil {
+		return nil, fmt.Errorf("failed to get user identity token from cookie: %v", err)
+	}
+	if err = json.Unmarshal([]byte(userIdentityTokenStr), &userInfo); err != nil {
+		return nil, fmt.Errorf("failed to parse user identity token: %v", err)
+	}
+	_, err = d.verify(c, userInfo.IDToken)
+	if err != nil {
+		return nil, err
+	}
+	return &userInfo, nil
+}
+
+// verify is used to validate the user ID token.
+func (d *DexObject) verify(ctx context.Context, rawIDToken string) (*oidc.IDToken, error) {
 	verifier, err := d.verifier()
 	if err != nil {
 		return nil, err
@@ -199,7 +215,7 @@ func (d *DexObject) handleCallback(c *gin.Context) {
 		return
 	}
 
-	idToken, err := d.Verify(r.Context(), rawIDToken)
+	idToken, err := d.verify(r.Context(), rawIDToken)
 	if err != nil {
 		errMsg := fmt.Sprintf("Failed to verify ID token: %v", err)
 		c.JSON(http.StatusOK, NewNumaflowAPIResponse(&errMsg, nil))
@@ -220,7 +236,7 @@ func (d *DexObject) handleCallback(c *gin.Context) {
 		return
 	}
 
-	res := authn.NewUserIdInfo(claims, rawIDToken, refreshToken)
+	res := authn.NewUserInfo(claims, rawIDToken, refreshToken)
 	tokenStr, err := json.Marshal(res)
 	if err != nil {
 		errMsg := fmt.Sprintf("Failed to convert to token string: %v", err)

server/apis/v1/handler.go

@@ -19,6 +19,7 @@ package v1
 import (
 	"bufio"
 	"context"
+	"encoding/json"
 	"fmt"
 	"io"
 	"math"
@@ -46,7 +47,6 @@ import (
 	"github.com/numaproj/numaflow/pkg/shared/util"
 	"github.com/numaproj/numaflow/server/authn"
 	"github.com/numaproj/numaflow/server/common"
-	"github.com/numaproj/numaflow/server/utils"
 	"github.com/numaproj/numaflow/webhook/validator"
 )
 
@@ -93,13 +93,13 @@ func (h *handler) AuthInfo(c *gin.Context) {
 		c.JSON(http.StatusUnauthorized, NewNumaflowAPIResponse(&errMsg, nil))
 		return
 	}
-	userIdentityToken, err := utils.ParseUserIdentityToken(userIdentityTokenStr)
-	if err != nil {
+	userInfo := &authn.UserInfo{}
+	if err = json.Unmarshal([]byte(userIdentityTokenStr), userInfo); err != nil {
 		errMsg := fmt.Sprintf("user is not authenticated, err: %s", err.Error())
 		c.JSON(http.StatusUnauthorized, NewNumaflowAPIResponse(&errMsg, nil))
 		return
 	}
-	res := authn.NewUserIdInfo(userIdentityToken.IDTokenClaims, userIdentityToken.IDToken, userIdentityToken.RefreshToken)
+	res := authn.NewUserInfo(userInfo.IDTokenClaims, userInfo.IDToken, userInfo.RefreshToken)
 	c.JSON(http.StatusOK, NewNumaflowAPIResponse(nil, res))
 }
 

server/authn/interface.go

@@ -0,0 +1,10 @@
+package authn
+
+import "github.com/gin-gonic/gin"
+
+type Authenticator interface {
+	// Authenticate is used to validate the user's identity.
+	// If the user is authenticated, the function returns user information.
+	// Otherwise, empty information with the corresponding error.
+	Authenticate(c *gin.Context) (*UserInfo, error)
+}

server/authn/user_id_info.go

@@ -32,17 +32,16 @@ type IDTokenClaims struct {
 	PreferredUsername string   `json:"preferred_username"`
 }
 
-// UserIdInfo includes information about the user identity
+// UserInfo includes information about the user identity
 // It holds the IDTokenClaims, IDToken and RefreshToken for the user
-type UserIdInfo struct {
+type UserInfo struct {
 	IDTokenClaims IDTokenClaims `json:"id_token_claims"`
 	IDToken       string        `json:"id_token"`
 	RefreshToken  string        `json:"refresh_token"`
 }
 
-// NewUserIdInfo return a Callback Response Object.
-func NewUserIdInfo(itc IDTokenClaims, idToken string, refreshToken string) UserIdInfo {
-	return UserIdInfo{
+func NewUserInfo(itc IDTokenClaims, idToken string, refreshToken string) UserInfo {
+	return UserInfo{
 		IDTokenClaims: itc,
 		IDToken:       idToken,
 		RefreshToken:  refreshToken,

server/authz/interface.go

@@ -0,0 +1,12 @@
+package authz
+
+import "github.com/gin-gonic/gin"
+
+type Authorizer interface {
+	// Authorize checks if a user is authorized to access the resource.
+	// c is the gin context.
+	// g is the list of groups the user belongs to.
+	// Authorize trusts that the user is already authenticated and directly uses the groups to authorize the user.
+	// please don't use gin to get the user information again.
+	Authorize(c *gin.Context, g []string) (bool, error)
+}

server/authz/rbac.go

@@ -36,9 +36,37 @@ const (
 	emptyString = ""
 )
 
-// GetEnforcer initializes the Casbin Enforcer with the model and policy.
-func GetEnforcer() (*casbin.Enforcer, error) {
+type CasbinObject struct {
+	enforcer *casbin.Enforcer
+}
+
+func NewCasbinObject() (*CasbinObject, error) {
+	enforcer, err := getEnforcer()
+	if err != nil {
+		return nil, err
+	}
+	return &CasbinObject{
+		enforcer: enforcer,
+	}, nil
+}
+
+func (cas *CasbinObject) Authorize(c *gin.Context, groups []string) (bool, error) {
+	resource := extractResource(c)
+	object := extractObject(c)
+	action := c.Request.Method
+	// Check if the user has permission for any of the groups.
+	for _, group := range groups {
+		// Get the user from the group. The group is in the format "group:role".
+		// Check if the user has permission using Casbin Enforcer.
+		if ok, _ := cas.enforcer.Enforce(group, resource, object, action); ok {
+			return true, nil
+		}
+	}
+	return false, fmt.Errorf("user is not authorized to execute the requested action")
+}
 
+// getEnforcer initializes the Casbin Enforcer with the model and policy.
+func getEnforcer() (*casbin.Enforcer, error) {
 	modelRBAC, err := model.NewModelFromString(rbacModel)
 	if err != nil {
 		return nil, err
@@ -104,8 +132,8 @@ func extractArgs(args ...interface{}) (string, string, error) {
 	return req, policy, nil
 }
 
-// ExtractResource extracts the resource from the request.
-func ExtractResource(c *gin.Context) string {
+// extractResource extracts the resource from the request.
+func extractResource(c *gin.Context) string {
 	// We use the namespace in the request as the resource.
 	resource := c.Param(ResourceNamespace)
 	if resource == emptyString {
@@ -114,8 +142,8 @@ func ExtractResource(c *gin.Context) string {
 	return resource
 }
 
-// ExtractObject extracts the object from the request.
-func ExtractObject(c *gin.Context) string {
+// extractObject extracts the object from the request.
+func extractObject(c *gin.Context) string {
 	action := c.Request.Method
 	// Get the route map from the context. Key is in the format "method:path".
 	routeMapKey := fmt.Sprintf("%s:%s", action, c.FullPath())

server/authz/route_map.go

@@ -20,20 +20,21 @@ package authz
 // corresponding to the route and a boolean to indicate whether the route requires
 // authorization.
 type RouteInfo struct {
-	Object       string
-	RequiresAuth bool
+	Object        string
+	RequiresAuthZ bool
 }
 
 // newRouteInfo creates a new RouteInfo object.
-func newRouteInfo(object string, requiresAuth bool) *RouteInfo {
+func newRouteInfo(object string, requiresAuthZ bool) *RouteInfo {
 	return &RouteInfo{
-		Object:       object,
-		RequiresAuth: requiresAuth,
+		Object:        object,
+		RequiresAuthZ: requiresAuthZ,
 	}
 }
 
-// RouteMap is a map of routes to their corresponding RouteInfo objects. This map is used to certain
-// information about the route.
+// RouteMap is a map of routes to their corresponding RouteInfo objects.
+// It saves the object corresponding to the route and a boolean to indicate
+// whether the route requires authorization.
 var RouteMap = map[string]*RouteInfo{
 	"GET:/api/v1/sysinfo":                                                         newRouteInfo(ObjectPipeline, false),
 	"GET:/api/v1/authinfo":                                                        newRouteInfo(ObjectEvents, false),

server/routes/routes.go

@@ -20,15 +20,12 @@ import (
 	"fmt"
 	"net/http"
 
-	"github.com/casbin/casbin/v2"
 	"github.com/gin-gonic/gin"
 
 	"github.com/numaproj/numaflow/pkg/shared/logging"
 	v1 "github.com/numaproj/numaflow/server/apis/v1"
 	"github.com/numaproj/numaflow/server/authn"
 	"github.com/numaproj/numaflow/server/authz"
-	"github.com/numaproj/numaflow/server/common"
-	"github.com/numaproj/numaflow/server/utils"
 )
 
 type SystemInfo struct {
@@ -62,12 +59,12 @@ func Routes(r *gin.Engine, sysInfo SystemInfo, authInfo AuthInfo, baseHref strin
 	// they share the AuthN/AuthZ middleware.
 	r1Group := r.Group("/api/v1")
 	if !authInfo.DisableAuth {
-		enforcer, err := authz.GetEnforcer()
+		authorizer, err := authz.NewCasbinObject()
 		if err != nil {
 			panic(err)
 		}
 		// Add the AuthN/AuthZ middleware to the group.
-		r1Group.Use(authMiddleware(enforcer, dexObj))
+		r1Group.Use(authMiddleware(authorizer, dexObj))
 	}
 	v1Routes(r1Group)
 	r1Group.GET("/sysinfo", func(c *gin.Context) {
@@ -141,49 +138,36 @@ func v1Routes(r gin.IRouter) {
 	r.GET("/namespaces/:namespace/pods/:pod/logs", handler.PodLogs)
 	// List of the Kubernetes events of a namespace.
 	r.GET("/namespaces/:namespace/events", handler.GetNamespaceEvents)
-
 }
 
-func authMiddleware(enforcer *casbin.Enforcer, dexObj *v1.DexObject) gin.HandlerFunc {
+// authMiddleware is the middleware for AuthN/AuthZ.
+// it ensures the user is authenticated and authorized
+// to execute the requested action before sending the request to the api handler.
+func authMiddleware(authorizer authz.Authorizer, authenticator authn.Authenticator) gin.HandlerFunc {
 	return func(c *gin.Context) {
 		// Authenticate the user.
-		userIdentityToken, err := authenticate(c, dexObj)
+		userInfo, err := authenticator.Authenticate(c)
 		if err != nil {
 			errMsg := fmt.Sprintf("failed to authenticate user: %v", err)
 			c.JSON(http.StatusUnauthorized, v1.NewNumaflowAPIResponse(&errMsg, nil))
 			c.Abort()
 			return
 		}
-		// Authorize the user and the request.
-		// Get the user from the user identity token.
-		groups := userIdentityToken.IDTokenClaims.Groups
-		resource := authz.ExtractResource(c)
-		object := authz.ExtractObject(c)
-		action := c.Request.Method
-		isAuthorized := false
-
 		// Get the route map from the context. Key is in the format "method:path".
-		routeMapKey := fmt.Sprintf("%s:%s", action, c.FullPath())
-		// Check if the route requires auth.
-		if authz.RouteMap[routeMapKey] != nil && authz.RouteMap[routeMapKey].RequiresAuth {
-			// Check if the user has permission for any of the groups.
-			for _, group := range groups {
-				// Get the user from the group. The group is in the format "group:role".
-				// Check if the user has permission using Casbin Enforcer.
-				if enforceRBAC(enforcer, group, resource, object, action) {
-					isAuthorized = true
-					c.Next()
-					break
-				}
-			}
+		routeMapKey := fmt.Sprintf("%s:%s", c.Request.Method, c.FullPath())
+		// Check if the route requires authorization.
+		if authz.RouteMap[routeMapKey] != nil && authz.RouteMap[routeMapKey].RequiresAuthZ {
 			// If the user is not authorized, return an error.
-			if !isAuthorized {
-				errMsg := "user is not authorized to execute the requested action."
+			if isAuthorized, _ := authorizer.Authorize(c, userInfo.IDTokenClaims.Groups); !isAuthorized {
+				errMsg := "user is not authorized to execute the requested action"
 				c.JSON(http.StatusForbidden, v1.NewNumaflowAPIResponse(&errMsg, nil))
 				c.Abort()
+			} else {
+				// If the user is authorized, continue the request.
+				c.Next()
 			}
-		} else if authz.RouteMap[routeMapKey] != nil && !authz.RouteMap[routeMapKey].RequiresAuth {
-			// If the route does not require auth, skip the authz check.
+		} else if authz.RouteMap[routeMapKey] != nil && !authz.RouteMap[routeMapKey].RequiresAuthZ {
+			// If the route does not require AuthZ, skip the AuthZ check.
 			c.Next()
 		} else {
 			// If the route is not present in the route map, return an error.
@@ -195,26 +179,3 @@ func authMiddleware(enforcer *casbin.Enforcer, dexObj *v1.DexObject) gin.Handler
 		}
 	}
 }
-
-// authenticate authenticates the user by consulting Dex.
-func authenticate(c *gin.Context, dexObj *v1.DexObject) (*authn.UserIdInfo, error) {
-	userIdentityTokenStr, err := c.Cookie(common.UserIdentityCookieName)
-	if err != nil {
-		return nil, fmt.Errorf("failed to get user identity token from cookie: %v", err)
-	}
-	userIdentityToken, err := utils.ParseUserIdentityToken(userIdentityTokenStr)
-	if err != nil {
-		return nil, fmt.Errorf("failed to parse user identity token: %v", err)
-	}
-	_, err = dexObj.Verify(c, userIdentityToken.IDToken)
-	if err != nil {
-		return nil, err
-	}
-	return &userIdentityToken, nil
-}
-
-// enforceRBAC checks if the user has permission based on the Casbin model and policy.
-func enforceRBAC(enforcer *casbin.Enforcer, user, resource, object, action string) bool {
-	ok, _ := enforcer.Enforce(user, resource, object, action)
-	return ok
-}