Skip to content

Commit

Permalink
fix: access path for auth endpoints (#1403)
Browse files Browse the repository at this point in the history 
Signed-off-by: veds-g <[email protected]>
  • Loading branch information
@veds-g
veds-g authored Dec 7, 2023
1 parent 0db9cd1 commit 263263b
Show file tree
Hide file tree
Showing 8 changed files with 85 additions and 62 deletions.
20 changes: 10 additions & 10 deletions server/authz/rbac.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,26 +46,28 @@ const (
)

// CasbinObject is the struct that implements the Authorizer interface.
// It contains the Casbin Enforcer, the current scopes, the default policy and the config reader.
// It contains the Casbin Enforcer, the current scopes, the default policy, the config reader and the route map.
// The config reader is used to watch for changes in the config file.
// The Casbin Enforcer is used to enforce the authorization policy.
// The current scopes are used to determine the user identity token to be used for authorization.
// policyDefault is the default policy to be used when the requested resource is not present in the policy.
// userPermCount is a cache to store the count of permissions for a user. If the user has permissions in the
// policy, we store the count in the cache and return based on the value.
// authRouteMap is a map of routes to their corresponding RouteInfo objects.
type CasbinObject struct {
enforcer *casbin.Enforcer
userPermCount *sync.Map
currentScopes []string
policyDefault string
configReader *viper.Viper
authRouteMap RouteMap
opts *options
rwMutex *sync.RWMutex
}

// NewCasbinObject returns a new CasbinObject. It initializes the Casbin Enforcer with the model and policy.
// It also initializes the config reader to watch for changes in the config file.
func NewCasbinObject(inputOptions ...Option) (*CasbinObject, error) {
func NewCasbinObject(authRouteMap RouteMap, inputOptions ...Option) (*CasbinObject, error) {
// Set the default options.
var opts = DefaultOptions()
// Apply the input options.
Expand Down Expand Up @@ -93,6 +95,7 @@ func NewCasbinObject(inputOptions ...Option) (*CasbinObject, error) {
currentScopes: currentScopes,
policyDefault: policyDefault,
configReader: configReader,
authRouteMap: authRouteMap,
opts: opts,
rwMutex: &sync.RWMutex{},
}
Expand All @@ -116,7 +119,7 @@ func (cas *CasbinObject) Authorize(c *gin.Context, userInfo *authn.UserInfo) boo
scopedList := getSubjectFromScope(currentScopes, userInfo)
// Get the resource, object and action from the request.
resource := extractResource(c)
object := extractObject(c)
object := extractObject(c, cas.authRouteMap)
action := c.Request.Method
userHasPolicies := false
// Check for the given scoped list if the user is authorized using any of the subjects in the list.
Expand Down Expand Up @@ -235,13 +238,10 @@ func extractResource(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())
// Return the object from the route map.
if RouteMap[routeMapKey] != nil {
return RouteMap[routeMapKey].Object
func extractObject(c *gin.Context, authRouteMap RouteMap) string {
// Return the object from the route map from context.
if authRouteMap.GetRouteFromContext(c) != nil {
return authRouteMap.GetRouteFromContext(c).Object
}
return emptyString
}
Expand Down
12 changes: 6 additions & 6 deletions server/authz/rbac_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ var (
// TestCreateAuthorizer is a test implementation of the NewCasbinObject function.
// It checks that the authorizer is created correctly and the policies, configs are loaded correctly.
func TestCreateAuthorizer(t *testing.T) {
authorizer, err := NewCasbinObject(WithPolicyMap(testPolicyMapPath), WithPropertyFile(testPropertyFilePath))
authorizer, err := NewCasbinObject(RouteMap{}, WithPolicyMap(testPolicyMapPath), WithPropertyFile(testPropertyFilePath))
assert.NoError(t, err)

// Test that the authorizer is not nil
Expand All @@ -56,7 +56,7 @@ func TestCreateAuthorizer(t *testing.T) {
// TestAuthorize is a test implementation of the Authorize functionality.
// It tests that the user is authorized correctly for the given request.
func TestAuthorize(t *testing.T) {
authorizer, err := NewCasbinObject(WithPolicyMap(testPolicyMapPath), WithPropertyFile(testPropertyFilePath))
authorizer, err := NewCasbinObject(RouteMap{}, WithPolicyMap(testPolicyMapPath), WithPropertyFile(testPropertyFilePath))
assert.NoError(t, err)

// Test that the authorizer is not nil
Expand Down Expand Up @@ -92,7 +92,7 @@ func TestAuthorize(t *testing.T) {
// Additionally, it tests that the default policy is applied correctly when a user is not found in the policy map.
// The default policy is set to "role:readonly" in the test data.
func TestDefaultPolicy(t *testing.T) {
authorizer, err := NewCasbinObject(WithPolicyMap(testPolicyMapPath), WithPropertyFile(testPropertyFilePath))
authorizer, err := NewCasbinObject(RouteMap{}, WithPolicyMap(testPolicyMapPath), WithPropertyFile(testPropertyFilePath))
assert.NoError(t, err)

// Test that the authorizer is not nil
Expand All @@ -119,7 +119,7 @@ func TestDefaultPolicy(t *testing.T) {
// It tests that the scopes are set correctly when the authorizer is created.
// Additionally, it tests that the required scopes are tested for the user.
func TestScopes(t *testing.T) {
authorizer, err := NewCasbinObject(WithPolicyMap(testPolicyMapPath), WithPropertyFile(testPropertyFilePath))
authorizer, err := NewCasbinObject(RouteMap{}, WithPolicyMap(testPolicyMapPath), WithPropertyFile(testPropertyFilePath))
assert.NoError(t, err)

// Test that the authorizer is not nil
Expand Down Expand Up @@ -161,7 +161,7 @@ func TestScopes(t *testing.T) {
// TestNamespaces is a test implementation of the Namespaces based access.
// It tests that a user can access a namespace that is in the policy map.
func TestNamespaces(t *testing.T) {
authorizer, err := NewCasbinObject(WithPolicyMap(testPolicyMapPath), WithPropertyFile(testPropertyFilePath))
authorizer, err := NewCasbinObject(RouteMap{}, WithPolicyMap(testPolicyMapPath), WithPropertyFile(testPropertyFilePath))
assert.NoError(t, err)

// Test that the authorizer is not nil
Expand All @@ -186,7 +186,7 @@ func TestNamespaces(t *testing.T) {
func TestConfigFileReload(t *testing.T) {
ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
defer cancel()
authorizer, err := NewCasbinObject(WithPolicyMap(testPolicyMapPath), WithPropertyFile(testPropertyReloadFilePath))
authorizer, err := NewCasbinObject(RouteMap{}, WithPolicyMap(testPolicyMapPath), WithPropertyFile(testPropertyReloadFilePath))
assert.NoError(t, err)

// Test that the authorizer is not nil
Expand Down
45 changes: 14 additions & 31 deletions server/authz/route_map.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,49 +30,32 @@ type RouteInfo struct {
RequiresAuthZ bool
}

// newRouteInfo creates a new RouteInfo object.
func newRouteInfo(object string, requiresAuthZ bool) *RouteInfo {
// NewRouteInfo creates a new RouteInfo object.
func NewRouteInfo(object string, requiresAuthZ bool) *RouteInfo {
return &RouteInfo{
Object: object,
RequiresAuthZ: requiresAuthZ,
}
}

// RouteMap is a map of routes to their corresponding RouteInfo objects.
// RouteMap type 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),
"GET:/api/v1/namespaces": newRouteInfo(ObjectEvents, false),
"GET:/api/v1/cluster-summary": newRouteInfo(ObjectPipeline, false),
"GET:/api/v1/namespaces/:namespace/pipelines": newRouteInfo(ObjectPipeline, true),
"POST:/api/v1/namespaces/:namespace/pipelines": newRouteInfo(ObjectPipeline, true),
"GET:/api/v1/namespaces/:namespace/pipelines/:pipeline": newRouteInfo(ObjectPipeline, true),
"GET:/api/v1/namespaces/:namespace/pipelines/:pipeline/health": newRouteInfo(ObjectPipeline, true),
"PUT:/api/v1/namespaces/:namespace/pipelines/:pipeline": newRouteInfo(ObjectPipeline, true),
"DELETE:/api/v1/namespaces/:namespace/pipelines/:pipeline": newRouteInfo(ObjectPipeline, true),
"PATCH:/api/v1/namespaces/:namespace/pipelines/:pipeline": newRouteInfo(ObjectPipeline, true),
"POST:/api/v1/namespaces/:namespace/isb-services": newRouteInfo(ObjectISBSvc, true),
"GET:/api/v1/namespaces/:namespace/isb-services": newRouteInfo(ObjectISBSvc, true),
"GET:/api/v1/namespaces/:namespace/isb-services/:isb-service": newRouteInfo(ObjectISBSvc, true),
"PUT:/api/v1/namespaces/:namespace/isb-services/:isb-service": newRouteInfo(ObjectISBSvc, true),
"DELETE:/api/v1/namespaces/:namespace/isb-services/:isb-service": newRouteInfo(ObjectISBSvc, true),
"GET:/api/v1/namespaces/:namespace/pipelines/:pipeline/isbs": newRouteInfo(ObjectPipeline, true),
"GET:/api/v1/namespaces/:namespace/pipelines/:pipeline/watermarks": newRouteInfo(ObjectPipeline, true),
"PUT:/api/v1/namespaces/:namespace/pipelines/:pipeline/vertices/:vertex": newRouteInfo(ObjectPipeline, true),
"GET:/api/v1/namespaces/:namespace/pipelines/:pipeline/vertices/metrics": newRouteInfo(ObjectPipeline, true),
"GET:/api/v1/namespaces/:namespace/pipelines/:pipeline/vertices/:vertex/pods": newRouteInfo(ObjectPipeline, true),
"GET:/api/v1/metrics/namespaces/:namespace/pods": newRouteInfo(ObjectPipeline, true),
"GET:/api/v1/namespaces/:namespace/pods/:pod/logs": newRouteInfo(ObjectPipeline, true),
"GET:/api/v1/namespaces/:namespace/events": newRouteInfo(ObjectEvents, true),
}
type RouteMap map[string]*RouteInfo

// GetRouteMapKey returns the key for the RouteMap.
// GetRouteMapKey returns the key for the AuthRouteMap.
// The key is a combination of the HTTP method and the path.
// The format is "method:path".
// For example, "GET:/api/v1/namespaces", "POST:/api/v1/namespaces".
// This key is used to get the RouteInfo object from the RouteMap.
// This key is used to get the RouteInfo object from the AuthRouteMap.
func GetRouteMapKey(c *gin.Context) string {
return fmt.Sprintf("%s:%s", c.Request.Method, c.FullPath())
}

// GetRouteFromContext returns the RouteInfo object from the AuthRouteMap based on the context.
func (r RouteMap) GetRouteFromContext(c *gin.Context) *RouteInfo {
if routeEntry, ok := r[GetRouteMapKey(c)]; ok {
return routeEntry
}
return nil
}
37 changes: 37 additions & 0 deletions server/cmd/start.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import (
"github.com/numaproj/numaflow/pkg/shared/logging"
sharedtls "github.com/numaproj/numaflow/pkg/shared/tls"
v1 "github.com/numaproj/numaflow/server/apis/v1"
"github.com/numaproj/numaflow/server/authz"
"github.com/numaproj/numaflow/server/routes"
)

Expand Down Expand Up @@ -66,6 +67,8 @@ func (s *server) Start() {
router := gin.New()
router.Use(gin.LoggerWithConfig(gin.LoggerConfig{SkipPaths: []string{"/livez"}}))
router.RedirectTrailingSlash = true
// sets the route map for authorization with the base href
authRouteMap := CreateAuthRouteMap(s.options.BaseHref)
router.Use(static.Serve(s.options.BaseHref, static.LocalFile("./ui/build", true)))
if s.options.BaseHref != "/" {
router.NoRoute(func(c *gin.Context) {
Expand All @@ -85,6 +88,7 @@ func (s *server) Start() {
ServerAddr: s.options.ServerAddr,
},
s.options.BaseHref,
authRouteMap,
)
router.Use(UrlRewrite(router))
server := http.Server{
Expand Down Expand Up @@ -138,3 +142,36 @@ func UrlRewrite(r *gin.Engine) gin.HandlerFunc {
c.Next()
}
}

// CreateAuthRouteMap creates the route map for authorization.
// The key is a combination of the HTTP method and the path along with the baseHref.
// For example, "GET:/api/v1/namespaces" becomes "GET:/baseHref/api/v1/namespaces".
// The value is a RouteInfo object.
func CreateAuthRouteMap(baseHref string) authz.RouteMap {
return authz.RouteMap{
"GET:" + baseHref + "api/v1/sysinfo": authz.NewRouteInfo(authz.ObjectPipeline, false),
"GET:" + baseHref + "api/v1/authinfo": authz.NewRouteInfo(authz.ObjectEvents, false),
"GET:" + baseHref + "api/v1/namespaces": authz.NewRouteInfo(authz.ObjectEvents, false),
"GET:" + baseHref + "api/v1/cluster-summary": authz.NewRouteInfo(authz.ObjectPipeline, false),
"GET:" + baseHref + "api/v1/namespaces/:namespace/pipelines": authz.NewRouteInfo(authz.ObjectPipeline, true),
"POST:" + baseHref + "api/v1/namespaces/:namespace/pipelines": authz.NewRouteInfo(authz.ObjectPipeline, true),
"GET:" + baseHref + "api/v1/namespaces/:namespace/pipelines/:pipeline": authz.NewRouteInfo(authz.ObjectPipeline, true),
"GET:" + baseHref + "api/v1/namespaces/:namespace/pipelines/:pipeline/health": authz.NewRouteInfo(authz.ObjectPipeline, true),
"PUT:" + baseHref + "api/v1/namespaces/:namespace/pipelines/:pipeline": authz.NewRouteInfo(authz.ObjectPipeline, true),
"DELETE:" + baseHref + "api/v1/namespaces/:namespace/pipelines/:pipeline": authz.NewRouteInfo(authz.ObjectPipeline, true),
"PATCH:" + baseHref + "api/v1/namespaces/:namespace/pipelines/:pipeline": authz.NewRouteInfo(authz.ObjectPipeline, true),
"POST:" + baseHref + "api/v1/namespaces/:namespace/isb-services": authz.NewRouteInfo(authz.ObjectISBSvc, true),
"GET:" + baseHref + "api/v1/namespaces/:namespace/isb-services": authz.NewRouteInfo(authz.ObjectISBSvc, true),
"GET:" + baseHref + "api/v1/namespaces/:namespace/isb-services/:isb-service": authz.NewRouteInfo(authz.ObjectISBSvc, true),
"PUT:" + baseHref + "api/v1/namespaces/:namespace/isb-services/:isb-service": authz.NewRouteInfo(authz.ObjectISBSvc, true),
"DELETE:" + baseHref + "api/v1/namespaces/:namespace/isb-services/:isb-service": authz.NewRouteInfo(authz.ObjectISBSvc, true),
"GET:" + baseHref + "api/v1/namespaces/:namespace/pipelines/:pipeline/isbs": authz.NewRouteInfo(authz.ObjectPipeline, true),
"GET:" + baseHref + "api/v1/namespaces/:namespace/pipelines/:pipeline/watermarks": authz.NewRouteInfo(authz.ObjectPipeline, true),
"PUT:" + baseHref + "api/v1/namespaces/:namespace/pipelines/:pipeline/vertices/:vertex": authz.NewRouteInfo(authz.ObjectPipeline, true),
"GET:" + baseHref + "api/v1/namespaces/:namespace/pipelines/:pipeline/vertices/metrics": authz.NewRouteInfo(authz.ObjectPipeline, true),
"GET:" + baseHref + "api/v1/namespaces/:namespace/pipelines/:pipeline/vertices/:vertex/pods": authz.NewRouteInfo(authz.ObjectPipeline, true),
"GET:" + baseHref + "api/v1/metrics/namespaces/:namespace/pods": authz.NewRouteInfo(authz.ObjectPipeline, true),
"GET:" + baseHref + "api/v1/namespaces/:namespace/pods/:pod/logs": authz.NewRouteInfo(authz.ObjectPipeline, true),
"GET:" + baseHref + "api/v1/namespaces/:namespace/events": authz.NewRouteInfo(authz.ObjectEvents, true),
}
}
18 changes: 8 additions & 10 deletions server/routes/routes.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ type AuthInfo struct {

var logger = logging.NewLogger().Named("server")

func Routes(r *gin.Engine, sysInfo SystemInfo, authInfo AuthInfo, baseHref string) {
func Routes(r *gin.Engine, sysInfo SystemInfo, authInfo AuthInfo, baseHref string, authRouteMap authz.RouteMap) {
r.GET("/livez", func(c *gin.Context) {
c.Status(http.StatusOK)
})
Expand All @@ -51,19 +51,19 @@ func Routes(r *gin.Engine, sysInfo SystemInfo, authInfo AuthInfo, baseHref strin
panic(err)
}
// noAuthGroup is a group of routes that do not require AuthN/AuthZ no matter whether auth is enabled.
noAuthGroup := r.Group("/auth/v1")
noAuthGroup := r.Group(baseHref + "auth/v1")
v1RoutesNoAuth(noAuthGroup, dexObj)

// r1Group is a group of routes that require AuthN/AuthZ when auth is enabled.
// they share the AuthN/AuthZ middleware.
r1Group := r.Group(baseHref + "api/v1")
if !authInfo.DisableAuth {
authorizer, err := authz.NewCasbinObject()
authorizer, err := authz.NewCasbinObject(authRouteMap)
if err != nil {
panic(err)
}
// Add the AuthN/AuthZ middleware to the group.
r1Group.Use(authMiddleware(authorizer, dexObj))
r1Group.Use(authMiddleware(authorizer, dexObj, authRouteMap))
v1Routes(r1Group, dexObj)
} else {
v1Routes(r1Group, nil)
Expand Down Expand Up @@ -144,7 +144,7 @@ func v1Routes(r gin.IRouter, dexObj *v1.DexObject) {
// 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 {
func authMiddleware(authorizer authz.Authorizer, authenticator authn.Authenticator, authRouteMap authz.RouteMap) gin.HandlerFunc {
return func(c *gin.Context) {
// Authenticate the user.
userInfo, err := authenticator.Authenticate(c)
Expand All @@ -154,10 +154,8 @@ func authMiddleware(authorizer authz.Authorizer, authenticator authn.Authenticat
c.Abort()
return
}
// Get the route map from the context. Key is in the format "method:path".
routeMapKey := authz.GetRouteMapKey(c)
// Check if the route requires authorization.
if authz.RouteMap[routeMapKey] != nil && authz.RouteMap[routeMapKey].RequiresAuthZ {
if authRouteMap.GetRouteFromContext(c) != nil && authRouteMap.GetRouteFromContext(c).RequiresAuthZ {
// Check if the user is authorized to execute the requested action.
isAuthorized := authorizer.Authorize(c, userInfo)
if isAuthorized {
Expand All @@ -169,12 +167,12 @@ func authMiddleware(authorizer authz.Authorizer, authenticator authn.Authenticat
c.JSON(http.StatusForbidden, v1.NewNumaflowAPIResponse(&errMsg, nil))
c.Abort()
}
} else if authz.RouteMap[routeMapKey] != nil && !authz.RouteMap[routeMapKey].RequiresAuthZ {
} else if authRouteMap.GetRouteFromContext(c) != nil && !authRouteMap.GetRouteFromContext(c).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.
logger.Errorw("route not present in routeMap", "route", routeMapKey)
logger.Errorw("route not present in routeMap", "route", authz.GetRouteMapKey(c))
errMsg := "Invalid route"
c.JSON(http.StatusForbidden, v1.NewNumaflowAPIResponse(&errMsg, nil))
c.Abort()
Expand Down
4 changes: 3 additions & 1 deletion server/routes/routes_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ limitations under the License.
package routes

import (
"github.com/numaproj/numaflow/server/authz"
"net/http"
"net/http/httptest"
"testing"
Expand All @@ -41,7 +42,8 @@ func TestRoutes(t *testing.T) {
DisableAuth: false,
DexServerAddr: "test-dex-server-addr",
}
Routes(router, sysInfo, authInfo, "/")
authRouteMap := authz.RouteMap{}
Routes(router, sysInfo, authInfo, "/", authRouteMap)
t.Run("/404", func(t *testing.T) {
w := httptest.NewRecorder()
req, err := http.NewRequest(http.MethodGet, "/404", nil)
Expand Down
3 changes: 2 additions & 1 deletion ui/src/components/common/AccountMenu/index.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import IconButton from "@mui/material/IconButton";
import { AppContextProps } from "../../../types/declarations/app";
import { AppContext } from "../../../App";
import { useNavigate } from "react-router-dom";
import { getBaseHref } from "../../../utils";
import Chip from "@mui/material/Chip";

export default function AccountMenu() {
Expand All @@ -12,7 +13,7 @@ export default function AccountMenu() {

const handleLogout = useCallback(async () => {
try {
const response = await fetch(`/auth/v1/logout`);
const response = await fetch(`${getBaseHref()}/auth/v1/logout`);
if (response.ok) {
navigate("/login");
}
Expand Down
Loading

0 comments on commit 263263b

Please sign in to comment.