eletrotupi / tcc/ commit / ff12ba9

api/auth: never hand back the raw user row

Every endpoint that returned a user serialised the whole Prisma row, so
the bcrypt hash, the reset token and the activation code crossed the
wire and were logged. One allow-list now shapes all of them, and the
tests assert those fields are absent so an endpoint cannot regress
Pedro Lucas Porcellis porcellis@eletrotupi.com · 11 days ago ff12ba99add07246bdaf39737d2c64072be1cfbf Browse files
Parents 491dca7
Changed files
12
Additions
+150
Deletions
-51
api/src/controllers/auth.ts modified+5 -13
api/src/controllers/auth.ts
1 1 
import { Request, Response, NextFunction } from "express";
2 2 
import { AuthenticatedRequest } from "@app/middleware/auth";
3 3
4 4 
import {
5 5 
  login,
23 23 
  PasswordResetSchema,
24 24 
  PasswordResetRequestSchema,
25 25 
  ActivateUserSchema,
26 26 
} from "@app/schemas";
27 27 
import { formatValidationError } from "@app/lib/errors/validationError";
28 + 
import { publicUser } from "@app/lib/serializers/user";
28 29
29 30 
import { getQueue, MailJobName } from "@app/lib/queue";
30 31
31 32 
export const AuthController = {
32 33 
  login: async (req: Request, res: Response, next: NextFunction) => {
52 53 
        });
53 54 
      }
54 55
55 56 
      return res.json({
56 57 
        token,
57 - 
        user,
58 + 
        user: publicUser(user),
58 59 
      });
59 60 
    } catch (err: any) {
60 61 
      if (err.message === "Invalid credentials") {
61 62 
        return res.status(401).json({ error: "Invalid credentials" });
62 63 
      }
120 121
121 122 
      res.json({
122 123 
        valid: true,
123 124 
        userId: payload.userId,
124 125 
        email: payload.email,
125 - 
        user: {
126 - 
          firstName: user.firstName,
127 - 
          email: user.email,
128 - 
          lastName: user.lastName,
129 - 
          updatedAt: user.updatedAt,
130 - 
          active: user.active,
131 - 
          avatarKey: user.avatarKey,
132 - 
          avatarURL: user.avatarURL,
133 - 
          pushToken: user.pushToken,
134 - 
        },
126 + 
        user: publicUser(user),
135 127 
      });
136 128 
    } catch (err: any) {
137 129 
      if (
138 130 
        err.name === "JsonWebTokenError" ||
139 131 
        err.name === "TokenExpiredError"
161 153 
      }
162 154
163 155 
      const result = await activateUser(user);
164 156
165 157 
      return res.status(200).json({
166 - 
        user: result,
158 + 
        user: publicUser(result),
167 159 
      });
168 160 
    } catch (err: any) {
169 161 
      next(err);
170 162 
    }
171 163 
  },
191 183 
        userId: user.id,
192 184 
        code: code,
193 185 
      });
194 186
195 187 
      res.json({
196 - 
        user,
188 + 
        user: publicUser(user),
197 189 
      });
198 190 
    } catch (err: any) {
199 191 
      next(err);
200 192 
    }
201 193 
  },
202 194 
};
api/src/controllers/users.ts modified+22 -31
api/src/controllers/users.ts
1 1 
import { Request, Response, NextFunction } from 'express';
2 2 
import {
3 3 
  generateToken
4 4 
} from '@app/services/auth.service';
5 5
27 27 
import {
28 28 
  AuthenticatedRequest
29 29 
} from '@app/middleware/auth';
30 30
31 31 
import { prisma } from '@app/lib/prisma';
32 + 
import { publicUser, publicUserSelect } from '@app/lib/serializers/user';
32 33 
import s3 from '@app/lib/s3';
33 34 
import { DeleteObjectCommand } from '@aws-sdk/client-s3';
34 - 
import { User } from '@prisma/client';
35 35
36 36 
// XXX: I've regreted already
37 37 
interface SingleFileRequest extends AuthenticatedRequest {
38 38 
  file?: any;
39 39 
}
40 -
41 - 
const withoutSecrets = (user: User) => {
42 - 
  const {
43 - 
    encryptedPassword,
44 - 
    passwordResetToken,
45 - 
    passwordResetExpires,
46 - 
    ...safe
47 - 
  } = user;
48 -
49 - 
  return safe;
50 - 
};
51 40
52 41 
export const UsersController = {
53 42 
  create: async (req: Request, res: Response, next: NextFunction) => {
54 43 
    try {
55 44 
      const parsed = CreateUserSchema.safeParse(req.body.user);
68 57 
        userId: user.id,
69 58 
        code: code
70 59 
      });
71 60
72 61 
      res.status(201).json({
73 - 
        token: jwtToken
62 + 
        token: jwtToken,
63 + 
        user: publicUser(user),
74 64 
      });
75 65 
    } catch (err) {
76 66 
      next(err);
77 67 
    }
78 68 
  },
101 91 
      }
102 92
103 93 
      const updated = await updateUser({ ...parsed.data, id: targetId });
104 94
105 95 
      return res.status(200).json({
106 - 
        user: withoutSecrets(updated)
96 + 
        user: publicUser(updated)
107 97 
      });
108 98 
    } catch (err) {
109 99 
      next(err);
110 100 
    }
111 101 
  },
138 128 
        message: 'File uploaded successfully',
139 129 
        fileLocation: req.file.location,
140 130 
        key: req.file.key,
141 131 
        filename: req.file.originalname,
142 132 
        size: req.file.size,
143 - 
        user: user
133 + 
        user: publicUser(user)
144 134 
      });
145 135 
    } catch (err) {
146 136 
      next(err);
147 137 
    }
148 138 
  },
159 149 
        data: parsed.data,
160 150 
      });
161 151
162 152 
      await syncDailyReminderJob(user);
163 153
164 - 
      return res.status(200).json({ user });
154 + 
      return res.status(200).json({ user: publicUser(user) });
155 + 
    } catch (err) {
156 + 
      next(err);
157 + 
    }
158 + 
  },
159 +
160 + 
  completeOnboarding: async (req: AuthenticatedRequest, res: Response, next: NextFunction) => {
161 + 
    try {
162 + 
      const user = await prisma.user.update({
163 + 
        where: { id: req.userId },
164 + 
        data: { onboardingSeenAt: new Date() },
165 + 
      });
166 +
167 + 
      return res.status(200).json({ user: publicUser(user) });
165 168 
    } catch (err) {
166 169 
      next(err);
167 170 
    }
168 171 
  },
169 172
170 173 
  me: async (req: AuthenticatedRequest, res: Response, next: NextFunction) => {
171 174 
    try {
172 175 
      const user = await prisma.user.findUnique({
173 176 
        where: { id: req.userId! },
174 - 
        select: {
175 - 
          id: true,
176 - 
          email: true,
177 - 
          firstName: true,
178 - 
          lastName: true,
179 - 
          updatedAt: true,
180 - 
          avatarKey: true,
181 - 
          avatarURL: true,
182 - 
          active: true,
183 - 
          pushToken: true,
184 - 
          notificationsEnabled: true,
185 - 
          dailyReminderTime: true,
186 - 
        },
177 + 
        select: publicUserSelect,
187 178 
      });
188 179
189 180 
      if (!user) return res.status(404).json({ error: "User not found" });
190 181
191 182 
      return res.json({ user });
196 187
197 188 
  removeAvatar: async (req: AuthenticatedRequest, res: Response, next: NextFunction) => {
198 189 
    const currentUser = await findUserById(Number(req.userId));
199 190
200 191 
    if (currentUser && currentUser?.avatarKey == null) {
201 - 
      return res.json({ user: currentUser })
192 + 
      return res.json({ user: publicUser(currentUser) })
202 193 
    }
203 194
204 195 
    await s3.send(
205 196 
      new DeleteObjectCommand({
206 197 
        Bucket: process.env.S3_BUCKET!, Key: currentUser?.avatarKey!
215 206 
        avatarKey: null
216 207 
      }
217 208 
    });
218 209
219 210 
    return res.json({
220 - 
      user
211 + 
      user: publicUser(user)
221 212 
    })
222 213 
  }
223 214 
}
api/src/lib/serializers/user.ts added+32
/dev/null → api/src/lib/serializers/user.ts
1 + 
import { Prisma, User } from '@prisma/client';
2 +
3 + 
// DB-level projection of the allow-list below — keep the two in sync.
4 + 
export const publicUserSelect = {
5 + 
  id: true,
6 + 
  email: true,
7 + 
  firstName: true,
8 + 
  lastName: true,
9 + 
  updatedAt: true,
10 + 
  avatarKey: true,
11 + 
  avatarURL: true,
12 + 
  active: true,
13 + 
  notificationsEnabled: true,
14 + 
  dailyReminderTime: true,
15 + 
  onboardingSeenAt: true,
16 + 
} satisfies Prisma.UserSelect;
17 +
18 + 
// The only shape a user leaves the API in. Everything not listed here
19 + 
// (encryptedPassword, reset secrets, activation code, pushToken) stays server-side.
20 + 
export const publicUser = (user: User) => ({
21 + 
  id: user.id,
22 + 
  email: user.email,
23 + 
  firstName: user.firstName,
24 + 
  lastName: user.lastName,
25 + 
  updatedAt: user.updatedAt,
26 + 
  avatarKey: user.avatarKey,
27 + 
  avatarURL: user.avatarURL,
28 + 
  active: user.active,
29 + 
  notificationsEnabled: user.notificationsEnabled,
30 + 
  dailyReminderTime: user.dailyReminderTime,
31 + 
  onboardingSeenAt: user.onboardingSeenAt,
32 + 
});
api/tests/auth/activate.test.ts modified+3 -1
api/tests/auth/activate.test.ts
1 1 
import { describe, it, expect, afterAll, beforeEach } from 'vitest'
2 2 
import request from 'supertest'
3 3 
import { createApp } from '@app/createApp'
4 4 
import {
5 5 
  buildPrisma,
6 6 
  cleanupTestDb,
7 7 
  createTestUser,
8 8 
  generateTestToken,
9 + 
  expectPublicUser,
9 10 
} from '../test-helper'
10 11
11 12 
const app = createApp()
12 13 
const db = buildPrisma()
13 14
37 38 
      .post('/auth/activate')
38 39 
      .send({ code: '123456' })
39 40
40 41 
    expect(res.status).toBe(200)
41 42 
    expect(res.body.user.active).toBe(true)
42 - 
    expect(res.body.user.activationCode).toBeNull()
43 + 
    expectPublicUser(res.body.user)
43 44
44 45 
    const dbUser = await db.user.findUnique({ where: { id: user.id } })
45 46 
    expect(dbUser!.active).toBe(true)
46 47 
    expect(dbUser!.activationCode).toBeNull()
47 48 
    expect(dbUser!.activationCodeExpiresAt).toBeNull()
152 153 
    const res = await request(app)
153 154 
      .post('/auth/resend_code')
154 155 
      .set('Authorization', `Bearer ${token}`)
155 156
156 157 
    expect(res.status).toBe(200)
158 + 
    expectPublicUser(res.body.user)
157 159
158 160 
    const dbUser = await db.user.findUnique({ where: { id: user.id } })
159 161 
    expect(dbUser!.activationCode).toMatch(/^\d{6}$/)
160 162 
    expect(dbUser!.activationCodeExpiresAt).not.toBeNull()
161 163 
  })
163 165 
  it('returns 401 without a token', async () => {
164 166 
    const res = await request(app).post('/auth/resend_code')
165 167 
    expect(res.status).toBe(401)
166 168 
  })
167 169 
})
api/tests/auth/login.test.ts modified+2 -1
api/tests/auth/login.test.ts
1 1 
import { describe, it, expect, afterAll, beforeEach } from 'vitest'
2 2 
import request from 'supertest'
3 3 
import { createApp } from '@app/createApp'
4 - 
import { buildPrisma, cleanupTestDb, createTestUser } from '../test-helper'
4 + 
import { buildPrisma, cleanupTestDb, createTestUser, expectPublicUser } from '../test-helper'
5 5 
import { recordedJobs, resetQueueMocks } from '../mocks/queue'
6 6
7 7 
const app = createApp()
8 8 
const db = buildPrisma()
9 9
23 23 
      .send({ email: 'login@example.com', password: 'senha123' })
24 24
25 25 
    expect(res.status).toBe(200)
26 26 
    expect(res.body.token).toBeTruthy()
27 27 
    expect(res.body.user.email).toBe('login@example.com')
28 + 
    expectPublicUser(res.body.user)
28 29 
  })
29 30
30 31 
  it('returns 401 for wrong password', async () => {
31 32 
    await createTestUser(db, { email: 'login@example.com', password: 'senha123' })
32 33
73 74 
        data: { userId: user.id, code: dbUser!.activationCode },
74 75 
      })
75 76 
    )
76 77 
  })
77 78 
})
api/tests/auth/verify.test.ts modified+2 -1
api/tests/auth/verify.test.ts
1 1 
import { describe, it, expect, afterAll, beforeEach } from 'vitest'
2 2 
import request from 'supertest'
3 3 
import { createApp } from '@app/createApp'
4 4 
import {
5 5 
  buildPrisma,
6 6 
  cleanupTestDb,
7 7 
  createTestUser,
8 8 
  generateTestToken,
9 + 
  expectPublicUser,
9 10 
} from '../test-helper'
10 11
11 12 
const app = createApp()
12 13 
const db = buildPrisma()
13 14
30 31 
    expect(res.status).toBe(200)
31 32 
    expect(res.body.valid).toBe(true)
32 33 
    expect(res.body.userId).toBe(user.id)
33 34 
    expect(res.body.email).toBe('verify@example.com')
34 35 
    expect(res.body.user.firstName).toBe('Pedro')
35 - 
    expect(res.body.user.encryptedPassword).toBeUndefined()
36 + 
    expectPublicUser(res.body.user)
36 37 
  })
37 38
38 39 
  it('returns 401 without an authorization header', async () => {
39 40 
    const res = await request(app).get('/auth/verify')
40 41 
    expect(res.status).toBe(401)
56 57 
      .set('Authorization', `Bearer ${token}`)
57 58
58 59 
    expect(res.status).toBe(404)
59 60 
  })
60 61 
})
api/tests/serializers/user.test.ts added+55
/dev/null → api/tests/serializers/user.test.ts
1 + 
import { describe, it, expect } from 'vitest'
2 + 
import { User } from '@prisma/client'
3 + 
import { publicUser } from '@app/lib/serializers/user'
4 +
5 + 
const row = {
6 + 
  id: 7,
7 + 
  firstName: 'Pedro',
8 + 
  lastName: 'Silva',
9 + 
  email: 'pedro@example.com',
10 + 
  encryptedPassword: 'bcrypt-hash',
11 + 
  passwordResetToken: 'reset-secret',
12 + 
  passwordResetExpires: new Date(),
13 + 
  createdAt: new Date(),
14 + 
  updatedAt: new Date(),
15 + 
  avatarKey: 'avatars/pedro.png',
16 + 
  avatarURL: 'https://cdn.example.com/avatars/pedro.png',
17 + 
  activationCode: '123456',
18 + 
  activationCodeExpiresAt: new Date(),
19 + 
  active: true,
20 + 
  pushToken: 'device-token',
21 + 
  notificationsEnabled: true,
22 + 
  dailyReminderTime: '08:30',
23 + 
  onboardingSeenAt: null,
24 + 
  termsAcceptedAt: new Date(),
25 + 
  termsVersion: '1',
26 + 
} as User
27 +
28 + 
describe('publicUser', () => {
29 + 
  it('keeps the profile fields the client needs', () => {
30 + 
    expect(publicUser(row)).toMatchObject({
31 + 
      id: 7,
32 + 
      email: 'pedro@example.com',
33 + 
      firstName: 'Pedro',
34 + 
      active: true,
35 + 
      onboardingSeenAt: null,
36 + 
    })
37 + 
  })
38 +
39 + 
  it('never carries a secret off the server', () => {
40 + 
    const safe = publicUser(row) as Record<string, unknown>
41 +
42 + 
    for (const field of [
43 + 
      'encryptedPassword',
44 + 
      'passwordResetToken',
45 + 
      'passwordResetExpires',
46 + 
      'activationCode',
47 + 
      'activationCodeExpiresAt',
48 + 
      'pushToken',
49 + 
      'termsAcceptedAt',
50 + 
      'termsVersion',
51 + 
    ]) {
52 + 
      expect(safe).not.toHaveProperty(field)
53 + 
    }
54 + 
  })
55 + 
})
api/tests/test-helper.ts modified+20
api/tests/test-helper.ts
1 1 
import { PrismaClient } from '@prisma/client'
2 2 
import { PrismaPg } from '@prisma/adapter-pg'
3 3 
import jwt from 'jsonwebtoken'
4 4 
import bcrypt from 'bcryptjs'
5 + 
import { expect } from 'vitest'
5 6
6 7 
export function buildPrisma(): PrismaClient {
7 8 
  const adapter = new PrismaPg({ connectionString: process.env.DATABASE_URL! })
8 9 
  return new PrismaClient({ adapter })
9 10 
}
37 38 
      ...(pushToken !== undefined && { pushToken }),
38 39 
      ...(notificationsEnabled !== undefined && { notificationsEnabled }),
39 40 
    },
40 41 
  })
41 42 
}
43 +
44 + 
// Fields that must never leave the API. Asserting absence at every
45 + 
// user-returning endpoint is the regression net for the raw-row leak: a failure
46 + 
// means an endpoint started handing back a Prisma row again.
47 + 
const SECRET_FIELDS = [
48 + 
  'encryptedPassword',
49 + 
  'passwordResetToken',
50 + 
  'passwordResetExpires',
51 + 
  'activationCode',
52 + 
  'activationCodeExpiresAt',
53 + 
  'pushToken',
54 + 
] as const
55 +
56 + 
export function expectPublicUser(user: Record<string, unknown> | undefined) {
57 + 
  expect(user).toBeDefined()
58 + 
  for (const field of SECRET_FIELDS) {
59 + 
    expect(user).not.toHaveProperty(field)
60 + 
  }
61 + 
}
api/tests/users/avatar.test.ts modified+3
api/tests/users/avatar.test.ts
1 1 
import { describe, it, expect, afterAll, beforeEach, vi } from 'vitest'
2 2 
import request from 'supertest'
3 3 
import { DeleteObjectCommand } from '@aws-sdk/client-s3'
4 4 
import { createApp } from '@app/createApp'
5 5 
import s3 from '@app/lib/s3'
6 6 
import {
7 7 
  buildPrisma,
8 8 
  cleanupTestDb,
9 9 
  createTestUser,
10 10 
  generateTestToken,
11 + 
  expectPublicUser,
11 12 
} from '../test-helper'
12 13
13 14 
import { resetAvatarMock, setUploadedFile } from '../mocks/avatar'
14 15
15 16 
const app = createApp()
46 47
47 48 
    expect(res.status).toBe(200)
48 49 
    expect(res.body.key).toBe('avatars/new.png')
49 50 
    expect(res.body.user.avatarURL).toBe(uploadedFile.location)
50 51 
    expect(res.body.user.avatarKey).toBe('avatars/new.png')
52 + 
    expectPublicUser(res.body.user)
51 53
52 54 
    const dbUser = await db.user.findUnique({ where: { id: user.id } })
53 55
54 56 
    expect(dbUser!.avatarURL).toBe(uploadedFile.location)
55 57 
  })
114 116 
      .set('Authorization', `Bearer ${token}`)
115 117
116 118 
    expect(res.status).toBe(200)
117 119 
    expect(res.body.user.avatarKey).toBeNull()
118 120 
    expect(res.body.user.avatarURL).toBeNull()
121 + 
    expectPublicUser(res.body.user)
119 122 
    expect(mockedSend).toHaveBeenCalledOnce()
120 123
121 124 
    const dbUser = await db.user.findUnique({ where: { id: user.id } })
122 125
123 126 
    expect(dbUser!.avatarKey).toBeNull()
139 142 
  it('returns 401 without a token', async () => {
140 143 
    const res = await request(app).delete('/users/me/avatar')
141 144 
    expect(res.status).toBe(401)
142 145 
  })
143 146 
})
api/tests/users/me.test.ts modified+2 -2
api/tests/users/me.test.ts
1 1 
import { describe, it, expect, afterAll, beforeEach } from 'vitest'
2 2 
import request from 'supertest'
3 3 
import { createApp } from '@app/createApp'
4 - 
import { buildPrisma, cleanupTestDb, createTestUser, generateTestToken } from '../test-helper'
4 + 
import { buildPrisma, cleanupTestDb, createTestUser, generateTestToken, expectPublicUser } from '../test-helper'
5 5
6 6 
const app = createApp()
7 7 
const db = buildPrisma()
8 8
9 9 
afterAll(() => db.$disconnect())
19 19 
      .set('Authorization', `Bearer ${token}`)
20 20
21 21 
    expect(res.status).toBe(200)
22 22 
    expect(res.body.user.email).toBe('me@example.com')
23 23 
    expect(res.body.user.firstName).toBe('Pedro')
24 - 
    expect(res.body.user.encryptedPassword).toBeUndefined()
24 + 
    expectPublicUser(res.body.user)
25 25 
  })
26 26
27 27 
  it('returns 401 without token', async () => {
28 28 
    const res = await request(app).get('/users/me')
29 29 
    expect(res.status).toBe(401)
30 30 
  })
31 31 
})
api/tests/users/preferences.test.ts modified+2
api/tests/users/preferences.test.ts
1 1 
import { describe, it, expect, afterAll, beforeEach } from 'vitest'
2 2 
import request from 'supertest'
3 3 
import { createApp } from '@app/createApp'
4 4 
import { DailyReminderJobName } from '@app/lib/queue/types'
5 5 
import {
6 6 
  buildPrisma,
7 7 
  cleanupTestDb,
8 8 
  createTestUser,
9 9 
  generateTestToken,
10 + 
  expectPublicUser,
10 11 
} from '../test-helper'
11 12
12 13 
import { recordedJobs, resetQueueMocks } from '../mocks/queue'
13 14
14 15 
const app = createApp()
32 33 
      .send({ user: { notificationsEnabled: true, dailyReminderTime: '08:30' } })
33 34
34 35 
    expect(res.status).toBe(200)
35 36 
    expect(res.body.user.notificationsEnabled).toBe(true)
36 37 
    expect(res.body.user.dailyReminderTime).toBe('08:30')
38 + 
    expectPublicUser(res.body.user)
37 39
38 40 
    const dbUser = await db.user.findUnique({ where: { id: user.id } })
39 41 
    expect(dbUser!.dailyReminderTime).toBe('08:30')
40 42
41 43 
    expect(recordedJobs).toContainEqual(
92 94 
      .send({ user: { notificationsEnabled: false } })
93 95
94 96 
    expect(res.status).toBe(401)
95 97 
  })
96 98 
})
api/tests/users/update.test.ts modified+2 -2
api/tests/users/update.test.ts
1 1 
import { describe, it, expect, afterAll, beforeEach } from 'vitest'
2 2 
import request from 'supertest'
3 3 
import { createApp } from '@app/createApp'
4 4 
import {
5 5 
  buildPrisma,
6 6 
  cleanupTestDb,
7 7 
  createTestUser,
8 8 
  generateTestToken,
9 + 
  expectPublicUser,
9 10 
} from '../test-helper'
10 11
11 12 
const app = createApp()
12 13 
const db = buildPrisma()
13 14
35 36 
      })
36 37
37 38 
    expect(res.status).toBe(200)
38 39 
    expect(res.body.user.firstName).toBe('New')
39 40 
    expect(res.body.user.lastName).toBe('Name')
40 - 
    expect(res.body.user.encryptedPassword).toBeUndefined()
41 - 
    expect(res.body.user.passwordResetToken).toBeUndefined()
41 + 
    expectPublicUser(res.body.user)
42 42
43 43 
    const dbUser = await db.user.findUnique({ where: { id: user.id } })
44 44 
    expect(dbUser!.firstName).toBe('New')
45 45 
  })
46 46
113 113 
      .send({ user: { id: user.id, email: 'taken@example.com' } })
114 114
115 115 
    expect(res.status).toBe(409)
116 116 
  })
117 117 
})