mirror of
https://github.com/rcourtman/Pulse.git
synced 2026-09-10 10:35:51 +00:00
fix(api): give OIDC-only instances one definition of session admin
On the OIDC-only pattern there is no local admin, so SSO principals are the only administrators the instance has. ensureAdminSession already knows that: sessionUserCarriesAdminPrivileges admits the configured admin, anyone holding an RBAC admin grant, and any SSO principal when no local admin is configured. Three guards did not use it. They compared the session username against cfg.AuthUser directly, which on those instances is empty, so they could admit nobody at all. The same operator was admitted by the settings routes and refused by discovery, by public URL capture, and by config export and import, which is three answers to one question. Verified against an unlicensed OIDC-only router before and after. Before, sessionUserCarriesAdminPrivileges returned true for the SSO owner while canCapturePublicURL and discovery isAdminRequest returned false and /api/config/export returned 403. After, all four agree. This does not widen anything. On an instance that does configure a local admin, an unrelated SSO principal is still not an administrator and every one of these guards still refuses them, which the parity test pins in both directions. RequirePlatformAdmin is deliberately left alone. Its stricter session rule is documented as intentional for the hosted control plane rather than an oversight, and loosening it is a different decision from this one. Contract-Neutral: behavioral fix on existing routes, no request or response shape change; three guards switched to the canonical session-admin helper they should already have used
This commit is contained in:
@@ -201,15 +201,16 @@ func (h *DiscoveryHandlers) isAdminRequest(r *http.Request) bool {
|
||||
}
|
||||
}
|
||||
|
||||
// 3. Check for configured admin session (OIDC/SAML/local session)
|
||||
// 3. Check for an admin session (OIDC/SAML/local session). The admin test is
|
||||
// sessionUserCarriesAdminPrivileges, the same one the settings routes apply,
|
||||
// so an RBAC admin grant and the SSO-principal-with-no-local-admin case both
|
||||
// count. Comparing against h.config.AuthUser alone cannot match on an
|
||||
// instance whose only administrators are SSO principals.
|
||||
if cookie, err := readSessionCookie(r); err == nil && cookie.Value != "" {
|
||||
if ValidateSession(cookie.Value) {
|
||||
configuredAdmin := strings.TrimSpace(h.config.AuthUser)
|
||||
if configuredAdmin != "" {
|
||||
sessionUser := strings.TrimSpace(GetSessionUsername(cookie.Value))
|
||||
if strings.EqualFold(sessionUser, configuredAdmin) {
|
||||
return true
|
||||
}
|
||||
sessionUser := strings.TrimSpace(GetSessionUsername(cookie.Value))
|
||||
if sessionUserCarriesAdminPrivileges(h.config, sessionUser) {
|
||||
return true
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -0,0 +1,130 @@
|
||||
package api
|
||||
|
||||
import (
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"strconv"
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/rcourtman/pulse-go-rewrite/internal/config"
|
||||
"github.com/rcourtman/pulse-go-rewrite/pkg/auth"
|
||||
"golang.org/x/crypto/bcrypt"
|
||||
)
|
||||
|
||||
func adminParitySession(t *testing.T, user string) *http.Cookie {
|
||||
t.Helper()
|
||||
tok := "admin-parity-" + strconv.FormatInt(time.Now().UnixNano(), 10)
|
||||
GetSessionStore().CreateSession(tok, time.Hour, "browser", "127.0.0.1", user)
|
||||
return &http.Cookie{Name: sessionCookieName(false), Value: tok}
|
||||
}
|
||||
|
||||
func adminParityConfig(t *testing.T, adminUser string) *config.Config {
|
||||
t.Helper()
|
||||
dir := t.TempDir()
|
||||
// An API token is present so the export/import path treats auth as required
|
||||
// on the OIDC-only instance too, which is the shape that actually 403s.
|
||||
record, err := config.NewAPITokenRecord("admin-parity-token-123.12345678", "parity", []string{config.ScopeSettingsWrite})
|
||||
if err != nil {
|
||||
t.Fatalf("NewAPITokenRecord: %v", err)
|
||||
}
|
||||
cfg := &config.Config{DataPath: dir, ConfigPath: dir, APITokens: []config.APITokenRecord{*record}}
|
||||
if adminUser != "" {
|
||||
hashed, err := bcrypt.GenerateFromPassword([]byte("admin-parity-password"), bcrypt.DefaultCost)
|
||||
if err != nil {
|
||||
t.Fatalf("bcrypt: %v", err)
|
||||
}
|
||||
cfg.AuthUser = adminUser
|
||||
cfg.AuthPass = string(hashed)
|
||||
}
|
||||
return cfg
|
||||
}
|
||||
|
||||
func exportStatusFor(t *testing.T, router *Router, user string) int {
|
||||
t.Helper()
|
||||
cookie := adminParitySession(t, user)
|
||||
req := httptest.NewRequest(http.MethodPost, "/api/config/export",
|
||||
strings.NewReader(`{"passphrase":"long-enough-passphrase"}`))
|
||||
req.Header.Set("Content-Type", "application/json")
|
||||
req.AddCookie(cookie)
|
||||
req.Header.Set("X-CSRF-Token", generateCSRFToken(cookie.Value))
|
||||
rec := httptest.NewRecorder()
|
||||
router.Handler().ServeHTTP(rec, req)
|
||||
return rec.Code
|
||||
}
|
||||
|
||||
// On the OIDC-only pattern there is no local admin, so SSO principals are the
|
||||
// instance's administrators and ensureAdminSession admits them. Several guards
|
||||
// compared the session user against cfg.AuthUser instead, which is empty here,
|
||||
// so they could admit nobody at all and locked the operator out of discovery,
|
||||
// public URL capture and their own config export.
|
||||
func TestOIDCOnlyAdminReachesEveryAdminGuard(t *testing.T) {
|
||||
prev := auth.GetAuthorizer()
|
||||
auth.SetAuthorizer(&auth.DefaultAuthorizer{})
|
||||
defer auth.SetAuthorizer(prev)
|
||||
|
||||
cfg := adminParityConfig(t, "")
|
||||
router := NewRouter(cfg, nil, nil, nil, nil, "1.0.0")
|
||||
ssoAdmin := "sso:owner@example.com"
|
||||
|
||||
if !sessionUserCarriesAdminPrivileges(cfg, ssoAdmin) {
|
||||
t.Fatal("precondition: the canonical helper must treat this principal as an admin")
|
||||
}
|
||||
|
||||
req := httptest.NewRequest(http.MethodGet, "/api/health", nil)
|
||||
req.AddCookie(adminParitySession(t, ssoAdmin))
|
||||
if !canCapturePublicURL(cfg, req) {
|
||||
t.Error("canCapturePublicURL refused the OIDC-only administrator")
|
||||
}
|
||||
|
||||
h := &DiscoveryHandlers{config: cfg}
|
||||
req2 := httptest.NewRequest(http.MethodGet, "/api/discovery/status", nil)
|
||||
req2.AddCookie(adminParitySession(t, ssoAdmin))
|
||||
if !h.isAdminRequest(req2) {
|
||||
t.Error("discovery isAdminRequest refused the OIDC-only administrator")
|
||||
}
|
||||
|
||||
if code := exportStatusFor(t, router, ssoAdmin); code == http.StatusForbidden {
|
||||
t.Error("config export refused the OIDC-only administrator")
|
||||
}
|
||||
}
|
||||
|
||||
// The parity fix must not widen anything. On an instance that does configure a
|
||||
// local admin, an unrelated SSO principal is not an administrator and every one
|
||||
// of these guards must still refuse them.
|
||||
func TestUnrelatedSSOUserStillRefusedWhenLocalAdminConfigured(t *testing.T) {
|
||||
prev := auth.GetAuthorizer()
|
||||
auth.SetAuthorizer(&auth.DefaultAuthorizer{})
|
||||
defer auth.SetAuthorizer(prev)
|
||||
|
||||
cfg := adminParityConfig(t, "admin")
|
||||
router := NewRouter(cfg, nil, nil, nil, nil, "1.0.0")
|
||||
outsider := "sso:outsider@example.com"
|
||||
|
||||
if sessionUserCarriesAdminPrivileges(cfg, outsider) {
|
||||
t.Fatal("precondition: an unrelated SSO user must not be an admin when a local admin is configured")
|
||||
}
|
||||
|
||||
req := httptest.NewRequest(http.MethodGet, "/api/health", nil)
|
||||
req.AddCookie(adminParitySession(t, outsider))
|
||||
if canCapturePublicURL(cfg, req) {
|
||||
t.Error("canCapturePublicURL admitted a non-admin SSO user")
|
||||
}
|
||||
|
||||
h := &DiscoveryHandlers{config: cfg}
|
||||
req2 := httptest.NewRequest(http.MethodGet, "/api/discovery/status", nil)
|
||||
req2.AddCookie(adminParitySession(t, outsider))
|
||||
if h.isAdminRequest(req2) {
|
||||
t.Error("discovery isAdminRequest admitted a non-admin SSO user")
|
||||
}
|
||||
|
||||
if code := exportStatusFor(t, router, outsider); code != http.StatusForbidden {
|
||||
t.Errorf("config export for a non-admin SSO user = %d, want 403", code)
|
||||
}
|
||||
|
||||
// The configured admin is unaffected.
|
||||
if code := exportStatusFor(t, router, "admin"); code == http.StatusForbidden {
|
||||
t.Error("config export refused the configured local admin")
|
||||
}
|
||||
}
|
||||
@@ -5007,16 +5007,17 @@ func canCapturePublicURL(cfg *config.Config, req *http.Request) bool {
|
||||
}
|
||||
}
|
||||
|
||||
// Session (Browser): allow capture only for the configured local admin session.
|
||||
// This prevents low-privilege session users from poisoning public URL auto-detection.
|
||||
// Session (Browser): allow capture only for an admin session. This prevents
|
||||
// low-privilege session users from poisoning public URL auto-detection.
|
||||
// The admin test is sessionUserCarriesAdminPrivileges, the same one the
|
||||
// settings routes apply, rather than a local comparison against
|
||||
// cfg.AuthUser: that comparison cannot match on an instance whose only
|
||||
// administrators are SSO principals, so it locked those operators out.
|
||||
if cookie, err := readSessionCookie(req); err == nil && cookie.Value != "" {
|
||||
if ValidateSession(cookie.Value) {
|
||||
adminUser := strings.TrimSpace(cfg.AuthUser)
|
||||
if adminUser != "" {
|
||||
username := strings.TrimSpace(GetSessionUsername(cookie.Value))
|
||||
if constantTimeStringEqual(strings.ToLower(username), strings.ToLower(adminUser)) {
|
||||
return true
|
||||
}
|
||||
username := strings.TrimSpace(GetSessionUsername(cookie.Value))
|
||||
if sessionUserCarriesAdminPrivileges(cfg, username) {
|
||||
return true
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -437,8 +437,12 @@ func (r *Router) registerConfigSystemRoutes(updateHandlers *UpdateHandlers) {
|
||||
hasValidSession = ValidateSession(cookie.Value)
|
||||
if hasValidSession {
|
||||
sessionUsername = strings.TrimSpace(GetSessionUsername(cookie.Value))
|
||||
configuredAdmin := strings.TrimSpace(r.config.AuthUser)
|
||||
sessionIsAdmin = configuredAdmin != "" && strings.EqualFold(sessionUsername, configuredAdmin)
|
||||
// Same admin test the settings routes apply. Comparing
|
||||
// against r.config.AuthUser alone cannot match on an
|
||||
// instance whose only administrators are SSO principals,
|
||||
// which locked those operators out of their own config
|
||||
// export and import.
|
||||
sessionIsAdmin = sessionUserCarriesAdminPrivileges(r.config, sessionUsername)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -567,8 +571,12 @@ func (r *Router) registerConfigSystemRoutes(updateHandlers *UpdateHandlers) {
|
||||
hasValidSession = ValidateSession(cookie.Value)
|
||||
if hasValidSession {
|
||||
sessionUsername = strings.TrimSpace(GetSessionUsername(cookie.Value))
|
||||
configuredAdmin := strings.TrimSpace(r.config.AuthUser)
|
||||
sessionIsAdmin = configuredAdmin != "" && strings.EqualFold(sessionUsername, configuredAdmin)
|
||||
// Same admin test the settings routes apply. Comparing
|
||||
// against r.config.AuthUser alone cannot match on an
|
||||
// instance whose only administrators are SSO principals,
|
||||
// which locked those operators out of their own config
|
||||
// export and import.
|
||||
sessionIsAdmin = sessionUserCarriesAdminPrivileges(r.config, sessionUsername)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user