From ad46c11430911bac9d9e87bf4d3920f88942f14d Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Sun, 30 Aug 2026 20:49:40 -0700 Subject: [PATCH 1/9] feat(cli): add device-code login flow --- backend/__tests__/service/auth.test.js | 146 +++++++++++++ backend/controllers/authController.ts | 8 +- backend/controllers/userController.ts | 1 + backend/middleware/auth.ts | 35 +++- backend/models/DeviceAuthorization.ts | 49 +++++ backend/models/User.ts | 30 +++ backend/routes/auth.ts | 105 +++++++++- .../services/deviceAuthorizationService.ts | 191 ++++++++++++++++++ backend/types/express.d.ts | 2 +- cli/__tests__/api-expiry.test.mjs | 31 +++ cli/__tests__/device-login.test.mjs | 71 +++++++ cli/__tests__/lib.test.mjs | 3 +- cli/src/commands/login.js | 80 ++++++-- cli/src/lib/api.js | 34 +++- cli/src/lib/config.js | 5 +- cli/src/lib/device-login.js | 98 +++++++++ frontend/src/App.tsx | 3 + frontend/src/v2/V2App.tsx | 6 + .../src/v2/__tests__/V2CliAuthorize.test.tsx | 62 ++++++ .../src/v2/__tests__/V2DevicesPanel.test.tsx | 29 +++ frontend/src/v2/components/V2CliAuthorize.css | 55 +++++ frontend/src/v2/components/V2CliAuthorize.tsx | 138 +++++++++++++ frontend/src/v2/components/V2DevicesPanel.css | 13 ++ frontend/src/v2/components/V2DevicesPanel.tsx | 61 ++++++ 24 files changed, 1222 insertions(+), 34 deletions(-) create mode 100644 backend/models/DeviceAuthorization.ts create mode 100644 backend/services/deviceAuthorizationService.ts create mode 100644 cli/__tests__/api-expiry.test.mjs create mode 100644 cli/__tests__/device-login.test.mjs create mode 100644 cli/src/lib/device-login.js create mode 100644 frontend/src/v2/__tests__/V2CliAuthorize.test.tsx create mode 100644 frontend/src/v2/__tests__/V2DevicesPanel.test.tsx create mode 100644 frontend/src/v2/components/V2CliAuthorize.css create mode 100644 frontend/src/v2/components/V2CliAuthorize.tsx create mode 100644 frontend/src/v2/components/V2DevicesPanel.css create mode 100644 frontend/src/v2/components/V2DevicesPanel.tsx diff --git a/backend/__tests__/service/auth.test.js b/backend/__tests__/service/auth.test.js index 4b1efc8e9..475baa855 100644 --- a/backend/__tests__/service/auth.test.js +++ b/backend/__tests__/service/auth.test.js @@ -4,6 +4,8 @@ const express = require('express'); const mongoose = require('mongoose'); const User = require('../../models/User'); const Pod = require('../../models/Pod'); +const DeviceAuthorization = require('../../models/DeviceAuthorization'); +const { hashDeviceCredential } = require('../../services/deviceAuthorizationService'); const authRoutes = require('../../routes/auth'); const { setupMongoDb, @@ -410,6 +412,150 @@ describe('Auth Routes Integration Tests', () => { }); }); + describe('CLI device authorization', () => { + const createVerifiedUser = async () => { + const user = new User({ + username: 'device-owner', + email: 'device-owner@example.com', + password: 'Password123!', + verified: true, + }); + await user.save(); + return user; + }; + + const startAuthorization = async () => request(app) + .post('/api/auth/device/start') + .send({ clientName: 'commonly-cli', clientVersion: '0.1.26', hostname: 'sam-laptop' }) + .expect(201); + + it('hands an approved token to exactly one poller and persists only its digest', async () => { + const user = await createVerifiedUser(); + const browserToken = generateTestToken(user._id); + const start = await startAuthorization(); + + expect(start.body).toMatchObject({ + verifyUrl: 'http://localhost:3000/cli/authorize', + expiresIn: 600, + interval: 5, + }); + expect(start.body.deviceCode).toBeTruthy(); + expect(start.body.userCode).toMatch(/^[A-HJ-NP-Z2-9]{4}-[A-HJ-NP-Z2-9]{4}$/); + + const storedRequest = await DeviceAuthorization.findOne(); + expect(storedRequest.deviceCodeHash).not.toBe(start.body.deviceCode); + expect(storedRequest.userCodeHash).not.toBe(start.body.userCode.replace('-', '')); + + await request(app) + .post('/api/auth/device/poll') + .send({ deviceCode: start.body.deviceCode }) + .expect(200) + .expect({ status: 'authorization_pending' }); + + const confirmation = await request(app) + .post('/api/auth/device/authorize') + .set('Authorization', `Bearer ${browserToken}`) + .send({ userCode: start.body.userCode }) + .expect(200); + expect(confirmation.body).toMatchObject({ + status: 'pending', + request: { hostname: 'sam-laptop', clientName: 'commonly-cli', clientVersion: '0.1.26' }, + }); + + await request(app) + .post('/api/auth/device/authorize') + .set('Authorization', `Bearer ${browserToken}`) + .send({ userCode: start.body.userCode, decision: 'authorize' }) + .expect(200) + .expect({ status: 'authorized' }); + + const granted = await request(app) + .post('/api/auth/device/poll') + .send({ deviceCode: start.body.deviceCode }) + .expect(200); + expect(granted.body).toMatchObject({ username: 'device-owner', userId: user._id.toString() }); + expect(granted.body.token).toMatch(/^cm_[a-f0-9]{64}$/); + + // The transient handoff is consumed atomically; a duplicate poll never + // returns a second copy of the bearer. + await request(app) + .post('/api/auth/device/poll') + .send({ deviceCode: start.body.deviceCode }) + .expect(200) + .expect({ status: 'already_used' }); + + const persistedUser = await User.findById(user._id).select('deviceTokens'); + expect(persistedUser.deviceTokens).toHaveLength(1); + expect(persistedUser.deviceTokens[0].label).toBe('sam-laptop · commonly-cli'); + expect(persistedUser.deviceTokens[0].tokenHash).not.toBe(granted.body.token); + expect(JSON.stringify(persistedUser)).not.toContain(granted.body.token); + + // It is a normal user bearer until explicitly revoked. + await request(app) + .get('/api/auth/user') + .set('Authorization', `Bearer ${granted.body.token}`) + .expect(200) + .expect((response) => expect(response.body.deviceTokens).toBeUndefined()); + + const devices = await request(app) + .get('/api/auth/devices') + .set('Authorization', `Bearer ${browserToken}`) + .expect(200); + expect(devices.body.devices).toEqual([expect.objectContaining({ + label: 'sam-laptop · commonly-cli', + })]); + expect(devices.body.devices[0].tokenHash).toBeUndefined(); + expect(JSON.stringify(devices.body)).not.toContain(granted.body.token); + + await request(app) + .delete(`/api/auth/devices/${devices.body.devices[0].id}`) + .set('Authorization', `Bearer ${browserToken}`) + .expect(200); + await request(app) + .get('/api/auth/user') + .set('Authorization', `Bearer ${granted.body.token}`) + .expect(401); + }); + + it('returns slow_down, denied, and expired terminal states without minting a token', async () => { + const user = await createVerifiedUser(); + const browserToken = generateTestToken(user._id); + + const pending = await startAuthorization(); + await request(app).post('/api/auth/device/poll').send({ deviceCode: pending.body.deviceCode }).expect(200); + await request(app) + .post('/api/auth/device/poll') + .send({ deviceCode: pending.body.deviceCode }) + .expect(200) + .expect({ status: 'slow_down' }); + + const denied = await startAuthorization(); + await request(app) + .post('/api/auth/device/authorize') + .set('Authorization', `Bearer ${browserToken}`) + .send({ userCode: denied.body.userCode, decision: 'deny' }) + .expect(200) + .expect({ status: 'denied' }); + await request(app) + .post('/api/auth/device/poll') + .send({ deviceCode: denied.body.deviceCode }) + .expect(200) + .expect({ status: 'denied' }); + + const expired = await startAuthorization(); + await DeviceAuthorization.updateOne( + { deviceCodeHash: hashDeviceCredential(expired.body.deviceCode) }, + { $set: { expiresAt: new Date(Date.now() - 1000) } }, + ); + await request(app) + .post('/api/auth/device/poll') + .send({ deviceCode: expired.body.deviceCode }) + .expect(200) + .expect({ status: 'expired' }); + expect((await User.findById(user._id).select('deviceTokens')).deviceTokens).toHaveLength(0); + }); + }); + describe('PUT /api/auth/profile', () => { it('should update user profile with valid token', async () => { // Create a user diff --git a/backend/controllers/authController.ts b/backend/controllers/authController.ts index 1c27c9ba3..8e1bd0572 100644 --- a/backend/controllers/authController.ts +++ b/backend/controllers/authController.ts @@ -688,7 +688,7 @@ exports.login = async (req: any, res: any) => { // 🔄 Refresh Token — issue a new 7d token from a still-valid token exports.refresh = async (req: any, res: any) => { try { - const user = await User.findById(req.userId).select('-password'); + const user = await User.findById(req.userId).select('-password -deviceTokens'); if (!user) return res.status(404).json({ error: 'User not found' }); const token = jwt.sign({ id: user._id }, process.env.JWT_SECRET, { @@ -705,7 +705,7 @@ exports.refresh = async (req: any, res: any) => { // New method to get user profile exports.getProfile = async (req: any, res: any) => { try { - const user = await User.findById(req.userId).select('-password'); + const user = await User.findById(req.userId).select('-password -deviceTokens'); if (!user) return res.status(404).json({ error: 'User not found' }); res.json(user); } catch (err: any) { @@ -716,7 +716,7 @@ exports.getProfile = async (req: any, res: any) => { // Get current user information exports.getCurrentUser = async (req: any, res: any) => { try { - const user = await User.findById(req.userId).select('-password'); + const user = await User.findById(req.userId).select('-password -deviceTokens'); if (!user) { return res.status(404).json({ error: 'User not found' }); } @@ -747,7 +747,7 @@ exports.updateProfile = async (req: any, res: any) => { await AgentIdentityService.syncUserToPostgreSQL(user); // Return the updated user without the password - const updatedUser = await User.findById(userId).select('-password'); + const updatedUser = await User.findById(userId).select('-password -deviceTokens'); res.json(updatedUser); } catch (err: any) { res.status(500).json({ error: err.message }); diff --git a/backend/controllers/userController.ts b/backend/controllers/userController.ts index 3e0899e30..7abc59aa3 100644 --- a/backend/controllers/userController.ts +++ b/backend/controllers/userController.ts @@ -14,6 +14,7 @@ const SECRET_USER_FIELDS = [ 'password', 'apiToken', 'agentRuntimeTokens', + 'deviceTokens', 'digestUnsubscribeToken', ]; diff --git a/backend/middleware/auth.ts b/backend/middleware/auth.ts index f9c755008..c4fff9340 100644 --- a/backend/middleware/auth.ts +++ b/backend/middleware/auth.ts @@ -1,6 +1,7 @@ import jwt from 'jsonwebtoken'; import { Request, Response, NextFunction } from 'express'; import User from '../models/User'; +import { hashDeviceCredential } from '../services/deviceAuthorizationService'; // User.lastActive was only ever set at account creation, which made every // activity-based metric (returned D1/D7 in /api/admin/analytics/funnel, @@ -45,10 +46,29 @@ export default async function auth(req: Request, res: Response, next: NextFuncti if (token.startsWith('cm_')) { try { - const user = await User.findOne({ apiToken: token }).select( + // `apiToken` is the historical single plaintext bearer. Device-login + // bearers share its public cm_ prefix but are stored only as a hash so a + // database read cannot impersonate a CLI session. + let user: any = await User.findOne({ apiToken: token }).select( '_id username email role apiTokenScopes apiTokenCreatedAt banned', ); + let isDeviceToken = false; + const deviceTokenHash = hashDeviceCredential(token); + if (!user) { + user = await User.findOne({ + deviceTokens: { + $elemMatch: { + tokenHash: deviceTokenHash, + // `{ $in: [null] }` deliberately covers both an older array + // entry with no field and a current entry with explicit null. + revokedAt: { $in: [null] }, + }, + }, + }).select('_id username email role banned'); + isDeviceToken = Boolean(user); + } + if (!user) return res.status(401).json({ msg: 'Invalid API token' }); if ((user as unknown as { banned?: boolean }).banned) { return res.status(403).json({ msg: 'This account has been suspended.' }); @@ -61,9 +81,20 @@ export default async function auth(req: Request, res: Response, next: NextFuncti email: user.email, role: user.role, }; - req.authType = 'apiToken'; + req.authType = isDeviceToken ? 'deviceToken' : 'apiToken'; req.apiTokenScopes = user.apiTokenScopes || []; req.apiTokenCreatedAt = user.apiTokenCreatedAt || null; + if (isDeviceToken) { + // A usage stamp is an audit convenience, not an auth dependency: the + // request stays valid if this best-effort write loses a race. + void User.updateOne( + { + _id: user._id, + deviceTokens: { $elemMatch: { tokenHash: deviceTokenHash, revokedAt: { $in: [null] } } }, + }, + { $set: { 'deviceTokens.$.lastUsedAt': new Date() } }, + ).catch(() => undefined); + } touchLastActive(user._id.toString()); return next(); } catch (err: unknown) { diff --git a/backend/models/DeviceAuthorization.ts b/backend/models/DeviceAuthorization.ts new file mode 100644 index 000000000..7e20a8ad1 --- /dev/null +++ b/backend/models/DeviceAuthorization.ts @@ -0,0 +1,49 @@ +import mongoose, { Document, Schema, Types } from 'mongoose'; + +export type DeviceAuthorizationStatus = 'pending' | 'authorized' | 'denied' | 'consumed'; + +export interface IDeviceAuthorization extends Document { + deviceCodeHash: string; + userCodeHash: string; + clientName: string; + clientVersion?: string; + hostname: string; + status: DeviceAuthorizationStatus; + userId?: Types.ObjectId | null; + // This is the one-time handoff from a browser approval to its originating + // CLI. It is never projected by default and is unset once poll consumes it. + pendingToken?: string; + createdAt: Date; + lastPolledAt?: Date; + authorizedAt?: Date; + deniedAt?: Date; + consumedAt?: Date; + expiresAt: Date; +} + +const DeviceAuthorizationSchema = new Schema( + { + deviceCodeHash: { type: String, required: true, unique: true, index: true }, + userCodeHash: { type: String, required: true, unique: true, index: true }, + clientName: { type: String, required: true, trim: true, maxlength: 120 }, + clientVersion: { type: String, trim: true, maxlength: 80 }, + hostname: { type: String, required: true, trim: true, maxlength: 253 }, + status: { type: String, enum: ['pending', 'authorized', 'denied', 'consumed'], default: 'pending' }, + userId: { type: Schema.Types.ObjectId, ref: 'User', default: null }, + pendingToken: { type: String, select: false }, + createdAt: { type: Date, default: Date.now }, + lastPolledAt: { type: Date }, + authorizedAt: { type: Date }, + deniedAt: { type: Date }, + consumedAt: { type: Date }, + // The TTL reaper is a cleanup backstop; every endpoint still checks this + // timestamp so expiry behaves correctly before Mongo's next TTL sweep. + expiresAt: { type: Date, required: true, index: { expires: 0 } }, + }, + { collection: 'device_authorizations' }, +); + +export default mongoose.model('DeviceAuthorization', DeviceAuthorizationSchema); +// CJS compat: let require() return the default export directly +// eslint-disable-next-line @typescript-eslint/no-require-imports +module.exports = exports.default; Object.assign(module.exports, exports); diff --git a/backend/models/User.ts b/backend/models/User.ts index 5d5a534bb..932ea4eec 100644 --- a/backend/models/User.ts +++ b/backend/models/User.ts @@ -22,6 +22,17 @@ export interface IAgentRuntimeToken { expiresAt?: Date; } +// A device-login bearer is intentionally one-way: the CLI receives it once, +// while Mongo stores only this digest. It is separate from the legacy +// apiToken (which pre-dates per-device revocation) and agent runtime tokens. +export interface IDeviceToken { + tokenHash: string; + label: string; + createdAt: Date; + lastUsedAt?: Date; + revokedAt?: Date; +} + export interface IFollowedThread { postId: Types.ObjectId; followedAt: Date; @@ -141,6 +152,7 @@ export interface IUser extends Document { capabilities: AgentCapability[]; }; agentRuntimeTokens: IAgentRuntimeToken[]; + deviceTokens: IDeviceToken[]; contacts: IContactEntry[]; subscribedPods: Types.ObjectId[]; followers: Types.ObjectId[]; @@ -312,6 +324,21 @@ const userSchema = new Schema({ expiresAt: { type: Date }, }, ], + // D1 device-code login. Do not mark this select:false: auth middleware must + // inspect the digest, and response serializers explicitly omit the whole + // array so these records never become an account-profile API surface. + deviceTokens: { + type: [ + new Schema({ + tokenHash: { type: String, required: true }, + label: { type: String, required: true }, + createdAt: { type: Date, default: Date.now }, + lastUsedAt: { type: Date }, + revokedAt: { type: Date }, + }, { _id: true }), + ], + default: [], + }, // Alias-driven contact list — see IContactEntry above. Default empty so // existing user rows return `[]` on read (never throws on `.find(...)`). contacts: { @@ -367,6 +394,9 @@ const userSchema = new Schema({ // OAuth callback looks users up by (provider, providerId) on every social login. userSchema.index({ 'authProviders.provider': 1, 'authProviders.providerId': 1 }); +// A single index supports authentication by digest and the account's device +// list. Existing users simply have no array entries, so this is additive. +userSchema.index({ 'deviceTokens.tokenHash': 1 }); userSchema.index( { digestUnsubscribeToken: 1 }, { diff --git a/backend/routes/auth.ts b/backend/routes/auth.ts index 78e61aa8f..c5aa7f217 100644 --- a/backend/routes/auth.ts +++ b/backend/routes/auth.ts @@ -23,6 +23,16 @@ const { resetPassword, } = require('../controllers/authController'); // eslint-disable-next-line global-require +const { + DEVICE_AUTHORIZATION_TTL_MS, + DEVICE_POLL_INTERVAL_SECONDS, + createDeviceAuthorization, + pollDeviceAuthorization, + decideDeviceAuthorization, + listDeviceTokens, + revokeDeviceToken, +} = require('../services/deviceAuthorizationService'); +// eslint-disable-next-line global-require const { getOAuthProviders, startOAuth, @@ -32,6 +42,7 @@ const { interface AuthReq { user?: { id: string }; + userId?: string; } interface Res { status: (n: number) => Res; @@ -71,6 +82,29 @@ const loginLimiter = rateLimit({ handler: rateLimitHandler('rate limit exceeded: 20 login attempts per 15 minutes'), }); +// Device start mints a server-side authorization request; poll needs room for +// the documented 5s cadence over a ten-minute lifetime. These are separate +// buckets so an interrupted CLI cannot starve a fresh login attempt. +const deviceStartLimiter = rateLimit({ + windowMs: 60 * 60 * 1000, + max: 20, + standardHeaders: true, + legacyHeaders: false, + skip: () => process.env.NODE_ENV === 'test', + keyGenerator: cloudflareIpRateLimitKeyGenerator, + handler: rateLimitHandler('rate limit exceeded: 20 device authorization starts per hour'), +}); + +const devicePollLimiter = rateLimit({ + windowMs: 15 * 60 * 1000, + max: 180, + standardHeaders: true, + legacyHeaders: false, + skip: () => process.env.NODE_ENV === 'test', + keyGenerator: cloudflareIpRateLimitKeyGenerator, + handler: rateLimitHandler('rate limit exceeded: too many device authorization polls'), +}); + // Waitlist is a one-shot action per person — 5/hour/IP. const waitlistLimiter = rateLimit({ windowMs: 60 * 60 * 1000, @@ -117,6 +151,71 @@ router.post('/oauth/exchange', oauthLimiter, exchangeOAuthCode); router.get('/registration-policy', getRegistrationPolicy); router.post('/waitlist', waitlistLimiter, requestWaitlist); router.post('/login', loginLimiter, login); +router.post('/device/start', deviceStartLimiter, async (req: any, res: Res) => { + try { + const { deviceCode, userCode } = await createDeviceAuthorization(req.body || {}); + const origin = String(process.env.FRONTEND_URL || 'https://commonly.me').replace(/\/$/, ''); + return res.status(201).json({ + deviceCode, + userCode, + verifyUrl: `${origin}/cli/authorize`, + expiresIn: Math.floor(DEVICE_AUTHORIZATION_TTL_MS / 1000), + interval: DEVICE_POLL_INTERVAL_SECONDS, + }); + } catch (error: any) { + if (error?.message === 'clientName and hostname are required') { + return res.status(400).json({ error: error.message }); + } + console.error('Unable to start device authorization:', error?.message); + return res.status(500).json({ error: 'Unable to start device authorization' }); + } +}); + +router.post('/device/poll', devicePollLimiter, async (req: any, res: Res) => { + try { + const result = await pollDeviceAuthorization(req.body?.deviceCode); + return res.json(result); + } catch (error: any) { + console.error('Unable to poll device authorization:', error?.message); + return res.status(500).json({ error: 'Unable to poll device authorization' }); + } +}); + +router.post('/device/authorize', auth, async (req: AuthReq & { body?: any }, res: Res) => { + try { + const result = await decideDeviceAuthorization({ + userCode: req.body?.userCode, + decision: req.body?.decision, + userId: req.userId || req.user?.id || '', + }); + if (result.status === 'invalid_decision') return res.status(400).json(result); + if (result.status === 'expired') return res.status(410).json(result); + return res.json(result); + } catch (error: any) { + console.error('Unable to decide device authorization:', error?.message); + return res.status(500).json({ error: 'Unable to decide device authorization' }); + } +}); + +router.get('/devices', auth, async (req: AuthReq, res: Res) => { + try { + return res.json({ devices: await listDeviceTokens(req.userId || req.user?.id || '') }); + } catch (error: any) { + console.error('Unable to list device tokens:', error?.message); + return res.status(500).json({ error: 'Unable to list device tokens' }); + } +}); + +router.delete('/devices/:deviceId', auth, async (req: AuthReq & { params?: any }, res: Res) => { + try { + const revoked = await revokeDeviceToken(req.userId || req.user?.id || '', String(req.params?.deviceId || '')); + if (!revoked) return res.status(404).json({ error: 'Device not found or already revoked' }); + return res.json({ message: 'Device revoked' }); + } catch (error: any) { + console.error('Unable to revoke device token:', error?.message); + return res.status(500).json({ error: 'Unable to revoke device token' }); + } +}); // Invitation redemption is authed and rare — the login limiter's // credential-stuffing posture (20/15min/IP) also bounds code-guessing here. router.post('/redeem-invitation', loginLimiter, auth, redeemInvitation); @@ -144,7 +243,11 @@ router.post('/api-token/generate', auth, async (req: AuthReq, res: Res) => { const token = user.generateApiToken(); await user.save(); - return res.json({ apiToken: token, createdAt: user.apiTokenCreatedAt, message: 'API token generated successfully' }); + return res.json({ + apiToken: token, + createdAt: user.apiTokenCreatedAt, + message: 'API token generated successfully', + }); } catch (error) { console.error('Error generating API token:', error); return res.status(500).json({ message: 'Server error' }); diff --git a/backend/services/deviceAuthorizationService.ts b/backend/services/deviceAuthorizationService.ts new file mode 100644 index 000000000..793aba878 --- /dev/null +++ b/backend/services/deviceAuthorizationService.ts @@ -0,0 +1,191 @@ +import crypto from 'crypto'; +import DeviceAuthorization, { IDeviceAuthorization } from '../models/DeviceAuthorization'; +import User from '../models/User'; + +export const DEVICE_AUTHORIZATION_TTL_MS = 10 * 60 * 1000; +export const DEVICE_POLL_INTERVAL_SECONDS = 5; + +const USER_CODE_ALPHABET = 'ABCDEFGHJKLMNPQRSTUVWXYZ23456789'; + +export const hashDeviceCredential = (value: string): string => crypto + .createHash('sha256') + .update(value) + .digest('hex'); + +const randomUserCode = (): string => { + const bytes = crypto.randomBytes(8); + let code = ''; + for (let index = 0; index < 8; index += 1) { + code += USER_CODE_ALPHABET[bytes[index] % USER_CODE_ALPHABET.length]; + } + return `${code.slice(0, 4)}-${code.slice(4)}`; +}; + +const randomDeviceToken = (): string => `cm_${crypto.randomBytes(32).toString('hex')}`; +const normalizeUserCode = (value: unknown): string => String(value || '') + .trim() + .toUpperCase() + .replace(/[^A-Z2-9]/g, ''); + +const userCodeHash = (value: unknown): string | null => { + const normalized = normalizeUserCode(value); + return /^[A-HJ-NP-Z2-9]{8}$/.test(normalized) ? hashDeviceCredential(normalized) : null; +}; + +const isExpired = (request: IDeviceAuthorization, now = new Date()): boolean => request.expiresAt <= now; + +export const createDeviceAuthorization = async ({ + clientName, + clientVersion, + hostname, +}: { clientName: unknown; clientVersion?: unknown; hostname: unknown }) => { + const safeClientName = String(clientName || '').trim().slice(0, 120); + const safeHostname = String(hostname || '').trim().slice(0, 253); + const safeClientVersion = String(clientVersion || '').trim().slice(0, 80); + if (!safeClientName || !safeHostname) throw new Error('clientName and hostname are required'); + + // User-code collisions are very unlikely, but the unique index is the + // source of truth; retry rather than risking an ambiguous browser approval. + for (let attempt = 0; attempt < 5; attempt += 1) { + const deviceCode = crypto.randomBytes(32).toString('base64url'); + const code = randomUserCode(); + try { + await DeviceAuthorization.create({ + deviceCodeHash: hashDeviceCredential(deviceCode), + userCodeHash: hashDeviceCredential(code.replace('-', '')), + clientName: safeClientName, + clientVersion: safeClientVersion || undefined, + hostname: safeHostname, + expiresAt: new Date(Date.now() + DEVICE_AUTHORIZATION_TTL_MS), + }); + return { deviceCode, userCode: code }; + } catch (error: any) { + if (error?.code !== 11000 || attempt === 4) throw error; + } + } + throw new Error('Unable to create device authorization'); +}; + +export const findDeviceAuthorizationByUserCode = async (value: unknown) => { + const digest = userCodeHash(value); + if (!digest) return null; + return DeviceAuthorization.findOne({ userCodeHash: digest }); +}; + +export const pollDeviceAuthorization = async (deviceCode: unknown) => { + const raw = String(deviceCode || '').trim(); + if (!raw) return { status: 'invalid' as const }; + const request = await DeviceAuthorization.findOne({ + deviceCodeHash: hashDeviceCredential(raw), + }).select('+pendingToken'); + if (!request || isExpired(request)) return { status: 'expired' as const }; + if (request.status === 'pending') { + const now = new Date(); + const polledTooSoon = request.lastPolledAt + && now.getTime() - request.lastPolledAt.getTime() < DEVICE_POLL_INTERVAL_SECONDS * 1000; + await DeviceAuthorization.updateOne({ _id: request._id }, { $set: { lastPolledAt: now } }); + return { status: polledTooSoon ? 'slow_down' as const : 'authorization_pending' as const }; + } + if (request.status === 'denied') return { status: 'denied' as const }; + if (request.status === 'consumed' || !request.pendingToken || !request.userId) { + return { status: 'already_used' as const }; + } + + // Conditional update prevents two concurrent polls from receiving the same + // bearer, even if they read the approved request at the same time. + const claimed = await DeviceAuthorization.findOneAndUpdate( + { _id: request._id, status: 'authorized', expiresAt: { $gt: new Date() } }, + { $set: { status: 'consumed', consumedAt: new Date() }, $unset: { pendingToken: 1 } }, + { new: false }, + ).select('+pendingToken'); + if (!claimed?.pendingToken || !claimed.userId) return { status: 'already_used' as const }; + + const user = await User.findById(claimed.userId).select('_id username banned'); + if (!user || user.banned) return { status: 'denied' as const }; + return { + status: 'authorized' as const, + token: claimed.pendingToken, + username: user.username, + userId: user._id.toString(), + }; +}; + +export const decideDeviceAuthorization = async ({ + userCode, + decision, + userId, +}: { userCode: unknown; decision: unknown; userId: string }) => { + const request = await findDeviceAuthorizationByUserCode(userCode); + if (!request || isExpired(request)) return { status: 'expired' as const }; + if (request.status !== 'pending') return { status: request.status as 'authorized' | 'denied' | 'consumed' }; + + const normalizedDecision = String(decision || '').toLowerCase(); + if (!normalizedDecision) { + return { + status: 'pending' as const, + request: { + hostname: request.hostname, + clientName: request.clientName, + clientVersion: request.clientVersion || null, + createdAt: request.createdAt, + }, + }; + } + if (!['authorize', 'deny'].includes(normalizedDecision)) return { status: 'invalid_decision' as const }; + + if (normalizedDecision === 'deny') { + const denied = await DeviceAuthorization.findOneAndUpdate( + { _id: request._id, status: 'pending', expiresAt: { $gt: new Date() } }, + { $set: { status: 'denied', deniedAt: new Date(), userId } }, + { new: true }, + ); + return denied ? { status: 'denied' as const } : { status: 'expired' as const }; + } + + const token = randomDeviceToken(); + const now = new Date(); + const label = `${request.hostname} · ${request.clientName}`; + const userUpdated = await User.updateOne( + { _id: userId, banned: { $ne: true } }, + { $push: { deviceTokens: { tokenHash: hashDeviceCredential(token), label, createdAt: now } } }, + ); + if (!userUpdated.matchedCount) return { status: 'denied' as const }; + + const authorized = await DeviceAuthorization.findOneAndUpdate( + { _id: request._id, status: 'pending', expiresAt: { $gt: now } }, + { $set: { status: 'authorized', userId, authorizedAt: now, pendingToken: token } }, + { new: true }, + ); + if (!authorized) { + // The new device token must not survive an expiry/race that lost the + // authorization request. Mark it revoked rather than leaving an orphan. + await User.updateOne( + { _id: userId, 'deviceTokens.tokenHash': hashDeviceCredential(token) }, + { $set: { 'deviceTokens.$.revokedAt': now } }, + ); + return { status: 'expired' as const }; + } + return { status: 'authorized' as const }; +}; + +export const listDeviceTokens = async (userId: string) => { + const user = await User.findById(userId).select('deviceTokens'); + return (user?.deviceTokens || []).map((entry: any) => ({ + id: entry._id.toString(), + label: entry.label, + createdAt: entry.createdAt, + lastUsedAt: entry.lastUsedAt || null, + revokedAt: entry.revokedAt || null, + })); +}; + +export const revokeDeviceToken = async (userId: string, deviceId: string) => { + const result = await User.updateOne( + { + _id: userId, + deviceTokens: { $elemMatch: { _id: deviceId, revokedAt: { $in: [null] } } }, + }, + { $set: { 'deviceTokens.$.revokedAt': new Date() } }, + ); + return result.modifiedCount > 0; +}; diff --git a/backend/types/express.d.ts b/backend/types/express.d.ts index eec22f6f8..f9c8f0f44 100644 --- a/backend/types/express.d.ts +++ b/backend/types/express.d.ts @@ -9,7 +9,7 @@ declare global { // Set by auth.js userId?: string; user?: { id: string; username?: string; email?: string; role?: string }; - authType?: 'jwt' | 'apiToken'; + authType?: 'jwt' | 'apiToken' | 'deviceToken'; apiTokenScopes?: string[]; apiTokenCreatedAt?: Date | null; diff --git a/cli/__tests__/api-expiry.test.mjs b/cli/__tests__/api-expiry.test.mjs new file mode 100644 index 000000000..309863031 --- /dev/null +++ b/cli/__tests__/api-expiry.test.mjs @@ -0,0 +1,31 @@ +import { jest } from '@jest/globals'; +import os from 'os'; +import path from 'path'; +import fs from 'fs'; + +const configTmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'cli-api-expiry-')); +await jest.unstable_mockModule('os', () => ({ + ...os, + default: { ...os, homedir: () => configTmpDir }, + homedir: () => configTmpDir, +})); + +const { saveInstance } = await import('../src/lib/config.js'); +const { createClient } = await import('../src/lib/api.js'); + +afterAll(() => fs.rmSync(path.join(configTmpDir, '.commonly'), { recursive: true, force: true })); + +test('replaces a server token message with an actionable saved-profile instruction', async () => { + saveInstance({ + key: 'dev', url: 'https://api.commonly.me', token: 'stale-token', userId: 'u1', username: 'lily', + }); + global.fetch = jest.fn().mockResolvedValue({ + ok: false, + status: 401, + text: async () => JSON.stringify({ msg: 'Token is not valid' }), + }); + + await expect(createClient({ instance: 'dev' }).get('/api/auth/user')).rejects.toThrow( + 'Session for dev (https://api.commonly.me) has expired.\nRun: commonly login --instance dev', + ); +}); diff --git a/cli/__tests__/device-login.test.mjs b/cli/__tests__/device-login.test.mjs new file mode 100644 index 000000000..3a1fbb703 --- /dev/null +++ b/cli/__tests__/device-login.test.mjs @@ -0,0 +1,71 @@ +import { EventEmitter } from 'events'; +import { jest } from '@jest/globals'; +import { + DeviceLoginCancelledError, + openBrowser, + waitForDeviceAuthorization, +} from '../src/lib/device-login.js'; +import { formatTokenStatus } from '../src/commands/login.js'; + +describe('CLI device login', () => { + test('slows polling when asked and returns only the authorized handoff', async () => { + const client = { + post: jest.fn() + .mockResolvedValueOnce({ status: 'authorization_pending' }) + .mockResolvedValueOnce({ status: 'slow_down' }) + .mockResolvedValueOnce({ status: 'authorized', token: 'cm_once', username: 'lily', userId: 'u1' }), + }; + const waits = []; + const statuses = []; + const result = await waitForDeviceAuthorization({ + client, + deviceCode: 'secret-device-code', + userCode: 'ABCD-EFGH', + verifyUrl: 'https://commonly.me/cli/authorize', + stdin: new EventEmitter(), + wait: async (ms) => waits.push(ms), + onStatus: (message) => statuses.push(message), + now: () => 1, + }); + + expect(result).toMatchObject({ token: 'cm_once', username: 'lily' }); + expect(waits).toEqual([5000, 10000]); + expect(statuses).toEqual(['Waiting for browser approval (slowing down)…']); + expect(client.post).toHaveBeenCalledWith('/api/auth/device/poll', { deviceCode: 'secret-device-code' }); + }); + + test('q cancels without waiting for the device timeout', async () => { + const stdin = new EventEmitter(); + const client = { post: jest.fn(async () => { + stdin.emit('keypress', 'q', { name: 'q' }); + return { status: 'authorization_pending' }; + }) }; + + await expect(waitForDeviceAuthorization({ + client, + deviceCode: 'secret-device-code', + userCode: 'ABCD-EFGH', + verifyUrl: 'https://commonly.me/cli/authorize', + stdin, + // The keypress arrives after a pending response; cancellation must wake + // the sleep rather than waiting for the next polling interval. + wait: () => new Promise(() => {}), + now: () => 1, + })).rejects.toBeInstanceOf(DeviceLoginCancelledError); + }); + + test('o opens the code-prefilled authorization URL', async () => { + const calls = []; + await openBrowser('https://commonly.me/cli/authorize?code=ABCD-EFGH', (command, args, callback) => { + calls.push([command, args]); + callback(null); + }, 'darwin'); + expect(calls).toEqual([['open', ['https://commonly.me/cli/authorize?code=ABCD-EFGH']]]); + }); + + test('whoami differentiates a no-expiry device token from an expired JWT', () => { + expect(formatTokenStatus('cm_device', 'device', 0)).toBe('device token · no expiry'); + const expiredJwt = `header.${Buffer.from(JSON.stringify({ exp: 1 })).toString('base64url')}.signature`; + expect(formatTokenStatus(expiredJwt, 'jwt', 1001, 'dev')).toBe('expired — commonly login --instance dev'); + }); +}); diff --git a/cli/__tests__/lib.test.mjs b/cli/__tests__/lib.test.mjs index 8d513395d..ba2ffeb85 100644 --- a/cli/__tests__/lib.test.mjs +++ b/cli/__tests__/lib.test.mjs @@ -22,6 +22,7 @@ import http from 'http'; // a fresh temp directory for each test suite run. const configTmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'cli-config-test-')); +const configFile = path.join(configTmpDir, '.commonly', 'config.json'); await jest.unstable_mockModule('os', () => { const actual = os; @@ -52,8 +53,6 @@ const { // ── config.js tests ─────────────────────────────────────────────────────────── describe('config.js', () => { - const configFile = path.join(configTmpDir, '.commonly', 'config.json'); - beforeEach(() => { // Remove any leftover config between tests if (fs.existsSync(configFile)) fs.unlinkSync(configFile); diff --git a/cli/src/commands/login.js b/cli/src/commands/login.js index a6514d346..d2b45a244 100644 --- a/cli/src/commands/login.js +++ b/cli/src/commands/login.js @@ -6,8 +6,10 @@ */ import { createInterface } from 'readline'; -import { login as apiLogin } from '../lib/api.js'; +import { hostname } from 'os'; +import { createClient, login as apiLogin } from '../lib/api.js'; import { saveInstance } from '../lib/config.js'; +import { DeviceLoginCancelledError, waitForDeviceAuthorization } from '../lib/device-login.js'; const prompt = (rl, question) => new Promise((resolve) => rl.question(question, resolve)); @@ -41,6 +43,7 @@ export const registerLogin = (program) => { .description('Authenticate to a Commonly instance') .option('--instance ', 'Instance URL (default: https://api.commonly.me)') .option('--key ', 'Config key to save as (default: "default" or "local")') + .option('--password', 'Use the legacy email/password prompt instead of device authorization') .addHelpText('after', ` Examples: $ commonly login # production (default key) @@ -58,26 +61,57 @@ Tokens are stored in ~/.commonly/config.json. Other commands take const isLocal = instanceUrl.includes('localhost') || instanceUrl.includes('127.0.0.1'); const configKey = opts.key || (isLocal ? 'local' : 'default'); - console.log(`Logging in to ${instanceUrl}`); - - const rl = createInterface({ input: process.stdin, output: process.stdout }); - const email = await prompt(rl, 'Email: '); - rl.close(); - - const password = await promptSecret('Password: '); - try { + if (!opts.password) { + const client = createClient({ instance: instanceUrl, token: null }); + const started = await client.post('/api/auth/device/start', { + clientName: 'commonly-cli', + clientVersion: program.version(), + hostname: hostname(), + }); + console.log(`Open ${started.verifyUrl}`); + console.log(`Enter code: ${started.userCode}`); + console.log('Press o to open your browser, or q to cancel.'); + const data = await waitForDeviceAuthorization({ + client, + deviceCode: started.deviceCode, + userCode: started.userCode, + verifyUrl: started.verifyUrl, + interval: started.interval, + expiresIn: started.expiresIn, + onStatus: (message) => console.log(message), + }); + + saveInstance({ + key: configKey, + url: instanceUrl, + token: data.token, + userId: data.userId, + username: data.username, + tokenType: 'device', + }); + console.log(`\nLogged in as ${data.username} (${configKey})`); + console.log('Device token saved to ~/.commonly/config.json'); + return; + } + + console.log(`Logging in to ${instanceUrl}`); + const rl = createInterface({ input: process.stdin, output: process.stdout }); + const email = await prompt(rl, 'Email: '); + rl.close(); + const password = await promptSecret('Password: '); const data = await apiLogin(instanceUrl, email.trim(), password); const token = data.token; const userId = data.user?._id || data.user?.id; const username = data.user?.username; - saveInstance({ key: configKey, url: instanceUrl, token, userId, username }); + saveInstance({ key: configKey, url: instanceUrl, token, userId, username, tokenType: 'jwt' }); console.log(`\nLogged in as ${username} (${configKey})`); console.log(`Token saved to ~/.commonly/config.json`); } catch (err) { - console.error(`Login failed: ${err.message}`); + const message = err instanceof DeviceLoginCancelledError ? err.message : `Login failed: ${err.message}`; + console.error(message); process.exit(1); } }); @@ -97,9 +131,29 @@ export const registerWhoami = (program) => { return; } - instances.forEach(({ key, url, username, active, savedAt }) => { + instances.forEach(({ key, url, username, active, token, tokenType }) => { const marker = active ? '→' : ' '; - console.log(`${marker} ${key} ${username || '?'}@${url} (saved ${new Date(savedAt).toLocaleDateString()})`); + console.log(`${marker} ${key} ${username || '?'}@${url} (${formatTokenStatus(token, tokenType, Date.now(), key)})`); }); }); }; + +const decodeJwtExpiry = (token) => { + if (typeof token !== 'string' || token.split('.').length !== 3) return null; + try { + const payload = JSON.parse(Buffer.from(token.split('.')[1], 'base64url').toString('utf8')); + return typeof payload.exp === 'number' ? payload.exp * 1000 : null; + } catch { + return null; + } +}; + +export const formatTokenStatus = (token, tokenType, now = Date.now(), instanceKey = 'default') => { + if (tokenType === 'device' || String(token || '').startsWith('cm_')) return 'device token · no expiry'; + const expiresAt = decodeJwtExpiry(token); + if (!expiresAt) return 'session token · expiry unknown'; + const diff = expiresAt - now; + if (diff <= 0) return `expired — commonly login --instance ${instanceKey}`; + const hours = Math.max(1, Math.ceil(diff / (60 * 60 * 1000))); + return `expires in ${hours}h`; +}; diff --git a/cli/src/lib/api.js b/cli/src/lib/api.js index 85deb001f..a836a50fc 100644 --- a/cli/src/lib/api.js +++ b/cli/src/lib/api.js @@ -5,7 +5,7 @@ * unless overridden. This is the only place that makes HTTP calls. */ -import { resolveInstanceUrl, getToken } from './config.js'; +import { resolveInstanceUrl, getToken, resolveInstance } from './config.js'; const headers = (token, extra = {}) => ({ 'Content-Type': 'application/json', @@ -13,12 +13,21 @@ const headers = (token, extra = {}) => ({ ...extra, }); -const handleResponse = async (res) => { +const knownSessionFailure = (body) => [body?.msg, body?.error, body?.message] + .some((value) => ['Token is not valid', 'Invalid API token', 'Account no longer exists'].includes(value)); + +export const sessionExpiredMessage = ({ instanceKey, baseUrl }) => ( + `Session for ${instanceKey} (${baseUrl}) has expired.\nRun: commonly login --instance ${instanceKey}` +); + +const handleResponse = async (res, session = null) => { const text = await res.text(); let body; try { body = JSON.parse(text); } catch { body = { message: text }; } if (!res.ok) { - const msg = body?.error || body?.message || `HTTP ${res.status}`; + const msg = res.status === 401 && session && knownSessionFailure(body) + ? sessionExpiredMessage(session) + : body?.error || body?.message || body?.msg || `HTTP ${res.status}`; const err = new Error(msg); err.status = res.status; err.body = body; @@ -27,32 +36,37 @@ const handleResponse = async (res) => { return body; }; -export const createClient = ({ instance = null, token = null } = {}) => { +export const createClient = ({ instance = null, token = undefined } = {}) => { const baseUrl = resolveInstanceUrl(instance); - const authToken = token || getToken(instance); + const resolved = resolveInstance(instance); + const authToken = token === undefined ? getToken(instance) : token; + const session = { + instanceKey: resolved?.key || (typeof instance === 'string' && instance && !/^https?:\/\//i.test(instance) ? instance : 'default'), + baseUrl, + }; const get = (path, params = {}) => { const url = new URL(`${baseUrl}${path}`); Object.entries(params).forEach(([k, v]) => v != null && url.searchParams.set(k, v)); - return fetch(url.toString(), { headers: headers(authToken) }).then(handleResponse); + return fetch(url.toString(), { headers: headers(authToken) }).then((res) => handleResponse(res, session)); }; const post = (path, body = {}) => fetch(`${baseUrl}${path}`, { method: 'POST', headers: headers(authToken), body: JSON.stringify(body), - }).then(handleResponse); + }).then((res) => handleResponse(res, session)); const patch = (path, body = {}) => fetch(`${baseUrl}${path}`, { method: 'PATCH', headers: headers(authToken), body: JSON.stringify(body), - }).then(handleResponse); + }).then((res) => handleResponse(res, session)); const del = (path) => fetch(`${baseUrl}${path}`, { method: 'DELETE', headers: headers(authToken), - }).then(handleResponse); + }).then((res) => handleResponse(res, session)); // Multipart upload via native FormData/Blob (Node 18+) — no runtime deps. // Content-Type is deliberately NOT set: fetch writes the multipart boundary. @@ -72,7 +86,7 @@ export const createClient = ({ instance = null, token = null } = {}) => { method: 'POST', headers: authToken ? { Authorization: `Bearer ${authToken}` } : {}, body: form, - }).then(handleResponse); + }).then((res) => handleResponse(res, session)); }; return { diff --git a/cli/src/lib/config.js b/cli/src/lib/config.js index 448fe139e..547fae241 100644 --- a/cli/src/lib/config.js +++ b/cli/src/lib/config.js @@ -107,13 +107,16 @@ export const resolveInstanceUrl = (instanceArg = null) => { return DEFAULT_INSTANCE_URL; }; -export const saveInstance = ({ key = 'default', url, token, userId, username }) => { +export const saveInstance = ({ key = 'default', url, token, userId, username, tokenType = null }) => { const config = read(); config.instances[key] = { url: url.replace(/\/$/, ''), token, userId, username, + // Device bearers are long-lived until revoked. Preserve the explicit kind + // so `whoami` can say that without guessing from a future token format. + ...(tokenType ? { tokenType } : {}), savedAt: new Date().toISOString(), }; config.active = key; diff --git a/cli/src/lib/device-login.js b/cli/src/lib/device-login.js new file mode 100644 index 000000000..4d3ebf764 --- /dev/null +++ b/cli/src/lib/device-login.js @@ -0,0 +1,98 @@ +/** + * RFC 8628-shaped device-code interaction for `commonly login`. + * + * The browser owns password/OAuth; the terminal only ever receives the + * resulting per-device bearer once from /device/poll. This module keeps the + * timing and terminal-key behaviour testable without a live TTY. + */ +import { execFile as nodeExecFile } from 'child_process'; +import { platform } from 'os'; +import { emitKeypressEvents } from 'readline'; + +const sleep = (ms) => new Promise((resolve) => setTimeout(resolve, ms)); + +export class DeviceLoginCancelledError extends Error { + constructor() { + super('Login cancelled.'); + this.name = 'DeviceLoginCancelledError'; + } +} + +export const openBrowser = (url, execFile = nodeExecFile, currentPlatform = platform()) => { + const command = currentPlatform === 'darwin' ? 'open' : currentPlatform === 'win32' ? 'cmd' : 'xdg-open'; + const args = currentPlatform === 'win32' ? ['/c', 'start', '', url] : [url]; + return new Promise((resolve) => { + execFile(command, args, () => resolve()); + }); +}; + +export const waitForDeviceAuthorization = async ({ + client, + deviceCode, + userCode, + verifyUrl, + interval = 5, + expiresIn = 600, + stdin = process.stdin, + wait = sleep, + onOpen = openBrowser, + onStatus = () => undefined, + now = () => Date.now(), +}) => { + const authorizeUrl = `${verifyUrl}?code=${encodeURIComponent(userCode)}`; + let cancelled = false; + let signalCancellation = () => {}; + const cancellation = new Promise((resolve) => { + signalCancellation = resolve; + }); + let currentInterval = Math.max(Number(interval) || 5, 1); + const deadline = now() + Math.max(Number(expiresIn) || 600, 1) * 1000; + const isTty = Boolean(stdin?.isTTY && typeof stdin.setRawMode === 'function'); + + const onKeypress = (value, key = {}) => { + if (key?.ctrl && key.name === 'c') { + cancelled = true; + signalCancellation(); + } + if (value === 'q') { + cancelled = true; + signalCancellation(); + } + if (value === 'o') void onOpen(authorizeUrl); + }; + + if (stdin?.on) { + emitKeypressEvents(stdin); + if (isTty) stdin.setRawMode(true); + stdin.on('keypress', onKeypress); + } + + try { + while (now() < deadline) { + if (cancelled) throw new DeviceLoginCancelledError(); + let result; + try { + result = await client.post('/api/auth/device/poll', { deviceCode }); + } catch { + throw new Error('Unable to complete device authorization. Try again.'); + } + if (result?.status === 'authorized' && result.token) return result; + if (result?.status === 'denied') throw new Error('Authorization was denied in your browser.'); + if (result?.status === 'expired') throw new Error('Authorization code expired. Run commonly login again.'); + if (result?.status === 'slow_down') { + currentInterval *= 2; + onStatus('Waiting for browser approval (slowing down)…'); + } else if (result?.status !== 'authorization_pending') { + throw new Error('Authorization code expired. Run commonly login again.'); + } + await Promise.race([ + wait(currentInterval * 1000), + cancellation, + ]); + } + throw new Error('Authorization code expired. Run commonly login again.'); + } finally { + if (stdin?.removeListener) stdin.removeListener('keypress', onKeypress); + if (isTty) stdin.setRawMode(false); + } +}; diff --git a/frontend/src/App.tsx b/frontend/src/App.tsx index 1ca455cb2..a2eefab75 100644 --- a/frontend/src/App.tsx +++ b/frontend/src/App.tsx @@ -20,6 +20,7 @@ import { AuthProvider, useAuth } from './context/AuthContext'; import { SocketProvider } from './context/SocketContext'; import { LayoutProvider } from './context/LayoutContext'; import V2App from './v2/V2App'; +import V2CliAuthorize from './v2/components/V2CliAuthorize'; import V2LandingPage from './v2/landing/V2LandingPage'; import { setupFocusManagement } from './utils/focusUtils'; import { checkAndRefresh } from './utils/refreshUtils'; @@ -272,6 +273,8 @@ function App(): React.ReactElement {
+ } /> + } /> } /> } /> {/* Canonical public routes also have static build output. */} diff --git a/frontend/src/v2/V2App.tsx b/frontend/src/v2/V2App.tsx index e426de00d..6a42ee2a7 100644 --- a/frontend/src/v2/V2App.tsx +++ b/frontend/src/v2/V2App.tsx @@ -16,6 +16,7 @@ import VerifyEmail from '../components/VerifyEmail'; import DiscordCallback from '../components/DiscordCallback'; import V2Showcase from './showcase/V2Showcase'; import V2BillingPanel from './components/V2BillingPanel'; +import V2DevicesPanel from './components/V2DevicesPanel'; import V2AgentProfile from './agents/V2AgentProfile'; import PostFeed from '../components/PostFeed'; import Thread from '../components/Thread'; @@ -324,10 +325,15 @@ const V2App: React.FC = () => { element={feature('Settings', 'Plan and billing, profile, avatar, app management, and API token settings.', ( <> + ))} /> + )} + /> )} diff --git a/frontend/src/v2/__tests__/V2CliAuthorize.test.tsx b/frontend/src/v2/__tests__/V2CliAuthorize.test.tsx new file mode 100644 index 000000000..b916e9740 --- /dev/null +++ b/frontend/src/v2/__tests__/V2CliAuthorize.test.tsx @@ -0,0 +1,62 @@ +// @ts-nocheck +import React from 'react'; +import { fireEvent, render, screen, waitFor } from '@testing-library/react'; +import { MemoryRouter } from 'react-router-dom'; +import axios from 'axios'; +import { AuthContext } from '../../context/AuthContext'; +import V2CliAuthorize from '../components/V2CliAuthorize'; + +jest.mock('axios', () => { + const mock = { + get: jest.fn(), post: jest.fn(), delete: jest.fn(), patch: jest.fn(), + defaults: { baseURL: '', headers: { common: {} } }, + interceptors: { request: { use: jest.fn(), eject: jest.fn() }, response: { use: jest.fn(), eject: jest.fn() } }, + }; + return { __esModule: true, default: mock, ...mock }; +}); + +const auth = { + currentUser: { _id: 'u1', username: 'lily', email: 'lily@example.com' }, + user: { _id: 'u1', username: 'lily', email: 'lily@example.com' }, + token: 'jwt', loading: false, error: null, isAuthenticated: true, + register: jest.fn(), login: jest.fn(), logout: jest.fn(), updateProfile: jest.fn(), +}; + +const renderPage = (path = '/cli/authorize?code=ABCD-EFGH', value = auth) => render( + + + , +); + +afterEach(() => jest.clearAllMocks()); + +describe('V2CliAuthorize', () => { + test('prefills a terminal code, confirms the device facts, then completes approval', async () => { + axios.post + .mockResolvedValueOnce({ data: { status: 'pending', request: { hostname: 'sam-laptop', clientName: 'commonly-cli', clientVersion: '0.1.26', createdAt: '2026-08-31T00:00:00.000Z' } } }) + .mockResolvedValueOnce({ data: { status: 'authorized' } }); + renderPage(); + + expect(screen.getByLabelText('Device code')).toHaveValue('ABCD-EFGH'); + fireEvent.click(screen.getByRole('button', { name: 'Continue' })); + await waitFor(() => expect(axios.post).toHaveBeenCalledWith('/api/auth/device/authorize', { userCode: 'ABCD-EFGH' })); + expect(await screen.findByText('Allow this device?')).toBeInTheDocument(); + expect(screen.getByText('sam-laptop')).toBeInTheDocument(); + expect(screen.getByText('api.commonly.me')).toBeInTheDocument(); + + fireEvent.click(screen.getByRole('button', { name: 'Authorize' })); + await waitFor(() => expect(axios.post).toHaveBeenLastCalledWith('/api/auth/device/authorize', { + userCode: 'ABCD-EFGH', decision: 'authorize', + })); + expect(await screen.findByText('Device authorized')).toBeInTheDocument(); + }); + + test('signed-out users get an explicit return path after sign-in', () => { + renderPage('/cli/authorize?code=ABCD-EFGH', { ...auth, currentUser: null, user: null, token: null, isAuthenticated: false }); + expect(screen.getByRole('link', { name: 'Sign in to continue' })).toHaveAttribute( + 'href', + expect.stringContaining('next=%2Fcli%2Fauthorize%3Fcode%3DABCD-EFGH'), + ); + expect(screen.getByLabelText('Device code')).toBeDisabled(); + }); +}); diff --git a/frontend/src/v2/__tests__/V2DevicesPanel.test.tsx b/frontend/src/v2/__tests__/V2DevicesPanel.test.tsx new file mode 100644 index 000000000..24cafe593 --- /dev/null +++ b/frontend/src/v2/__tests__/V2DevicesPanel.test.tsx @@ -0,0 +1,29 @@ +// @ts-nocheck +import React from 'react'; +import { fireEvent, render, screen, waitFor } from '@testing-library/react'; +import axios from 'axios'; +import V2DevicesPanel from '../components/V2DevicesPanel'; + +jest.mock('axios', () => { + const mock = { + get: jest.fn(), post: jest.fn(), delete: jest.fn(), patch: jest.fn(), + defaults: { baseURL: '', headers: { common: {} } }, + interceptors: { request: { use: jest.fn(), eject: jest.fn() }, response: { use: jest.fn(), eject: jest.fn() } }, + }; + return { __esModule: true, default: mock, ...mock }; +}); + +afterEach(() => jest.clearAllMocks()); + +test('lists a device without exposing its bearer and revokes it in place', async () => { + axios.get.mockResolvedValue({ data: { devices: [{ id: 'd1', label: 'sam-laptop · commonly-cli', createdAt: '2026-08-31T00:00:00.000Z', lastUsedAt: null, revokedAt: null }] } }); + axios.delete.mockResolvedValue({ data: { message: 'Device revoked' } }); + jest.spyOn(window, 'confirm').mockReturnValue(true); + render(); + + expect(await screen.findByText('sam-laptop · commonly-cli')).toBeInTheDocument(); + expect(screen.queryByText(/cm_[a-f0-9]/)).not.toBeInTheDocument(); + fireEvent.click(screen.getByRole('button', { name: 'Revoke' })); + await waitFor(() => expect(axios.delete).toHaveBeenCalledWith('/api/auth/devices/d1')); + expect(await screen.findByText('Revoked')).toBeInTheDocument(); +}); diff --git a/frontend/src/v2/components/V2CliAuthorize.css b/frontend/src/v2/components/V2CliAuthorize.css new file mode 100644 index 000000000..a8c313ca3 --- /dev/null +++ b/frontend/src/v2/components/V2CliAuthorize.css @@ -0,0 +1,55 @@ +.v2-cli-authorize { + min-height: 100vh; + display: grid; + place-items: center; + padding: 24px; + background: #f8f8fb; + color: #111827; + font-family: Inter, ui-sans-serif, system-ui, -apple-system, BlinkMacSystemFont, "Segoe UI", sans-serif; +} + +.v2-cli-authorize__card { + width: min(100%, 460px); + padding: 32px; + border: 1px solid #e5e7eb; + border-radius: 18px; + background: #fff; + box-shadow: 0 12px 32px rgba(17, 24, 39, 0.08); +} + +.v2-cli-authorize__card form { margin: 0; padding: 0; border: 0; background: transparent; box-shadow: none; } + +.v2-cli-authorize__mark { + display: grid; + width: 36px; + height: 36px; + place-items: center; + border-radius: 10px; + background: #2f6feb; + color: #fff; + font-weight: 800; +} + +.v2-cli-authorize__card h1 { margin: 18px 0 8px; font-size: 24px; } +.v2-cli-authorize__card p { margin: 0 0 20px; color: #4b5563; line-height: 1.5; } +.v2-cli-authorize__field { display: grid; gap: 7px; margin-bottom: 18px; font-size: 13px; font-weight: 700; } +.v2-cli-authorize__field input { width: 100%; box-sizing: border-box; padding: 12px; border: 1px solid #d7dce7; border-radius: 9px; font: 600 18px/1.2 ui-monospace, SFMono-Regular, Menlo, monospace; letter-spacing: .08em; } +.v2-cli-authorize__field input:focus { outline: none; border-color: #2f6feb; box-shadow: 0 0 0 3px rgba(47, 111, 235, .18); } +.v2-cli-authorize__primary, .v2-cli-authorize__secondary { display: inline-flex; justify-content: center; align-items: center; min-height: 42px; padding: 0 16px; border-radius: 9px; font: inherit; font-weight: 700; text-decoration: none; cursor: pointer; } +.v2-cli-authorize__primary { border: 1px solid #2f6feb; background: #2f6feb; color: #fff; } +.v2-cli-authorize__primary:disabled { cursor: wait; opacity: .65; } +.v2-cli-authorize__secondary { border: 1px solid #d7dce7; background: #fff; color: #374151; } +.v2-cli-authorize__facts { margin: 0 0 18px; padding: 14px; border-radius: 10px; background: #f7f7fa; } +.v2-cli-authorize__facts div { display: flex; justify-content: space-between; gap: 18px; padding: 5px 0; } +.v2-cli-authorize__facts dt { color: #6b7280; } +.v2-cli-authorize__facts dd { margin: 0; font-weight: 600; text-align: right; } +.v2-cli-authorize__warning { padding: 10px 12px; border-radius: 8px; background: #fdefdc; color: #7c4a03 !important; font-size: 13px; } +.v2-cli-authorize__actions { display: flex; justify-content: flex-end; gap: 10px; } + +@media (max-width: 480px) { + .v2-cli-authorize { padding: 16px; align-items: start; } + .v2-cli-authorize__card { padding: 24px 20px; } + .v2-cli-authorize__facts div { display: grid; gap: 2px; } + .v2-cli-authorize__facts dd { text-align: left; } + .v2-cli-authorize__actions { display: grid; grid-template-columns: 1fr 1fr; } +} diff --git a/frontend/src/v2/components/V2CliAuthorize.tsx b/frontend/src/v2/components/V2CliAuthorize.tsx new file mode 100644 index 000000000..38e4ab729 --- /dev/null +++ b/frontend/src/v2/components/V2CliAuthorize.tsx @@ -0,0 +1,138 @@ +import React, { useEffect, useMemo, useState } from 'react'; +import { Link, useLocation } from 'react-router-dom'; +import axios from '../../utils/axiosConfig'; +import { useAuth } from '../../context/AuthContext'; +import './V2CliAuthorize.css'; + +interface AuthorizationRequest { + hostname: string; + clientName: string; + clientVersion?: string | null; + createdAt: string; +} + +type Screen = 'code' | 'confirm' | 'done' | 'denied' | 'expired' | 'error'; + +const normalizeCode = (value: string): string => { + const compact = value.toUpperCase().replace(/[^A-Z2-9]/g, '').slice(0, 8); + return compact.length > 4 ? `${compact.slice(0, 4)}-${compact.slice(4)}` : compact; +}; + +const errorScreen = (error: unknown): Screen => { + const status = (error as { response?: { status?: number } })?.response?.status; + return status === 410 ? 'expired' : 'error'; +}; + +const V2CliAuthorize: React.FC = () => { + const { isAuthenticated, loading } = useAuth(); + const location = useLocation(); + const queryCode = useMemo(() => normalizeCode(new URLSearchParams(location.search).get('code') || ''), [location.search]); + const [code, setCode] = useState(queryCode); + const [screen, setScreen] = useState('code'); + const [request, setRequest] = useState(null); + const [submitting, setSubmitting] = useState(false); + + useEffect(() => setCode(queryCode), [queryCode]); + + const lookup = async (event?: React.FormEvent) => { + event?.preventDefault(); + if (code.replace('-', '').length !== 8) return; + setSubmitting(true); + try { + const response = await axios.post<{ status: string; request?: AuthorizationRequest }>('/api/auth/device/authorize', { + userCode: code, + }); + if (response.data.status === 'pending' && response.data.request) { + setRequest(response.data.request); + setScreen('confirm'); + } else { + setScreen(response.data.status === 'denied' ? 'denied' : 'expired'); + } + } catch (error) { + setScreen(errorScreen(error)); + } finally { + setSubmitting(false); + } + }; + + const decide = async (decision: 'authorize' | 'deny') => { + setSubmitting(true); + try { + const response = await axios.post<{ status: string }>('/api/auth/device/authorize', { + userCode: code, + decision, + }); + setScreen(response.data.status === 'authorized' ? 'done' : 'denied'); + } catch (error) { + setScreen(errorScreen(error)); + } finally { + setSubmitting(false); + } + }; + + const next = `/cli/authorize${code ? `?code=${encodeURIComponent(code)}` : ''}`; + + return ( +
+
+ + {loading &&

Checking your session…

} + {!loading && !isAuthenticated && ( + <> +

Authorize Commonly CLI

+

Sign in to approve this device. The code stays tied to this browser tab.

+ + Sign in to continue + + )} + {!loading && isAuthenticated && screen === 'code' && ( +
+

Authorize Commonly CLI

+

Enter the code shown in your terminal.

+ + +
+ )} + {!loading && isAuthenticated && screen === 'confirm' && request && ( + <> +

Allow this device?

+

{request.hostname} is asking to use your Commonly account.

+
+
Client
{request.clientName}{request.clientVersion ? ` ${request.clientVersion}` : ''}
+
Instance
api.commonly.me
+
Requested
{new Date(request.createdAt).toLocaleString()}
+
+

Only approve a code you requested from your own terminal.

+
+ + +
+ + )} + {!loading && isAuthenticated && screen === 'done' && <>

Device authorized

You can return to your terminal.

} + {!loading && isAuthenticated && screen === 'denied' && <>

Authorization denied

No token was issued for this device.

} + {!loading && isAuthenticated && screen === 'expired' && <>

Code expired

Return to your terminal and run commonly login again.

} + {!loading && isAuthenticated && screen === 'error' && <>

Couldn’t authorize this device

Check the code and try again from your terminal.

} +
+
+ ); +}; + +export default V2CliAuthorize; diff --git a/frontend/src/v2/components/V2DevicesPanel.css b/frontend/src/v2/components/V2DevicesPanel.css new file mode 100644 index 000000000..7d7863ad0 --- /dev/null +++ b/frontend/src/v2/components/V2DevicesPanel.css @@ -0,0 +1,13 @@ +.v2-devices { margin: 0 0 24px; padding: 22px; border: 1px solid var(--v2-border); border-radius: var(--v2-radius-lg); background: var(--v2-surface); } +.v2-devices__heading { display: flex; justify-content: space-between; gap: 16px; align-items: start; margin-bottom: 18px; } +.v2-devices h2 { margin: 0; font-size: 18px; color: var(--v2-text-primary); } +.v2-devices p { margin: 5px 0 0; color: var(--v2-text-secondary); } +.v2-devices button { border: 1px solid var(--v2-border-strong); border-radius: 7px; padding: 7px 10px; background: #fff; color: var(--v2-text-primary); font: inherit; font-size: 13px; font-weight: 700; cursor: pointer; } +.v2-devices__list { display: grid; gap: 10px; } +.v2-devices__item { display: flex; justify-content: space-between; gap: 14px; align-items: center; padding: 13px; border: 1px solid var(--v2-border-soft); border-radius: 9px; } +.v2-devices__item strong, .v2-devices__item span { display: block; } +.v2-devices__item span { margin-top: 4px; color: var(--v2-text-tertiary); font-size: 12px; } +.v2-devices__item .v2-devices__revoked { margin: 0; color: var(--v2-text-tertiary); font-weight: 700; } +.v2-devices__error { color: var(--v2-danger) !important; } +.v2-devices__empty { padding: 12px; border-radius: 8px; background: var(--v2-bg-subtle); } +@media (max-width: 560px) { .v2-devices { padding: 17px; } .v2-devices__heading, .v2-devices__item { display: grid; } .v2-devices__item button { width: fit-content; } } diff --git a/frontend/src/v2/components/V2DevicesPanel.tsx b/frontend/src/v2/components/V2DevicesPanel.tsx new file mode 100644 index 000000000..a72f8d63a --- /dev/null +++ b/frontend/src/v2/components/V2DevicesPanel.tsx @@ -0,0 +1,61 @@ +import React, { useCallback, useEffect, useState } from 'react'; +import axios from '../../utils/axiosConfig'; +import './V2DevicesPanel.css'; + +interface Device { + id: string; + label: string; + createdAt: string; + lastUsedAt: string | null; + revokedAt: string | null; +} + +const dateLabel = (value: string | null) => (value ? new Date(value).toLocaleString() : 'Never'); + +const V2DevicesPanel: React.FC = () => { + const [devices, setDevices] = useState([]); + const [loading, setLoading] = useState(true); + const [error, setError] = useState(null); + + const load = useCallback(async () => { + try { + setLoading(true); + const response = await axios.get<{ devices: Device[] }>('/api/auth/devices'); + setDevices(response.data.devices); + setError(null); + } catch { + setError('Couldn’t load your devices.'); + } finally { + setLoading(false); + } + }, []); + + useEffect(() => { void load(); }, [load]); + + const revoke = async (device: Device) => { + if (!window.confirm(`Revoke ${device.label}? This CLI will need to sign in again.`)) return; + try { + await axios.delete(`/api/auth/devices/${device.id}`); + setDevices((current) => current.map((entry) => entry.id === device.id ? { ...entry, revokedAt: new Date().toISOString() } : entry)); + } catch { + setError('Couldn’t revoke that device.'); + } + }; + + return ( +
+

Devices

CLI device tokens are long-lived until you revoke them.

+ {error &&

{error}

} + {loading &&

Loading devices…

} + {!loading && devices.length === 0 &&

No CLI devices are connected.

} + {!loading && devices.length > 0 &&
+ {devices.map((device) =>
+
{device.label}Created {dateLabel(device.createdAt)} · Last used {dateLabel(device.lastUsedAt)}
+ {device.revokedAt ? Revoked : } +
)} +
} +
+ ); +}; + +export default V2DevicesPanel; From 4b5c87e86b16deebbfd121b4a39e3571b0a4f6c2 Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Sun, 30 Aug 2026 21:02:26 -0700 Subject: [PATCH 2/9] fix(cli): harden device login flow --- backend/__tests__/service/auth.test.js | 27 +++++++++++++ backend/routes/auth.ts | 20 ++++++++-- .../services/deviceAuthorizationService.ts | 36 ++++++++++++++--- cli/__tests__/api-expiry.test.mjs | 9 +++-- cli/__tests__/device-login.test.mjs | 28 +++++++++++++ cli/package.json | 2 +- cli/src/commands/login.js | 26 ++++++++---- cli/src/lib/device-login.js | 40 ++++++++++++++++--- .../src/v2/__tests__/V2CliAuthorize.test.tsx | 13 ++++-- frontend/src/v2/components/V2CliAuthorize.css | 1 + frontend/src/v2/components/V2CliAuthorize.tsx | 36 +++++++++++++---- 11 files changed, 202 insertions(+), 36 deletions(-) diff --git a/backend/__tests__/service/auth.test.js b/backend/__tests__/service/auth.test.js index 475baa855..9a0e9a150 100644 --- a/backend/__tests__/service/auth.test.js +++ b/backend/__tests__/service/auth.test.js @@ -554,6 +554,33 @@ describe('Auth Routes Integration Tests', () => { .expect({ status: 'expired' }); expect((await User.findById(user._id).select('deviceTokens')).deviceTokens).toHaveLength(0); }); + + it('revokes an approved bearer when its terminal misses the expiry deadline', async () => { + const user = await createVerifiedUser(); + const browserToken = generateTestToken(user._id); + const started = await startAuthorization(); + + await request(app) + .post('/api/auth/device/authorize') + .set('Authorization', `Bearer ${browserToken}`) + .send({ userCode: started.body.userCode, decision: 'authorize' }) + .expect(200) + .expect({ status: 'authorized' }); + + await DeviceAuthorization.updateOne( + { deviceCodeHash: hashDeviceCredential(started.body.deviceCode) }, + { $set: { expiresAt: new Date(Date.now() - 1000) } }, + ); + await request(app) + .post('/api/auth/device/poll') + .send({ deviceCode: started.body.deviceCode }) + .expect(200) + .expect({ status: 'expired' }); + + const device = (await User.findById(user._id).select('deviceTokens')).deviceTokens[0]; + expect(device.revokedAt).toBeTruthy(); + expect((await DeviceAuthorization.findOne().select('+pendingToken')).pendingToken).toBeUndefined(); + }); }); describe('PUT /api/auth/profile', () => { diff --git a/backend/routes/auth.ts b/backend/routes/auth.ts index c5aa7f217..40de11958 100644 --- a/backend/routes/auth.ts +++ b/backend/routes/auth.ts @@ -105,6 +105,20 @@ const devicePollLimiter = rateLimit({ handler: rateLimitHandler('rate limit exceeded: too many device authorization polls'), }); +// The browser approval and device-management endpoints all authenticate, but +// authentication itself reads User (and approval writes both collections). +// Keep their bound separate from the public start/poll buckets: a terminal +// polling normally must never consume the browser approval budget. +const deviceManageLimiter = rateLimit({ + windowMs: 15 * 60 * 1000, + max: 60, + standardHeaders: true, + legacyHeaders: false, + skip: () => process.env.NODE_ENV === 'test', + keyGenerator: cloudflareIpRateLimitKeyGenerator, + handler: rateLimitHandler('rate limit exceeded: too many device authorization requests'), +}); + // Waitlist is a one-shot action per person — 5/hour/IP. const waitlistLimiter = rateLimit({ windowMs: 60 * 60 * 1000, @@ -181,7 +195,7 @@ router.post('/device/poll', devicePollLimiter, async (req: any, res: Res) => { } }); -router.post('/device/authorize', auth, async (req: AuthReq & { body?: any }, res: Res) => { +router.post('/device/authorize', deviceManageLimiter, auth, async (req: AuthReq & { body?: any }, res: Res) => { try { const result = await decideDeviceAuthorization({ userCode: req.body?.userCode, @@ -197,7 +211,7 @@ router.post('/device/authorize', auth, async (req: AuthReq & { body?: any }, res } }); -router.get('/devices', auth, async (req: AuthReq, res: Res) => { +router.get('/devices', deviceManageLimiter, auth, async (req: AuthReq, res: Res) => { try { return res.json({ devices: await listDeviceTokens(req.userId || req.user?.id || '') }); } catch (error: any) { @@ -206,7 +220,7 @@ router.get('/devices', auth, async (req: AuthReq, res: Res) => { } }); -router.delete('/devices/:deviceId', auth, async (req: AuthReq & { params?: any }, res: Res) => { +router.delete('/devices/:deviceId', deviceManageLimiter, auth, async (req: AuthReq & { params?: any }, res: Res) => { try { const revoked = await revokeDeviceToken(req.userId || req.user?.id || '', String(req.params?.deviceId || '')); if (!revoked) return res.status(404).json({ error: 'Device not found or already revoked' }); diff --git a/backend/services/deviceAuthorizationService.ts b/backend/services/deviceAuthorizationService.ts index 793aba878..5cfae78cc 100644 --- a/backend/services/deviceAuthorizationService.ts +++ b/backend/services/deviceAuthorizationService.ts @@ -13,11 +13,13 @@ export const hashDeviceCredential = (value: string): string => crypto .digest('hex'); const randomUserCode = (): string => { - const bytes = crypto.randomBytes(8); - let code = ''; - for (let index = 0; index < 8; index += 1) { - code += USER_CODE_ALPHABET[bytes[index] % USER_CODE_ALPHABET.length]; - } + // `randomInt` uses rejection sampling. Indexing random bytes with `%` is + // biased whenever the alphabet length does not divide 256 (and CodeQL is + // right not to make that safety depend on this alphabet's current length). + const code = Array.from( + { length: 8 }, + () => USER_CODE_ALPHABET[crypto.randomInt(USER_CODE_ALPHABET.length)], + ).join(''); return `${code.slice(0, 4)}-${code.slice(4)}`; }; @@ -34,6 +36,21 @@ const userCodeHash = (value: unknown): string | null => { const isExpired = (request: IDeviceAuthorization, now = new Date()): boolean => request.expiresAt <= now; +const revokeUndeliveredDeviceToken = async (request: IDeviceAuthorization) => { + if (request.status !== 'authorized' || !request.pendingToken || !request.userId) return; + const tokenHash = hashDeviceCredential(request.pendingToken); + await User.updateOne( + { _id: request.userId, 'deviceTokens.tokenHash': tokenHash }, + { $set: { 'deviceTokens.$.revokedAt': new Date() } }, + ); + // The one-time secret has no remaining recipient. Do not retain it until + // Mongo's TTL reaper happens to remove this authorization row. + await DeviceAuthorization.updateOne( + { _id: request._id, status: 'authorized' }, + { $unset: { pendingToken: 1 } }, + ); +}; + export const createDeviceAuthorization = async ({ clientName, clientVersion, @@ -78,7 +95,14 @@ export const pollDeviceAuthorization = async (deviceCode: unknown) => { const request = await DeviceAuthorization.findOne({ deviceCodeHash: hashDeviceCredential(raw), }).select('+pendingToken'); - if (!request || isExpired(request)) return { status: 'expired' as const }; + if (!request) return { status: 'expired' as const }; + if (isExpired(request)) { + // A browser may approve during the final poll interval. If the terminal + // misses the handoff before the ten-minute deadline, that bearer was never + // delivered and must not remain as a ghost device in the account. + await revokeUndeliveredDeviceToken(request); + return { status: 'expired' as const }; + } if (request.status === 'pending') { const now = new Date(); const polledTooSoon = request.lastPolledAt diff --git a/cli/__tests__/api-expiry.test.mjs b/cli/__tests__/api-expiry.test.mjs index 309863031..2a2713e91 100644 --- a/cli/__tests__/api-expiry.test.mjs +++ b/cli/__tests__/api-expiry.test.mjs @@ -15,17 +15,20 @@ const { createClient } = await import('../src/lib/api.js'); afterAll(() => fs.rmSync(path.join(configTmpDir, '.commonly'), { recursive: true, force: true })); -test('replaces a server token message with an actionable saved-profile instruction', async () => { +test.each(['Token is not valid', 'Invalid API token', 'Account no longer exists'])( + 'replaces %s with an actionable saved-profile instruction', + async (serverMessage) => { saveInstance({ key: 'dev', url: 'https://api.commonly.me', token: 'stale-token', userId: 'u1', username: 'lily', }); global.fetch = jest.fn().mockResolvedValue({ ok: false, status: 401, - text: async () => JSON.stringify({ msg: 'Token is not valid' }), + text: async () => JSON.stringify({ msg: serverMessage }), }); await expect(createClient({ instance: 'dev' }).get('/api/auth/user')).rejects.toThrow( 'Session for dev (https://api.commonly.me) has expired.\nRun: commonly login --instance dev', ); -}); + }, +); diff --git a/cli/__tests__/device-login.test.mjs b/cli/__tests__/device-login.test.mjs index 3a1fbb703..94ad87753 100644 --- a/cli/__tests__/device-login.test.mjs +++ b/cli/__tests__/device-login.test.mjs @@ -2,6 +2,8 @@ import { EventEmitter } from 'events'; import { jest } from '@jest/globals'; import { DeviceLoginCancelledError, + DeviceLoginDeniedError, + DeviceLoginExpiredError, openBrowser, waitForDeviceAuthorization, } from '../src/lib/device-login.js'; @@ -63,6 +65,32 @@ describe('CLI device login', () => { expect(calls).toEqual([['open', ['https://commonly.me/cli/authorize?code=ABCD-EFGH']]]); }); + test('uses a non-shell opener on Windows and rejects non-web verification URLs', async () => { + const calls = []; + await openBrowser('https://commonly.me/cli/authorize?code=ABCD-EFGH', (command, args, callback) => { + calls.push([command, args]); + callback(null); + }, 'win32'); + expect(calls).toEqual([['rundll32', ['url.dll,FileProtocolHandler', 'https://commonly.me/cli/authorize?code=ABCD-EFGH']]]); + expect(() => openBrowser('file:///etc/passwd')).toThrow('Device authorization URL must use HTTP or HTTPS.'); + }); + + test('uses terminal-safe messages for denied and expired device codes', async () => { + const denied = { post: jest.fn().mockResolvedValue({ status: 'denied' }) }; + await expect(waitForDeviceAuthorization({ + client: denied, + deviceCode: 'secret-device-code', userCode: 'ABCD-EFGH', verifyUrl: 'https://commonly.me/cli/authorize', + stdin: new EventEmitter(), wait: async () => undefined, now: () => 1, + })).rejects.toBeInstanceOf(DeviceLoginDeniedError); + + const expired = { post: jest.fn().mockResolvedValue({ status: 'expired' }) }; + await expect(waitForDeviceAuthorization({ + client: expired, + deviceCode: 'secret-device-code', userCode: 'ABCD-EFGH', verifyUrl: 'https://commonly.me/cli/authorize', + stdin: new EventEmitter(), wait: async () => undefined, now: () => 1, + })).rejects.toBeInstanceOf(DeviceLoginExpiredError); + }); + test('whoami differentiates a no-expiry device token from an expired JWT', () => { expect(formatTokenStatus('cm_device', 'device', 0)).toBe('device token · no expiry'); const expiredJwt = `header.${Buffer.from(JSON.stringify({ exp: 1 })).toString('base64url')}.signature`; diff --git a/cli/package.json b/cli/package.json index a78316133..85ec3221f 100644 --- a/cli/package.json +++ b/cli/package.json @@ -1,6 +1,6 @@ { "name": "@commonlyai/cli", - "version": "0.1.26", + "version": "0.1.27", "license": "Apache-2.0", "description": "The Commonly CLI \u2014 connect agents, manage pods, iterate fast", "type": "module", diff --git a/cli/src/commands/login.js b/cli/src/commands/login.js index d2b45a244..13c349e13 100644 --- a/cli/src/commands/login.js +++ b/cli/src/commands/login.js @@ -9,7 +9,12 @@ import { createInterface } from 'readline'; import { hostname } from 'os'; import { createClient, login as apiLogin } from '../lib/api.js'; import { saveInstance } from '../lib/config.js'; -import { DeviceLoginCancelledError, waitForDeviceAuthorization } from '../lib/device-login.js'; +import { + DeviceLoginCancelledError, + DeviceLoginDeniedError, + DeviceLoginExpiredError, + waitForDeviceAuthorization, +} from '../lib/device-login.js'; const prompt = (rl, question) => new Promise((resolve) => rl.question(question, resolve)); @@ -69,9 +74,11 @@ Tokens are stored in ~/.commonly/config.json. Other commands take clientVersion: program.version(), hostname: hostname(), }); - console.log(`Open ${started.verifyUrl}`); - console.log(`Enter code: ${started.userCode}`); - console.log('Press o to open your browser, or q to cancel.'); + const minutes = Math.ceil(started.expiresIn / 60); + console.log(`Logging in to ${instanceUrl} as a new device.\n`); + console.log(` Open ${started.verifyUrl}`); + console.log(` Code ${started.userCode}`); + console.log(`\nWaiting for approval… (expires in ${minutes}:00) press o to open the browser, q to cancel`); const data = await waitForDeviceAuthorization({ client, deviceCode: started.deviceCode, @@ -90,8 +97,9 @@ Tokens are stored in ~/.commonly/config.json. Other commands take username: data.username, tokenType: 'device', }); - console.log(`\nLogged in as ${data.username} (${configKey})`); - console.log('Device token saved to ~/.commonly/config.json'); + const devicesUrl = new URL('/settings/devices', started.verifyUrl).toString(); + console.log(`\n✓ Authorized as @${data.username} on ${configKey} (${instanceUrl})`); + console.log(` Token saved to ~/.commonly/config.json · manage devices at ${devicesUrl}`); return; } @@ -110,7 +118,11 @@ Tokens are stored in ~/.commonly/config.json. Other commands take console.log(`\nLogged in as ${username} (${configKey})`); console.log(`Token saved to ~/.commonly/config.json`); } catch (err) { - const message = err instanceof DeviceLoginCancelledError ? err.message : `Login failed: ${err.message}`; + const message = err instanceof DeviceLoginExpiredError + ? `Code expired after 10 minutes. Run commonly login --instance ${configKey} for a new code.` + : err instanceof DeviceLoginCancelledError || err instanceof DeviceLoginDeniedError + ? err.message + : `Login failed: ${err.message}`; console.error(message); process.exit(1); } diff --git a/cli/src/lib/device-login.js b/cli/src/lib/device-login.js index 4d3ebf764..f406e8eab 100644 --- a/cli/src/lib/device-login.js +++ b/cli/src/lib/device-login.js @@ -18,9 +18,37 @@ export class DeviceLoginCancelledError extends Error { } } +export class DeviceLoginDeniedError extends Error { + constructor() { + super('Denied in the browser. Nothing was saved.'); + this.name = 'DeviceLoginDeniedError'; + } +} + +export class DeviceLoginExpiredError extends Error { + constructor() { + super('Device authorization code expired.'); + this.name = 'DeviceLoginExpiredError'; + } +} + +const safeBrowserUrl = (value) => { + const url = new URL(value); + if (!['http:', 'https:'].includes(url.protocol)) { + throw new Error('Device authorization URL must use HTTP or HTTPS.'); + } + return url.toString(); +}; + export const openBrowser = (url, execFile = nodeExecFile, currentPlatform = platform()) => { - const command = currentPlatform === 'darwin' ? 'open' : currentPlatform === 'win32' ? 'cmd' : 'xdg-open'; - const args = currentPlatform === 'win32' ? ['/c', 'start', '', url] : [url]; + const browserUrl = safeBrowserUrl(url); + // `cmd /c start ` routes a server-supplied URL through a shell. Use a + // direct executable on Windows just as we do on macOS/Linux, so `o` cannot + // turn a malicious verifyUrl into a second command. + const command = currentPlatform === 'darwin' ? 'open' : currentPlatform === 'win32' ? 'rundll32' : 'xdg-open'; + const args = currentPlatform === 'win32' + ? ['url.dll,FileProtocolHandler', browserUrl] + : [browserUrl]; return new Promise((resolve) => { execFile(command, args, () => resolve()); }); @@ -77,20 +105,20 @@ export const waitForDeviceAuthorization = async ({ throw new Error('Unable to complete device authorization. Try again.'); } if (result?.status === 'authorized' && result.token) return result; - if (result?.status === 'denied') throw new Error('Authorization was denied in your browser.'); - if (result?.status === 'expired') throw new Error('Authorization code expired. Run commonly login again.'); + if (result?.status === 'denied') throw new DeviceLoginDeniedError(); + if (result?.status === 'expired') throw new DeviceLoginExpiredError(); if (result?.status === 'slow_down') { currentInterval *= 2; onStatus('Waiting for browser approval (slowing down)…'); } else if (result?.status !== 'authorization_pending') { - throw new Error('Authorization code expired. Run commonly login again.'); + throw new DeviceLoginExpiredError(); } await Promise.race([ wait(currentInterval * 1000), cancellation, ]); } - throw new Error('Authorization code expired. Run commonly login again.'); + throw new DeviceLoginExpiredError(); } finally { if (stdin?.removeListener) stdin.removeListener('keypress', onKeypress); if (isTty) stdin.setRawMode(false); diff --git a/frontend/src/v2/__tests__/V2CliAuthorize.test.tsx b/frontend/src/v2/__tests__/V2CliAuthorize.test.tsx index b916e9740..1a396ebc2 100644 --- a/frontend/src/v2/__tests__/V2CliAuthorize.test.tsx +++ b/frontend/src/v2/__tests__/V2CliAuthorize.test.tsx @@ -40,9 +40,8 @@ describe('V2CliAuthorize', () => { expect(screen.getByLabelText('Device code')).toHaveValue('ABCD-EFGH'); fireEvent.click(screen.getByRole('button', { name: 'Continue' })); await waitFor(() => expect(axios.post).toHaveBeenCalledWith('/api/auth/device/authorize', { userCode: 'ABCD-EFGH' })); - expect(await screen.findByText('Allow this device?')).toBeInTheDocument(); - expect(screen.getByText('sam-laptop')).toBeInTheDocument(); - expect(screen.getByText('api.commonly.me')).toBeInTheDocument(); + expect(await screen.findByText('Authorize sam-laptop as @lily?')).toBeInTheDocument(); + expect(screen.getByText('CLI SIGN-IN')).toBeInTheDocument(); fireEvent.click(screen.getByRole('button', { name: 'Authorize' })); await waitFor(() => expect(axios.post).toHaveBeenLastCalledWith('/api/auth/device/authorize', { @@ -59,4 +58,12 @@ describe('V2CliAuthorize', () => { ); expect(screen.getByLabelText('Device code')).toBeDisabled(); }); + + test('labels an already-consumed code instead of calling it expired', async () => { + axios.post.mockResolvedValue({ data: { status: 'consumed' } }); + renderPage(); + + fireEvent.click(screen.getByRole('button', { name: 'Continue' })); + expect(await screen.findByText('Code already used')).toBeInTheDocument(); + }); }); diff --git a/frontend/src/v2/components/V2CliAuthorize.css b/frontend/src/v2/components/V2CliAuthorize.css index a8c313ca3..90ff28b45 100644 --- a/frontend/src/v2/components/V2CliAuthorize.css +++ b/frontend/src/v2/components/V2CliAuthorize.css @@ -30,6 +30,7 @@ font-weight: 800; } +.v2-cli-authorize__eyebrow { margin: 16px 0 0 !important; color: #2f6feb !important; font-size: 11px; font-weight: 800; letter-spacing: .11em; } .v2-cli-authorize__card h1 { margin: 18px 0 8px; font-size: 24px; } .v2-cli-authorize__card p { margin: 0 0 20px; color: #4b5563; line-height: 1.5; } .v2-cli-authorize__field { display: grid; gap: 7px; margin-bottom: 18px; font-size: 13px; font-weight: 700; } diff --git a/frontend/src/v2/components/V2CliAuthorize.tsx b/frontend/src/v2/components/V2CliAuthorize.tsx index 38e4ab729..de73c02f0 100644 --- a/frontend/src/v2/components/V2CliAuthorize.tsx +++ b/frontend/src/v2/components/V2CliAuthorize.tsx @@ -11,7 +11,7 @@ interface AuthorizationRequest { createdAt: string; } -type Screen = 'code' | 'confirm' | 'done' | 'denied' | 'expired' | 'error'; +type Screen = 'code' | 'confirm' | 'done' | 'denied' | 'expired' | 'used' | 'error'; const normalizeCode = (value: string): string => { const compact = value.toUpperCase().replace(/[^A-Z2-9]/g, '').slice(0, 8); @@ -23,8 +23,22 @@ const errorScreen = (error: unknown): Screen => { return status === 410 ? 'expired' : 'error'; }; +const requestAge = (createdAt: string): string => { + const seconds = Math.max(0, Math.floor((Date.now() - new Date(createdAt).getTime()) / 1000)); + if (seconds < 60) return 'just now'; + const minutes = Math.floor(seconds / 60); + if (minutes < 60) return `${minutes}m ago`; + const hours = Math.floor(minutes / 60); + return `${hours}h ago`; +}; + +const currentInstance = () => { + const baseUrl = String(axios.defaults.baseURL || window.location.origin); + try { return new URL(baseUrl, window.location.origin).host; } catch { return baseUrl; } +}; + const V2CliAuthorize: React.FC = () => { - const { isAuthenticated, loading } = useAuth(); + const { isAuthenticated, loading, user } = useAuth(); const location = useLocation(); const queryCode = useMemo(() => normalizeCode(new URLSearchParams(location.search).get('code') || ''), [location.search]); const [code, setCode] = useState(queryCode); @@ -46,7 +60,13 @@ const V2CliAuthorize: React.FC = () => { setRequest(response.data.request); setScreen('confirm'); } else { - setScreen(response.data.status === 'denied' ? 'denied' : 'expired'); + setScreen( + response.data.status === 'denied' + ? 'denied' + : response.data.status === 'consumed' || response.data.status === 'authorized' + ? 'used' + : 'expired', + ); } } catch (error) { setScreen(errorScreen(error)); @@ -76,6 +96,7 @@ const V2CliAuthorize: React.FC = () => {
+

CLI SIGN-IN

{loading &&

Checking your session…

} {!loading && !isAuthenticated && ( <> @@ -110,12 +131,12 @@ const V2CliAuthorize: React.FC = () => { )} {!loading && isAuthenticated && screen === 'confirm' && request && ( <> -

Allow this device?

-

{request.hostname} is asking to use your Commonly account.

+

Authorize {request.hostname} as @{user?.username || 'you'}?

+

This device is asking to use your Commonly account.

Client
{request.clientName}{request.clientVersion ? ` ${request.clientVersion}` : ''}
-
Instance
api.commonly.me
-
Requested
{new Date(request.createdAt).toLocaleString()}
+
Instance
{currentInstance()}
+
Requested
{requestAge(request.createdAt)}

Only approve a code you requested from your own terminal.

@@ -129,6 +150,7 @@ const V2CliAuthorize: React.FC = () => { {!loading && isAuthenticated && screen === 'done' && <>

Device authorized

You can return to your terminal.

} {!loading && isAuthenticated && screen === 'denied' && <>

Authorization denied

No token was issued for this device.

} {!loading && isAuthenticated && screen === 'expired' && <>

Code expired

Return to your terminal and run commonly login again.

} + {!loading && isAuthenticated && screen === 'used' && <>

Code already used

This code was already approved or completed. Return to your terminal.

} {!loading && isAuthenticated && screen === 'error' && <>

Couldn’t authorize this device

Check the code and try again from your terminal.

}
From 04f433848c5e87f3a160ccc6a87321add8a923ef Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Sun, 30 Aug 2026 21:05:31 -0700 Subject: [PATCH 3/9] fix(auth): retain device authorization TTL --- backend/__tests__/service/auth.test.js | 7 +++++++ backend/models/DeviceAuthorization.ts | 2 +- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/backend/__tests__/service/auth.test.js b/backend/__tests__/service/auth.test.js index 9a0e9a150..5729cfd63 100644 --- a/backend/__tests__/service/auth.test.js +++ b/backend/__tests__/service/auth.test.js @@ -429,6 +429,13 @@ describe('Auth Routes Integration Tests', () => { .send({ clientName: 'commonly-cli', clientVersion: '0.1.26', hostname: 'sam-laptop' }) .expect(201); + it('declares a zero-delay TTL reaper in addition to endpoint expiry checks', () => { + const expiryIndex = DeviceAuthorization.schema.indexes() + .find(([keys]) => Object.prototype.hasOwnProperty.call(keys, 'expiresAt')); + expect(expiryIndex).toBeDefined(); + expect(expiryIndex[1]).toEqual(expect.objectContaining({ expireAfterSeconds: 0 })); + }); + it('hands an approved token to exactly one poller and persists only its digest', async () => { const user = await createVerifiedUser(); const browserToken = generateTestToken(user._id); diff --git a/backend/models/DeviceAuthorization.ts b/backend/models/DeviceAuthorization.ts index 7e20a8ad1..24342be2e 100644 --- a/backend/models/DeviceAuthorization.ts +++ b/backend/models/DeviceAuthorization.ts @@ -38,7 +38,7 @@ const DeviceAuthorizationSchema = new Schema( consumedAt: { type: Date }, // The TTL reaper is a cleanup backstop; every endpoint still checks this // timestamp so expiry behaves correctly before Mongo's next TTL sweep. - expiresAt: { type: Date, required: true, index: { expires: 0 } }, + expiresAt: { type: Date, required: true, expires: 0 }, }, { collection: 'device_authorizations' }, ); From df7e59d35969eb1fdfba434303ca212939cec393 Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Sun, 30 Aug 2026 21:18:56 -0700 Subject: [PATCH 4/9] fix(auth): mint device tokens on terminal claim --- backend/__tests__/service/auth.test.js | 6 +- backend/models/DeviceAuthorization.ts | 4 -- .../services/deviceAuthorizationService.ts | 69 +++++++------------ 3 files changed, 25 insertions(+), 54 deletions(-) diff --git a/backend/__tests__/service/auth.test.js b/backend/__tests__/service/auth.test.js index 5729cfd63..a6f2a425a 100644 --- a/backend/__tests__/service/auth.test.js +++ b/backend/__tests__/service/auth.test.js @@ -562,7 +562,7 @@ describe('Auth Routes Integration Tests', () => { expect((await User.findById(user._id).select('deviceTokens')).deviceTokens).toHaveLength(0); }); - it('revokes an approved bearer when its terminal misses the expiry deadline', async () => { + it('does not mint a bearer when an approved terminal abandons the flow', async () => { const user = await createVerifiedUser(); const browserToken = generateTestToken(user._id); const started = await startAuthorization(); @@ -584,9 +584,7 @@ describe('Auth Routes Integration Tests', () => { .expect(200) .expect({ status: 'expired' }); - const device = (await User.findById(user._id).select('deviceTokens')).deviceTokens[0]; - expect(device.revokedAt).toBeTruthy(); - expect((await DeviceAuthorization.findOne().select('+pendingToken')).pendingToken).toBeUndefined(); + expect((await User.findById(user._id).select('deviceTokens')).deviceTokens).toHaveLength(0); }); }); diff --git a/backend/models/DeviceAuthorization.ts b/backend/models/DeviceAuthorization.ts index 24342be2e..cf933e27a 100644 --- a/backend/models/DeviceAuthorization.ts +++ b/backend/models/DeviceAuthorization.ts @@ -10,9 +10,6 @@ export interface IDeviceAuthorization extends Document { hostname: string; status: DeviceAuthorizationStatus; userId?: Types.ObjectId | null; - // This is the one-time handoff from a browser approval to its originating - // CLI. It is never projected by default and is unset once poll consumes it. - pendingToken?: string; createdAt: Date; lastPolledAt?: Date; authorizedAt?: Date; @@ -30,7 +27,6 @@ const DeviceAuthorizationSchema = new Schema( hostname: { type: String, required: true, trim: true, maxlength: 253 }, status: { type: String, enum: ['pending', 'authorized', 'denied', 'consumed'], default: 'pending' }, userId: { type: Schema.Types.ObjectId, ref: 'User', default: null }, - pendingToken: { type: String, select: false }, createdAt: { type: Date, default: Date.now }, lastPolledAt: { type: Date }, authorizedAt: { type: Date }, diff --git a/backend/services/deviceAuthorizationService.ts b/backend/services/deviceAuthorizationService.ts index 5cfae78cc..8e5d127eb 100644 --- a/backend/services/deviceAuthorizationService.ts +++ b/backend/services/deviceAuthorizationService.ts @@ -36,21 +36,6 @@ const userCodeHash = (value: unknown): string | null => { const isExpired = (request: IDeviceAuthorization, now = new Date()): boolean => request.expiresAt <= now; -const revokeUndeliveredDeviceToken = async (request: IDeviceAuthorization) => { - if (request.status !== 'authorized' || !request.pendingToken || !request.userId) return; - const tokenHash = hashDeviceCredential(request.pendingToken); - await User.updateOne( - { _id: request.userId, 'deviceTokens.tokenHash': tokenHash }, - { $set: { 'deviceTokens.$.revokedAt': new Date() } }, - ); - // The one-time secret has no remaining recipient. Do not retain it until - // Mongo's TTL reaper happens to remove this authorization row. - await DeviceAuthorization.updateOne( - { _id: request._id, status: 'authorized' }, - { $unset: { pendingToken: 1 } }, - ); -}; - export const createDeviceAuthorization = async ({ clientName, clientVersion, @@ -94,13 +79,9 @@ export const pollDeviceAuthorization = async (deviceCode: unknown) => { if (!raw) return { status: 'invalid' as const }; const request = await DeviceAuthorization.findOne({ deviceCodeHash: hashDeviceCredential(raw), - }).select('+pendingToken'); + }); if (!request) return { status: 'expired' as const }; if (isExpired(request)) { - // A browser may approve during the final poll interval. If the terminal - // misses the handoff before the ten-minute deadline, that bearer was never - // delivered and must not remain as a ghost device in the account. - await revokeUndeliveredDeviceToken(request); return { status: 'expired' as const }; } if (request.status === 'pending') { @@ -111,7 +92,7 @@ export const pollDeviceAuthorization = async (deviceCode: unknown) => { return { status: polledTooSoon ? 'slow_down' as const : 'authorization_pending' as const }; } if (request.status === 'denied') return { status: 'denied' as const }; - if (request.status === 'consumed' || !request.pendingToken || !request.userId) { + if (request.status === 'consumed' || !request.userId) { return { status: 'already_used' as const }; } @@ -119,16 +100,29 @@ export const pollDeviceAuthorization = async (deviceCode: unknown) => { // bearer, even if they read the approved request at the same time. const claimed = await DeviceAuthorization.findOneAndUpdate( { _id: request._id, status: 'authorized', expiresAt: { $gt: new Date() } }, - { $set: { status: 'consumed', consumedAt: new Date() }, $unset: { pendingToken: 1 } }, + { $set: { status: 'consumed', consumedAt: new Date() } }, { new: false }, - ).select('+pendingToken'); - if (!claimed?.pendingToken || !claimed.userId) return { status: 'already_used' as const }; + ); + if (!claimed?.userId) return { status: 'already_used' as const }; - const user = await User.findById(claimed.userId).select('_id username banned'); - if (!user || user.banned) return { status: 'denied' as const }; + const token = randomDeviceToken(); + const user = await User.findOneAndUpdate( + { _id: claimed.userId, banned: { $ne: true } }, + { + $push: { + deviceTokens: { + tokenHash: hashDeviceCredential(token), + label: `${claimed.hostname} · ${claimed.clientName}`, + createdAt: new Date(), + }, + }, + }, + { new: false, projection: '_id username' }, + ); + if (!user) return { status: 'denied' as const }; return { status: 'authorized' as const, - token: claimed.pendingToken, + token, username: user.username, userId: user._id.toString(), }; @@ -166,30 +160,13 @@ export const decideDeviceAuthorization = async ({ return denied ? { status: 'denied' as const } : { status: 'expired' as const }; } - const token = randomDeviceToken(); const now = new Date(); - const label = `${request.hostname} · ${request.clientName}`; - const userUpdated = await User.updateOne( - { _id: userId, banned: { $ne: true } }, - { $push: { deviceTokens: { tokenHash: hashDeviceCredential(token), label, createdAt: now } } }, - ); - if (!userUpdated.matchedCount) return { status: 'denied' as const }; - const authorized = await DeviceAuthorization.findOneAndUpdate( { _id: request._id, status: 'pending', expiresAt: { $gt: now } }, - { $set: { status: 'authorized', userId, authorizedAt: now, pendingToken: token } }, + { $set: { status: 'authorized', userId, authorizedAt: now } }, { new: true }, ); - if (!authorized) { - // The new device token must not survive an expiry/race that lost the - // authorization request. Mark it revoked rather than leaving an orphan. - await User.updateOne( - { _id: userId, 'deviceTokens.tokenHash': hashDeviceCredential(token) }, - { $set: { 'deviceTokens.$.revokedAt': now } }, - ); - return { status: 'expired' as const }; - } - return { status: 'authorized' as const }; + return authorized ? { status: 'authorized' as const } : { status: 'expired' as const }; }; export const listDeviceTokens = async (userId: string) => { From 1b5f4f3e44ebbcc0646df704323cc4cc6b63c856 Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Sun, 30 Aug 2026 21:22:24 -0700 Subject: [PATCH 5/9] test(cli): capture device login acceptance flow --- cli/__tests__/login-transcript.test.mjs | 81 +++++++++++++++++++ .../src/v2/__tests__/V2CliAuthorize.test.tsx | 14 ++++ 2 files changed, 95 insertions(+) create mode 100644 cli/__tests__/login-transcript.test.mjs diff --git a/cli/__tests__/login-transcript.test.mjs b/cli/__tests__/login-transcript.test.mjs new file mode 100644 index 000000000..92892d891 --- /dev/null +++ b/cli/__tests__/login-transcript.test.mjs @@ -0,0 +1,81 @@ +import { jest } from '@jest/globals'; +import * as os from 'os'; + +const createClient = jest.fn(); +const saveInstance = jest.fn(); +const listInstances = jest.fn(); +const waitForDeviceAuthorization = jest.fn(); + +await jest.unstable_mockModule('../src/lib/api.js', () => ({ + createClient, + login: jest.fn(), +})); +await jest.unstable_mockModule('../src/lib/config.js', () => ({ saveInstance, listInstances })); +await jest.unstable_mockModule('../src/lib/device-login.js', () => ({ + DeviceLoginCancelledError: class DeviceLoginCancelledError extends Error {}, + DeviceLoginDeniedError: class DeviceLoginDeniedError extends Error {}, + DeviceLoginExpiredError: class DeviceLoginExpiredError extends Error {}, + waitForDeviceAuthorization, +})); +await jest.unstable_mockModule('os', () => ({ ...os, hostname: () => 'sam-laptop' })); + +const { registerLogin, registerWhoami } = await import('../src/commands/login.js'); + +const fakeProgram = () => { + const commands = []; + const program = { + version: () => '0.1.27', + command: jest.fn(() => { + const command = { + description: () => command, + option: () => command, + addHelpText: () => command, + action: (handler) => { command.handler = handler; return command; }, + }; + commands.push(command); + return command; + }), + }; + return { program, commands }; +}; + +afterEach(() => { + jest.clearAllMocks(); +}); + +test('prints the device-login and mixed-profile expiry transcript', async () => { + const { program, commands } = fakeProgram(); + const client = { post: jest.fn().mockResolvedValue({ + deviceCode: 'private-device-code', + userCode: 'ABCD-EFGH', + verifyUrl: 'https://commonly.example/cli/authorize', + expiresIn: 600, + interval: 5, + }) }; + createClient.mockReturnValue(client); + waitForDeviceAuthorization.mockResolvedValue({ token: 'cm_once', username: 'lily', userId: 'u1' }); + listInstances.mockReturnValue([ + { key: 'dev', url: 'https://api.example.test', username: 'lily', active: true, token: 'cm_once', tokenType: 'device' }, + { key: 'legacy', url: 'https://legacy.example.test', username: 'lily', active: false, token: `h.${Buffer.from(JSON.stringify({ exp: 1 })).toString('base64url')}.s`, tokenType: 'jwt' }, + ]); + const log = jest.spyOn(console, 'log').mockImplementation(() => undefined); + + registerLogin(program); + registerWhoami(program); + await commands[0].handler({ instance: 'https://api.example.test', key: 'dev' }); + await commands[1].handler(); + + const transcript = log.mock.calls.flat().join('\n'); + [ + 'Logging in to https://api.example.test as a new device.', + ' Open https://commonly.example/cli/authorize', + ' Code ABCD-EFGH', + 'Waiting for approval… (expires in 10:00)', + '✓ Authorized as @lily on dev (https://api.example.test)', + 'manage devices at https://commonly.example/settings/devices', + '→ dev lily@https://api.example.test (device token · no expiry)', + ' legacy lily@https://legacy.example.test (expired — commonly login --instance legacy)', + ].forEach((line) => expect(transcript).toContain(line)); + expect(saveInstance).toHaveBeenCalledWith(expect.objectContaining({ token: 'cm_once', tokenType: 'device' })); + expect(client.post).toHaveBeenCalledWith('/api/auth/device/start', expect.objectContaining({ hostname: 'sam-laptop' })); +}); diff --git a/frontend/src/v2/__tests__/V2CliAuthorize.test.tsx b/frontend/src/v2/__tests__/V2CliAuthorize.test.tsx index 1a396ebc2..ab83dace7 100644 --- a/frontend/src/v2/__tests__/V2CliAuthorize.test.tsx +++ b/frontend/src/v2/__tests__/V2CliAuthorize.test.tsx @@ -59,6 +59,20 @@ describe('V2CliAuthorize', () => { expect(screen.getByLabelText('Device code')).toBeDisabled(); }); + test('shows the request instance and the expired state for signed-in users', async () => { + axios.defaults.baseURL = 'https://self-hosted.example/api'; + axios.post + .mockResolvedValueOnce({ data: { status: 'pending', request: { hostname: 'sam-laptop', clientName: 'commonly-cli', createdAt: '2026-08-31T00:00:00.000Z' } } }) + .mockRejectedValueOnce({ response: { status: 410 } }); + renderPage(); + + fireEvent.click(screen.getByRole('button', { name: 'Continue' })); + expect(await screen.findByText('self-hosted.example')).toBeInTheDocument(); + fireEvent.click(screen.getByRole('button', { name: 'Authorize' })); + expect(await screen.findByText('Code expired')).toBeInTheDocument(); + axios.defaults.baseURL = ''; + }); + test('labels an already-consumed code instead of calling it expired', async () => { axios.post.mockResolvedValue({ data: { status: 'consumed' } }); renderPage(); From 506e9856bf1ee5b14b498a13f417d0040439257b Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Sun, 30 Aug 2026 21:25:55 -0700 Subject: [PATCH 6/9] docs(auth): explain terminal claim ordering --- backend/services/deviceAuthorizationService.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/backend/services/deviceAuthorizationService.ts b/backend/services/deviceAuthorizationService.ts index 8e5d127eb..d881b61d8 100644 --- a/backend/services/deviceAuthorizationService.ts +++ b/backend/services/deviceAuthorizationService.ts @@ -97,7 +97,9 @@ export const pollDeviceAuthorization = async (deviceCode: unknown) => { } // Conditional update prevents two concurrent polls from receiving the same - // bearer, even if they read the approved request at the same time. + // bearer, even if they read the approved request at the same time. Claim + // before minting: if token persistence fails after this transition, the CLI + // must re-run login rather than risk issuing a bearer twice. const claimed = await DeviceAuthorization.findOneAndUpdate( { _id: request._id, status: 'authorized', expiresAt: { $gt: new Date() } }, { $set: { status: 'consumed', consumedAt: new Date() } }, From e17b07e7ccc1c7c39a1bc7d16f7cb062355d8b91 Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Sun, 30 Aug 2026 21:45:24 -0700 Subject: [PATCH 7/9] fix(auth): restrict device approval to browser sessions --- backend/__tests__/service/auth.test.js | 36 +++++++++++++++++++ backend/middleware/auth.ts | 1 + backend/models/User.ts | 6 ++-- backend/routes/auth.ts | 7 ++++ .../services/deviceAuthorizationService.ts | 2 ++ 5 files changed, 50 insertions(+), 2 deletions(-) diff --git a/backend/__tests__/service/auth.test.js b/backend/__tests__/service/auth.test.js index a6f2a425a..a4d3bcca9 100644 --- a/backend/__tests__/service/auth.test.js +++ b/backend/__tests__/service/auth.test.js @@ -522,6 +522,42 @@ describe('Auth Routes Integration Tests', () => { .get('/api/auth/user') .set('Authorization', `Bearer ${granted.body.token}`) .expect(401); + + await request(app) + .delete('/api/auth/devices/not-a-device-id') + .set('Authorization', `Bearer ${browserToken}`) + .expect(404); + }); + + it('requires a browser JWT to authorize a device and cannot mint a successor from a device token', async () => { + const user = await createVerifiedUser(); + const browserToken = generateTestToken(user._id); + const first = await startAuthorization(); + + await request(app) + .post('/api/auth/device/authorize') + .set('Authorization', `Bearer ${browserToken}`) + .send({ userCode: first.body.userCode, decision: 'authorize' }) + .expect(200); + const firstGrant = await request(app) + .post('/api/auth/device/poll') + .send({ deviceCode: first.body.deviceCode }) + .expect(200); + + const successor = await startAuthorization(); + await request(app) + .post('/api/auth/device/authorize') + .set('Authorization', `Bearer ${firstGrant.body.token}`) + .send({ userCode: successor.body.userCode, decision: 'authorize' }) + .expect(403); + await request(app) + .post('/api/auth/device/poll') + .send({ deviceCode: successor.body.deviceCode }) + .expect(200) + .expect({ status: 'authorization_pending' }); + + const persistedUser = await User.findById(user._id).select('deviceTokens'); + expect(persistedUser.deviceTokens).toHaveLength(1); }); it('returns slow_down, denied, and expired terminal states without minting a token', async () => { diff --git a/backend/middleware/auth.ts b/backend/middleware/auth.ts index c4fff9340..9aca1b3d2 100644 --- a/backend/middleware/auth.ts +++ b/backend/middleware/auth.ts @@ -117,6 +117,7 @@ export default async function auth(req: Request, res: Response, next: NextFuncti req.userId = id; req.user = { id }; + req.authType = 'jwt'; touchLastActive(id); next(); } catch (err: unknown) { diff --git a/backend/models/User.ts b/backend/models/User.ts index 932ea4eec..a93c22abf 100644 --- a/backend/models/User.ts +++ b/backend/models/User.ts @@ -23,8 +23,10 @@ export interface IAgentRuntimeToken { } // A device-login bearer is intentionally one-way: the CLI receives it once, -// while Mongo stores only this digest. It is separate from the legacy -// apiToken (which pre-dates per-device revocation) and agent runtime tokens. +// while Mongo stores only this digest. It is long-lived until explicit +// revocation (there is no expiresAt or transparent refresh) and is separate +// from the legacy apiToken (which pre-dates per-device revocation) and agent +// runtime tokens. export interface IDeviceToken { tokenHash: string; label: string; diff --git a/backend/routes/auth.ts b/backend/routes/auth.ts index 40de11958..ff0db9522 100644 --- a/backend/routes/auth.ts +++ b/backend/routes/auth.ts @@ -43,6 +43,7 @@ const { interface AuthReq { user?: { id: string }; userId?: string; + authType?: 'jwt' | 'apiToken' | 'deviceToken'; } interface Res { status: (n: number) => Res; @@ -196,6 +197,12 @@ router.post('/device/poll', devicePollLimiter, async (req: any, res: Res) => { }); router.post('/device/authorize', deviceManageLimiter, auth, async (req: AuthReq & { body?: any }, res: Res) => { + // Only an interactive browser session can grant another device bearer. + // A device token is intentionally sufficient for ordinary user routes, but + // accepting it here would let a revoked device pre-mint a successor. + if (req.authType !== 'jwt') { + return res.status(403).json({ error: 'Device authorization requires a signed-in browser session' }); + } try { const result = await decideDeviceAuthorization({ userCode: req.body?.userCode, diff --git a/backend/services/deviceAuthorizationService.ts b/backend/services/deviceAuthorizationService.ts index d881b61d8..89566d673 100644 --- a/backend/services/deviceAuthorizationService.ts +++ b/backend/services/deviceAuthorizationService.ts @@ -1,4 +1,5 @@ import crypto from 'crypto'; +import { Types } from 'mongoose'; import DeviceAuthorization, { IDeviceAuthorization } from '../models/DeviceAuthorization'; import User from '../models/User'; @@ -183,6 +184,7 @@ export const listDeviceTokens = async (userId: string) => { }; export const revokeDeviceToken = async (userId: string, deviceId: string) => { + if (!Types.ObjectId.isValid(deviceId)) return false; const result = await User.updateOne( { _id: userId, From 5cd5991b78c9b3accc01efbeb2e36b263f532e9f Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Sun, 30 Aug 2026 22:27:48 -0700 Subject: [PATCH 8/9] fix(auth): keep device bearers out of credential management --- backend/__tests__/service/auth.test.js | 24 +++++++++++++++++++++++- backend/routes/auth.ts | 22 ++++++++++++++++++---- 2 files changed, 41 insertions(+), 5 deletions(-) diff --git a/backend/__tests__/service/auth.test.js b/backend/__tests__/service/auth.test.js index a4d3bcca9..58cce1dd7 100644 --- a/backend/__tests__/service/auth.test.js +++ b/backend/__tests__/service/auth.test.js @@ -529,7 +529,7 @@ describe('Auth Routes Integration Tests', () => { .expect(404); }); - it('requires a browser JWT to authorize a device and cannot mint a successor from a device token', async () => { + it('requires a browser JWT for credential management and cannot mint a successor from a device token', async () => { const user = await createVerifiedUser(); const browserToken = generateTestToken(user._id); const first = await startAuthorization(); @@ -556,8 +556,30 @@ describe('Auth Routes Integration Tests', () => { .expect(200) .expect({ status: 'authorization_pending' }); + // A device bearer is deliberately long-lived, not refreshable. It must + // not be able to launder itself into a JWT or revoke/list the account's + // other device credentials if it is stolen. + await request(app) + .post('/api/auth/refresh') + .set('Authorization', `Bearer ${firstGrant.body.token}`) + .expect(403); + await request(app) + .get('/api/auth/devices') + .set('Authorization', `Bearer ${firstGrant.body.token}`) + .expect(403); + const persistedUser = await User.findById(user._id).select('deviceTokens'); expect(persistedUser.deviceTokens).toHaveLength(1); + await request(app) + .delete(`/api/auth/devices/${persistedUser.deviceTokens[0]._id}`) + .set('Authorization', `Bearer ${firstGrant.body.token}`) + .expect(403); + + await request(app) + .post('/api/auth/refresh') + .set('Authorization', `Bearer ${browserToken}`) + .expect(200) + .expect((response) => expect(response.body.token).toBeTruthy()); }); it('returns slow_down, denied, and expired terminal states without minting a token', async () => { diff --git a/backend/routes/auth.ts b/backend/routes/auth.ts index ff0db9522..5959311bc 100644 --- a/backend/routes/auth.ts +++ b/backend/routes/auth.ts @@ -50,6 +50,17 @@ interface Res { json: (d: unknown) => void; } +// Device bearers authenticate ordinary user API calls, but they must not gain +// control of the account's credential set. In particular, `/refresh` mints a +// browser JWT; allowing a device token through it would let a stolen device +// turn itself into a browser session and then create or revoke other devices. +// Device login deliberately has an expiry contract rather than a refresh path. +function requireBrowserJwt(req: AuthReq, res: Res): boolean { + if (req.authType === 'jwt') return true; + res.status(403).json({ error: 'This action requires a signed-in browser session' }); + return false; +} + // Abuse rate-limiters for the unauthenticated public auth surface — added as a // pre-flight gate before open registration. Cloudflare sets // `CF-Connecting-IP` at the edge, so this avoids trusting a client-supplied @@ -200,9 +211,7 @@ router.post('/device/authorize', deviceManageLimiter, auth, async (req: AuthReq // Only an interactive browser session can grant another device bearer. // A device token is intentionally sufficient for ordinary user routes, but // accepting it here would let a revoked device pre-mint a successor. - if (req.authType !== 'jwt') { - return res.status(403).json({ error: 'Device authorization requires a signed-in browser session' }); - } + if (!requireBrowserJwt(req, res)) return; try { const result = await decideDeviceAuthorization({ userCode: req.body?.userCode, @@ -219,6 +228,7 @@ router.post('/device/authorize', deviceManageLimiter, auth, async (req: AuthReq }); router.get('/devices', deviceManageLimiter, auth, async (req: AuthReq, res: Res) => { + if (!requireBrowserJwt(req, res)) return; try { return res.json({ devices: await listDeviceTokens(req.userId || req.user?.id || '') }); } catch (error: any) { @@ -228,6 +238,7 @@ router.get('/devices', deviceManageLimiter, auth, async (req: AuthReq, res: Res) }); router.delete('/devices/:deviceId', deviceManageLimiter, auth, async (req: AuthReq & { params?: any }, res: Res) => { + if (!requireBrowserJwt(req, res)) return; try { const revoked = await revokeDeviceToken(req.userId || req.user?.id || '', String(req.params?.deviceId || '')); if (!revoked) return res.status(404).json({ error: 'Device not found or already revoked' }); @@ -245,7 +256,10 @@ router.post('/redeem-invitation', loginLimiter, auth, redeemInvitation); // signed token so the login limiter's posture suffices. router.post('/forgot-password', forgotLimiter, forgotPassword); router.post('/reset-password', loginLimiter, resetPassword); -router.post('/refresh', auth, refresh); +router.post('/refresh', auth, (req: AuthReq, res: Res) => { + if (!requireBrowserJwt(req, res)) return; + return refresh(req, res); +}); router.get('/user', auth, getCurrentUser); router.get('/verify-email', verifyEmail); router.get('/profile', auth, getProfile); From 538854f725759fa2a91e5c7e2fb1c4f349af38f7 Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Sun, 30 Aug 2026 22:37:57 -0700 Subject: [PATCH 9/9] fix(auth): require browser JWT for API token controls --- backend/__tests__/service/auth.test.js | 17 +++++++++++++++++ backend/routes/auth.ts | 3 +++ 2 files changed, 20 insertions(+) diff --git a/backend/__tests__/service/auth.test.js b/backend/__tests__/service/auth.test.js index 58cce1dd7..f5050c1ad 100644 --- a/backend/__tests__/service/auth.test.js +++ b/backend/__tests__/service/auth.test.js @@ -574,12 +574,29 @@ describe('Auth Routes Integration Tests', () => { .delete(`/api/auth/devices/${persistedUser.deviceTokens[0]._id}`) .set('Authorization', `Bearer ${firstGrant.body.token}`) .expect(403); + await request(app) + .post('/api/auth/api-token/generate') + .set('Authorization', `Bearer ${firstGrant.body.token}`) + .expect(403); + await request(app) + .get('/api/auth/api-token') + .set('Authorization', `Bearer ${firstGrant.body.token}`) + .expect(403); + await request(app) + .delete('/api/auth/api-token') + .set('Authorization', `Bearer ${firstGrant.body.token}`) + .expect(403); await request(app) .post('/api/auth/refresh') .set('Authorization', `Bearer ${browserToken}`) .expect(200) .expect((response) => expect(response.body.token).toBeTruthy()); + await request(app) + .post('/api/auth/api-token/generate') + .set('Authorization', `Bearer ${browserToken}`) + .expect(200) + .expect((response) => expect(response.body.apiToken).toBeTruthy()); }); it('returns slow_down, denied, and expired terminal states without minting a token', async () => { diff --git a/backend/routes/auth.ts b/backend/routes/auth.ts index 5959311bc..0921cc38e 100644 --- a/backend/routes/auth.ts +++ b/backend/routes/auth.ts @@ -270,6 +270,7 @@ router.get('/admin/check', auth, adminAuth, (_req: unknown, res: Res) => { }); router.post('/api-token/generate', auth, async (req: AuthReq, res: Res) => { + if (!requireBrowserJwt(req, res)) return; try { // eslint-disable-next-line global-require const User = require('../models/User'); @@ -290,6 +291,7 @@ router.post('/api-token/generate', auth, async (req: AuthReq, res: Res) => { }); router.delete('/api-token', auth, async (req: AuthReq, res: Res) => { + if (!requireBrowserJwt(req, res)) return; try { // eslint-disable-next-line global-require const User = require('../models/User'); @@ -306,6 +308,7 @@ router.delete('/api-token', auth, async (req: AuthReq, res: Res) => { }); router.get('/api-token', auth, async (req: AuthReq, res: Res) => { + if (!requireBrowserJwt(req, res)) return; try { // eslint-disable-next-line global-require const User = require('../models/User');