Fix SEC-09: hide admin nav links from non-admin users
- Add IsAdmin bool to PageData (embedded in all page view structs)
- Remove redundant IsAdmin from DashboardData
- Add isAdmin() helper to derive admin status from request claims
- Set IsAdmin in all page-level handlers that populate PageData
- Wrap admin-only nav links in base.html with {{if .IsAdmin}}
- Add tests: non-admin dashboard/profile hide admin links,
admin dashboard shows them
Security: navigation links to /accounts, /audit, /policies,
and /pgcreds are now only rendered for admin users. Server-side
authorization (requireAdminRole middleware) was already in place;
this change removes the information leak of showing links that
return 403 to non-admin users.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -39,7 +39,7 @@ func (u *UIServer) handleAccountsList(w http.ResponseWriter, r *http.Request) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
u.render(w, "accounts", AccountsData{
|
u.render(w, "accounts", AccountsData{
|
||||||
PageData: PageData{CSRFToken: csrfToken, ActorName: u.actorName(r)},
|
PageData: PageData{CSRFToken: csrfToken, ActorName: u.actorName(r), IsAdmin: isAdmin(r)},
|
||||||
Accounts: accounts,
|
Accounts: accounts,
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
@@ -183,7 +183,7 @@ func (u *UIServer) handleAccountDetail(w http.ResponseWriter, r *http.Request) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
u.render(w, "account_detail", AccountDetailData{
|
u.render(w, "account_detail", AccountDetailData{
|
||||||
PageData: PageData{CSRFToken: csrfToken, ActorName: u.actorName(r)},
|
PageData: PageData{CSRFToken: csrfToken, ActorName: u.actorName(r), IsAdmin: isAdmin(r)},
|
||||||
Account: acct,
|
Account: acct,
|
||||||
Roles: roles,
|
Roles: roles,
|
||||||
AllRoles: knownRoles,
|
AllRoles: knownRoles,
|
||||||
@@ -790,7 +790,7 @@ func (u *UIServer) handlePGCredsList(w http.ResponseWriter, r *http.Request) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
u.render(w, "pgcreds", PGCredsData{
|
u.render(w, "pgcreds", PGCredsData{
|
||||||
PageData: PageData{CSRFToken: csrfToken, ActorName: u.actorName(r)},
|
PageData: PageData{CSRFToken: csrfToken, ActorName: u.actorName(r), IsAdmin: isAdmin(r)},
|
||||||
Creds: creds,
|
Creds: creds,
|
||||||
UncredentialedAccounts: uncredentialed,
|
UncredentialedAccounts: uncredentialed,
|
||||||
CredGrants: credGrants,
|
CredGrants: credGrants,
|
||||||
|
|||||||
@@ -86,7 +86,7 @@ func (u *UIServer) handleAuditDetail(w http.ResponseWriter, r *http.Request) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
u.render(w, "audit_detail", AuditDetailData{
|
u.render(w, "audit_detail", AuditDetailData{
|
||||||
PageData: PageData{CSRFToken: csrfToken, ActorName: u.actorName(r)},
|
PageData: PageData{CSRFToken: csrfToken, ActorName: u.actorName(r), IsAdmin: isAdmin(r)},
|
||||||
Event: event,
|
Event: event,
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
@@ -116,7 +116,7 @@ func (u *UIServer) buildAuditData(r *http.Request, page int, csrfToken string) (
|
|||||||
}
|
}
|
||||||
|
|
||||||
return AuditData{
|
return AuditData{
|
||||||
PageData: PageData{CSRFToken: csrfToken, ActorName: u.actorName(r)},
|
PageData: PageData{CSRFToken: csrfToken, ActorName: u.actorName(r), IsAdmin: isAdmin(r)},
|
||||||
Events: events,
|
Events: events,
|
||||||
EventTypes: auditEventTypes,
|
EventTypes: auditEventTypes,
|
||||||
FilterType: filterType,
|
FilterType: filterType,
|
||||||
|
|||||||
@@ -281,6 +281,7 @@ func (u *UIServer) handleProfilePage(w http.ResponseWriter, r *http.Request) {
|
|||||||
PageData: PageData{
|
PageData: PageData{
|
||||||
CSRFToken: csrfToken,
|
CSRFToken: csrfToken,
|
||||||
ActorName: u.actorName(r),
|
ActorName: u.actorName(r),
|
||||||
|
IsAdmin: isAdmin(r),
|
||||||
},
|
},
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
@@ -393,6 +394,7 @@ func (u *UIServer) handleSelfChangePassword(w http.ResponseWriter, r *http.Reque
|
|||||||
PageData: PageData{
|
PageData: PageData{
|
||||||
CSRFToken: csrfToken,
|
CSRFToken: csrfToken,
|
||||||
ActorName: u.actorName(r),
|
ActorName: u.actorName(r),
|
||||||
|
IsAdmin: isAdmin(r),
|
||||||
Flash: "Password updated successfully. Other active sessions have been revoked.",
|
Flash: "Password updated successfully. Other active sessions have been revoked.",
|
||||||
},
|
},
|
||||||
})
|
})
|
||||||
|
|||||||
@@ -17,15 +17,13 @@ func (u *UIServer) handleDashboard(w http.ResponseWriter, r *http.Request) {
|
|||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
claims := claimsFromContext(r.Context())
|
admin := isAdmin(r)
|
||||||
isAdmin := claims != nil && claims.HasRole("admin")
|
|
||||||
|
|
||||||
data := DashboardData{
|
data := DashboardData{
|
||||||
PageData: PageData{CSRFToken: csrfToken, ActorName: u.actorName(r)},
|
PageData: PageData{CSRFToken: csrfToken, ActorName: u.actorName(r), IsAdmin: admin},
|
||||||
IsAdmin: isAdmin,
|
|
||||||
}
|
}
|
||||||
|
|
||||||
if isAdmin {
|
if admin {
|
||||||
accounts, err := u.db.ListAccounts()
|
accounts, err := u.db.ListAccounts()
|
||||||
if err != nil {
|
if err != nil {
|
||||||
u.renderError(w, r, http.StatusInternalServerError, "failed to load accounts")
|
u.renderError(w, r, http.StatusInternalServerError, "failed to load accounts")
|
||||||
|
|||||||
@@ -61,7 +61,7 @@ func (u *UIServer) handlePoliciesPage(w http.ResponseWriter, r *http.Request) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
data := PoliciesData{
|
data := PoliciesData{
|
||||||
PageData: PageData{CSRFToken: csrfToken, ActorName: u.actorName(r)},
|
PageData: PageData{CSRFToken: csrfToken, ActorName: u.actorName(r), IsAdmin: isAdmin(r)},
|
||||||
Rules: views,
|
Rules: views,
|
||||||
AllActions: allActionStrings,
|
AllActions: allActionStrings,
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -569,6 +569,13 @@ func (u *UIServer) clientIP(r *http.Request) string {
|
|||||||
return middleware.ClientIP(r, proxyIP)
|
return middleware.ClientIP(r, proxyIP)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// isAdmin reports whether the authenticated user holds the "admin" role.
|
||||||
|
// Returns false if claims are absent.
|
||||||
|
func isAdmin(r *http.Request) bool {
|
||||||
|
claims := claimsFromContext(r.Context())
|
||||||
|
return claims != nil && claims.HasRole("admin")
|
||||||
|
}
|
||||||
|
|
||||||
// actorName resolves the username of the currently authenticated user from the
|
// actorName resolves the username of the currently authenticated user from the
|
||||||
// request context. Returns an empty string if claims are absent or the account
|
// request context. Returns an empty string if claims are absent or the account
|
||||||
// cannot be found; callers should treat an empty string as "not logged in".
|
// cannot be found; callers should treat an empty string as "not logged in".
|
||||||
@@ -594,6 +601,10 @@ type PageData struct {
|
|||||||
// ActorName is the username of the currently logged-in user, populated by
|
// ActorName is the username of the currently logged-in user, populated by
|
||||||
// handlers so the base template can display it in the navigation bar.
|
// handlers so the base template can display it in the navigation bar.
|
||||||
ActorName string
|
ActorName string
|
||||||
|
// IsAdmin is true when the logged-in user holds the "admin" role.
|
||||||
|
// Used by the base template to conditionally render admin-only navigation
|
||||||
|
// links (SEC-09: non-admin users must not see links they cannot access).
|
||||||
|
IsAdmin bool
|
||||||
}
|
}
|
||||||
|
|
||||||
// LoginData is the view model for the login page.
|
// LoginData is the view model for the login page.
|
||||||
@@ -609,7 +620,6 @@ type LoginData struct {
|
|||||||
// DashboardData is the view model for the dashboard page.
|
// DashboardData is the view model for the dashboard page.
|
||||||
type DashboardData struct {
|
type DashboardData struct {
|
||||||
PageData
|
PageData
|
||||||
IsAdmin bool
|
|
||||||
RecentEvents []*db.AuditEventView
|
RecentEvents []*db.AuditEventView
|
||||||
TotalAccounts int
|
TotalAccounts int
|
||||||
ActiveAccounts int
|
ActiveAccounts int
|
||||||
|
|||||||
@@ -527,3 +527,121 @@ func TestAccountDetailShowsPGCredsSection(t *testing.T) {
|
|||||||
t.Error("human account detail page must not include pgcreds-section")
|
t.Error("human account detail page must not include pgcreds-section")
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// ---- SEC-09: admin nav link visibility tests ----
|
||||||
|
|
||||||
|
// issueUserSession creates a human account with the "user" role (non-admin),
|
||||||
|
// issues a JWT, tracks it, and returns the raw token string.
|
||||||
|
func issueUserSession(t *testing.T, u *UIServer) string {
|
||||||
|
t.Helper()
|
||||||
|
acct, err := u.db.CreateAccount("regular-user", model.AccountTypeHuman, "")
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("CreateAccount: %v", err)
|
||||||
|
}
|
||||||
|
if err := u.db.SetRoles(acct.ID, []string{"user"}, nil); err != nil {
|
||||||
|
t.Fatalf("SetRoles: %v", err)
|
||||||
|
}
|
||||||
|
tok, claims, err := token.IssueToken(u.privKey, testIssuer, acct.UUID, []string{"user"}, time.Hour)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("IssueToken: %v", err)
|
||||||
|
}
|
||||||
|
if err := u.db.TrackToken(claims.JTI, acct.ID, claims.IssuedAt, claims.ExpiresAt); err != nil {
|
||||||
|
t.Fatalf("TrackToken: %v", err)
|
||||||
|
}
|
||||||
|
return tok
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestNonAdminDashboardHidesAdminNavLinks verifies that a non-admin user's
|
||||||
|
// dashboard does not contain links to admin-only pages (SEC-09).
|
||||||
|
func TestNonAdminDashboardHidesAdminNavLinks(t *testing.T) {
|
||||||
|
u := newTestUIServer(t)
|
||||||
|
mux := http.NewServeMux()
|
||||||
|
u.Register(mux)
|
||||||
|
|
||||||
|
userToken := issueUserSession(t, u)
|
||||||
|
|
||||||
|
req := authenticatedGET(t, userToken, "/dashboard")
|
||||||
|
rr := httptest.NewRecorder()
|
||||||
|
mux.ServeHTTP(rr, req)
|
||||||
|
|
||||||
|
if rr.Code != http.StatusOK {
|
||||||
|
t.Fatalf("status = %d, want 200; body: %s", rr.Code, rr.Body.String())
|
||||||
|
}
|
||||||
|
|
||||||
|
body := rr.Body.String()
|
||||||
|
for _, adminPath := range []string{
|
||||||
|
`href="/accounts"`,
|
||||||
|
`href="/audit"`,
|
||||||
|
`href="/policies"`,
|
||||||
|
`href="/pgcreds"`,
|
||||||
|
} {
|
||||||
|
if strings.Contains(body, adminPath) {
|
||||||
|
t.Errorf("non-admin dashboard contains admin link %s — SEC-09 violation", adminPath)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Dashboard link should still be present.
|
||||||
|
if !strings.Contains(body, `href="/dashboard"`) {
|
||||||
|
t.Error("dashboard link missing from non-admin nav")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestAdminDashboardShowsAdminNavLinks verifies that an admin user's
|
||||||
|
// dashboard contains all admin navigation links.
|
||||||
|
func TestAdminDashboardShowsAdminNavLinks(t *testing.T) {
|
||||||
|
u := newTestUIServer(t)
|
||||||
|
mux := http.NewServeMux()
|
||||||
|
u.Register(mux)
|
||||||
|
|
||||||
|
adminToken, _, _ := issueAdminSession(t, u)
|
||||||
|
|
||||||
|
req := authenticatedGET(t, adminToken, "/dashboard")
|
||||||
|
rr := httptest.NewRecorder()
|
||||||
|
mux.ServeHTTP(rr, req)
|
||||||
|
|
||||||
|
if rr.Code != http.StatusOK {
|
||||||
|
t.Fatalf("status = %d, want 200; body: %s", rr.Code, rr.Body.String())
|
||||||
|
}
|
||||||
|
|
||||||
|
body := rr.Body.String()
|
||||||
|
for _, adminPath := range []string{
|
||||||
|
`href="/accounts"`,
|
||||||
|
`href="/audit"`,
|
||||||
|
`href="/policies"`,
|
||||||
|
`href="/pgcreds"`,
|
||||||
|
} {
|
||||||
|
if !strings.Contains(body, adminPath) {
|
||||||
|
t.Errorf("admin dashboard missing admin link %s", adminPath)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestNonAdminProfileHidesAdminNavLinks verifies that the profile page
|
||||||
|
// also hides admin nav links for non-admin users (SEC-09).
|
||||||
|
func TestNonAdminProfileHidesAdminNavLinks(t *testing.T) {
|
||||||
|
u := newTestUIServer(t)
|
||||||
|
mux := http.NewServeMux()
|
||||||
|
u.Register(mux)
|
||||||
|
|
||||||
|
userToken := issueUserSession(t, u)
|
||||||
|
|
||||||
|
req := authenticatedGET(t, userToken, "/profile")
|
||||||
|
rr := httptest.NewRecorder()
|
||||||
|
mux.ServeHTTP(rr, req)
|
||||||
|
|
||||||
|
if rr.Code != http.StatusOK {
|
||||||
|
t.Fatalf("status = %d, want 200; body: %s", rr.Code, rr.Body.String())
|
||||||
|
}
|
||||||
|
|
||||||
|
body := rr.Body.String()
|
||||||
|
for _, adminPath := range []string{
|
||||||
|
`href="/accounts"`,
|
||||||
|
`href="/audit"`,
|
||||||
|
`href="/policies"`,
|
||||||
|
`href="/pgcreds"`,
|
||||||
|
} {
|
||||||
|
if strings.Contains(body, adminPath) {
|
||||||
|
t.Errorf("non-admin profile page contains admin link %s — SEC-09 violation", adminPath)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
@@ -12,10 +12,10 @@
|
|||||||
<span class="nav-brand">MCIAS</span>
|
<span class="nav-brand">MCIAS</span>
|
||||||
<ul class="nav-links">
|
<ul class="nav-links">
|
||||||
<li><a href="/dashboard">Dashboard</a></li>
|
<li><a href="/dashboard">Dashboard</a></li>
|
||||||
<li><a href="/accounts">Accounts</a></li>
|
{{if .IsAdmin}}<li><a href="/accounts">Accounts</a></li>
|
||||||
<li><a href="/audit">Audit</a></li>
|
<li><a href="/audit">Audit</a></li>
|
||||||
<li><a href="/policies">Policies</a></li>
|
<li><a href="/policies">Policies</a></li>
|
||||||
<li><a href="/pgcreds">PG Creds</a></li>
|
<li><a href="/pgcreds">PG Creds</a></li>{{end}}
|
||||||
{{if .ActorName}}<li><a href="/profile">{{.ActorName}}</a></li>{{end}}
|
{{if .ActorName}}<li><a href="/profile">{{.ActorName}}</a></li>{{end}}
|
||||||
<li><form method="POST" action="/logout" style="margin:0"><button class="btn btn-sm btn-secondary" type="submit">Logout</button></form></li>
|
<li><form method="POST" action="/logout" style="margin:0"><button class="btn btn-sm btn-secondary" type="submit">Logout</button></form></li>
|
||||||
</ul>
|
</ul>
|
||||||
|
|||||||
Reference in New Issue
Block a user