From d83c30db089d97179cea86b9096c089d05e658c1 Mon Sep 17 00:00:00 2001 From: Copilot Date: Mon, 2 Mar 2026 13:20:07 -0800 Subject: [PATCH] Revert "feat(admin): require admin-only role for UI auth" This reverts commit 0040864f19182b07ab6428f3f04c285ce3b2bac2. --- weed/admin/dash/auth_middleware.go | 10 ++++++-- weed/admin/dash/oidc_auth.go | 16 +++++++++---- weed/admin/dash/oidc_auth_test.go | 9 +++---- weed/admin/handlers/auth_config.go | 8 ++++--- weed/admin/handlers/auth_handlers.go | 8 ++++++- weed/command/admin.go | 35 +++++++++++++++++++++------- weed/command/mini.go | 17 ++++++++++---- 7 files changed, 77 insertions(+), 26 deletions(-) diff --git a/weed/admin/dash/auth_middleware.go b/weed/admin/dash/auth_middleware.go index a80fe282a..dd2f9abed 100644 --- a/weed/admin/dash/auth_middleware.go +++ b/weed/admin/dash/auth_middleware.go @@ -14,7 +14,7 @@ func (s *AdminServer) ShowLogin(w http.ResponseWriter, r *http.Request) { } // HandleLogin handles login form submission. -func (s *AdminServer) HandleLogin(store sessions.Store, adminUser, adminPassword string) http.HandlerFunc { +func (s *AdminServer) HandleLogin(store sessions.Store, adminUser, adminPassword, readOnlyUser, readOnlyPassword string) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { if err := r.ParseForm(); err != nil { http.Redirect(w, r, "/login?error=Invalid form submission", http.StatusSeeOther) @@ -34,10 +34,16 @@ func (s *AdminServer) HandleLogin(store sessions.Store, adminUser, adminPassword loginUsername := r.FormValue("username") loginPassword := r.FormValue("password") + var role string var authenticated bool // Check admin credentials. if adminPassword != "" && loginUsername == adminUser && subtle.ConstantTimeCompare([]byte(loginPassword), []byte(adminPassword)) == 1 { + role = "admin" + authenticated = true + } else if readOnlyPassword != "" && loginUsername == readOnlyUser && subtle.ConstantTimeCompare([]byte(loginPassword), []byte(readOnlyPassword)) == 1 { + // Check read-only credentials. + role = "readonly" authenticated = true } @@ -47,7 +53,7 @@ func (s *AdminServer) HandleLogin(store sessions.Store, adminUser, adminPassword } session.Values["authenticated"] = true session.Values["username"] = loginUsername - session.Values["role"] = "admin" + session.Values["role"] = role csrfToken, err := generateCSRFToken() if err != nil { http.Redirect(w, r, "/login?error=Unable to create session. Please try again or contact administrator.", http.StatusSeeOther) diff --git a/weed/admin/dash/oidc_auth.go b/weed/admin/dash/oidc_auth.go index 9c20fe959..8378b0340 100644 --- a/weed/admin/dash/oidc_auth.go +++ b/weed/admin/dash/oidc_auth.go @@ -167,7 +167,7 @@ func (c OIDCAuthConfig) Validate() error { } if c.RoleMapping.DefaultRole != "" && !isSupportedAdminRole(c.RoleMapping.DefaultRole) { - return fmt.Errorf("admin.oidc.role_mapping.default_role must be admin") + return fmt.Errorf("admin.oidc.role_mapping.default_role must be one of: admin, readonly") } for i, rule := range c.RoleMapping.Rules { @@ -178,7 +178,7 @@ func (c OIDCAuthConfig) Validate() error { return fmt.Errorf("admin.oidc.role_mapping.rules[%d].value is required", i) } if !isSupportedAdminRole(rule.Role) { - return fmt.Errorf("admin.oidc.role_mapping.rules[%d].role must be admin", i) + return fmt.Errorf("admin.oidc.role_mapping.rules[%d].role must be one of: admin, readonly", i) } } @@ -459,10 +459,18 @@ func mapClaimsToRoles(claims *providers.TokenClaims, mapping *providers.RoleMapp } func resolveAdminRole(roles []string) (string, error) { + hasReadonly := false for _, role := range roles { - if normalizeAdminRole(role) == "admin" { + role = normalizeAdminRole(role) + if role == "admin" { return "admin", nil } + if role == "readonly" { + hasReadonly = true + } + } + if hasReadonly { + return "readonly", nil } return "", fmt.Errorf("OIDC user does not map to an allowed admin role") } @@ -524,7 +532,7 @@ func normalizeOIDCAuthConfig(config OIDCAuthConfig) OIDCAuthConfig { func isSupportedAdminRole(role string) bool { switch normalizeAdminRole(role) { - case "admin": + case "admin", "readonly": return true default: return false diff --git a/weed/admin/dash/oidc_auth_test.go b/weed/admin/dash/oidc_auth_test.go index e4f0064b4..967d81660 100644 --- a/weed/admin/dash/oidc_auth_test.go +++ b/weed/admin/dash/oidc_auth_test.go @@ -40,20 +40,21 @@ func TestOIDCAuthConfigEffectiveScopesIncludesOpenID(t *testing.T) { func TestMapClaimsToRolesAndResolveAdminRole(t *testing.T) { claims := &providers.TokenClaims{ Claims: map[string]interface{}{ - "groups": []interface{}{"seaweedfs-admins"}, + "groups": []interface{}{"seaweedfs-readers", "seaweedfs-admins"}, }, } roleMapping := &providers.RoleMapping{ Rules: []providers.MappingRule{ + {Claim: "groups", Value: "seaweedfs-readers", Role: "readonly"}, {Claim: "groups", Value: "seaweedfs-admins", Role: "admin"}, }, - DefaultRole: "admin", + DefaultRole: "readonly", } roles := mapClaimsToRoles(claims, roleMapping) - if len(roles) != 1 { - t.Fatalf("expected 1 mapped role, got %d (%v)", len(roles), roles) + if len(roles) != 2 { + t.Fatalf("expected 2 mapped roles, got %d (%v)", len(roles), roles) } role, err := resolveAdminRole(roles) diff --git a/weed/admin/handlers/auth_config.go b/weed/admin/handlers/auth_config.go index 972ba2cd7..12803b33d 100644 --- a/weed/admin/handlers/auth_config.go +++ b/weed/admin/handlers/auth_config.go @@ -3,9 +3,11 @@ package handlers import "github.com/seaweedfs/seaweedfs/weed/admin/dash" type AuthConfig struct { - AdminUser string - AdminPassword string - OIDCAuth *dash.OIDCAuthService + AdminUser string + AdminPassword string + ReadOnlyUser string + ReadOnlyPassword string + OIDCAuth *dash.OIDCAuthService } func (c AuthConfig) LocalAuthEnabled() bool { diff --git a/weed/admin/handlers/auth_handlers.go b/weed/admin/handlers/auth_handlers.go index 67ab0a4eb..dadc46a17 100644 --- a/weed/admin/handlers/auth_handlers.go +++ b/weed/admin/handlers/auth_handlers.go @@ -73,7 +73,13 @@ func (a *AuthHandlers) HandleLogin() http.HandlerFunc { } } - return a.adminServer.HandleLogin(a.sessionStore, a.authConfig.AdminUser, a.authConfig.AdminPassword) + return a.adminServer.HandleLogin( + a.sessionStore, + a.authConfig.AdminUser, + a.authConfig.AdminPassword, + a.authConfig.ReadOnlyUser, + a.authConfig.ReadOnlyPassword, + ) } // HandleOIDCLogin starts the OIDC authorization code flow. diff --git a/weed/command/admin.go b/weed/command/admin.go index 63e1d108a..32365d4f4 100644 --- a/weed/command/admin.go +++ b/weed/command/admin.go @@ -57,8 +57,8 @@ func init() { a.adminUser = cmdAdmin.Flag.String("adminUser", "admin", "admin interface username") a.adminPassword = cmdAdmin.Flag.String("adminPassword", "", "admin interface password (if empty, auth is disabled)") - a.readOnlyUser = cmdAdmin.Flag.String("readOnlyUser", "", "deprecated: read-only admin users are no longer supported") - a.readOnlyPassword = cmdAdmin.Flag.String("readOnlyPassword", "", "deprecated: read-only admin users are no longer supported") + a.readOnlyUser = cmdAdmin.Flag.String("readOnlyUser", "", "read-only user username (optional, for view-only access)") + a.readOnlyPassword = cmdAdmin.Flag.String("readOnlyPassword", "", "read-only user password (optional, for view-only access; requires adminPassword to be set)") a.icebergPort = cmdAdmin.Flag.Int("iceberg.port", 8181, "Iceberg REST Catalog port (0 to hide in UI)") } @@ -93,8 +93,12 @@ var cmdAdmin = &Command{ Authentication: - Local auth: set adminUser/adminPassword for username/password login - If adminPassword is set, users must login with adminUser/adminPassword (full access) + - Optional read-only access: set readOnlyUser and readOnlyPassword for view-only access + - Read-only users can view cluster status and configurations but cannot make changes + - IMPORTANT: When read-only credentials are configured, adminPassword MUST also be set + - This ensures an admin account exists to manage and authorize read-only access - OIDC auth: configure [admin.oidc] in security.toml for Authorization Code flow - - OIDC role mapping must resolve users to admin + - OIDC role mapping must resolve users to admin or readonly - If neither local auth nor OIDC is configured, the admin interface runs without authentication - Sessions are secured with auto-generated session keys @@ -161,8 +165,18 @@ func runAdmin(cmd *Command, args []string) bool { fmt.Println("Error: -adminUser cannot be empty when -adminPassword is set") return false } - if *a.readOnlyUser != "" || *a.readOnlyPassword != "" { - fmt.Println("Error: read-only admin users are no longer supported; remove -readOnlyUser/-readOnlyPassword") + if *a.readOnlyPassword != "" && *a.readOnlyUser == "" { + fmt.Println("Error: -readOnlyUser is required when -readOnlyPassword is set") + return false + } + // Security validation: prevent username conflicts between admin and read-only users + if *a.adminUser != "" && *a.readOnlyUser != "" && *a.adminUser == *a.readOnlyUser { + fmt.Println("Error: -adminUser and -readOnlyUser must be different when both are configured") + return false + } + // Security validation: admin password is required for read-only user + if *a.readOnlyPassword != "" && *a.adminPassword == "" { + fmt.Println("Error: -adminPassword must be set when -readOnlyPassword is configured") return false } @@ -192,6 +206,9 @@ func runAdmin(cmd *Command, args []string) bool { if *a.adminPassword != "" { fmt.Printf("Local credentials: Enabled (admin user: %s)\n", *a.adminUser) } + if *a.readOnlyPassword != "" { + fmt.Printf("Read-only access: Enabled (read-only user: %s)\n", *a.readOnlyUser) + } if oidcEnabled { fmt.Printf("OIDC: Enabled (issuer: %s)\n", viper.GetString("admin.oidc.issuer")) } @@ -317,9 +334,11 @@ func startAdminServer(ctx context.Context, options AdminOptions, enableUI bool, // Create handlers and setup routes authConfig := handlers.AuthConfig{ - AdminUser: *options.adminUser, - AdminPassword: *options.adminPassword, - OIDCAuth: oidcAuthService, + AdminUser: *options.adminUser, + AdminPassword: *options.adminPassword, + ReadOnlyUser: *options.readOnlyUser, + ReadOnlyPassword: *options.readOnlyPassword, + OIDCAuth: oidcAuthService, } adminHandlers := handlers.NewAdminHandlers(adminServer, store, authConfig) adminHandlers.SetupRoutes(r, enableUI) diff --git a/weed/command/mini.go b/weed/command/mini.go index c78184b03..0ff0b9b51 100644 --- a/weed/command/mini.go +++ b/weed/command/mini.go @@ -277,8 +277,8 @@ func initMiniAdminFlags() { miniAdminOptions.dataDir = cmdMini.Flag.String("admin.dataDir", "", "directory to store admin configuration and data files") miniAdminOptions.adminUser = cmdMini.Flag.String("admin.user", "admin", "admin interface username") miniAdminOptions.adminPassword = cmdMini.Flag.String("admin.password", "", "admin interface password (if empty, auth is disabled)") - miniAdminOptions.readOnlyUser = cmdMini.Flag.String("admin.readOnlyUser", "", "deprecated: read-only admin users are no longer supported") - miniAdminOptions.readOnlyPassword = cmdMini.Flag.String("admin.readOnlyPassword", "", "deprecated: read-only admin users are no longer supported") + miniAdminOptions.readOnlyUser = cmdMini.Flag.String("admin.readOnlyUser", "", "read-only user username (optional, for view-only access)") + miniAdminOptions.readOnlyPassword = cmdMini.Flag.String("admin.readOnlyPassword", "", "read-only user password (optional, for view-only access; requires admin.password to be set)") } func init() { @@ -989,8 +989,17 @@ func startMiniAdminWithWorker(allServicesReady chan struct{}) { if *miniAdminOptions.adminPassword != "" && *miniAdminOptions.adminUser == "" { glog.Fatalf("Error: -admin.user cannot be empty when -admin.password is set") } - if *miniAdminOptions.readOnlyUser != "" || *miniAdminOptions.readOnlyPassword != "" { - glog.Fatalf("Error: read-only admin users are no longer supported; remove -admin.readOnlyUser/-admin.readOnlyPassword") + if *miniAdminOptions.readOnlyPassword != "" && *miniAdminOptions.readOnlyUser == "" { + glog.Fatalf("Error: -admin.readOnlyUser is required when -admin.readOnlyPassword is set") + } + // Security validation: prevent username conflicts between admin and read-only users + if *miniAdminOptions.adminUser != "" && *miniAdminOptions.readOnlyUser != "" && + *miniAdminOptions.adminUser == *miniAdminOptions.readOnlyUser { + glog.Fatalf("Error: -admin.user and -admin.readOnlyUser must be different when both are configured") + } + // Security validation: admin password is required for read-only user + if *miniAdminOptions.readOnlyPassword != "" && *miniAdminOptions.adminPassword == "" { + glog.Fatalf("Error: -admin.password must be set when -admin.readOnlyPassword is configured") } // gRPC port should have been initialized by ensureAllPortsAvailableOnIP in runMini