mirror of
https://github.com/rcourtman/Pulse.git
synced 2026-09-10 02:25:56 +00:00
Fail closed when proxy auth configures a role header but no admin role
A reverse-proxy deployment that set PROXY_AUTH_ROLE_HEADER without also setting PROXY_AUTH_ADMIN_ROLE granted every proxy-authenticated user full administrator access. CheckProxyAuth only evaluated roles when both values were non-empty, so the half-configuration skipped role gating entirely and returned isAdmin=true. docs/PROXY_AUTH.md has always documented an `admin` default for that variable, but the Config struct's envconfig `default` tags are legacy and never applied (config.go), so nothing ever populated it. CheckProxyAuth is the single admin verdict all 20+ proxy-auth gates consume, so the fail-open reached every one of them. Verified on a scratch instance with PROXY_AUTH_ROLE_HEADER set and no admin role: a request carrying only `X-Proxy-Roles: user` received HTTP 200 and the full admin payload from GET /api/system/settings, HTTP 200 from POST /api/system/settings/update, and proxyAuthIsAdmin=true from /api/security/status. All three now return 403 / false, while `X-Proxy-Roles: admin` still passes. Resolve the documented default in both layers that can produce the verdict: config load populates ProxyAuthAdminRole when proxy auth is configured, and CheckProxyAuth now keys role gating on the role header alone, resolving an empty admin role through config.DefaultProxyAuthAdminRole. Configuring a role header is the operator's signal that admin access is role-gated; leaving the admin role unset must not switch that off. Deployments that intentionally treat every proxied user as an admin are unaffected: that is still expressed by leaving the role header unset.
This commit is contained in:
+1
-1
@@ -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.
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
+12
-4
@@ -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")
|
||||
}
|
||||
|
||||
@@ -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",
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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")
|
||||
|
||||
Reference in New Issue
Block a user