diff --git a/docs/runbooks/first-user-walk.md b/docs/runbooks/first-user-walk.md index 2724dd569..68ff71d21 100644 --- a/docs/runbooks/first-user-walk.md +++ b/docs/runbooks/first-user-walk.md @@ -18,11 +18,33 @@ rename it without changing the exclusion list too. ## Run it +### Prod: sign in to the existing test account + +Prod has no signup for the walk. The run signs in to one existing test account +and runs the six covered steps (login, first pod, hosted install, hosted reply, +invite preview, and the BYO step that reports not-covered). It removes only the +pod, seat and agent it made, and it never deletes the account. + ```bash -COMMONLY_WALK_PASSWORD="$(openssl rand -base64 24)" \ +COMMONLY_WALK_EMAIL= \ +COMMONLY_WALK_PASSWORD="" \ node scripts/first-user-walk.mjs --base https://api.commonly.me ``` +- Without `COMMONLY_WALK_EMAIL` a prod run is refused before any request. +- The manifest records `"mode": "signin-prod"`. The result file has no + `signup` or `signup-workspace` steps. + +### Signup (non-prod targets with no account given) + +Prod never signs up. A prod base without `COMMONLY_WALK_EMAIL` is refused before +any request, so this form is for other targets only: + +```bash +COMMONLY_WALK_PASSWORD="$(openssl rand -base64 24)" \ + node scripts/first-user-walk.mjs --base https://api-dev.example.test +``` + - Generate a fresh password per run. The script reads it from the environment and never writes it anywhere. Do not put a credential in the command you paste into a pod, a PR or a task row. @@ -45,8 +67,10 @@ COMMONLY_WALK_EMAIL= COMMONLY_WALK_PASSWORD= - The run signs in and never signs up. It removes only the pods, seats and agents it created, and it never deletes the shared account. - `COMMONLY_WALK_EMAIL` aimed at any host other than staging is refused before - the walk starts, so the shared account cannot be used on prod. -- The manifest records `"mode": "signin"` for these runs. + the walk starts. Prod uses the same variable name, so keep staging credentials + out of a shell that runs the walk against prod. +- The manifest records `"mode": "signin"` for these runs. Prod runs record + `"mode": "signin-prod"`. ## Cleanup is part of the run @@ -88,7 +112,7 @@ Write is atomic (temp file, then rename), so a reader never sees a half file. "schema": 1, "sha": "", "startedAt": "...", "finishedAt": "...", - "base": "https://api.commonly.me", "mode": "signup", + "base": "https://api.commonly.me", "mode": "signin-prod", "walk": "walk-", "overall": "pass | fail | incomplete", "exitCode": 0, @@ -103,7 +127,7 @@ Write is atomic (temp file, then rename), so a reader never sees a half file. (Bring your own connect). Today every full run is `incomplete`, never `pass`, until that step is built. Roll back on `fail` only; treat `incomplete` as not-green but not as a regression. -- `mode` is `signup` or `signin`, or `not-chosen` when the run was refused before a mode was accepted (no request was made). +- `mode` is `signup`, `signin` (staging) or `signin-prod`, or `not-chosen` when the run was refused before a mode was accepted (no request was made). - `cleanup.status` is `not-run` (and `ok` null) when the run stopped before cleanup could start, for example a refused mode. - The file holds no password, token or email. A `needs-admin-purge` cleanup points SRE at the manifest in `~/.commonly/state/walk-706/`. diff --git a/scripts/first-user-walk-mode.mjs b/scripts/first-user-walk-mode.mjs index 17baa8301..f8ead15a4 100644 --- a/scripts/first-user-walk-mode.mjs +++ b/scripts/first-user-walk-mode.mjs @@ -1,18 +1,28 @@ -// How the walk gets its account (TASK-706). Prod signs up a fresh account every -// run. Staging needs a verified email, which a walk address cannot receive, so -// SRE pre-verifies one shared walk account there and the walk signs in to it -// instead. Its credentials come from env only, and that account is never deleted. -// Exact host and https only: a substring match would send the shared password to -// any host whose name contains "staging". +// How the walk gets its account (TASK-706, TASK-729). Staging needs a verified +// email, which a walk address cannot receive, so SRE pre-verifies one shared walk +// account there and the walk signs in to it instead. Prod has no signup for the +// walk: it signs in to an existing test account, and that account is never +// deleted. Signup runs only on non-prod targets with no account given. +// Credentials come from env only. Exact host and https only: a substring match +// would send a shared password to any host whose name contains "staging". const STAGING_HOSTS = new Set(['api-staging.commonly.me']); +const PROD_HOSTS = new Set(['api.commonly.me']); // `deleteAccount` is the one value main() hands to cleanup, so the mode decides -// the account's fate here and nowhere else. +// the account's fate here and nowhere else. The password is always COMMONLY_WALK_PASSWORD. export const chooseWalkMode = ({ base, env }) => { + const url = new URL(base); + // A trailing dot names the same host but would slip past the exact-host checks below. + const host = url.hostname.replace(/\.$/, ''); const email = env.COMMONLY_WALK_EMAIL || ''; + // Checked before the prod branch: over plain http the signup path would send the password in cleartext. + if (PROD_HOSTS.has(host) && url.protocol !== 'https:') throw new Error('prod walk requires https; refusing before any request'); + if (url.protocol === 'https:' && PROD_HOSTS.has(host)) { + if (!email) throw new Error('prod has no walk signup: set COMMONLY_WALK_EMAIL to an existing test account'); + return { mode: 'signin-prod', email, deleteAccount: false }; + } if (!email) return { mode: 'signup', deleteAccount: true }; - const url = new URL(base); - if (url.protocol !== 'https:' || !STAGING_HOSTS.has(url.hostname)) { + if (url.protocol !== 'https:' || !STAGING_HOSTS.has(host)) { throw new Error('COMMONLY_WALK_EMAIL is set but the target is not the staging API over https; the shared sign-in account exists on staging only'); } return { mode: 'signin', email, deleteAccount: false }; diff --git a/scripts/first-user-walk-mode.test.mjs b/scripts/first-user-walk-mode.test.mjs index 363fa3041..a2870573e 100644 --- a/scripts/first-user-walk-mode.test.mjs +++ b/scripts/first-user-walk-mode.test.mjs @@ -10,15 +10,19 @@ test('staging with COMMONLY_WALK_EMAIL takes the sign-in path', () => { assert.deepEqual(chooseWalkMode({ base: STAGING, env: { COMMONLY_WALK_EMAIL: 'walk@commonly.me' } }), { mode: 'signin', email: 'walk@commonly.me', deleteAccount: false }); }); -test('no COMMONLY_WALK_EMAIL signs up a fresh account, on any target, and that account is deleted', () => { - assert.deepEqual(chooseWalkMode({ base: PROD, env: {} }), { mode: 'signup', deleteAccount: true }); +test('no COMMONLY_WALK_EMAIL signs up a fresh account on staging, and that account is deleted', () => { assert.deepEqual(chooseWalkMode({ base: STAGING, env: {} }), { mode: 'signup', deleteAccount: true }); }); -test('the shared sign-in account is refused on prod', () => { - assert.throws(() => chooseWalkMode({ base: PROD, env: { COMMONLY_WALK_EMAIL: 'walk@commonly.me' } }), /staging only/); +test('prod never signs up: it signs in to the existing account from env, which is never deleted', () => { + assert.deepEqual(chooseWalkMode({ base: PROD, env: { COMMONLY_WALK_EMAIL: 'walk-test@commonly.me' } }), { mode: 'signin-prod', email: 'walk-test@commonly.me', deleteAccount: false }); }); +test('prod without an existing test account is refused, not signed up', () => { + assert.throws(() => chooseWalkMode({ base: PROD, env: {} }), /no walk signup/); +}); + + test('a host that merely contains "staging" never receives the shared account', () => { for (const base of ['https://staging.attacker.example', 'https://api-staging.commonly.me.attacker.example', 'https://evil-staging.example']) { assert.throws(() => chooseWalkMode({ base, env: { COMMONLY_WALK_EMAIL: 'walk@commonly.me' } }), /staging API over https/, base); diff --git a/scripts/first-user-walk.mjs b/scripts/first-user-walk.mjs index c9a1b8f25..14e43a08e 100644 --- a/scripts/first-user-walk.mjs +++ b/scripts/first-user-walk.mjs @@ -20,7 +20,13 @@ * token or the email. * * Usage: - * COMMONLY_WALK_PASSWORD=... node scripts/first-user-walk.mjs [--base https://api.commonly.me] + * COMMONLY_WALK_EMAIL=... COMMONLY_WALK_PASSWORD=... \ + * node scripts/first-user-walk.mjs [--base https://api.commonly.me] + * + * Prod: no signup. The run signs in to one existing test account (the email and + * password above, from env only), runs the six covered steps, removes only what it + * created, and never deletes that account. Without COMMONLY_WALK_EMAIL a prod run + * is refused before any request. * * Staging: signup needs a verified email, so SRE's shared walk account is used * instead. Set COMMONLY_WALK_EMAIL and COMMONLY_WALK_PASSWORD from env (never a @@ -241,9 +247,9 @@ export const runWalk = async ({ // Starts false: a cleanup that throws must not be reported as ok. let cleanupOk = false; try { - expect(password, 'COMMONLY_WALK_PASSWORD is not set'); walkMode = chooseWalkMode({ base: BASE, env }); - if (walkMode.mode === 'signin') email = walkMode.email; + expect(password, 'COMMONLY_WALK_PASSWORD is not set'); + if (walkMode.email) email = walkMode.email; manifest.mode = walkMode.mode; let token = null; diff --git a/scripts/first-user-walk.test.mjs b/scripts/first-user-walk.test.mjs index eebdc48f4..931835ff2 100644 --- a/scripts/first-user-walk.test.mjs +++ b/scripts/first-user-walk.test.mjs @@ -23,7 +23,7 @@ const SHARED_EMAIL = 'walk-shared@commonly.test'; const fakeBackend = ({ preview = 200, create = 'ok', password = PASSWORD, echoRegister = false, seeded = false, } = {}) => { - const state = { accountExists: seeded, pods: [], lastAgent: null, calls: [] }; + const state = { accountExists: seeded, pods: [], lastAgent: null, calls: [], loginEmails: [] }; const reply = (status, data) => ({ status, json: async () => data }); const fetchImpl = async (url, init = {}) => { const u = new URL(url); @@ -40,6 +40,7 @@ const fakeBackend = ({ return reply(201, {}); } if (method === 'POST' && p === '/api/auth/login') { + state.loginEmails.push(body.email); if (state.accountExists && body.password === password) { return reply(200, { token: TOKEN, user: { _id: USER_ID, role: 'user' } }); } @@ -95,7 +96,8 @@ const dirs = () => { return { stateDir: path.join(root, 'state'), resultPath: path.join(root, 'result', 'result.json') }; }; -const run = (fetchImpl, { stateDir, resultPath }, { password = PASSWORD, env = {}, base = 'https://api.commonly.me' } = {}) => runWalk({ +// Non-prod, non-staging by default: a run with no account given signs up, as the signup tests expect. +const run = (fetchImpl, { stateDir, resultPath }, { password = PASSWORD, env = {}, base = 'https://api-dev.commonly.test' } = {}) => runWalk({ base, env: { COMMONLY_WALK_PASSWORD: password, ...env }, fetchImpl, @@ -198,6 +200,50 @@ test('signin mode never deletes the shared account, and the result says so', asy assert.equal(result.steps.some((s) => s.name === 'signup-workspace'), false); }); +test('prod signs in to the existing test account: no signup, the six covered steps, the account is never deleted', async () => { + const dirsFor = dirs(); + const { fetchImpl, state } = fakeBackend({ seeded: true }); + const { exitCode, result } = await run(fetchImpl, dirsFor, { + base: 'https://api.commonly.me', + env: { COMMONLY_WALK_EMAIL: 'walk-test@commonly.test' }, + }); + + assert.equal(result.mode, 'signin-prod'); + assert.equal(exitCode, 2); + assert.equal(result.overall, 'incomplete'); + assert.deepEqual(result.steps.map((st) => st.name), ['login', 'first-pod', 'hosted-install', 'hosted-reply', 'invite-preview-api', 'byo-connect']); + assert.equal(state.calls.includes('POST /api/auth/register'), false); + assert.equal(state.calls.includes('DELETE /api/users/me'), false); + assert.equal(state.accountExists, true); + assert.equal(state.pods.length, 0); + assert.equal(result.cleanup.ok, true); + // The run signs in with the env email, and that email never reaches the result or manifest. + assert.deepEqual(state.loginEmails, ['walk-test@commonly.test']); + assert.equal(readFileSync(dirsFor.resultPath, 'utf8').includes('walk-test@commonly.test'), false); + const [manifestFile] = readdirSync(dirsFor.stateDir); + assert.equal(readFileSync(path.join(dirsFor.stateDir, manifestFile), 'utf8').includes('walk-test@commonly.test'), false); +}); + +test('a trailing dot on the prod host is still prod: refused without an account, never signed up', async () => { + const dirsFor = dirs(); + const { fetchImpl, state } = fakeBackend(); + const { exitCode } = await run(fetchImpl, dirsFor, { base: 'https://api.commonly.me.' }); + + assert.equal(exitCode, 1); + assert.equal(state.calls.includes('POST /api/auth/register'), false); + assert.equal(state.calls.length, 0); +}); + +test('prod without an existing test account is refused before any request', async () => { + const dirsFor = dirs(); + const { fetchImpl, state } = fakeBackend({ seeded: true }); + const { exitCode, result } = await run(fetchImpl, dirsFor, { base: 'https://api.commonly.me' }); + + assert.equal(exitCode, 1); + assert.equal(result.mode, 'not-chosen'); + assert.equal(state.calls.length, 0); +}); + test('a refused mode (shared email aimed at prod) makes no request, is not reported as signup, and exits 1', async () => { const dirsFor = dirs(); const { fetchImpl, state } = fakeBackend({ seeded: true }); @@ -246,7 +292,7 @@ test('a missing password fails before any request, reports cleanup as not-run, a const counting = (url, init) => { requests += 1; return fetchImpl(url, init); }; const { exitCode, result } = await runWalk({ base: 'https://api.commonly.me', - env: {}, + env: { COMMONLY_WALK_EMAIL: 'walk-test@commonly.test' }, fetchImpl: counting, ...dirsFor, sleep: async () => {}, @@ -255,11 +301,22 @@ test('a missing password fails before any request, reports cleanup as not-run, a }); assert.equal(exitCode, 1); assert.equal(result.overall, 'fail'); + assert.equal(result.mode, 'signin-prod'); assert.equal(result.cleanup.status, 'not-run'); assert.equal(requests, 0); assert.equal(existsSync(dirsFor.resultPath), true); }); +test('prod over plain http is refused before any request, so the password never goes out in cleartext', async () => { + const dirsFor = dirs(); + const { fetchImpl, state } = fakeBackend(); + const { exitCode, result } = await run(fetchImpl, dirsFor, { base: 'http://api.commonly.me' }); + + assert.equal(exitCode, 1); + assert.equal(result.mode, 'not-chosen'); + assert.equal(state.calls.length, 0); +}); + test('a create that throws, with no match in the list, is recorded as unresolved and needs-admin-purge', async () => { const dirsFor = dirs(); const { fetchImpl, state } = fakeBackend({ create: 'throw' });