From c94eabeb40add797d07dd62407b85cbb0f3b6723 Mon Sep 17 00:00:00 2001 From: Jeff McCune Date: Thu, 29 Jan 2026 21:52:22 -0800 Subject: [PATCH] Add configurable OIDC group-to-role mapping via CLI flags Add --viewer-groups, --editor-groups, and --owner-groups flags that accept comma-separated OIDC group names to map to each built-in role. When unset, defaults to the original group names (viewer, editor, owner). Introduces GroupMapping struct in the rbac package that replaces the hardcoded switch statement, wired through Config -> Server -> Handler. Co-Authored-By: Claude Opus 4.5 --- cli/cli.go | 12 +++ cli/cli_test.go | 35 +++++++ console/console.go | 18 +++- console/rbac/rbac.go | 132 ++++++++++++++++++++++--- console/rbac/rbac_test.go | 170 ++++++++++++++++++++++++++++++++ console/secrets/authz.go | 16 +-- console/secrets/authz_test.go | 36 ++++--- console/secrets/handler.go | 18 ++-- console/secrets/handler_test.go | 73 +++++++------- 9 files changed, 433 insertions(+), 77 deletions(-) diff --git a/cli/cli.go b/cli/cli.go index 192a163..09cfb7f 100644 --- a/cli/cli.go +++ b/cli/cli.go @@ -12,6 +12,7 @@ import ( "github.com/spf13/cobra" "github.com/holos-run/holos-console/console" + "github.com/holos-run/holos-console/console/rbac" ) var ( @@ -26,6 +27,9 @@ var ( refreshTokenTTL string namespace string logLevel string + viewerGroups string + editorGroups string + ownerGroups string ) // Command returns the root cobra command for the CLI. @@ -79,6 +83,11 @@ func Command() *cobra.Command { // Kubernetes flags cmd.Flags().StringVar(&namespace, "namespace", "holos-console", "Kubernetes namespace for secrets") + // RBAC group mapping flags + cmd.Flags().StringVar(&viewerGroups, "viewer-groups", "", "Comma-separated OIDC groups that map to the viewer role (default: viewer)") + cmd.Flags().StringVar(&editorGroups, "editor-groups", "", "Comma-separated OIDC groups that map to the editor role (default: editor)") + cmd.Flags().StringVar(&ownerGroups, "owner-groups", "", "Comma-separated OIDC groups that map to the owner role (default: owner)") + // Logging flags cmd.PersistentFlags().StringVar(&logLevel, "log-level", "info", "Log level (debug, info, warn, error)") @@ -194,6 +203,9 @@ func Run(cmd *cobra.Command, args []string) error { IDTokenTTL: idTTL, RefreshTokenTTL: refreshTTL, Namespace: namespace, + ViewerGroups: rbac.ParseGroups(viewerGroups), + EditorGroups: rbac.ParseGroups(editorGroups), + OwnerGroups: rbac.ParseGroups(ownerGroups), } server := console.New(cfg) diff --git a/cli/cli_test.go b/cli/cli_test.go index c4fe2b4..50ed3e7 100644 --- a/cli/cli_test.go +++ b/cli/cli_test.go @@ -119,6 +119,41 @@ func TestDeriveIssuer(t *testing.T) { } } +func TestGroupFlags(t *testing.T) { + t.Run("viewer-groups flag is registered", func(t *testing.T) { + cmd := Command() + f := cmd.Flags().Lookup("viewer-groups") + if f == nil { + t.Fatal("expected --viewer-groups flag to be registered") + } + if f.DefValue != "" { + t.Errorf("expected empty default, got %q", f.DefValue) + } + }) + + t.Run("editor-groups flag is registered", func(t *testing.T) { + cmd := Command() + f := cmd.Flags().Lookup("editor-groups") + if f == nil { + t.Fatal("expected --editor-groups flag to be registered") + } + if f.DefValue != "" { + t.Errorf("expected empty default, got %q", f.DefValue) + } + }) + + t.Run("owner-groups flag is registered", func(t *testing.T) { + cmd := Command() + f := cmd.Flags().Lookup("owner-groups") + if f == nil { + t.Fatal("expected --owner-groups flag to be registered") + } + if f.DefValue != "" { + t.Errorf("expected empty default, got %q", f.DefValue) + } + }) +} + func TestTTLParsing(t *testing.T) { tests := []struct { name string diff --git a/console/console.go b/console/console.go index 2a8d70f..e5f78da 100644 --- a/console/console.go +++ b/console/console.go @@ -32,6 +32,7 @@ import ( "golang.org/x/net/http2/h2c" "github.com/holos-run/holos-console/console/oidc" + "github.com/holos-run/holos-console/console/rbac" "github.com/holos-run/holos-console/console/rpc" "github.com/holos-run/holos-console/console/secrets" "github.com/holos-run/holos-console/gen/holos/console/v1/consolev1connect" @@ -77,6 +78,18 @@ type Config struct { // Namespace is the Kubernetes namespace for secrets. // Default: "holos-console" Namespace string + + // ViewerGroups are the OIDC groups that map to the viewer role. + // When nil, defaults to ["viewer"]. + ViewerGroups []string + + // EditorGroups are the OIDC groups that map to the editor role. + // When nil, defaults to ["editor"]. + EditorGroups []string + + // OwnerGroups are the OIDC groups that map to the owner role. + // When nil, defaults to ["owner"]. + OwnerGroups []string } // OIDCConfig is the OIDC configuration injected into the frontend. @@ -186,8 +199,11 @@ func (s *Server) Serve(ctx context.Context) error { slog.Info("no kubernetes config available, using dummy-secret only") } + // Create RBAC group mapping from configuration + groupMapping := rbac.NewGroupMapping(s.cfg.ViewerGroups, s.cfg.EditorGroups, s.cfg.OwnerGroups) + // Register SecretsService (protected - requires auth) - secretsHandler := secrets.NewHandler(secretsK8s) + secretsHandler := secrets.NewHandler(secretsK8s, groupMapping) secretsPath, secretsHTTPHandler := consolev1connect.NewSecretsServiceHandler(secretsHandler, protectedInterceptors) mux.Handle(secretsPath, secretsHTTPHandler) diff --git a/console/rbac/rbac.go b/console/rbac/rbac.go index 45a60cb..0ae013b 100644 --- a/console/rbac/rbac.go +++ b/console/rbac/rbac.go @@ -63,26 +63,75 @@ func HasPermission(role Role, permission Permission) bool { return perms[permission] } -// MapGroupToRole maps a group name to a Role using case-insensitive matching. -// Returns RoleUnspecified for unknown groups. -func MapGroupToRole(group string) Role { - switch strings.ToLower(group) { - case "viewer": - return RoleViewer - case "editor": - return RoleEditor - case "owner": - return RoleOwner - default: +// GroupMapping holds the mapping from OIDC group names to roles. +// When custom groups are provided for a role, only those groups map to that role. +// When no custom groups are provided (nil), the default group name is used. +type GroupMapping struct { + groupToRole map[string]Role +} + +// NewGroupMapping creates a GroupMapping. For each role, if the provided slice is +// non-nil, those group names are used; otherwise the default group name +// ("viewer", "editor", or "owner") is used. +func NewGroupMapping(viewerGroups, editorGroups, ownerGroups []string) *GroupMapping { + if viewerGroups == nil { + viewerGroups = []string{"viewer"} + } + if editorGroups == nil { + editorGroups = []string{"editor"} + } + if ownerGroups == nil { + ownerGroups = []string{"owner"} + } + + m := make(map[string]Role) + for _, g := range viewerGroups { + m[strings.ToLower(g)] = RoleViewer + } + for _, g := range editorGroups { + m[strings.ToLower(g)] = RoleEditor + } + for _, g := range ownerGroups { + m[strings.ToLower(g)] = RoleOwner + } + + return &GroupMapping{groupToRole: m} +} + +// ParseGroups splits a comma-separated string into a slice of trimmed group names. +// Returns nil for an empty string. +func ParseGroups(s string) []string { + if s == "" { + return nil + } + parts := strings.Split(s, ",") + groups := make([]string, 0, len(parts)) + for _, p := range parts { + g := strings.TrimSpace(p) + if g != "" { + groups = append(groups, g) + } + } + if len(groups) == 0 { + return nil + } + return groups +} + +// MapGroupToRole maps a group name to a Role using the configured mapping. +func (gm *GroupMapping) MapGroupToRole(group string) Role { + role, ok := gm.groupToRole[strings.ToLower(group)] + if !ok { return RoleUnspecified } + return role } // MapGroupsToRoles maps a slice of group names to roles, filtering out unknown groups. -func MapGroupsToRoles(groups []string) []Role { +func (gm *GroupMapping) MapGroupsToRoles(groups []string) []Role { roles := make([]Role, 0, len(groups)) for _, g := range groups { - role := MapGroupToRole(g) + role := gm.MapGroupToRole(g) if role != RoleUnspecified { roles = append(roles, role) } @@ -90,6 +139,63 @@ func MapGroupsToRoles(groups []string) []Role { return roles } +// CheckAccess verifies that the user has at least one role that grants the required permission. +// This is the method form that uses the configured group mapping. +func (gm *GroupMapping) CheckAccess(userGroups, allowedRoles []string, permission Permission) error { + // Map user groups to roles + userRoles := gm.MapGroupsToRoles(userGroups) + + // Find the minimum required role level from allowed roles + minLevel := -1 + for _, r := range allowedRoles { + role := gm.MapGroupToRole(r) + if role != RoleUnspecified { + level := roleLevel[role] + if minLevel < 0 || level < minLevel { + minLevel = level + } + } + } + + // If no valid allowed roles, deny access + if minLevel < 0 { + return connect.NewError( + connect.CodePermissionDenied, + fmt.Errorf("RBAC: authorization denied (allowed roles: [%s])", + strings.Join(allowedRoles, " ")), + ) + } + + // Check if any user role is at or above the minimum level AND has the required permission + for _, userRole := range userRoles { + if roleLevel[userRole] >= minLevel && HasPermission(userRole, permission) { + return nil + } + } + + return connect.NewError( + connect.CodePermissionDenied, + fmt.Errorf("RBAC: authorization denied (allowed roles: [%s])", + strings.Join(allowedRoles, " ")), + ) +} + +// defaultMapping is the package-level default GroupMapping using built-in group names. +var defaultMapping = NewGroupMapping(nil, nil, nil) + +// MapGroupToRole maps a group name to a Role using case-insensitive matching. +// Returns RoleUnspecified for unknown groups. +// Uses the default group mapping (viewer, editor, owner). +func MapGroupToRole(group string) Role { + return defaultMapping.MapGroupToRole(group) +} + +// MapGroupsToRoles maps a slice of group names to roles, filtering out unknown groups. +// Uses the default group mapping. +func MapGroupsToRoles(groups []string) []Role { + return defaultMapping.MapGroupsToRoles(groups) +} + // roleLevel defines the hierarchy level of each role for comparison. // Higher values indicate more privileged roles. var roleLevel = map[Role]int{ diff --git a/console/rbac/rbac_test.go b/console/rbac/rbac_test.go index b250b90..ba60bc7 100644 --- a/console/rbac/rbac_test.go +++ b/console/rbac/rbac_test.go @@ -220,6 +220,176 @@ func TestMapGroupsToRoles(t *testing.T) { }) } +func TestNewGroupMapping(t *testing.T) { + t.Run("default mapping uses viewer, editor, owner groups", func(t *testing.T) { + gm := NewGroupMapping(nil, nil, nil) + if got := gm.MapGroupToRole("viewer"); got != RoleViewer { + t.Errorf("expected RoleViewer for 'viewer', got %v", got) + } + if got := gm.MapGroupToRole("editor"); got != RoleEditor { + t.Errorf("expected RoleEditor for 'editor', got %v", got) + } + if got := gm.MapGroupToRole("owner"); got != RoleOwner { + t.Errorf("expected RoleOwner for 'owner', got %v", got) + } + if got := gm.MapGroupToRole("unknown"); got != RoleUnspecified { + t.Errorf("expected RoleUnspecified for 'unknown', got %v", got) + } + }) + + t.Run("custom viewer groups", func(t *testing.T) { + gm := NewGroupMapping([]string{"readers", "readonly"}, nil, nil) + // Custom groups should map to the role + if got := gm.MapGroupToRole("readers"); got != RoleViewer { + t.Errorf("expected RoleViewer for 'readers', got %v", got) + } + if got := gm.MapGroupToRole("readonly"); got != RoleViewer { + t.Errorf("expected RoleViewer for 'readonly', got %v", got) + } + // Default "viewer" should no longer map when custom groups are set + if got := gm.MapGroupToRole("viewer"); got != RoleUnspecified { + t.Errorf("expected RoleUnspecified for 'viewer' when custom viewer groups set, got %v", got) + } + // Other defaults should still work + if got := gm.MapGroupToRole("editor"); got != RoleEditor { + t.Errorf("expected RoleEditor for 'editor', got %v", got) + } + if got := gm.MapGroupToRole("owner"); got != RoleOwner { + t.Errorf("expected RoleOwner for 'owner', got %v", got) + } + }) + + t.Run("custom editor groups", func(t *testing.T) { + gm := NewGroupMapping(nil, []string{"writers", "developers"}, nil) + if got := gm.MapGroupToRole("writers"); got != RoleEditor { + t.Errorf("expected RoleEditor for 'writers', got %v", got) + } + if got := gm.MapGroupToRole("developers"); got != RoleEditor { + t.Errorf("expected RoleEditor for 'developers', got %v", got) + } + if got := gm.MapGroupToRole("editor"); got != RoleUnspecified { + t.Errorf("expected RoleUnspecified for 'editor' when custom editor groups set, got %v", got) + } + }) + + t.Run("custom owner groups", func(t *testing.T) { + gm := NewGroupMapping(nil, nil, []string{"admins", "superusers"}) + if got := gm.MapGroupToRole("admins"); got != RoleOwner { + t.Errorf("expected RoleOwner for 'admins', got %v", got) + } + if got := gm.MapGroupToRole("superusers"); got != RoleOwner { + t.Errorf("expected RoleOwner for 'superusers', got %v", got) + } + if got := gm.MapGroupToRole("owner"); got != RoleUnspecified { + t.Errorf("expected RoleUnspecified for 'owner' when custom owner groups set, got %v", got) + } + }) + + t.Run("all custom groups", func(t *testing.T) { + gm := NewGroupMapping( + []string{"readers"}, + []string{"writers"}, + []string{"admins"}, + ) + if got := gm.MapGroupToRole("readers"); got != RoleViewer { + t.Errorf("expected RoleViewer for 'readers', got %v", got) + } + if got := gm.MapGroupToRole("writers"); got != RoleEditor { + t.Errorf("expected RoleEditor for 'writers', got %v", got) + } + if got := gm.MapGroupToRole("admins"); got != RoleOwner { + t.Errorf("expected RoleOwner for 'admins', got %v", got) + } + // None of the defaults should work + if got := gm.MapGroupToRole("viewer"); got != RoleUnspecified { + t.Errorf("expected RoleUnspecified for 'viewer', got %v", got) + } + if got := gm.MapGroupToRole("editor"); got != RoleUnspecified { + t.Errorf("expected RoleUnspecified for 'editor', got %v", got) + } + if got := gm.MapGroupToRole("owner"); got != RoleUnspecified { + t.Errorf("expected RoleUnspecified for 'owner', got %v", got) + } + }) + + t.Run("case-insensitive matching with custom groups", func(t *testing.T) { + gm := NewGroupMapping([]string{"Readers"}, nil, nil) + if got := gm.MapGroupToRole("readers"); got != RoleViewer { + t.Errorf("expected RoleViewer for 'readers', got %v", got) + } + if got := gm.MapGroupToRole("READERS"); got != RoleViewer { + t.Errorf("expected RoleViewer for 'READERS', got %v", got) + } + }) +} + +func TestGroupMappingMapGroupsToRoles(t *testing.T) { + t.Run("maps custom groups to roles", func(t *testing.T) { + gm := NewGroupMapping([]string{"readers"}, []string{"writers"}, nil) + roles := gm.MapGroupsToRoles([]string{"readers", "writers", "unknown"}) + if len(roles) != 2 { + t.Fatalf("expected 2 roles, got %d", len(roles)) + } + }) +} + +func TestGroupMappingCheckAccess(t *testing.T) { + t.Run("grants access with custom group mapping", func(t *testing.T) { + gm := NewGroupMapping([]string{"readers"}, nil, nil) + // User is in "readers" group, which maps to viewer role + // Secret allows "readers" (which maps to viewer) + err := gm.CheckAccess([]string{"readers"}, []string{"readers"}, PermissionSecretsRead) + if err != nil { + t.Errorf("expected access granted, got error: %v", err) + } + }) + + t.Run("denies access when custom group not in allowed roles", func(t *testing.T) { + gm := NewGroupMapping([]string{"readers"}, []string{"writers"}, nil) + // User is in "readers" group (viewer role), needs write permission + err := gm.CheckAccess([]string{"readers"}, []string{"readers"}, PermissionSecretsWrite) + if err == nil { + t.Fatal("expected PermissionDenied error, got nil") + } + }) +} + +func TestParseGroups(t *testing.T) { + t.Run("parses comma-separated groups", func(t *testing.T) { + groups := ParseGroups("readers,writers,admins") + if len(groups) != 3 { + t.Fatalf("expected 3 groups, got %d: %v", len(groups), groups) + } + if groups[0] != "readers" || groups[1] != "writers" || groups[2] != "admins" { + t.Errorf("unexpected groups: %v", groups) + } + }) + + t.Run("trims whitespace", func(t *testing.T) { + groups := ParseGroups(" readers , writers , admins ") + if len(groups) != 3 { + t.Fatalf("expected 3 groups, got %d: %v", len(groups), groups) + } + if groups[0] != "readers" || groups[1] != "writers" || groups[2] != "admins" { + t.Errorf("unexpected groups: %v", groups) + } + }) + + t.Run("returns nil for empty string", func(t *testing.T) { + groups := ParseGroups("") + if groups != nil { + t.Errorf("expected nil for empty string, got %v", groups) + } + }) + + t.Run("single group", func(t *testing.T) { + groups := ParseGroups("admins") + if len(groups) != 1 || groups[0] != "admins" { + t.Errorf("expected [admins], got %v", groups) + } + }) +} + func TestCheckAccess(t *testing.T) { t.Run("grants access when user has matching role with permission", func(t *testing.T) { // User is in "viewer" group, secret allows "viewer" role diff --git a/console/secrets/authz.go b/console/secrets/authz.go index 0221a20..32e31b2 100644 --- a/console/secrets/authz.go +++ b/console/secrets/authz.go @@ -10,26 +10,26 @@ import ( // CheckReadAccess verifies that the user has permission to read secrets. // Uses role-based access control with the PERMISSION_SECRETS_READ permission. -func CheckReadAccess(userGroups, allowedRoles []string) error { - return rbac.CheckAccess(userGroups, allowedRoles, rbac.PermissionSecretsRead) +func CheckReadAccess(gm *rbac.GroupMapping, userGroups, allowedRoles []string) error { + return gm.CheckAccess(userGroups, allowedRoles, rbac.PermissionSecretsRead) } // CheckWriteAccess verifies that the user has permission to write secrets. // Uses role-based access control with the PERMISSION_SECRETS_WRITE permission. -func CheckWriteAccess(userGroups, allowedRoles []string) error { - return rbac.CheckAccess(userGroups, allowedRoles, rbac.PermissionSecretsWrite) +func CheckWriteAccess(gm *rbac.GroupMapping, userGroups, allowedRoles []string) error { + return gm.CheckAccess(userGroups, allowedRoles, rbac.PermissionSecretsWrite) } // CheckDeleteAccess verifies that the user has permission to delete secrets. // Uses role-based access control with the PERMISSION_SECRETS_DELETE permission. -func CheckDeleteAccess(userGroups, allowedRoles []string) error { - return rbac.CheckAccess(userGroups, allowedRoles, rbac.PermissionSecretsDelete) +func CheckDeleteAccess(gm *rbac.GroupMapping, userGroups, allowedRoles []string) error { + return gm.CheckAccess(userGroups, allowedRoles, rbac.PermissionSecretsDelete) } // CheckListAccess verifies that the user has permission to list secrets. // Uses role-based access control with the PERMISSION_SECRETS_LIST permission. -func CheckListAccess(userGroups, allowedRoles []string) error { - return rbac.CheckAccess(userGroups, allowedRoles, rbac.PermissionSecretsList) +func CheckListAccess(gm *rbac.GroupMapping, userGroups, allowedRoles []string) error { + return gm.CheckAccess(userGroups, allowedRoles, rbac.PermissionSecretsList) } // CheckAccess verifies that the user has at least one group in common with the allowed groups. diff --git a/console/secrets/authz_test.go b/console/secrets/authz_test.go index 918fa2f..b552d3c 100644 --- a/console/secrets/authz_test.go +++ b/console/secrets/authz_test.go @@ -5,14 +5,22 @@ import ( "testing" "connectrpc.com/connect" + "github.com/holos-run/holos-console/console/rbac" ) +// defaultGM returns the default GroupMapping for tests. +func defaultGM() *rbac.GroupMapping { + return rbac.NewGroupMapping(nil, nil, nil) +} + func TestCheckReadAccess(t *testing.T) { + gm := defaultGM() + t.Run("allows read access for viewer role", func(t *testing.T) { userGroups := []string{"viewer"} allowedRoles := []string{"viewer"} - err := CheckReadAccess(userGroups, allowedRoles) + err := CheckReadAccess(gm, userGroups, allowedRoles) if err != nil { t.Errorf("expected nil error (access granted), got %v", err) } @@ -22,7 +30,7 @@ func TestCheckReadAccess(t *testing.T) { userGroups := []string{"owner"} allowedRoles := []string{"viewer"} - err := CheckReadAccess(userGroups, allowedRoles) + err := CheckReadAccess(gm, userGroups, allowedRoles) if err != nil { t.Errorf("expected nil error (access granted for owner), got %v", err) } @@ -32,7 +40,7 @@ func TestCheckReadAccess(t *testing.T) { userGroups := []string{"developers"} allowedRoles := []string{"viewer", "editor"} - err := CheckReadAccess(userGroups, allowedRoles) + err := CheckReadAccess(gm, userGroups, allowedRoles) if err == nil { t.Fatal("expected PermissionDenied error, got nil") } @@ -47,11 +55,13 @@ func TestCheckReadAccess(t *testing.T) { } func TestCheckWriteAccess(t *testing.T) { + gm := defaultGM() + t.Run("allows write access for editor role", func(t *testing.T) { userGroups := []string{"editor"} allowedRoles := []string{"editor"} - err := CheckWriteAccess(userGroups, allowedRoles) + err := CheckWriteAccess(gm, userGroups, allowedRoles) if err != nil { t.Errorf("expected nil error (access granted), got %v", err) } @@ -61,7 +71,7 @@ func TestCheckWriteAccess(t *testing.T) { userGroups := []string{"owner"} allowedRoles := []string{"editor"} - err := CheckWriteAccess(userGroups, allowedRoles) + err := CheckWriteAccess(gm, userGroups, allowedRoles) if err != nil { t.Errorf("expected nil error (access granted for owner), got %v", err) } @@ -71,7 +81,7 @@ func TestCheckWriteAccess(t *testing.T) { userGroups := []string{"viewer"} allowedRoles := []string{"editor"} - err := CheckWriteAccess(userGroups, allowedRoles) + err := CheckWriteAccess(gm, userGroups, allowedRoles) if err == nil { t.Fatal("expected PermissionDenied error, got nil") } @@ -86,11 +96,13 @@ func TestCheckWriteAccess(t *testing.T) { } func TestCheckDeleteAccess(t *testing.T) { + gm := defaultGM() + t.Run("allows delete access for owner role", func(t *testing.T) { userGroups := []string{"owner"} allowedRoles := []string{"owner"} - err := CheckDeleteAccess(userGroups, allowedRoles) + err := CheckDeleteAccess(gm, userGroups, allowedRoles) if err != nil { t.Errorf("expected nil error (access granted), got %v", err) } @@ -100,7 +112,7 @@ func TestCheckDeleteAccess(t *testing.T) { userGroups := []string{"editor"} allowedRoles := []string{"owner"} - err := CheckDeleteAccess(userGroups, allowedRoles) + err := CheckDeleteAccess(gm, userGroups, allowedRoles) if err == nil { t.Fatal("expected PermissionDenied error, got nil") } @@ -117,7 +129,7 @@ func TestCheckDeleteAccess(t *testing.T) { userGroups := []string{"viewer"} allowedRoles := []string{"owner"} - err := CheckDeleteAccess(userGroups, allowedRoles) + err := CheckDeleteAccess(gm, userGroups, allowedRoles) if err == nil { t.Fatal("expected PermissionDenied error, got nil") } @@ -125,11 +137,13 @@ func TestCheckDeleteAccess(t *testing.T) { } func TestCheckListAccess(t *testing.T) { + gm := defaultGM() + t.Run("allows list access for viewer role", func(t *testing.T) { userGroups := []string{"viewer"} allowedRoles := []string{"viewer"} - err := CheckListAccess(userGroups, allowedRoles) + err := CheckListAccess(gm, userGroups, allowedRoles) if err != nil { t.Errorf("expected nil error (access granted), got %v", err) } @@ -139,7 +153,7 @@ func TestCheckListAccess(t *testing.T) { userGroups := []string{"developers"} allowedRoles := []string{"viewer"} - err := CheckListAccess(userGroups, allowedRoles) + err := CheckListAccess(gm, userGroups, allowedRoles) if err == nil { t.Fatal("expected PermissionDenied error, got nil") } diff --git a/console/secrets/handler.go b/console/secrets/handler.go index 827403e..9e44ece 100644 --- a/console/secrets/handler.go +++ b/console/secrets/handler.go @@ -9,6 +9,7 @@ import ( corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" + "github.com/holos-run/holos-console/console/rbac" "github.com/holos-run/holos-console/console/rpc" consolev1 "github.com/holos-run/holos-console/gen/holos/console/v1" "github.com/holos-run/holos-console/gen/holos/console/v1/consolev1connect" @@ -17,12 +18,13 @@ import ( // Handler implements the SecretsService. type Handler struct { consolev1connect.UnimplementedSecretsServiceHandler - k8s *K8sClient + k8s *K8sClient + groupMapping *rbac.GroupMapping } // NewHandler creates a new SecretsService handler. -func NewHandler(k8s *K8sClient) *Handler { - return &Handler{k8s: k8s} +func NewHandler(k8s *K8sClient, groupMapping *rbac.GroupMapping) *Handler { + return &Handler{k8s: k8s, groupMapping: groupMapping} } // ListSecrets returns all secrets with accessibility info for the current user. @@ -51,7 +53,7 @@ func (h *Handler) ListSecrets( // Skip secrets with invalid annotations continue } - accessible := CheckListAccess(claims.Groups, allowedRoles) == nil + accessible := CheckListAccess(h.groupMapping, claims.Groups, allowedRoles) == nil if accessible { accessibleCount++ } @@ -127,7 +129,7 @@ func (h *Handler) DeleteSecret( if err != nil { return nil, connect.NewError(connect.CodeInvalidArgument, err) } - if err := CheckDeleteAccess(claims.Groups, allowedRoles); err != nil { + if err := CheckDeleteAccess(h.groupMapping, claims.Groups, allowedRoles); err != nil { slog.WarnContext(ctx, "secret delete denied", slog.String("action", "secret_delete_denied"), slog.String("secret", req.Msg.Name), @@ -176,7 +178,7 @@ func (h *Handler) CreateSecret( // Check that the user has write permission based on their own roles. // Use the requested allowed_roles as the resource roles for the access check. - if err := CheckWriteAccess(claims.Groups, req.Msg.AllowedRoles); err != nil { + if err := CheckWriteAccess(h.groupMapping, claims.Groups, req.Msg.AllowedRoles); err != nil { slog.WarnContext(ctx, "secret create denied", slog.String("action", "secret_create_denied"), slog.String("secret", req.Msg.Name), @@ -234,7 +236,7 @@ func (h *Handler) UpdateSecret( if err != nil { return nil, connect.NewError(connect.CodeInvalidArgument, err) } - if err := CheckWriteAccess(claims.Groups, allowedRoles); err != nil { + if err := CheckWriteAccess(h.groupMapping, claims.Groups, allowedRoles); err != nil { logAuditDenied(ctx, claims, secret.Name, allowedRoles) slog.WarnContext(ctx, "secret update denied", slog.String("action", "secret_update_denied"), @@ -267,7 +269,7 @@ func (h *Handler) returnSecret(ctx context.Context, claims *rpc.Claims, secret * if err != nil { return nil, connect.NewError(connect.CodeInvalidArgument, err) } - if err := CheckReadAccess(claims.Groups, allowedRoles); err != nil { + if err := CheckReadAccess(h.groupMapping, claims.Groups, allowedRoles); err != nil { logAuditDenied(ctx, claims, secret.Name, allowedRoles) return nil, err } diff --git a/console/secrets/handler_test.go b/console/secrets/handler_test.go index c2ee9a1..56a5bba 100644 --- a/console/secrets/handler_test.go +++ b/console/secrets/handler_test.go @@ -10,6 +10,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes/fake" + "github.com/holos-run/holos-console/console/rbac" "github.com/holos-run/holos-console/console/rpc" consolev1 "github.com/holos-run/holos-console/gen/holos/console/v1" ) @@ -71,7 +72,7 @@ func TestHandler_GetSecret(t *testing.T) { } fakeClient := fake.NewClientset(secret) k8sClient := NewK8sClient(fakeClient, "test-namespace") - handler := NewHandler(k8sClient) + handler := NewHandler(k8sClient, rbac.NewGroupMapping(nil, nil, nil)) // Create authenticated context with matching role group claims := &rpc.Claims{ @@ -113,7 +114,7 @@ func TestHandler_GetSecret(t *testing.T) { } fakeClient := fake.NewClientset(secret) k8sClient := NewK8sClient(fakeClient, "test-namespace") - handler := NewHandler(k8sClient) + handler := NewHandler(k8sClient, rbac.NewGroupMapping(nil, nil, nil)) // Context without claims ctx := context.Background() @@ -153,7 +154,7 @@ func TestHandler_GetSecret(t *testing.T) { } fakeClient := fake.NewClientset(secret) k8sClient := NewK8sClient(fakeClient, "test-namespace") - handler := NewHandler(k8sClient) + handler := NewHandler(k8sClient, rbac.NewGroupMapping(nil, nil, nil)) // Create authenticated context with non-matching role group claims := &rpc.Claims{ @@ -187,7 +188,7 @@ func TestHandler_GetSecret(t *testing.T) { // Given: Authenticated user, secret does not exist fakeClient := fake.NewClientset() k8sClient := NewK8sClient(fakeClient, "test-namespace") - handler := NewHandler(k8sClient) + handler := NewHandler(k8sClient, rbac.NewGroupMapping(nil, nil, nil)) claims := &rpc.Claims{ Sub: "user-123", @@ -220,7 +221,7 @@ func TestHandler_GetSecret(t *testing.T) { // Given: Request with empty secret name fakeClient := fake.NewClientset() k8sClient := NewK8sClient(fakeClient, "test-namespace") - handler := NewHandler(k8sClient) + handler := NewHandler(k8sClient, rbac.NewGroupMapping(nil, nil, nil)) claims := &rpc.Claims{ Sub: "user-123", @@ -267,7 +268,7 @@ func TestHandler_AuditLogging(t *testing.T) { } fakeClient := fake.NewClientset(secret) k8sClient := NewK8sClient(fakeClient, "test-namespace") - handler := NewHandler(k8sClient) + handler := NewHandler(k8sClient, rbac.NewGroupMapping(nil, nil, nil)) // Capture logs logHandler := &testLogHandler{} @@ -341,7 +342,7 @@ func TestHandler_AuditLogging(t *testing.T) { } fakeClient := fake.NewClientset(secret) k8sClient := NewK8sClient(fakeClient, "test-namespace") - handler := NewHandler(k8sClient) + handler := NewHandler(k8sClient, rbac.NewGroupMapping(nil, nil, nil)) // Capture logs logHandler := &testLogHandler{} @@ -416,7 +417,7 @@ func TestHandler_DeleteSecret(t *testing.T) { } fakeClient := fake.NewClientset(secret) k8sClient := NewK8sClient(fakeClient, "test-namespace") - handler := NewHandler(k8sClient) + handler := NewHandler(k8sClient, rbac.NewGroupMapping(nil, nil, nil)) claims := &rpc.Claims{ Sub: "user-123", @@ -437,7 +438,7 @@ func TestHandler_DeleteSecret(t *testing.T) { t.Run("returns Unauthenticated for missing auth", func(t *testing.T) { fakeClient := fake.NewClientset() k8sClient := NewK8sClient(fakeClient, "test-namespace") - handler := NewHandler(k8sClient) + handler := NewHandler(k8sClient, rbac.NewGroupMapping(nil, nil, nil)) ctx := context.Background() req := connect.NewRequest(&consolev1.DeleteSecretRequest{Name: "my-secret"}) @@ -472,7 +473,7 @@ func TestHandler_DeleteSecret(t *testing.T) { } fakeClient := fake.NewClientset(secret) k8sClient := NewK8sClient(fakeClient, "test-namespace") - handler := NewHandler(k8sClient) + handler := NewHandler(k8sClient, rbac.NewGroupMapping(nil, nil, nil)) claims := &rpc.Claims{ Sub: "user-123", @@ -512,7 +513,7 @@ func TestHandler_DeleteSecret(t *testing.T) { } fakeClient := fake.NewClientset(secret) k8sClient := NewK8sClient(fakeClient, "test-namespace") - handler := NewHandler(k8sClient) + handler := NewHandler(k8sClient, rbac.NewGroupMapping(nil, nil, nil)) claims := &rpc.Claims{ Sub: "user-123", @@ -540,7 +541,7 @@ func TestHandler_DeleteSecret(t *testing.T) { t.Run("returns NotFound for non-existent secret", func(t *testing.T) { fakeClient := fake.NewClientset() k8sClient := NewK8sClient(fakeClient, "test-namespace") - handler := NewHandler(k8sClient) + handler := NewHandler(k8sClient, rbac.NewGroupMapping(nil, nil, nil)) claims := &rpc.Claims{ Sub: "user-123", @@ -568,7 +569,7 @@ func TestHandler_DeleteSecret(t *testing.T) { t.Run("returns InvalidArgument for empty name", func(t *testing.T) { fakeClient := fake.NewClientset() k8sClient := NewK8sClient(fakeClient, "test-namespace") - handler := NewHandler(k8sClient) + handler := NewHandler(k8sClient, rbac.NewGroupMapping(nil, nil, nil)) claims := &rpc.Claims{ Sub: "user-123", @@ -610,7 +611,7 @@ func TestHandler_DeleteSecret_AuditLogging(t *testing.T) { } fakeClient := fake.NewClientset(secret) k8sClient := NewK8sClient(fakeClient, "test-namespace") - handler := NewHandler(k8sClient) + handler := NewHandler(k8sClient, rbac.NewGroupMapping(nil, nil, nil)) logHandler := &testLogHandler{} oldLogger := slog.Default() @@ -655,7 +656,7 @@ func TestHandler_DeleteSecret_AuditLogging(t *testing.T) { } fakeClient := fake.NewClientset(secret) k8sClient := NewK8sClient(fakeClient, "test-namespace") - handler := NewHandler(k8sClient) + handler := NewHandler(k8sClient, rbac.NewGroupMapping(nil, nil, nil)) logHandler := &testLogHandler{} oldLogger := slog.Default() @@ -691,7 +692,7 @@ func TestHandler_CreateSecret(t *testing.T) { // Given: No secrets exist, user is editor fakeClient := fake.NewClientset() k8sClient := NewK8sClient(fakeClient, "test-namespace") - handler := NewHandler(k8sClient) + handler := NewHandler(k8sClient, rbac.NewGroupMapping(nil, nil, nil)) claims := &rpc.Claims{ Sub: "user-123", @@ -721,7 +722,7 @@ func TestHandler_CreateSecret(t *testing.T) { t.Run("returns Unauthenticated for missing auth", func(t *testing.T) { fakeClient := fake.NewClientset() k8sClient := NewK8sClient(fakeClient, "test-namespace") - handler := NewHandler(k8sClient) + handler := NewHandler(k8sClient, rbac.NewGroupMapping(nil, nil, nil)) ctx := context.Background() req := connect.NewRequest(&consolev1.CreateSecretRequest{ @@ -747,7 +748,7 @@ func TestHandler_CreateSecret(t *testing.T) { t.Run("returns PermissionDenied for viewer", func(t *testing.T) { fakeClient := fake.NewClientset() k8sClient := NewK8sClient(fakeClient, "test-namespace") - handler := NewHandler(k8sClient) + handler := NewHandler(k8sClient, rbac.NewGroupMapping(nil, nil, nil)) claims := &rpc.Claims{ Sub: "user-123", @@ -779,7 +780,7 @@ func TestHandler_CreateSecret(t *testing.T) { t.Run("returns InvalidArgument for empty name", func(t *testing.T) { fakeClient := fake.NewClientset() k8sClient := NewK8sClient(fakeClient, "test-namespace") - handler := NewHandler(k8sClient) + handler := NewHandler(k8sClient, rbac.NewGroupMapping(nil, nil, nil)) claims := &rpc.Claims{ Sub: "user-123", @@ -811,7 +812,7 @@ func TestHandler_CreateSecret(t *testing.T) { t.Run("returns InvalidArgument for empty allowed_roles", func(t *testing.T) { fakeClient := fake.NewClientset() k8sClient := NewK8sClient(fakeClient, "test-namespace") - handler := NewHandler(k8sClient) + handler := NewHandler(k8sClient, rbac.NewGroupMapping(nil, nil, nil)) claims := &rpc.Claims{ Sub: "user-123", @@ -849,7 +850,7 @@ func TestHandler_CreateSecret(t *testing.T) { } fakeClient := fake.NewClientset(existing) k8sClient := NewK8sClient(fakeClient, "test-namespace") - handler := NewHandler(k8sClient) + handler := NewHandler(k8sClient, rbac.NewGroupMapping(nil, nil, nil)) claims := &rpc.Claims{ Sub: "user-123", @@ -883,7 +884,7 @@ func TestHandler_CreateSecret_AuditLogging(t *testing.T) { t.Run("logs secret_create on success", func(t *testing.T) { fakeClient := fake.NewClientset() k8sClient := NewK8sClient(fakeClient, "test-namespace") - handler := NewHandler(k8sClient) + handler := NewHandler(k8sClient, rbac.NewGroupMapping(nil, nil, nil)) logHandler := &testLogHandler{} oldLogger := slog.Default() @@ -920,7 +921,7 @@ func TestHandler_CreateSecret_AuditLogging(t *testing.T) { t.Run("logs secret_create_denied on RBAC failure", func(t *testing.T) { fakeClient := fake.NewClientset() k8sClient := NewK8sClient(fakeClient, "test-namespace") - handler := NewHandler(k8sClient) + handler := NewHandler(k8sClient, rbac.NewGroupMapping(nil, nil, nil)) logHandler := &testLogHandler{} oldLogger := slog.Default() @@ -975,7 +976,7 @@ func TestHandler_UpdateSecret(t *testing.T) { } fakeClient := fake.NewClientset(secret) k8sClient := NewK8sClient(fakeClient, "test-namespace") - handler := NewHandler(k8sClient) + handler := NewHandler(k8sClient, rbac.NewGroupMapping(nil, nil, nil)) claims := &rpc.Claims{ Sub: "user-123", @@ -1004,7 +1005,7 @@ func TestHandler_UpdateSecret(t *testing.T) { // Given: Request without claims fakeClient := fake.NewClientset() k8sClient := NewK8sClient(fakeClient, "test-namespace") - handler := NewHandler(k8sClient) + handler := NewHandler(k8sClient, rbac.NewGroupMapping(nil, nil, nil)) ctx := context.Background() req := connect.NewRequest(&consolev1.UpdateSecretRequest{ @@ -1045,7 +1046,7 @@ func TestHandler_UpdateSecret(t *testing.T) { } fakeClient := fake.NewClientset(secret) k8sClient := NewK8sClient(fakeClient, "test-namespace") - handler := NewHandler(k8sClient) + handler := NewHandler(k8sClient, rbac.NewGroupMapping(nil, nil, nil)) claims := &rpc.Claims{ Sub: "user-123", @@ -1079,7 +1080,7 @@ func TestHandler_UpdateSecret(t *testing.T) { // Given: Secret does not exist fakeClient := fake.NewClientset() k8sClient := NewK8sClient(fakeClient, "test-namespace") - handler := NewHandler(k8sClient) + handler := NewHandler(k8sClient, rbac.NewGroupMapping(nil, nil, nil)) claims := &rpc.Claims{ Sub: "user-123", @@ -1113,7 +1114,7 @@ func TestHandler_UpdateSecret(t *testing.T) { // Given: Request with empty name fakeClient := fake.NewClientset() k8sClient := NewK8sClient(fakeClient, "test-namespace") - handler := NewHandler(k8sClient) + handler := NewHandler(k8sClient, rbac.NewGroupMapping(nil, nil, nil)) claims := &rpc.Claims{ Sub: "user-123", @@ -1147,7 +1148,7 @@ func TestHandler_UpdateSecret(t *testing.T) { // Given: Request with empty data fakeClient := fake.NewClientset() k8sClient := NewK8sClient(fakeClient, "test-namespace") - handler := NewHandler(k8sClient) + handler := NewHandler(k8sClient, rbac.NewGroupMapping(nil, nil, nil)) claims := &rpc.Claims{ Sub: "user-123", @@ -1196,7 +1197,7 @@ func TestHandler_UpdateSecret_AuditLogging(t *testing.T) { } fakeClient := fake.NewClientset(secret) k8sClient := NewK8sClient(fakeClient, "test-namespace") - handler := NewHandler(k8sClient) + handler := NewHandler(k8sClient, rbac.NewGroupMapping(nil, nil, nil)) logHandler := &testLogHandler{} oldLogger := slog.Default() @@ -1248,7 +1249,7 @@ func TestHandler_UpdateSecret_AuditLogging(t *testing.T) { } fakeClient := fake.NewClientset(secret) k8sClient := NewK8sClient(fakeClient, "test-namespace") - handler := NewHandler(k8sClient) + handler := NewHandler(k8sClient, rbac.NewGroupMapping(nil, nil, nil)) logHandler := &testLogHandler{} oldLogger := slog.Default() @@ -1303,7 +1304,7 @@ func TestHandler_GetSecret_MultipleKeys(t *testing.T) { } fakeClient := fake.NewClientset(secret) k8sClient := NewK8sClient(fakeClient, "test-namespace") - handler := NewHandler(k8sClient) + handler := NewHandler(k8sClient, rbac.NewGroupMapping(nil, nil, nil)) claims := &rpc.Claims{ Sub: "user-123", @@ -1364,7 +1365,7 @@ func TestHandler_ListSecrets(t *testing.T) { } fakeClient := fake.NewClientset(secretWithLabel, secretWithoutLabel) k8sClient := NewK8sClient(fakeClient, "test-namespace") - handler := NewHandler(k8sClient) + handler := NewHandler(k8sClient, rbac.NewGroupMapping(nil, nil, nil)) claims := &rpc.Claims{ Sub: "user-123", @@ -1427,7 +1428,7 @@ func TestHandler_ListSecrets(t *testing.T) { } fakeClient := fake.NewClientset(accessibleSecret, inaccessibleSecret) k8sClient := NewK8sClient(fakeClient, "test-namespace") - handler := NewHandler(k8sClient) + handler := NewHandler(k8sClient, rbac.NewGroupMapping(nil, nil, nil)) claims := &rpc.Claims{ Sub: "user-123", @@ -1485,7 +1486,7 @@ func TestHandler_ListSecrets(t *testing.T) { // Given: Request without claims in context fakeClient := fake.NewClientset() k8sClient := NewK8sClient(fakeClient, "test-namespace") - handler := NewHandler(k8sClient) + handler := NewHandler(k8sClient, rbac.NewGroupMapping(nil, nil, nil)) ctx := context.Background() req := connect.NewRequest(&consolev1.ListSecretsRequest{}) @@ -1516,7 +1517,7 @@ func TestHandler_ListSecrets(t *testing.T) { } fakeClient := fake.NewClientset(secretWithoutLabel) k8sClient := NewK8sClient(fakeClient, "test-namespace") - handler := NewHandler(k8sClient) + handler := NewHandler(k8sClient, rbac.NewGroupMapping(nil, nil, nil)) claims := &rpc.Claims{ Sub: "user-123",