diff --git a/services/api/src/routes/users.test.js b/services/api/src/routes/users.test.js index 14874ca8..0b14f1d1 100644 --- a/services/api/src/routes/users.test.js +++ b/services/api/src/routes/users.test.js @@ -61,6 +61,20 @@ describe('/1/users', () => { expect(updatedUser.name).toBe('Other Name'); }); + it('should allow submitting an unchanged email', async () => { + const user = await createUser({ email: 'self@bar.com' }); + const response = await request('PATCH', '/1/users/me', { firstName: 'New', email: 'self@bar.com' }, { user }); + expect(response).toHaveStatus(200); + expect(response.body.data.firstName).toBe('New'); + }); + + it('should still reject an email taken by another user', async () => { + await createUser({ email: 'other@bar.com' }); + const user = await createUser({ email: 'self@bar.com' }); + const response = await request('PATCH', '/1/users/me', { email: 'other@bar.com' }, { user }); + expect(response).toHaveStatus(400); + }); + it('should be able to patch the device token', async () => { let user = await createUser(); const response = await request( @@ -322,6 +336,27 @@ describe('/1/users', () => { expect(dbUser.name).toEqual('New Name'); }); + it('should allow submitting an unchanged email', async () => { + const admin = await createAdmin(); + const user1 = await createUser({ email: 'old@bar.com' }); + const response = await request( + 'PATCH', + `/1/users/${user1.id}`, + { firstName: 'New', email: 'old@bar.com' }, + { user: admin }, + ); + expect(response).toHaveStatus(200); + expect(response.body.data.firstName).toBe('New'); + }); + + it('should still reject an email taken by another user', async () => { + const admin = await createAdmin(); + await createUser({ email: 'taken@bar.com' }); + const user1 = await createUser({ email: 'mine@bar.com' }); + const response = await request('PATCH', `/1/users/${user1.id}`, { email: 'taken@bar.com' }, { user: admin }); + expect(response).toHaveStatus(400); + }); + it('should deny access to non-admins', async () => { const user = await createUser({}); const user1 = await createUser({ firstName: 'New', lastName: 'Name' }); diff --git a/services/api/src/utils/middleware/params.js b/services/api/src/utils/middleware/params.js index 5a4bf34a..b4165a44 100644 --- a/services/api/src/utils/middleware/params.js +++ b/services/api/src/utils/middleware/params.js @@ -17,6 +17,7 @@ function fetchByParam(Model, options = {}) { ctx.throw(403); } ctx.state[docName] = doc; + excludeFromUniqueChecks(ctx, doc); } catch (error) { ctx.throw(400, error); } @@ -35,10 +36,20 @@ function fetchByParamWithSlug(Model, options) { ctx.throw(401); } ctx.state[docName] = doc; + excludeFromUniqueChecks(ctx, doc); return next(); }; } +// Unique checks in update validation exclude the target document by an id in +// the body, which clients have no reason to send. Take it from the document the +// route already resolved. Update validation strips it again before assign. +function excludeFromUniqueChecks(ctx, doc) { + if (ctx.method === 'PATCH' || ctx.method === 'PUT') { + ctx.request.body.id = doc.id; + } +} + async function checkAccess(ctx, doc, options = {}) { const { hasAccess = () => true } = options; return await hasAccess(ctx, doc); @@ -49,6 +60,7 @@ async function checkAccess(ctx, doc, options = {}) { // when performing "self" checks for "writeAccess". function isSelf(ctx, next) { ctx.state.user = ctx.state.authUser; + excludeFromUniqueChecks(ctx, ctx.state.authUser); return next(); } diff --git a/services/api/src/utils/templates.js b/services/api/src/utils/templates.js index 8a5f5acc..39cf05ae 100644 --- a/services/api/src/utils/templates.js +++ b/services/api/src/utils/templates.js @@ -17,10 +17,12 @@ const renderer = new TemplateRenderer({ async function renderTemplate(options) { const { dir, ...rest } = resolveOptions(options); - const template = await resolveTemplateArg(options); + const { template, isFile } = await resolveTemplateArg(options); return renderer.run({ - dir, + // A body loaded from the database is the template source itself, so + // resolving it against "dir" would read it as a (very long) filename. + dir: isFile ? dir : undefined, template, params: { ...rest, @@ -50,16 +52,16 @@ async function resolveTemplateArg(options) { if (typeof template === 'string' && path.extname(template) !== '') { // Return template filename if an extension if found. - return template; + return { template, isFile: true }; } else if (template) { const templateBody = await resolveTemplateBody(options); // If the template arg is a name then find the template // and return the body for the channel. Fall back to the // template name to find it as a file. - return templateBody || template; + return templateBody ? { template: templateBody } : { template, isFile: true }; } else { - return body; + return { template: body }; } } diff --git a/services/api/src/utils/templates.test.js b/services/api/src/utils/templates.test.js index 3a0c176f..dd11c9c9 100644 --- a/services/api/src/utils/templates.test.js +++ b/services/api/src/utils/templates.test.js @@ -56,6 +56,26 @@ describe('renderTemplate', () => { expect(result.body).toBe('Hello from doc, Frank'); }); + it('should render a long template from document', async () => { + // A body longer than the filesystem name limit must not be resolved as a path. + const email = ['---', 'subject: Welcome', '---', '', 'Hello {{name}},', '', 'x'.repeat(500)].join('\n'); + await Template.create({ + name: 'long', + email, + }); + + const result = await renderTemplate({ + channel: 'email', + template: 'long', + params: { + name: 'Frank', + }, + }); + + expect(result.meta.subject).toBe('Welcome'); + expect(result.body).toContain('Hello Frank,'); + }); + it('should be able to pass a raw template with channel', async () => { const result = await renderTemplate({ template: 'Hello',