feat: connect planner via OAuth 2.1 / OIDC#32
Conversation
2c4d394 to
9826035
Compare
f302a42 to
5ff85f2
Compare
| smoke: | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 | ||
| with: | ||
| persist-credentials: false | ||
| - uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6.4.0 | ||
| with: | ||
| node-version-file: ".nvmrc" | ||
| - run: npm ci | ||
| - run: | | ||
| npm run prettier:check | ||
| npm run lint | ||
| npm run fallow |
There was a problem hiding this comment.
Maybe pull these into another pr to fix?
714f6dc to
215a38d
Compare
82dbb02 to
8e0c04b
Compare
|
@till, would you mind reviewing this one? |
till
left a comment
There was a problem hiding this comment.
I started, but I need to look more.
| smoke: | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 | ||
| with: | ||
| persist-credentials: false | ||
| - uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6.4.0 | ||
| with: | ||
| node-version-file: ".nvmrc" | ||
| - run: npm ci | ||
| - run: | | ||
| npm run prettier:check | ||
| npm run lint | ||
| npm run fallow |
There was a problem hiding this comment.
Maybe pull these into another pr to fix?
| - run: npm ci | ||
| - run: | | ||
| npm run prettier:check | ||
| npm run lint |
There was a problem hiding this comment.
I don't understand these changes.
There was a problem hiding this comment.
Yeah, that was really not well implemented.
I added an e2e test, but the workflow file took a beating in the process. I've added a commit, that makes the final diff read well.
Once review is complete, I'll rebase the confusion away.
e31a213 to
326ae91
Compare
1c48d90 to
430959a
Compare
6ad9514 to
6fe91f7
Compare
…d tests Core changes: - Configure Better Auth with jwt, oauthProvider, magicLink plugins - Route login flow through OAuth authorize endpoint with callbackURL - Add skipStateCookieCheck for cross-site redirect compatibility - Add database seeding for planner OAuth client (jsonb, idempotent, multi-URI) - Add heroku-release.sh running migrate.js on every deploy - Add dev helper for magic link capture in non-production - Add test helper with isolated PG schema per test Tests cover: provider seeding, authorize, token exchange with PKCE, JWKS verification, and full integration flow.
Adds a CI workflow with unit tests, coverage, and a Playwright e2e job. The e2e test covers the full magic link OAuth flow end-to-end.
cc6b9fe to
14c1035
Compare
|
I have deployed both PRs for I also reviewed the PRs against OWASP top 10 for 2025, and fixed the few minor things that surfaced. This unfortunately meant that I had to make some changes since I originally marked this as ready for review. I chose to rewrite the git history, in order to make the PRs easier to follow ... i.e. without any side quests. They should read fairly linearly now. Testing on Heroku To verify the OAuth 2.1 flow end-to-end: Flow A: Sign in with GitHub
Flow B: Sign in with Magic Link
If something goes wrongCheck the staging app logs: heroku logs --tail -a codebar-stagingOAuth errors from the planner side show as (codebar) Authentication failure! <error_type>. The auth |
till
left a comment
There was a problem hiding this comment.
This is a lot. But LMK, what you think, not hard no's on anything. I will have a look again.
|
|
||
| ## Overview | ||
|
|
||
| The auth app (`auth.codebar.io`) is an OAuth 2.1 / OIDC provider built with |
There was a problem hiding this comment.
It already serves profile, logout and I think link/unlink capabilities today. Those will stay around.
| */ | ||
|
|
||
| // ponytail: raw SQL because Better Auth doesn't expose a public API to | ||
| // create OAuth clients without admin auth. If the schema changes, update here. |
There was a problem hiding this comment.
Just curious, why would we not use admin-api?
| const error = c.req.query("error"); | ||
| const success = c.req.query("success"); | ||
| const redirectUrl = validateRedirectUrl(c.req.query("redirect_url"), ""); | ||
| function getCallbackURL(c) { |
There was a problem hiding this comment.
Maybe this should be somewhere else, like in the utlities. This will is pretty large already.
|
|
||
| app.route("/demo", demoHandler); | ||
|
|
||
| // Dev-only endpoint for Playwright tests to retrieve captured magic links |
There was a problem hiding this comment.
Similar, maybe spin them out to another file.
|
|
||
| if (!apiKey) { | ||
| console.log(`Magic Link for ${email}: ${url}`); | ||
| devMagicLinks.push({ |
There was a problem hiding this comment.
This should be more obvious, e.g. not rely on apiKey to be set (which could be a mistake), but instead use the same environment guard.
| export const AUTH_DEFAULT_PORT = 3001; | ||
| export const PLANNER_DEFAULT_PORT = 3000; | ||
|
|
||
| const parseRedirectUris = () => |
There was a problem hiding this comment.
Does the auth app have a staging? Otherwise, how would this work.
| const testInstance = await getTestInstance(); | ||
| const app = createApp(testInstance.auth, testInstance.db); | ||
|
|
||
| const params = new URLSearchParams({ |
There was a problem hiding this comment.
I'd put this into a helper and make it more descriptive. But the params repeats itself.
Maybe generally, I haven't look at it all in detail, but it could use some test helpers for all the tests.
| @@ -0,0 +1,17 @@ | |||
| import { defineConfig, devices } from "@playwright/test"; | |||
| // extra signed cookie check fails on Heroku/Cloudflare because the | ||
| // `__Secure-better-auth.state` cookie (SameSite=Lax) doesn't survive | ||
| // the cross-site redirect from GitHub back to the callback. | ||
| // This is a known pattern: https://github.com/better-auth/better-auth/issues/4969 |
There was a problem hiding this comment.
The issue is closed and from last year, is this even applicable?
| // Dev-only endpoint for Playwright tests to retrieve captured magic links | ||
| // Guarded by NODE_ENV check AND remote address — requests from non-local | ||
| // sources are rejected even if NODE_ENV isn't production (e.g. staging). | ||
| if (process.env.NODE_ENV !== "production") { |
There was a problem hiding this comment.
Could also fetch this in config and re-use it here. So the code is not using process.env everywhere?
Summary
Connect the auth service to the planner via OAuth 2.1 / OIDC using Better Auth's built-in
oauthProviderandjwtplugins. The planner acts as a first-party OAuth client.Commits
chore: add @better-auth/oauth-provider and @playwright/test dependencies@better-auth/oauth-providerruntime dep, bumpbetter-authto 1.6.20, add@playwright/testdev depfeat: add JWT and OAuth provider plugins with login flow, seeding, and testsjwt/oauthProvider/magicLinkplugins, OAuth authorize login flow, PKCE seeding, test infrastructure with schema isolation, 49 new unit/integration testsci: add CI pipeline and Playwright e2e test for OAuth flowdocs: add architecture documentation covering OAuth 2.1 flowsChanges
Auth configuration (
src/auth.js)jwt()plugin — issuesid_tokens withemailandnameclaims for the planner (RS256)oauthProvider()plugin — exposes OAuth 2.1 authorize/token endpoints (/api/auth/oauth2/*)account.skipStateCookieCheck: true— cross-site OAuth callback compatibilitytrustedOriginswith localhost + auth base URLConfig (
src/config.js)AUTH_DEFAULT_PORTandPLANNER_DEFAULT_PORTconstantsplanner_redirect_urisandallowed_redirectsfromPLANNER_REDIRECT_URISenv var (comma-separated, supports multiple environments)Login flow (
src/app/routes/auth.js,src/app/components/login.js)redirect_url-based flow withcallbackURLpointing to the OAuth authorize endpointshowLogin,showMagicLinkForm,sendMagicLink,startGitHubOAuth)getCallbackURLpreserves OAuth query params (PKCE, state, etc.) from the authorize requestcallbackURLDatabase seeding
src/app/db/seed-client.js— inserts the planner OAuth client via raw SQL withON CONFLICT DO UPDATEfor idempotency. Accepts redirect URIs as JSON array via$1::jsonb. Validates schemaName to prevent SQL injection.scripts/migrate.js— runs migrations and seeds the planner clientscripts/heroku-release.sh— runsnode scripts/migrate.json every deployDev test helpers
src/dev/magic-links.js— captures sent magic links whenSENDGRID_API_KEYis unsetsrc/app/app.js— exposes/api/test/magic-linksGET/DELETE in non-production, magic link endpoint localhost-restrictedCI
e2ejob running Playwright after unit testschromium-headless-shellfor smaller downloadpermissions: contents: readat workflow levelDocumentation
docs/architecture.md— architecture overview with sequence diagrams, environment layout, env vars, and operational detailsTesting
158 unit tests, 0 failures. New tests cover:
redirect_uri