authentication plugins as a generic client-auth hook#1203
Open
ziomarco wants to merge 10 commits into
Open
Conversation
Introduce the configuration knobs for the authentication extension point: - AuthType::Plugin (auth_type = "plugin") with Display, FromStr and a plugin() predicate, following the existing variant conventions. - User.server_role for backend role impersonation on plugin-derived logins. - general.background_workers (default 4, env PGDOG_BACKGROUND_WORKERS) to cap concurrent authentication-plugin calls. - Documented commented examples in example.pgdog.toml / example.users.toml, including the note that server_auth/server_password must be supplied when server_role is used. - Serde round-trip and default-value tests colocated with existing tests. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Introduce the FFI types (PdString, PdAuthContext, PdAuthResult) and safe wrappers (AuthContext, AuthDecision, AuthGrant) for a client authentication extension point. Plugins allocate the result strings and free them via the paired pgdog_authenticate_free symbol so plugins with a custom global allocator stay sound; the host copies the values out before freeing. Resolve pgdog_authenticate / pgdog_authenticate_free in Plugin::load and add Plugin::has_authenticate and Plugin::authenticate. Bump pgdog-plugin to 0.4.0. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Generate the pgdog_authenticate and pgdog_authenticate_free symbols from a fn(AuthContext) -> AuthDecision. The user function runs inside catch_unwind (with AssertUnwindSafe, since the FFI context holds raw pointers); a panic becomes AuthDecision::Deny instead of unwinding across the FFI boundary. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add the async driver that consults `pgdog_authenticate`-capable plugins for `auth_type = "plugin"` logins. Owned credential strings move into a single `spawn_blocking` that builds the `PdAuthContext` and iterates the loaded plugins in order; the first non-Skip decision wins. When every plugin skips (or none is loaded) the client is denied: there is no password fallback. Concurrency is capped by a semaphore sized from `background_workers`, and a task join failure is treated as a denial. Adds the `PluginDenied` and `PluginNoDecision` `AuthResult` variants. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add a login branch for `auth_type = "plugin"` (non-admin) that mirrors the passthrough cleartext exchange, hands the credential to the plugin driver, and maps the outcome. On Allow it threads an `effective_user` (derived user or startup user) through `Connection::new`, the error and log paths; a Deny or all-Skip sends the generic auth error and logs the reason only when `log_connections` is set. `check_password` treats `AuthType::Plugin` like `Plain` on the admin path. `databases::add_authenticated` provisions a pool from the grant's backend credentials without comparing passwords, never overwriting a users.toml entry (it only fills credential gaps). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…lidation Thread `User.server_role` to the backend as a `role` startup parameter (a session default, so RESET ALL / DISCARD ALL restore it rather than clear it) via `Address`. On pools that set a `server_role`, reject client-issued `SET ROLE` / `RESET ROLE` / `SET SESSION AUTHORIZATION` with a 42501 error while keeping the session alive; the guard is hooked in `QueryEngine::set` next to the existing route-change check, before the backend connects. `Cluster` carries the role so the guard can see it. Add load-time validation: refuse a plugin exporting `pgdog_authenticate` without `pgdog_authenticate_free`; warn when `auth_type = "plugin"` has no capable plugin or runs without `tls_client_required`; warn when a `server_role` user lacks usable backend credentials. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Exercises `auth_type = "plugin"` end to end. Adds a `test-plugin-auth` cdylib and an rspec suite covering: allow via `secret-<user>`, generic denial for wrong/deny/panic/all-skip credentials (with the deny reason logged only in PgDog, never sent to the client), role impersonation with auto-provisioning, role survival across connection reuse and cleanup, the `SET ROLE`/`RESET ROLE`/`SET SESSION AUTHORIZATION` escape guard, and a read-only pool rejecting INSERT. The pre-existing routing-plugin spec runs unchanged as a regression phase. Also fixes `Databases::launch` disabling pools with an empty client password list. Under plugin auth the client password is supplied per login by the plugin and pools authenticate to Postgres with `server_user`/`server_password`/`server_auth`, so auto-provisioned impersonation pools (which never carry a client password) were wrongly disabled and reported "connection pool is down". The gate is now skipped when `auth_type = "plugin"`. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The impersonation guard only ran on the `Command::Set` path, so two
routing paths reached Postgres without it: session-mode multi-statement
batches forwarded verbatim (e.g. `SELECT 1; SET ROLE superuser`) and
`set_config('role', <non-constant>)`, which falls through to passthrough
because the value isn't a parseable constant. A client on a `server_role`
impersonation pool could use either to escape the fixed role.
Move the guard to a single chokepoint in the query parser, which sees
every statement form (single SET, multi-statement batches, and
set_config) and has the cluster in context. It emits a new
`Command::RoleLocked` that the engine turns into a 42501 error while
keeping the session alive, matching the previous single-statement guard.
Impersonation pools now always run the full parser so the regex
fast-path can't forward a role escape unparsed; normal pools are
unaffected (a single `Option` check). `RESET ALL` / `DISCARD ALL` remain
allowed since they restore the startup-parameter role.
Also correct a comment in client/mod.rs: comms and stats still use the
startup-packet user; only Connection::new, error responses, and log
lines use the derived effective user.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The empty-client-password launch gate had `&& !plugin_auth` appended, which disabled the guard for every cluster whenever `auth_type` was plugin and left it fully on otherwise. The real invariant is whether a cluster can authenticate to Postgres, which is per-cluster, not global. Replace the global flag with `Cluster::has_backend_credentials()`: a cluster launches with an empty client-password list when it carries a server password, an external-identity mechanism, or an impersonation `server_role`. The default scram/md5 path is unchanged — a normal cluster with no client password and no backend credentials is still disabled. Also add the missing docs URL anchor on the `server_role` field. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The rebase onto the new-parser port left the guard only in the new_parser variant of QueryParser::query(), silently dropping it from the default build, and role_escape_target() only spoke protobuf. Re-insert the chokepoint in the old-parser query() and give role_escape_target() a new_parser implementation over pg_raw_parse nodes, mirroring how set_config() is split across the two engines. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
a74e8d4 to
adb6c7c
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This is the first of two PRs reworking #1084 (JWT client auth) into a general extension point. It adds one new plugin hook,
pgdog_authenticate, next to the existingpgdog_route, so a plugin can validate a client credential. There is no JWT anywhere in this PR; core only learns how to ask a plugin "is this credential good?". It also adds a per-userserver_roleoption so a pool can impersonate a role on the backend, and abackground_workerssetting that caps how many auth plugin calls run at once.Design was discussed first in an RFC (gist: https://gist.github.com/ziomarco/ad18e5c5f65472f13671958514b30c0d) and approved with a few adjustments, all of which are implemented here.
Motivation
In #1084 I put JWT validation directly into
Client::check_password. The feedback was that it belongs in a plugin, but the plugin system only had a query-routing hook, so there was nowhere for auth to plug in. This PR builds that missing seam.Authentication is a multi-message exchange, so I did not try to hand the client socket to a plugin. pgdog keeps driving the wire exactly like passthrough auth does today: it sends
AuthenticationCleartextPassword, reads the password message, and only then asks the plugins about the credential. Each plugin answers Skip, Allow, or Deny, and the first non-Skip answer wins, in[[plugins]]order. The same hook works for LDAP, OIDC introspection, RADIUS, Vault, or any custom check without touching core again.The plugin call runs inside
tokio::task::spawn_blocking, since auth happens once per connection and a plugin may need to block on I/O (a JWKS fetch, an LDAP bind). That is different fromroute(), which runs per query and has to stay fast.What's in this PR
pgdog-plugin: thePdAuthContext/PdAuthResultFFI types, safeAuthContext/AuthDecision/AuthGrantwrappers, and the symbol resolution and call path. The result carries plugin-allocated strings, so there is a pairedpgdog_authenticate_free: pgdog copies the strings out and the plugin frees its own allocations, which stays sound even if a plugin uses a custom allocator. A plugin that exportspgdog_authenticatewithout the free symbol is refused at load.pgdog-macros: an#[authentication]attribute, the same idea as#[route]. It generates both symbols and wraps the user function incatch_unwind, so a panicking plugin becomes a Deny rather than taking down the pooler.pgdog-config: theauth_type = "plugin"variant, the per-userserver_role, andbackground_workers.pgdog: thespawn_blockingdriver, one new branch inClient::login,databases::add_authenticatedfor auto-provisioning a pool on Allow (the existingadd()compares the credential against the stored password, which cannot work with a per-login token), therolestartup parameter, the guard that rejects client-issued role changes on impersonation pools, and load-time validation.For
server_role, the role is pushed as arolestartup-packet parameter. Startup parameters are session defaults, soRESET ALLandDISCARD ALLrestore the role instead of clearing it, and pgdog's connection cleanup between checkouts needs no special handling. The one thing that does need handling is a client trying to escape the role, so pgdog rejectsSET ROLE,RESET ROLE, andSET SESSION AUTHORIZATION(including the multi-statement andset_config()spellings) on pools that set aserver_role. The statement is refused with a normal Postgres error and the session stays open.Decisions from the RFC review
A few points were settled during review and are worth calling out:
auth_type = "plugin"is explicit, so if nothing claims the credential the login fails.background_workers(default 4) caps concurrent auth plugin calls, so a slow or stuck plugin cannot exhaust the blocking pool.server_roleis a core per-user option, not something JWT-specific. A[[users]]entry that sets it has to supply a working backend credential (server_password, or aserver_authmechanism), since the user config no longer carries the Postgres password. pgdog warns at config load if that is missing.auth_type = "plain"and passthrough. pgdog warns at startup whentls_client_required = false.pgdog-plugingoes from 0.3.0 to 0.4.0 so stale plugins are rejected cleanly by the existing version gate. The in-tree plugins are updated to match.Testing
cargo nextest run --profile devis green (2260 tests). New unit tests cover the FFI round-trip (String toPdStringand back, the Allow/Deny/Skip encoding, and free-once-then-safe semantics), the auth driver's ordering and all-skip-denies behavior,add_authenticated's gap-filling and never-overwrite rules, and the role-escape guard across single-statement, multi-statement, andset_config()forms.There is a new integration suite under
integration/plugins/auth(run bybash integration/plugins/run.sh) with a smalltest-plugin-authbuilt through#[authentication]. It checks that a good credential connects and queries, that wrong / deny / panic / all-skip credentials all get the generic auth error while the deny reason stays in the pgdog log, that an impersonated role shows up inSELECT current_userand survives connection reuse and cleanup, that role-escape attempts are rejected, and that a read-only grant blocks INSERT. The existing routing-plugin spec runs unchanged as a regression check. I also walked through each auth scenario by hand with psql against a running pgdog.What's next
PR 2 is the JWT plugin itself: an in-tree
plugins/pgdog-jwt-authcdylib (the validator ported from #1084, with the JWKS cache made synchronous), built into the Docker image, plus theintegration/jwtsuite ported over and documentation for both the hook and the plugin. All of the JWT configuration moves out of[general]and into the plugin's own config file, which is what made thejwt_user_suffixhack from #1084 unnecessary.