From 984b0c233502cbeef3a1781b338a0f162134b7d0 Mon Sep 17 00:00:00 2001 From: Konrad Neitzel Date: Fri, 28 Aug 2026 12:39:47 +0200 Subject: [PATCH] Skip /js/ in the Access Gate and set denied-page CSP so Denied UI can render. Co-authored-by: Cursor --- CONTEXT.md | 8 ++++++-- README.md | 29 +++++++++++++++++++++++++++-- access_gate.go | 18 ++++++++++++++++-- access_gate_test.go | 25 +++++++++++++++++++++++++ example_test.go | 9 ++++++++- top_menu_env.go | 27 +++++++++++++++++++++++++++ top_menu_env_test.go | 25 +++++++++++++++++++++++++ 7 files changed, 134 insertions(+), 7 deletions(-) create mode 100644 top_menu_env.go create mode 100644 top_menu_env_test.go diff --git a/CONTEXT.md b/CONTEXT.md index 68f7349..7e27754 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -25,5 +25,9 @@ The Nextcloud groups configured for an ExApp (comma-separated deploy env `REQUIR _Avoid_: AppAPI scopes, route access_level, admin-only top menu, treating the ExApp id as an implicit group name **Access Gate**: -The Library check that enforces Required Groups for the Requesting user on ExApp HTTP traffic (403 or denied UI when not a member; 401 without a user; 503 when membership cannot be determined). Lifecycle paths stay ungated. -_Avoid_: Nextcloud middleware, HaRP ACL, admin bypass +The Library check that enforces Required Groups for the Requesting user on ExApp HTTP traffic (403 or denied UI when not a member; 401 without a user; 503 when membership cannot be determined). Lifecycle paths and top-menu script URLs under `/js/` stay ungated so Denied UI can load in the Nextcloud shell. +_Avoid_: Nextcloud middleware, HaRP ACL, admin bypass, gating the top-menu bootstrap script + +**Top Menu visibility**: +Whether the ExApp app icon in the Nextcloud top menu is shown to all logged-in users or to Nextcloud admins only. Configured per deploy via env `TOP_MENU_ADMIN_REQUIRED` (`0` or `1`); the ExApp passes the value to AppAPI when registering the top-menu entry on enable. Independent of route `access_level` in info.xml and of Required Groups. +_Avoid_: route access_level, Required Groups, AppAPI group ACL diff --git a/README.md b/README.md index d5c2cfe..3866ca2 100644 --- a/README.md +++ b/README.md @@ -14,7 +14,8 @@ Import: `gitea.neitzel.de/konrad/go-nc-exapp` (package `gonexapp`). - **UserFromRequest** — extract the requesting user from inbound AppAPI-proxied requests - **OCSClient** — authenticated OCS calls that always append `format=json` - **AppAPIPreferences** — parameterized get/set of a string ExApp preference (caller supplies app id and key) -- **Access Gate** — optional Required Groups enforcement (`Wrap` + `Check`), English denied HTML for browsers, positive membership cache; env helpers for `REQUIRED_GROUPS` / `REQUIRED_GROUPS_CACHE_SECONDS` +- **Access Gate** — optional Required Groups enforcement (`Wrap` + `Check`), English denied HTML for browsers (200 + `frame-ancestors 'self'`), positive membership cache; default skip for lifecycle paths and **`/js/`** top-menu scripts; env helpers for `REQUIRED_GROUPS` / `REQUIRED_GROUPS_CACHE_SECONDS` +- **Top Menu visibility** — `TopMenuAdminRequired` helper for deploy env `TOP_MENU_ADMIN_REQUIRED` (`0` / `1` for AppAPI top-menu OCS) **Excluded** @@ -49,11 +50,35 @@ Each ExApp chooses its own preference keys; this library does not hardcode produ Declare `REQUIRED_GROUPS` and `REQUIRED_GROUPS_CACHE_SECONDS` in the ExApp `info.xml` so Deploy options can set them. +The Gate skips `/heartbeat`, `/enabled`, `/init`, and any path under **`/js/`** (AppAPI top-menu bootstrap). Serve the registered top-menu script under `/js/…` so a non-member still loads it and can show Denied UI in the Nextcloud shell. API routes stay gated. + +Denied HTML is **200** with `Content-Security-Policy: … frame-ancestors 'self'`. Without that header AppAPI’s proxy defaults to `frame-ancestors 'none'` and a denied iframe stays blank. Non-HTML denials remain **403**. + +### Top Menu visibility (`TOP_MENU_ADMIN_REQUIRED`) + +Declare in `info.xml` under ``. At enable time the ExApp reads the env and passes `"0"` or `"1"` to AppAPI’s top-menu OCS `adminRequired`. Only `0` and `1` are valid; anything else falls back to `DefaultTopMenuAdminRequired` (`true` → admins only). + +```go +adminRequired := gonexapp.TopMenuAdminRequired( + os.Getenv(gonexapp.EnvTopMenuAdminRequired), + gonexapp.DefaultTopMenuAdminRequired, +) +// use adminRequired in POST …/ui/top-menu when registering the menu entry +``` + +**Applying a change:** AppAPI registers the top menu when the ExApp receives `PUT /enabled?enabled=1`. Changing the deploy env alone does not update the menu entry. + +1. Set the new value in Deploy options (UI) or `occ app_api:app:register … --env TOP_MENU_ADMIN_REQUIRED=…` / update deploy config. +2. Recreate or restart the ExApp container so the new env is present. +3. Re-run lifecycle: disable then enable the ExApp (UI or `occ app_api:app:disable` / `app_api:app:enable`), or `occ app_api:app:update … -e` after an image/info update. + +Route `access_level` in `info.xml` is separate and only changes when AppAPI re-reads `info.xml` on register/update — not via this env. + Runnable package examples: `go test -run Example`. ## Domain language -See [CONTEXT.md](./CONTEXT.md) for AppAPI credentials, Requesting user, ExApp preference, OCS, Required Groups, and Access Gate terminology. +See [CONTEXT.md](./CONTEXT.md) for AppAPI credentials, Requesting user, ExApp preference, OCS, Required Groups, Access Gate, and Top Menu visibility terminology. ## Testing diff --git a/access_gate.go b/access_gate.go index 97ba5a0..10d6cbe 100644 --- a/access_gate.go +++ b/access_gate.go @@ -181,9 +181,10 @@ func (g AccessGate) normalized() *AccessGate { func (g *AccessGate) writeDenied(w http.ResponseWriter, r *http.Request) { if acceptsHTML(r.Header.Get("Accept")) { - // 200 so AppAPI's proxy CSP keeps frame-ancestors 'self' and the ExApp - // iframe can show the message (403 responses get frame-ancestors 'none'). + // 200 plus frame-ancestors 'self': AppAPI's default proxy CSP uses + // frame-ancestors 'none' unless the ExApp sets CSP, which blanks iframes. w.Header().Set("Content-Type", "text/html; charset=utf-8") + w.Header().Set("Content-Security-Policy", deniedCSP) w.WriteHeader(http.StatusOK) _, _ = w.Write(deniedHTML) return @@ -197,6 +198,9 @@ func acceptsHTML(accept string) bool { func (g *AccessGate) shouldSkip(path string) bool { path = normalizeGatePath(path) + if isTopMenuScriptPath(path) { + return true + } for _, p := range defaultSkipPaths { if path == p { return true @@ -210,6 +214,13 @@ func (g *AccessGate) shouldSkip(path string) bool { return false } +// isTopMenuScriptPath reports AppAPI top-menu bootstrap scripts under /js/. +// Those must load for a non-member so the shell can show Denied UI; gating +// them yields a blank embedded page (script Accept is not text/html → 403). +func isTopMenuScriptPath(path string) bool { + return path == "/js" || strings.HasPrefix(path, "/js/") +} + func normalizeGatePath(path string) string { path = strings.TrimSuffix(path, "/") if path == "" { @@ -220,6 +231,9 @@ func normalizeGatePath(path string) string { var defaultSkipPaths = []string{"/heartbeat", "/enabled", "/init"} +// deniedCSP lets AppAPI proxy the denied page into an ExApp iframe. +const deniedCSP = "default-src 'none'; base-uri 'none'; form-action 'none'; frame-ancestors 'self'; style-src 'unsafe-inline'" + var deniedHTML = []byte(` diff --git a/access_gate_test.go b/access_gate_test.go index 90ba7bd..a70f82c 100644 --- a/access_gate_test.go +++ b/access_gate_test.go @@ -168,6 +168,31 @@ func TestAccessGateDeniesNonMemberWithHTML(t *testing.T) { if !strings.Contains(rec.Body.String(), "Access denied") { t.Fatalf("body=%q", rec.Body.String()) } + csp := rec.Header().Get("Content-Security-Policy") + if !strings.Contains(csp, "frame-ancestors 'self'") { + t.Fatalf("csp=%q", csp) + } +} + +func TestAccessGateSkipsTopMenuScriptPrefix(t *testing.T) { + gate := gonexapp.AccessGate{Groups: []string{"dns-ops"}} + h := gate.Wrap(okInner()) + + for _, path := range []string{"/js/checkdns-main.js", "/js/app.js", "/js"} { + req := httptest.NewRequest(http.MethodGet, path, nil) + rec := httptest.NewRecorder() + h.ServeHTTP(rec, req) + if rec.Code != http.StatusOK || rec.Body.String() != "ok" { + t.Fatalf("%s: got %d %q", path, rec.Code, rec.Body.String()) + } + } + + req := httptest.NewRequest(http.MethodGet, "/json", nil) + rec := httptest.NewRecorder() + h.ServeHTTP(rec, req) + if rec.Code != http.StatusUnauthorized { + t.Fatalf("/json should stay gated, got %d", rec.Code) + } } func TestAccessGateLookupFailureServiceUnavailable(t *testing.T) { diff --git a/example_test.go b/example_test.go index 92b76ef..a574ac0 100644 --- a/example_test.go +++ b/example_test.go @@ -71,7 +71,6 @@ func ExampleAccessGate_Wrap() { inner := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { _, _ = io.WriteString(w, "ok") }) - // Empty Required Groups: gate is inactive. h := gonexapp.AccessGate{}.Wrap(inner) rec := httptest.NewRecorder() @@ -87,3 +86,11 @@ func ExampleResolveRequiredGroups() { // [] // 0 } + +func ExampleTopMenuAdminRequired() { + fmt.Println(gonexapp.TopMenuAdminRequired("0", gonexapp.DefaultTopMenuAdminRequired)) + fmt.Println(gonexapp.TopMenuAdminRequired("maybe", gonexapp.DefaultTopMenuAdminRequired)) + // Output: + // 0 + // 1 +} diff --git a/top_menu_env.go b/top_menu_env.go new file mode 100644 index 0000000..c81b4f2 --- /dev/null +++ b/top_menu_env.go @@ -0,0 +1,27 @@ +package gonexapp + +import "strings" + +// EnvTopMenuAdminRequired is the conventional deploy env name for Top Menu visibility. +// Declare it in the ExApp info.xml environment-variables section. +const EnvTopMenuAdminRequired = "TOP_MENU_ADMIN_REQUIRED" + +// DefaultTopMenuAdminRequired is used when TOP_MENU_ADMIN_REQUIRED is unset or invalid. +const DefaultTopMenuAdminRequired = true + +// TopMenuAdminRequired returns "1" or "0" for the AppAPI top-menu OCS adminRequired field. +// Only "0" and "1" are accepted; any other value falls back to defaultAdminRequired. +// An empty envValue means unset and also uses defaultAdminRequired. +func TopMenuAdminRequired(envValue string, defaultAdminRequired bool) string { + switch strings.TrimSpace(envValue) { + case "1": + return "1" + case "0": + return "0" + default: + if defaultAdminRequired { + return "1" + } + return "0" + } +} diff --git a/top_menu_env_test.go b/top_menu_env_test.go new file mode 100644 index 0000000..0f2996d --- /dev/null +++ b/top_menu_env_test.go @@ -0,0 +1,25 @@ +package gonexapp + +import "testing" + +func TestTopMenuAdminRequired(t *testing.T) { + tests := []struct { + env string + defAdmin bool + want string + }{ + {"", true, "1"}, + {"", false, "0"}, + {"1", true, "1"}, + {"0", true, "0"}, + {" 1 ", true, "1"}, + {"yes", true, "1"}, + {"yes", false, "0"}, + {"2", true, "1"}, + } + for _, tc := range tests { + if got := TopMenuAdminRequired(tc.env, tc.defAdmin); got != tc.want { + t.Errorf("TopMenuAdminRequired(%q, %v) = %q, want %q", tc.env, tc.defAdmin, got, tc.want) + } + } +}