RBAC checks for app list api

Author: akclace

internal/server/app_apis.go

@@ -512,7 +512,7 @@ func (s *Server) authenticateAndServeApp(w http.ResponseWriter, r *http.Request,
 	}
 
 	s.Trace().Msgf("Authenticated user %s, doing authorization check", userId)
-	authorized, err := s.rbacManager.AuthorizeAccess(userId, app.AppEntry.AppPathDomain(), string(appAuth), types.PermissionAccess)
+	authorized, err := s.rbacManager.Authorize(userId, app.AppEntry.AppPathDomain(), string(appAuth), types.PermissionAccess)
 	if err != nil {
 		http.Error(w, err.Error(), http.StatusInternalServerError)
 		return
@@ -916,8 +916,16 @@ func (s *Server) GetApps(ctx context.Context, appPathGlob string, internal bool)
 		return nil, types.CreateRequestError(err.Error(), http.StatusBadRequest)
 	}
 
+	userId := system.GetContextUserId(ctx)
 	ret := make([]types.AppResponse, 0, len(filteredApps))
 	for _, app := range filteredApps {
+		authorized, err := s.AuthorizeList(userId, &app)
+		if err != nil {
+			return nil, types.CreateRequestError(err.Error(), http.StatusInternalServerError)
+		}
+		if !authorized {
+			continue
+		}
 		retApp, err := s.GetApp(app.AppPathDomain, false)
 		if err != nil {
 			return nil, types.CreateRequestError(err.Error(), http.StatusInternalServerError)

internal/server/audit_middleware.go

@@ -13,9 +13,9 @@ import (
 	"sync/atomic"
 	"time"
 
+	"github.com/go-chi/chi/middleware"
 	"github.com/openrundev/openrun/internal/system"
 	"github.com/openrundev/openrun/internal/types"
-	"github.com/go-chi/chi/middleware"
 	"github.com/segmentio/ksuid"
 )
 

internal/server/openrun_plugin.go

@@ -36,30 +36,6 @@ type openrunPlugin struct {
 	server *Server
 }
 
-func (c *openrunPlugin) verifyHasAccess(userId string, appAuth types.AppAuthnType) bool {
-	if appAuth == types.AppAuthnDefault {
-		appAuth = types.AppAuthnType(c.server.config.Security.AppDefaultAuthType)
-	}
-
-	// Verify user_id as set in authenticateAndServeApp
-	if appAuth == "" || appAuth == types.AppAuthnNone {
-		// No auth required for this app, allow access
-		return true
-	} else if appAuth == types.AppAuthnSystem {
-		return userId == types.ADMIN_USER
-	} else if appAuth == "cert" || strings.HasPrefix(string(appAuth), "cert_") {
-		return userId == string(appAuth)
-	} else {
-		provider, _, ok := strings.Cut(string(userId), ":")
-		if !ok {
-			c.server.Warn().Str("user_id", userId).Msg("Unknown user_id format")
-			return false
-		}
-		// Check Oauth provider is the same as the app's provider
-		return provider == string(appAuth)
-	}
-}
-
 func (c *openrunPlugin) ListAllApps(thread *starlark.Thread, builtin *starlark.Builtin, args starlark.Tuple, kwargs []starlark.Tuple) (starlark.Value, error) {
 	return c.listAppsImpl(thread, builtin, args, kwargs, false, "list_all_apps")
 }
@@ -102,10 +78,6 @@ func (c *openrunPlugin) listAppsImpl(thread *starlark.Thread, _ *starlark.Builti
 	userId := system.GetRequestUserId(thread)
 	ret := starlark.List{}
 	for _, app := range apps {
-		if permCheck && !c.verifyHasAccess(userId, app.Auth) {
-			continue
-		}
-
 		// Filter out internal apps
 		if app.MainApp != "" && !bool(include_internal) {
 			continue
@@ -143,6 +115,16 @@ func (c *openrunPlugin) listAppsImpl(thread *starlark.Thread, _ *starlark.Builti
 			}
 		}
 
+		if permCheck {
+			hasAccess, err := c.server.AuthorizeList(userId, &app)
+			if err != nil {
+				return nil, err
+			}
+			if !hasAccess {
+				continue
+			}
+		}
+
 		v := starlark.Dict{}
 		v.SetKey(starlark.String("name"), starlark.String(app.Name))
 		v.SetKey(starlark.String("url"), starlark.String(types.GetAppUrl(app.AppPathDomain, c.server.config)))

internal/server/rbac.go

@@ -40,25 +40,30 @@ func NewRBACHandler(logger *types.Logger, rbacConfig *types.RBACConfig, serverCo
 	return rbacManager, nil
 }
 
-func (h *RBACManager) AuthorizeAccess(user string, appPathDomain types.AppPathDomain, appAuthSetting string, permission types.RBACPermission) (bool, error) {
+func (h *RBACManager) Authorize(user string, appPathDomain types.AppPathDomain, appAuthSetting string, permission types.RBACPermission) (bool, error) {
 	h.mu.RLock()
 	defer h.mu.RUnlock()
 
 	if !h.rbacConfig.Enabled {
-		// rbac is not enabled, authorize all access
+		// rbac is not enabled, authorize all requests
 		return true, nil
 	}
 
-	if user != "" && user == h.serverConfig.AdminUser {
+	if user != "" && user == types.ADMIN_USER {
 		// admin user is always authorized if enabled
 		return true, nil
 	}
 
-	if !strings.HasPrefix(appAuthSetting, RBAC_AUTH_PREFIX) {
-		// if app auth is not rbac enabled, authorize access
+	if !strings.HasPrefix(appAuthSetting, RBAC_AUTH_PREFIX) && permission == types.PermissionAccess {
+		// if app auth does not have rbac enabled, authorize access for Access permission
+		// If authenticated, then app access is allowed
 		return true, nil
 	}
 
+	// Trim stage and preview suffixes, grant check are done on the main app path
+	appPathDomain.Path = strings.TrimSuffix(appPathDomain.Path, types.STAGE_SUFFIX)
+	appPathDomain.Path = strings.TrimSuffix(appPathDomain.Path, types.PREVIEW_SUFFIX)
+
 	return h.checkGrants(user, appPathDomain, permission)
 }
 

internal/server/rbac_test.go

@@ -500,7 +500,7 @@ func TestAuthorizeAccess(t *testing.T) {
 				t.Fatalf("failed to create RBACManager: %v", err)
 			}
 
-			result, err := rbacManager.AuthorizeAccess(tt.user, tt.appPathDomain, tt.appAuthSetting, tt.permission)
+			result, err := rbacManager.Authorize(tt.user, tt.appPathDomain, tt.appAuthSetting, tt.permission)
 
 			if tt.expectError {
 				if err == nil {
@@ -600,7 +600,7 @@ func TestAuthorizeAccessWithGroupHierarchy(t *testing.T) {
 				t.Fatalf("failed to create RBACManager: %v", err)
 			}
 
-			result, err := rbacManager.AuthorizeAccess(tt.user, tt.appPathDomain, "rbac:test", tt.permission)
+			result, err := rbacManager.Authorize(tt.user, tt.appPathDomain, "rbac:test", tt.permission)
 			if err != nil {
 				t.Errorf("unexpected error: %v", err)
 				return
@@ -710,7 +710,7 @@ func TestAuthorizeAccessWithRoleHierarchy(t *testing.T) {
 				t.Fatalf("failed to create RBACManager: %v", err)
 			}
 
-			result, err := rbacManager.AuthorizeAccess(tt.user, tt.appPathDomain, "rbac:test", tt.permission)
+			result, err := rbacManager.Authorize(tt.user, tt.appPathDomain, "rbac:test", tt.permission)
 			if err != nil {
 				t.Errorf("unexpected error: %v", err)
 				return
@@ -833,7 +833,7 @@ func TestUpdateRBACConfig(t *testing.T) {
 			}
 
 			// Test that the update worked by checking authorization
-			result, err := rbacManager.AuthorizeAccess("user1", types.AppPathDomain{Path: "/new", Domain: ""}, "rbac:test", types.PermissionList)
+			result, err := rbacManager.Authorize("user1", types.AppPathDomain{Path: "/new", Domain: ""}, "rbac:test", types.PermissionList)
 			if err != nil {
 				t.Errorf("unexpected error during authorization test: %v", err)
 				return
@@ -884,7 +884,7 @@ func TestAuthorizeAccessConcurrency(t *testing.T) {
 		go func(user string) {
 			defer func() { done <- true }()
 
-			result, err := rbacManager.AuthorizeAccess(user, types.AppPathDomain{Path: "/test", Domain: ""}, "rbac:test", types.PermissionList)
+			result, err := rbacManager.Authorize(user, types.AppPathDomain{Path: "/test", Domain: ""}, "rbac:test", types.PermissionList)
 			if err != nil {
 				t.Errorf("unexpected error: %v", err)
 				return

internal/server/server.go

@@ -300,12 +300,12 @@ func (s *Server) UpdateDynamicConfig(ctx context.Context, newConfig *types.Dynam
 	}
 
 	newConfig.VersionId = "ver_" + ksuid.New().String()
-	err := s.db.UpdateConfig(ctx, system.GetContextUserId(ctx), currentVersionId, newConfig)
+	err := s.updateDynamicConfigCache(ctx, newConfig)
 	if err != nil {
 		return nil, fmt.Errorf("error updating dynamic config: %w", err)
 	}
 
-	err = s.updateDynamicConfigCache(ctx, newConfig)
+	err = s.db.UpdateConfig(ctx, system.GetContextUserId(ctx), currentVersionId, newConfig)
 	if err != nil {
 		return nil, fmt.Errorf("error updating dynamic config: %w", err)
 	}
@@ -841,3 +841,43 @@ func (s *Server) ParseGlob(appGlob string) ([]types.AppInfo, error) {
 
 	return matched, nil
 }
+
+// AuthorizeList checks if the user has access to perform list operation on the specified app
+// For RBAC mode, uses RBAC permissions. For non-RBAC mode, look at whether app is using
+// same authentication types as used by the caller
+func (s *Server) AuthorizeList(userId string, app *types.AppInfo) (bool, error) {
+	appAuthStr := string(app.Auth)
+	if s.rbacManager.rbacConfig.Enabled {
+		// RBAC auth is enabled, verify access
+		return s.rbacManager.Authorize(userId, app.AppPathDomain, appAuthStr, types.PermissionList)
+	}
+
+	if userId != "" && userId == types.ADMIN_USER {
+		// Admin user is always authorized
+		return true, nil
+	}
+
+	appAuthStr = strings.TrimPrefix(appAuthStr, RBAC_AUTH_PREFIX)
+	appAuth := types.AppAuthnType(appAuthStr)
+	if appAuth == types.AppAuthnDefault {
+		appAuth = types.AppAuthnType(s.config.Security.AppDefaultAuthType)
+	}
+
+	// Verify user_id as set in authenticateAndServeApp
+	if appAuth == "" || appAuth == types.AppAuthnNone {
+		// No auth required for this app, authorize access
+		return true, nil
+	} else if appAuth == types.AppAuthnSystem {
+		return userId != "" && userId == types.ADMIN_USER, nil
+	} else if appAuth == "cert" || strings.HasPrefix(string(appAuth), "cert_") {
+		return userId == string(appAuth), nil
+	} else {
+		provider, _, ok := strings.Cut(string(userId), ":")
+		if !ok {
+			s.Warn().Str("user_id", userId).Msg("Unknown user_id format")
+			return false, nil
+		}
+		// Check Oauth provider is the same as the app's provider
+		return provider == string(appAuth), nil
+	}
+}