diff --git a/src/controllers/locations.js b/src/controllers/locations.js index bc3d50f..6a073d6 100644 --- a/src/controllers/locations.js +++ b/src/controllers/locations.js @@ -33,6 +33,7 @@ const locationAssociations = { include: [ { model: models.Organization, + attributes: models.Organization.getPublicAttributes(), include: [models.Phone], }, models.Phone, diff --git a/src/controllers/organizations.js b/src/controllers/organizations.js index 2199dc3..de394ef 100644 --- a/src/controllers/organizations.js +++ b/src/controllers/organizations.js @@ -2,7 +2,26 @@ import Joi from 'joi'; import organizationSchemas from './validation/organizations'; import models from '../models'; import { updateInstance, createInstance } from '../services/data-changes'; -import { NotFoundError } from '../utils/errors'; +import { ForbiddenError, NotFoundError } from '../utils/errors'; + +const normalizeEmail = (email) => { + if (email == null) { + return email; + } + + const trimmedEmail = email.trim(); + return trimmedEmail || null; +}; + +const assertUserCanAccessOrganization = (req, organizationId) => { + if (req.userIsAdmin) { + return; + } + + if (!req.userOrganizationIds || !req.userOrganizationIds.includes(organizationId)) { + throw new ForbiddenError('Not authorized to view this organization'); + } +}; export default { find: async (req, res, next) => { @@ -12,7 +31,9 @@ export default { const { searchString } = req.query; const filterParameters = searchString ? { searchString: searchString.trim() } : {}; - const organizations = await models.Organization.findMatching(filterParameters); + const organizations = await models.Organization.findMatching(filterParameters, 10, { + attributes: models.Organization.getPublicAttributes(), + }); res.send(organizations); } catch (err) { next(err); @@ -26,14 +47,17 @@ export default { const { name, description, + email: rawEmail, url, metadata, } = req.body; + const email = normalizeEmail(rawEmail); const modelCreateFunction = models.Organization.create.bind(models.Organization); const createdOrganization = await createInstance(req.user, modelCreateFunction, { name, description, + email, url, }, { metadata }); @@ -48,18 +72,23 @@ export default { await Joi.validate(req, organizationSchemas.update, { allowUnknown: true }); const { organizationId } = req.params; + assertUserCanAccessOrganization(req, organizationId); const organization = await models.Organization.findByPk(organizationId); if (!organization) { throw new NotFoundError('Organization not found'); } - const editableFields = ['name', 'description', 'url']; - const { metadata, ...updateParams } = req.body; + const editableFields = ['name', 'description', 'email', 'url']; + const { metadata, email, ...updateParams } = req.body; + const normalizedUpdateParams = email === undefined ? updateParams : { + ...updateParams, + email: normalizeEmail(email), + }; await updateInstance( req.user, organization, - updateParams, + normalizedUpdateParams, { fields: editableFields, metadata }, ); @@ -104,4 +133,22 @@ export default { next(err); } }, + + get: async (req, res, next) => { + try { + await Joi.validate(req, organizationSchemas.get, { allowUnknown: true }); + + const { organizationId } = req.params; + assertUserCanAccessOrganization(req, organizationId); + + const organization = await models.Organization.findByPk(organizationId); + if (!organization) { + throw new NotFoundError('Organization not found'); + } + + res.send(organization); + } catch (err) { + next(err); + } + }, }; diff --git a/src/controllers/validation/organizations.js b/src/controllers/validation/organizations.js index ac9b7ba..5d75118 100644 --- a/src/controllers/validation/organizations.js +++ b/src/controllers/validation/organizations.js @@ -4,6 +4,7 @@ const updateMetadataSchema = Joi.object().keys({ source: Joi.string(), lastUpdated: Joi.date().iso(), }); +const emailSchema = Joi.string().trim().email().allow('', null); export default { find: { @@ -16,6 +17,7 @@ export default { body: Joi.object().keys({ name: Joi.string().required(), description: Joi.string(), + email: emailSchema, url: Joi.string(), metadata: updateMetadataSchema, }).required(), @@ -28,11 +30,18 @@ export default { body: Joi.object().keys({ name: Joi.string(), description: Joi.string().allow(''), + email: emailSchema, url: Joi.string().allow(''), metadata: updateMetadataSchema, }).required(), }, + get: { + params: Joi.object().keys({ + organizationId: Joi.string().guid().required(), + }).required(), + }, + getLocations: { params: Joi.object().keys({ organizationId: Joi.string().guid().required(), diff --git a/src/models/location.js b/src/models/location.js index 6360657..53d2128 100644 --- a/src/models/location.js +++ b/src/models/location.js @@ -594,7 +594,10 @@ module.exports = (sequelize, DataTypes, Op) => { ], }, ] : [ - sequelize.models.Organization, + { + model: sequelize.models.Organization, + attributes: sequelize.models.Organization.getPublicAttributes(), + }, sequelize.models.EventRelatedInfo, { model: sequelize.models.Service, diff --git a/src/models/organization.js b/src/models/organization.js index 4c6c0fa..94e5f93 100644 --- a/src/models/organization.js +++ b/src/models/organization.js @@ -31,7 +31,7 @@ module.exports = (sequelize, DataTypes, Op) => { Organization.hasMany(models.Phone, { foreignKey: 'organization_id' }); }; - Organization.findMatching = (filterParameters, limit = 10) => { + Organization.findMatching = (filterParameters, limit = 10, queryOptions = {}) => { const { searchString } = filterParameters; const where = {}; @@ -39,8 +39,12 @@ module.exports = (sequelize, DataTypes, Op) => { where.name = { [Op.iLike]: `%${searchString}%` }; } - return Organization.findAll({ limit, where }); + return Organization.findAll({ limit, where, ...queryOptions }); }; + Organization.getPublicAttributes = () => ({ + exclude: ['email'], + }); + return Organization; }; diff --git a/src/routes.js b/src/routes.js index 7c3b231..3b1dbcf 100644 --- a/src/routes.js +++ b/src/routes.js @@ -18,6 +18,7 @@ import { export default (app) => { app.get('/organizations', organizations.find); + app.get('/organizations/:organizationId', getUser, organizations.get); app.post('/organizations', getUser, dataEntryAuth, organizations.create); app.patch('/organizations/:organizationId', getUser, dataEntryAuth, organizations.update); app.get('/organizations/:organizationId/locations', organizations.getLocations); diff --git a/test/integration/__snapshots__/get-location-info.test.js.snap b/test/integration/__snapshots__/get-location-info.test.js.snap index cdc61e0..7be643b 100644 --- a/test/integration/__snapshots__/get-location-info.test.js.snap +++ b/test/integration/__snapshots__/get-location-info.test.js.snap @@ -7,7 +7,6 @@ Object { "Organization": Object { "Phones": Object {}, "description": "An organization meant for testing purposes.", - "email": null, "name": "The Test Org", "url": "www.streetlives.com", }, diff --git a/test/integration/create-organization.test.js b/test/integration/create-organization.test.js index 57593df..be94db7 100644 --- a/test/integration/create-organization.test.js +++ b/test/integration/create-organization.test.js @@ -29,6 +29,22 @@ describe('create organization', () => { expect(dbOrg).toHaveProperty('url', orgParams.url); }); + it('should trim and persist a valid email address', async () => { + await request(app) + .post('/organizations') + .send({ ...orgParams, name: 'Org With Email', email: ' intake@streetlives.org ' }) + .expect(201); + + const dbOrg = await models.Organization.findOne({ where: { name: 'Org With Email' } }); + expect(dbOrg).toHaveProperty('email', 'intake@streetlives.org'); + }); + + it('should reject an invalid email address', () => + request(app) + .post('/organizations') + .send({ ...orgParams, name: 'Org With Invalid Email', email: 'not-an-email' }) + .expect(400)); + describe('when no custom metadata is specified', () => { it('should create metadata with current time as the last action date', async () => { const startTime = Date.now(); diff --git a/test/integration/find-locations.test.js b/test/integration/find-locations.test.js index 66c5976..85ae0ea 100644 --- a/test/integration/find-locations.test.js +++ b/test/integration/find-locations.test.js @@ -53,6 +53,7 @@ describe('find locations', () => { { name: 'The Test Org', description: 'An organization meant for testing purposes.', + email: 'contact@streetlives.org', Services: [ { name: 'A specific offering', @@ -209,6 +210,21 @@ describe('find locations', () => { .expect(200) .then(expectMatchNearbyLocations)); + it('should not expose organization email in public location results', () => + request(app) + .get('/locations') + .query({ + latitude: originLatitude, + longitude: originLongitude, + radius, + }) + .expect(200) + .then((res) => { + res.body.forEach((returnedLocation) => { + expect(returnedLocation.Organization).not.toHaveProperty('email'); + }); + })); + it('should not return locations marked "hidden from search" (meant for comments only)', () => request(app) .get('/locations') diff --git a/test/integration/get-location-info.test.js b/test/integration/get-location-info.test.js index f661498..12ab473 100644 --- a/test/integration/get-location-info.test.js +++ b/test/integration/get-location-info.test.js @@ -13,6 +13,7 @@ describe('get location info', () => { { name: 'The Test Org', description: 'An organization meant for testing purposes.', + email: 'contact@streetlives.org', url: 'www.streetlives.com', Services: [{ name: 'A specific offering', @@ -86,6 +87,7 @@ describe('get location info', () => { .get(`/locations/${location.id}`) .expect(200) .then((res) => { + expect(res.body.Organization).not.toHaveProperty('email'); const strippedFields = stripTimestampsAndIds(res.body); expect(strippedFields).toMatchSnapshot(); })); diff --git a/test/integration/get-organizations.test.js b/test/integration/get-organizations.test.js new file mode 100644 index 0000000..02f9fc6 --- /dev/null +++ b/test/integration/get-organizations.test.js @@ -0,0 +1,111 @@ +/** + * @jest-environment node + */ + +import express from 'express'; +import request from 'supertest'; +import app from '../../src/app'; +import models from '../../src/models'; + +describe('get organizations', () => { + let organization; + + const buildAppWithClaims = (claims) => { + const appWithClaims = express(); + appWithClaims.use((req, res, next) => { + req.apiGateway = { + event: { + requestContext: { + authorizer: { + claims, + }, + }, + }, + }; + next(); + }); + appWithClaims.use(app); + return appWithClaims; + }; + + const buildProductionApp = () => { + let productionApp; + const originalNodeEnv = process.env.NODE_ENV; + + jest.isolateModules(() => { + process.env.NODE_ENV = 'production'; + productionApp = require('../../src/app').default; // eslint-disable-line global-require + }); + + process.env.NODE_ENV = originalNodeEnv; + return productionApp; + }; + + beforeEach(async () => { + organization = await models.Organization.create({ + name: 'Organization With Email', + description: 'An organization used to test public/private payloads.', + email: 'contact@streetlives.org', + url: 'www.streetlives.com', + }); + }); + + afterEach(() => models.Organization.destroy({ where: {} })); + + it('should not expose email in the public organizations list', async () => { + const res = await request(app) + .get('/organizations') + .expect(200); + + const returnedOrganization = res.body.find(org => org.id === organization.id); + expect(returnedOrganization).toBeDefined(); + expect(returnedOrganization).not.toHaveProperty('email'); + }); + + it('should return 401 when no authenticated user is present in production', () => + request(buildProductionApp()) + .get(`/organizations/${organization.id}`) + .expect(401)); + + it('should return 403 when an anonymous test user fetches a specific organization', () => + request(app) + .get(`/organizations/${organization.id}`) + .expect(403)); + + it('should return 403 when the authenticated user is not authorized for the organization', () => { + const appWithClaims = buildAppWithClaims({ + sub: 'user-id', + 'custom:orgs': '11111111-1111-1111-1111-111111111111', + }); + + return request(appWithClaims) + .get(`/organizations/${organization.id}`) + .expect(403); + }); + + it('should return email for an organization member', async () => { + const appWithClaims = buildAppWithClaims({ + sub: 'user-id', + 'custom:orgs': organization.id, + }); + + const res = await request(appWithClaims) + .get(`/organizations/${organization.id}`) + .expect(200); + + expect(res.body).toHaveProperty('email', organization.email); + }); + + it('should return email when fetching a specific organization as an admin', async () => { + const appWithClaims = buildAppWithClaims({ + sub: 'admin-id', + 'cognito:groups': 'StreetlivesAdmins', + }); + + const res = await request(appWithClaims) + .get(`/organizations/${organization.id}`) + .expect(200); + + expect(res.body).toHaveProperty('email', organization.email); + }); +}); diff --git a/test/integration/update-organization.test.js b/test/integration/update-organization.test.js new file mode 100644 index 0000000..ad6efeb --- /dev/null +++ b/test/integration/update-organization.test.js @@ -0,0 +1,138 @@ +/** + * @jest-environment node + */ + +import express from 'express'; +import request from 'supertest'; +import app from '../../src/app'; +import models from '../../src/models'; + +describe('update organization', () => { + let organization; + + const buildAppWithClaims = (claims) => { + const appWithClaims = express(); + appWithClaims.use((req, res, next) => { + req.apiGateway = { + event: { + requestContext: { + authorizer: { + claims, + }, + }, + }, + }; + next(); + }); + appWithClaims.use(app); + return appWithClaims; + }; + + const buildProductionApp = () => { + let productionApp; + const originalNodeEnv = process.env.NODE_ENV; + + jest.isolateModules(() => { + process.env.NODE_ENV = 'production'; + productionApp = require('../../src/app').default; // eslint-disable-line global-require + }); + + process.env.NODE_ENV = originalNodeEnv; + return productionApp; + }; + + beforeEach(async () => { + organization = await models.Organization.create({ + name: 'Existing Org', + description: 'An organization for update tests.', + email: 'existing@streetlives.org', + url: 'www.streetlives.com', + }); + }); + + afterEach(() => Promise.all([ + models.Organization.destroy({ where: {} }), + models.Metadata.destroy({ where: {} }), + ])); + + it('should return 401 when no authenticated user is present in production', () => + request(buildProductionApp()) + .patch(`/organizations/${organization.id}`) + .send({ email: 'updated@streetlives.org' }) + .expect(401)); + + it('should return 403 when an anonymous test user updates a specific organization', () => + request(app) + .patch(`/organizations/${organization.id}`) + .send({ email: 'updated@streetlives.org' }) + .expect(403)); + + it('should return 403 when the authenticated user is not authorized for the organization', () => { + const appWithClaims = buildAppWithClaims({ + sub: 'user-id', + 'custom:orgs': '11111111-1111-1111-1111-111111111111', + }); + + return request(appWithClaims) + .patch(`/organizations/${organization.id}`) + .send({ email: 'updated@streetlives.org' }) + .expect(403); + }); + + it('should trim and persist a valid email address for an organization member', async () => { + const appWithClaims = buildAppWithClaims({ + sub: 'user-id', + 'custom:orgs': organization.id, + }); + + await request(appWithClaims) + .patch(`/organizations/${organization.id}`) + .send({ email: ' updated@streetlives.org ' }) + .expect(204); + + const updatedOrganization = await models.Organization.findByPk(organization.id); + expect(updatedOrganization).toHaveProperty('email', 'updated@streetlives.org'); + }); + + it('should allow admins to update organization email', async () => { + const appWithClaims = buildAppWithClaims({ + sub: 'admin-id', + 'cognito:groups': 'StreetlivesAdmins', + }); + + await request(appWithClaims) + .patch(`/organizations/${organization.id}`) + .send({ email: 'admin-updated@streetlives.org' }) + .expect(204); + + const updatedOrganization = await models.Organization.findByPk(organization.id); + expect(updatedOrganization).toHaveProperty('email', 'admin-updated@streetlives.org'); + }); + + it('should clear email when updated with a blank string by an organization member', async () => { + const appWithClaims = buildAppWithClaims({ + sub: 'user-id', + 'custom:orgs': organization.id, + }); + + await request(appWithClaims) + .patch(`/organizations/${organization.id}`) + .send({ email: ' ' }) + .expect(204); + + const updatedOrganization = await models.Organization.findByPk(organization.id); + expect(updatedOrganization).toHaveProperty('email', null); + }); + + it('should reject an invalid email address for an authorized organization member', () => { + const appWithClaims = buildAppWithClaims({ + sub: 'user-id', + 'custom:orgs': organization.id, + }); + + return request(appWithClaims) + .patch(`/organizations/${organization.id}`) + .send({ email: 'invalid-email' }) + .expect(400); + }); +});