diff --git a/README.md b/README.md index d932d70..349688c 100644 --- a/README.md +++ b/README.md @@ -314,7 +314,15 @@ and sign out at `https:///_google-auth/logout`. stock nginx template. - **Plugin updates:** `sudo dokku plugin:update google-auth` rebuilds the service image and recreates the container with freshly written settings. - `dokku google-auth:report` names the running service's state; if it ever says + Editing the plugin directory in place works too: the image is tagged with a + fingerprint of its source, so `google-auth:restart` (and anything else that + starts the service) rebuilds when the Go code has moved rather than reusing + the binary that happens to be tagged. This matters because the shell half of + the plugin takes effect the moment the files change while the binary does + not, and a container running an older build accepts the mount, accepts + `GOOGLE_AUTH_APP_CONFIG_DIR`, and ignores both. + `dokku google-auth:report` names the running service's state — it asks the + service itself rather than trusting Docker's metadata — and if it ever says it is ignoring per-app lists, `dokku google-auth:restart` recreates it. - **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 diff --git a/functions b/functions index 7102eb2..e34590d 100644 --- a/functions +++ b/functions @@ -210,7 +210,7 @@ fn-ga-apply-list-change() { return 0 fi if ! fn-ga-service-reads-app-lists; then - dokku_log_info1 "recreating the auth service so it can read per-app lists" + dokku_log_info1 "recreating the auth service so it can read per-app lists (this rebuilds the image if the plugin was updated)" fn-ga-service-start return 0 fi @@ -275,7 +275,7 @@ fn-ga-app-set-enabled() { local dir dir="$(fn-ga-app-dir "$APP")" if [[ "$ENABLED" == "true" ]]; then - mkdir -p "$dir" + dir="$(fn-ga-app-dir-ensure "$APP")" touch "$dir/enabled" else rm -f "$dir/enabled" @@ -326,9 +326,8 @@ fn-ga-validate-pattern() { fn-ga-exclude-add() { declare APP="$1" PATTERN="$2" local dir file - dir="$(fn-ga-app-dir "$APP")" + dir="$(fn-ga-app-dir-ensure "$APP")" file="$dir/excludes" - mkdir -p "$dir" touch "$file" grep -qxF "$PATTERN" "$file" || printf '%s\n' "$PATTERN" >>"$file" } @@ -605,29 +604,103 @@ fn-ga-image-exists() { docker image inspect "$GOOGLE_AUTH_IMAGE" >/dev/null 2>&1 } +# Fingerprint of everything that ends up in the service image. The image is +# labelled with it so an upgraded plugin rebuilds instead of reusing a binary +# built from older source. Without this the shell half of the plugin upgrades +# the moment the files change — new commands, new nginx config, new env var — +# while the binary enforcing them stays whatever was built first, and the +# mismatch is invisible from the outside. +fn-ga-source-hash() { + local dir="$GOOGLE_AUTH_PLUGIN_DIR" file + { + while IFS= read -r file; do + printf '%s ' "${file#"$dir/"}" + sha256sum "$file" | awk '{print $1}' + done < <( + { + printf '%s\n' "$dir/Dockerfile" "$dir/go.mod" + find "$dir/cmd" "$dir/internal" -type f 2>/dev/null + } | LC_ALL=C sort + ) + } | sha256sum | awk '{print $1}' +} + +# True when the built image matches the plugin's current source. +fn-ga-image-current() { + fn-ga-image-exists || return 1 + local labelled + labelled="$(docker image inspect -f '{{index .Config.Labels "google-auth.source-hash"}}' "$GOOGLE_AUTH_IMAGE" 2>/dev/null)" + # Images built before this label existed report an empty value or "", + # and are stale by definition. + [[ -n "$labelled" && "$labelled" != "" ]] || return 1 + [[ "$labelled" == "$(fn-ga-source-hash)" ]] +} + fn-ga-build-image() { command -v docker >/dev/null 2>&1 || dokku_log_fail "docker is required to build the google-auth service image" dokku_log_info1 "Building $GOOGLE_AUTH_IMAGE (first build downloads the Go toolchain image; this can take a few minutes)" - docker image build -t "$GOOGLE_AUTH_IMAGE" "$GOOGLE_AUTH_PLUGIN_DIR" + docker image build \ + --label "google-auth.source-hash=$(fn-ga-source-hash)" \ + -t "$GOOGLE_AUTH_IMAGE" "$GOOGLE_AUTH_PLUGIN_DIR" } fn-ga-service-running() { [[ "$(docker container inspect -f '{{.State.Running}}' "$GOOGLE_AUTH_SERVICE_NAME" 2>/dev/null)" == "true" ]] } -# True when the running container can actually read per-app lists. That takes -# both halves: the bind mount, and the environment variable pointing at it. -# Either one alone makes the service ignore every per-app list and fall back to -# the global one — silently, and in the permissive direction — so both are -# checked. A container started by an older version of this plugin has neither; -# one recreated from a stale service.env has the mount without the variable. +# Asks the running service which per-app config directory it is using, and +# prints it. This is the only check that sees the binary rather than the +# container around it; a build from before per-app lists existed answers with a +# bare "ok" and names no directory. Fails if the service is unreachable or too +# old to answer. +fn-ga-service-app-config-dir() { + command -v curl >/dev/null 2>&1 || return 1 + local port body dir + port="$(fn-ga-global-get port "$GOOGLE_AUTH_DEFAULT_PORT")" + body="$(curl -fsS --max-time 3 "http://127.0.0.1:${port}${GOOGLE_AUTH_ROUTE_PREFIX}/healthz" 2>/dev/null)" || return 1 + dir="$(sed -n 's/.*"app_config_dir":"\([^"]*\)".*/\1/p' <<<"$body")" + [[ -n "$dir" ]] || return 1 + printf '%s' "$dir" +} + +# True when the running service can actually read per-app lists. That takes +# three things, and missing any of them makes the service ignore every per-app +# list and fall back to the global one — silently, and in the permissive +# direction — so all three are checked: +# +# the bind mount — a container from an older plugin has none; +# the env var naming it — one recreated from a stale service.env has the +# mount without the variable; +# a binary that reads both — one recreated from a stale *image* has the +# mount and the variable and ignores them, which +# no amount of docker metadata can reveal. Only +# the service's own answer distinguishes it. fn-ga-service-reads-app-lists() { local inspected inspected="$(docker container inspect \ -f '{{range .Mounts}}mount={{println .Destination}}{{end}}{{range .Config.Env}}env={{println .}}{{end}}' \ "$GOOGLE_AUTH_SERVICE_NAME" 2>/dev/null)" || return 1 grep -qxF "mount=$GOOGLE_AUTH_APP_CONFIG_MOUNT" <<<"$inspected" || return 1 - grep -qxF "env=GOOGLE_AUTH_APP_CONFIG_DIR=$GOOGLE_AUTH_APP_CONFIG_MOUNT" <<<"$inspected" + grep -qxF "env=GOOGLE_AUTH_APP_CONFIG_DIR=$GOOGLE_AUTH_APP_CONFIG_MOUNT" <<<"$inspected" || return 1 + + local reported + if reported="$(fn-ga-service-app-config-dir)"; then + [[ "$reported" == "$GOOGLE_AUTH_APP_CONFIG_MOUNT" ]] + return $? + fi + # No answer (curl missing, or the service is not reachable on the loopback + # port): fall back to checking that the image was built from this plugin's + # current source and that the container is running that image. + fn-ga-image-current && fn-ga-container-runs-current-image +} + +# True when the running container was created from the current service image, +# rather than from an earlier build still tagged over. +fn-ga-container-runs-current-image() { + local running current + running="$(docker container inspect -f '{{.Image}}' "$GOOGLE_AUTH_SERVICE_NAME" 2>/dev/null)" || return 1 + current="$(docker image inspect -f '{{.Id}}' "$GOOGLE_AUTH_IMAGE" 2>/dev/null)" || return 1 + [[ -n "$running" && "$running" == "$current" ]] } fn-ga-write-env-file() { @@ -637,8 +710,13 @@ fn-ga-write-env-file() { emails="$(fn-ga-global-get-list allowed-emails | paste -sd, -)" denied="$(fn-ga-global-get-list denied-emails | paste -sd, -)" mkdir -p "$GOOGLE_AUTH_DATA_ROOT" - umask 077 - cat >"$envfile" <"$envfile" <Sign in again

`) } +// handleHealthz is both the container health probe and the plugin's way of +// asking the running binary what it is doing. It reports the per-app config +// directory because nothing outside the process can: a container recreated +// from a stale image has the bind mount and GOOGLE_AUTH_APP_CONFIG_DIR in its +// environment while running a binary from before per-app lists existed, which +// ignores both and quietly falls back to the global lists. That build answers +// this endpoint with a bare "ok" and no directory, which is what tells the two +// apart. func (s *Server) handleHealthz(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.Header().Set("Cache-Control", "no-store") w.WriteHeader(http.StatusOK) - fmt.Fprint(w, "ok") + fmt.Fprintf(w, `{"ok":true,"app_config_dir":%q}`, s.cfg.AppConfigDir) } // handleStatus is a small human-readable page for debugging. diff --git a/subcommands/report b/subcommands/report index b7a15f7..bddbd2b 100755 --- a/subcommands/report +++ b/subcommands/report @@ -102,7 +102,11 @@ cmd-google-auth-report() { # Worth calling out: every per-app list below is being ignored, and the # apps that have one are falling back to the broader global list. dokku_log_verbose "Service: running, but IGNORING all per-app lists" - dokku_log_verbose " (started without the per-app config; fix with: dokku google-auth:restart)" + if ! fn-ga-image-current; then + dokku_log_verbose " (the service image is older than the plugin; fix with: dokku google-auth:restart)" + else + dokku_log_verbose " (started without the per-app config; fix with: dokku google-auth:restart)" + fi fi local app diff --git a/test/access-list-test.sh b/test/access-list-test.sh index 22a9a77..9489aa8 100755 --- a/test/access-list-test.sh +++ b/test/access-list-test.sh @@ -187,6 +187,17 @@ grep -qx "GOOGLE_AUTH_ALLOWED_EMAILS=guest@partner.com" "$ENV_FILE" || fail "env 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" +[[ "$(stat -c '%a' "$ENV_FILE")" == "600" ]] || fail "the env file holds the client secret and must stay 0600" +# Writing the env file tightens the umask to protect that secret. It must not +# tighten anything else: google-auth:enable writes the env file (starting the +# service) and then creates the app's config directory, which the unprivileged +# service has to be able to traverse. +fn-ga-app-set-enabled umask-app true +[[ "$(stat -c '%a' "$DATA/apps/umask-app")" == "711" ]] || + fail "a directory created after the env file must still be traversable by the container uid" +fn-ga-exclude-add umask-app /api/hook +[[ "$(stat -c '%a' "$DATA/apps/umask-app")" == "711" ]] || + fail "adding an exclusion must leave the app directory traversable" echo "ok: service env file" # --- report shows both scopes, enabled or not --- @@ -216,40 +227,89 @@ grep -q 'proxy_set_header X-Google-Auth-App "my-app";' "$APP_CONF" || echo "ok: per-app list change refreshes the nginx config" # --- a running service only counts if it can really read per-app lists --- -# It takes both the bind mount and the env var naming it. A container recreated -# from a service.env written before per-app lists existed has the mount but not -# the variable, ignores every per-app list, and still looks healthy — so the -# check must not be satisfied by the mount alone. +# It takes the bind mount, the env var naming it, and a binary that reads both. +# Every combination that is missing one still looks healthy from outside while +# quietly falling back to the global lists, so none of them may satisfy the +# check. mkdir -p "$WORK/bin-docker" cat >"$WORK/bin-docker/docker" <<'EOF' #!/bin/sh -# Stands in for `docker container inspect`, replaying a canned inspection. -if [ "$1" = "container" ] && [ "$2" = "inspect" ]; then - cat "$DOCKER_INSPECT_FIXTURE" - exit 0 -fi +# Stands in for the docker CLI, replaying canned inspections. +case "$1 $2" in + "container inspect") cat "$DOCKER_INSPECT_FIXTURE" ;; + "image inspect") printf '%s\n' "$DOCKER_IMAGE_FIXTURE" ;; +esac exit 0 EOF -chmod +x "$WORK/bin-docker/docker" +cat >"$WORK/bin-docker/curl" <<'EOF' +#!/bin/sh +# Stands in for the healthz probe against the running service. An empty +# fixture means the service did not answer. +[ -s "$CURL_BODY_FIXTURE" ] || exit 7 +cat "$CURL_BODY_FIXTURE" +EOF +chmod +x "$WORK/bin-docker/docker" "$WORK/bin-docker/curl" -# Runs the check against one canned inspection, in a subshell so the stub and -# its fixture do not leak into the rest of the file. +# Runs the check against one canned healthz body and one canned container +# inspection, in a subshell so the stubs and fixtures do not leak into the rest +# of the file. +# shellcheck disable=SC2030,SC2031 # the subshell is what keeps the stubs local reads_app_lists() ( + local healthz="$1" + shift export DOCKER_INSPECT_FIXTURE="$WORK/inspect-fixture" + export CURL_BODY_FIXTURE="$WORK/healthz-fixture" + export DOCKER_IMAGE_FIXTURE="" printf '%s\n' "$@" >"$DOCKER_INSPECT_FIXTURE" + printf '%s' "$healthz" >"$CURL_BODY_FIXTURE" PATH="$WORK/bin-docker:$PATH" fn-ga-service-reads-app-lists ) -reads_app_lists "mount=/data/apps" "env=GOOGLE_AUTH_APP_CONFIG_DIR=/data/apps" "env=GOOGLE_AUTH_CLIENT_ID=x" || - fail "a container with both the mount and the env var should read per-app lists" -reads_app_lists "mount=/data/apps" "env=GOOGLE_AUTH_CLIENT_ID=x" && +CURRENT_HEALTHZ='{"ok":true,"app_config_dir":"/data/apps"}' +# The reply from a build made before per-app lists existed. +LEGACY_HEALTHZ='ok' + +reads_app_lists "$CURRENT_HEALTHZ" "mount=/data/apps" "env=GOOGLE_AUTH_APP_CONFIG_DIR=/data/apps" "env=GOOGLE_AUTH_CLIENT_ID=x" || + fail "a container with the mount, the env var, and a current binary should read per-app lists" +# The case the plugin used to miss entirely: docker metadata is perfect because +# the current shell created the container, but the image it started is old, so +# the binary inside ignores the mount and every per-app list with it. +reads_app_lists "$LEGACY_HEALTHZ" "mount=/data/apps" "env=GOOGLE_AUTH_APP_CONFIG_DIR=/data/apps" "env=GOOGLE_AUTH_CLIENT_ID=x" && + fail "a container running a pre-per-app-lists binary must not count, however well configured" +reads_app_lists '{"ok":true,"app_config_dir":"/somewhere/else"}' "mount=/data/apps" "env=GOOGLE_AUTH_APP_CONFIG_DIR=/data/apps" && + fail "a service reading some other directory is not reading the mounted one" +reads_app_lists "$CURRENT_HEALTHZ" "mount=/data/apps" "env=GOOGLE_AUTH_CLIENT_ID=x" && fail "a container with the mount but no GOOGLE_AUTH_APP_CONFIG_DIR ignores per-app lists" -reads_app_lists "env=GOOGLE_AUTH_APP_CONFIG_DIR=/data/apps" && +reads_app_lists "$CURRENT_HEALTHZ" "env=GOOGLE_AUTH_APP_CONFIG_DIR=/data/apps" && fail "a container with the env var but no mount has nothing to read" -reads_app_lists "" && fail "a container with neither should not count" +reads_app_lists "$CURRENT_HEALTHZ" "" && fail "a container with neither should not count" +# Unreachable service: with no answer to go on, an image that cannot be shown +# to match the plugin's source is assumed stale rather than assumed good. +reads_app_lists "" "mount=/data/apps" "env=GOOGLE_AUTH_APP_CONFIG_DIR=/data/apps" && + fail "an unanswering service with an unverifiable image should not count" echo "ok: per-app list readiness check" +# --- the built image has to match the source the plugin is running from --- +# The shell half of the plugin upgrades as soon as the files change; the binary +# only upgrades when the image is rebuilt. Reusing an image just because one +# exists is what lets a per-app list be configured, reported, and ignored. +# shellcheck disable=SC2030,SC2031 # the subshell is what keeps the stubs local +image_current() ( + export DOCKER_IMAGE_FIXTURE="$1" + PATH="$WORK/bin-docker:$PATH" + fn-ga-image-current +) + +SOURCE_HASH="$(fn-ga-source-hash)" +[[ -n "$SOURCE_HASH" ]] || fail "source hash should not be empty" +[[ "$SOURCE_HASH" == "$(fn-ga-source-hash)" ]] || fail "source hash should be stable across calls" +image_current "$SOURCE_HASH" || fail "an image labelled with the current source hash is current" +image_current "0000000000000000" && fail "an image built from other source is not current" +image_current "" && fail "an unlabelled image (built before this check) is not current" +image_current "" && fail "an image with no label value is not current" +echo "ok: service image tracks the plugin source" + # --- 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"