gsadmin/pulse
1
0
Fork 0
mirror of https://github.com/rcourtman/Pulse.git synced 2026-09-10 02:25:56 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(api): stop advertising settings surfaces the routes refuse

Browse Source 
A settings capability is a promise the routes have to keep. Without an RBAC
licence the authorizer allows every action, so capabilities derived from it
alone reported true while the matching route stayed gated by
ensureSettingsScope and in turn ensureAdminSession.

canAccessPermissionSurface already refused to trust the authorizer for a
non-admin proxy caller. The session half of that rule was never written, so a
non-admin session on an unlicensed instance was told apiAccessRead,
apiAccessWrite, singleSignOnRead and singleSignOnWrite were all available. The
nav gates on exactly those flags, so the API Access and Single Sign-On tabs
rendered, their first request came back 403, and the user got an error toast
on a tab they were never able to use. Everything routed through
canAccessAdminSurface was already correct, which is why authenticationRead and
the audit surfaces behaved and these two did not.

The fallback uses snapshot.sessionIsAdmin, which derives from the same
sessionUserCarriesAdminPrivileges the routes enforce, so the capability now
matches the answer the route will give. That keeps the OIDC-only pattern
working, where SSO principals are the instance's only administrators.

Nothing was reachable that should not have been. This is a capability
reporting fix, not an access control one.

Refs #1672

Contract-Neutral: settingsCapabilities JSON shape is unchanged (same 14 fields, same types); this corrects a wrong value returned to non-admin sessions when no RBAC authorizer is registered, no public-contract delta
This commit is contained in:
rcourtman
2026-08-05 12:02:53 +01:00
parent 7977f3661a
commit 28fd2d1c15
2 changed files with 161 additions and 3 deletions
Download Patch File Download Diff File Expand all files Collapse all files
internal/api/security_status_capabilities.go
+20 -3
View File
@@ -182,9 +182,26 @@ func (r *Router) canAccessPermissionSurface(snapshot securityStatusAuthSnapshot,
return false
}
if snapshot.authMethod == "proxy" && !snapshot.proxyIsAdmin {
if _, isDefaultAuthorizer := r.authorizer.(*internalauth.DefaultAuthorizer); isDefaultAuthorizer {
return false
// Without a real RBAC authorizer, Authorize allows every action, so it
// cannot be the sole input to a capability. The routes these capabilities
// describe are still gated, by ensureSettingsScope and in turn
// ensureAdminSession, so reporting the capability from the authorizer alone
// advertises a surface the caller will be refused: the tab renders and its
// first request comes back 403. Fall back to the same admin identity the
// routes enforce, which snapshot.sessionIsAdmin already derives from
// sessionUserCarriesAdminPrivileges. Only the proxy half of this rule was
// ever written, so session and SSO callers were told they could reach API
// token management and SSO provider configuration when they could not.
if _, isDefaultAuthorizer := r.authorizer.(*internalauth.DefaultAuthorizer); isDefaultAuthorizer {
switch snapshot.authMethod {
case "proxy":
if !snapshot.proxyIsAdmin {
return false
}
case "session":
if !snapshot.sessionIsAdmin {
return false
}
}
}
internal/api/security_status_capability_enforcement_test.go
+141
View File
@@ -0,0 +1,141 @@
package api
import (
"encoding/json"
"net/http"
"net/http/httptest"
"strconv"
"testing"
"time"
"github.com/rcourtman/pulse-go-rewrite/internal/config"
"github.com/rcourtman/pulse-go-rewrite/pkg/auth"
"golang.org/x/crypto/bcrypt"
)
func newCapabilityConfig(t *testing.T, adminUser string) *config.Config {
t.Helper()
tempDir := t.TempDir()
cfg := &config.Config{DataPath: tempDir, ConfigPath: tempDir}
if adminUser != "" {
hashed, err := bcrypt.GenerateFromPassword([]byte("capability-password"), bcrypt.DefaultCost)
if err != nil {
t.Fatalf("bcrypt: %v", err)
}
cfg.AuthUser = adminUser
cfg.AuthPass = string(hashed)
}
return cfg
}
func capabilitySessionCookie(t *testing.T, username string) *http.Cookie {
t.Helper()
token := "capability-session-" + strconv.FormatInt(time.Now().UnixNano(), 10)
GetSessionStore().CreateSession(token, time.Hour, "browser", "127.0.0.1", username)
return &http.Cookie{Name: sessionCookieName(false), Value: token}
}
func fetchSettingsCapabilities(t *testing.T, router *Router, cookie *http.Cookie) map[string]any {
t.Helper()
req := httptest.NewRequest(http.MethodGet, "/api/security/status", nil)
req.AddCookie(cookie)
rec := httptest.NewRecorder()
router.Handler().ServeHTTP(rec, req)
if rec.Code != http.StatusOK {
t.Fatalf("security status = %d, want 200", rec.Code)
}
var payload map[string]any
if err := json.Unmarshal(rec.Body.Bytes(), &payload); err != nil {
t.Fatalf("unmarshal security status: %v", err)
}
caps, ok := payload["settingsCapabilities"].(map[string]any)
if !ok {
t.Fatalf("settingsCapabilities missing from %s", rec.Body.String())
}
return caps
}
// A settings capability is a promise the routes have to keep. Without an RBAC
// licence Authorize allows everything, so capabilities derived from it alone
// reported true while ensureSettingsScope refused the matching request, and the
// API Access and Single Sign-On tabs rendered for users who then got a 403 and
// an error toast. Refs discussion #1672.
func TestSettingsCapabilitiesMatchRouteEnforcementWithoutRBAC(t *testing.T) {
prevAuthorizer := auth.GetAuthorizer()
auth.SetAuthorizer(&auth.DefaultAuthorizer{})
defer auth.SetAuthorizer(prevAuthorizer)
cfg := newCapabilityConfig(t, "admin")
router := NewRouter(cfg, nil, nil, nil, nil, "1.0.0")
caps := fetchSettingsCapabilities(t, router, capabilitySessionCookie(t, "sso:outsider@example.com"))
for _, key := range []string{
"apiAccessRead",
"apiAccessWrite",
"singleSignOnRead",
"singleSignOnWrite",
"authenticationRead",
"authenticationWrite",
} {
if caps[key] != false {
t.Fatalf("%s = %v for a non-admin session, want false", key, caps[key])
}
}
// Every capability the non-admin was refused must in fact be refused by the
// route, otherwise the fix has swung into hiding a usable surface.
for _, probe := range []struct{ method, path string }{
{http.MethodGet, "/api/security/tokens"},
{http.MethodGet, "/api/security/sso/providers"},
} {
req := httptest.NewRequest(probe.method, probe.path, nil)
req.AddCookie(capabilitySessionCookie(t, "sso:outsider@example.com"))
rec := httptest.NewRecorder()
router.Handler().ServeHTTP(rec, req)
if rec.Code != http.StatusForbidden {
t.Fatalf("%s %s = %d, want 403 to match the withheld capability", probe.method, probe.path, rec.Code)
}
}
}
// The configured admin must keep every surface, or the fix hides settings from
// the only account that can reach them.
func TestSettingsCapabilitiesGrantConfiguredAdminWithoutRBAC(t *testing.T) {
prevAuthorizer := auth.GetAuthorizer()
auth.SetAuthorizer(&auth.DefaultAuthorizer{})
defer auth.SetAuthorizer(prevAuthorizer)
cfg := newCapabilityConfig(t, "admin")
router := NewRouter(cfg, nil, nil, nil, nil, "1.0.0")
caps := fetchSettingsCapabilities(t, router, capabilitySessionCookie(t, "admin"))
for _, key := range []string{"apiAccessRead", "apiAccessWrite", "singleSignOnRead", "singleSignOnWrite"} {
if caps[key] != true {
t.Fatalf("%s = %v for the configured admin, want true", key, caps[key])
}
}
}
// On the OIDC-only pattern there is no local admin, so SSO principals are the
// instance's administrators. They must keep the tabs, and the routes agree.
func TestSettingsCapabilitiesGrantSSOAdminWhenNoLocalAdminConfigured(t *testing.T) {
prevAuthorizer := auth.GetAuthorizer()
auth.SetAuthorizer(&auth.DefaultAuthorizer{})
defer auth.SetAuthorizer(prevAuthorizer)
cfg := newCapabilityConfig(t, "")
router := NewRouter(cfg, nil, nil, nil, nil, "1.0.0")
caps := fetchSettingsCapabilities(t, router, capabilitySessionCookie(t, "sso:owner@example.com"))
if caps["apiAccessRead"] != true {
t.Fatalf("apiAccessRead = %v on an OIDC-only instance, want true", caps["apiAccessRead"])
}
req := httptest.NewRequest(http.MethodGet, "/api/security/tokens", nil)
req.AddCookie(capabilitySessionCookie(t, "sso:owner@example.com"))
rec := httptest.NewRecorder()
router.Handler().ServeHTTP(rec, req)
if rec.Code == http.StatusForbidden {
t.Fatal("OIDC-only instance must not refuse its own SSO administrator")
}
}