diff --git a/server/entity/User.ts b/server/entity/User.ts index 644b38893..cd9f8778c 100644 --- a/server/entity/User.ts +++ b/server/entity/User.ts @@ -51,6 +51,8 @@ export class User { 'settings', ]; + static readonly serializationExcludedFields: string[] = ['settings']; + public displayName: string; @PrimaryGeneratedColumn() @@ -173,6 +175,16 @@ export class User { return filtered; } + // settings is eager-loaded and would otherwise ride along into every nested User response + public toJSON(): Partial { + return Object.assign( + {}, + ...(Object.keys(this) as (keyof User)[]) + .filter((k) => !User.serializationExcludedFields.includes(k)) + .map((k) => ({ [k]: this[k] })) + ); + } + public hasPermission( permissions: Permission | Permission[], options?: PermissionCheckOptions diff --git a/server/routes/auth.test.ts b/server/routes/auth.test.ts index 152277a91..d787165cf 100644 --- a/server/routes/auth.test.ts +++ b/server/routes/auth.test.ts @@ -12,6 +12,11 @@ import ImageProxy from '@server/lib/imageproxy'; import { getSettings } from '@server/lib/settings'; import { checkUser } from '@server/middleware/auth'; import { setupTestDb } from '@server/test/db'; +import { + allowlistedSettings, + assertNoCredentials, + seedUserSettings, +} from '@server/test/userSettings'; import { ApiError } from '@server/types/error'; import axios from 'axios'; import type { Express } from 'express'; @@ -613,6 +618,30 @@ describe('GET /auth/me', () => { settings.notifications.agents.email.options.userEmailRequired = false; }); + + it('returns only the allowlisted settings fields', async () => { + await seedUserSettings('admin@seerr.dev'); + const agent = await authenticatedAgent('admin@seerr.dev', 'test1234'); + + const res = await agent.get('/auth/me'); + + assert.strictEqual(res.status, 200); + assert.deepStrictEqual(Object.keys(res.body.settings).sort(), [ + 'discoverRegion', + 'locale', + 'notificationTypes', + 'originalLanguage', + 'streamingRegion', + 'watchlistSyncMovies', + 'watchlistSyncTv', + ]); + assert.strictEqual(res.body.settings.locale, allowlistedSettings.locale); + assert.strictEqual( + res.body.settings.discoverRegion, + allowlistedSettings.discoverRegion + ); + assertNoCredentials(res.body); + }); }); describe('POST /auth/local', () => { diff --git a/server/routes/auth.ts b/server/routes/auth.ts index eb5f0eff3..f83dc8015 100644 --- a/server/routes/auth.ts +++ b/server/routes/auth.ts @@ -52,7 +52,18 @@ authRoutes.get('/me', isAuthenticated(), async (req, res) => { logger.warn(`User ${user.username} has no valid email address`); } - return res.status(200).json(user); + return res.status(200).json({ + ...user.toJSON(), + settings: user.settings && { + locale: user.settings.locale, + discoverRegion: user.settings.discoverRegion, + streamingRegion: user.settings.streamingRegion, + originalLanguage: user.settings.originalLanguage, + notificationTypes: user.settings.notificationTypes, + watchlistSyncMovies: user.settings.watchlistSyncMovies, + watchlistSyncTv: user.settings.watchlistSyncTv, + }, + }); }); authRoutes.post('/plex', async (req, res, next) => { diff --git a/server/routes/blocklist.test.ts b/server/routes/blocklist.test.ts new file mode 100644 index 000000000..9273783ef --- /dev/null +++ b/server/routes/blocklist.test.ts @@ -0,0 +1,106 @@ +import assert from 'node:assert/strict'; +import { before, describe, it } from 'node:test'; + +import { MediaStatus, MediaType } from '@server/constants/media'; +import { getRepository } from '@server/datasource'; +import { Blocklist } from '@server/entity/Blocklist'; +import Media from '@server/entity/Media'; +import { getSettings } from '@server/lib/settings'; +import { checkUser } from '@server/middleware/auth'; +import { setupTestDb } from '@server/test/db'; +import { + assertNoCredentials, + seedUserSettings, +} from '@server/test/userSettings'; +import type { Express } from 'express'; +import express from 'express'; +import session from 'express-session'; +import request from 'supertest'; +import authRoutes from './auth'; +import blocklistRoutes from './blocklist'; + +let app: Express; + +function createApp() { + const app = express(); + app.use(express.json()); + app.use( + session({ + secret: 'test-secret', + resave: false, + saveUninitialized: false, + }) + ); + app.use(checkUser); + app.use('/auth', authRoutes); + app.use('/blocklist', blocklistRoutes); + app.use( + ( + err: { status?: number; message?: string }, + _req: express.Request, + res: express.Response, + // eslint-disable-next-line @typescript-eslint/no-unused-vars + _next: express.NextFunction + ) => { + res + .status(err.status ?? 500) + .json({ status: err.status ?? 500, message: err.message }); + } + ); + return app; +} + +before(async () => { + app = createApp(); +}); + +setupTestDb(); + +async function loginAs(email: string, password: string) { + const settings = getSettings(); + const priorLocalLogin = settings.main.localLogin; + settings.main.localLogin = true; + + try { + const agent = request.agent(app); + const res = await agent.post('/auth/local').send({ email, password }); + assert.strictEqual(res.status, 200); + return agent; + } finally { + settings.main.localLogin = priorLocalLogin; + } +} + +describe('GET /blocklist/:id', () => { + // this route replies with res.send() rather than res.json() + it('omits notification settings from the blocklisting user', async () => { + const blocklistUser = await seedUserSettings('demo@seerr.dev'); + + const media = await getRepository(Media).save( + new Media({ + mediaType: MediaType.MOVIE, + tmdbId: 12345, + status: MediaStatus.BLOCKLISTED, + status4k: MediaStatus.UNKNOWN, + }) + ); + + await getRepository(Blocklist).save( + new Blocklist({ + mediaType: MediaType.MOVIE, + tmdbId: 12345, + title: 'Blocked Movie', + user: blocklistUser, + media, + }) + ); + + const admin = await loginAs('admin@seerr.dev', 'test1234'); + const res = await admin.get('/blocklist/12345?mediaType=movie'); + + assert.strictEqual(res.status, 200); + assert.strictEqual(res.body.user.email, 'demo@seerr.dev'); + assert.ok(!('settings' in res.body.user)); + assertNoCredentials(res.body); + }); +}); diff --git a/server/routes/issue.test.ts b/server/routes/issue.test.ts index 6948801d9..cd98770e7 100644 --- a/server/routes/issue.test.ts +++ b/server/routes/issue.test.ts @@ -13,12 +13,17 @@ import { checkUser } from '@server/middleware/auth'; import { IssueCommentSubscriber } from '@server/subscriber/IssueCommentSubscriber'; import { IssueSubscriber } from '@server/subscriber/IssueSubscriber'; import { setupTestDb } from '@server/test/db'; +import { + assertNoCredentials, + seedUserSettings, +} from '@server/test/userSettings'; import type { Express } from 'express'; import express from 'express'; import session from 'express-session'; import request from 'supertest'; import authRoutes from './auth'; import issueRoutes from './issue'; +import issueCommentRoutes from './issueComment'; const sendIssueNotificationMock = mock.method( IssueSubscriber.prototype as unknown as { @@ -51,6 +56,7 @@ function createApp() { app.use(checkUser); app.use('/auth', authRoutes); app.use('/issue', issueRoutes); + app.use('/issueComment', issueCommentRoutes); app.use( ( err: { status?: number; message?: string }, @@ -215,3 +221,30 @@ describe('POST /issue', () => { assert.strictEqual(res.body.message, 'Issue user not found'); }); }); + +describe('GET /issueComment/:commentId', () => { + it('omits notification settings from the comment author', async () => { + const userRepo = getRepository(User); + const media = await seedMedia(); + const friend = await userRepo.findOneOrFail({ + where: { email: 'demo@seerr.dev' }, + }); + await seedUserSettings('demo@seerr.dev'); + + const agent = await loginAs('admin@seerr.dev', 'test1234'); + const created = await agent.post('/issue').send({ + issueType: IssueType.VIDEO, + message: 'Playback stutters near the end.', + mediaId: media.id, + userId: friend.id, + }); + assert.strictEqual(created.status, 201); + + const res = await agent.get(`/issueComment/${created.body.comments[0].id}`); + + assert.strictEqual(res.status, 200); + assert.strictEqual(res.body.user.email, 'demo@seerr.dev'); + assert.ok(!('settings' in res.body.user)); + assertNoCredentials(res.body); + }); +}); diff --git a/server/routes/request.test.ts b/server/routes/request.test.ts index 9050eaa7c..6e0f27dc2 100644 --- a/server/routes/request.test.ts +++ b/server/routes/request.test.ts @@ -24,6 +24,10 @@ import { getSettings } from '@server/lib/settings'; import { checkUser } from '@server/middleware/auth'; import { MediaRequestSubscriber } from '@server/subscriber/MediaRequestSubscriber'; import { setupTestDb } from '@server/test/db'; +import { + assertNoCredentials, + seedUserSettings, +} from '@server/test/userSettings'; import type { Express } from 'express'; import express from 'express'; import session from 'express-session'; @@ -807,6 +811,27 @@ describe('PUT /request/:requestId (quota)', () => { }); }); +describe('GET /request/:requestId', () => { + it('omits notification settings from requestedBy and modifiedBy', async () => { + const pending = await seedRequest(); + await seedUserSettings('demo@seerr.dev'); + await seedUserSettings('admin@seerr.dev'); + + const admin = await loginAs('admin@seerr.dev', 'test1234'); + const approved = await admin.post(`/request/${pending.id}/approve`); + assert.strictEqual(approved.status, 200); + + const res = await admin.get(`/request/${pending.id}`); + + assert.strictEqual(res.status, 200); + assert.strictEqual(res.body.requestedBy.email, 'demo@seerr.dev'); + assert.strictEqual(res.body.modifiedBy.email, 'admin@seerr.dev'); + assert.ok(!('settings' in res.body.requestedBy)); + assert.ok(!('settings' in res.body.modifiedBy)); + assertNoCredentials(res.body); + }); +}); + describe('POST /request/:requestId/:status', () => { const cases = [ { action: 'approve', expected: MediaRequestStatus.APPROVED }, diff --git a/server/routes/user/index.test.ts b/server/routes/user/index.test.ts new file mode 100644 index 000000000..2513ea7c7 --- /dev/null +++ b/server/routes/user/index.test.ts @@ -0,0 +1,147 @@ +import assert from 'node:assert/strict'; +import { before, describe, it } from 'node:test'; + +import { MediaStatus, MediaType } from '@server/constants/media'; +import { getRepository } from '@server/datasource'; +import Media from '@server/entity/Media'; +import { User } from '@server/entity/User'; +import { Watchlist } from '@server/entity/Watchlist'; +import { getSettings } from '@server/lib/settings'; +import { checkUser, isAuthenticated } from '@server/middleware/auth'; +import authRoutes from '@server/routes/auth'; +import { setupTestDb } from '@server/test/db'; +import { + assertNoCredentials, + seedUserSettings, +} from '@server/test/userSettings'; +import type { Express } from 'express'; +import express from 'express'; +import session from 'express-session'; +import request from 'supertest'; +import userRoutes from '.'; + +let app: Express; + +function createApp() { + const app = express(); + app.use(express.json()); + app.use( + session({ + secret: 'test-secret', + resave: false, + saveUninitialized: false, + }) + ); + app.use(checkUser); + app.use('/auth', authRoutes); + app.use('/user', isAuthenticated(), userRoutes); + app.use( + ( + err: { status?: number; message?: string }, + _req: express.Request, + res: express.Response, + // eslint-disable-next-line @typescript-eslint/no-unused-vars + _next: express.NextFunction + ) => { + res + .status(err.status ?? 500) + .json({ status: err.status ?? 500, message: err.message }); + } + ); + return app; +} + +before(async () => { + app = createApp(); +}); + +setupTestDb(); + +async function loginAs(email: string, password: string) { + const settings = getSettings(); + const priorLocalLogin = settings.main.localLogin; + settings.main.localLogin = true; + + try { + const agent = request.agent(app); + const res = await agent.post('/auth/local').send({ email, password }); + assert.strictEqual(res.status, 200); + return agent; + } finally { + settings.main.localLogin = priorLocalLogin; + } +} + +describe('GET /user/:id/watchlist', () => { + it('omits notification settings from every requestedBy in the page', async () => { + const owner = await seedUserSettings('demo@seerr.dev'); + + const media = await getRepository(Media).save( + new Media({ + mediaType: MediaType.MOVIE, + tmdbId: 12345, + status: MediaStatus.UNKNOWN, + status4k: MediaStatus.UNKNOWN, + }) + ); + + await getRepository(Watchlist).save( + new Watchlist({ + mediaType: MediaType.MOVIE, + tmdbId: 12345, + title: 'Watchlisted Movie', + ratingKey: 'rk-12345', + requestedBy: owner, + media, + }) + ); + + const admin = await loginAs('admin@seerr.dev', 'test1234'); + const res = await admin.get(`/user/${owner.id}/watchlist`); + + assert.strictEqual(res.status, 200); + assert.strictEqual(res.body.results.length, 1); + assert.ok(!('settings' in res.body.results[0].requestedBy)); + assertNoCredentials(res.body); + }); +}); + +describe('GET /user/:id', () => { + it('still returns full settings to the user themselves', async () => { + const owner = await seedUserSettings('demo@seerr.dev'); + + const agent = await loginAs('demo@seerr.dev', 'test1234'); + const res = await agent.get(`/user/${owner.id}`); + + assert.strictEqual(res.status, 200); + assert.strictEqual(res.body.settings.pgpKey, 'test-pgp-key'); + assert.strictEqual( + res.body.settings.pushoverUserKey, + 'test-pushover-user-key' + ); + }); + + it('still returns full settings to a manage-users admin', async () => { + const owner = await seedUserSettings('demo@seerr.dev'); + + const admin = await loginAs('admin@seerr.dev', 'test1234'); + const res = await admin.get(`/user/${owner.id}`); + + assert.strictEqual(res.status, 200); + assert.strictEqual(res.body.settings.pgpKey, 'test-pgp-key'); + }); + + it('strips settings for an unrelated user', async () => { + const admin = await getRepository(User).findOneOrFail({ + where: { email: 'admin@seerr.dev' }, + }); + await seedUserSettings('admin@seerr.dev'); + + const agent = await loginAs('demo@seerr.dev', 'test1234'); + const res = await agent.get(`/user/${admin.id}`); + + assert.strictEqual(res.status, 200); + assert.ok(!('settings' in res.body)); + assertNoCredentials(res.body); + }); +}); diff --git a/server/test/userSettings.ts b/server/test/userSettings.ts new file mode 100644 index 000000000..8ab0bb129 --- /dev/null +++ b/server/test/userSettings.ts @@ -0,0 +1,52 @@ +import { getRepository } from '@server/datasource'; +import { User } from '@server/entity/User'; +import { UserSettings } from '@server/entity/UserSettings'; +import assert from 'node:assert/strict'; + +const credentials = { + pgpKey: 'test-pgp-key', + discordIds: ['112233445566'], + pushbulletAccessToken: 'test-pushbullet-token', + pushoverApplicationToken: 'test-pushover-app-token', + pushoverUserKey: 'test-pushover-user-key', + telegramChatId: '-1001234567', +}; + +export const allowlistedSettings = { + locale: 'de', + discoverRegion: 'DE', + streamingRegion: 'DE', + originalLanguage: 'de', + watchlistSyncMovies: true, + watchlistSyncTv: false, +}; + +export async function seedUserSettings(email: string): Promise { + const userRepository = getRepository(User); + const user = await userRepository.findOneOrFail({ where: { email } }); + + user.settings = new UserSettings({ + ...credentials, + ...allowlistedSettings, + notificationTypes: {}, + }); + + return userRepository.save(user); +} + +export function assertNoCredentials(body: unknown): void { + const serialized = JSON.stringify(body); + + const secrets = [ + credentials.pgpKey, + ...credentials.discordIds, + credentials.pushbulletAccessToken, + credentials.pushoverApplicationToken, + credentials.pushoverUserKey, + credentials.telegramChatId, + ]; + + for (const secret of secrets) { + assert.ok(!serialized.includes(secret), `${secret} was serialized`); + } +}