feat(auth): QR device-authorization sign-in for shared control-room computer (ADR 0008)#1495
Open
jakebromberg wants to merge 4 commits into
Open
feat(auth): QR device-authorization sign-in for shared control-room computer (ADR 0008)#1495jakebromberg wants to merge 4 commits into
jakebromberg wants to merge 4 commits into
Conversation
6 tasks
Schema constraint shape reportdata-shape report errored (exit 0): node:internal/modules/runmain:107 triggerUncaughtException( ^ Error ERRMODULENOTFOUND: Cannot find package 'postgres' imported from /home/runner/work/Backend-Service/Backend-Service/scripts/schema-shape-report.mjs Did you mean to import "postgres/cjs/src/index.js"? at Object.getPackageJ; manual check required |
9 tasks
Member
Author
|
Followup for the wxyc-shared OpenAPI mirror: WXYC/wxyc-shared#195. |
…om computer (ADR 0008) Stand up better-auth's `device-authorization` plugin (RFC 8628) so the iOS app can authorize a 12h browser session at dj.wxyc.org without typing a password on the shared keyboard. Browser POSTs /auth/device/code → renders the QR + plaintext user_code → polls /auth/device/token. The DJ scans, the iOS app POSTs /auth/device/approve carrying its session, the browser's next poll returns the session. Two hooks live in shared/authentication/src/device-authorization.ts so they're directly unit-testable: `applyDeviceApproveRoleGate` rejects `member`-role (or no-membership) approvers with `access_denied` before the plugin flips the device-code row, and `applyDeviceTokenSessionTtl` clamps the just-created session's expiresAt and rewrites the response body's `expires_in` to 12h. /device/deny stays ungated — the plugin's userId match already prevents cross-user denial; the role gate would duplicate logic without security gain. /auth/device/code, /auth/device/approve, /auth/device/deny ride the existing authMutationRateLimit (10/15min per-IP); /auth/device/token is intentionally excluded so it doesn't shadow the plugin's RFC 8628 slow_down JSON. Migration 0106 adds the `auth_device_code` table verbatim from the plugin's schema contract — id PK, device_code/user_code unique, user_id nullable FK to auth_user with ON DELETE cascade. Verification URI is https://dj.wxyc.org/device-auth so a universal-link fallback stays open later; iOS reads user_code directly from the QR payload.
…ream zod parse doesn't throw
`deviceAuthorizationOptionsSchema` declares the `schema` field as
`z.custom(() => true)` with NO `.optional()` (see
node_modules/better-auth/dist/plugins/device-authorization/index.mjs:28).
At TypeScript level the field types as `unknown` so omitting it
compiles; at runtime `parse()` throws a `ZodError: expected nonoptional,
received undefined` and the auth-service crashes at module load. Local
typecheck and unit tests miss it because the unit tests mock
`better-auth/api` (they never instantiate the real plugin) and tsc
honors the looser type.
Pass `schema: {}` so `mergeSchema(schema, options?.schema)` folds in
the plugin's default field map verbatim. Verified by booting the
built dist locally — module loads clean instead of crashing on the
ZodError that took out CI Integration-Tests.
… before approve/deny in integration test Two coupled bugs the unit suite couldn't see; both surfaced when CI's Integration-Tests run against the real Docker stack. 1. The before-hook read `ctx.context.session?.user` but the session is NOT pre-populated on /device/approve — the plugin's handler resolves it itself via `getSessionFromCtx` (see node_modules/better-auth/dist/plugins/device-authorization/routes.mjs deviceApprove). My early-return on missing session no-op'd the role gate every time, so a member's request reached the plugin handler and got 400 device_code_not_claimed instead of my intended 403 access_denied. Resolve the session inside the hook with `getSessionFromCtx(ctx)` so the role lookup runs against the same session the handler will see. 2. The integration spec called /device/approve and /device/deny directly without claiming via GET /device first. The plugin enforces `deviceCodeRecord.userId` set before either route accepts the row (routes.mjs:422 + 493), so all three specs were 400 instead of 200/403. Added a `claimDeviceCode` helper that calls GET /device?user_code=… with the session cookie and wired it into all three flows. Also updated the member-denied test's narrator comment: the member's claim DOES set userId on the row, so the DJ's re-claim is a no-op afterwards (the member effectively burns the user_code for that QR run; the DJ scans a fresh one). Acceptable v1 behavior — control-room context, not adversarial.
jakebromberg
force-pushed
the
feature/qr-device-authorization
branch
from
28712c0 to
21af77f
Compare
…h doesn't trip pollingInterval The happy-path spec called /device/token twice — once before approve to assert authorization_pending, once after to assert the 12h session. The plugin's deviceToken handler stamps `lastPolledAt` on every call and returns `slow_down` (400) on any poll within `interval` (5s, RFC 8628 — routes.mjs:201). The second call landed ~1s after the first, so CI saw 400 instead of 200. Lift the pre-approval poll into its own test against a fresh device_code row. Each test now makes exactly one /device/token call, so `lastPolledAt` never gates its own assertion. `afterEach` truncates auth_device_code between specs so cross-test polling rate-limiting can't apply either.
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
1 participant
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
device-authorizationplugin (RFC 8628) so the iOS app can authorize a 12h browser session at dj.wxyc.org without typing a password on the shared control-room keyboard.deviceCodetable; gates/device/approveto roles ≥dj; clamps device-auth sessions to 12h via anafter-hook (password sign-in unaffected); rate-limits/device/code+/device/approve+/device/deny(intentionally NOT/device/token— the plugin enforcespollingIntervalitself)./device/approve+/device/deny; the ADR's informal/device/verifydoesn't exist).Closes #1494.
Surface
POST /auth/device/codeuser_codenamespace.POST /auth/device/tokenpollingIntervalserver-side vialastPolledAt→slow_downJSON. An HTTP 429 here would shadow it.GET /auth/device?user_code=…POST /auth/device/approvedjhooks.beforerejectsmemberwithaccess_deniedbefore the row flips.POST /auth/device/denyuserIdmatch check is sufficient defense; no role gate.Where the hooks live
shared/authentication/src/device-authorization.ts(new) holds two pure helpers —applyDeviceApproveRoleGate(userId, selectMemberRole)andapplyDeviceTokenSessionTtl(token, body, now, updateSessionExpiry)— that take their DB capability as a thunk so the unit tests don't need to stand upcreateAuthMiddleware.auth.definition.ts's middleware wrappers readctx.context.session.user.id/ctx.context.newSession.session.token/ctx.context.returnedand pass them through.The 12h TTL is a hardcoded
DEVICE_SESSION_TTL_MSconstant — product decision, not configuration. Ops can drop it in an incident by editing the constant.Out of scope (called out in #1494)
WXYC/dj-site.LAContext.evaluatePolicy—WXYC/wxyc-dj-tool-ios.WXYC/wxyc-sharedPR againstapi.yaml.Test plan
npm run typecheck— clean across all workspaces.npm run lint— no errors on touched files (only pre-existing warnings elsewhere).npm run format:check— clean.node scripts/validate-migrations.mjs— 104 entries, 0 warnings.npm run test:unit -- --testPathPatterns=device-authorization— 10/10 (4 role-gate cases ×dj/musicDirector/stationManager/member/ no-membership / non-WXYCRole; 4 session-TTL cases for the constant, body mutation, target token, and expiry math).npm run test:unit -- --testPathPatterns="authentication|auth\\.middleware"— 119/119, no regressions in adjacent auth suites.npm run ci:testmock— Docker integration suite (coverstests/integration/device-authorization.spec.js: happy path withexpires_in ≈ 43200, member-denied path with code remainingpending, deny path with subsequentaccess_denied).