diff --git a/docs/PROXY_AUTH.md b/docs/PROXY_AUTH.md index 03cd41510..3e6cc56b7 100644 --- a/docs/PROXY_AUTH.md +++ b/docs/PROXY_AUTH.md @@ -23,7 +23,7 @@ Authenticate users via your existing reverse proxy (Authentik, Authelia, Cloudfl | `PROXY_AUTH_ADMIN_ROLE` | Role name that grants admin access. | `admin` | | `PROXY_AUTH_LOGOUT_URL` | URL to redirect to after logout. | - | -If `PROXY_AUTH_ROLE_HEADER` and `PROXY_AUTH_ADMIN_ROLE` are configured, admin access fails closed unless the role header is present and contains the configured admin role. A missing or blank role header still authenticates the user, but the user is treated as non-admin. +Setting `PROXY_AUTH_ROLE_HEADER` turns on role gating. From then on admin access fails closed unless the role header is present and contains the admin role — `PROXY_AUTH_ADMIN_ROLE` if you set it, otherwise the `admin` default. A missing or blank role header still authenticates the user, but the user is treated as non-admin. If you intentionally want every proxy-authenticated user to be an admin, leave `PROXY_AUTH_ROLE_HEADER` unset and protect Pulse entirely at the proxy/IdP layer. diff --git a/docs/release-control/v6/internal/subsystems/api-contracts.md b/docs/release-control/v6/internal/subsystems/api-contracts.md index 730025d7f..6ec703cc5 100644 --- a/docs/release-control/v6/internal/subsystems/api-contracts.md +++ b/docs/release-control/v6/internal/subsystems/api-contracts.md @@ -299,12 +299,15 @@ evidence. Pulse Mobile's OTA gate separately proves the app against server lines already in customers' hands. Proxy-auth administrator evaluation is a shared auth/API contract. Once -`PROXY_AUTH_ROLE_HEADER` and `PROXY_AUTH_ADMIN_ROLE` are configured, -`internal/api/auth.go` must treat a valid proxy-auth user with a missing or -blank role header as authenticated but non-admin, and only an explicit -configured admin role may pass admin gates. Installations that intentionally -make every proxy-authenticated user an admin must do that by leaving the role -header unset and protecting Pulse at the proxy/IdP layer. +`PROXY_AUTH_ROLE_HEADER` is configured, `internal/api/auth.go` must treat a +valid proxy-auth user with a missing or blank role header as authenticated but +non-admin, and only an explicit admin role may pass admin gates. An unset +`PROXY_AUTH_ADMIN_ROLE` resolves to `config.DefaultProxyAuthAdminRole` and must +never disable role gating: `CheckProxyAuth` is the single admin verdict every +proxy-auth gate consumes, so a half-configured role header would otherwise +promote every proxied user to administrator across all of them. Installations +that intentionally make every proxy-authenticated user an admin must do that by +leaving the role header unset and protecting Pulse at the proxy/IdP layer. Local credential auth reads are also part of this shared boundary. Handlers in `internal/api/auth.go` and `internal/api/router.go` that compare `AuthUser` and diff --git a/internal/api/auth.go b/internal/api/auth.go index 60af74355..aa7a71e8d 100644 --- a/internal/api/auth.go +++ b/internal/api/auth.go @@ -569,10 +569,17 @@ func CheckProxyAuth(cfg *config.Config, r *http.Request) (bool, string, bool) { } } - // Check admin role if configured. Once a role header and admin role are - // configured, admin access must be proven by an explicit role value. + // Check admin role if configured. Configuring a role header is the operator's + // signal that admin access is role-gated, so from that point admin must be + // proven by an explicit role value. An unset admin role resolves to the + // documented default rather than switching gating off — leaving it empty + // would silently promote every proxy-authenticated user to administrator. isAdmin := true // Default to admin if no role checking configured - if cfg.ProxyAuthRoleHeader != "" && cfg.ProxyAuthAdminRole != "" { + if cfg.ProxyAuthRoleHeader != "" { + adminRole := strings.TrimSpace(cfg.ProxyAuthAdminRole) + if adminRole == "" { + adminRole = config.DefaultProxyAuthAdminRole + } roles := r.Header.Get(cfg.ProxyAuthRoleHeader) isAdmin = false if strings.TrimSpace(roles) == "" { @@ -587,7 +594,7 @@ func CheckProxyAuth(cfg *config.Config, r *http.Request) (bool, string, bool) { } roleList := strings.Split(roles, separator) for _, role := range roleList { - if strings.TrimSpace(role) == cfg.ProxyAuthAdminRole { + if strings.TrimSpace(role) == adminRole { isAdmin = true break } @@ -595,6 +602,7 @@ func CheckProxyAuth(cfg *config.Config, r *http.Request) (bool, string, bool) { } log.Debug(). Str("roles", roles). + Str("admin_role", adminRole). Bool("is_admin", isAdmin). Msg("Proxy auth roles checked") } diff --git a/internal/api/security_test.go b/internal/api/security_test.go index 0b90f27b9..1d605ff28 100644 --- a/internal/api/security_test.go +++ b/internal/api/security_test.go @@ -1854,6 +1854,67 @@ func TestRequireAdmin_ProxyAuthNoRoleHeaderDefaultsToAdmin(t *testing.T) { } } +// Configuring a role header is the operator's signal that admin access is +// role-gated. When ProxyAuthAdminRole is left unset the gate must fall back to +// the documented default role rather than switching off: the empty-admin-role +// short circuit let a proxied "user" read and write /api/system/settings. +func TestRequireAdmin_ProxyAuthRoleHeaderWithoutAdminRoleStillGates(t *testing.T) { + cfg := &config.Config{ + ProxyAuthSecret: "secret123", + ProxyAuthUserHeader: "X-Remote-User", + ProxyAuthRoleHeader: "X-Remote-Roles", + // No ProxyAuthAdminRole set - must resolve to the documented default. + } + + handlerCalled := false + handler := RequireAdmin(cfg, func(w http.ResponseWriter, r *http.Request) { + handlerCalled = true + w.WriteHeader(http.StatusOK) + }) + + w := httptest.NewRecorder() + req := httptest.NewRequest("GET", "/api/admin/test", nil) + req.Header.Set("X-Proxy-Secret", "secret123") + req.Header.Set("X-Remote-User", "regular-user") + req.Header.Set("X-Remote-Roles", "user|viewer") + handler(w, req) + + if handlerCalled { + t.Error("RequireAdmin should not call handler for a non-admin proxy user when the admin role is unset") + } + if w.Code != http.StatusForbidden { + t.Errorf("RequireAdmin returned status %d, want %d", w.Code, http.StatusForbidden) + } +} + +func TestRequireAdmin_ProxyAuthRoleHeaderWithoutAdminRoleAllowsDefaultRole(t *testing.T) { + cfg := &config.Config{ + ProxyAuthSecret: "secret123", + ProxyAuthUserHeader: "X-Remote-User", + ProxyAuthRoleHeader: "X-Remote-Roles", + } + + handlerCalled := false + handler := RequireAdmin(cfg, func(w http.ResponseWriter, r *http.Request) { + handlerCalled = true + w.WriteHeader(http.StatusOK) + }) + + w := httptest.NewRecorder() + req := httptest.NewRequest("GET", "/api/admin/test", nil) + req.Header.Set("X-Proxy-Secret", "secret123") + req.Header.Set("X-Remote-User", "admin-user") + req.Header.Set("X-Remote-Roles", config.DefaultProxyAuthAdminRole+"|user") + handler(w, req) + + if !handlerCalled { + t.Error("RequireAdmin should call handler for a proxy user carrying the default admin role") + } + if w.Code != http.StatusOK { + t.Errorf("RequireAdmin returned status %d, want %d", w.Code, http.StatusOK) + } +} + func TestRequireAdmin_ProxyAuthConfiguredRoleHeaderMissingForbidden(t *testing.T) { cfg := &config.Config{ ProxyAuthSecret: "secret123", diff --git a/internal/config/config.go b/internal/config/config.go index 54820580b..645bac4ec 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -42,6 +42,14 @@ const ( DefaultGuestMetadataMaxConcurrent = 4 ) +// DefaultProxyAuthAdminRole is the role name that grants admin access when a +// proxy-auth deployment configures a role header without naming an admin role. +// The struct's envconfig `default` tag never applies (see the Config note +// below), so this default has to be resolved explicitly by every consumer of +// ProxyAuthAdminRole — otherwise role gating silently switches off and every +// proxy-authenticated user is treated as an administrator. +const DefaultProxyAuthAdminRole = "admin" + // Vars for mocking system calls in tests var ( osStat = os.Stat @@ -1392,6 +1400,11 @@ func load(initLogging bool) (*Config, error) { if adminRole := os.Getenv("PROXY_AUTH_ADMIN_ROLE"); adminRole != "" { cfg.ProxyAuthAdminRole = adminRole log.Info().Str("role", adminRole).Msg("Proxy auth admin role configured") + } else if cfg.ProxyAuthAdminRole == "" { + // Documented default (docs/PROXY_AUTH.md). Leaving it empty while a + // role header is configured would disable role gating entirely. + cfg.ProxyAuthAdminRole = DefaultProxyAuthAdminRole + log.Info().Str("role", cfg.ProxyAuthAdminRole).Msg("Proxy auth admin role defaulted") } if logoutURL := os.Getenv("PROXY_AUTH_LOGOUT_URL"); logoutURL != "" { cfg.ProxyAuthLogoutURL = logoutURL diff --git a/internal/config/config_load_test.go b/internal/config/config_load_test.go index 70d15bfbe..5dc3b9f49 100644 --- a/internal/config/config_load_test.go +++ b/internal/config/config_load_test.go @@ -366,6 +366,35 @@ func TestLoad_ProxyAuth(t *testing.T) { assert.Equal(t, "X-User", cfg.ProxyAuthUserHeader) } +// A deployment that configures a role header but never names an admin role must +// still get the documented default. Leaving ProxyAuthAdminRole empty made +// CheckProxyAuth skip role gating entirely, so every proxied user read and wrote +// admin-only settings. +func TestLoad_ProxyAuthAdminRoleDefaultsWhenUnset(t *testing.T) { + t.Setenv("PULSE_DATA_DIR", t.TempDir()) + t.Setenv("PROXY_AUTH_SECRET", "secret") + t.Setenv("PROXY_AUTH_USER_HEADER", "X-User") + t.Setenv("PROXY_AUTH_ROLE_HEADER", "X-Roles") + + cfg, err := Load() + require.NoError(t, err) + + assert.Equal(t, DefaultProxyAuthAdminRole, cfg.ProxyAuthAdminRole) +} + +func TestLoad_ProxyAuthAdminRoleEnvOverridesDefault(t *testing.T) { + t.Setenv("PULSE_DATA_DIR", t.TempDir()) + t.Setenv("PROXY_AUTH_SECRET", "secret") + t.Setenv("PROXY_AUTH_USER_HEADER", "X-User") + t.Setenv("PROXY_AUTH_ROLE_HEADER", "X-Roles") + t.Setenv("PROXY_AUTH_ADMIN_ROLE", "pulse-admins") + + cfg, err := Load() + require.NoError(t, err) + + assert.Equal(t, "pulse-admins", cfg.ProxyAuthAdminRole) +} + func TestLegacyOIDCEnvProvider(t *testing.T) { t.Setenv("PULSE_DATA_DIR", t.TempDir()) t.Setenv("OIDC_ENABLED", "true")