Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: allow specifying disable-auth when creating the server #1251

Merged
merged 13 commits into from
Oct 23, 2023
2 changes: 1 addition & 1 deletion cmd/commands/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ func NewServerCommand() *cobra.Command {
command.Flags().BoolVar(&namespaced, "namespaced", false, "Whether to run in namespaced scope, defaults to false.")
command.Flags().StringVar(&managedNamespace, "managed-namespace", sharedutil.LookupEnvStringOr("NAMESPACE", "numaflow-system"), "The namespace that the server watches when \"--namespaced\" is \"true\".")
command.Flags().StringVar(&baseHref, "base-href", "/", "Base href for Numaflow server, defaults to '/'.")
command.Flags().BoolVar(&disableAuth, "disable-auth", false, "Whether to disable authentication, defaults to false.")
command.Flags().BoolVar(&disableAuth, "disable-auth", true, "Whether to disable authentication and authorization, defaults to true for easy on-boarding.")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be better to set the default to false here, but add the arg true in the manifests.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sounds good.

command.Flags().StringVar(&dexServerAddr, "dex-server-addr", "http://numaflow-dex-server:5556", "The address of the Dex server.")
return command
}
1 change: 1 addition & 0 deletions config/advanced-install/namespaced-numaflow-server.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@ spec:
containers:
- args:
- server
- --disable-auth=true
- --namespaced
env:
- name: NAMESPACE
Expand Down
2 changes: 2 additions & 0 deletions config/base/numaflow-server/numaflow-server-deployment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ spec:
image: quay.io/numaproj/numaflow:latest
args:
- "server"
# By default, turn off authentication and authorization.
Copy link
Member Author

@KeranYang KeranYang Oct 23, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made this change, thinking if we by default enable auth, it could add complexity to first-time numaflow users. @jy4096 also mentioned that we want to disable auth for e2e API testing.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cc: @vigith This also replies to your comment above. :)

- "--disable-auth=true"
imagePullPolicy: Always
volumeMounts:
- mountPath: /ui/build/runtime-env.js
Expand Down
1 change: 0 additions & 1 deletion server/apis/v1/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,6 @@ func NewHandler() (*handler, error) {
kubeClient: kubeClient,
metricsClient: metricsClient,
numaflowClient: numaflowClient,
// TODO: get args like disableAuth, dexServerAddr in
}, nil
}

Expand Down
3 changes: 2 additions & 1 deletion server/auth/rbac.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,9 @@ package auth

import (
_ "embed"
"fmt"
)

/* commented the following codes out to make linter happy.
var (
//go:embed rbac-model.conf
rbacModel string
Expand All @@ -30,3 +30,4 @@ var (
func main() {
fmt.Print(rbacModel)
}
*/
24 changes: 20 additions & 4 deletions server/cmd/start.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,15 +70,28 @@ func (s *server) Start() {
c.File("./ui/build/index.html")
})
}
routes.Routes(router, routes.SystemInfo{ManagedNamespace: s.options.ManagedNamespace, Namespaced: s.options.Namespaced, Version: numaflow.GetVersion().String()})
routes.Routes(
router,
routes.SystemInfo{
ManagedNamespace: s.options.ManagedNamespace,
Namespaced: s.options.Namespaced,
Version: numaflow.GetVersion().String()},
routes.AuthInfo{
DisableAuth: s.options.DisableAuth,
DexServerAddr: s.options.DexServerAddr,
})
router.Use(UrlRewrite(router))
server := http.Server{
Addr: fmt.Sprintf(":%d", s.options.Port),
Handler: router,
}

if s.options.Insecure {
logger.Infow("Starting server (TLS disabled) on "+server.Addr, "version", numaflow.GetVersion())
logger.Infow(
"Starting server (TLS disabled) on "+server.Addr,
"version", numaflow.GetVersion(),
"disable-auth", s.options.DisableAuth,
"dex-server-addr", s.options.DexServerAddr)
if err := server.ListenAndServe(); err != nil {
panic(err)
}
Expand All @@ -88,8 +101,11 @@ func (s *server) Start() {
panic(err)
}
server.TLSConfig = &tls.Config{Certificates: []tls.Certificate{*cert}, MinVersion: tls.VersionTLS12}

logger.Infow("Starting server on "+server.Addr, "version", numaflow.GetVersion())
logger.Infow(
"Starting server on "+server.Addr,
"version", numaflow.GetVersion(),
"disable-auth", s.options.DisableAuth,
"dex-server-addr", s.options.DexServerAddr)
if err := server.ListenAndServeTLS("", ""); err != nil {
panic(err)
}
Expand Down
109 changes: 63 additions & 46 deletions server/routes/routes.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,53 +32,33 @@ type SystemInfo struct {
Version string `json:"version"`
}

func Routes(r *gin.Engine, sysinfo SystemInfo) {
type AuthInfo struct {
DisableAuth bool `json:"disableAuth"`
DexServerAddr string `json:"dexServerAddr"`
}

func Routes(r *gin.Engine, sysInfo SystemInfo, authInfo AuthInfo) {
r.GET("/livez", func(c *gin.Context) {
c.Status(http.StatusOK)
})

r.Any("/dex/*name", v1.DexReverseProxy)

// noAuthGroup is a group of routes that do not require AuthN/AuthZ no matter whether auth is enabled.
noAuthGroup := r.Group("/auth/v1")
v1RoutesNoAuth(noAuthGroup)
enforcer, _ := getEnforcer()
r1Group := r.Group("/api/v1")
r1Group.Use(func(c *gin.Context) {
userIdentityTokenStr, err := c.Cookie("user-identity-token")
if err != nil {
errMsg := "user is not authenticated."
c.JSON(http.StatusUnauthorized, v1.NewNumaflowAPIResponse(&errMsg, nil))
return
}
userIdentityToken := v1.GetUserIdentityToken(userIdentityTokenStr)
groups := userIdentityToken.IDTokenClaims.Groups
// user := c.DefaultQuery("user", "readonly")
ns := c.Param("namespace")
if ns == "" {
c.Next()
return
}
resource := "pipeline"
action := c.Request.Method
auth := false

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, ns, resource, action) {
auth = true
c.Next()
}
}
if !auth {
errMsg := "user is not authorized to execute the requested action."
c.JSON(http.StatusForbidden, v1.NewNumaflowAPIResponse(&errMsg, nil))
c.Abort()
}
})
// r1Group is a group of routes that require AuthN/AuthZ when auth is enabled.
// they share the AuthN/AuthZ middleware.
r1Group := r.Group("/api/v1")
enforcer, _ := getEnforcer()
if !authInfo.DisableAuth {
// Add the AuthN/AuthZ middleware to the group.
r1Group.Use(authMiddleware(enforcer))
}
v1Routes(r1Group)
r1Group.GET("/sysinfo", func(c *gin.Context) {
c.JSON(http.StatusOK, v1.NewNumaflowAPIResponse(nil, sysinfo))
c.JSON(http.StatusOK, v1.NewNumaflowAPIResponse(nil, sysInfo))
})
}

Expand Down Expand Up @@ -148,6 +128,43 @@ func v1Routes(r gin.IRouter) {
r.GET("/namespaces/:namespace/events", handler.GetNamespaceEvents)
}

func authMiddleware(enforcer *casbin.Enforcer) gin.HandlerFunc {
return func(c *gin.Context) {
userIdentityTokenStr, err := c.Cookie("user-identity-token")
if err != nil {
errMsg := "user is not authenticated."
c.JSON(http.StatusUnauthorized, v1.NewNumaflowAPIResponse(&errMsg, nil))
return
}
userIdentityToken := v1.GetUserIdentityToken(userIdentityTokenStr)
groups := userIdentityToken.IDTokenClaims.Groups
// user := c.DefaultQuery("user", "readonly")
ns := c.Param("namespace")
if ns == "" {
c.Next()
return
}
resource := "pipeline"
action := c.Request.Method
auth := false

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, ns, resource, action) {
auth = true
c.Next()
}
}
if !auth {
errMsg := "user is not authorized to execute the requested action."
c.JSON(http.StatusForbidden, v1.NewNumaflowAPIResponse(&errMsg, nil))
c.Abort()
}
}
}

func getEnforcer() (*casbin.Enforcer, error) {
modelText := `
[request_definition]
Expand Down Expand Up @@ -178,13 +195,13 @@ func getEnforcer() (*casbin.Enforcer, error) {
return nil, err
}
rules := [][]string{
[]string{"role:jyuadmin", "jyu-dex-poc*", "pipeline", "GET"},
[]string{"role:jyuadmin", "jyu-dex-poc*", "pipeline", "POST"},
[]string{"role:jyuadmin", "jyu-dex-poc*", "pipeline", "PATCH"},
[]string{"role:jyuadmin", "jyu-dex-poc*", "pipeline", "PUT"},
[]string{"role:jyuadmin", "jyu-dex-poc*", "pipeline", "DELETE"},
[]string{"role:jyuadmin", "jyu-dex-poc*", "pipeline", "UPDATE"},
[]string{"role:jyureadonly", "jyu-dex-poc*", "pipeline", "GET"},
{"role:jyuadmin", "jyu-dex-poc*", "pipeline", "GET"},
{"role:jyuadmin", "jyu-dex-poc*", "pipeline", "POST"},
{"role:jyuadmin", "jyu-dex-poc*", "pipeline", "PATCH"},
{"role:jyuadmin", "jyu-dex-poc*", "pipeline", "PUT"},
{"role:jyuadmin", "jyu-dex-poc*", "pipeline", "DELETE"},
{"role:jyuadmin", "jyu-dex-poc*", "pipeline", "UPDATE"},
{"role:jyureadonly", "jyu-dex-poc*", "pipeline", "GET"},
}

areRulesAdded, err := enforcer.AddPolicies(rules)
Expand All @@ -193,8 +210,8 @@ func getEnforcer() (*casbin.Enforcer, error) {
}

rulesGroup := [][]string{
[]string{"jyu-dex-poc:admin", "role:jyuadmin"},
[]string{"jyu-dex-poc:readonly", "role:jyureadonly"},
{"jyu-dex-poc:admin", "role:jyuadmin"},
{"jyu-dex-poc:readonly", "role:jyureadonly"},
}

areRulesAdded, err = enforcer.AddNamedGroupingPolicies("g", rulesGroup)
Expand Down
8 changes: 6 additions & 2 deletions server/routes/routes_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,12 @@ func TestRoutes(t *testing.T) {
ManagedNamespace: managedNamespace,
Namespaced: namespaced,
}
Routes(router, sysInfo)

authInfo := AuthInfo{
DisableAuth: false,
DexServerAddr: "test-dex-server-addr",
}
Routes(router, sysInfo, authInfo)
t.Run("/404", func(t *testing.T) {
w := httptest.NewRecorder()
req, err := http.NewRequest(http.MethodGet, "/404", nil)
Expand All @@ -52,5 +57,4 @@ func TestRoutes(t *testing.T) {
router.ServeHTTP(w, req)
assert.Equal(t, http.StatusOK, w.Code)
})

}