Add OAuth login (PKCE) with bearer-auth API calls

[lukaskroepfl]

Summary

• Adds bitmovin login and bitmovin logout. Login runs a browser-based OAuth flow (Authorization Code + PKCE), captures the callback on a fixed loopback port, and stores access + refresh tokens in the config file.
• Subsequent commands construct the SDK with Authorization: Bearer … and refresh the access token silently when it nears expiry; the user only re-authenticates when the refresh token itself stops working.
• Credential resolution order is now: --api-key flag > BITMOVIN_API_KEY env var > stored OAuth session > api-key in config. Config file is written with mode 0600.

Notes

• IdP defaults are env-overridable: BITMOVIN_OAUTH_ISSUER, BITMOVIN_OAUTH_CLIENT_ID, BITMOVIN_OAUTH_AUTHORIZE_URL, BITMOVIN_OAUTH_TOKEN_URL, BITMOVIN_OAUTH_SCOPE, BITMOVIN_OAUTH_REDIRECT_PORT.
• config show now reports the OAuth user, expiry, and whether a refresh token is present (text + --json).
• bitmovin login --print-url skips opening a browser (useful over SSH).

Test plan

• npm run lint && npm test — 184 tests passing locally
• bitmovin login opens browser, completes auth, stores session
• bitmovin account info works using bearer auth from the stored session
• Tamper with expiresAt in config (or wait) → next command silently refreshes
• bitmovin logout clears OAuth, preserves any api-key fallback
• --api-key flag / BITMOVIN_API_KEY still override OAuth

🤖 Generated with Claude Code

[mateun]

Review notes — happy to chat about any of these.

Strong suggestions

1. Refresh tokens stored in plaintext JSON — src/lib/config.ts:36-41

chmod 0600 is the right minimum, but plaintext refresh tokens on disk is a step down from the standard CLIs we're competing with (gh, aws, gcloud use OS keychains: macOS Keychain, Windows Credential Manager, libsecret on Linux). For a CLI marketed as "OAuth recommended", this is a reasonable thing to do as a follow-up — but worth at least filing as a known-limitation issue before shipping, and mentioning in the README that the session is on-disk.

Also, there's a small TOCTOU window: if the file already existed with 0644, writeFileSync keeps that mode, then chmodSync tightens it. An attacker on the same machine could read in between. Switch to a temp-file-then-rename pattern, or fs.openSync with {mode: 0o600} followed by writeSync/closeSync, to remove the window.

2. SDK bearer-token override is fragile — src/lib/client.ts:69-78

apiKey: 'oauth',                   // placeholder so SDK validation passes
headers: { 'X-Api-Key': '', Authorization: `Bearer ${session.accessToken}` },
fetch: createBearerFetch(),        // strips X-Api-Key on the way out

This works today but silently breaks the next time the SDK changes how it builds requests (e.g. if it stops respecting a caller-supplied fetch, or starts setting X-Api-Key later in the pipeline). At minimum: add a test that asserts the outgoing request has Authorization: Bearer … and no X-Api-Key header. Better: open an issue on the SDK to support bearer auth natively, and link it from the comment.

3. No timeout on the loopback callback server — src/lib/oauth.ts:74-122

If the user closes the browser tab without completing auth, bitmovin login hangs indefinitely with no way out except Ctrl+C. Add a timeout (3–5 min seems standard) that rejects the result promise and closes the server. Also worth wiring up a SIGINT handler so Ctrl+C closes the listener cleanly rather than leaving the port lingering — relevant because the port is fixed (27315) and "free the port" is in the error message at line 127.

4. Windows cmd /c start \"\" <url> will mangle URLs containing & — src/lib/oauth.ts:58-62

OAuth authorize URLs always contain & between query params. Without shell: true, start may interpret them as command separators on Windows. Either set shell: true and quote the URL, or pull in a small dependency like open that handles this correctly across platforms. (Bonus: open also handles WSL and the various Linux quirks.)

Nits

5. Dead branch in login.ts — src/commands/login.ts:31-34

if (!urlAnnounced) {
  this.log('Login completed.');
}

runLoginFlow always invokes onAuthorizeUrl in the current implementation, so this branch never runs. Either remove it or move the announce-side-effect into the command so the dead-code intent is clearer.

6. renderPage body parameter is not escaped — src/lib/oauth.ts:130-135

The title argument is run through escapeHtml but body is interpolated raw — every caller must remember to escape. Today all callers do, so this is a footgun rather than a bug. Either escape inside renderPage, or change the signature to take bodyText: string so escaping is centralized.

7. ID token signature isn't verified — src/lib/oauth.ts:262-274

The decoded email/sub is display-only and never used for auth decisions, so this is acceptable — but add a one-line comment that says so explicitly, so a future reader doesn't accidentally start gating behavior on these claims.

8. README env-var list is missing BITMOVIN_OAUTH_SCOPE — README.md:32-37

The list mentions ISSUER, CLIENT_ID, REDIRECT_PORT, AUTHORIZE_URL, TOKEN_URL but omits BITMOVIN_OAUTH_SCOPE, which is supported in resolveEndpoints. Either drop the env var or list it.

9. Token-type not validated — src/lib/oauth.ts:188

We send Authorization: Bearer … unconditionally, even if the IdP responds with token_type: "DPoP" or similar. A defensive check that throws a clear error ("Unsupported token type: …") would save a confusing 401-loop later. Low risk in practice — the configured IdP returns Bearer — but cheap to add.

Question, not blocking

10. Concurrent CLI invocations & token rotation: if a user runs two bitmovin … commands at the same time and both see an expired access token, both will hit the refresh endpoint. With rotating refresh tokens, one of them will end up invalidated. Probably rare enough to accept, but worth a sentence in the PR description acknowledging the trade-off, or a follow-up issue.

[lukaskroepfl]

Thanks for the thorough review @mateun — addressed almost all of it in 1ca649e. Per-point notes:

Strong

1. Refresh tokens / TOCTOU — Replaced writeFileSync → chmodSync with an atomic openSync('w', 0o600) → writeSync → renameSync pattern, so the file's first inode never exists with a wider mode. Tmp file is cleaned up if rename fails. README now flags that the session lives on disk as JSON and that keychain integration is a follow-up.
2. Bearer SDK override — Added test/lib/client-bearer.test.ts: it spies on the global fetch and asserts the outgoing request carries Authorization: Bearer … and no X-Api-Key. Will flag if the SDK ever changes how it composes request headers.
3. No timeout on loopback — 5-min auth timeout via Promise.race; process.once('SIGINT', …) closes the server and exits 130 so the fixed port doesn't leak. Both are torn down in the finally.
4. Windows URL mangling — Swapped the hand-rolled spawn('cmd', ['/c', 'start', '""', url]) for the open package, which handles & quoting on Windows and the WSL / xdg-open quirks correctly.

Nits
5. Dead branch — gone; also wired --print-url through to runLoginFlow({noOpenBrowser: true}) so the flag now actually skips the browser spawn.
6. renderPage body escaping — body is now escapeHtmld inside renderPage. Callers updated.
7. ID-token signature — added an explicit comment on decodeIdTokenClaims that the claims are display-only and that gating behavior on them would require verifying the signature.
8. BITMOVIN_OAUTH_SCOPE in README — listed.
9. Token-type validation — tokenResponseToSession now throws "Unsupported OAuth token_type …" if the IdP returns anything other than bearer.

Question (10) — concurrent refresh — agreed it's possible and worth tracking, but didn't fix in this PR. Easiest mitigation is a per-config-file lockfile around the refresh-then-save step. Will open a follow-up issue.

Builds clean, 187 tests passing locally (+3). Ready for another look.