diff --git a/.github/workflows/build-and-test.yaml b/.github/workflows/build-and-test.yaml index 579e1633..d6f1344a 100644 --- a/.github/workflows/build-and-test.yaml +++ b/.github/workflows/build-and-test.yaml @@ -66,11 +66,51 @@ jobs: pnpm run build pnpm test - - name: Build web + # Web previously ran `build` only, so its unit and e2e suites never + # gated a merge. Lint, typecheck and unit tests are fast and + # deterministic, so they block. + - name: Install web dependencies run: | cd web pnpm install - pnpm run build + + - name: Lint web + run: | + cd web + pnpm lint + + - name: Typecheck web + run: | + cd web + pnpm typecheck + + - name: Unit test web + run: | + cd web + pnpm test + + - name: Build web + run: | + cd web + pnpm run build + + # E2e is fully mocked (e2e/mock-api.ts intercepts every backend call), + # but it drives a real browser, so it is reported without blocking until + # it has proven stable across a few merges. + # + # TODO: remove continue-on-error once that is established. It is here to + # avoid wedging the merge queue on a browser flake, not because e2e + # failures are acceptable. + - name: Install Playwright browser + run: | + cd web + pnpm exec playwright install --with-deps chromium + + - name: E2e test web (non-blocking for now) + continue-on-error: true + run: | + cd web + pnpm test:e2e build-and-test-android: name: Build and Test Android diff --git a/api/src/auth/auth.service.ts b/api/src/auth/auth.service.ts index 8b658f63..cb788f09 100644 --- a/api/src/auth/auth.service.ts +++ b/api/src/auth/auth.service.ts @@ -21,6 +21,12 @@ import { EmailVerificationDocument, } from './schemas/email-verification.schema' +// For register and login, which hold the hash in memory before responding. +export const withoutPassword = (user: UserDocument) => { + const { password, ...safe } = user.toObject() + return safe +} + @Injectable() export class AuthService { constructor( @@ -39,7 +45,9 @@ export class AuthService { async login(userData: any) { await this.turnstileService.verify(userData.turnstileToken) - const user = await this.usersService.findOne({ email: userData.email }) + const user = await this.usersService.findOneWithPassword({ + email: userData.email, + }) if (!user) { throw new HttpException( { error: 'Invalid credentials' }, @@ -60,7 +68,7 @@ export class AuthService { const payload = { email: user.email, sub: user._id } return { accessToken: this.jwtService.sign(payload), - user, + user: withoutPassword(user), } } @@ -101,7 +109,7 @@ export class AuthService { const payload = { email: user.email, sub: user._id } return { accessToken: this.jwtService.sign(payload), - user, + user: withoutPassword(user), } } @@ -140,7 +148,7 @@ export class AuthService { return { accessToken: this.jwtService.sign(payload), - user, + user: withoutPassword(user), } } @@ -240,7 +248,9 @@ export class AuthService { input: { oldPassword: string; newPassword: string }, user: UserDocument, ) { - const userToUpdate = await this.usersService.findOne({ _id: user._id }) + const userToUpdate = await this.usersService.findOneWithPassword({ + _id: user._id, + }) if (!userToUpdate) { throw new HttpException({ error: 'User not found' }, HttpStatus.NOT_FOUND) } diff --git a/api/src/auth/without-password.spec.ts b/api/src/auth/without-password.spec.ts new file mode 100644 index 00000000..bc24b3ab --- /dev/null +++ b/api/src/auth/without-password.spec.ts @@ -0,0 +1,41 @@ +import { withoutPassword } from './auth.service' +import type { UserDocument } from '../users/schemas/user.schema' + +// Fake document: only toObject() matters to the helper. +const doc = (fields: Record) => + ({ toObject: () => ({ ...fields }) }) as unknown as UserDocument + +describe('withoutPassword', () => { + it('removes the password hash', () => { + const safe = withoutPassword( + doc({ email: 'a@b.com', password: '$2a$10$hashedvalue' }) + ) + + expect(safe).not.toHaveProperty('password') + }) + + it('keeps everything else the client needs', () => { + const safe = withoutPassword( + doc({ + _id: 'u1', + email: 'a@b.com', + name: 'Ada', + role: 'regular', + password: '$2a$10$hashedvalue', + }) + ) + + expect(safe).toEqual({ + _id: 'u1', + email: 'a@b.com', + name: 'Ada', + role: 'regular', + }) + }) + + it('is harmless when the hash was never loaded', () => { + const safe = withoutPassword(doc({ email: 'a@b.com' })) + + expect(safe).toEqual({ email: 'a@b.com' }) + }) +}) diff --git a/api/src/gateway/escape-regexp.spec.ts b/api/src/gateway/escape-regexp.spec.ts new file mode 100644 index 00000000..56679bc2 --- /dev/null +++ b/api/src/gateway/escape-regexp.spec.ts @@ -0,0 +1,35 @@ +import { escapeRegExp } from './escape-regexp' + +describe('escapeRegExp', () => { + it('leaves plain text untouched', () => { + expect(escapeRegExp('hello world')).toBe('hello world') + expect(escapeRegExp('+14155550101')).toBe('\\+14155550101') + }) + + it('makes an unbalanced paren safe to compile', () => { + // Unescaped, `new RegExp('(')` throws and would fail the whole request. + expect(() => new RegExp(escapeRegExp('('))).not.toThrow() + expect(() => new RegExp(escapeRegExp('a)b['))).not.toThrow() + }) + + it('treats wildcards as literal characters', () => { + const pattern = new RegExp(escapeRegExp('.*'), 'i') + // A literal ".*" must not match arbitrary text. + expect(pattern.test('anything at all')).toBe(false) + expect(pattern.test('literally .* here')).toBe(true) + }) + + it('treats a dot as a dot, not "any character"', () => { + const pattern = new RegExp(escapeRegExp('a.c'), 'i') + expect(pattern.test('abc')).toBe(false) + expect(pattern.test('a.c')).toBe(true) + }) + + it('defuses a nested quantifier (ReDoS shape)', () => { + const pattern = new RegExp(escapeRegExp('(a+)+$'), 'i') + const start = Date.now() + pattern.test('a'.repeat(2000) + 'b') + // As a literal it is a plain substring scan, not catastrophic backtracking. + expect(Date.now() - start).toBeLessThan(100) + }) +}) diff --git a/api/src/gateway/escape-regexp.ts b/api/src/gateway/escape-regexp.ts new file mode 100644 index 00000000..a45d74ce --- /dev/null +++ b/api/src/gateway/escape-regexp.ts @@ -0,0 +1,11 @@ +/** + * Escape every regular-expression metacharacter in a string so it can be + * embedded in a RegExp as a literal. + * + * Search terms come straight from user input. Without this, "(" alone throws + * a SyntaxError and fails the request, "." matches any character instead of a + * dot, and a nested quantifier is a ReDoS vector. + */ +export function escapeRegExp(value: string): string { + return value.replace(/[.*+?^${}()|[\]\\]/g, '\\$&') +} diff --git a/api/src/gateway/gateway.controller.ts b/api/src/gateway/gateway.controller.ts index 0eab5a28..450c972d 100644 --- a/api/src/gateway/gateway.controller.ts +++ b/api/src/gateway/gateway.controller.ts @@ -160,8 +160,9 @@ export class GatewayController { const page = req.query.page ? parseInt(req.query.page, 10) : 1; const limit = req.query.limit ? Math.min(parseInt(req.query.limit, 10), 100) : 50; const type = req.query.type || ''; - - const result = await this.gatewayService.getMessages(deviceId, type, page, limit); + const search = req.query.search || ''; + + const result = await this.gatewayService.getMessages(deviceId, type, page, limit, search); return result; } diff --git a/api/src/gateway/gateway.service.spec.ts b/api/src/gateway/gateway.service.spec.ts index 7f43cd83..45eeb8e0 100644 --- a/api/src/gateway/gateway.service.spec.ts +++ b/api/src/gateway/gateway.service.spec.ts @@ -802,6 +802,65 @@ describe('GatewayService', () => { HttpException, ) }) + + it('should search across message body, recipient and sender', async () => { + await service.getMessages(mockDeviceId, '', 1, 10, 'alice') + + const expectedQuery = { + device: mockDevice._id, + $or: [ + { message: /alice/i }, + { recipient: /alice/i }, + { sender: /alice/i }, + ], + } + + expect(mockSmsModel.countDocuments).toHaveBeenCalledWith(expectedQuery) + expect(mockSmsModel.find).toHaveBeenCalledWith( + expectedQuery, + null, + expect.any(Object), + ) + }) + + it('should combine search with the type filter', async () => { + await service.getMessages(mockDeviceId, 'sent', 1, 10, 'alice') + + expect(mockSmsModel.find).toHaveBeenCalledWith( + expect.objectContaining({ + device: mockDevice._id, + type: SMSType.SENT, + $or: expect.any(Array), + }), + null, + expect.any(Object), + ) + }) + + it('should ignore an empty or whitespace-only search', async () => { + await service.getMessages(mockDeviceId, '', 1, 10, ' ') + + expect(mockSmsModel.find).toHaveBeenCalledWith( + { device: mockDevice._id }, + null, + expect.any(Object), + ) + }) + + it('should escape regex metacharacters in the search term', async () => { + // Unescaped, this throws a SyntaxError and fails the request. + await expect( + service.getMessages(mockDeviceId, '', 1, 10, '('), + ).resolves.toBeDefined() + + // A wildcard must be matched literally, not treated as "any character". + await service.getMessages(mockDeviceId, '', 1, 10, '.*') + + const call = mockSmsModel.find.mock.calls.at(-1) + const messagePattern = call[0].$or[0].message as RegExp + expect(messagePattern.test('anything at all')).toBe(false) + expect(messagePattern.test('contains .* literally')).toBe(true) + }) }) describe('getStatsForUser', () => { diff --git a/api/src/gateway/gateway.service.ts b/api/src/gateway/gateway.service.ts index fe396c90..440ea60c 100644 --- a/api/src/gateway/gateway.service.ts +++ b/api/src/gateway/gateway.service.ts @@ -24,6 +24,7 @@ import { WebhookEvent } from '../webhook/webhook-event.enum' import { WebhookService } from '../webhook/webhook.service' import { BillingService } from '../billing/billing.service' import { SmsQueueService } from './queue/sms-queue.service' +import { escapeRegExp } from './escape-regexp' @Injectable() export class GatewayService { @@ -947,6 +948,7 @@ export class GatewayService { type = '', page = 1, limit = 50, + search = '', ): Promise<{ data: any[]; meta: any }> { const device = await this.deviceModel.findById(deviceId) @@ -972,6 +974,19 @@ export class GatewayService { query.type = SMSType.RECEIVED } + // Free-text search across the message body and either party's number. + // The input is escaped before it reaches RegExp: an unescaped "(" throws + // and fails the request, and a crafted pattern is a ReDoS vector. + const trimmedSearch = search?.trim() + if (trimmedSearch) { + const pattern = new RegExp(escapeRegExp(trimmedSearch), 'i') + query.$or = [ + { message: pattern }, + { recipient: pattern }, + { sender: pattern }, + ] + } + // Get total count for pagination metadata const total = await this.smsModel.countDocuments(query) diff --git a/api/src/users/schemas/user.schema.spec.ts b/api/src/users/schemas/user.schema.spec.ts new file mode 100644 index 00000000..dc07c12f --- /dev/null +++ b/api/src/users/schemas/user.schema.spec.ts @@ -0,0 +1,14 @@ +import { UserSchema } from './user.schema' + +describe('User schema', () => { + it('never selects the password hash by default', () => { + expect(UserSchema.path('password').options.select).toBe(false) + }) + + it.each(['email', 'name', 'role', 'emailVerifiedAt'])( + 'still returns %s by default', + (field) => { + expect(UserSchema.path(field).options.select).toBeUndefined() + } + ) +}) diff --git a/api/src/users/schemas/user.schema.ts b/api/src/users/schemas/user.schema.ts index 66050f06..14dae2dd 100644 --- a/api/src/users/schemas/user.schema.ts +++ b/api/src/users/schemas/user.schema.ts @@ -23,7 +23,8 @@ export class User { @Prop({ type: String, trim: true }) phone?: string - @Prop({ type: String }) + // Never loaded unless a caller asks with .select('+password'). + @Prop({ type: String, select: false }) password: string @Prop({ type: String, default: UserRole.REGULAR }) diff --git a/api/src/users/users.service.ts b/api/src/users/users.service.ts index 8678d69d..8550f449 100644 --- a/api/src/users/users.service.ts +++ b/api/src/users/users.service.ts @@ -18,6 +18,11 @@ export class UsersService { return await this.userModel.findOne(params) } + // Only for flows that verify a password. Never return this to a client. + async findOneWithPassword(params) { + return await this.userModel.findOne(params).select('+password') + } + async findAll() { return await this.userModel.find() } diff --git a/web/.env.example b/web/.env.example index 23023d1c..ac555934 100644 --- a/web/.env.example +++ b/web/.env.example @@ -4,9 +4,10 @@ NEXT_PUBLIC_LATEST_APP_VERSION_CODE=17 NEXT_PUBLIC_GOOGLE_CLIENT_ID= NEXT_PUBLIC_TAWKTO_EMBED_URL= -AUTH_SECRET= # https://generate-secret.vercel.app/32 - -DATABASE_URL=mongodb://adminUser:adminPassword@textbee-db:27017/textbee?authSource=admin +# Used by NextAuth (next-auth v4) to sign/verify the session JWT. Must match +# the value read by proxy.ts (process.env.NEXTAUTH_SECRET). +NEXTAUTH_SECRET= # https://generate-secret.vercel.app/32 +NEXTAUTH_URL=http://localhost:3000 MAIL_HOST= MAIL_PORT= diff --git a/web/.eslintrc.json b/web/.eslintrc.json deleted file mode 100644 index d2a18faf..00000000 --- a/web/.eslintrc.json +++ /dev/null @@ -1,6 +0,0 @@ -{ - "extends": "next/core-web-vitals", - "rules": { - "react/no-unescaped-entities": "off" - } -} diff --git a/web/.gitignore b/web/.gitignore index c574c9f1..bd43cb79 100644 --- a/web/.gitignore +++ b/web/.gitignore @@ -7,6 +7,9 @@ # testing /coverage +/test-results +/playwright-report +/playwright/.cache # next.js /.next/ diff --git a/web/app/(app)/(auth)/(components)/register-form.tsx b/web/app/(app)/(auth)/(components)/register-form.tsx index 30f7928a..7c199d4e 100644 --- a/web/app/(app)/(auth)/(components)/register-form.tsx +++ b/web/app/(app)/(auth)/(components)/register-form.tsx @@ -36,12 +36,18 @@ const registerSchema = z.object({ .min(1, { message: 'Please complete the bot verification' }), }) -type RegisterFormValues = z.infer +// marketingOptIn is `.optional().default(true)`, so zod's input and output +// types differ: optional going in, guaranteed coming out. The form holds the +// input shape and the submit handler receives the output shape, which is what +// useForm's third generic is for. Using z.infer (the output) for both made the +// resolver unassignable. +type RegisterFormInput = z.input +type RegisterFormValues = z.output export default function RegisterForm() { const router = useRouter() - const form = useForm({ + const form = useForm({ resolver: zodResolver(registerSchema), defaultValues: { name: '', diff --git a/web/app/(app)/(auth)/(components)/request-password-reset-form.tsx b/web/app/(app)/(auth)/(components)/request-password-reset-form.tsx index 24c80af2..371da5d4 100644 --- a/web/app/(app)/(auth)/(components)/request-password-reset-form.tsx +++ b/web/app/(app)/(auth)/(components)/request-password-reset-form.tsx @@ -100,7 +100,7 @@ export default function RequestPasswordResetForm() { return (
- + Reset your password diff --git a/web/app/(app)/(auth)/(components)/reset-password-form.tsx b/web/app/(app)/(auth)/(components)/reset-password-form.tsx index 54ef108b..22ca41e5 100644 --- a/web/app/(app)/(auth)/(components)/reset-password-form.tsx +++ b/web/app/(app)/(auth)/(components)/reset-password-form.tsx @@ -82,7 +82,7 @@ export default function ResetPasswordForm({ return (
- + Reset your password diff --git a/web/app/(app)/(auth)/layout.tsx b/web/app/(app)/(auth)/layout.tsx index 48eacd27..b5311859 100644 --- a/web/app/(app)/(auth)/layout.tsx +++ b/web/app/(app)/(auth)/layout.tsx @@ -1,7 +1,16 @@ +import Footer from '@/components/shared/footer' + +// The footer is rendered per-section rather than in (app)/layout.tsx, because +// the dashboard's fixed sidebar would otherwise overlap a full-width footer. export default function AuthLayout({ children, }: { children: React.ReactNode }) { - return <>{children} + return ( + <> + {children} +