Refuse a throwaway plugin storage secret in bootstrap, before any role is created
CI / full-gate (push) Successful in 2m56s
CI / full-gate (push) Successful in 2m56s
This commit is contained in:
@@ -777,7 +777,9 @@ What the host does guarantee:
|
|||||||
**Passwords are derived, never stored** — each is `HMAC-SHA256(PLUGIN_DB_SECRET, <plugin id>)`, so
|
**Passwords are derived, never stored** — each is `HMAC-SHA256(PLUGIN_DB_SECRET, <plugin id>)`, so
|
||||||
`bootstrap` and `web` compute the same value independently and nothing has to be written down.
|
`bootstrap` and `web` compute the same value independently and nothing has to be written down.
|
||||||
Rotate every plugin's password by changing `PLUGIN_DB_SECRET` and running `docker compose up -d`,
|
Rotate every plugin's password by changing `PLUGIN_DB_SECRET` and running `docker compose up -d`,
|
||||||
which re-applies each role's password and leaves the data alone.
|
which re-applies each role's password and leaves the data alone. Under `REQUIRE_SECURE_SECRETS` a
|
||||||
|
missing, empty or throwaway secret is refused — in `bootstrap` before it creates any role, so no
|
||||||
|
database is ever given a password derivable from a constant in this repo.
|
||||||
|
|
||||||
**Only `bootstrap` holds superuser credentials.** It alone gets `PLUGIN_DB_ADMIN_URL`, the DSN that
|
**Only `bootstrap` holds superuser credentials.** It alone gets `PLUGIN_DB_ADMIN_URL`, the DSN that
|
||||||
may `CREATE DATABASE`/`CREATE ROLE`. `web` gets `PLUGIN_DB_URL`, which names the server and carries
|
may `CREATE DATABASE`/`CREATE ROLE`. `web` gets `PLUGIN_DB_URL`, which names the server and carries
|
||||||
@@ -1020,7 +1022,7 @@ The app is **environment-agnostic**: no `NODE_ENV`, every behaviour its own expl
|
|||||||
| `CSRF_SECRET` | dev throwaway | signs our double-submit CSRF token; enforced by `REQUIRE_SECURE_SECRETS` |
|
| `CSRF_SECRET` | dev throwaway | signs our double-submit CSRF token; enforced by `REQUIRE_SECURE_SECRETS` |
|
||||||
| `PLUGIN_DB_URL` | _unset_ (dev: `postgres://postgres:5432`) | credential-free Postgres base URL for [plugin storage](#plugin-storage); unset ⇒ storage off, and a plugin declaring it aborts boot |
|
| `PLUGIN_DB_URL` | _unset_ (dev: `postgres://postgres:5432`) | credential-free Postgres base URL for [plugin storage](#plugin-storage); unset ⇒ storage off, and a plugin declaring it aborts boot |
|
||||||
| `PLUGIN_DB_ADMIN_URL` | _unset_ (dev: the bundled superuser) | the DSN that provisions each plugin's database and role — read by the one-shot `bootstrap` service **only**, never by `web` |
|
| `PLUGIN_DB_ADMIN_URL` | _unset_ (dev: the bundled superuser) | the DSN that provisions each plugin's database and role — read by the one-shot `bootstrap` service **only**, never by `web` |
|
||||||
| `PLUGIN_DB_SECRET` | dev throwaway | derives each plugin's database password; enforced by `REQUIRE_SECURE_SECRETS` once `PLUGIN_DB_URL` is set |
|
| `PLUGIN_DB_SECRET` | dev throwaway | derives each plugin's database password; `REQUIRE_SECURE_SECRETS` enforces it in `web` once `PLUGIN_DB_URL` is set, and in `bootstrap` whenever a plugin declares storage |
|
||||||
|
|
||||||
### Canonical host (one public URL)
|
### Canonical host (one public URL)
|
||||||
|
|
||||||
|
|||||||
@@ -35,6 +35,7 @@ services:
|
|||||||
# Provisions the plugin databases web connects to above, as the dev superuser.
|
# Provisions the plugin databases web connects to above, as the dev superuser.
|
||||||
environment:
|
environment:
|
||||||
PLUGIN_DB_ADMIN_URL: postgres://${POSTGRES_USER:-ory}:${POSTGRES_PASSWORD:-ory}@postgres:5432/ory
|
PLUGIN_DB_ADMIN_URL: postgres://${POSTGRES_USER:-ory}:${POSTGRES_PASSWORD:-ory}@postgres:5432/ory
|
||||||
|
REQUIRE_SECURE_SECRETS: "false" # dev derives from the throwaway, as web does
|
||||||
volumes:
|
volumes:
|
||||||
- .:/app
|
- .:/app
|
||||||
|
|
||||||
|
|||||||
@@ -153,6 +153,7 @@ services:
|
|||||||
# `storage` fails the seed loudly. The secret must match web's; both derive the same passwords.
|
# `storage` fails the seed loudly. The secret must match web's; both derive the same passwords.
|
||||||
PLUGIN_DB_ADMIN_URL: ${PLUGIN_DB_ADMIN_URL:-}
|
PLUGIN_DB_ADMIN_URL: ${PLUGIN_DB_ADMIN_URL:-}
|
||||||
PLUGIN_DB_SECRET: ${PLUGIN_DB_SECRET:-}
|
PLUGIN_DB_SECRET: ${PLUGIN_DB_SECRET:-}
|
||||||
|
REQUIRE_SECURE_SECRETS: "true" # refuse the throwaway secret here too, before any role is created
|
||||||
volumes:
|
volumes:
|
||||||
- ./ory/kratos/tokenizer:/etc/config/kratos/tokenizer
|
- ./ory/kratos/tokenizer:/etc/config/kratos/tokenizer
|
||||||
command: node src/auth/bootstrap.ts
|
command: node src/auth/bootstrap.ts
|
||||||
|
|||||||
@@ -19,6 +19,17 @@ test("web and bootstrap resolve the same plugin storage secret", () => {
|
|||||||
assert.match(resolvePluginDbSecret({ PLUGIN_DB_SECRET: "" }), /dev-insecure/); // empty is unset, not a secret
|
assert.match(resolvePluginDbSecret({ PLUGIN_DB_SECRET: "" }), /dev-insecure/); // empty is unset, not a secret
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// bootstrap writes these passwords into Postgres, so it must refuse the publicly-known throwaway
|
||||||
|
// before creating a role with one — not leave web to notice afterwards.
|
||||||
|
test("bootstrap refuses a missing, empty or throwaway plugin storage secret when hardened", () => {
|
||||||
|
const hardened = { REQUIRE_SECURE_SECRETS: "true" };
|
||||||
|
for (const secret of [undefined, "", "dev-insecure-plugin-db-secret"]) {
|
||||||
|
const env = secret === undefined ? hardened : { ...hardened, PLUGIN_DB_SECRET: secret };
|
||||||
|
assert.throws(() => resolvePluginDbSecret(env), /PLUGIN_DB_SECRET/, `for ${JSON.stringify(secret)}`);
|
||||||
|
}
|
||||||
|
assert.equal(resolvePluginDbSecret({ ...hardened, PLUGIN_DB_SECRET: "a-real-secret" }), "a-real-secret");
|
||||||
|
});
|
||||||
|
|
||||||
test("loads dev defaults when the environment is empty", () => {
|
test("loads dev defaults when the environment is empty", () => {
|
||||||
const c = loadConfig({});
|
const c = loadConfig({});
|
||||||
assert.equal(c.port, 3000);
|
assert.equal(c.port, 3000);
|
||||||
|
|||||||
+8
-6
@@ -8,11 +8,13 @@ export type LogLevel = (typeof LOG_LEVELS)[number];
|
|||||||
|
|
||||||
const DEV_PLUGIN_DB_SECRET = "dev-insecure-plugin-db-secret";
|
const DEV_PLUGIN_DB_SECRET = "dev-insecure-plugin-db-secret";
|
||||||
|
|
||||||
// bootstrap resolves the plugin-storage secret through this, web through loadConfig — and the two
|
// The one resolution both processes use — they must agree exactly, or web connects with a password
|
||||||
// must agree exactly or web connects with a password the role was never given. Compose passes an
|
// the role was never given. Compose passes an unset variable through as "", so empty means unset.
|
||||||
// unset variable through as "", so empty means throwaway here as it does in readSecret.
|
// `enforce` says whether storage is actually in play: web once PLUGIN_DB_URL is configured,
|
||||||
export function resolvePluginDbSecret(env: Env): string {
|
// bootstrap once a plugin declares storage. Enforced, the throwaway is refused — bootstrap is what
|
||||||
return env["PLUGIN_DB_SECRET"] || DEV_PLUGIN_DB_SECRET;
|
// writes these passwords into Postgres, so it must refuse *before* creating a role with one.
|
||||||
|
export function resolvePluginDbSecret(env: Env, enforce?: boolean): string {
|
||||||
|
return readSecret(env, "PLUGIN_DB_SECRET", DEV_PLUGIN_DB_SECRET, enforce ?? readBool(env, "REQUIRE_SECURE_SECRETS", false));
|
||||||
}
|
}
|
||||||
|
|
||||||
export interface Config {
|
export interface Config {
|
||||||
@@ -165,7 +167,7 @@ export function loadConfig(env: Env = process.env): Config {
|
|||||||
// credentials: the superuser DSN that provisions stays in bootstrap, so a plugin cannot read it
|
// credentials: the superuser DSN that provisions stays in bootstrap, so a plugin cannot read it
|
||||||
// out of web's environment. Unset ⇒ storage is off and a plugin declaring it fails loud at boot,
|
// out of web's environment. Unset ⇒ storage is off and a plugin declaring it fails loud at boot,
|
||||||
// which is also why the secret is only enforced once a URL is configured.
|
// which is also why the secret is only enforced once a URL is configured.
|
||||||
pluginDbSecret: readSecret(env, "PLUGIN_DB_SECRET", DEV_PLUGIN_DB_SECRET, requireSecure && Boolean(env["PLUGIN_DB_URL"])),
|
pluginDbSecret: resolvePluginDbSecret(env, requireSecure && Boolean(env["PLUGIN_DB_URL"])),
|
||||||
pluginDbUrl: readOptionalUrl(env, "PLUGIN_DB_URL"),
|
pluginDbUrl: readOptionalUrl(env, "PLUGIN_DB_URL"),
|
||||||
port: readPort(env),
|
port: readPort(env),
|
||||||
// Optional instant-revoke, off by default. When on, an admin deactivate/delete or permission
|
// Optional instant-revoke, off by default. When on, an admin deactivate/delete or permission
|
||||||
|
|||||||
@@ -3,9 +3,6 @@
|
|||||||
## Unfinnished work
|
## Unfinnished work
|
||||||
|
|
||||||
- [ ] Add a way to configure plugins directly when installing. **Decided: the manifest declares it, not an `.env`** — a declared schema is validatable at boot, so a missing or mistyped setting fails loud and named the way a stray `package.json` now does, and the picker/docs can be generated from the declaration. Open: where the operator *supplies* the values (env var per key, a `config/` file, or both), and whether a secret may be declared at all.
|
- [ ] Add a way to configure plugins directly when installing. **Decided: the manifest declares it, not an `.env`** — a declared schema is validatable at boot, so a missing or mistyped setting fails loud and named the way a stray `package.json` now does, and the picker/docs can be generated from the declaration. Open: where the operator *supplies* the values (env var per key, a `config/` file, or both), and whether a secret may be declared at all.
|
||||||
- [ ] A provisioned plugin database is invisible and unremovable. Nothing lists what exists, so an operator cannot see that `plugin_<id>` is there, nor that its plugin is gone — and uninstalling never drops it (deliberately, so data survives), leaving orphans with no supported way to clean them. Same shape as the uninstalled-plugin grant gap above. Sketch: a read-only "provisioned, but no installed plugin claims it" list plus a documented drop procedure.
|
|
||||||
- [ ] Decide who owns the connection ceiling for plugin storage. Each storage plugin opens its own pool against one Postgres, whose default `max_connections` is 100, and nothing warns as plugins are added. Either state a per-plugin pool ceiling in README → Plugin storage or record that the plugin owns it.
|
|
||||||
- [ ] E2E that a plugin's data actually survives a restart. `storage.test.ts` proves provisioning against a real Postgres (opt-in via `PLUGIN_DB_ADMIN_URL`), but no Playwright flow writes through a plugin page and reads it back after `docker compose restart web`.
|
|
||||||
- [ ] Rename the plugin "admin" to something less generic, like "auth-admin" or "users-groups-admin".
|
- [ ] Rename the plugin "admin" to something less generic, like "auth-admin" or "users-groups-admin".
|
||||||
- [ ] Guard the group paths against self-lockout, or accept them explicitly. The self-revoke guard covers only your own *direct* grants on the Users screen; unticking a permission on a group you belong to, removing yourself from that group, or deleting it can all still strip your own effective access with no warning. Recorded in AGENTS.md as a known gap — the robust fix is a "last effective holder" check, which needs a reverse Keto query.
|
- [ ] Guard the group paths against self-lockout, or accept them explicitly. The self-revoke guard covers only your own *direct* grants on the Users screen; unticking a permission on a group you belong to, removing yourself from that group, or deleting it can all still strip your own effective access with no warning. Recorded in AGENTS.md as a known gap — the robust fix is a "last effective holder" check, which needs a reverse Keto query.
|
||||||
- [ ] The permission picker has no concurrency baseline, so two operators editing the same user/group silently discard each other's change. Sketch: post the rendered set as a hidden baseline; if it no longer matches Keto, re-render with "this changed while you had the page open" rather than applying.
|
- [ ] The permission picker has no concurrency baseline, so two operators editing the same user/group silently discard each other's change. Sketch: post the rendered set as a hidden baseline; if it no longer matches Keto, re-render with "this changed while you had the page open" rather than applying.
|
||||||
|
|||||||
Reference in New Issue
Block a user