diff --git a/README.md b/README.md index 3744e5e..0f4df15 100644 --- a/README.md +++ b/README.md @@ -51,6 +51,14 @@ API-key-protected endpoints, health checks) can be excluded per app. and authenticated with a secret generated at configure time. Session cookies are `Secure`, `HttpOnly`, `SameSite=Lax`, and pinned to the host they were minted on. +- **Per-app access lists** work because every generated location tells the + service which app the request belongs to, via an `X-Google-Auth-App` header + that nginx sets itself — overwriting anything a client sent. The plugin's + `apps/` directory is bind-mounted into the container read-only, so the + service reads an app's lists on demand instead of needing a restart. The + destination app also rides through the OAuth `state` parameter, because + Google's callback lands on the auth host, which may belong to a different + app than the one being signed in to. ### Headers your apps receive @@ -88,8 +96,12 @@ it into `/var/lib/dokku/plugins/available/` and: the app's current upstream (whose name changes if you remap ports). - `post-delete`, `post-app-rename`, `post-app-clone` — keep plugin state in sync with app lifecycle. -- **State** lives under `/var/lib/dokku/data/google-auth/` (global config + - one directory per enabled app). Secrets are `0600`. +- **State** lives under `/var/lib/dokku/data/google-auth/`: `global/` for the + shared config and `apps//` for each app's exclusions and access lists. + The data root is `0700` and secrets are `0600`. `apps/` is `0711` with + `0644` list files, because it is bind-mounted into the service container, + which runs as an unprivileged uid and has to read those lists — the `0700` + root still keeps other host users out, and `global/` is never mounted. The nginx integration relies on a stable, documented dokku feature: the generated vhost for every app contains @@ -161,51 +173,78 @@ Other flags (all optional, all persisted): | `--port

` | host port (127.0.0.1 only) for the auth service | `2999` | | `--regenerate-cookie-secret` | rotate the session encryption key (signs everyone out) | — | -These flags **replace** the list they name, which suits initial setup. For -one-at-a-time changes afterwards, see the next section. +These flags set the **global** lists and **replace** the list they name, which +suits initial setup. For one-at-a-time changes afterwards, see the next +section. ### 3. Decide who is allowed in -`--allow-domain` and `--allow-email` together form the **allowlist**, and at -least one entry is required. Any account matching neither is rejected — there -is no "allow everyone" mode. So to limit access to a specific list of people, -use only addresses and no domain: +Access lists exist in two scopes — one global default and an optional override +per app. Every command takes the scope as its first argument: an app name, or +`--global`. ```bash -dokku google-auth:allow greyson@signal.org alice@signal.org +dokku google-auth:allow --global signal.org # everyone at signal.org, by default +dokku google-auth:allow my-app ceo@signal.org # …but my-app is just this person +dokku google-auth:deny --global former@signal.org # nobody, anywhere +dokku google-auth:deny my-app bob@signal.org # bob, only on my-app ``` -The **deny list** is checked first and wins over both allow rules, which is -how you cut off one person without narrowing the whole domain: +The rules, in the order the auth service applies them: + +1. **A deny match rejects the account.** The global and per-app deny lists are + *combined*, so a global denial cannot be lifted by an app. +2. **Otherwise the allow list decides**, and it is strict: an account matching + nothing is rejected. There is no "allow everyone" mode, and at least one + global allow entry is required. +3. **An app with its own allow entries uses only those**, ignoring the global + list entirely. An app with none inherits the global list. + +That third rule is the useful one and the surprising one. It lets a single app +be narrowed to a few people, or opened to an outside collaborator who is not +in the global list at all: + +| | global: `signal.org` | effect | +|---|---|---| +| `app-a` | no entries | anyone `@signal.org` | +| `app-b` | `ceo@signal.org` | **only** `ceo@signal.org` | +| `app-c` | `guest@partner.com` | **only** `guest@partner.com` — not `@signal.org` | + +It also means the global list is a default, not a ceiling: an app can admit +someone it does not cover. `google-auth:allow` warns the first time an app +gains an entry, since that is the moment it stops inheriting. + +All eight forms: ```bash -dokku google-auth:allow signal.org # everyone at signal.org… -dokku google-auth:deny former@signal.org # …except this account -``` +dokku google-auth:allow --global # show the global allow list +dokku google-auth:allow my-app # show my-app's (or that it inherits) +dokku google-auth:allow --global signal.org # a domain — any verified account there +dokku google-auth:allow my-app guest@partner.com # one address +dokku google-auth:unallow my-app guest@partner.com # remove either kind -These four commands each change one entry at a time and restart the auth -service for you: - -```bash -dokku google-auth:allow # show the allow list -dokku google-auth:allow signal.org # a domain (any verified account there) -dokku google-auth:allow guest@partner.com # one address -dokku google-auth:unallow guest@partner.com # remove either kind - -dokku google-auth:deny # show the deny list -dokku google-auth:deny former@signal.org -dokku google-auth:undeny former@signal.org +dokku google-auth:deny --global # show the global deny list +dokku google-auth:deny my-app # show my-app's, plus the global ones +dokku google-auth:undeny --global former@signal.org ``` Changes take effect on the affected user's **next request**: session cookies are re-checked against the current lists rather than trusted until they -expire, so denying (or unallowing) someone with a live session ends it. To -sign out everyone at once instead, use +expire, so denying (or unallowing) someone with a live session ends it — and a +session minted for one app is not accepted by an app whose list excludes them. +To sign out everyone at once instead, use `configure --regenerate-cookie-secret`. -`unallow` refuses to remove the last allow entry, since an empty allowlist -locks everyone out of every enabled app. `dokku google-auth:report` shows -both lists as they currently stand. +Two guardrails: `unallow --global` refuses to remove the last global entry, +since an empty global allow list locks everyone out of every app that inherits +it (emptying an *app's* list is fine — it goes back to inheriting). +`dokku google-auth:report` shows the global lists and each app's, and states +whether an app inherits or overrides. + +Global lists reach the auth service in its environment, so changing one +restarts the shared container. Per-app lists are read from disk on demand, so +changing one takes effect within a couple of seconds with no restart and no +interruption to other apps. ### 4. Protect apps @@ -240,8 +279,9 @@ protect them yourself (API key, HMAC signature, etc.). ```bash dokku google-auth:report # global + per-app status, incl. both access lists dokku google-auth:report my-app # one app -dokku google-auth:allow alice@signal.org # let someone in -dokku google-auth:deny former@signal.org # cut someone off +dokku google-auth:allow --global alice@signal.org # let someone in everywhere +dokku google-auth:allow my-app alice@signal.org # …or just on one app +dokku google-auth:deny --global former@signal.org # cut someone off dokku google-auth:disable my-app # turn SSO off for an app dokku google-auth:logs -t # follow auth service logs (sign-ins, denials) dokku google-auth:restart # restart the auth service @@ -274,6 +314,11 @@ and sign out at `https:///_google-auth/logout`. stock nginx template. - **Plugin updates:** `sudo dokku plugin:update google-auth` rebuilds the service image and restarts the container automatically. +- **An app's own lists need its nginx config to be current**, since that is + what tells the service which app a request belongs to. The plugin rewrites + the config whenever you change an app's lists (and on every deploy), so this + is normally invisible — but an app that has not been deployed or touched + since an upgrade falls back to the global lists until then. ## Uninstall @@ -293,7 +338,9 @@ shellcheck): ```bash mise install # install pinned toolchain mise run test # Go unit tests (full OAuth flow against a fake Google) -mise run test-nginx-conf # generate nginx config + validate with real nginx in docker +mise run test-nginx-conf # generate nginx config + validate with real nginx in docker +mise run test-access-lists # allow/deny commands against a fake dokku layout +mise run test-help # help output shapes + every subcommand documented mise run shellcheck # lint all plugin scripts mise run check # everything CI would run mise run docker-build # build the service image locally @@ -307,6 +354,6 @@ internal/authproxy/ OAuth flow, session crypto, HTTP handlers (+ tests) functions shared bash for the plugin (config store, nginx generation) subcommands/ dokku google-auth:* commands install, nginx-pre-reload, core-post-deploy, post-* dokku lifecycle triggers -test/nginx-conf-test.sh bash integration test for the generated nginx config +test/ bash tests: nginx config generation, access lists, help output Dockerfile multi-stage build → static binary in a scratch image ``` diff --git a/commands b/commands index 18492b4..879c001 100755 --- a/commands +++ b/commands @@ -7,10 +7,10 @@ case "$1" in help_content() { cat <...], Let a domain or address sign in (no arguments lists the allow list) - google-auth:unallow , Remove a domain or address from the allow list - google-auth:deny [

...], Block an address even if the allow list covers it (no arguments lists the deny list) - google-auth:undeny , Remove an address from the deny list + google-auth:allow |--global [...], Let a domain or address sign in (no entries lists the allow list) + google-auth:unallow |--global , Remove a domain or address from an allow list + google-auth:deny |--global [
...], Block an address even if the allow list covers it (no entries lists the deny list) + google-auth:undeny |--global , Remove an address from a deny list google-auth:enable , Require Google sign-in for all requests to google-auth:disable , Remove Google sign-in from google-auth:exclude , Exempt path prefixes (/path) or regexes (re:^/x) from sign-in @@ -45,14 +45,24 @@ help_content echo ' --port 127.0.0.1 port for the auth service (default 2999)' echo ' --regenerate-cookie-secret rotate the session key (signs everyone out)' echo '' - echo 'The allow rules are a strict allowlist: an account that matches none of them' - echo 'is rejected, and at least one rule is required. The deny list wins over both.' + echo 'These flags set the GLOBAL lists and REPLACE the list they name, so they suit' + echo 'initial setup. To change one entry afterwards — globally or for a single app —' + echo 'use google-auth:allow / :unallow / :deny / :undeny.' echo '' - echo 'These flags REPLACE the list they name, so they suit initial setup. To add or' - echo 'remove one person afterwards, use google-auth:allow / :unallow / :deny /' - echo ':undeny, which change one entry at a time and restart the service for you.' + echo 'Who gets in:' + echo '' + echo ' * The allow rules are a strict allowlist: an account matching none of them' + echo ' is rejected, and at least one global rule is required.' + echo ' * An app with its own allow entries uses ONLY those, ignoring the global' + echo ' list. An app with none inherits the global list.' + echo ' * Deny lists combine: an address denied globally or for the app is' + echo ' rejected, and no app can lift a global denial.' else - help_content + # Plain `dokku help` gets a single summary line, the way dokku's own + # plugins behave; the command list belongs to `dokku google-auth:help`. + cat </dev/null 2>&1; then @@ -73,19 +75,57 @@ fn-ga-global-set-list() { chmod 600 "$file" } -# Additive counterparts to fn-ga-global-set-list, for the allow/deny commands. -fn-ga-global-list-add() { - declare KEY="$1" VALUE="$2" - mkdir -p "$GOOGLE_AUTH_DATA_ROOT/global" - local file="$GOOGLE_AUTH_DATA_ROOT/global/$KEY" - touch "$file" - chmod 600 "$file" +# --- access lists, scoped to "global" or to one app --- +# +# An app's allow entries replace the global ones for that app; deny entries from +# both scopes are combined. The auth service implements that precedence — these +# helpers only store the entries. + +fn-ga-list-file() { + declare SCOPE="$1" KEY="$2" + if [[ "$SCOPE" == "global" ]]; then + echo "$GOOGLE_AUTH_DATA_ROOT/global/$KEY" + else + echo "$(fn-ga-app-dir "$SCOPE")/$KEY" + fi +} + +fn-ga-list-get() { + declare SCOPE="$1" KEY="$2" + cat "$(fn-ga-list-file "$SCOPE" "$KEY")" 2>/dev/null || true +} + +fn-ga-list-count() { + declare SCOPE="$1" KEY="$2" + fn-ga-list-get "$SCOPE" "$KEY" | grep -c . || true +} + +fn-ga-list-contains() { + declare SCOPE="$1" KEY="$2" VALUE="$3" + grep -qxF "$VALUE" "$(fn-ga-list-file "$SCOPE" "$KEY")" 2>/dev/null +} + +fn-ga-list-add() { + declare SCOPE="$1" KEY="$2" VALUE="$3" + local file + file="$(fn-ga-list-file "$SCOPE" "$KEY")" + if [[ "$SCOPE" == "global" ]]; then + mkdir -p "$GOOGLE_AUTH_DATA_ROOT/global" + touch "$file" + chmod 600 "$file" + else + fn-ga-app-dir-ensure "$SCOPE" >/dev/null + touch "$file" + # Readable through the service's read-only bind mount; see fn-ga-app-dir-ensure. + chmod 644 "$file" + fi grep -qxF "$VALUE" "$file" || printf '%s\n' "$VALUE" >>"$file" } -fn-ga-global-list-remove() { - declare KEY="$1" VALUE="$2" - local file="$GOOGLE_AUTH_DATA_ROOT/global/$KEY" tmp +fn-ga-list-remove() { + declare SCOPE="$1" KEY="$2" VALUE="$3" + local file tmp + file="$(fn-ga-list-file "$SCOPE" "$KEY")" [[ -f "$file" ]] || return 0 tmp="$(mktemp)" grep -vxF "$VALUE" "$file" >"$tmp" || true @@ -93,14 +133,88 @@ fn-ga-global-list-remove() { rm -f "$tmp" } -fn-ga-global-list-contains() { - declare KEY="$1" VALUE="$2" - grep -qxF "$VALUE" "$GOOGLE_AUTH_DATA_ROOT/global/$KEY" 2>/dev/null +# Sets GA_SCOPE from a command's first argument: "global" for --global, +# otherwise a verified app name. Fails (and exits) on anything else. +fn-ga-resolve-scope() { + declare CMD="$1" ARG="${2:-}" + case "$ARG" in + --global) + GA_SCOPE=global + ;; + "") + dokku_log_fail "usage: dokku $CMD |--global [...]" + ;; + -*) + dokku_log_fail "unknown flag '$ARG' — pass an app name or --global" + ;; + *) + verify_app_name "$ARG" + GA_SCOPE="$ARG" + ;; + esac } -fn-ga-global-list-count() { - declare KEY="$1" - fn-ga-global-get-list "$KEY" | grep -c . || true +# Human-readable scope for log lines: "globally" or "for my-app". +fn-ga-scope-label() { + declare SCOPE="$1" + if [[ "$SCOPE" == "global" ]]; then + echo "globally" + else + echo "for $SCOPE" + fi +} + +# The scope as it is typed on the command line, for suggested commands. +fn-ga-scope-arg() { + declare SCOPE="$1" + if [[ "$SCOPE" == "global" ]]; then + echo "--global" + else + echo "$SCOPE" + fi +} + +# Advisory mirror of the service's allow-list precedence (an app's entries +# replace the global ones), used only to warn operators. The authority is +# emailAllowedFor in internal/authproxy. +fn-ga-email-effectively-allowed() { + declare SCOPE="$1" EMAIL="$2" + local effective="$SCOPE" + if [[ "$SCOPE" != "global" ]] && + [[ "$(fn-ga-list-count "$SCOPE" allowed-domains)" -eq 0 && + "$(fn-ga-list-count "$SCOPE" allowed-emails)" -eq 0 ]]; then + effective=global + fi + fn-ga-list-contains "$effective" allowed-emails "$EMAIL" && return 0 + fn-ga-list-contains "$effective" allowed-domains "${EMAIL##*@}" +} + +# Makes a list change take effect. Global lists travel in the container's +# environment and need a restart; per-app lists are read live through the bind +# mount, so they only need the mount to actually be there. +fn-ga-apply-list-change() { + declare SCOPE="$1" + if [[ "$SCOPE" == "global" ]]; then + fn-ga-reload-service-config + return 0 + fi + # An app's lists only apply if its nginx config tells the service which app a + # request belongs to. Configs written before that header existed would make + # the app fall back to the global lists, so refresh it here rather than wait + # for the next deploy. This is a no-op when the config is already current. + if fn-google-auth-app-enabled "$SCOPE"; then + fn-ga-apply "$SCOPE" + fi + if ! fn-ga-service-running; then + dokku_log_verbose "auth service is not running; changes apply when it starts" + return 0 + fi + if ! fn-ga-service-has-app-mount; then + dokku_log_info1 "recreating the auth service so it can read per-app lists" + fn-ga-service-start + return 0 + fi + dokku_log_verbose "in effect within a few seconds (no restart needed)" } # Entries end up in a comma/space separated env var, so they may contain @@ -137,6 +251,20 @@ fn-ga-app-dir() { echo "$GOOGLE_AUTH_DATA_ROOT/apps/$APP" } +# Creates an app's state directory with modes the auth service can use. It reads +# per-app lists through a read-only bind mount as an unprivileged uid, so the +# directories must be traversable and the list files readable. Nothing is +# exposed to other users on the host: $GOOGLE_AUTH_DATA_ROOT itself stays 0700, +# and secrets live in global/, which is never mounted. +fn-ga-app-dir-ensure() { + declare APP="$1" + local dir + dir="$(fn-ga-app-dir "$APP")" + mkdir -p "$dir" + chmod 711 "$GOOGLE_AUTH_DATA_ROOT/apps" "$dir" 2>/dev/null || true + printf '%s' "$dir" +} + fn-google-auth-app-enabled() { declare APP="$1" [[ -f "$(fn-ga-app-dir "$APP")/enabled" ]] @@ -272,6 +400,9 @@ fn-ga-generate-conf() { # Managed by the dokku google-auth plugin — do not edit by hand. # Regenerated on every deploy and by google-auth:* commands. +# Every location below sets X-Google-Auth-App explicitly, which both tells the +# auth service whose access lists to apply and overwrites any value a client +# tried to send. location = ${GOOGLE_AUTH_ROUTE_PREFIX}/verify { internal; proxy_pass http://127.0.0.1:${port}; @@ -280,6 +411,7 @@ location = ${GOOGLE_AUTH_ROUTE_PREFIX}/verify { proxy_set_header Host \$host; proxy_set_header X-Forwarded-Proto \$scheme; proxy_set_header X-Forwarded-For \$remote_addr; + proxy_set_header X-Google-Auth-App "${APP}"; } location ^~ ${GOOGLE_AUTH_ROUTE_PREFIX}/ { @@ -290,6 +422,7 @@ location ^~ ${GOOGLE_AUTH_ROUTE_PREFIX}/ { proxy_set_header X-Forwarded-For \$remote_addr; proxy_set_header X-Forwarded-Port \$server_port; proxy_set_header X-Auth-Request-Redirect ""; + proxy_set_header X-Google-Auth-App "${APP}"; } location @google_auth_signin { @@ -299,6 +432,7 @@ location @google_auth_signin { proxy_set_header X-Forwarded-Proto \$scheme; proxy_set_header X-Forwarded-For \$remote_addr; proxy_set_header X-Auth-Request-Redirect \$request_uri; + proxy_set_header X-Google-Auth-App "${APP}"; } EOF @@ -321,6 +455,7 @@ EOF proxy_set_header X-Auth-Request-User ""; proxy_set_header X-Auth-Request-Email ""; proxy_set_header X-Auth-Request-Name ""; + proxy_set_header X-Google-Auth-App ""; } EOF done < <(fn-ga-excludes "$APP") @@ -341,6 +476,7 @@ $(fn-ga-proxy-directives "$upstream" "$timeout") proxy_set_header X-Auth-Request-User \$google_auth_user; proxy_set_header X-Auth-Request-Email \$google_auth_email; proxy_set_header X-Auth-Request-Name \$google_auth_name; + proxy_set_header X-Google-Auth-App ""; } EOF } @@ -479,6 +615,14 @@ fn-ga-service-running() { [[ "$(docker container inspect -f '{{.State.Running}}' "$GOOGLE_AUTH_SERVICE_NAME" 2>/dev/null)" == "true" ]] } +# True when the running container has the per-app config mount. A container +# started by an older version of this plugin will not, and would silently +# ignore per-app lists. +fn-ga-service-has-app-mount() { + docker container inspect -f '{{range .Mounts}}{{println .Destination}}{{end}}' \ + "$GOOGLE_AUTH_SERVICE_NAME" 2>/dev/null | grep -qxF "$GOOGLE_AUTH_APP_CONFIG_MOUNT" +} + fn-ga-write-env-file() { local envfile="$GOOGLE_AUTH_DATA_ROOT/service.env" local domains emails denied @@ -495,6 +639,7 @@ GOOGLE_AUTH_AUTH_HOST=$(fn-ga-global-get auth-host) GOOGLE_AUTH_ALLOWED_DOMAINS=$domains GOOGLE_AUTH_ALLOWED_EMAILS=$emails GOOGLE_AUTH_DENIED_EMAILS=$denied +GOOGLE_AUTH_APP_CONFIG_DIR=$GOOGLE_AUTH_APP_CONFIG_MOUNT GOOGLE_AUTH_COOKIE_NAME=$(fn-ga-global-get cookie-name _google_auth) GOOGLE_AUTH_SESSION_TTL=$(fn-ga-global-get session-ttl 24h) GOOGLE_AUTH_ALLOW_INSECURE=$(fn-ga-global-get allow-insecure false) @@ -508,12 +653,15 @@ fn-ga-service-start() { fn-ga-write-env-file local port port="$(fn-ga-global-get port "$GOOGLE_AUTH_DEFAULT_PORT")" + mkdir -p "$GOOGLE_AUTH_DATA_ROOT/apps" + chmod 711 "$GOOGLE_AUTH_DATA_ROOT/apps" 2>/dev/null || true docker container rm -f "$GOOGLE_AUTH_SERVICE_NAME" >/dev/null 2>&1 || true docker container run -d \ --name "$GOOGLE_AUTH_SERVICE_NAME" \ --restart=unless-stopped \ -p "127.0.0.1:${port}:2999" \ --env-file "$GOOGLE_AUTH_DATA_ROOT/service.env" \ + -v "$GOOGLE_AUTH_DATA_ROOT/apps:${GOOGLE_AUTH_APP_CONFIG_MOUNT}:ro" \ "$GOOGLE_AUTH_IMAGE" >/dev/null dokku_log_info1 "google-auth service running on 127.0.0.1:${port}" } diff --git a/install b/install index 4fbd5ca..3141f2c 100755 --- a/install +++ b/install @@ -10,6 +10,10 @@ DATA_ROOT="$DOKKU_LIB_ROOT/data/google-auth" mkdir -p "$DATA_ROOT/global" "$DATA_ROOT/apps" chown -R dokku:dokku "$DATA_ROOT" 2>/dev/null || true chmod 700 "$DATA_ROOT" +# apps/ is bind-mounted into the service container, which runs as an +# unprivileged uid and must be able to traverse it to read per-app access +# lists. The 0700 on DATA_ROOT still keeps other host users out. +chmod 711 "$DATA_ROOT/apps" if command -v docker >/dev/null 2>&1; then echo "-----> Building dokku-google-auth service image (first build can take a few minutes)" @@ -25,6 +29,7 @@ if command -v docker >/dev/null 2>&1; then --restart=unless-stopped \ -p "127.0.0.1:${port}:2999" \ --env-file "$DATA_ROOT/service.env" \ + -v "$DATA_ROOT/apps:/data/apps:ro" \ dokku-google-auth:latest >/dev/null fi else diff --git a/internal/authproxy/appconfig.go b/internal/authproxy/appconfig.go new file mode 100644 index 0000000..c2d4bfa --- /dev/null +++ b/internal/authproxy/appconfig.go @@ -0,0 +1,114 @@ +package authproxy + +import ( + "os" + "path/filepath" + "strings" + "sync" + "time" +) + +// accessLists is one scope's worth of authorization rules — either the global +// config or a single app's overrides. +type accessLists struct { + AllowedDomains []string + AllowedEmails []string + DeniedEmails []string +} + +func (a accessLists) hasAllowRules() bool { + return len(a.AllowedDomains) > 0 || len(a.AllowedEmails) > 0 +} + +// appStore reads per-app allow/deny lists from a directory the dokku plugin +// bind-mounts read-only (one subdirectory per app, one file per list). Reading +// from disk rather than the environment means the plugin can change an app's +// lists without restarting this service and interrupting every other app. +// +// Results are cached for a short TTL because the lists are consulted on every +// authenticated request. +type appStore struct { + dir string + ttl time.Duration + now func() time.Time + + mu sync.Mutex + cache map[string]cachedLists +} + +type cachedLists struct { + lists accessLists + loadedAt time.Time +} + +func newAppStore(dir string) *appStore { + return &appStore{ + dir: dir, + ttl: 2 * time.Second, + now: time.Now, + cache: map[string]cachedLists{}, + } +} + +// lists returns the app's own rules, or a zero value when the app has none, +// the name is unusable, or per-app config is disabled. A zero value means +// "inherit the global config", so every failure path is a safe fallback. +func (s *appStore) lists(app string) accessLists { + if s == nil || s.dir == "" || !validAppName(app) { + return accessLists{} + } + + s.mu.Lock() + defer s.mu.Unlock() + if hit, ok := s.cache[app]; ok && s.now().Sub(hit.loadedAt) < s.ttl { + return hit.lists + } + + lists := accessLists{ + AllowedDomains: readDomainFile(filepath.Join(s.dir, app, "allowed-domains")), + AllowedEmails: readListFile(filepath.Join(s.dir, app, "allowed-emails")), + DeniedEmails: readListFile(filepath.Join(s.dir, app, "denied-emails")), + } + s.cache[app] = cachedLists{lists: lists, loadedAt: s.now()} + return lists +} + +func readListFile(path string) []string { + // A missing or unreadable file is normal: most apps have no overrides. + raw, err := os.ReadFile(path) + if err != nil { + return nil + } + return splitList(string(raw)) +} + +func readDomainFile(path string) []string { + entries := readListFile(path) + for i, d := range entries { + entries[i] = strings.TrimPrefix(d, "@") + } + return entries +} + +// validAppName guards the path built from an nginx-supplied header. dokku app +// names are lowercase alphanumerics with dashes, dots, and underscores; this +// deliberately rejects anything that could escape the config directory. +func validAppName(app string) bool { + if app == "" || len(app) > 100 { + return false + } + if app == "." || app == ".." || strings.HasPrefix(app, ".") { + return false + } + for _, r := range app { + switch { + case r >= 'a' && r <= 'z', + r >= 'A' && r <= 'Z', + r >= '0' && r <= '9', + r == '-', r == '_', r == '.': + default: + return false + } + } + return true +} diff --git a/internal/authproxy/appconfig_test.go b/internal/authproxy/appconfig_test.go new file mode 100644 index 0000000..a032cc6 --- /dev/null +++ b/internal/authproxy/appconfig_test.go @@ -0,0 +1,261 @@ +package authproxy + +import ( + "net/http" + "net/http/httptest" + "net/url" + "os" + "path/filepath" + "testing" + "time" +) + +// writeAppLists lays out one app's config the way the dokku plugin does. +func writeAppLists(t *testing.T, dir, app string, files map[string]string) { + t.Helper() + appDir := filepath.Join(dir, app) + if err := os.MkdirAll(appDir, 0o755); err != nil { + t.Fatal(err) + } + for name, content := range files { + if err := os.WriteFile(filepath.Join(appDir, name), []byte(content), 0o644); err != nil { + t.Fatal(err) + } + } +} + +func TestAppStoreReadsLists(t *testing.T) { + dir := t.TempDir() + writeAppLists(t, dir, "my-app", map[string]string{ + "allowed-domains": "Signal.org\n@example.com\n", + "allowed-emails": "Guest@Partner.com\n", + "denied-emails": "former@signal.org\n", + }) + store := newAppStore(dir) + + got := store.lists("my-app") + if len(got.AllowedDomains) != 2 || got.AllowedDomains[0] != "signal.org" || got.AllowedDomains[1] != "example.com" { + t.Errorf("AllowedDomains = %v", got.AllowedDomains) + } + if len(got.AllowedEmails) != 1 || got.AllowedEmails[0] != "guest@partner.com" { + t.Errorf("AllowedEmails = %v", got.AllowedEmails) + } + if len(got.DeniedEmails) != 1 || got.DeniedEmails[0] != "former@signal.org" { + t.Errorf("DeniedEmails = %v", got.DeniedEmails) + } + + // An app with no directory, and a disabled store, both mean "inherit". + if store.lists("other-app").hasAllowRules() { + t.Error("an app with no config should have no allow rules") + } + if newAppStore("").lists("my-app").hasAllowRules() { + t.Error("an empty config dir should disable per-app lists") + } +} + +// The app name reaches the service in a header, so it must never be able to +// walk out of the config directory. +func TestAppStoreRejectsUnusableNames(t *testing.T) { + dir := t.TempDir() + writeAppLists(t, dir, "escaped", map[string]string{"allowed-emails": "attacker@evil.com\n"}) + store := newAppStore(dir) + + for _, name := range []string{ + "", ".", "..", "../escaped", "..%2fescaped", "/escaped", "sub/escaped", + ".hidden", "app name", "app;rm", "app\x00", "app\n", + } { + if got := store.lists(name); got.hasAllowRules() || len(got.DeniedEmails) > 0 { + t.Errorf("lists(%q) returned rules; want empty", name) + } + if validAppName(name) { + t.Errorf("validAppName(%q) = true, want false", name) + } + } + for _, name := range []string{"my-app", "my_app", "app.example", "App2"} { + if !validAppName(name) { + t.Errorf("validAppName(%q) = false, want true", name) + } + } +} + +func TestAppStoreCachesUntilTTL(t *testing.T) { + dir := t.TempDir() + writeAppLists(t, dir, "my-app", map[string]string{"allowed-emails": "first@signal.org\n"}) + + now := time.Unix(1700000000, 0) + store := newAppStore(dir) + store.now = func() time.Time { return now } + + if got := store.lists("my-app").AllowedEmails[0]; got != "first@signal.org" { + t.Fatalf("first read = %q", got) + } + writeAppLists(t, dir, "my-app", map[string]string{"allowed-emails": "second@signal.org\n"}) + if got := store.lists("my-app").AllowedEmails[0]; got != "first@signal.org" { + t.Errorf("within the TTL the cached value should stand, got %q", got) + } + now = now.Add(store.ttl + time.Second) + if got := store.lists("my-app").AllowedEmails[0]; got != "second@signal.org" { + t.Errorf("after the TTL the change should be picked up, got %q", got) + } +} + +// newPerAppServer is a server whose global list allows all of signal.org, plus +// per-app overrides on disk. +func newPerAppServer(t *testing.T, apps map[string]map[string]string) *Server { + t.Helper() + dir := t.TempDir() + for app, files := range apps { + writeAppLists(t, dir, app, files) + } + s := newTestServer(t, "http://unused.invalid") + s.cfg.AppConfigDir = dir + s.apps = newAppStore(dir) + return s +} + +func TestPerAppAllowReplacesGlobal(t *testing.T) { + s := newPerAppServer(t, map[string]map[string]string{ + "narrowed": {"allowed-emails": "ceo@signal.org\n"}, + "widened": {"allowed-emails": "guest@partner.com\n"}, + }) + + cases := []struct { + app, email string + want bool + }{ + // No per-app config: the global list applies. + {"inherits", "anyone@signal.org", true}, + {"inherits", "guest@partner.com", false}, + {"", "anyone@signal.org", true}, + // Own allow list: it replaces the global one entirely. + {"narrowed", "ceo@signal.org", true}, + {"narrowed", "anyone@signal.org", false}, + // An app may admit someone the global list does not cover. + {"widened", "guest@partner.com", true}, + {"widened", "anyone@signal.org", false}, + // An unknown app name falls back to global rather than failing open. + {"no-such-app", "anyone@signal.org", true}, + {"../narrowed", "anyone@signal.org", true}, + } + for _, c := range cases { + if got := s.emailAllowedFor(c.app, c.email); got != c.want { + t.Errorf("emailAllowedFor(%q, %q) = %v, want %v", c.app, c.email, got, c.want) + } + } +} + +func TestPerAppDenyCombinesWithGlobal(t *testing.T) { + s := newPerAppServer(t, map[string]map[string]string{ + "app-a": {"denied-emails": "bob@signal.org\n"}, + // An app's own deny list must not resurrect a globally denied account. + "app-b": {"denied-emails": "someone@signal.org\n"}, + // Nor may an app's allow list override a global denial. + "app-c": {"allowed-emails": "gone@signal.org\n"}, + }) + s.cfg.DeniedEmails = []string{"gone@signal.org"} + + cases := []struct { + app, email string + want bool + }{ + {"app-a", "bob@signal.org", false}, // denied for this app + {"app-b", "bob@signal.org", true}, // ...but not for another + {"app-a", "gone@signal.org", false}, // global denial still applies + {"app-b", "gone@signal.org", false}, // even with its own deny list + {"app-c", "gone@signal.org", false}, // even when the app allows them + {"inherits", "gone@signal.org", false}, + } + for _, c := range cases { + if got := s.emailAllowedFor(c.app, c.email); got != c.want { + t.Errorf("emailAllowedFor(%q, %q) = %v, want %v", c.app, c.email, got, c.want) + } + } +} + +// The app name is read from a header, so verify must use the value nginx set +// and a session must not be portable to an app with stricter rules. +func TestVerifyUsesAppHeader(t *testing.T) { + s := newPerAppServer(t, map[string]map[string]string{ + "strict": {"allowed-emails": "ceo@signal.org\n"}, + }) + sess := sessionClaims{Email: "dev@signal.org", User: "1", Host: appHost, + Exp: time.Now().Add(time.Hour).Unix()} + val, err := s.box.seal("session", sess) + if err != nil { + t.Fatal(err) + } + verify := func(app string) int { + r := httptest.NewRequest("GET", "http://"+appHost+RoutePrefix+"/verify", nil) + r.AddCookie(&http.Cookie{Name: "_google_auth", Value: val}) + if app != "" { + r.Header.Set(AppHeader, app) + } + return do(s.Routes(), r).Code + } + + if got := verify("lenient"); got != http.StatusOK { + t.Errorf("app inheriting the global list: got %d, want 200", got) + } + if got := verify("strict"); got != http.StatusUnauthorized { + t.Errorf("app whose list excludes the user: got %d, want 401", got) + } + if got := verify(""); got != http.StatusOK { + t.Errorf("no app header: got %d, want 200 (global rules)", got) + } +} + +// The callback runs on the auth host, which may serve a different app than the +// one being signed in to. Authorization must follow the destination app. +func TestCallbackAuthorizesAgainstDestinationApp(t *testing.T) { + google := fakeGoogle(t, goodClaims()) // signs in greyson@signal.org + defer google.Close() + s := newPerAppServer(t, map[string]map[string]string{ + "strict": {"allowed-emails": "ceo@signal.org\n"}, + }) + s.cfg.TokenURL = google.URL + + callback := func(app string) *httptest.ResponseRecorder { + tok, err := s.box.seal("state", stateClaims{ + Host: appHost, App: app, RD: "/", Proto: "https", Nonce: randToken(), + Exp: time.Now().Add(10 * time.Minute).Unix(), + }) + if err != nil { + t.Fatal(err) + } + r := httptest.NewRequest("GET", + "http://"+authHost+RoutePrefix+"/callback?code=good-code&state="+url.QueryEscape(tok), nil) + // The auth host's own app name, which must not be what decides. + r.Header.Set(AppHeader, "auth-host-app") + return do(s.Routes(), r) + } + + if w := callback("lenient"); w.Code != http.StatusFound { + t.Errorf("destination inheriting the global list: got %d, want 302; body: %s", w.Code, w.Body.String()) + } + if w := callback("strict"); w.Code != http.StatusForbidden { + t.Errorf("destination whose list excludes the user: got %d, want 403", w.Code) + } +} + +// The hd hint should follow the app's effective domain list, not the global one. +func TestStartHintsAppDomain(t *testing.T) { + s := newPerAppServer(t, map[string]map[string]string{ + "partner-app": {"allowed-domains": "partner.com\n"}, + }) + start := func(app string) string { + r := httptest.NewRequest("GET", "http://"+appHost+RoutePrefix+"/start", nil) + r.Header.Set("Accept", "text/html") + r.Header.Set(AppHeader, app) + loc, err := url.Parse(do(s.Routes(), r).Header().Get("Location")) + if err != nil { + t.Fatal(err) + } + return loc.Query().Get("hd") + } + if got := start("inherits"); got != "signal.org" { + t.Errorf("hd for an inheriting app = %q, want signal.org", got) + } + if got := start("partner-app"); got != "partner.com" { + t.Errorf("hd for an app with its own domain = %q, want partner.com", got) + } +} diff --git a/internal/authproxy/claims.go b/internal/authproxy/claims.go index 015e1b4..7af418c 100644 --- a/internal/authproxy/claims.go +++ b/internal/authproxy/claims.go @@ -4,7 +4,12 @@ package authproxy // It remembers which app host the user was visiting and where to send them // after sign-in. type stateClaims struct { - Host string `json:"h"` // app host the user was visiting + Host string `json:"h"` // app host the user was visiting + // App is the dokku app that host belongs to. It rides through the flow + // because the callback runs on the auth host, which may be a different + // app than the one being signed in to — and it is that destination app's + // access lists that decide. + App string `json:"a"` RD string `json:"r"` // relative path to return to Proto string `json:"p"` // http or https Nonce string `json:"n"` diff --git a/internal/authproxy/config.go b/internal/authproxy/config.go index 158f18e..934cd9c 100644 --- a/internal/authproxy/config.go +++ b/internal/authproxy/config.go @@ -14,6 +14,11 @@ const ( // RoutePrefix is the URL namespace the proxy owns on every protected // host. It must match the locations the plugin writes into nginx. RoutePrefix = "/_google-auth" + + // AppHeader carries the dokku app name a request belongs to. Every nginx + // location that reaches this service sets it explicitly, which also + // overwrites anything a client tried to send. + AppHeader = "X-Google-Auth-App" ) // Config holds everything the auth service needs. It is normally populated @@ -44,6 +49,12 @@ type Config struct { // whole domain it belongs to. DeniedEmails []string + // AppConfigDir holds one subdirectory per app with that app's own + // allow/deny lists, bind-mounted read-only by the plugin. An app's allow + // rules replace the global ones; deny lists are combined. Empty disables + // per-app config, leaving the global lists in charge. + AppConfigDir string + CookieName string SessionTTL time.Duration ListenAddr string @@ -68,6 +79,7 @@ func ConfigFromEnv() (Config, error) { AuthHost: normalizeHost(os.Getenv("GOOGLE_AUTH_AUTH_HOST")), CookieName: envOr("GOOGLE_AUTH_COOKIE_NAME", "_google_auth"), ListenAddr: envOr("GOOGLE_AUTH_LISTEN", ":2999"), + AppConfigDir: os.Getenv("GOOGLE_AUTH_APP_CONFIG_DIR"), AuthorizeURL: envOr("GOOGLE_AUTH_AUTHORIZE_URL", googleAuthorizeURL), TokenURL: envOr("GOOGLE_AUTH_TOKEN_URL", googleTokenURL), AllowInsecure: os.Getenv("GOOGLE_AUTH_ALLOW_INSECURE") == "true", diff --git a/internal/authproxy/flow_test.go b/internal/authproxy/flow_test.go index 7a7259e..59d7fa6 100644 --- a/internal/authproxy/flow_test.go +++ b/internal/authproxy/flow_test.go @@ -354,8 +354,8 @@ func TestEmailAllowed(t *testing.T) { "signal.org": false, } for email, want := range cases { - if got := s.emailAllowed(email); got != want { - t.Errorf("emailAllowed(%q) = %v, want %v", email, got, want) + if got := s.emailAllowedFor("", email); got != want { + t.Errorf("emailAllowedFor(%q) = %v, want %v", email, got, want) } } } @@ -373,8 +373,8 @@ func TestEmailDenied(t *testing.T) { "guest@partner.com": false, } for email, want := range cases { - if got := s.emailAllowed(email); got != want { - t.Errorf("emailAllowed(%q) = %v, want %v", email, got, want) + if got := s.emailAllowedFor("", email); got != want { + t.Errorf("emailAllowedFor(%q) = %v, want %v", email, got, want) } } } diff --git a/internal/authproxy/server.go b/internal/authproxy/server.go index c20c2ba..b37d58c 100644 --- a/internal/authproxy/server.go +++ b/internal/authproxy/server.go @@ -25,6 +25,7 @@ type Server struct { cfg Config box *box nonces *nonceCache + apps *appStore client *http.Client } @@ -37,6 +38,7 @@ func New(cfg Config) (*Server, error) { cfg: cfg, box: b, nonces: newNonceCache(), + apps: newAppStore(cfg.AppConfigDir), client: &http.Client{Timeout: 15 * time.Second}, }, nil } @@ -96,8 +98,10 @@ func (s *Server) handleStart(w http.ResponseWriter, r *http.Request) { return } + app := appFromRequest(r) st := stateClaims{ Host: host, + App: app, RD: rd, Proto: s.proto(r), Nonce: randToken(), @@ -115,9 +119,9 @@ func (s *Server) handleStart(w http.ResponseWriter, r *http.Request) { q.Set("response_type", "code") q.Set("scope", "openid email profile") q.Set("state", token) - if len(s.cfg.AllowedDomains) == 1 { - // UX hint only; real enforcement happens in emailAllowed. - q.Set("hd", s.cfg.AllowedDomains[0]) + if domains := s.effectiveLists(app).AllowedDomains; len(domains) == 1 { + // UX hint only; real enforcement happens in emailAllowedFor. + q.Set("hd", domains[0]) } http.Redirect(w, r, s.cfg.AuthorizeURL+"?"+q.Encode(), http.StatusFound) } @@ -157,9 +161,11 @@ func (s *Server) handleCallback(w http.ResponseWriter, r *http.Request) { s.htmlError(w, http.StatusForbidden, "Google returned an invalid identity token.") return } + // Authorize against the app the user is signing in to, not the app serving + // this callback — those differ whenever the auth host belongs elsewhere. email := strings.ToLower(idTok.Email) - if !s.emailAllowed(email) { - log.Printf("callback: denied %s (not in allowed domains/emails) for host %s", email, st.Host) + if !s.emailAllowedFor(st.App, email) { + log.Printf("callback: denied %s for app %q on host %s (not allowed)", email, st.App, st.Host) s.htmlError(w, http.StatusForbidden, "You are signed in to Google as "+html.EscapeString(email)+", but that account is not allowed to access this app.") return @@ -288,28 +294,64 @@ func (s *Server) sessionFromRequest(r *http.Request) (*sessionClaims, bool) { } // Re-check authorization on every request so allow/deny list changes take // effect immediately instead of whenever existing sessions happen to - // expire. - if !s.emailAllowed(sess.Email) { + // expire. This is also what keeps a session minted for one app from being + // accepted by an app with stricter lists. + if !s.emailAllowedFor(appFromRequest(r), sess.Email) { return nil, false } return &sess, true } -// emailAllowed applies the denylist first (it always wins), then the -// allowlist: an address must match an allowed email or an allowed domain. -func (s *Server) emailAllowed(email string) bool { +// effectiveLists resolves the rules that apply to one app: the app's own allow +// rules replace the global ones when it has any (so an app can be narrowed to a +// few people, or opened to an outside collaborator, independently of the +// global list), while deny lists are combined so a global denial can never be +// undone by an app's config. +func (s *Server) effectiveLists(app string) accessLists { + own := s.apps.lists(app) + out := accessLists{ + AllowedDomains: s.cfg.AllowedDomains, + AllowedEmails: s.cfg.AllowedEmails, + DeniedEmails: s.cfg.DeniedEmails, + } + if own.hasAllowRules() { + out.AllowedDomains = own.AllowedDomains + out.AllowedEmails = own.AllowedEmails + } + if len(own.DeniedEmails) > 0 { + out.DeniedEmails = append(append([]string{}, out.DeniedEmails...), own.DeniedEmails...) + } + return out +} + +// emailAllowedFor applies app's rules: the deny list first (it always wins), +// then the allow list, where an address must match an allowed email or an +// allowed domain. An empty app means "global rules only". +func (s *Server) emailAllowedFor(app, email string) bool { email = strings.ToLower(email) - if slices.Contains(s.cfg.DeniedEmails, email) { + lists := s.effectiveLists(app) + if slices.Contains(lists.DeniedEmails, email) { return false } - if slices.Contains(s.cfg.AllowedEmails, email) { + if slices.Contains(lists.AllowedEmails, email) { return true } at := strings.LastIndex(email, "@") if at < 0 { return false } - return slices.Contains(s.cfg.AllowedDomains, email[at+1:]) + return slices.Contains(lists.AllowedDomains, email[at+1:]) +} + +// appFromRequest reads the app name nginx stamps on every request that reaches +// this service. Clients cannot influence it: each generated location sets the +// header explicitly, replacing whatever arrived from outside. +func appFromRequest(r *http.Request) string { + app := strings.TrimSpace(r.Header.Get(AppHeader)) + if !validAppName(app) { + return "" + } + return app } func (s *Server) redirectURI() string { diff --git a/mise.toml b/mise.toml index 4a72d89..4077121 100644 --- a/mise.toml +++ b/mise.toml @@ -34,10 +34,14 @@ run = "test/nginx-conf-test.sh" description = "Exercise the allow/deny list commands against a fake dokku layout" run = "test/access-list-test.sh" +[tasks.test-help] +description = "Check the help output shapes and that every subcommand is documented" +run = "test/help-test.sh" + [tasks.docker-build] description = "Build the auth service docker image" run = "docker build -t dokku-google-auth:latest ." [tasks.check] description = "Run everything CI would run" -depends = ["fmt-check", "vet", "test", "shellcheck", "test-nginx-conf", "test-access-lists"] +depends = ["fmt-check", "vet", "test", "shellcheck", "test-nginx-conf", "test-access-lists", "test-help"] diff --git a/subcommands/allow b/subcommands/allow index 3f73b3b..672576d 100755 --- a/subcommands/allow +++ b/subcommands/allow @@ -4,28 +4,41 @@ set -eo pipefail source "$(dirname "$(dirname "${BASH_SOURCE[0]}")")/functions" cmd-google-auth-allow() { - declare desc="allow a domain or address to sign in" + declare desc="allow a domain or address to sign in, globally or for one app" local cmd="google-auth:allow" [[ "$1" == "$cmd" ]] && shift 1 + fn-ga-resolve-scope "$cmd" "${1:-}" + local scope="$GA_SCOPE" + shift 1 || true + if [[ $# -eq 0 ]]; then - dokku_log_info2 "google-auth allow list" + dokku_log_info2 "google-auth allow list ($(fn-ga-scope-label "$scope"))" local listed found=false while IFS= read -r listed; do [[ -z "$listed" ]] && continue dokku_log_verbose "$listed (domain)" found=true - done < <(fn-ga-global-get-list allowed-domains) + done < <(fn-ga-list-get "$scope" allowed-domains) while IFS= read -r listed; do [[ -z "$listed" ]] && continue dokku_log_verbose "$listed" found=true - done < <(fn-ga-global-get-list allowed-emails) - [[ "$found" == "false" ]] && dokku_log_verbose "(empty — nobody can sign in)" + done < <(fn-ga-list-get "$scope" allowed-emails) + if [[ "$found" == "false" ]]; then + if [[ "$scope" == "global" ]]; then + dokku_log_verbose "(empty — nobody can sign in)" + else + dokku_log_verbose "(none — $scope uses the global allow list)" + fi + fi return 0 fi - local entry + local entry inherited=false + [[ "$scope" != "global" && "$(fn-ga-list-count "$scope" allowed-domains)" -eq 0 && + "$(fn-ga-list-count "$scope" allowed-emails)" -eq 0 ]] && inherited=true + for entry in "$@"; do fn-ga-validate-list-entry "$entry" || dokku_log_fail "invalid entry '$entry' — use a domain (signal.org) or a full address (guest@partner.com), with no spaces or commas" @@ -33,18 +46,25 @@ cmd-google-auth-allow() { if [[ "$(fn-ga-allow-entry-kind "$entry")" == "domain" ]]; then entry="${entry#@}" [[ "$entry" == *.* ]] || dokku_log_fail "'$entry' does not look like a domain" - fn-ga-global-list-add allowed-domains "$entry" - dokku_log_info1 "allowed domain: $entry (any verified account at $entry)" + fn-ga-list-add "$scope" allowed-domains "$entry" + dokku_log_info1 "allowed domain $(fn-ga-scope-label "$scope"): $entry (any verified account at $entry)" else - fn-ga-global-list-add allowed-emails "$entry" - dokku_log_info1 "allowed: $entry" - if fn-ga-global-list-contains denied-emails "$entry"; then - dokku_log_warn "$entry is on the deny list, which wins — run: dokku google-auth:undeny $entry" + fn-ga-list-add "$scope" allowed-emails "$entry" + dokku_log_info1 "allowed $(fn-ga-scope-label "$scope"): $entry" + if fn-ga-list-contains "$scope" denied-emails "$entry" || + fn-ga-list-contains global denied-emails "$entry"; then + dokku_log_warn "$entry is on a deny list, which wins — run: dokku google-auth:undeny $(fn-ga-scope-arg "$scope") $entry" fi fi done - fn-ga-reload-service-config + # The first entry an app gets takes it off the global list entirely, which is + # easy to do by accident. + if [[ "$inherited" == "true" ]]; then + dokku_log_warn "$scope no longer uses the global allow list — only the entries above can sign in to it" + fi + + fn-ga-apply-list-change "$scope" } cmd-google-auth-allow "$@" diff --git a/subcommands/deny b/subcommands/deny index 706b604..f5fac42 100755 --- a/subcommands/deny +++ b/subcommands/deny @@ -8,15 +8,25 @@ cmd-google-auth-deny() { local cmd="google-auth:deny" [[ "$1" == "$cmd" ]] && shift 1 + fn-ga-resolve-scope "$cmd" "${1:-}" + local scope="$GA_SCOPE" + shift 1 || true + if [[ $# -eq 0 ]]; then - dokku_log_info2 "google-auth deny list" + dokku_log_info2 "google-auth deny list ($(fn-ga-scope-label "$scope"))" local listed found=false while IFS= read -r listed; do [[ -z "$listed" ]] && continue dokku_log_verbose "$listed" found=true - done < <(fn-ga-global-get-list denied-emails) + done < <(fn-ga-list-get "$scope" denied-emails) [[ "$found" == "false" ]] && dokku_log_verbose "(empty)" + if [[ "$scope" != "global" ]]; then + while IFS= read -r listed; do + [[ -z "$listed" ]] && continue + dokku_log_verbose "$listed (global)" + done < <(fn-ga-list-get global denied-emails) + fi return 0 fi @@ -26,13 +36,13 @@ cmd-google-auth-deny() { dokku_log_fail "invalid entry '$entry' — use a full address (former@signal.org), with no spaces or commas" entry="${entry,,}" [[ "$entry" == *@* ]] || - dokku_log_fail "google-auth:deny takes a full address, not a domain; to stop allowing all of '$entry', run: dokku google-auth:unallow $entry" - fn-ga-global-list-add denied-emails "$entry" - dokku_log_info1 "denied: $entry ($(fn-ga-global-list-count denied-emails) denied)" + dokku_log_fail "google-auth:deny takes a full address, not a domain; to stop allowing all of '$entry', run: dokku google-auth:unallow $(fn-ga-scope-arg "$scope") $entry" + fn-ga-list-add "$scope" denied-emails "$entry" + dokku_log_info1 "denied $(fn-ga-scope-label "$scope"): $entry" done dokku_log_verbose "denied users lose access on their next request; existing sessions are not honored" - fn-ga-reload-service-config + fn-ga-apply-list-change "$scope" } cmd-google-auth-deny "$@" diff --git a/subcommands/report b/subcommands/report index 0febc08..8e3ac67 100755 --- a/subcommands/report +++ b/subcommands/report @@ -5,10 +5,11 @@ source "$(dirname "$(dirname "${BASH_SOURCE[0]}")")/functions" fn-ga-report-app() { declare APP="$1" + local pattern found dokku_log_info2 "$APP google-auth information" if fn-google-auth-app-enabled "$APP"; then dokku_log_verbose "Enabled: true" - local pattern found=false + found=false while IFS= read -r pattern; do [[ -z "$pattern" ]] && continue if [[ "$found" == "false" ]]; then @@ -19,14 +20,49 @@ fn-ga-report-app() { fi done < <(fn-ga-excludes "$APP") [[ "$found" == "false" ]] && dokku_log_verbose "Excluded: (none)" + else + dokku_log_verbose "Enabled: false" + fi + + # Access lists are worth showing either way: they are often set before an app + # is enabled, and they persist across disable/enable. + local allow_entries=() + while IFS= read -r pattern; do + [[ -n "$pattern" ]] && allow_entries+=("$pattern (domain)") + done < <(fn-ga-list-get "$APP" allowed-domains) + while IFS= read -r pattern; do + [[ -n "$pattern" ]] && allow_entries+=("$pattern") + done < <(fn-ga-list-get "$APP" allowed-emails) + if [[ ${#allow_entries[@]} -eq 0 ]]; then + dokku_log_verbose "Allowed: (inherits the global allow list)" + else + dokku_log_verbose "Allowed: ${allow_entries[0]} (replaces the global allow list)" + local i + for ((i = 1; i < ${#allow_entries[@]}; i++)); do + dokku_log_verbose " ${allow_entries[i]}" + done + fi + + found=false + while IFS= read -r pattern; do + [[ -z "$pattern" ]] && continue + if [[ "$found" == "false" ]]; then + dokku_log_verbose "Denied: $pattern" + found=true + else + dokku_log_verbose " $pattern" + fi + done < <(fn-ga-list-get "$APP" denied-emails) + [[ "$found" == "false" ]] && dokku_log_verbose "Denied: (none beyond the global deny list)" + + if fn-google-auth-app-enabled "$APP"; then if [[ -f "$(fn-ga-conf-path "$APP")" ]]; then dokku_log_verbose "Nginx: $(fn-ga-conf-path "$APP")" else dokku_log_verbose "Nginx: (config pending; will be written on next deploy)" fi - else - dokku_log_verbose "Enabled: false" fi + return 0 } cmd-google-auth-report() { @@ -53,9 +89,9 @@ cmd-google-auth-report() { dokku_log_verbose "Client secret: $([[ -n "$(fn-ga-global-get client-secret)" ]] && echo '(set)' || echo '(unset)')" dokku_log_verbose "Auth host: $(fn-ga-global-get auth-host '(unset)')" dokku_log_verbose "Callback URL: https://$(fn-ga-global-get auth-host '')${GOOGLE_AUTH_ROUTE_PREFIX}/callback" - dokku_log_verbose "Allowed domains: $(fn-ga-global-get-list allowed-domains | paste -sd' ' -)" + dokku_log_verbose "Allowed domains: $(fn-ga-global-get-list allowed-domains | paste -sd' ' -) (global default)" dokku_log_verbose "Allowed emails: $(fn-ga-global-get-list allowed-emails | paste -sd' ' -)" - dokku_log_verbose "Denied emails: $(fn-ga-global-get-list denied-emails | paste -sd' ' -)" + dokku_log_verbose "Denied emails: $(fn-ga-global-get-list denied-emails | paste -sd' ' -) (applies to every app)" dokku_log_verbose "Session TTL: $(fn-ga-global-get session-ttl 24h)" dokku_log_verbose "Service port: 127.0.0.1:$(fn-ga-global-get port "$GOOGLE_AUTH_DEFAULT_PORT")" if fn-ga-service-running; then diff --git a/subcommands/unallow b/subcommands/unallow index d52c24e..18a8b2d 100755 --- a/subcommands/unallow +++ b/subcommands/unallow @@ -4,47 +4,57 @@ set -eo pipefail source "$(dirname "$(dirname "${BASH_SOURCE[0]}")")/functions" cmd-google-auth-unallow() { - declare desc="remove a domain or address from the allow list" + declare desc="remove a domain or address from an allow list" local cmd="google-auth:unallow" [[ "$1" == "$cmd" ]] && shift 1 - [[ $# -gt 0 ]] || dokku_log_fail "usage: dokku google-auth:unallow (see: dokku google-auth:allow)" + fn-ga-resolve-scope "$cmd" "${1:-}" + local scope="$GA_SCOPE" + shift 1 || true + + [[ $# -gt 0 ]] || dokku_log_fail "usage: dokku $cmd |--global " # Lowercase, drop any leading "@", and de-duplicate so the lockout check # below counts each entry once. local entries=() mapfile -t entries < <(printf '%s\n' "$@" | tr '[:upper:]' '[:lower:]' | sed 's/^@//' | awk '!seen[$0]++') - # Refuse before touching anything if this would leave the allow list empty, - # which locks everyone out of every enabled app. local entry total present=0 - total=$(($(fn-ga-global-list-count allowed-domains) + $(fn-ga-global-list-count allowed-emails))) + total=$(($(fn-ga-list-count "$scope" allowed-domains) + $(fn-ga-list-count "$scope" allowed-emails))) for entry in "${entries[@]}"; do - fn-ga-global-list-contains allowed-domains "$entry" && present=$((present + 1)) - fn-ga-global-list-contains allowed-emails "$entry" && present=$((present + 1)) + fn-ga-list-contains "$scope" allowed-domains "$entry" && present=$((present + 1)) + fn-ga-list-contains "$scope" allowed-emails "$entry" && present=$((present + 1)) done - if [[ $((total - present)) -le 0 ]]; then - dokku_log_fail "that would empty the allow list and lock everyone out; add the replacement first with: dokku google-auth:allow " + + # Emptying the global list locks everyone out of every enabled app, so refuse + # before touching anything. Emptying an app's list is fine — it falls back to + # the global one. + if [[ "$scope" == "global" && $((total - present)) -le 0 ]]; then + dokku_log_fail "that would empty the global allow list and lock everyone out; add the replacement first with: dokku google-auth:allow --global " fi local removed for entry in "${entries[@]}"; do removed=false - if fn-ga-global-list-contains allowed-domains "$entry"; then - fn-ga-global-list-remove allowed-domains "$entry" - dokku_log_info1 "removed allowed domain: $entry" + if fn-ga-list-contains "$scope" allowed-domains "$entry"; then + fn-ga-list-remove "$scope" allowed-domains "$entry" + dokku_log_info1 "removed allowed domain $(fn-ga-scope-label "$scope"): $entry" removed=true fi - if fn-ga-global-list-contains allowed-emails "$entry"; then - fn-ga-global-list-remove allowed-emails "$entry" - dokku_log_info1 "removed: $entry" + if fn-ga-list-contains "$scope" allowed-emails "$entry"; then + fn-ga-list-remove "$scope" allowed-emails "$entry" + dokku_log_info1 "removed $(fn-ga-scope-label "$scope"): $entry" removed=true fi - [[ "$removed" == "true" ]] || dokku_log_warn "$entry was not on the allow list" + [[ "$removed" == "true" ]] || dokku_log_warn "$entry was not on the allow list $(fn-ga-scope-label "$scope")" done + if [[ "$scope" != "global" && $((total - present)) -le 0 ]]; then + dokku_log_info1 "$scope has no allow entries left and now uses the global allow list" + fi + dokku_log_verbose "removed users lose access on their next request; existing sessions are not honored" - fn-ga-reload-service-config + fn-ga-apply-list-change "$scope" } cmd-google-auth-unallow "$@" diff --git a/subcommands/undeny b/subcommands/undeny index f2451ea..007e458 100755 --- a/subcommands/undeny +++ b/subcommands/undeny @@ -4,29 +4,41 @@ set -eo pipefail source "$(dirname "$(dirname "${BASH_SOURCE[0]}")")/functions" cmd-google-auth-undeny() { - declare desc="remove an address from the deny list" + declare desc="remove an address from a deny list" local cmd="google-auth:undeny" [[ "$1" == "$cmd" ]] && shift 1 - [[ $# -gt 0 ]] || dokku_log_fail "usage: dokku google-auth:undeny (see: dokku google-auth:deny)" + fn-ga-resolve-scope "$cmd" "${1:-}" + local scope="$GA_SCOPE" + shift 1 || true + + [[ $# -gt 0 ]] || dokku_log_fail "usage: dokku $cmd |--global " local entry for entry in "$@"; do entry="${entry,,}" - if ! fn-ga-global-list-contains denied-emails "$entry"; then - dokku_log_warn "$entry was not on the deny list" + if ! fn-ga-list-contains "$scope" denied-emails "$entry"; then + dokku_log_warn "$entry was not on the deny list $(fn-ga-scope-label "$scope")" + # A global denial cannot be lifted per app; say so rather than let the + # operator think the address is unblocked now. + if [[ "$scope" != "global" ]] && fn-ga-list-contains global denied-emails "$entry"; then + dokku_log_warn "$entry is denied globally, which no app can override — run: dokku google-auth:undeny --global $entry" + fi continue fi - fn-ga-global-list-remove denied-emails "$entry" - dokku_log_info1 "removed from deny list: $entry" - # Undenying only stops the explicit block; the allow list still decides. - if ! fn-ga-global-list-contains allowed-emails "$entry" && - ! fn-ga-global-list-contains allowed-domains "${entry#*@}"; then - dokku_log_warn "$entry still cannot sign in — nothing on the allow list covers it (see: dokku google-auth:allow)" + fn-ga-list-remove "$scope" denied-emails "$entry" + dokku_log_info1 "removed from the deny list $(fn-ga-scope-label "$scope"): $entry" + if [[ "$scope" != "global" ]] && fn-ga-list-contains global denied-emails "$entry"; then + dokku_log_warn "$entry is still denied globally — run: dokku google-auth:undeny --global $entry" + continue + fi + # Undenying only lifts the block; the allow lists still decide. + if ! fn-ga-email-effectively-allowed "$scope" "$entry"; then + dokku_log_warn "$entry still cannot sign in — nothing on the allow list covers it (see: dokku google-auth:allow $(fn-ga-scope-arg "$scope"))" fi done - fn-ga-reload-service-config + fn-ga-apply-list-change "$scope" } cmd-google-auth-undeny "$@" diff --git a/test/access-list-test.sh b/test/access-list-test.sh index a6c3254..7242edd 100755 --- a/test/access-list-test.sh +++ b/test/access-list-test.sh @@ -1,19 +1,21 @@ #!/usr/bin/env bash # Exercises the allow/deny list helpers and the google-auth:allow / :unallow / -# :deny / :undeny subcommands against a fake dokku layout, and checks that the -# lists reach the env file the auth service reads. +# :deny / :undeny subcommands against a fake dokku layout — both the global +# scope and per-app scopes — and checks that global lists reach the env file +# while per-app lists land where the container's bind mount expects them. set -eo pipefail ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" WORK="$(mktemp -d)" trap 'rm -rf "$WORK"' EXIT -# Fake dokku host layout. +# Fake dokku host layout, with two apps that exist as far as dokku is concerned. export DOKKU_ROOT="$WORK/dokku-root" export DOKKU_LIB_ROOT="$WORK/dokku-lib" export PLUGIN_CORE_AVAILABLE_PATH="$WORK/nonexistent" # force built-in fallbacks -GLOBAL="$DOKKU_LIB_ROOT/data/google-auth/global" -mkdir -p "$GLOBAL" +DATA="$DOKKU_LIB_ROOT/data/google-auth" +GLOBAL="$DATA/global" +mkdir -p "$GLOBAL" "$DOKKU_ROOT/my-app" "$DOKKU_ROOT/other-app" # Stub docker so the subcommands see the service as not running and never touch # a real container. This test is about list handling, not container management. @@ -46,6 +48,13 @@ expect_output() { grep -qF "$needle" <<<"$out" || fail "expected '$needle' in google-auth:$1 output, got: $out" } +expect_fails() { + local why="$1" + shift + ga "$@" >/dev/null 2>&1 && fail "$why" + return 0 +} + # --- entry validation and classification --- fn-ga-validate-list-entry "guest@partner.com" || fail "address should be a valid entry" fn-ga-validate-list-entry "signal.org" || fail "domain should be a valid entry" @@ -57,77 +66,163 @@ fn-ga-validate-list-entry "" && fail "empty entry should be rejected" [[ "$(fn-ga-allow-entry-kind "guest@partner.com")" == "email" ]] || fail "address misclassified" echo "ok: entry validation and classification" -# --- list helpers --- -fn-ga-global-list-add allowed-emails "a@x.com" -fn-ga-global-list-add allowed-emails "a@x.com" # idempotent -[[ "$(fn-ga-global-list-count allowed-emails)" -eq 1 ]] || fail "duplicate add should be a no-op" -fn-ga-global-list-contains allowed-emails "a@x.com" || fail "contains should find the entry" -fn-ga-global-list-contains allowed-emails "a@x.co" && fail "contains should match whole lines only" -fn-ga-global-list-remove allowed-emails "a@x.com" -[[ "$(fn-ga-global-list-count allowed-emails)" -eq 0 ]] || fail "remove should empty the list" -fn-ga-global-list-remove allowed-emails "nope@x.com" # missing entry is not an error -echo "ok: list helpers" +# --- list helpers, in both scopes --- +for scope in global my-app; do + fn-ga-list-add "$scope" allowed-emails "a@x.com" + fn-ga-list-add "$scope" allowed-emails "a@x.com" # idempotent + [[ "$(fn-ga-list-count "$scope" allowed-emails)" -eq 1 ]] || fail "$scope: duplicate add should be a no-op" + fn-ga-list-contains "$scope" allowed-emails "a@x.com" || fail "$scope: contains should find the entry" + fn-ga-list-contains "$scope" allowed-emails "a@x.co" && fail "$scope: contains should match whole lines only" + fn-ga-list-remove "$scope" allowed-emails "a@x.com" + [[ "$(fn-ga-list-count "$scope" allowed-emails)" -eq 0 ]] || fail "$scope: remove should empty the list" + fn-ga-list-remove "$scope" allowed-emails "nope@x.com" # missing entry is not an error +done +[[ "$(fn-ga-list-file global allowed-emails)" == "$GLOBAL/allowed-emails" ]] || fail "global list path wrong" +[[ "$(fn-ga-list-file my-app allowed-emails)" == "$DATA/apps/my-app/allowed-emails" ]] || fail "per-app list path wrong" +echo "ok: list helpers in both scopes" -# --- allow --- -ga allow >/dev/null || fail "listing an empty allow list should succeed" -ga allow Signal.org @Example.com Guest@Partner.com >/dev/null +# --- scope resolution --- +expect_fails "a missing scope should be rejected" allow +expect_fails "an unknown flag should be rejected" allow --oops x +expect_fails "an app that does not exist should be rejected" allow ghost-app a@b.com +expect_output "unknown flag" allow --oops x +expect_output "does not exist" allow ghost-app a@b.com +echo "ok: scope resolution" + +# --- global allow --- +ga allow --global Signal.org @Example.com Guest@Partner.com >/dev/null grep -qx "signal.org" "$GLOBAL/allowed-domains" || fail "bare domain should be stored lowercased" grep -qx "example.com" "$GLOBAL/allowed-domains" || fail "@domain should be stored without the @" grep -qx "guest@partner.com" "$GLOBAL/allowed-emails" || fail "address should be stored lowercased" -ga allow "a@b.com,c@d.com" 2>/dev/null && fail "allow should reject an entry with a comma" -ga allow "localhost" 2>/dev/null && fail "allow should reject a domain with no dot" -[[ "$(fn-ga-global-list-count allowed-domains)" -eq 2 ]] || fail "rejected entries should not be stored" -echo "ok: allow" +expect_fails "allow should reject an entry with a comma" allow --global "a@b.com,c@d.com" +expect_fails "allow should reject a domain with no dot" allow --global localhost +[[ "$(fn-ga-list-count global allowed-domains)" -eq 2 ]] || fail "rejected entries should not be stored" +echo "ok: global allow" -# --- deny wins, and contradictions are surfaced --- -ga deny Former@Signal.org >/dev/null -grep -qx "former@signal.org" "$GLOBAL/denied-emails" || fail "deny should store the address lowercased" -ga deny signal.org 2>/dev/null && fail "deny should reject a bare domain" -ga deny guest@partner.com >/dev/null -# guest@partner.com is on both lists now; allow must say deny wins. -expect_output "deny list" allow guest@partner.com -echo "ok: deny" +# --- per-app allow lives in the app's own directory and warns about the switch --- +expect_output "no longer uses the global allow list" allow my-app ceo@signal.org +grep -qx "ceo@signal.org" "$DATA/apps/my-app/allowed-emails" || fail "per-app entry should be stored under apps/" +grep -qx "ceo@signal.org" "$GLOBAL/allowed-emails" && fail "a per-app entry must not touch the global list" +expect_output "(none — other-app uses the global allow list)" allow other-app +# The warning is only for the first entry, when the app stops inheriting. +out="$(ga allow my-app cto@signal.org 2>&1)" +grep -qF "no longer uses the global allow list" <<<"$out" && + fail "the inheritance warning should only fire on the first entry" +echo "ok: per-app allow" + +# --- the service must be able to read per-app lists through its bind mount --- +[[ "$(stat -c '%a' "$DATA/apps")" == "711" ]] || fail "apps/ must be traversable by the container uid" +[[ "$(stat -c '%a' "$DATA/apps/my-app")" == "711" ]] || fail "apps// must be traversable by the container uid" +[[ "$(stat -c '%a' "$DATA/apps/my-app/allowed-emails")" == "644" ]] || fail "per-app lists must be readable by the container uid" +[[ "$(stat -c '%a' "$GLOBAL/allowed-emails")" == "600" ]] || fail "global lists should stay 0600" +echo "ok: per-app file modes" + +# --- deny is per scope, and a global denial cannot be lifted by an app --- +ga deny --global former@signal.org >/dev/null +ga deny my-app bob@signal.org >/dev/null +grep -qx "bob@signal.org" "$DATA/apps/my-app/denied-emails" || fail "per-app denial should be stored under apps/" +grep -qx "bob@signal.org" "$GLOBAL/denied-emails" && fail "a per-app denial must not touch the global list" +expect_fails "deny should reject a bare domain" deny my-app signal.org +# Listing an app's deny list also shows the global entries that apply to it. +expect_output "former@signal.org (global)" deny my-app +ga deny my-app former@signal.org >/dev/null +expect_output "still denied globally" undeny my-app former@signal.org +expect_output "denied globally, which no app can override" undeny other-app former@signal.org +echo "ok: deny scoping" + +# --- allow warns when a deny list (either scope) will win --- +expect_output "on a deny list, which wins" allow my-app bob@signal.org +expect_output "on a deny list, which wins" allow --global former@signal.org +echo "ok: deny-wins warnings" # --- undeny --- -ga undeny Guest@Partner.com >/dev/null -fn-ga-global-list-contains denied-emails "guest@partner.com" && fail "undeny should remove the address" -expect_output "was not on the deny list" undeny never@denied.com -ga deny stranger@elsewhere.com >/dev/null -expect_output "still cannot sign in" undeny stranger@elsewhere.com +ga undeny my-app bob@signal.org >/dev/null +fn-ga-list-contains my-app denied-emails "bob@signal.org" && fail "undeny should remove the address" +expect_output "was not on the deny list" undeny my-app never@denied.com +ga deny other-app stranger@elsewhere.com >/dev/null +expect_output "still cannot sign in" undeny other-app stranger@elsewhere.com echo "ok: undeny" -# --- unallow, including the lockout guardrail --- -ga unallow @Example.com >/dev/null -fn-ga-global-list-contains allowed-domains "example.com" && fail "unallow should remove the domain" -expect_output "was not on the allow list" unallow absent@nowhere.com +# --- unallow: the lockout guardrail is global-only --- +ga unallow --global @Example.com >/dev/null +fn-ga-list-contains global allowed-domains "example.com" && fail "unallow should remove the domain" +expect_output "was not on the allow list" unallow --global absent@nowhere.com -# signal.org + guest@partner.com remain; removing both (with a duplicate to -# check de-duplication) must be refused, and must not change anything. +# Removing every remaining global entry (with a duplicate, to check +# de-duplication) must be refused and change nothing. before="$(cat "$GLOBAL/allowed-domains" "$GLOBAL/allowed-emails")" -ga unallow signal.org guest@partner.com SIGNAL.ORG 2>/dev/null && - fail "unallow should refuse to empty the allow list" +expect_fails "unallow should refuse to empty the global allow list" \ + unallow --global signal.org guest@partner.com former@signal.org SIGNAL.ORG [[ "$(cat "$GLOBAL/allowed-domains" "$GLOBAL/allowed-emails")" == "$before" ]] || fail "a refused unallow must leave the lists untouched" -ga unallow guest@partner.com >/dev/null || fail "unallow should still remove a non-final entry" -[[ "$(fn-ga-global-list-count allowed-domains)" -eq 1 ]] || fail "signal.org should remain allowed" +ga unallow --global guest@partner.com >/dev/null || fail "unallow should still remove a non-final entry" +[[ "$(fn-ga-list-count global allowed-domains)" -eq 1 ]] || fail "signal.org should remain allowed" + +# Emptying an app's list is allowed: it falls back to the global one. +expect_output "now uses the global allow list" \ + unallow my-app ceo@signal.org cto@signal.org bob@signal.org +[[ "$(fn-ga-list-count my-app allowed-emails)" -eq 0 ]] || fail "the app's allow list should be empty" +[[ "$(fn-ga-list-count global allowed-domains)" -eq 1 ]] || fail "the global list must survive an app's unallow" echo "ok: unallow and lockout guardrail" -# --- configure's replace-the-list flags stay consistent with the above --- +# --- configure's replace-the-list flags still drive the GLOBAL lists --- "$ROOT/subcommands/configure" google-auth:configure \ --allow-domain signal.org --deny-email one@signal.org --deny-email two@signal.org >/dev/null 2>&1 || true -[[ "$(fn-ga-global-list-count denied-emails)" -eq 2 ]] || fail "--deny-email should replace the deny list" +[[ "$(fn-ga-list-count global denied-emails)" -eq 2 ]] || fail "--deny-email should replace the global deny list" "$ROOT/subcommands/configure" google-auth:configure --clear-deny-emails >/dev/null 2>&1 || true -[[ "$(fn-ga-global-list-count denied-emails)" -eq 0 ]] || fail "--clear-deny-emails should empty the deny list" +[[ "$(fn-ga-list-count global denied-emails)" -eq 0 ]] || fail "--clear-deny-emails should empty the global deny list" +[[ "$(fn-ga-list-count my-app denied-emails)" -eq 0 ]] || fail "configure should not touch per-app lists" echo "ok: configure flags" -# --- the lists reach the service env file --- -ga deny former@signal.org >/dev/null -ga allow guest@partner.com >/dev/null +# --- global lists reach the env file; per-app lists reach the mount --- +# Start from a known set so the env file can be asserted exactly. +ga unallow --global former@signal.org >/dev/null 2>&1 || true +ga deny --global former@signal.org >/dev/null +ga allow --global guest@partner.com >/dev/null +ga allow my-app ceo@signal.org >/dev/null 2>&1 fn-ga-write-env-file -ENV_FILE="$DOKKU_LIB_ROOT/data/google-auth/service.env" -grep -qx "GOOGLE_AUTH_ALLOWED_DOMAINS=signal.org" "$ENV_FILE" || fail "env file missing allowed domains" -grep -qx "GOOGLE_AUTH_ALLOWED_EMAILS=guest@partner.com" "$ENV_FILE" || fail "env file missing allowed emails" -grep -qx "GOOGLE_AUTH_DENIED_EMAILS=former@signal.org" "$ENV_FILE" || fail "env file missing denied emails" +ENV_FILE="$DATA/service.env" +grep -qx "GOOGLE_AUTH_ALLOWED_DOMAINS=signal.org" "$ENV_FILE" || fail "env file missing global allowed domains" +grep -qx "GOOGLE_AUTH_ALLOWED_EMAILS=guest@partner.com" "$ENV_FILE" || fail "env file missing global allowed emails" +grep -qx "GOOGLE_AUTH_DENIED_EMAILS=former@signal.org" "$ENV_FILE" || fail "env file missing global denied emails" +grep -qx "GOOGLE_AUTH_APP_CONFIG_DIR=/data/apps" "$ENV_FILE" || fail "env file must point the service at the mount" +grep -q "ceo@signal.org" "$ENV_FILE" && fail "per-app entries must not be baked into the env file" echo "ok: service env file" +# --- report shows both scopes, enabled or not --- +out="$("$ROOT/subcommands/report" google-auth:report my-app)" +grep -qF "ceo@signal.org" <<<"$out" || fail "report should show the app's allow entries: $out" +grep -qF "replaces the global allow list" <<<"$out" || fail "report should say the app's list replaces global: $out" +out="$("$ROOT/subcommands/report" google-auth:report other-app)" +grep -qF "inherits the global allow list" <<<"$out" || fail "report should say an app inherits: $out" +echo "ok: report" + +# --- setting an app's list refreshes its nginx config --- +# Per-app lists depend on nginx stamping the app name, so a config written +# before that header existed has to be rewritten; otherwise the app would +# silently fall back to the global lists. +cat >"$DOKKU_ROOT/my-app/nginx.conf" <<'EOF' +upstream my-app-5000 { + server 172.17.0.3:5000; +} +EOF +fn-ga-app-set-enabled my-app true +APP_CONF="$DOKKU_ROOT/my-app/nginx.conf.d/google-auth.conf" +mkdir -p "$(dirname "$APP_CONF")" +echo "# stale config from an older plugin version" >"$APP_CONF" +ga allow my-app auditor@signal.org >/dev/null 2>&1 +grep -q 'proxy_set_header X-Google-Auth-App "my-app";' "$APP_CONF" || + fail "changing an app's list should rewrite its nginx config to stamp the app name" +echo "ok: per-app list change refreshes the nginx config" + +# --- lifecycle triggers carry per-app lists --- +"$ROOT/post-app-rename" my-app renamed-app +[[ ! -d "$DATA/apps/my-app" ]] || fail "rename should move the app's directory" +fn-ga-list-contains renamed-app allowed-emails "ceo@signal.org" || fail "rename should keep the app's allow list" +"$ROOT/post-app-clone" renamed-app clone-app +fn-ga-list-contains clone-app allowed-emails "ceo@signal.org" || fail "clone should copy the app's allow list" +"$ROOT/post-delete" clone-app +[[ ! -d "$DATA/apps/clone-app" ]] || fail "delete should remove the app's directory" +echo "ok: lifecycle triggers" + echo "ALL ACCESS LIST TESTS PASSED" diff --git a/test/help-test.sh b/test/help-test.sh new file mode 100755 index 0000000..3f5d278 --- /dev/null +++ b/test/help-test.sh @@ -0,0 +1,52 @@ +#!/usr/bin/env bash +# Checks the two shapes of help output: `dokku help` must get a single summary +# line (so the plugin doesn't spam the global command list), while +# `dokku google-auth:help` documents every subcommand the plugin ships. +set -eo pipefail + +ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" + +fail() { + echo "FAIL: $*" 1>&2 + exit 1 +} + +# --- `dokku help`: exactly one "name, description" line --- +top="$("$ROOT/commands" help)" +[[ "$(wc -l <<<"$top")" -eq 1 ]] || + fail "plain 'dokku help' should emit one summary line, got $(wc -l <<<"$top")" +[[ "$top" =~ ^[[:space:]]+google-auth,\ .+ ]] || + fail "summary line should read ' google-auth, ', got: $top" +echo "ok: dokku help shows one line" + +# --- `dokku google-auth:help`: the full list --- +full="$("$ROOT/commands" google-auth:help)" +grep -q 'Usage: dokku google-auth\[:COMMAND\]' <<<"$full" || fail "help should print a usage line" + +# Every subcommand file (except the no-argument default) must be documented. +for path in "$ROOT"/subcommands/*; do + sub="$(basename "$path")" + [[ "$sub" == "default" ]] && continue + grep -q "google-auth:$sub" <<<"$full" || + fail "google-auth:$sub exists in subcommands/ but is undocumented in google-auth:help" +done +echo "ok: every subcommand is documented" + +# Descriptions must not contain commas: the table is rendered with +# `column -s,` so a comma splits the line into a bogus third column. +while IFS= read -r line; do + [[ "$line" =~ ^[[:space:]]+google-auth: ]] || continue + [[ "$(tr -cd ',' <<<"$line" | wc -c)" -le 1 ]] || + fail "help entry has more than one comma, which breaks the column layout: $line" +done < <(sed -n '/^help_content$/q;p' "$ROOT/commands") +echo "ok: help entries have no stray commas" + +# --- unknown subcommands still fall through to dokku's dispatcher --- +set +e +"$ROOT/commands" google-auth:does-not-exist >/dev/null 2>&1 +code=$? +set -e +[[ "$code" -eq 10 ]] || fail "unknown command should exit 10 (DOKKU_NOT_IMPLEMENTED_EXIT), got $code" +echo "ok: unknown subcommand exits 10" + +echo "ALL HELP TESTS PASSED" diff --git a/test/nginx-conf-test.sh b/test/nginx-conf-test.sh index 00788f1..75562be 100755 --- a/test/nginx-conf-test.sh +++ b/test/nginx-conf-test.sh @@ -60,6 +60,32 @@ grep -q 'proxy_set_header X-Forwarded-Email \$google_auth_email;' "$CONF" || fai grep -q 'proxy_set_header X-Forwarded-Email "";' "$CONF" || fail "excluded paths should strip identity headers" echo "ok: conf contents" +# --- the app name must be stamped on every request that reaches the service --- +# Checking the invariant rather than a fixed list of locations: any location +# that proxies to the auth service must set X-Google-Auth-App to this app, or +# the service would fall back to the global lists (silently widening access for +# an app whose own allow list is narrower). Any location that proxies to the +# app must blank it, so clients cannot pass one through. +awk -v app="$APP" ' + /^location/ { block = $0; inside = 1; to_service = 0; to_app = 0; stamped = 0; blanked = 0; next } + inside && /^}/ { + if (to_service && !stamped) { printf "location reaching the auth service without the app header: %s\n", block; bad = 1 } + if (to_app && !blanked) { printf "location reaching the app without blanking the app header: %s\n", block; bad = 1 } + inside = 0; next + } + inside { + if ($0 ~ /proxy_pass http:\/\/127\.0\.0\.1:/) to_service = 1 + if ($0 ~ /proxy_pass http:\/\/myapp-5000;/) to_app = 1 + if ($0 == sprintf(" proxy_set_header X-Google-Auth-App \"%s\";", app)) stamped = 1 + if ($0 == " proxy_set_header X-Google-Auth-App \"\";") blanked = 1 + } + END { exit bad } +' "$CONF" || fail "X-Google-Auth-App is not handled consistently across locations" +# Guard the guard: the awk above must actually see both kinds of location. +[[ "$(grep -c 'proxy_set_header X-Google-Auth-App "myapp";' "$CONF")" -eq 3 ]] || + fail "expected the app header on the three auth-service locations" +echo "ok: app name stamped for the auth service, blanked for the app" + # --- write/remove behavior --- [[ "$(fn-ga-write-conf "$APP")" == "changed" ]] || fail "first write should report changed" [[ "$(fn-ga-write-conf "$APP")" == "unchanged" ]] || fail "second write should report unchanged" @@ -100,6 +126,61 @@ EOF else fail "nginx -t rejected the generated config" fi + + # --- run it for real, and try to spoof the app name --- + # Per-app access lists are only as trustworthy as X-Google-Auth-App, so prove + # with a live nginx that a client-supplied value never reaches the auth + # service. Stand-ins for the two backends run inside the same nginx: + # :2999 pretends to be the auth service and reports the app name it saw, + # :8081 pretends to be the app and echoes what it was forwarded. + cat >"$WORK/nginx-runtime.conf" </dev/null && break + i=$((i + 1)); sleep 0.1 + done + echo "--- plain ---" + wget -q -O - http://127.0.0.1:8080/some/page 2>&1 + echo "--- spoofed ---" + wget -q -O - --header "X-Google-Auth-App: spoofed-app" http://127.0.0.1:8080/some/page 2>&1 + ' 2>/dev/null)" || fail "could not run the live nginx spoofing check: $runtime_out" + + if [[ "$(grep -c 'forwarded-email=app-seen=myapp' <<<"$runtime_out")" -ne 2 ]]; then + fail "nginx should have told the auth service the app name on both requests, got: $runtime_out" + fi + if grep -q "spoofed-app" <<<"$runtime_out"; then + fail "a client-supplied X-Google-Auth-App reached the auth service: $runtime_out" + fi + echo "ok: live nginx sends the real app name and discards a spoofed one" else echo "skip: docker unavailable, skipped real nginx validation" fi