diff --git a/.changeset/device-authorization-decision-race.md b/.changeset/device-authorization-decision-race.md new file mode 100644 index 0000000000..6147e0054c --- /dev/null +++ b/.changeset/device-authorization-decision-race.md @@ -0,0 +1,7 @@ +--- +"better-auth": minor +--- + +When the same device code is approved and denied at the same time, for example from two open approval pages, only the first decision now takes effect, and the other request fails with `invalid_request` and "Device code already processed". Previously, both requests reported success and the later one replaced the first, so a denied device could still receive tokens. + +The `/device/approve` error type no longer lists `device_code_already_processed`. The server never returned that code; an already decided code has always returned `invalid_request`. TypeScript code that compares against `device_code_already_processed` must check `invalid_request` instead. diff --git a/.changeset/device-authorization-expired-polling.md b/.changeset/device-authorization-expired-polling.md new file mode 100644 index 0000000000..33bc3ab7a7 --- /dev/null +++ b/.changeset/device-authorization-expired-polling.md @@ -0,0 +1,5 @@ +--- +"better-auth": patch +--- + +Device clients that poll `/device/token` or `/oauth2/token` too soon after their device code expires now receive `expired_token` right away. Previously they received `slow_down` and learned that the code had expired only on a later poll. diff --git a/docs/content/docs/plugins/device-authorization.mdx b/docs/content/docs/plugins/device-authorization.mdx index 8457f7f036..bd757ec248 100644 --- a/docs/content/docs/plugins/device-authorization.mdx +++ b/docs/content/docs/plugins/device-authorization.mdx @@ -319,7 +319,10 @@ After displaying the user code, poll for the Better Auth session token. The resp Example polling implementation: ```ts -let pollingInterval = 5; // Start with 5 seconds +// `data` is the response from `authClient.device.code()` above +if (!data) throw new Error("Could not start device authorization"); +const { device_code, interval } = data; +let pollingInterval = interval; // Seconds const pollForToken = async () => { const { data, error } = await authClient.device.token({ grant_type: "urn:ietf:params:oauth:grant-type:device_code", @@ -363,11 +366,11 @@ pollForToken(); The user authorization flow requires two steps: -1. **Code Verification**: Validate the user code via `GET /device`. The verification request claims the pending device code for the calling session. -2. **Authorization**: The session that claimed the code can approve or deny it. +1. **Code Verification**: Validate the user code via `GET /device`. The verification request claims the pending device code for the signed-in user. +2. **Authorization**: The user who claimed the code can approve or deny it. - Users must be authenticated when calling `GET /device`, because the verification step binds the pending device code to that session. Only the same session can later approve or deny. If the user is not authenticated when entering the code, redirect them to the login page with a return URL and re-call `GET /device` after sign-in. + Users must be authenticated when calling `GET /device`, because the verification step binds the pending device code to the signed-in user. Only that user can later approve or deny it. If the user is not authenticated when entering the code, redirect them to the login page with a return URL and re-call `GET /device` after sign-in. When the OAuth Provider integration is enabled, `GET /device` also returns the approved resource context to the authenticated user who owns the request. Show that context in the approval UI alongside the client and scopes. Standalone Device Authorization does not return an RFC 8707 `resource` field. @@ -427,7 +430,14 @@ export default function DeviceAuthorizationPage() { ### Approving or Denying Device -Users must be authenticated to approve or deny device authorization requests: +Users must be authenticated to approve or deny device authorization requests. Only the user who claimed the code with `GET /device`, or the user it was pre-bound to with `user_id`, can decide it, and each code accepts one decision. Approve and deny return one of these errors instead of `success: true`: + +| Error Code | When | +| ----------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | +| `unauthorized` | No user is signed in | +| `invalid_request` | The user code is unknown, has no user yet (it was neither claimed with `GET /device` nor pre-bound with `user_id`), or already has a decision ("Device code already processed"), including from a concurrent approve or deny that finished first | +| `expired_token` | The code has expired | +| `access_denied` | The code belongs to another user | #### Approve Device @@ -479,30 +489,35 @@ export default function DeviceApprovalPage() { const handleApprove = async () => { setIsProcessing(true); try { - await authClient.device.approve({ - userCode: userCode, - }); - // Show success message + const { error } = await authClient.device.approve({ userCode }); + if (error) { + alert(`Failed to approve device: ${error.error_description}`); + return; + } alert("Device approved successfully!"); window.location.href = "/"; - } catch (error) { - alert("Failed to approve device"); + } catch { + alert("Could not reach the server. Try again."); + } finally { + setIsProcessing(false); } - setIsProcessing(false); }; const handleDeny = async () => { setIsProcessing(true); try { - await authClient.device.deny({ - userCode: userCode, - }); + const { error } = await authClient.device.deny({ userCode }); + if (error) { + alert(`Failed to deny device: ${error.error_description}`); + return; + } alert("Device denied"); window.location.href = "/"; - } catch (error) { - alert("Failed to deny device"); + } catch { + alert("Could not reach the server. Try again."); + } finally { + setIsProcessing(false); } - setIsProcessing(false); }; if (!user) { @@ -591,13 +606,13 @@ Device-code issuance makes up to 3 attempts to overcome unique-key collisions. I The device flow defines specific error codes: -| Error Code | Description | -| ----------------------- | ------------------------------------------- | -| `authorization_pending` | User hasn't approved yet (continue polling) | -| `slow_down` | Polling too frequently (increase interval) | -| `expired_token` | Device code has expired | -| `access_denied` | User denied the authorization | -| `invalid_grant` | Invalid device code or client ID | +| Error Code | Description | +| ----------------------- | ----------------------------------------------------------------------------- | +| `authorization_pending` | User hasn't approved yet (continue polling) | +| `slow_down` | Polled too soon (add 5 seconds to the interval for this and every later poll) | +| `expired_token` | Device code has expired | +| `access_denied` | User denied the authorization | +| `invalid_grant` | Invalid device code or client ID | ## Example: CLI Application @@ -733,12 +748,12 @@ authenticateCLI().catch((err) => { ## Security Considerations -1. **Verification and polling limits**: `/device` is limited to 5 requests over a window equal to the configured device-code lifetime. `/device/token` keeps its own polling interval and `slow_down` behavior +1. **Verification and polling limits**: `/device` is limited to 5 requests over a window equal to the configured device-code lifetime. Token polling enforces each code's interval instead: a poll that arrives sooner than `interval` after the last accepted poll returns `slow_down` 2. **Code Expiration**: Device and user codes expire after the configured time (default: 30 minutes) 3. **Client Validation**: Always validate client IDs in production to prevent unauthorized access 4. **HTTPS and approval UI**: Follow the [RFC 8628 TLS and user-interaction requirements](https://datatracker.ietf.org/doc/html/rfc8628#section-3.3) in production, including explicit approval or denial and [device-possession and remote-phishing guidance](https://datatracker.ietf.org/doc/html/rfc8628#section-5.4) 5. **User Code Format**: User codes use a limited character set (excluding similar-looking characters like 0/O, 1/I) to reduce typing errors -6. **Authentication Required**: Users must be authenticated when calling `GET /device`. The verification step claims the pending device code for the calling session, and only that session can later approve or deny it +6. **Authentication Required**: Users must be authenticated when calling `GET /device`. The verification step claims the pending device code for the signed-in user, and only that user can later approve or deny it 7. **Pre-binding**: Device codes issued with `user_id` skip the claiming step and can only be approved or denied by that user. Pass `user_id` from trusted server-side code only ## Options @@ -749,7 +764,7 @@ authenticateCLI().catch((err) => { **expiresIn**: The expiration time for device codes. Default: `"30m"` (30 minutes). -**interval**: The minimum polling interval. Default: `"5s"` (5 seconds). +**interval**: The minimum polling interval, returned to the device as `interval` in seconds. Default: `"5s"` (5 seconds). **userCodeLength**: The length of the user code. Maximum: `191`. Default: `8`. @@ -841,7 +856,7 @@ export const deviceCodeTableFields = [ { name: "pollingInterval", type: "number", - description: "Minimum seconds between polls", + description: "Minimum milliseconds between polls", isOptional: true, }, ]; diff --git a/packages/better-auth/src/plugins/device-authorization/device-authorization.test.ts b/packages/better-auth/src/plugins/device-authorization/device-authorization.test.ts index f1172c5f5b..9d0be23f0e 100644 --- a/packages/better-auth/src/plugins/device-authorization/device-authorization.test.ts +++ b/packages/better-auth/src/plugins/device-authorization/device-authorization.test.ts @@ -381,6 +381,15 @@ describe("device authorization flow", async () => { vi.useRealTimers(); }); + const pollDeviceToken = (deviceCode: string) => + auth.api.deviceToken({ + body: { + grant_type: "urn:ietf:params:oauth:grant-type:device_code", + device_code: deviceCode, + client_id: "test-client", + }, + }); + it("should return authorization_pending when not approved", async () => { const { device_code } = await auth.api.deviceCode({ body: { @@ -433,6 +442,43 @@ describe("device authorization flow", async () => { vi.useRealTimers(); }); + it("returns expired_token, not slow_down, for an expired code polled too soon", async () => { + vi.useFakeTimers(); + const { device_code } = await auth.api.deviceCode({ + body: { client_id: "test-client" }, + }); + await vi.advanceTimersByTimeAsync(299_500); + await expect(pollDeviceToken(device_code)).rejects.toMatchObject({ + body: { error: "authorization_pending" }, + }); + + await vi.advanceTimersByTimeAsync(1000); + await expect(pollDeviceToken(device_code)).rejects.toMatchObject({ + body: { error: "expired_token" }, + }); + }); + + it("keeps the original interval after slow_down", async () => { + vi.useFakeTimers(); + const { device_code } = await auth.api.deviceCode({ + body: { client_id: "test-client" }, + }); + await expect(pollDeviceToken(device_code)).rejects.toMatchObject({ + body: { error: "authorization_pending" }, + }); + + await vi.advanceTimersByTimeAsync(1000); + await expect(pollDeviceToken(device_code)).rejects.toMatchObject({ + body: { error: "slow_down" }, + }); + + // The rejected poll neither restarts the wait nor lengthens it. + await vi.advanceTimersByTimeAsync(1000); + await expect(pollDeviceToken(device_code)).rejects.toMatchObject({ + body: { error: "authorization_pending" }, + }); + }); + it("should return error for invalid device code", async () => { await expect( auth.api.deviceToken({ @@ -900,6 +946,74 @@ describe("device authorization flow", async () => { }); }); + describe("concurrent approval decisions", () => { + it("keeps a denial that lands between the approval's read and write", async () => { + const { headers } = await signInWithTestUser(); + const { device_code, user_code } = await auth.api.deviceCode({ + body: { client_id: "test-client" }, + }); + await auth.api.deviceVerify({ query: { user_code }, headers }); + + const { adapter } = await auth.$context; + const findOne = adapter.findOne.bind(adapter); + let denied = false; + const spy = vi.spyOn(adapter, "findOne").mockImplementation((async ( + input: Parameters[0], + ) => { + const row = await findOne(input); + if (!denied && input.model === "deviceCode") { + denied = true; + await auth.api.deviceDeny({ body: { userCode: user_code }, headers }); + } + return row; + }) as typeof adapter.findOne); + try { + await expect( + auth.api.deviceApprove({ body: { userCode: user_code }, headers }), + ).rejects.toMatchObject({ + body: { error_description: "Device code already processed" }, + }); + } finally { + spy.mockRestore(); + } + + const stored = await db.findOne({ + model: "deviceCode", + where: [{ field: "deviceCode", value: device_code }], + }); + expect(stored?.status).toBe("denied"); + }); + + it("rejects a decision on a code that expires before the write", async () => { + const { headers } = await signInWithTestUser(); + const { device_code, user_code } = await auth.api.deviceCode({ + body: { client_id: "test-client" }, + }); + await auth.api.deviceVerify({ query: { user_code }, headers }); + const deviceCodeRecord = await db.findOne({ + model: "deviceCode", + where: [{ field: "deviceCode", value: device_code }], + }); + + // At `expiresAt` the pending check still passes but the write does not. + vi.useFakeTimers({ toFake: ["Date"] }); + vi.setSystemTime(new Date(deviceCodeRecord!.expiresAt)); + try { + await expect( + auth.api.deviceApprove({ body: { userCode: user_code }, headers }), + ).rejects.toMatchObject({ body: { error: "expired_token" } }); + } finally { + vi.useRealTimers(); + } + + const stored = await db.findOne({ + model: "deviceCode", + where: [{ field: "deviceCode", value: device_code }], + }); + expect(stored?.status).toBe("pending"); + }); + }); + describe("concurrent token redemption", () => { // Invariant: an approved device code is single-use. Two polls racing to // redeem the same approved code must yield exactly one token; the loser diff --git a/packages/better-auth/src/plugins/device-authorization/routes.ts b/packages/better-auth/src/plugins/device-authorization/routes.ts index 6932d4194d..c55530f46e 100644 --- a/packages/better-auth/src/plugins/device-authorization/routes.ts +++ b/packages/better-auth/src/plugins/device-authorization/routes.ts @@ -107,6 +107,95 @@ async function findDeviceCodeByUserCode( return findByUserCode(normalizedUserCode); } +const expiredUserCode = () => + new APIError("BAD_REQUEST", { + error: "expired_token", + error_description: + DEVICE_AUTHORIZATION_ERROR_CODES.EXPIRED_USER_CODE.message, + }); + +const deviceCodeAlreadyProcessed = () => + new APIError("BAD_REQUEST", { + error: "invalid_request", + error_description: + DEVICE_AUTHORIZATION_ERROR_CODES.DEVICE_CODE_ALREADY_PROCESSED.message, + }); + +/** Throws unless the device code is unexpired and still awaiting a decision. */ +function assertDeviceCodePending(deviceCode: DeviceCode) { + if (deviceCode.expiresAt < new Date()) { + throw expiredUserCode(); + } + if (deviceCode.status !== "pending") { + throw deviceCodeAlreadyProcessed(); + } +} + +/** + * Records the signed-in owner's approval or denial of a pending device code. + * The write is guarded on the pending status, owner, and expiry, so a decision + * that loses a race with another approve or deny, or outlasts the code, fails + * instead of overwriting it. + */ +async function recordDeviceCodeDecision( + ctx: GenericEndpointContext, + userCode: string, + decision: "approved" | "denied", +) { + const session = await getSessionFromCtx(ctx); + if (!session) { + throw new APIError("UNAUTHORIZED", { + error: "unauthorized", + error_description: + DEVICE_AUTHORIZATION_ERROR_CODES.AUTHENTICATION_REQUIRED.message, + }); + } + + const deviceCodeRecord = await findDeviceCodeByUserCode(ctx, userCode); + if (!deviceCodeRecord) { + throw new APIError("BAD_REQUEST", { + error: "invalid_request", + error_description: + DEVICE_AUTHORIZATION_ERROR_CODES.INVALID_USER_CODE.message, + }); + } + + assertDeviceCodePending(deviceCodeRecord); + + if (!deviceCodeRecord.userId) { + throw new APIError("BAD_REQUEST", { + error: "invalid_request", + error_description: + DEVICE_AUTHORIZATION_ERROR_CODES.DEVICE_CODE_NOT_CLAIMED.message, + }); + } + + if (deviceCodeRecord.userId !== session.user.id) { + throw new APIError("FORBIDDEN", { + error: "access_denied", + error_description: `You are not authorized to ${decision === "approved" ? "approve" : "deny"} this device authorization`, + }); + } + + const decided = await ctx.context.adapter.incrementOne({ + model: "deviceCode", + where: [ + { field: "id", value: deviceCodeRecord.id }, + { field: "status", value: "pending" }, + { field: "userId", value: session.user.id }, + { field: "expiresAt", operator: "gt", value: new Date() }, + ], + increment: {}, + set: { status: decision }, + }); + if (!decided) { + // The guard treats a code as expired from `expiresAt` on. + throw deviceCodeRecord.expiresAt <= new Date() + ? expiredUserCode() + : deviceCodeAlreadyProcessed(); + } +} + const deviceCodeBodySchema = z.object({ client_id: z.string().meta({ description: "The client ID of the application", @@ -544,6 +633,19 @@ export async function redeemDeviceCode< } const authorization = await input.authorizeRedemption(deviceCodeRecord); + // Expiry is terminal, so it takes precedence over slow_down. + if (deviceCodeRecord.expiresAt < new Date()) { + await input.ctx.context.adapter.delete({ + model: "deviceCode", + where: [{ field: "id", value: deviceCodeRecord.id }], + }); + throw new APIError("BAD_REQUEST", { + error: "expired_token", + error_description: + DEVICE_AUTHORIZATION_ERROR_CODES.EXPIRED_DEVICE_CODE.message, + }); + } + if (deviceCodeRecord.lastPolledAt && deviceCodeRecord.pollingInterval) { const timeSinceLastPoll = Date.now() - new Date(deviceCodeRecord.lastPolledAt).getTime(); @@ -562,18 +664,6 @@ export async function redeemDeviceCode< update: { lastPolledAt: new Date() }, }); - if (deviceCodeRecord.expiresAt < new Date()) { - await input.ctx.context.adapter.delete({ - model: "deviceCode", - where: [{ field: "id", value: deviceCodeRecord.id }], - }); - throw new APIError("BAD_REQUEST", { - error: "expired_token", - error_description: - DEVICE_AUTHORIZATION_ERROR_CODES.EXPIRED_DEVICE_CODE.message, - }); - } - if (deviceCodeRecord.status === "pending") { throw new APIError("BAD_REQUEST", { error: "authorization_pending", @@ -909,45 +999,53 @@ export const deviceVerify = < }, ); -export const deviceApprove = createAuthEndpoint( - "/device/approve", - { - method: "POST", - body: z.object({ - userCode: z.string().meta({ - description: "The user code to approve", - }), - }), - error: z.object({ - error: z - .enum([ - "invalid_request", - "expired_token", - "device_code_already_processed", - "unauthorized", - "access_denied", - ]) - .meta({ - description: "Error code", +/** `/device/approve` and `/device/deny`, which differ only in the decision. */ +const deviceDecisionEndpoint = < + Path extends "/device/approve" | "/device/deny", +>( + path: Path, +) => { + const decision = path === "/device/approve" ? "approved" : "denied"; + const action = decision === "approved" ? "Approve" : "Deny"; + return createAuthEndpoint( + path, + { + method: "POST", + body: z.object({ + userCode: z.string().meta({ + description: `The user code to ${action.toLowerCase()}`, }), - error_description: z.string().meta({ - description: "Detailed error description", }), - }), - requireHeaders: true, - metadata: { - openapi: { - description: "Approve device authorization", - responses: { - 200: { - description: "Success", - content: { - "application/json": { - schema: { - type: "object", - properties: { - success: { - type: "boolean", + error: z.object({ + error: z + .enum([ + "invalid_request", + "expired_token", + "unauthorized", + "access_denied", + ]) + .meta({ + description: "Error code", + }), + error_description: z.string().meta({ + description: "Detailed error description", + }), + }), + requireHeaders: true, + metadata: { + openapi: { + description: `${action} device authorization`, + responses: { + 200: { + description: "Success", + content: { + "application/json": { + schema: { + type: "object", + properties: { + success: { + type: "boolean", + }, }, }, }, @@ -957,203 +1055,18 @@ export const deviceApprove = createAuthEndpoint( }, }, }, - }, - async (ctx) => { - const session = await getSessionFromCtx(ctx); - if (!session) { - throw new APIError("UNAUTHORIZED", { - error: "unauthorized", - error_description: - DEVICE_AUTHORIZATION_ERROR_CODES.AUTHENTICATION_REQUIRED.message, + async (ctx) => { + await recordDeviceCodeDecision(ctx, ctx.body.userCode, decision); + return ctx.json({ + success: true, }); - } - - const { userCode } = ctx.body; - const deviceCodeRecord = await findDeviceCodeByUserCode(ctx, userCode); - - if (!deviceCodeRecord) { - throw new APIError("BAD_REQUEST", { - error: "invalid_request", - error_description: - DEVICE_AUTHORIZATION_ERROR_CODES.INVALID_USER_CODE.message, - }); - } - - if (deviceCodeRecord.expiresAt < new Date()) { - throw new APIError("BAD_REQUEST", { - error: "expired_token", - error_description: - DEVICE_AUTHORIZATION_ERROR_CODES.EXPIRED_USER_CODE.message, - }); - } - - if (deviceCodeRecord.status !== "pending") { - throw new APIError("BAD_REQUEST", { - error: "invalid_request", - error_description: - DEVICE_AUTHORIZATION_ERROR_CODES.DEVICE_CODE_ALREADY_PROCESSED - .message, - }); - } - - if (!deviceCodeRecord.userId) { - throw new APIError("BAD_REQUEST", { - error: "invalid_request", - error_description: - DEVICE_AUTHORIZATION_ERROR_CODES.DEVICE_CODE_NOT_CLAIMED.message, - }); - } - - if (deviceCodeRecord.userId !== session.user.id) { - throw new APIError("FORBIDDEN", { - error: "access_denied", - error_description: - "You are not authorized to approve this device authorization", - }); - } - - // Update device code with approved status and user ID - await ctx.context.adapter.update({ - model: "deviceCode", - where: [ - { - field: "id", - value: deviceCodeRecord.id, - }, - ], - update: { - status: "approved", - userId: session.user.id, - }, - }); - - return ctx.json({ - success: true, - }); - }, -); - -export const deviceDeny = createAuthEndpoint( - "/device/deny", - { - method: "POST", - body: z.object({ - userCode: z.string().meta({ - description: "The user code to deny", - }), - }), - error: z.object({ - error: z - .enum([ - "invalid_request", - "expired_token", - "unauthorized", - "access_denied", - ]) - .meta({ - description: "Error code", - }), - error_description: z.string().meta({ - description: "Detailed error description", - }), - }), - requireHeaders: true, - metadata: { - openapi: { - description: "Deny device authorization", - responses: { - 200: { - description: "Success", - content: { - "application/json": { - schema: { - type: "object", - properties: { - success: { - type: "boolean", - }, - }, - }, - }, - }, - }, - }, - }, }, - }, - async (ctx) => { - const session = await getSessionFromCtx(ctx); - if (!session) { - throw new APIError("UNAUTHORIZED", { - error: "unauthorized", - error_description: - DEVICE_AUTHORIZATION_ERROR_CODES.AUTHENTICATION_REQUIRED.message, - }); - } + ); +}; - const { userCode } = ctx.body; - const deviceCodeRecord = await findDeviceCodeByUserCode(ctx, userCode); +export const deviceApprove = deviceDecisionEndpoint("/device/approve"); - if (!deviceCodeRecord) { - throw new APIError("BAD_REQUEST", { - error: "invalid_request", - error_description: - DEVICE_AUTHORIZATION_ERROR_CODES.INVALID_USER_CODE.message, - }); - } - - if (deviceCodeRecord.expiresAt < new Date()) { - throw new APIError("BAD_REQUEST", { - error: "expired_token", - error_description: - DEVICE_AUTHORIZATION_ERROR_CODES.EXPIRED_USER_CODE.message, - }); - } - - if (deviceCodeRecord.status !== "pending") { - throw new APIError("BAD_REQUEST", { - error: "invalid_request", - error_description: - DEVICE_AUTHORIZATION_ERROR_CODES.DEVICE_CODE_ALREADY_PROCESSED - .message, - }); - } - - if (!deviceCodeRecord.userId) { - throw new APIError("BAD_REQUEST", { - error: "invalid_request", - error_description: - DEVICE_AUTHORIZATION_ERROR_CODES.DEVICE_CODE_NOT_CLAIMED.message, - }); - } - - if (deviceCodeRecord.userId !== session.user.id) { - throw new APIError("FORBIDDEN", { - error: "access_denied", - error_description: - "You are not authorized to deny this device authorization", - }); - } - - await ctx.context.adapter.update({ - model: "deviceCode", - where: [ - { - field: "id", - value: deviceCodeRecord.id, - }, - ], - update: { - status: "denied", - userId: session.user.id, - }, - }); - - return ctx.json({ - success: true, - }); - }, -); +export const deviceDeny = deviceDecisionEndpoint("/device/deny"); /** * @internal