feat: enhance admin role checks to support multiple roles configuration (#43)

Signed-off-by: Noste <83548733+Noooste@users.noreply.github.com>

backend/internal/auth/auth.go

@@ -252,15 +252,18 @@ func (a *Service) ExtractRolesFromAccessToken(accessToken string) []string {
 	return extractRoles(claims, a.authConfig.OIDC.RoleAttributePath)
 }
 
-// IsAdmin checks if the user has admin role
+// IsAdmin checks if the user has any of the configured admin roles.
 func (a *Service) IsAdmin(userInfo *UserInfo) bool {
-	if a.authConfig.OIDC.AdminRole == "" {
+	adminRoles := a.authConfig.OIDC.EffectiveAdminRoles()
+	if len(adminRoles) == 0 {
 		return false
 	}
 
 	for _, role := range userInfo.Roles {
-		if role == a.authConfig.OIDC.AdminRole {
-			return true
+		for _, adminRole := range adminRoles {
+			if role == adminRole {
+				return true
+			}
 		}
 	}
 

backend/internal/auth/auth_test.go

@@ -609,23 +609,35 @@ func TestExtractRolesFromAccessToken_BadJSONInPayload(t *testing.T) {
 
 func TestIsAdmin(t *testing.T) {
 	tests := []struct {
-		name      string
-		adminRole string
-		userRoles []string
-		want      bool
+		name       string
+		adminRole  string
+		adminRoles []string
+		userRoles  []string
+		want       bool
 	}{
-		{"empty admin role config returns false", "", []string{"admin"}, false},
-		{"user has admin role", "admin", []string{"viewer", "admin"}, true},
-		{"user lacks admin role", "admin", []string{"viewer"}, false},
-		{"user has no roles", "admin", nil, false},
-		{"role match is exact (case-sensitive)", "admin", []string{"Admin"}, false},
+		{"empty admin role config returns false", "", nil, []string{"admin"}, false},
+		{"user has admin role", "admin", nil, []string{"viewer", "admin"}, true},
+		{"user lacks admin role", "admin", nil, []string{"viewer"}, false},
+		{"user has no roles", "admin", nil, nil, false},
+		{"role match is exact (case-sensitive)", "admin", nil, []string{"Admin"}, false},
+
+		// admin_roles list
+		{"user matches first entry in admin_roles", "", []string{"group1", "group2"}, []string{"group1"}, true},
+		{"user matches second entry in admin_roles", "", []string{"group1", "group2"}, []string{"group2"}, true},
+		{"user matches none of admin_roles", "", []string{"group1", "group2"}, []string{"group3"}, false},
+		{"empty admin_roles list returns false", "", []string{}, []string{"group1"}, false},
+
+		// admin_role + admin_roles merge
+		{"matches single admin_role when admin_roles set too", "admin", []string{"ops"}, []string{"admin"}, true},
+		{"matches admin_roles entry when admin_role set too", "admin", []string{"ops"}, []string{"ops"}, true},
+		{"matches neither admin_role nor admin_roles", "admin", []string{"ops"}, []string{"viewer"}, false},
 	}
 
 	for _, tc := range tests {
 		t.Run(tc.name, func(t *testing.T) {
 			svc := &Service{
 				authConfig: &config.AuthConfig{
-					OIDC: config.OIDCConfig{AdminRole: tc.adminRole},
+					OIDC: config.OIDCConfig{AdminRole: tc.adminRole, AdminRoles: tc.adminRoles},
 				},
 			}
 			if got := svc.IsAdmin(&UserInfo{Roles: tc.userRoles}); got != tc.want {

backend/internal/config/config.go

@@ -78,6 +78,7 @@ type OIDCConfig struct {
 	NameAttribute     string   `mapstructure:"name_attribute"`
 	RoleAttributePath string   `mapstructure:"role_attribute_path"`
 	AdminRole         string   `mapstructure:"admin_role"`
+	AdminRoles        []string `mapstructure:"admin_roles"`
 	TLSSkipVerify     bool     `mapstructure:"tls_skip_verify"`
 	SessionMaxAge     int      `mapstructure:"session_max_age"`
 	CookieName        string   `mapstructure:"cookie_name"`
@@ -86,6 +87,29 @@ type OIDCConfig struct {
 	CookieSameSite    string   `mapstructure:"cookie_same_site"`
 }
 
+// EffectiveAdminRoles returns the deduplicated list of admin roles drawn from
+// both admin_role (legacy single-value) and admin_roles (list). A user is
+// considered an admin if any of their roles matches any entry in this list.
+func (o OIDCConfig) EffectiveAdminRoles() []string {
+	seen := make(map[string]struct{}, len(o.AdminRoles)+1)
+	var roles []string
+	add := func(r string) {
+		if r == "" {
+			return
+		}
+		if _, ok := seen[r]; ok {
+			return
+		}
+		seen[r] = struct{}{}
+		roles = append(roles, r)
+	}
+	add(o.AdminRole)
+	for _, r := range o.AdminRoles {
+		add(r)
+	}
+	return roles
+}
+
 // CORSConfig contains CORS settings for frontend communication
 type CORSConfig struct {
 	Enabled          bool     `mapstructure:"enabled"`
@@ -231,6 +255,7 @@ func bindEnvVars() {
 	viper.BindEnv("auth.oidc.name_attribute", "GARAGE_UI_AUTH_OIDC_NAME_ATTRIBUTE")
 	viper.BindEnv("auth.oidc.role_attribute_path", "GARAGE_UI_AUTH_OIDC_ROLE_ATTRIBUTE_PATH")
 	viper.BindEnv("auth.oidc.admin_role", "GARAGE_UI_AUTH_OIDC_ADMIN_ROLE")
+	viper.BindEnv("auth.oidc.admin_roles", "GARAGE_UI_AUTH_OIDC_ADMIN_ROLES")
 	viper.BindEnv("auth.oidc.tls_skip_verify", "GARAGE_UI_AUTH_OIDC_TLS_SKIP_VERIFY")
 	viper.BindEnv("auth.oidc.session_max_age", "GARAGE_UI_AUTH_OIDC_SESSION_MAX_AGE")
 	viper.BindEnv("auth.oidc.cookie_name", "GARAGE_UI_AUTH_OIDC_COOKIE_NAME")
@@ -291,11 +316,12 @@ func (c *Config) Validate() error {
 			return fmt.Errorf("oidc scopes are required when oidc is enabled")
 		}
 		// Every authenticated route on this service grants full admin
-		// access — there is no separate authorization layer. An empty
-		// admin_role would therefore promote every user in the IdP realm
-		// to cluster admin. Require operators to opt in explicitly.
-		if c.Auth.OIDC.AdminRole == "" {
-			return fmt.Errorf("oidc admin_role is required when oidc is enabled: leaving it empty would grant cluster-admin access to any authenticated IdP user")
+		// access — there is no separate authorization layer. Empty
+		// admin role configuration would therefore promote every user
+		// in the IdP realm to cluster admin. Require operators to opt
+		// in explicitly via admin_role or admin_roles.
+		if len(c.Auth.OIDC.EffectiveAdminRoles()) == 0 {
+			return fmt.Errorf("oidc admin_role or admin_roles is required when oidc is enabled: leaving them empty would grant cluster-admin access to any authenticated IdP user")
 		}
 	}
 

backend/internal/config/config_test.go

@@ -3,6 +3,7 @@ package config
 import (
 	"os"
 	"path/filepath"
+	"reflect"
 	"strings"
 	"testing"
 
@@ -292,7 +293,33 @@ func TestValidate(t *testing.T) {
 				applyValidOIDC(c)
 				c.Auth.OIDC.AdminRole = ""
 			},
-			wantErrContains: "oidc admin_role is required",
+			wantErrContains: "oidc admin_role or admin_roles is required",
+		},
+		{
+			name: "oidc enabled with admin_roles only is valid",
+			mutate: func(c *Config) {
+				applyValidOIDC(c)
+				c.Auth.OIDC.AdminRole = ""
+				c.Auth.OIDC.AdminRoles = []string{"group1", "group2"}
+			},
+			wantErrContains: "",
+		},
+		{
+			name: "oidc enabled with both admin_role and admin_roles is valid",
+			mutate: func(c *Config) {
+				applyValidOIDC(c)
+				c.Auth.OIDC.AdminRoles = []string{"group2", "group3"}
+			},
+			wantErrContains: "",
+		},
+		{
+			name: "oidc enabled with empty admin_role and empty admin_roles rejected",
+			mutate: func(c *Config) {
+				applyValidOIDC(c)
+				c.Auth.OIDC.AdminRole = ""
+				c.Auth.OIDC.AdminRoles = []string{}
+			},
+			wantErrContains: "oidc admin_role or admin_roles is required",
 		},
 		{
 			name:            "oidc fully configured is valid",
@@ -498,6 +525,32 @@ func TestValidate_TokenAuthExplicitlyEnabled(t *testing.T) {
 	}
 }
 
+func TestEffectiveAdminRoles(t *testing.T) {
+	tests := []struct {
+		name       string
+		adminRole  string
+		adminRoles []string
+		want       []string
+	}{
+		{"both empty", "", nil, nil},
+		{"single only", "admin", nil, []string{"admin"}},
+		{"list only", "", []string{"a", "b"}, []string{"a", "b"}},
+		{"merge single + list", "admin", []string{"viewer", "ops"}, []string{"admin", "viewer", "ops"}},
+		{"dedupes overlap", "admin", []string{"admin", "ops"}, []string{"admin", "ops"}},
+		{"dedupes within list", "", []string{"a", "a", "b"}, []string{"a", "b"}},
+		{"skips empty strings in list", "admin", []string{"", "ops", ""}, []string{"admin", "ops"}},
+	}
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			o := OIDCConfig{AdminRole: tc.adminRole, AdminRoles: tc.adminRoles}
+			got := o.EffectiveAdminRoles()
+			if !reflect.DeepEqual(got, tc.want) {
+				t.Errorf("EffectiveAdminRoles() = %v, want %v", got, tc.want)
+			}
+		})
+	}
+}
+
 func TestIsProduction(t *testing.T) {
 	tests := []struct {
 		env  string

backend/internal/routes/routes.go

@@ -236,7 +236,8 @@ func SetupRoutes(
 				// ID token and the userinfo endpoint (Keycloak emits resource_access
 				// only in the access token by default), so fall back to the access
 				// token and then the userinfo endpoint before denying access.
-				if cfg.Auth.OIDC.AdminRole != "" {
+				adminRoles := cfg.Auth.OIDC.EffectiveAdminRoles()
+				if len(adminRoles) > 0 {
 					if !authService.IsAdmin(userInfo) {
 						if roles := authService.ExtractRolesFromAccessToken(token.AccessToken); len(roles) > 0 {
 							userInfo.Roles = roles
@@ -250,9 +251,9 @@ func SetupRoutes(
 					if !authService.IsAdmin(userInfo) {
 						logger.Warn().
 							Str("username", userInfo.Username).
-							Str("required_role", cfg.Auth.OIDC.AdminRole).
+							Strs("required_roles", adminRoles).
 							Strs("roles", userInfo.Roles).
-							Msg("OIDC login denied: user does not have required admin role")
+							Msg("OIDC login denied: user does not have any required admin role")
 						return c.Status(fiber.StatusForbidden).JSON(fiber.Map{
 							"error": "User does not have the required admin role",
 						})

config.example.yaml

@@ -75,7 +75,14 @@ auth:
 
     # Role-based access (optional)
     role_attribute_path: "resource_access.garage-ui.roles"
+    # Single admin role (backward-compatible).
     admin_role: "admin"
+    # Multiple admin roles: a user is granted admin if ANY of their roles
+    # matches ANY entry below. Values from admin_role and admin_roles are
+    # merged, so you can set either, both, or only admin_roles.
+    # admin_roles:
+    #   - "garage-admins"
+    #   - "platform-team"
 
     # TLS configuration
     tls_skip_verify: false # Only set to true for testing, not recommended for production

helm/garage-ui/values.yaml

@@ -84,9 +84,13 @@ config:
       email_attribute: "email"
       username_attribute: "preferred_username"
       name_attribute: "name"
-      # Role-based access control
+      # Role-based access control.
+      # Set admin_role (single value, backward-compatible) and/or admin_roles
+      # (list). Values from both are merged: a user is granted admin if ANY
+      # of their roles matches ANY entry across these two fields.
       role_attribute_path: "resource_access.garage-ui.roles"
       admin_role: "admin"
+      admin_roles: []
       # TLS settings
       tls_skip_verify: false
       # Session settings