diff --git a/admin-client/src/pages/Site.svelte b/admin-client/src/pages/Site.svelte index 3281907c0..902020791 100644 --- a/admin-client/src/pages/Site.svelte +++ b/admin-client/src/pages/Site.svelte @@ -9,7 +9,6 @@ fetchTasks, fetchBuildTaskTypes, fetchUserEnvironmentVariables, - fetchUsers, updateSite, addSiteBuildTask, updateSiteBuildTask, @@ -29,7 +28,6 @@ SiteFormWebhook, SiteMetadata, TaskTable, - UserTable, } from '../components'; import { destroySite } from '../flows'; import { selectSiteDomains, stateColor } from '../lib/utils'; @@ -43,7 +41,6 @@ $: buildTasksPromise = fetchTasks({ site: id, limit: 10 }); $: buildTaskTypesPromise = fetchBuildTaskTypes(); $: orgsPromise = fetchOrganizations({ limit: 100 }); - $: usersPromise = fetchUsers({ site: id }); $: uevsPromise = fetchUserEnvironmentVariables({ site: id }); const initEditedBuildTask = { @@ -308,11 +305,6 @@ - - - - - diff --git a/api/admin/controllers/site.js b/api/admin/controllers/site.js index 2fa9edbe6..7c0b02406 100644 --- a/api/admin/controllers/site.js +++ b/api/admin/controllers/site.js @@ -69,8 +69,9 @@ module.exports = wrapHandlers({ params: { id }, } = req; - const site = await Site.withUsers(id); - const hook = await GithubBuildHelper.createSiteWebhook(site, site.Users); + const site = await Site.findOne({ where: { id } }); + const users = site.getOrgUsers(); + const hook = await GithubBuildHelper.createSiteWebhook(site, users); return res.json(hook || []); }, @@ -80,8 +81,9 @@ module.exports = wrapHandlers({ params: { id }, } = req; - const site = await Site.withUsers(id); - const hooks = await GithubBuildHelper.listSiteWebhooks(site, site.Users); + const site = await Site.findOne({ where: { id } }); + const users = site.getOrgUsers(); + const hooks = await GithubBuildHelper.listSiteWebhooks(site, users); return res.json(hooks); }, diff --git a/api/admin/controllers/user.js b/api/admin/controllers/user.js index 70ab7b528..5ddcd5c91 100644 --- a/api/admin/controllers/user.js +++ b/api/admin/controllers/user.js @@ -80,7 +80,7 @@ module.exports = wrapHandlers({ }, async list(req, res) { - const { limit, page, organization, search, site } = req.query; + const { limit, page, organization, search } = req.query; const serialize = (users) => userSerializer.serializeMany(users, true); @@ -90,10 +90,6 @@ module.exports = wrapHandlers({ scopes.push(User.searchScope(search)); } - if (site) { - scopes.push(User.siteScope(site)); - } - if (organization) { scopes.push(User.orgScope(organization)); scopes.push('withOrganizationRoles'); diff --git a/api/authorizers/site.js b/api/authorizers/site.js index 286a12fb0..317db0a93 100644 --- a/api/authorizers/site.js +++ b/api/authorizers/site.js @@ -101,8 +101,6 @@ const create = async (user, siteParams) => { } }; -const addUser = () => Promise.resolve(); - const createBuild = (user, site) => authorize(user, site); const showActions = (user, site) => authorize(user, site); @@ -114,15 +112,11 @@ const update = (user, site) => authorize(user, site); const destroy = (user, site) => authorize(user, site).then(() => authorizeRepositoryAdmin(user, site)); -const removeUser = (user, site) => authorize(user, site); - module.exports = { create, findOne, update, destroy, - addUser, - removeUser, showActions, createBuild, }; diff --git a/api/controllers/build.js b/api/controllers/build.js index 049d029c8..41f770b76 100644 --- a/api/controllers/build.js +++ b/api/controllers/build.js @@ -7,7 +7,7 @@ const siteAuthorizer = require('../authorizers/site'); const SocketIOSubscriber = require('../services/SocketIOSubscriber'); const EventCreator = require('../services/EventCreator'); const { wrapHandlers } = require('../utils'); -const { Build, Domain, User, Event, Site, SiteBranchConfig } = require('../models'); +const { Build, Domain, Event, Site, SiteBranchConfig } = require('../models'); const { getSocket } = require('../socketIO'); const decodeb64 = (str) => Buffer.from(str, 'base64').toString('utf8'); @@ -89,16 +89,7 @@ module.exports = wrapHandlers({ id: req.body.buildId, site: req.body.siteId, }, - include: [ - { - model: Site, - include: [ - { - model: User, - }, - ], - }, - ], + include: Site, }); if (!requestBuild) { @@ -134,20 +125,7 @@ module.exports = wrapHandlers({ async status(req, res) { const { params, body } = req; - const build = await fetchModelById(params.id, Build, { - include: [ - { - model: Site, - include: [ - { - model: User, - }, - Domain, - SiteBranchConfig, - ], - }, - ], - }); + const build = await fetchModelById(params.id, Build, { include: Site }); if (!build) { return res.notFound(); @@ -189,13 +167,7 @@ module.exports = wrapHandlers({ include: [ { model: Site, - include: [ - { - model: User, - }, - Domain, - SiteBranchConfig, - ], + include: [Domain, SiteBranchConfig], }, ], }); diff --git a/api/controllers/site.js b/api/controllers/site.js index e325fccc1..9c8b6d89e 100644 --- a/api/controllers/site.js +++ b/api/controllers/site.js @@ -2,8 +2,6 @@ const _ = require('underscore'); const authorizer = require('../authorizers/site'); const SiteCreator = require('../services/SiteCreator'); const SiteDestroyer = require('../services/SiteDestroyer'); -const SiteMembershipCreator = require('../services/SiteMembershipCreator'); -const UserActionCreator = require('../services/UserActionCreator'); const EventCreator = require('../services/EventCreator'); const siteSerializer = require('../serializers/site'); const domainSerializer = require('../serializers/domain'); @@ -11,7 +9,6 @@ const buildTaskSerializer = require('../serializers/build-task'); const { Organization, Site, - User, Event, Domain, SiteBranchConfig, @@ -19,26 +16,10 @@ const { BuildTaskType, BuildTask, } = require('../models'); -const siteErrors = require('../responses/siteErrors'); -const { - ValidationError, - validBasicAuthUsername, - validBasicAuthPassword, -} = require('../utils/validators'); + const { toInt, wrapHandlers, appMatch } = require('../utils'); const { fetchModelById } = require('../utils/queryDatabase'); -const stripCredentials = ({ username, password }) => { - if (validBasicAuthUsername(username) && validBasicAuthPassword(password)) { - return { - username, - password, - }; - } - - throw new ValidationError('username or password is not valid.'); -}; - module.exports = wrapHandlers({ async findAllForUser(req, res) { const { user } = req; @@ -99,69 +80,6 @@ module.exports = wrapHandlers({ return res.json({}); }, - async addUser(req, res) { - const { body, user } = req; - if (!body.owner || !body.repository) { - return res.badRequest(); - } - - await authorizer.addUser(user, body); - const site = await SiteMembershipCreator.createSiteMembership({ - user, - siteParams: body, - }); - EventCreator.audit(Event.labels.USER_ACTION, req.user, 'SiteUser Created', { - siteUser: { - siteId: site.id, - userId: user.id, - }, - }); - const siteJSON = await siteSerializer.serialize(site); - return res.json(siteJSON); - }, - - async removeUser(req, res) { - const siteId = Number(req.params.site_id); - const userId = Number(req.params.user_id); - - if (_.isNaN(siteId) || _.isNaN(userId)) { - return res.badRequest(); - } - - const site = await Site.withUsers(siteId); - if (!site) { - return res.notFound(); - } - - if (site.Users.length === 1) { - return res.badRequest({ - message: siteErrors.USER_REQUIRED, - }); - } - - await authorizer.removeUser(req.user, site); - await SiteMembershipCreator.revokeSiteMembership({ - user: req.user, - site, - userId, - }); - EventCreator.audit(Event.labels.USER_ACTION, req.user, 'SiteUser Removed', { - siteUser: { - siteId: site.id, - userId, - }, - }); - // UserActionCreator to be deleted - await UserActionCreator.addRemoveAction({ - userId: req.user.id, - targetId: userId, - targetType: 'user', - siteId: site.id, - }); - const siteJSON = await siteSerializer.serialize(site); - return res.json(siteJSON); - }, - async create(req, res) { const { body: { owner, template, organizationId, repository, engine }, @@ -185,7 +103,7 @@ module.exports = wrapHandlers({ site, }); await site.reload({ - include: [Organization, User], + include: [Organization], }); const siteJSON = siteSerializer.serializeNew(site); return res.json(siteJSON); @@ -225,45 +143,6 @@ module.exports = wrapHandlers({ return res.json(siteJSON); }, - async addBasicAuth(req, res) { - const { body, params, user } = req; - - const { site_id: siteId } = params; - - const site = await fetchModelById(siteId, Site.forUser(user)); - - if (!site) { - return res.notFound(); - } - - const credentials = stripCredentials(body); - - await site.update({ - basicAuth: credentials, - }); - - const siteJSON = siteSerializer.serializeNew(site); - return res.json(siteJSON); - }, - - async removeBasicAuth(req, res) { - const { params, user } = req; - const { site_id: siteId } = params; - - const site = await fetchModelById(siteId, Site.forUser(user)); - - if (!site) { - return res.notFound(); - } - - await site.update({ - basicAuth: {}, - }); - - const siteJSON = siteSerializer.serializeNew(site); - return res.json(siteJSON); - }, - async getSiteDomains(req, res) { const { user, diff --git a/api/models/build.js b/api/models/build.js index 6b3967d13..b89a74814 100644 --- a/api/models/build.js +++ b/api/models/build.js @@ -20,15 +20,7 @@ const States = buildEnum([ 'success', ]); -const associate = ({ - Build, - BuildLog, - BuildTask, - Organization, - Site, - User, - OrganizationRole, -}) => { +const associate = ({ Build, BuildLog, BuildTask, Organization, Site, User }) => { // Associations Build.hasMany(BuildLog, { foreignKey: 'build', @@ -69,59 +61,28 @@ const associate = ({ }, ], })); + // this name is a relic from when Site's had Users, now it describes Users + // with access to the Site via an Organization Build.addScope('forSiteUser', (user) => ({ where: { - [Op.and]: [ - { - [Op.or]: [ - { - '$Site.Users.id$': { - [Op.not]: null, - }, - }, - { - '$Site.organizationId$': { - [Op.not]: null, - }, - }, - ], - }, - { - [Op.or]: [ - { - '$Site.Users.id$': user.id, - }, - { - '$Site.Organization.OrganizationRoles.userId$': user.id, - }, - ], - }, - ], + '$Site.Organization.OrganizationRoles.userId$': user.id, }, - include: [ - { - model: Site, - required: true, - include: [ - { - model: User, - required: false, - }, - { - model: Organization, - required: false, - include: [ - { - model: OrganizationRole, - }, - ], - }, - ], - }, - ], + include: Site.scope('withOrgUsers'), })); }; +async function getSiteOrgUsers() { + const build = this; + + const { Site } = build.sequelize.models; + + if (!build.Site) { + await build.reload({ include: Site }); + } + + return build.Site.getOrgUsers(); +} + const generateToken = () => URLSafeBase64.encode(crypto.randomBytes(32)); const beforeValidate = (build) => { @@ -334,6 +295,7 @@ module.exports = (sequelize, DataTypes) => { Build.prototype.isComplete = isComplete; Build.prototype.isInProgress = isInProgress; Build.prototype.canStart = canStart; + Build.prototype.getSiteOrgUsers = getSiteOrgUsers; Build.States = States; Build.orgScope = (id) => ({ method: ['byOrg', id], diff --git a/api/models/index.js b/api/models/index.js index 3e3defd6a..161117dcb 100644 --- a/api/models/index.js +++ b/api/models/index.js @@ -33,7 +33,6 @@ require('./build-task')(sequelize, DataTypes); require('./site')(sequelize, DataTypes); require('./site-branch-config')(sequelize, DataTypes); require('./site-build-task')(sequelize, DataTypes); -require('./site-user')(sequelize, DataTypes); require('./user')(sequelize, DataTypes); require('./user-action')(sequelize, DataTypes); require('./action-type')(sequelize, DataTypes); diff --git a/api/models/organization.js b/api/models/organization.js index b6a6aac76..d179fcac9 100644 --- a/api/models/organization.js +++ b/api/models/organization.js @@ -82,6 +82,23 @@ const associate = ({ Organization, OrganizationRole, Role, Site, User }) => { }); }; +async function addRoleUser(user, roleName = 'user') { + const org = this; + const { Role } = this.sequelize.models; + + const role = await Role.findOne({ + where: { + name: roleName, + }, + }); + + return org.addUser(user, { + through: { + roleId: role.id, + }, + }); +} + module.exports = (sequelize, DataTypes) => { const Organization = sequelize.define( 'Organization', @@ -152,5 +169,6 @@ module.exports = (sequelize, DataTypes) => { Organization.scope({ method: ['forManagerRole', user], }); + Organization.prototype.addRoleUser = addRoleUser; return Organization; }; diff --git a/api/models/site-user.js b/api/models/site-user.js deleted file mode 100644 index d6361152d..000000000 --- a/api/models/site-user.js +++ /dev/null @@ -1,31 +0,0 @@ -const associate = ({ User, Site, SiteUser }) => { - SiteUser.belongsTo(Site, { - foreignKey: 'site_users', - targetKey: 'id', - }); - SiteUser.belongsTo(User, { - foreignKey: 'user_sites', - targetKey: 'id', - }); -}; - -module.exports = (sequelize, DataTypes) => { - const SiteUser = sequelize.define( - 'SiteUser', - { - buildNotificationSetting: { - type: DataTypes.ENUM, - values: ['none', 'builds', 'site'], - defaultValue: 'site', - }, - }, - { - tableName: 'site_users__user_sites', - timestamps: false, - }, - ); - - SiteUser.associate = associate; - - return SiteUser; -}; diff --git a/api/models/site.js b/api/models/site.js index c81ca2972..25f0c427e 100644 --- a/api/models/site.js +++ b/api/models/site.js @@ -35,7 +35,6 @@ const associate = ({ Site, SiteBranchConfig, SiteBuildTask, - SiteUser, User, UserAction, UserEnvironmentVariable, @@ -50,11 +49,6 @@ const associate = ({ Site.hasMany(SiteBranchConfig, { foreignKey: 'siteId', }); - Site.belongsToMany(User, { - through: SiteUser, - foreignKey: 'site_users', - timestamps: false, - }); Site.hasMany(UserAction, { as: 'userActions', foreignKey: 'siteId', @@ -102,46 +96,25 @@ const associate = ({ }, ], })); + // this name is a relic from when Site's had Users, now it describes Users + // with access to the Site via an Organization + // don't call this scope directly because it requires the below `includes` + // from `withOrgUsers Site.addScope('forUser', (user) => ({ where: { - [Op.and]: [ - { - [Op.or]: [ - { - '$Users.id$': { - [Op.not]: null, - }, - }, - { - organizationId: { - [Op.not]: null, - }, - }, - ], - }, - { - [Op.or]: [ - { - '$Users.id$': user.id, - }, - { - '$Organization.OrganizationRoles.userId$': user.id, - }, - ], - }, - ], + '$Organization.OrganizationRoles.userId$': user.id, }, + })); + + Site.addScope('withOrgUsers', () => ({ include: [ - { - model: User, - required: false, - }, { model: Organization, required: false, include: [ { model: OrganizationRole, + include: User, }, ], }, @@ -158,6 +131,16 @@ const beforeValidate = (site) => { } }; +async function getOrgUsers() { + const site = this; + + const { Site } = site.sequelize.models; + + const foundSite = await Site.withOrgUsers().findOne({ where: { id: site.id } }); + + return foundSite.Organization.OrganizationRoles.map((role) => role.User); +} + module.exports = (sequelize, DataTypes) => { const Site = sequelize.define( 'Site', @@ -293,10 +276,6 @@ module.exports = (sequelize, DataTypes) => { ); Site.associate = associate; - Site.withUsers = (id) => - Site.findByPk(id, { - include: [sequelize.models.User], - }); Site.orgScope = (id) => ({ method: ['byOrg', id], }); @@ -304,11 +283,14 @@ module.exports = (sequelize, DataTypes) => { method: ['byIdOrText', search], }); Site.forUser = (user) => + Site.scope([{ method: ['forUser', user] }, { method: ['withOrgUsers'] }]); + Site.withOrgUsers = () => Site.scope({ - method: ['forUser', user], + method: ['withOrgUsers'], }); Site.domainFromContext = (context) => (context === 'site' ? 'domain' : 'demoDomain'); Site.branchFromContext = (context) => context === 'site' ? 'defaultBranch' : 'demoBranch'; + Site.prototype.getOrgUsers = getOrgUsers; return Site; }; diff --git a/api/models/user.js b/api/models/user.js index f9a3c5365..050abf6d3 100644 --- a/api/models/user.js +++ b/api/models/user.js @@ -6,8 +6,6 @@ const associate = ({ Organization, OrganizationRole, Role, - Site, - SiteUser, UAAIdentity, User, UserAction, @@ -16,11 +14,6 @@ const associate = ({ User.hasMany(Build, { foreignKey: 'user', }); - User.belongsToMany(Site, { - through: SiteUser, - foreignKey: 'user_sites', - timestamps: false, - }); User.hasMany(UserAction, { foreignKey: 'userId', as: 'userActions', @@ -82,14 +75,6 @@ const associate = ({ }, ], })); - User.addScope('bySite', (id) => ({ - include: [ - { - model: Site, - where: { id }, - }, - ], - })); User.addScope('withUAAIdentity', { include: UAAIdentity, }); @@ -198,9 +183,6 @@ module.exports = (sequelize, DataTypes) => { User.orgScope = (id) => ({ method: ['byOrg', id], }); - User.siteScope = (id) => ({ - method: ['bySite', id], - }); User.searchScope = (search) => ({ method: ['byIdOrText', search], }); diff --git a/api/routers/site.js b/api/routers/site.js index d7071ddad..e5b71d864 100644 --- a/api/routers/site.js +++ b/api/routers/site.js @@ -7,15 +7,11 @@ const { csrfProtection, sessionAuth } = require('../middlewares'); router.use(csrfProtection); router.get('/site', sessionAuth, SiteController.findAllForUser); -router.post('/site/user', sessionAuth, SiteController.addUser); -router.delete('/site/:site_id/user/:user_id', sessionAuth, SiteController.removeUser); router.post('/site', sessionAuth, SiteController.create); router.get('/site/:id', sessionAuth, SiteController.findById); router.put('/site/:id', sessionAuth, SiteController.update); router.delete('/site/:id', sessionAuth, SiteController.destroy); router.get('/site/:site_id/domains', sessionAuth, SiteController.getSiteDomains); -router.post('/site/:site_id/basic-auth', sessionAuth, SiteController.addBasicAuth); -router.delete('/site/:site_id/basic-auth', sessionAuth, SiteController.removeBasicAuth); router.get('/site/:site_id/task', sessionAuth, SiteController.getSiteBuildTasks); router.put( '/site/:site_id/task/:task_id', diff --git a/api/serializers/site.js b/api/serializers/site.js index 40750d79e..53fda4863 100644 --- a/api/serializers/site.js +++ b/api/serializers/site.js @@ -1,7 +1,6 @@ const yaml = require('js-yaml'); const { omitBy, pick } = require('../utils'); -const { Organization, Site, User } = require('../models'); -const userSerializer = require('./user'); +const { Organization, Site } = require('../models'); const { siteViewLink, siteViewDomain, hideBasicAuthPassword } = require('../utils/site'); const DomainService = require('../services/Domain'); @@ -120,11 +119,6 @@ const serializeObject = (site, isSystemAdmin) => { delete json.SiteBranchConfigs; } - if (json.Users) { - json.users = site.Users.map((u) => userSerializer.toJSON(u)); - delete json.Users; - } - if (json.Organization) { json.organizationId = site.Organization.id; delete json.Organization; @@ -152,7 +146,7 @@ function serializeMany(sites, isSystemAdmin) { } const serialize = (serializable) => { - const include = [User.scope('withGithub'), Organization]; + const include = [Organization]; if (serializable.length !== undefined) { const siteIds = serializable.map((site) => site.id); diff --git a/api/services/GithubBuildHelper.js b/api/services/GithubBuildHelper.js index 98b2b96a0..ce8b95b2b 100644 --- a/api/services/GithubBuildHelper.js +++ b/api/services/GithubBuildHelper.js @@ -4,9 +4,9 @@ const config = require('../../config'); // Loops through supplied list of users, until it // finds a user with a valid access token -const getAccessTokenWithCertainPermissions = async (site, siteUsers, permission) => { +const getAccessTokenWithCertainPermissions = async (site, users, permission) => { let count = 0; - const users = siteUsers + const filteredUsers = users .filter((u) => u.githubAccessToken && u.signedInAt) .sort((a, b) => b.signedInAt - a.signedInAt); @@ -26,36 +26,36 @@ const getAccessTokenWithCertainPermissions = async (site, siteUsers, permission) return user.githubAccessToken; } count += 1; - return getNextToken(users[count]); + return getNextToken(filteredUsers[count]); } catch { count += 1; - return getNextToken(users[count]); + return getNextToken(filteredUsers[count]); } }; - return getNextToken(users[count]); + return getNextToken(filteredUsers[count]); }; -const getAccessTokenWithPushPermissions = async (site, siteUsers) => - getAccessTokenWithCertainPermissions(site, siteUsers, 'push'); +const getAccessTokenWithPushPermissions = async (site, users) => + getAccessTokenWithCertainPermissions(site, users, 'push'); -const getAccessTokenWithAdminPermissions = async (site, siteUsers) => - getAccessTokenWithCertainPermissions(site, siteUsers, 'admin'); +const getAccessTokenWithAdminPermissions = async (site, users) => + getAccessTokenWithCertainPermissions(site, users, 'admin'); -const createSiteWebhook = async (site, siteUsers) => { - const githubAccessToken = await getAccessTokenWithAdminPermissions(site, siteUsers); +const createSiteWebhook = async (site, users) => { + const githubAccessToken = await getAccessTokenWithAdminPermissions(site, users); return GitHub.setWebhook(site, githubAccessToken); }; -const listSiteWebhooks = async (site, siteUsers) => { - const githubAccessToken = await getAccessTokenWithAdminPermissions(site, siteUsers); +const listSiteWebhooks = async (site, users) => { + const githubAccessToken = await getAccessTokenWithAdminPermissions(site, users); return GitHub.listSiteWebhooks(site, githubAccessToken); }; const loadBuildUserAccessToken = async (build) => { let githubAccessToken; const site = build.Site; - const users = site.Users; + const users = await build.getSiteOrgUsers(); const buildUser = users.find((u) => u.id === build.user); if (buildUser) { githubAccessToken = await getAccessTokenWithPushPermissions(site, [buildUser]); diff --git a/api/services/RepositoryVerifier.js b/api/services/RepositoryVerifier.js index 8d1c49967..83e265bb7 100644 --- a/api/services/RepositoryVerifier.js +++ b/api/services/RepositoryVerifier.js @@ -1,10 +1,11 @@ const GitHub = require('./GitHub'); -const { User, Site } = require('../models'); +const { Site } = require('../models'); const { logger } = require('../../winston'); -const verifyNextRepo = (site, userIndex = 0) => { +const verifyNextRepo = async (site, userIndex = 0) => { let found; - return GitHub.getRepository(site.Users[userIndex], site.owner, site.repository) + const users = await site.getOrgUsers(); + return GitHub.getRepository(users[userIndex], site.owner, site.repository) .then((repo) => { found = repo; if (found) { @@ -17,42 +18,19 @@ const verifyNextRepo = (site, userIndex = 0) => { }) .catch(logger.warn) .then(() => { - if (!found && site.Users[userIndex + 1]) { + if (!found && users[userIndex + 1]) { return verifyNextRepo(site, userIndex + 1); } return Promise.resolve(); }); }; -const verifyRepos = () => - Site.findAll({ - include: [User.scope('withGithub')], - order: [[User, 'signedInAt', 'DESC']], - }).then((sites) => Promise.allSettled(sites.map((site) => verifyNextRepo(site)))); - -const verifyUserRepos = async (user) => { - const repoLastVerified = new Date(); - - const [repos, sites] = await Promise.all([ - GitHub.getRepositories(user.githubAccessToken), - user.getSites(), - ]); - - const verified = sites - .filter((site) => { - const fullName = [site.owner, site.repository].join('/').toUpperCase(); - return repos.find((repo) => repo.full_name.toUpperCase() === fullName); - }) - .map((site) => - site.update({ - repoLastVerified, - }), - ); - - return Promise.all(verified); +const verifyRepos = async () => { + return Site.findAll().then((sites) => + Promise.allSettled(sites.map((site) => verifyNextRepo(site))), + ); }; module.exports = { verifyRepos, - verifyUserRepos, }; diff --git a/api/services/SiteBuildQueue.js b/api/services/SiteBuildQueue.js index 85534d5c1..04f8b4e30 100644 --- a/api/services/SiteBuildQueue.js +++ b/api/services/SiteBuildQueue.js @@ -127,7 +127,7 @@ SiteBuildQueue.setupTaskEnv = async (buildId) => { { model: Site, required: true, - include: [UserEnvironmentVariable, User, SiteBranchConfig, Domain], + include: [UserEnvironmentVariable, SiteBranchConfig, Domain], }, ], }); diff --git a/api/services/SiteCreator.js b/api/services/SiteCreator.js index 480744426..1408995fb 100644 --- a/api/services/SiteCreator.js +++ b/api/services/SiteCreator.js @@ -178,11 +178,7 @@ async function saveAndBuildSite({ site, user }) { user, }); - await Promise.all([ - site.addUser(user.id), - Build.create(buildParams).then((build) => build.enqueue()), - ]); - + await Build.create(buildParams).then((build) => build.enqueue()); return site; } diff --git a/api/services/SiteMembershipCreator.js b/api/services/SiteMembershipCreator.js deleted file mode 100644 index 9bfd5fbe4..000000000 --- a/api/services/SiteMembershipCreator.js +++ /dev/null @@ -1,97 +0,0 @@ -const GitHub = require('./GitHub'); -const { Site, User } = require('../models'); -const siteErrors = require('../responses/siteErrors'); -const config = require('../../config'); - -const checkGithubRepository = ({ user, owner, repository }) => - GitHub.getRepository(user, owner, repository).then((repo) => { - if (!repo) { - throw { - message: `The repository ${owner}/${repository} does not exist.`, - status: 400, - }; - } - if (!repo.permissions || !repo.permissions.push) { - throw { - message: siteErrors.WRITE_ACCESS_REQUIRED, - status: 400, - }; - } - return true; - }); - -const paramsForExistingSite = (siteParams) => ({ - owner: siteParams.owner ? siteParams.owner.toLowerCase() : null, - repository: siteParams.repository ? siteParams.repository.toLowerCase() : null, -}); - -const throwExistingSiteErrors = ({ site, user }) => { - if (!site) { - const error = new Error('The site you are trying to add does not exist'); - error.status = 404; - throw error; - } - - const existingUser = site.Users.find((candidate) => candidate.id === user.id); - if (existingUser) { - const error = new Error(`You've already added this site to ${config.app.appName}`); - error.status = 400; - throw error; - } - - return checkGithubRepository({ - user, - owner: site.owner, - repository: site.repository, - }); -}; - -const createSiteMembership = ({ user, siteParams }) => { - let site; - - return Site.findOne({ - where: paramsForExistingSite(siteParams), - include: [User], - }) - .then((fetchedSite) => { - site = fetchedSite; - return throwExistingSiteErrors({ site, user }); - }) - .then(() => site.addUser(user)) - .then(() => site); -}; - -const revokeSiteMembership = ({ user, site, userId }) => - GitHub.checkPermissions(user, site.owner, site.repository) - .then((permissions) => { - if (user.id !== Number(userId) && !permissions.admin) { - throw { - message: siteErrors.ADMIN_ACCESS_REQUIRED, - status: 400, - }; - } - }) - .then(() => { - const userToRemove = site.Users.find((u) => u.id === Number(userId)); - - if (!userToRemove) { - throw { - message: siteErrors.NO_ASSOCIATED_USER, - status: 404, - }; - } - - if (userToRemove.username.toLowerCase() === site.owner.toLowerCase()) { - throw { - message: siteErrors.OWNER_REMOVE, - status: 400, - }; - } - - return site.removeUser(userToRemove); - }); - -module.exports = { - createSiteMembership, - revokeSiteMembership, -}; diff --git a/api/services/Webhooks.js b/api/services/Webhooks.js index 8e424644c..dcaf1e02d 100644 --- a/api/services/Webhooks.js +++ b/api/services/Webhooks.js @@ -11,7 +11,7 @@ const findSiteForWebhookRequest = (payload) => { owner: owner.toLowerCase(), repository: repository.toLowerCase(), }, - include: [User, Organization], + include: [Organization], }); }; @@ -60,15 +60,11 @@ const createBuildForWebhookRequest = async (payload, site) => { const { pushed_at: pushedAt } = payload.repository; const username = login.toLowerCase(); - let user = site.Users.find((u) => u.username === username); - if (!user) { - user = await User.findOne({ - where: { username }, - }); - if (user) { - await site.addUser(user); - } - } + // it's okay if we don't find a valid User here since we'll + // still search for a token in loadBuildUserAccessToken + const user = await User.findOne({ + where: { username }, + }); if (user) { await user.update({ pushedAt: new Date(pushedAt * 1000), @@ -108,18 +104,7 @@ const pushWebhookRequest = async (payload) => { const site = await findSiteForWebhookRequest(payload); if (shouldBuildForSite(site)) { const build = await createBuildForWebhookRequest(payload, site); - await build.reload({ - include: [ - { - model: Site, - include: [ - { - model: User, - }, - ], - }, - ], - }); + await build.reload({ include: Site }); await GithubBuildHelper.reportBuildStatus(build); } } diff --git a/frontend/actions/actionCreators/siteActions.js b/frontend/actions/actionCreators/siteActions.js index a8d0692c9..059657ee2 100644 --- a/frontend/actions/actionCreators/siteActions.js +++ b/frontend/actions/actionCreators/siteActions.js @@ -3,8 +3,6 @@ const sitesReceivedType = 'SITES_RECEIVED'; const siteAddedType = 'SITE_ADDED'; const siteUpdatedType = 'SITE_UPDATED'; const siteDeletedType = 'SITE_DELETED'; -const siteUserAddedType = 'SITE_USER_ADDED'; -const siteUserRemovedType = 'SITE_USER_REMOVED'; const siteBranchesReceivedType = 'SITE_BRANCHES_RECIEVED'; const siteBasicAuthSavedType = 'SITE_BASIC_AUTH_SAVED'; const siteBasicAuthRemovedType = 'SITE_BASIC_AUTH_REMOVED'; @@ -34,16 +32,6 @@ const siteDeleted = (siteId) => ({ siteId, }); -const siteUserAdded = (site) => ({ - type: siteUserAddedType, - site, -}); - -const siteUserRemoved = (site) => ({ - type: siteUserRemovedType, - site, -}); - const siteBasicAuthSaved = (site) => ({ type: siteBasicAuthSavedType, siteId: site.id, @@ -67,10 +55,6 @@ export { siteUpdatedType, siteDeleted, siteDeletedType, - siteUserAdded, - siteUserAddedType, - siteUserRemoved, - siteUserRemovedType, siteBranchesReceivedType, siteBasicAuthSaved, siteBasicAuthSavedType, diff --git a/frontend/actions/dispatchActions.js b/frontend/actions/dispatchActions.js index 6c890cc74..b818ffde9 100644 --- a/frontend/actions/dispatchActions.js +++ b/frontend/actions/dispatchActions.js @@ -6,8 +6,6 @@ import { siteAdded as createSiteAddedAction, siteUpdated as createSiteUpdatedAction, siteDeleted as createSiteDeletedAction, - siteUserAdded as createSiteUserAddedAction, - siteUserRemoved as siteUserRemovedAction, siteBasicAuthSaved as createSiteBasicAuthSavedAction, siteBasicAuthRemoved as createSiteBasicAuthRemovedAction, } from './actionCreators/siteActions'; @@ -36,14 +34,6 @@ const dispatchSiteDeletedAction = (siteId) => { dispatch(createSiteDeletedAction(siteId)); }; -const dispatchUserAddedToSiteAction = (site) => { - dispatch(createSiteUserAddedAction(site)); -}; - -const dispatchUserRemovedFromSiteAction = (site) => { - dispatch(siteUserRemovedAction(site)); -}; - const dispatchShowAddNewSiteFieldsAction = () => { dispatch(createShowAddNewSiteFieldsAction()); }; @@ -66,8 +56,6 @@ export { dispatchSiteAddedAction, dispatchSiteUpdatedAction, dispatchSiteDeletedAction, - dispatchUserAddedToSiteAction, - dispatchUserRemovedFromSiteAction, dispatchShowAddNewSiteFieldsAction, dispatchHideAddNewSiteFieldsAction, dispatchResetFormAction, diff --git a/frontend/actions/siteActions.js b/frontend/actions/siteActions.js index cd78159ef..1044125b5 100644 --- a/frontend/actions/siteActions.js +++ b/frontend/actions/siteActions.js @@ -7,17 +7,12 @@ import { dispatchSiteAddedAction, dispatchSiteUpdatedAction, dispatchSiteDeletedAction, - dispatchUserAddedToSiteAction, - dispatchUserRemovedFromSiteAction, - dispatchShowAddNewSiteFieldsAction, dispatchHideAddNewSiteFieldsAction, dispatchResetFormAction, dispatchSiteBasicAuthRemovedAction, dispatchSiteBasicAuthSavedAction, } from './dispatchActions'; -import userActions from './userActions'; - const alertError = (error) => { window.scrollTo(0, 0); alertActions.httpError(error.message); @@ -52,36 +47,6 @@ export default { }); }, - addUserToSite({ owner, repository }, navigate) { - return federalist - .addUserToSite({ - owner, - repository, - }) - .then(dispatchUserAddedToSiteAction) - .then(navigate) - .catch((err) => { - // Getting a 404 here signals that the site does not - // yet exist in Pages, so we want to show the - // additional Add New Site fields - if (err.response && err.response.status === 404) { - dispatchShowAddNewSiteFieldsAction(); - } else { - resetFormOnError(err); - } - }); - }, - - removeUserFromSite(siteId, userId) { - return federalist - .removeUserFromSite(siteId, userId) - .then(dispatchUserRemovedFromSiteAction) - .then(this.fetchSites) - .then(userActions.fetchUser) - .then(() => alertActions.alertSuccess('Successfully removed.')) - .catch(alertError); - }, - updateSite(site, data) { return federalist .updateSite(site, data) diff --git a/frontend/pages/sites/new/index.jsx b/frontend/pages/sites/new/index.jsx index c4f3e89ef..6e63d08c9 100644 --- a/frontend/pages/sites/new/index.jsx +++ b/frontend/pages/sites/new/index.jsx @@ -44,11 +44,6 @@ function AddSite() { const navigate = useNavigate(); - function onAddUserSubmit({ repoUrl }) { - const { owner, repository } = getOwnerAndRepo(repoUrl); - siteActions.addUserToSite({ owner, repository }, () => navigate('/sites')); - } - function onCreateSiteSubmit({ repoUrl, engine, repoOrganizationId }) { const { owner, repository } = getOwnerAndRepo(repoUrl); siteActions.addSite( @@ -62,10 +57,6 @@ function AddSite() { ); } - // select the function to use on form submit based on - // the showAddNewSiteFields flag - const formSubmitFunc = showAddNewSiteFields ? onCreateSiteSubmit : onAddUserSubmit; - if (isLoading) { return ; } @@ -111,7 +102,7 @@ function AddSite() { }} organizations={organizations} showAddNewSiteFields={showAddNewSiteFields} - onSubmit={formSubmitFunc} + onSubmit={onCreateSiteSubmit} />
diff --git a/frontend/reducers/sites.js b/frontend/reducers/sites.js index e4de974c1..2d88ca3c5 100644 --- a/frontend/reducers/sites.js +++ b/frontend/reducers/sites.js @@ -5,8 +5,6 @@ import { siteUpdatedType as SITE_UPDATED, siteDeletedType as SITE_DELETED, siteBranchesReceivedType as SITE_BRANCHES_RECEIVED, - siteUserAddedType as SITE_USER_ADDED, - siteUserRemovedType as SITE_USER_REMOVED, siteBasicAuthSavedType as SITE_BASIC_AUTH_SAVED, siteBasicAuthRemovedType as SITE_BASIC_AUTH_REMOVED, } from '../actions/actionCreators/siteActions'; @@ -79,21 +77,6 @@ export default function sites(state = initialState, action) { data: state.data.filter((site) => site.id !== action.siteId), }; - case SITE_USER_ADDED: - return action.site - ? { - isLoading: false, - data: state.data.concat(action.site), - } - : state; - - case SITE_USER_REMOVED: { - return { - isLoading: false, - data: [action.site, ...state.data.filter((site) => site.id !== action.site.id)], - }; - } - default: return state; } diff --git a/frontend/selectors/organization/index.js b/frontend/selectors/organization/index.js index 1a8d9c023..6c9724693 100644 --- a/frontend/selectors/organization/index.js +++ b/frontend/selectors/organization/index.js @@ -28,10 +28,6 @@ export const orgFilterOptions = (state) => { id: 'all-options', name: 'All', }); - keyValues.push({ - id: 'unassociated', - name: 'Sites without an organization', - }); return keyValues; }; diff --git a/frontend/selectors/site/index.js b/frontend/selectors/site/index.js index 59441088e..e0d7be774 100644 --- a/frontend/selectors/site/index.js +++ b/frontend/selectors/site/index.js @@ -3,15 +3,6 @@ export const currentSite = (state, id) => export const groupSitesByOrg = (state, orgId) => { if (orgId === 'all-options') return state; - if (orgId === 'unassociated') { - const data = state.data.filter((site) => !site.organizationId); - - return { - ...state, - data, - }; - } - const data = state.data.filter((site) => site.organizationId === Number(orgId)); return { diff --git a/frontend/util/federalistApi.js b/frontend/util/federalistApi.js index 07bade4fe..b3a13c40d 100644 --- a/frontend/util/federalistApi.js +++ b/frontend/util/federalistApi.js @@ -153,34 +153,6 @@ export default { return request(`site/${siteId}/user-action`); }, - addUserToSite({ owner, repository }) { - return request( - 'site/user', - { - method: 'POST', - data: { - owner, - repository, - }, - }, - { - // we want to handle the error elsewhere in order - // to show the additional AddSite fields - handleHttpError: false, - }, - ); - }, - - removeUserFromSite(siteId, userId) { - return request( - `site/${siteId}/user/${userId}`, - { method: 'DELETE' }, - { - handleHttpError: false, - }, - ); - }, - addSite(site) { return request( 'site', diff --git a/migrations/20241204200612-drop-site-users.js b/migrations/20241204200612-drop-site-users.js new file mode 100644 index 000000000..1b46c2836 --- /dev/null +++ b/migrations/20241204200612-drop-site-users.js @@ -0,0 +1,11 @@ +'use strict'; + +exports.up = async (db) => { + await db.runSql('DROP INDEX IF EXISTS "site_users__user_sites_user_sites_idx";'); + await db.runSql('DROP INDEX IF EXISTS "site_users__user_sites_site_users_idx";'); + return db.dropTable('site_users__user_sites'); +}; + +exports.down = async (db) => { + // TODO: down migration +}; diff --git a/scripts/exportSitesAsCsv.js b/scripts/exportSitesAsCsv.js index 1d5ec28e0..bd418ff02 100644 --- a/scripts/exportSitesAsCsv.js +++ b/scripts/exportSitesAsCsv.js @@ -39,6 +39,7 @@ function consolidateOnSiteId(sites) { return ids.map((id, i) => { const sitesById = _.where(sites, { id }); + // TODO: if using this again, need to replace site.users const users = sitesById.map((site) => site.users[0]); return { diff --git a/test/api/admin/requests/user.test.js b/test/api/admin/requests/user.test.js index b4f006c23..4cebfcdfc 100644 --- a/test/api/admin/requests/user.test.js +++ b/test/api/admin/requests/user.test.js @@ -119,17 +119,17 @@ describe('Admin - Users API', () => { const org1 = await factory.organization.create(); const org2 = await factory.organization.create(); - org1.addUser(user1, { + await org1.addUser(user1, { through: { roleId: managerRole.id, }, }); - org1.addUser(user2, { + await org1.addUser(user2, { through: { roleId: userRole.id, }, }); - org2.addUser(user1, { + await org2.addUser(user1, { through: { roleId: userRole.id, }, diff --git a/test/api/requests/build-logs.test.js b/test/api/requests/build-logs.test.js index 89482c0f8..585b80c50 100644 --- a/test/api/requests/build-logs.test.js +++ b/test/api/requests/build-logs.test.js @@ -4,13 +4,8 @@ const app = require('../../../app'); const factory = require('../support/factory'); const { authenticatedSession } = require('../support/session'); const validateAgainstJSONSchema = require('../support/validateAgainstJSONSchema'); -const { - BuildLog, - Organization, - OrganizationRole, - User, - Role, -} = require('../../../api/models'); +const { createSiteUserOrg } = require('../support/site-user'); +const { BuildLog, Organization, OrganizationRole, User } = require('../../../api/models'); function clean() { return Promise.all([ @@ -30,22 +25,8 @@ function clean() { } describe('Build Log API', () => { - let userRole; - before(async () => { await clean(); - [userRole] = await Promise.all([ - Role.findOne({ - where: { - name: 'user', - }, - }), - Role.findOne({ - where: { - name: 'manager', - }, - }), - ]); }); afterEach(clean); @@ -65,50 +46,32 @@ describe('Build Log API', () => { }); describe('successfully fetching build logs', () => { - const prepareAndFetchLogData = () => { - const userPromise = factory.user(); - const sitePromise = factory.site({ - users: Promise.all([userPromise]), - }); - const buildPromise = factory.build({ - user: userPromise, - site: sitePromise, - }); + it('should response with builds logs for the given build', async () => { + const { site, user } = await createSiteUserOrg(); + const cookie = await authenticatedSession(user); + const build = await factory.build({ user, site }); + + const logs = await BuildLog.bulkCreate( + Array(2000) + .fill(0) + .map(() => ({ + output: + // eslint-disable-next-line max-len + 'Lorem ipsum dolor sit amet, consectetur adipiscing elit. Nullam fringilla, arcu ut ultricies auctor, elit quam consequat neque, eu blandit metus lorem non turpis.', + source: 'ALL', + build: build.id, + })), + ); + + const buildId = logs[0].get({ + plain: true, + }).build; + + const response = await request(app) + .get(`/v0/build/${buildId}/log`) + .set('Cookie', cookie) + .expect(200); - return Promise.props({ - user: userPromise, - site: sitePromise, - build: buildPromise, - }) - .then(({ build, user }) => - Promise.all([ - BuildLog.bulkCreate( - Array(2000) - .fill(0) - .map(() => ({ - output: - // eslint-disable-next-line max-len - 'Lorem ipsum dolor sit amet, consectetur adipiscing elit. Nullam fringilla, arcu ut ultricies auctor, elit quam consequat neque, eu blandit metus lorem non turpis.', - source: 'ALL', - build: build.id, - })), - ), - authenticatedSession(user), - ]), - ) - .then(([logs, cookie]) => { - const buildId = logs[0].get({ - plain: true, - }).build; - - return request(app) - .get(`/v0/build/${buildId}/log`) - .set('Cookie', cookie) - .expect(200); - }); - }; - - const expectedResponse = (response, done) => { validateAgainstJSONSchema('GET', '/build/{build_id}/log', 200, response.body); expect(response.body).to.be.an('object'); expect(response.body).to.have.keys([ @@ -124,74 +87,11 @@ describe('Build Log API', () => { expect(response.body.offset).to.equal(0); expect(response.body.output_count).to.equal('1000'); expect(response.body.output).to.have.length(1000); - done(); - }; - - it('should render builds logs for the given build', (done) => { - prepareAndFetchLogData() - .then((response) => expectedResponse(response, done)) - .catch(done); - }); - - it('should render logs if user is not associated to the build', (done) => { - prepareAndFetchLogData() - .then((response) => expectedResponse(response, done)) - .catch(done); - }); - }); - - describe('successfully handling organization build logs', () => { - it('should accept organization user build log requests', async () => { - const user = await factory.user(); - const orgUser = await factory.user(); - const org = await factory.organization.create(); - const site = await factory.site({ - users: [user], - organizationId: org.id, - }); - const build = await factory.build({ - user, - site: site.id, - }); - - await org.addUser(orgUser, { - through: { - roleId: userRole.id, - }, - }); - - const logs = await BuildLog.bulkCreate( - Array(20) - .fill(0) - .map(() => ({ - output: - // eslint-disable-next-line max-len - 'Lorem ipsum dolor sit amet, consectetur adipiscing elit. Nullam fringilla, arcu ut ultricies auctor, elit quam consequat neque, eu blandit metus lorem non turpis.', - source: 'ALL', - build: build.id, - })), - ); - - await authenticatedSession(orgUser).then((cookie) => { - const buildId = logs[0].get({ - plain: true, - }).build; - - return request(app) - .get(`/v0/build/${buildId}/log`) - .set('Cookie', cookie) - .expect(200); - }); }); it('should reject non-organization user build log requests', async () => { - const user = await factory.user(); + const { site, user } = await createSiteUserOrg(); const nonOrgUser = await factory.user(); - const org = await factory.organization.create(); - const site = await factory.site({ - users: [user], - organizationId: org.id, - }); const build = await factory.build({ user, site: site.id, @@ -278,10 +178,7 @@ describe('Build Log API', () => { let cookie; beforeEach(async () => { - const user = await factory.user(); - const site = await factory.site({ - users: [user], - }); + const { site, user } = await createSiteUserOrg(); cookie = await authenticatedSession(user); build = await factory.build({ user, site }); }); diff --git a/test/api/requests/build-tasks.test.js b/test/api/requests/build-tasks.test.js index 013d3f6e1..05bd9cb10 100644 --- a/test/api/requests/build-tasks.test.js +++ b/test/api/requests/build-tasks.test.js @@ -6,6 +6,7 @@ const factory = require('../support/factory'); const { authenticatedSession } = require('../support/session'); const validateAgainstJSONSchema = require('../support/validateAgainstJSONSchema'); const { BuildTask, BuildTaskType, Build, User } = require('../../../api/models'); +const { createSiteUserOrg } = require('../support/site-user'); function clean() { return Promise.all([ @@ -29,11 +30,8 @@ function clean() { } async function prepTasks() { - const user = await factory.user(); + const { site, user } = await createSiteUserOrg(); const cookie = await authenticatedSession(user); - const site = await factory.site({ - users: [user], - }); const build = await factory.build({ user, site, @@ -195,11 +193,8 @@ describe('Build Task API', () => { describe('POST /v0/build/:build_id/task', () => { it('should create build tasks for a build', async () => { - const user = await factory.user(); + const { site, user } = await createSiteUserOrg(); const cookie = await authenticatedSession(user); - const site = await factory.site({ - users: [user], - }); const buildTaskType = await factory.buildTaskType(); const build = await factory.build({ user, diff --git a/test/api/requests/build.test.js b/test/api/requests/build.test.js index 9d9914b75..a62b96b66 100644 --- a/test/api/requests/build.test.js +++ b/test/api/requests/build.test.js @@ -12,6 +12,7 @@ const { authenticatedSession, unauthenticatedSession } = require('../support/ses const validateAgainstJSONSchema = require('../support/validateAgainstJSONSchema'); const { Build } = require('../../../api/models'); const csrfToken = require('../support/csrfToken'); +const { createSiteUserOrg } = require('../support/site-user'); const requestedCommitSha = 'a172b66c31e19d456a448041a5b3c2a70c32d8b7'; const clonedCommitSha = '7b8d23c07a2c3b5a140844a654d91e13c66b271a'; @@ -87,10 +88,7 @@ describe('Build API', () => { it(`returns a 404 if a build to restart is not associated with the site`, async () => { - const user = await factory.user(); - const site = await factory.site({ - users: [user], - }); + const { site, user } = await createSiteUserOrg(); const build = await factory.build({ user }); const cookie = await authenticatedSession(user); const response = request(app) @@ -135,10 +133,7 @@ describe('Build API', () => { beforeEach(async () => { nock.cleanAll(); - user = await factory.user(); - site = await factory.site({ - users: [user], - }); + ({ site, user } = await createSiteUserOrg()); reqBuild = await factory.build({ site, state: 'success', @@ -238,162 +233,103 @@ describe('Build API', () => { .catch(done); }); - it('should list builds for a site associated with the current user', (done) => { - let site; - let builds; - - const userPromise = factory.user(); - const sitePromise = factory.site({ - users: Promise.all([userPromise]), - }); - const buildsPromise = Promise.all([ + it('should list builds for a site associated with the current user', async () => { + const { site, user } = await createSiteUserOrg(); + const cookie = await authenticatedSession(user); + const builds = await Promise.all([ factory.build({ - site: sitePromise, + site, }), factory.build({ - site: sitePromise, - user: userPromise, + site, + user, }), factory.build({ - site: sitePromise, - user: userPromise, + site, + user, state: 'error', error: 'The build timed out', }), ]); - Promise.props({ - site: sitePromise, - builds: buildsPromise, - cookie: authenticatedSession(userPromise), - }) - .then((promisedValues) => { - ({ site, builds } = promisedValues); - const cookie = promisedValues.cookie; + const response = await request(app) + .get(`/v0/site/${site.id}/build`) + .set('Cookie', cookie) + .expect(200); - return request(app) - .get(`/v0/site/${site.id}/build`) - .set('Cookie', cookie) - .expect(200); - }) - .then((response) => { - expect(response.body).to.be.a('Array'); - expect(response.body).to.have.length(3); - - builds.forEach((build) => { - const responseBuild = response.body.find( - (candidate) => candidate.id === build.id, - ); - buildResponseExpectations(responseBuild, build); - }); + expect(response.body).to.be.a('Array'); + expect(response.body).to.have.length(3); - validateAgainstJSONSchema('GET', '/site/{site_id}/build', 200, response.body); - done(); - }) - .catch(done); + builds.forEach((build) => { + const responseBuild = response.body.find( + (candidate) => candidate.id === build.id, + ); + buildResponseExpectations(responseBuild, build); + }); + + validateAgainstJSONSchema('GET', '/site/{site_id}/build', 200, response.body); }); it(`should not list builds for a site - not associated with the current user`, (done) => { - let site; + not associated with the current user`, async () => { + const { site, user } = await createSiteUserOrg(); + const cookie = await authenticatedSession(user); - const userPromise = factory.user(); - const sitePromise = factory.site(); - const buildsPromise = Promise.all([ + await Promise.all([ factory.build({ - site: sitePromise, + site, }), factory.build({ - site: sitePromise, - user: userPromise, + site, + user, }), ]); - Promise.props({ - user: userPromise, - site: sitePromise, - builds: buildsPromise, - cookie: authenticatedSession(userPromise), - }) - .then((promisedValues) => { - site = promisedValues.site; - const cookie = promisedValues.cookie; + const response = request(app) + .get(`/v0/site/${site.id}/build`) + .set('Cookie', cookie) + .expect(403); - return request(app) - .get(`/v0/site/${site.id}/build`) - .set('Cookie', cookie) - .expect(403); - }) - .then((response) => { - validateAgainstJSONSchema('GET', '/site/{site_id}/build', 403, response.body); - done(); - }) - .catch(done); + validateAgainstJSONSchema('GET', '/site/{site_id}/build', 403, response.body); }); - it("shouldn't list more than 100 builds", (done) => { - const userPromise = factory.user(); - const sitePromise = factory.site({ - users: Promise.all([userPromise]), - }); - const cookiePromise = authenticatedSession(userPromise); + it("shouldn't list more than 100 builds", async () => { + const { site, user } = await createSiteUserOrg(); + const cookie = await authenticatedSession(user); - Promise.props({ - site: sitePromise, - cookie: cookiePromise, - user: userPromise, - }) - .then((props) => - factory - .bulkBuild( - { - site: props.site.id, - user: props.user.id, - username: props.user.username, - }, - 110, - ) - .then(() => props), - ) - .then(({ site, cookie }) => - request(app).get(`/v0/site/${site.id}/build`).set('Cookie', cookie).expect(200), - ) - .then((response) => { - expect(response.body).to.be.an('array'); - expect(response.body).to.have.length(100); - done(); - }) - .catch(done); + await factory.bulkBuild( + { + site: site.id, + user: user.id, + username: user.username, + }, + 110, + ); + const response = await request(app) + .get(`/v0/site/${site.id}/build`) + .set('Cookie', cookie) + .expect(200); + + expect(response.body).to.be.an('array'); + expect(response.body).to.have.length(100); }); - it('should not display unfound build', (done) => { - const userPromise = factory.user(); - const sitePromise = factory.site(); - const buildsPromise = Promise.all([ + it('should not display unfound build', async () => { + const { site, user } = await createSiteUserOrg(); + const cookie = await authenticatedSession(user); + await Promise.all([ factory.build({ - site: sitePromise, - user: userPromise, + site, + user, }), ]); - Promise.props({ - user: userPromise, - site: sitePromise, - builds: buildsPromise, - cookie: authenticatedSession(userPromise), - }) - .then((promisedValues) => { - const cookie = promisedValues.cookie; - return request(app) - .get('/v0/site/-1000/build') - .set('Cookie', cookie) - .expect(404); - }) - .then((response) => { - validateAgainstJSONSchema('GET', '/site/{site_id}/build', 404, response.body); - done(); - }) - .catch(done); + const response = await request(app) + .get('/v0/site/-1000/build') + .set('Cookie', cookie) + .expect(404); + + validateAgainstJSONSchema('GET', '/site/{site_id}/build', 404, response.body); }); it('should not display build when site id is NaN', (done) => { @@ -428,42 +364,26 @@ describe('Build API', () => { .catch(done); }); - it('should return a single build matching the given build id', (done) => { - let build; + it('should return a single build matching the given build id', async () => { + const { site, user } = await createSiteUserOrg(); + const cookie = await authenticatedSession(user); - const userPromise = factory.user(); - const sitePromise = factory.site({ - users: Promise.all([userPromise]), - }); - const buildPromise = factory.build({ - site: sitePromise, - user: userPromise, + const build = await factory.build({ + site, + user, state: 'error', error: 'The build timed out', }); - Promise.props({ - site: sitePromise, - build: buildPromise, - cookie: authenticatedSession(userPromise), - }) - .then((promisedValues) => { - ({ build } = promisedValues); - const cookie = promisedValues.cookie; + const response = await request(app) + .get(`/v0/build/${build.id}`) + .set('Cookie', cookie) + .expect(200); - return request(app) - .get(`/v0/build/${build.id}`) - .set('Cookie', cookie) - .expect(200); - }) - .then((response) => { - const responseBuild = response.body; - expect(responseBuild).to.be.a('Object'); - buildResponseExpectations(responseBuild, build); - validateAgainstJSONSchema('GET', '/build/{id}', 200, responseBuild); - done(); - }) - .catch(done); + const responseBuild = response.body; + expect(responseBuild).to.be.a('Object'); + buildResponseExpectations(responseBuild, build); + validateAgainstJSONSchema('GET', '/build/{id}', 200, responseBuild); }); it(`should not return the matching build @@ -567,11 +487,13 @@ describe('Build API', () => { const fetchContentStub = sinon .stub(GithubBuildHelper, 'fetchContent') .resolves('{}'); + + const { site, user } = await createSiteUserOrg(); const build = await factory.build({ requestedCommitSha, + site, + user, }); - const user = await build.getUser(); - const site = await build.getSite(); githubAPINocks.repo({ accessToken: user.githubAccessToken, @@ -597,12 +519,14 @@ describe('Build API', () => { state: 'success', commitSha: clonedCommitSha, }); + + const { site, user } = await createSiteUserOrg(); const build = await factory.build({ requestedCommitSha, clonedCommitSha, + site, + user, }); - const user = await build.getUser(); - const site = await build.getSite(); githubAPINocks.repo({ accessToken: user.githubAccessToken, @@ -628,12 +552,13 @@ describe('Build API', () => { state: 'success', commitSha: clonedCommitSha, }); + const { site, user } = await createSiteUserOrg(); const build = await factory.build({ requestedCommitSha, clonedCommitSha, + site, + user, }); - const user = await build.getUser(); - const site = await build.getSite(); githubAPINocks.repo({ accessToken: user.githubAccessToken, diff --git a/test/api/requests/domain.test.js b/test/api/requests/domain.test.js index 0347810a5..85553e71d 100644 --- a/test/api/requests/domain.test.js +++ b/test/api/requests/domain.test.js @@ -6,8 +6,9 @@ const csrfToken = require('../support/csrfToken'); const { authenticatedSession } = require('../support/session'); const validateAgainstJSONSchema = require('../support/validateAgainstJSONSchema'); const app = require('../../../app'); -const { Domain, Role, Site, SiteBranchConfig } = require('../../../api/models'); +const { Domain, Site, SiteBranchConfig } = require('../../../api/models'); const EventCreator = require('../../../api/services/EventCreator'); +const { createSiteUserOrg } = require('../support/site-user'); function clean() { return Promise.all([ @@ -104,11 +105,7 @@ describe('Domain API', () => { describe('when the domain does not exist', () => { it('returns a 404', async () => { const domainId = 1; - const userPromise = factory.user(); - const site = await factory.site({ - users: Promise.all([userPromise]), - }); - const user = await userPromise; + const { site, user } = await createSiteUserOrg(); const cookie = await authenticatedSession(user); const { body } = await request(app) @@ -129,11 +126,7 @@ describe('Domain API', () => { describe('when the parameters are valid', () => { it('deletes the domain when it is in a pending state', async () => { - const userPromise = factory.user(); - const site = await factory.site({ - users: Promise.all([userPromise]), - }); - const [user] = await Promise.all([userPromise]); + const { site, user } = await createSiteUserOrg(); const domain = await factory.domain.create({ siteId: site.id, siteBranchConfigId: site.SiteBranchConfigs[0].id, @@ -154,59 +147,9 @@ describe('Domain API', () => { expect(afterNumSBC).to.eq(beforeNumSBC - 1); }); - it('allows an org user to delete the domain in a pending state', async () => { - const org = await factory.organization.create(); - const role = await Role.findOne({ - where: { - name: 'user', - }, - }); - const user = await factory.user(); - const site = await factory.site(); - await org.addUser(user, { - through: { - roleId: role.id, - }, - }); - await org.addSite(site); - - const domain = await factory.domain.create({ - siteId: site.id, - siteBranchConfigId: site.SiteBranchConfigs[0].id, - state: 'pending', - }); - - const cookie = await authenticatedSession(user); - - const beforeCount = await Domain.count(); - - await request(app) - .delete(`/v0/site/${site.id}/domain/${domain.id}`) - .set('Cookie', cookie) - .set('x-csrf-token', csrfToken.getToken()) - .type('json') - .expect(200); - - const afterCount = await Domain.count(); - expect(afterCount).to.eq(beforeCount - 1); - }); - it(`does not allow a user to delete the domain when not in a pending state`, async () => { - const org = await factory.organization.create(); - const role = await Role.findOne({ - where: { - name: 'user', - }, - }); - const user = await factory.user(); - const site = await factory.site(); - await org.addUser(user, { - through: { - roleId: role.id, - }, - }); - await org.addSite(site); + const { site, user } = await createSiteUserOrg(); const domain = await factory.domain.create({ siteId: site.id, @@ -296,11 +239,7 @@ describe('Domain API', () => { describe('when the name already exists for a site domain', () => { it('returns a 400', async () => { - const userPromise = factory.user(); - const site = await factory.site({ - users: Promise.all([userPromise]), - }); - const user = await userPromise; + const { site, user } = await createSiteUserOrg(); const branch = 'demo'; const context = 'demo'; const names = 'www.agency.gov'; @@ -339,11 +278,7 @@ describe('Domain API', () => { describe('when the branch config already exists for a site domain', () => { it('returns a 400', async () => { - const userPromise = factory.user(); - const site = await factory.site({ - users: Promise.all([userPromise]), - }); - const user = await userPromise; + const { site, user } = await createSiteUserOrg(); const names1 = 'www.agency.gov'; const names2 = 'demo.agency.gov'; @@ -376,11 +311,7 @@ describe('Domain API', () => { describe('when the branch config does not exist for a site', () => { it('returns a 400', async () => { - const userPromise = factory.user(); - const site = await factory.site({ - users: Promise.all([userPromise]), - }); - const user = await userPromise; + const { site, user } = await createSiteUserOrg(); const names = 'www.agency.gov'; const cookie = await authenticatedSession(user); @@ -406,16 +337,12 @@ describe('Domain API', () => { describe(`when the branch config has a context of \`preview\` for a site domain`, () => { it('returns a 400', async () => { - const userPromise = factory.user(); - const site = await factory.site({ - users: Promise.all([userPromise]), - }); + const { site, user } = await createSiteUserOrg(); const sbc = await factory.siteBranchConfig.create({ site, branch: 'preview', context: 'preview', }); - const user = await userPromise; const names = 'www.agency.gov'; const cookie = await authenticatedSession(user); @@ -440,54 +367,7 @@ describe('Domain API', () => { describe('when the parameters are valid', () => { it('creates and returns the site domain', async () => { - const userPromise = factory.user(); - const site = await factory.site({ - users: Promise.all([userPromise]), - }); - const siteBranchConfigId = site.SiteBranchConfigs[0].id; - const user = await userPromise; - const names = 'www.agency.gov'; - - const cookie = await authenticatedSession(user); - - const beforeCount = await Domain.count(); - - const { body } = await request(app) - .post(`/v0/site/${site.id}/domain`) - .set('Cookie', cookie) - .set('x-csrf-token', csrfToken.getToken()) - .type('json') - .send({ - siteBranchConfigId, - names, - }) - .expect(200); - - const afterCount = await Domain.count(); - - validateAgainstJSONSchema('POST', '/site/{site_id}/domain', 200, body); - expect(body.names).to.equal(names); - expect(body.state).to.equal('pending'); - expect(body.siteId).to.equal(site.id); - expect(body.siteBranchConfigId).to.equal(siteBranchConfigId); - expect(afterCount).to.eq(beforeCount + 1); - }); - - it('allows an org user to create the site domain', async () => { - const org = await factory.organization.create(); - const role = await Role.findOne({ - where: { - name: 'user', - }, - }); - const user = await factory.user(); - const site = await factory.site(); - await org.addUser(user, { - through: { - roleId: role.id, - }, - }); - await org.addSite(site); + const { site, user } = await createSiteUserOrg(); const siteBranchConfigId = site.SiteBranchConfigs[0].id; const names = 'www.agency.gov'; @@ -587,11 +467,7 @@ describe('Domain API', () => { describe('when the name already exists for a site domain', () => { it('returns a 400', async () => { - const userPromise = factory.user(); - const site = await factory.site({ - users: Promise.all([userPromise]), - }); - const user = await userPromise; + const { site, user } = await createSiteUserOrg(); const branch = 'demo'; const context = 'demo'; const names = 'www.agency.gov'; @@ -635,11 +511,7 @@ describe('Domain API', () => { describe('when the branch config already exists for a site domain', () => { it('returns a 400', async () => { - const userPromise = factory.user(); - const site = await factory.site({ - users: Promise.all([userPromise]), - }); - const user = await userPromise; + const { site, user } = await createSiteUserOrg(); const branch = 'demo'; const context = 'demo'; const names = 'www.agency.gov'; @@ -685,11 +557,7 @@ describe('Domain API', () => { describe('when the branch config does not exist for a site', () => { it('returns a 400', async () => { - const userPromise = factory.user(); - const site = await factory.site({ - users: Promise.all([userPromise]), - }); - const user = await userPromise; + const { site, user } = await createSiteUserOrg(); const names = 'www.agency.gov'; const domain = await factory.domain.create({ siteId: site.id, @@ -720,11 +588,7 @@ describe('Domain API', () => { describe(`when the branch config has a context of \`preview\` for a site domain`, () => { it('returns a 400', async () => { - const userPromise = factory.user(); - const site = await factory.site({ - users: Promise.all([userPromise]), - }); - const user = await userPromise; + const { site, user } = await createSiteUserOrg(); const sbc2 = await factory.siteBranchConfig.create({ site, branch: 'preview', @@ -757,11 +621,7 @@ describe('Domain API', () => { describe('when the domain is not in a pending state', () => { it('returns a 400', async () => { - const userPromise = factory.user(); - const site = await factory.site({ - users: Promise.all([userPromise]), - }); - const user = await userPromise; + const { site, user } = await createSiteUserOrg(); const domain = await factory.domain.create({ siteId: site.id, siteBranchConfigId: site.SiteBranchConfigs[0].id, @@ -796,12 +656,8 @@ describe('Domain API', () => { describe('when the parameters are valid', () => { it('it updates a domain name', async () => { const updatedNames = 'updated.agency.gov'; - const userPromise = factory.user(); - const site = await factory.site({ - users: Promise.all([userPromise]), - }); + const { site, user } = await createSiteUserOrg(); const siteBranchConfigId = site.SiteBranchConfigs[0].id; - const user = await userPromise; const domain = await factory.domain.create({ siteId: site.id, siteBranchConfigId, @@ -832,12 +688,8 @@ describe('Domain API', () => { it('it updates a domain site branch config', async () => { const names = 'www.agency.gov'; - const userPromise = factory.user(); - const site = await factory.site({ - users: Promise.all([userPromise]), - }); + const { site, user } = await createSiteUserOrg(); const siteBranchConfigId = site.SiteBranchConfigs[0].id; - const user = await userPromise; const sbc2 = await factory.siteBranchConfig.create({ site, branch: 'other', @@ -875,12 +727,8 @@ describe('Domain API', () => { it('it updates a domain site branch config and names', async () => { const names = 'www.agency.gov'; const updatedNames = 'updated.agency.gov'; - const userPromise = factory.user(); - const site = await factory.site({ - users: Promise.all([userPromise]), - }); + const { site, user } = await createSiteUserOrg(); const siteBranchConfigId = site.SiteBranchConfigs[0].id; - const user = await userPromise; const sbc2 = await factory.siteBranchConfig.create({ site, branch: 'other', diff --git a/test/api/requests/published-branch.test.js b/test/api/requests/published-branch.test.js index 7b28e052d..e773d69ec 100644 --- a/test/api/requests/published-branch.test.js +++ b/test/api/requests/published-branch.test.js @@ -10,6 +10,7 @@ const app = require('../../../app'); const factory = require('../support/factory'); const { authenticatedSession } = require('../support/session'); const validateAgainstJSONSchema = require('../support/validateAgainstJSONSchema'); +const { createSiteUserOrg } = require('../support/site-user'); const s3Mock = mockClient(S3Client); @@ -37,72 +38,50 @@ describe('Published Branches API', () => { .catch(done); }); - it('should throw 404 when site_id is NaN', (done) => { - const user = factory.user(); - const site = factory.site({ - users: Promise.all([user]), + it('should throw 404 when site_id is NaN', async () => { + const site = await factory.site({ demoBranch: 'demo', }); - Promise.props({ - user, - site, - cookie: authenticatedSession(user), - }) - .then((promisedValues) => - request(app) - .get('/v0/site/NaN/published-branch') - .set('Cookie', promisedValues.cookie) - .expect(404), - ) - .then((response) => { - validateAgainstJSONSchema( - 'GET', - '/site/{site_id}/published-branch', - 404, - response.body, - ); - done(); - }) - .catch(done); + const { user } = await createSiteUserOrg({ site }); + const cookie = await authenticatedSession(user); + + const response = await request(app) + .get('/v0/site/NaN/published-branch') + .set('Cookie', cookie) + .expect(404); + + validateAgainstJSONSchema( + 'GET', + '/site/{site_id}/published-branch', + 404, + response.body, + ); }); - it('should throw 404 when site_id does not exist', (done) => { - const user = factory.user(); - const site = factory.site({ - users: Promise.all([user]), + it('should throw 404 when site_id does not exist', async () => { + const site = await factory.site({ demoBranch: 'demo', }); - Promise.props({ - user, - site, - cookie: authenticatedSession(user), - }) - .then((promisedValues) => - request(app) - .get('/v0/site/-1/published-branch') - .set('Cookie', promisedValues.cookie) - .expect(404), - ) - .then((response) => { - validateAgainstJSONSchema( - 'GET', - '/site/{site_id}/published-branch', - 404, - response.body, - ); - done(); - }) - .catch(done); + const { user } = await createSiteUserOrg({ site }); + const cookie = await authenticatedSession(user); + + const response = await request(app) + .get('/v0/site/-1/published-branch') + .set('Cookie', cookie) + .expect(404); + + validateAgainstJSONSchema( + 'GET', + '/site/{site_id}/published-branch', + 404, + response.body, + ); }); - it("should list the previews available on S3 for a user's site", (done) => { - let site; - const user = factory.user(); - const sitePromise = factory.site({ - users: Promise.all([user]), - }); + it("should list the previews available on S3 for a user's site", async () => { + const { site, user } = await createSiteUserOrg(); s3Mock .on(ListObjectsV2Command) @@ -124,44 +103,31 @@ describe('Published Branches API', () => { mockTokenRequest(); apiNocks.mockDefaultCredentials(); - Promise.props({ - user, - site: sitePromise, - cookie: authenticatedSession(user), - }) - .then((promisedValues) => { - ({ site } = promisedValues); - - return request(app) - .get(`/v0/site/${site.id}/published-branch`) - .set('Cookie', promisedValues.cookie) - .expect(200); - }) - .then((response) => { - validateAgainstJSONSchema( - 'GET', - '/site/{site_id}/published-branch', - 200, - response.body, - ); - const branchNames = response.body.map((branch) => branch.name); - expect(branchNames).to.have.length(4); - expect(branchNames).to.include(site.defaultBranch); - expect(branchNames).to.include('abc'); - expect(branchNames).to.include('def'); - expect(branchNames).to.include('ghi'); - done(); - }) - .catch(done); + const cookie = await authenticatedSession(user); + const response = await request(app) + .get(`/v0/site/${site.id}/published-branch`) + .set('Cookie', cookie) + .expect(200); + + validateAgainstJSONSchema( + 'GET', + '/site/{site_id}/published-branch', + 200, + response.body, + ); + const branchNames = response.body.map((branch) => branch.name); + expect(branchNames).to.have.length(4); + expect(branchNames).to.include(site.defaultBranch); + expect(branchNames).to.include('abc'); + expect(branchNames).to.include('def'); + expect(branchNames).to.include('ghi'); }); - it('should list the demo branch if one is set on the site', (done) => { - let site; - const user = factory.user(); - const sitePromise = factory.site({ - users: Promise.all([user]), + it('should list the demo branch if one is set on the site', async () => { + const site = await factory.site({ demoBranch: 'demo', }); + const { user } = await createSiteUserOrg({ site }); mockTokenRequest(); apiNocks.mockDefaultCredentials(); @@ -173,31 +139,20 @@ describe('Published Branches API', () => { ContinuationToken: 'A', }); - Promise.props({ - user, - site: sitePromise, - cookie: authenticatedSession(user), - }) - .then((promisedValues) => { - ({ site } = promisedValues); - - return request(app) - .get(`/v0/site/${site.id}/published-branch`) - .set('Cookie', promisedValues.cookie) - .expect(200); - }) - .then((response) => { - validateAgainstJSONSchema( - 'GET', - '/site/{site_id}/published-branch', - 200, - response.body, - ); - const branchNames = response.body.map((branch) => branch.name); - expect(branchNames).to.deep.equal([site.defaultBranch, site.demoBranch, 'abc']); - done(); - }) - .catch(done); + const cookie = await authenticatedSession(user); + const response = await request(app) + .get(`/v0/site/${site.id}/published-branch`) + .set('Cookie', cookie) + .expect(200); + + validateAgainstJSONSchema( + 'GET', + '/site/{site_id}/published-branch', + 200, + response.body, + ); + const branchNames = response.body.map((branch) => branch.name); + expect(branchNames).to.deep.equal([site.defaultBranch, site.demoBranch, 'abc']); }); it('should 403 if the user is not associated with the site', (done) => { @@ -228,15 +183,13 @@ describe('Published Branches API', () => { .catch(done); }); - it('returns a 400 if the access keys are invalid', (done) => { - let site; + it('returns a 400 if the access keys are invalid', async () => { const expected = 'S3 keys out of date. Update them with `npm run update-local-config`'; - const user = factory.user(); - const sitePromise = factory.site({ - users: Promise.all([user]), + const site = await factory.site({ demoBranch: 'demo', }); + const { user } = await createSiteUserOrg({ site }); mockTokenRequest(); apiNocks.mockDefaultCredentials(); @@ -245,32 +198,20 @@ describe('Published Branches API', () => { code: 'InvalidAccessKeyId', }); - Promise.props({ - user, - site: sitePromise, - cookie: authenticatedSession(user), - }) - .then((promisedValues) => { - ({ site } = promisedValues); + const cookie = await authenticatedSession(user); + const response = await request(app) + .get(`/v0/site/${site.id}/published-branch`) + .set('Cookie', cookie) + .expect(400); - return request(app) - .get(`/v0/site/${site.id}/published-branch`) - .set('Cookie', promisedValues.cookie) - .expect(400); - }) - .then((response) => { - validateAgainstJSONSchema( - 'GET', - '/site/{site_id}/published-branch', - 400, - response.body, - ); - throw response.body; - }) - .catch((error) => { - expect(error.message).to.equal(expected); - done(); - }); + validateAgainstJSONSchema( + 'GET', + '/site/{site_id}/published-branch', + 400, + response.body, + ); + + expect(response.body.message).to.equal(expected); }); }); @@ -295,39 +236,26 @@ describe('Published Branches API', () => { .catch(done); }); - it('should render a JSON response for a pubslished branch', (done) => { - let site; - const user = factory.user(); - const sitePromise = factory.site({ + it('should render a JSON response for a pubslished branch', async () => { + const site = await factory.site({ defaultBranch: 'main', - users: Promise.all([user]), }); - - Promise.props({ - user, - site: sitePromise, - cookie: authenticatedSession(user), - }) - .then((promisedValues) => { - ({ site } = promisedValues); - - return request(app) - .get(`/v0/site/${site.id}/published-branch/main`) - .set('Cookie', promisedValues.cookie) - .expect(200); - }) - .then((response) => { - validateAgainstJSONSchema( - 'GET', - '/site/{site_id}/published-branch/{branch}', - 200, - response.body, - ); - expect(response.body.site.id).to.equal(site.id); - expect(response.body.name).to.equal('main'); - done(); - }) - .catch(done); + const { user } = await createSiteUserOrg({ site }); + + const cookie = await authenticatedSession(user); + const response = await request(app) + .get(`/v0/site/${site.id}/published-branch/main`) + .set('Cookie', cookie) + .expect(200); + + validateAgainstJSONSchema( + 'GET', + '/site/{site_id}/published-branch/{branch}', + 200, + response.body, + ); + expect(response.body.site.id).to.equal(site.id); + expect(response.body.name).to.equal('main'); }); it('should 403 if the user is not associated with the site', (done) => { @@ -360,64 +288,46 @@ describe('Published Branches API', () => { .catch(done); }); - it('should require site id is a Number', (done) => { - const user = factory.user(); - const site = factory.site({ + it('should require site id is a Number', async () => { + const site = await factory.site({ defaultBranch: 'main', - users: Promise.all([user]), }); - Promise.props({ - user, - site, - cookie: authenticatedSession(user), - }) - .then((promisedValues) => - request(app) - .get('/v0/site/NaN/published-branch/main') - .set('Cookie', promisedValues.cookie) - .expect(404), - ) - .then((response) => { - validateAgainstJSONSchema( - 'GET', - '/site/{site_id}/published-branch/{branch}', - 404, - response.body, - ); - done(); - }) - .catch(done); + const { user } = await createSiteUserOrg({ site }); + const cookie = await authenticatedSession(user); + + const response = await request(app) + .get('/v0/site/NaN/published-branch/main') + .set('Cookie', cookie) + .expect(404); + + validateAgainstJSONSchema( + 'GET', + '/site/{site_id}/published-branch/{branch}', + 404, + response.body, + ); }); - it('should require site id is in the sites table', (done) => { - const user = factory.user(); - const site = factory.site({ + it('should require site id is in the sites table', async () => { + const site = await factory.site({ defaultBranch: 'main', - users: Promise.all([user]), }); - Promise.props({ - user, - site, - cookie: authenticatedSession(user), - }) - .then((promisedValues) => - request(app) - .get('/v0/site/-1/published-branch/main') - .set('Cookie', promisedValues.cookie) - .expect(404), - ) - .then((response) => { - validateAgainstJSONSchema( - 'GET', - '/site/{site_id}/published-branch/{branch}', - 404, - response.body, - ); - done(); - }) - .catch(done); + const { user } = await createSiteUserOrg({ site }); + const cookie = await authenticatedSession(user); + + const response = await request(app) + .get('/v0/site/-1/published-branch/main') + .set('Cookie', cookie) + .expect(404); + + validateAgainstJSONSchema( + 'GET', + '/site/{site_id}/published-branch/{branch}', + 404, + response.body, + ); }); }); }); diff --git a/test/api/requests/published-file.test.js b/test/api/requests/published-file.test.js index 4d548f5e3..be40222ab 100644 --- a/test/api/requests/published-file.test.js +++ b/test/api/requests/published-file.test.js @@ -10,6 +10,7 @@ const app = require('../../../app'); const factory = require('../support/factory'); const { authenticatedSession } = require('../support/session'); const validateAgainstJSONSchema = require('../support/validateAgainstJSONSchema'); +const { createSiteUserOrg } = require('../support/site-user'); const s3Mock = mockClient(S3Client); @@ -40,156 +41,118 @@ describe('Published Files API', () => { .catch(done); }); - it('should list the files published to the branch for the site', (done) => { - let site; - let prefix; - const userPromise = factory.user(); - const sitePromise = factory.site({ - defaultBranch: 'main', - users: Promise.all([userPromise]), - }); - const cookiePromise = authenticatedSession(userPromise); + it('should list the files published to the branch for the site', async () => { + const { site, user } = await createSiteUserOrg(); + const prefix = `site/${site.owner}/${site.repository}/`; + const cookie = await authenticatedSession(user); mockTokenRequest(); apiNocks.mockDefaultCredentials(); - Promise.props({ - user: userPromise, - site: sitePromise, - cookie: cookiePromise, - }) - .then((promisedValues) => { - ({ site } = promisedValues); - prefix = `site/${site.owner}/${site.repository}/`; - - s3Mock - .on(ListObjectsV2Command) - .resolvesOnce({ - IsTruncated: true, - Contents: [ - { - Key: `${prefix}abc`, - Size: 123, - }, - { - Key: `${prefix}abc/def`, - Size: 456, - }, - ], - ContinuationToken: 'A', - NextContinuationToken: 'B', - }) - .resolvesOnce({ - IsTruncated: false, - Contents: [ - { - Key: `${prefix}ghi`, - Size: 789, - }, - ], - ContinuationToken: 'B', - NextContinuationToken: null, - }); - - return request(app) - .get(`/v0/site/${site.id}/published-branch/main/published-file`) - .set('Cookie', promisedValues.cookie) - .expect(200); - }) - .then((response) => { - validateAgainstJSONSchema( - 'GET', - '/site/{site_id}/published-branch/{branch}/published-file', - 200, - response.body, - ); - - const { files } = response.body; - files.forEach((file) => { - delete file.publishedBranch; - }); - expect(files).to.deep.equal([ + s3Mock + .on(ListObjectsV2Command) + .resolvesOnce({ + IsTruncated: true, + Contents: [ { - name: 'abc', - size: 123, - key: `${prefix}abc`, + Key: `${prefix}abc`, + Size: 123, }, { - name: 'abc/def', - size: 456, - key: `${prefix}abc/def`, + Key: `${prefix}abc/def`, + Size: 456, }, + ], + ContinuationToken: 'A', + NextContinuationToken: 'B', + }) + .resolvesOnce({ + IsTruncated: false, + Contents: [ { - name: 'ghi', - size: 789, - key: `${prefix}ghi`, + Key: `${prefix}ghi`, + Size: 789, }, - ]); - done(); - }) - .catch(done); + ], + ContinuationToken: 'B', + NextContinuationToken: null, + }); + + const response = await request(app) + .get(`/v0/site/${site.id}/published-branch/main/published-file`) + .set('Cookie', cookie) + .expect(200); + + validateAgainstJSONSchema( + 'GET', + '/site/{site_id}/published-branch/{branch}/published-file', + 200, + response.body, + ); + + const { files } = response.body; + files.forEach((file) => { + delete file.publishedBranch; + }); + expect(files).to.deep.equal([ + { + name: 'abc', + size: 123, + key: `${prefix}abc`, + }, + { + name: 'abc/def', + size: 456, + key: `${prefix}abc/def`, + }, + { + name: 'ghi', + size: 789, + key: `${prefix}ghi`, + }, + ]); }); - it('should 404 is site owner is NaN', (done) => { - const userPromise = factory.user(); - const sitePromise = factory.site({ + it('should 404 is site owner is NaN', async () => { + const site = await factory.site({ defaultBranch: 'main', - users: Promise.all([userPromise]), }); - const cookiePromise = authenticatedSession(userPromise); - Promise.props({ - user: userPromise, - site: sitePromise, - cookie: cookiePromise, - }) - .then((promisedValues) => { - return request(app) - .get('/v0/site/NaN/published-branch/main/published-file') - .set('Cookie', promisedValues.cookie) - .expect(404); - }) - .then((response) => { - validateAgainstJSONSchema( - 'GET', - '/site/{site_id}/published-branch/{branch}/published-file', - 404, - response.body, - ); - done(); - }) - .catch(done); + const { user } = await createSiteUserOrg({ site }); + const cookie = await authenticatedSession(user); + + const response = await request(app) + .get('/v0/site/NaN/published-branch/main/published-file') + .set('Cookie', cookie) + .expect(404); + + validateAgainstJSONSchema( + 'GET', + '/site/{site_id}/published-branch/{branch}/published-file', + 404, + response.body, + ); }); - it('should 404 is site owner is not found', (done) => { - const userPromise = factory.user(); - const sitePromise = factory.site({ + it('should 404 is site owner is not found', async () => { + const site = await factory.site({ defaultBranch: 'main', - users: Promise.all([userPromise]), }); - const cookiePromise = authenticatedSession(userPromise); - Promise.props({ - user: userPromise, - site: sitePromise, - cookie: cookiePromise, - }) - .then((promisedValues) => { - return request(app) - .get('/v0/site/-1/published-branch/main/published-file') - .set('Cookie', promisedValues.cookie) - .expect(404); - }) - .then((response) => { - validateAgainstJSONSchema( - 'GET', - '/site/{site_id}/published-branch/{branch}/published-file', - 404, - response.body, - ); - done(); - }) - .catch(done); + const { user } = await createSiteUserOrg({ site }); + const cookie = await authenticatedSession(user); + + const response = await request(app) + .get('/v0/site/-1/published-branch/main/published-file') + .set('Cookie', cookie) + .expect(404); + + validateAgainstJSONSchema( + 'GET', + '/site/{site_id}/published-branch/{branch}/published-file', + 404, + response.body, + ); }); it('should 403 if the user is not associated with the site', (done) => { diff --git a/test/api/requests/site-branch-config.test.js b/test/api/requests/site-branch-config.test.js index 7ae71b9c7..9077e41d6 100644 --- a/test/api/requests/site-branch-config.test.js +++ b/test/api/requests/site-branch-config.test.js @@ -6,8 +6,9 @@ const factory = require('../support/factory'); const csrfToken = require('../support/csrfToken'); const { authenticatedSession } = require('../support/session'); const validateAgainstJSONSchema = require('../support/validateAgainstJSONSchema'); +const { createSiteUserOrg } = require('../support/site-user'); const app = require('../../../app'); -const { Build, Role, Site, SiteBranchConfig } = require('../../../api/models'); +const { Build, Site, SiteBranchConfig } = require('../../../api/models'); const EventCreator = require('../../../api/services/EventCreator'); function clean() { @@ -105,11 +106,7 @@ describe('Site Branch Config API', () => { describe('when the site branch config does not exist', () => { it('returns a 404', async () => { const branchConfigId = 1; - const userPromise = factory.user(); - const site = await factory.site({ - users: Promise.all([userPromise]), - }); - const user = await userPromise; + const { user, site } = await createSiteUserOrg(); const cookie = await authenticatedSession(user); const { body } = await request(app) @@ -130,14 +127,8 @@ describe('Site Branch Config API', () => { describe('when the parameters are valid', () => { it('deletes the site branch config and returns a 200', async () => { - const userPromise = factory.user(); - const site = await factory.site({ - users: Promise.all([userPromise]), - }); - const [sbc, user] = await Promise.all([ - factory.siteBranchConfig.create({ site }), - userPromise, - ]); + const { user, site } = await createSiteUserOrg(); + const sbc = await factory.siteBranchConfig.create({ site }); const cookie = await authenticatedSession(user); const beforeNumSBC = await SiteBranchConfig.count(); @@ -155,20 +146,7 @@ describe('Site Branch Config API', () => { it(`allows an org user to delete the site branch config and returns a 200`, async () => { - const org = await factory.organization.create(); - const role = await Role.findOne({ - where: { - name: 'user', - }, - }); - const user = await factory.user(); - const site = await factory.site(); - await org.addUser(user, { - through: { - roleId: role.id, - }, - }); - await org.addSite(site); + const { user, site } = await createSiteUserOrg(); const sbcs = await Promise.all([ factory.siteBranchConfig.create({ @@ -245,16 +223,13 @@ describe('Site Branch Config API', () => { describe('when there are no site branch configs for the site', () => { it('returns an empty array', async () => { - const userPromise = factory.user(); const site = await factory.site( - { - users: Promise.all([userPromise]), - }, + {}, { noSiteBranchConfig: true, }, ); - const user = await userPromise; + const { user } = await createSiteUserOrg({ site }); const cookie = await authenticatedSession(user); const { body } = await request(app) @@ -271,19 +246,14 @@ describe('Site Branch Config API', () => { describe('when there are site branch configs for the site', () => { it(`returns an array containing only the site branch configs for the site`, async () => { - const userPromise = factory.user(); const site = await factory.site( - { - users: Promise.all([userPromise]), - }, + {}, { noSiteBranchConfig: true, }, ); const otherSite = await factory.site( - { - users: Promise.all([userPromise]), - }, + {}, { noSiteBranchConfig: true, }, @@ -325,58 +295,7 @@ describe('Site Branch Config API', () => { }), ]); - const user = await userPromise; - const cookie = await authenticatedSession(user); - - const { body } = await request(app) - .get(`/v0/site/${site.id}/branch-config`) - .set('Cookie', cookie) - .type('json') - .expect(200); - - validateAgainstJSONSchema('GET', '/site/{site_id}/branch-config', 200, body); - expect(body).to.have.length(sbcs.length); - expect(body.map((sbc) => sbc.id)).to.have.members(sbcs.map((sbc) => sbc.id)); - }); - - it(`returns an array containing only the site branch configs - for a site for an org user in the site org`, async () => { - const org = await factory.organization.create(); - const role = await Role.findOne({ - where: { - name: 'user', - }, - }); - const user = await factory.user(); - const site = await factory.site(undefined, { - noSiteBranchConfig: true, - }); - await org.addUser(user, { - through: { - roleId: role.id, - }, - }); - await org.addSite(site); - - const sbcs = await Promise.all([ - factory.siteBranchConfig.create({ - site, - branch: 'test1', - }), - factory.siteBranchConfig.create({ - site, - branch: 'test2', - }), - factory.siteBranchConfig.create({ - site, - branch: 'test3', - }), - factory.siteBranchConfig.create({ - site, - branch: 'test4', - }), - ]); - + const { user } = await createSiteUserOrg({ site }); const cookie = await authenticatedSession(user); const { body } = await request(app) @@ -457,11 +376,7 @@ describe('Site Branch Config API', () => { describe('when the branch already exists for a site', () => { it('returns a 400', async () => { - const userPromise = factory.user(); - const site = await factory.site({ - users: Promise.all([userPromise]), - }); - const user = await userPromise; + const { user, site } = await createSiteUserOrg(); const branch = 'test1'; const context = 'preview'; await factory.siteBranchConfig.create({ site, branch }); @@ -488,11 +403,7 @@ describe('Site Branch Config API', () => { describe('when the branch name is invalid', () => { it('returns a 400', async () => { - const userPromise = factory.user(); - const site = await factory.site({ - users: Promise.all([userPromise]), - }); - const user = await userPromise; + const { user, site } = await createSiteUserOrg(); const branch = 'test bad branch name$'; const cookie = await authenticatedSession(user); const context = 'preview'; @@ -516,11 +427,7 @@ describe('Site Branch Config API', () => { }); it('returns a 400 when branch name is too long', async () => { - const userPromise = factory.user(); - const site = await factory.site({ - users: Promise.all([userPromise]), - }); - const user = await userPromise; + const { user, site } = await createSiteUserOrg(); const branch = Array(301).join('b'); const cookie = await authenticatedSession(user); const context = 'preview'; @@ -546,11 +453,7 @@ describe('Site Branch Config API', () => { describe('when the config is not valid', () => { it('returns a 400 with a number as a config', async () => { - const userPromise = factory.user(); - const site = await factory.site({ - users: Promise.all([userPromise]), - }); - const user = await userPromise; + const { user, site } = await createSiteUserOrg(); const branch = 'test1'; const config = '12345'; const context = 'preview'; @@ -576,11 +479,7 @@ describe('Site Branch Config API', () => { }); it('returns a 400 with a string as a config', async () => { - const userPromise = factory.user(); - const site = await factory.site({ - users: Promise.all([userPromise]), - }); - const user = await userPromise; + const { user, site } = await createSiteUserOrg(); const branch = 'test1'; const config = 'tests'; const context = 'preview'; @@ -608,16 +507,13 @@ describe('Site Branch Config API', () => { describe('when the context is invalid', () => { it('returns a 400', async () => { - const userPromise = factory.user(); const site = await factory.site( - { - users: Promise.all([userPromise]), - }, + {}, { noSiteBranchConfig: true, }, ); - const user = await userPromise; + const { user } = await createSiteUserOrg({ site }); const branch = 'test bad branch name$'; const cookie = await authenticatedSession(user); @@ -642,11 +538,7 @@ describe('Site Branch Config API', () => { describe('when the parameters are valid', () => { it('creates and returns the site branch config', async () => { - const userPromise = factory.user(); - const site = await factory.site({ - users: Promise.all([userPromise]), - }); - const user = await userPromise; + const { user, site } = await createSiteUserOrg(); const cookie = await authenticatedSession(user); const branch = 'my-test-branch'; const config = { @@ -678,11 +570,7 @@ describe('Site Branch Config API', () => { }); it('creates s3key for site context', async () => { - const userPromise = factory.user(); - const site = await factory.site({ - users: Promise.all([userPromise]), - }); - const user = await userPromise; + const { user, site } = await createSiteUserOrg(); const cookie = await authenticatedSession(user); const branch = 'my-test-branch'; const config = { @@ -718,11 +606,7 @@ describe('Site Branch Config API', () => { }); it('creates s3Key for demo context and kicks off a build', async () => { - const userPromise = factory.user(); - const site = await factory.site({ - users: Promise.all([userPromise]), - }); - const user = await userPromise; + const { user, site } = await createSiteUserOrg(); const cookie = await authenticatedSession(user); const branch = 'my-test-branch'; const config = { @@ -758,11 +642,7 @@ describe('Site Branch Config API', () => { }); it('creates s3Key for other context type', async () => { - const userPromise = factory.user(); - const site = await factory.site({ - users: Promise.all([userPromise]), - }); - const user = await userPromise; + const { user, site } = await createSiteUserOrg(); const cookie = await authenticatedSession(user); const branch = 'my-test-branch'; const config = { @@ -798,16 +678,13 @@ describe('Site Branch Config API', () => { }); it('creates and returns the preview site branch config', async () => { - const userPromise = factory.user(); const site = await factory.site( - { - users: Promise.all([userPromise]), - }, + {}, { noSiteBranchConfig: true, }, ); - const user = await userPromise; + const { user } = await createSiteUserOrg({ site }); const cookie = await authenticatedSession(user); const context = 'preview'; const config = { @@ -838,57 +715,10 @@ describe('Site Branch Config API', () => { expect(build).to.eq(null); expect(afterNumSBC).to.eq(beforeNumSBCs + 1); }); - - it('allows an org user to create and return the site branch config', async () => { - const org = await factory.organization.create(); - const role = await Role.findOne({ - where: { - name: 'user', - }, - }); - const user = await factory.user(); - const site = await factory.site(); - await org.addUser(user, { - through: { - roleId: role.id, - }, - }); - await org.addSite(site); - - const cookie = await authenticatedSession(user); - const branch = 'my-test-branch'; - const config = { - hello: 'world', - }; - - const beforeNumSBCs = await SiteBranchConfig.count(); - - const { body } = await request(app) - .post(`/v0/site/${site.id}/branch-config`) - .set('Cookie', cookie) - .set('x-csrf-token', csrfToken.getToken()) - .type('json') - .send({ - branch, - config, - }) - .expect(200); - - const afterNumSBC = await SiteBranchConfig.count(); - - validateAgainstJSONSchema('POST', '/site/{site_id}/branch-config', 200, body); - expect(body.branch).to.eq(branch); - expect(body.config).to.deep.eq(config); - expect(afterNumSBC).to.eq(beforeNumSBCs + 1); - }); }); it('creates and returns the site branch config with a yaml config', async () => { - const userPromise = factory.user(); - const site = await factory.site({ - users: Promise.all([userPromise]), - }); - const user = await userPromise; + const { user, site } = await createSiteUserOrg(); const cookie = await authenticatedSession(user); const branch = 'my-test-branch'; const configObject = { @@ -929,16 +759,12 @@ describe('Site Branch Config API', () => { }; const updatedBranch = 'updated-test-branch'; const updatedConfig = { hello: 'again' }; - const userPromise = factory.user(); - const site = await factory.site({ - users: Promise.all([userPromise]), - }); + const { user, site } = await createSiteUserOrg(); const sbc = await factory.siteBranchConfig.create({ site, branch: origBranch, config: origConfig, }); - const user = await userPromise; const cookie = await authenticatedSession(user); const beforeNumSBCs = await SiteBranchConfig.count(); @@ -975,17 +801,13 @@ describe('Site Branch Config API', () => { hello: 'world', }; const updatedConfig = { hello: 'again' }; - const userPromise = factory.user(); - const site = await factory.site({ - users: Promise.all([userPromise]), - }); + const { user, site } = await createSiteUserOrg(); const sbc = await factory.siteBranchConfig.create({ site, branch, context, config: origConfig, }); - const user = await userPromise; const cookie = await authenticatedSession(user); const beforeNumSBCs = await SiteBranchConfig.count(); @@ -1026,21 +848,18 @@ describe('Site Branch Config API', () => { hello: 'world', }; const updatedConfig = { hello: 'again' }; - const userPromise = factory.user(); const site = await factory.site( - { - users: Promise.all([userPromise]), - }, + {}, { noSiteBranchConfig: true, }, ); + const { user } = await createSiteUserOrg({ site }); const sbc = await factory.siteBranchConfig.create({ site, context, config: origConfig, }); - const user = await userPromise; const cookie = await authenticatedSession(user); const beforeNumSBCs = await SiteBranchConfig.count(); @@ -1080,10 +899,7 @@ describe('Site Branch Config API', () => { }; const updatedBranch = 'updated-test-branch'; const updatedConfig = { hello: 'again' }; - const userPromise = factory.user(); - const site = await factory.site({ - users: Promise.all([userPromise]), - }); + const { user, site } = await createSiteUserOrg(); await factory.siteBranchConfig.create({ site, branch: origBranch, @@ -1091,7 +907,6 @@ describe('Site Branch Config API', () => { context: 'preview', }); const notSBC = 90210; - const user = await userPromise; const cookie = await authenticatedSession(user); const { body } = await request(app) diff --git a/test/api/requests/site.test.js b/test/api/requests/site.test.js index 222196868..9de3b9637 100644 --- a/test/api/requests/site.test.js +++ b/test/api/requests/site.test.js @@ -13,13 +13,12 @@ const apiNocks = require('../support/cfAPINocks'); const { authenticatedSession, unauthenticatedSession } = require('../support/session'); const validateAgainstJSONSchema = require('../support/validateAgainstJSONSchema'); const csrfToken = require('../support/csrfToken'); +const { createSiteUserOrg } = require('../support/site-user'); -const { Build, Organization, Role, Site, User } = require('../../../api/models'); +const { Organization, Role, Site } = require('../../../api/models'); const SiteDestroyer = require('../../../api/services/SiteDestroyer'); -const siteErrors = require('../../../api/responses/siteErrors'); const QueueJobs = require('../../../api/queue-jobs'); const EventCreator = require('../../../api/services/EventCreator'); -const DomainService = require('../../../api/services/Domain'); const authErrorMessage = 'You are not permitted to perform this action. Are you sure you are logged in?'; @@ -58,56 +57,36 @@ describe('Site API', () => { .catch(done); }); - it('should render a list of sites associated with the user', (done) => { - let user; - let sites; - let response; + it('should return a list of sites associated with the user', async () => { + const user = await factory.user(); + const org = await factory.organization.create(); + await org.addRoleUser(user); + + const sites = await Promise.all( + Array(3) + .fill(0) + .map(async () => { + const { site } = await createSiteUserOrg({ user }); + return site; + }), + ); + const cookie = await authenticatedSession(user); + const response = await request(app) + .get('/v0/site') + .set('Cookie', cookie) + .expect(200); - factory - .user() - .then((model) => { - user = model; - const sitePromises = Array(3) - .fill(0) - .map(() => - factory.site({ - users: [user.id], - }), - ); - return Promise.all(sitePromises); - }) - .then((models) => { - sites = models; - return authenticatedSession(user); - }) - .then((cookie) => request(app).get('/v0/site').set('Cookie', cookie).expect(200)) - .then((resp) => { - response = resp; + validateAgainstJSONSchema('GET', '/site', 200, response.body); - validateAgainstJSONSchema('GET', '/site', 200, response.body); + expect(response.body).to.be.a('array'); + expect(response.body).to.have.length(3); - expect(response.body).to.be.a('array'); - expect(response.body).to.have.length(3); - - return Promise.all( - sites.map((site) => - Site.findByPk(site.id, { - include: [User], - }), - ), - ); - }) - .then((foundSites) => { - foundSites.forEach((site) => { - const responseSite = response.body.find( - (candidate) => candidate.id === site.id, - ); - expect(responseSite).not.to.be.undefined; - siteResponseExpectations(responseSite, site); - }); - done(); - }) - .catch(done); + sites.forEach((site) => { + const responseSite = response.body.find((candidate) => candidate.id === site.id); + expect(responseSite).not.to.be.undefined; + siteResponseExpectations(responseSite, site); + expect(responseSite).to.include.keys('canEditLiveUrl', 'canEditDemoUrl'); + }); }); it('should not render any sites not associated with the user', (done) => { @@ -129,60 +108,6 @@ describe('Site API', () => { }) .catch(done); }); - - it("should include sites' URL editability", (done) => { - let user; - let sites; - let response; - - factory - .user() - .then((model) => { - user = model; - const sitePromises = Array(3) - .fill(0) - .map(() => - factory.site({ - users: [user.id], - }), - ); - - return Promise.all(sitePromises); - }) - .then((models) => { - sites = models; - return authenticatedSession(user); - }) - .then((cookie) => request(app).get('/v0/site').set('Cookie', cookie).expect(200)) - .then((resp) => { - response = resp; - - validateAgainstJSONSchema('GET', '/site', 200, response.body); - - expect(response.body).to.be.a('array'); - expect(response.body).to.have.length(3); - - return Promise.all( - sites.map((site) => - Site.findByPk(site.id, { - include: [User], - }), - ), - ); - }) - .then((foundSites) => { - foundSites.forEach((site) => { - const responseSite = response.body.find( - (candidate) => candidate.id === site.id, - ); - expect(responseSite).not.to.be.undefined; - siteResponseExpectations(responseSite, site); - expect(responseSite).to.include.keys('canEditLiveUrl', 'canEditDemoUrl'); - }); - done(); - }) - .catch(done); - }); }); describe('GET /v0/site/:id', () => { @@ -198,29 +123,15 @@ describe('Site API', () => { .catch(done); }); - it('should render a JSON representation of the site', (done) => { - let site; - - factory - .site() - .then((s) => - Site.findByPk(s.id, { - include: [User], - }), - ) - .then((model) => { - site = model; - return authenticatedSession(site.Users[0]); - }) - .then((cookie) => - request(app).get(`/v0/site/${site.id}`).set('Cookie', cookie).expect(200), - ) - .then((response) => { - validateAgainstJSONSchema('GET', '/site/{id}', 200, response.body); - siteResponseExpectations(response.body, site); - done(); - }) - .catch(done); + it('should render a JSON representation of the site', async () => { + const { site, user } = await createSiteUserOrg(); + const cookie = await authenticatedSession(user); + const response = await request(app) + .get(`/v0/site/${site.id}`) + .set('Cookie', cookie) + .expect(200); + validateAgainstJSONSchema('GET', '/site/{id}', 200, response.body); + siteResponseExpectations(response.body, site); }); it(`should respond with a 404 if the user @@ -388,53 +299,43 @@ describe('Site API', () => { .catch(done); }); - it('should create a new site from an existing repository', (done) => { + it('should create a new site from an existing repository', async () => { const siteOwner = crypto.randomBytes(3).toString('hex'); const siteRepository = crypto.randomBytes(3).toString('hex'); cfMockServices(siteOwner, siteRepository); + const user = await factory.user(); + githubAPINocks.userOrganizations({ + accessToken: user.githubAccessToken, + organizations: [ + { + login: siteOwner, + }, + ], + }); - factory - .user() - .then((user) => { - githubAPINocks.userOrganizations({ - accessToken: user.githubAccessToken, - organizations: [ - { - login: siteOwner, - }, - ], - }); - - return authenticatedSession(user); - }) - .then((cookie) => - request(app) - .post('/v0/site') - .set('x-csrf-token', csrfToken.getToken()) - .send({ - owner: siteOwner, - repository: siteRepository, - defaultBranch: 'main', - engine: 'jekyll', - }) - .set('Cookie', cookie) - .expect(200), - ) - .then((response) => { - validateAgainstJSONSchema('POST', '/site', 200, response.body); - return Site.findOne({ - where: { - owner: siteOwner, - repository: siteRepository, - }, - }); - }) - .then((site) => { - expect(site).to.not.be.undefined; - done(); + const cookie = await authenticatedSession(user); + const response = request(app) + .post('/v0/site') + .set('x-csrf-token', csrfToken.getToken()) + .send({ + owner: siteOwner, + repository: siteRepository, + defaultBranch: 'main', + engine: 'jekyll', }) - .catch(done); + .set('Cookie', cookie) + .expect(200); + + validateAgainstJSONSchema('POST', '/site', 200, response.body); + const site = await Site.findOne({ + where: { + owner: siteOwner, + repository: siteRepository, + }, + }); + + expect(site).to.not.be.undefined; }); it(`should create a new site from an existing repository @@ -790,805 +691,117 @@ describe('Site API', () => { }); }); - describe('POST /v0/site/user', () => { + describe('DELETE /v0/site/:id', () => { + let queueDestroySiteInfra; + beforeEach(() => { - nock.cleanAll(); - githubAPINocks.repo(); + queueDestroySiteInfra = sinon + .stub(SiteDestroyer, 'queueDestroySiteInfra') + .resolves(); }); - it('should require a valid csrf token', (done) => { - authenticatedSession() - .then((cookie) => - request(app) - .post('/v0/site/user') - .set('x-csrf-token', 'bad-token') - .send({ - owner: 'partner-org', - repository: 'partner-site', - }) - .set('Cookie', cookie) - .expect(403), - ) - .then((response) => { - validateAgainstJSONSchema('POST', '/site/user', 403, response.body); - expect(response.body.message).to.equal('Invalid CSRF token'); - done(); - }) - .catch(done); + afterEach(() => { + sinon.restore(); }); it('should require authentication', (done) => { - unauthenticatedSession() - .then((cookie) => { - const newSiteRequest = request(app) - .post('/v0/site/user') - .set('x-csrf-token', csrfToken.getToken()) - .send({ - owner: 'partner-org', - repository: 'partner-site', - }) - .set('Cookie', cookie) - .expect(403); - - return newSiteRequest; - }) - .then((response) => { - validateAgainstJSONSchema('POST', '/site/user', 403, response.body); - expect(response.body.message).to.equal(authErrorMessage); - done(); - }) - .catch(done); - }); - - it('should add the user to the site', (done) => { - const userPromise = factory.user(); - let user; let site; - Promise.props({ - user: userPromise, - site: factory.site(), - cookie: authenticatedSession(userPromise), - }) - .then((models) => { - ({ user, site } = models); - - return request(app) - .post('/v0/site/user') - .set('x-csrf-token', csrfToken.getToken()) - .set('Cookie', models.cookie) - .send({ - owner: site.owner, - repository: site.repository, - }) - .expect(200); - }) - .then((response) => { - validateAgainstJSONSchema('POST', '/site/user', 200, response.body); - return Site.findByPk(site.id, { - include: [User], + factory + .site() + .then((model) => { + site = model; + nock.cleanAll(); + githubAPINocks.repo({ + owner: site.owner, + repository: site.repo, + response: [ + 200, + { + permissions: { + admin: true, + push: true, + }, + }, + ], }); + return unauthenticatedSession(); }) - .then((fetchedSite) => { - expect(fetchedSite.Users).to.be.an('array'); - const userIDs = fetchedSite.Users.map((u) => u.id); - expect(userIDs).to.include(user.id); - done(); - }) - .catch(done); - }); - - it('should respond with a 400 if no user or repository is specified', (done) => { - authenticatedSession() .then((cookie) => request(app) - .post('/v0/site/user') + .delete(`/v0/site/${site.id}`) .set('x-csrf-token', csrfToken.getToken()) .set('Cookie', cookie) - .send({}) - .expect(400), + .expect(403), ) .then((response) => { - validateAgainstJSONSchema('POST', '/site/user', 400, response.body); + validateAgainstJSONSchema('DELETE', '/site/{id}', 403, response.body); + expect(response.body.message).to.equal(authErrorMessage); done(); }) .catch(done); }); - it('should respond with a 400 if the user has already added the site', (done) => { - const userPromise = factory.user(); + it('should require a valid csrf token', (done) => { + let site; - Promise.props({ - site: factory.site({ - users: Promise.all([userPromise]), - }), - cookie: authenticatedSession(userPromise), - }) - .then(({ site, cookie }) => + factory + .site() + .then((model) => { + site = model; + return authenticatedSession(); + }) + .then((cookie) => request(app) - .post('/v0/site/user') - .set('x-csrf-token', csrfToken.getToken()) + .delete(`/v0/site/${site.id}`) + .set('x-csrf-token', 'bad-token') .set('Cookie', cookie) - .send({ - owner: site.owner, - repository: site.repository, - }) - .expect(400), + .expect(403), ) .then((response) => { - validateAgainstJSONSchema('POST', '/site/user', 400, response.body); - expect(response.body.message).to.eq( - `You've already added this site to ${config.app.appName}`, - ); + validateAgainstJSONSchema('PUT', '/site/{id}', 403, response.body); + expect(response.body.message).to.equal('Invalid CSRF token'); done(); }) .catch(done); }); - it(`should respond with a 400 if the user - does not have write access to repository`, (done) => { - const siteOwner = crypto.randomBytes(3).toString('hex'); - const siteRepository = crypto.randomBytes(3).toString('hex'); - + it('should allow a user to delete a site associated with their account', async () => { + const { site, user } = await createSiteUserOrg(); nock.cleanAll(); githubAPINocks.repo({ - owner: siteOwner, - repository: siteRepository, + owner: site.owner, + repository: site.repo, response: [ 200, { permissions: { - admin: false, - push: false, + admin: true, + push: true, }, }, ], }); - githubAPINocks.webhook(); + const cookie = await authenticatedSession(user); + const response = await request(app) + .delete(`/v0/site/${site.id}`) + .set('x-csrf-token', csrfToken.getToken()) + .set('Cookie', cookie) + .expect(200); - Promise.props({ - cookie: authenticatedSession(), - site: factory.site({ - owner: siteOwner, - repository: siteRepository, - }), - }) - .then(({ cookie, site }) => - request(app) - .post('/v0/site/user') - .set('x-csrf-token', csrfToken.getToken()) - .set('Cookie', cookie) - .send({ - owner: site.owner, - repository: site.repository, - }) - .expect(400), - ) - .then((response) => { - validateAgainstJSONSchema('POST', '/site/user', 400, response.body); - expect(response.body.message).to.eq( - 'You do not have write access to this repository', - ); - done(); - }) - .catch(done); - }); - - it('should respond with a 404 if the site does not exist', (done) => { - authenticatedSession() - .then((cookie) => - request(app) - .post('/v0/site/user') - .set('x-csrf-token', csrfToken.getToken()) - .set('Cookie', cookie) - .send({ - owner: 'this-is', - repository: 'not-real', - }) - .expect(404), - ) - .then((response) => { - validateAgainstJSONSchema('POST', '/site/user', 404, response.body); - expect(response.body.message).to.eq( - 'The site you are trying to add does not exist', - ); - done(); - }) - .catch(done); - }); - }); - - describe('DELETE /v0/site/:site_id/user/:user_id', () => { - const path = '/site/{site_id}/user/{user_id}'; - const requestPath = (siteId, userId) => `/v0/site/${siteId}/user/${userId}`; - - it('should require a valid csrf token', (done) => { - authenticatedSession() - .then((cookie) => - request(app) - .delete(requestPath(1, 1)) - .set('x-csrf-token', 'bad-token') - .set('Cookie', cookie) - .expect(403), - ) - .then((response) => { - validateAgainstJSONSchema('DELETE', path, 403, response.body); - expect(response.body.message).to.equal('Invalid CSRF token'); - done(); - }) - .catch(done); - }); - - it('should require authentication', (done) => { - unauthenticatedSession() - .then((cookie) => { - const newSiteRequest = request(app) - .delete(requestPath(1, 1)) - .set('x-csrf-token', csrfToken.getToken()) - .set('Cookie', cookie) - .expect(403); - return newSiteRequest; - }) - .then((response) => { - validateAgainstJSONSchema('DELETE', path, 403, response.body); - expect(response.body.message).to.equal(authErrorMessage); - done(); - }) - .catch(done); - }); - - it('should respond with a 400 if siteId or userId are not numbers', (done) => { - const userPromise = factory.user(); - - Promise.props({ - user: userPromise, - site: factory.site(), - cookie: authenticatedSession(userPromise), - }) - .then((models) => - request(app) - .delete(requestPath('a-site', 'a-user')) - .set('x-csrf-token', csrfToken.getToken()) - .set('Cookie', models.cookie) - .expect(400), - ) - .then((response) => { - validateAgainstJSONSchema('DELETE', path, 400, response.body); - expect(response.body.message).to.equal('Bad Request'); - done(); - }) - .catch(done); - }); - - it('should return a 404 if the site cannot be found', (done) => { - const userPromise = factory.user(); - - Promise.props({ - user: userPromise, - site: factory.site(), - cookie: authenticatedSession(userPromise), - }) - .then((models) => - request(app) - .delete(requestPath(1000, models.user.id)) - .set('x-csrf-token', csrfToken.getToken()) - .set('Cookie', models.cookie) - .expect(404), - ) - .then((response) => { - validateAgainstJSONSchema('DELETE', path, 404, response.body); - expect(response.body.message).to.equal('Not found'); - done(); - }) - .catch(done); - }); - - it('should remove the user from the site', (done) => { - const mike = factory.user({ - username: 'mike', - }); - const jane = factory.user({ - username: 'jane', - }); - let currentSite; - - Promise.props({ - user: jane, - site: factory.site({ - users: Promise.all([mike, jane]), - }), - cookie: authenticatedSession(jane), - }) - .then(({ user, site, cookie }) => { - currentSite = site; - - nock.cleanAll(); - githubAPINocks.repo({ - owner: site.owner, - repository: site.repo, - response: [ - 200, - { - permissions: { - admin: true, - push: true, - }, - }, - ], - }); - - return request(app) - .delete(requestPath(site.id, user.id)) - .set('x-csrf-token', csrfToken.getToken()) - .set('Cookie', cookie) - .expect(200); - }) - .then((response) => { - validateAgainstJSONSchema('DELETE', path, 200, response.body); - return Site.withUsers(currentSite.id); - }) - .then((fetchedSite) => { - expect(fetchedSite.Users).to.be.an('array'); - expect(fetchedSite.Users.length).to.equal(1); - done(); - }) - .catch(done); - }); - - it('should allow the owner to remove a user from the site', (done) => { - const username = 'b-user'; - const userPromise = factory.user({ - username, - }); - const anotherUser = factory.user(); - const siteProps = { - owner: username, - users: Promise.all([userPromise, anotherUser]), - }; - - let currentSite; - - nock.cleanAll(); - - Promise.props({ - user: userPromise, - site: factory.site(siteProps), - cookie: authenticatedSession(userPromise), - anotherUser, - }) - .then((models) => { - currentSite = models.site; - - githubAPINocks.repo({ - owner: username, - repository: models.site.repo, - response: [ - 200, - { - permissions: { - admin: true, - push: true, - }, - }, - ], - }); - - return request(app) - .delete(requestPath(models.site.id, models.anotherUser.id)) - .set('x-csrf-token', csrfToken.getToken()) - .set('Cookie', models.cookie) - .expect(200); - }) - .then((response) => { - validateAgainstJSONSchema('DELETE', path, 200, response.body); - return Site.withUsers(currentSite.id); - }) - .then((fetchedSite) => { - expect(fetchedSite.Users).to.be.an('array'); - expect(fetchedSite.Users.length).to.equal(1); - done(); - }) - .catch(done); - }); - - it('should respond with a 400 when deleting the final user', (done) => { - const userPromise = factory.user(); - - Promise.props({ - user: userPromise, - site: factory.site({ - users: Promise.all([userPromise]), - }), - cookie: authenticatedSession(userPromise), - }) - .then(({ user, site, cookie }) => - request(app) - .delete(requestPath(site.id, user.id)) - .set('x-csrf-token', csrfToken.getToken()) - .set('Cookie', cookie) - .expect(400), - ) - .then((response) => { - validateAgainstJSONSchema('DELETE', path, 400, response.body); - expect(response.body.message).to.equal(siteErrors.USER_REQUIRED); - done(); - }) - .catch(done); - }); - - it('should respond with a 404 when the user to delete is not found', (done) => { - const userPromise = factory.user(); - const otherUser = factory.user(); - - nock.cleanAll(); - - Promise.props({ - site: factory.site({ - users: Promise.all([userPromise, otherUser]), - }), - cookie: authenticatedSession(userPromise), - }) - .then((models) => { - githubAPINocks.repo({ - owner: 'james', - repository: models.site.repo, - response: [ - 200, - { - permissions: { - admin: true, - push: true, - }, - }, - ], - }); - - return request(app) - .delete(requestPath(models.site.id, 100000)) - .set('x-csrf-token', csrfToken.getToken()) - .set('Cookie', models.cookie) - .expect(404); - }) - .then((response) => { - validateAgainstJSONSchema('DELETE', path, 404, response.body); - expect(response.body.message).to.equal(siteErrors.NO_ASSOCIATED_USER); - done(); - }) - .catch(done); - }); - - it('should respond with a 400 when any user attempts to remove the owner', (done) => { - const ownerName = 'owner'; - const ownerUser = factory.user({ - username: ownerName, - }); - const normalUser = factory.user({ - username: 'not-owner', - }); - - const siteProps = { - owner: ownerName, - users: Promise.all([ownerUser, normalUser]), - }; - - nock.cleanAll(); - - Promise.props({ - user: ownerUser, - site: factory.site(siteProps), - cookie: authenticatedSession(normalUser), - }) - .then(({ user, site, cookie }) => { - githubAPINocks.repo({ - owner: ownerName, - repository: site.repo, - response: [ - 200, - { - permissions: { - admin: true, - push: true, - }, - }, - ], - }); - - return request(app) - .delete(requestPath(site.id, user.id)) - .set('x-csrf-token', csrfToken.getToken()) - .set('Cookie', cookie) - .expect(400); - }) - .then((response) => { - validateAgainstJSONSchema('DELETE', path, 400, response.body); - expect(response.body.message).to.equal(siteErrors.OWNER_REMOVE); - done(); - }) - .catch(done); - }); - - it(`should respond with a 400 - when the site owner attempts to remove themselves`, (done) => { - const username = 'a-user'; - const userPromise = factory.user({ - username, - }); - const anotherUser = factory.user(); - const siteProps = { - owner: username, - users: Promise.all([userPromise, anotherUser]), - }; - - nock.cleanAll(); - - Promise.props({ - user: userPromise, - site: factory.site(siteProps), - cookie: authenticatedSession(userPromise), - }) - .then(({ user, site, cookie }) => { - githubAPINocks.repo({ - owner: username, - repository: site.repo, - response: [ - 200, - { - permissions: { - admin: true, - push: true, - }, - }, - ], - }); - - return request(app) - .delete(`/v0/site/${site.id}/user/${user.id}`) - .set('x-csrf-token', csrfToken.getToken()) - .set('Cookie', cookie) - .expect(400); - }) - .then((response) => { - validateAgainstJSONSchema('DELETE', path, 400, response.body); - expect(response.body.message).to.equal(siteErrors.OWNER_REMOVE); - done(); - }) - .catch(done); - }); - - it('should respond with a 400 if the user does not have admin access', (done) => { - const username = 'jane'; - const userA = factory.user(); - const userB = factory.user(); - const repo = 'whatever'; - const siteProps = { - owner: username, - repository: repo, - users: Promise.all([userA, userB]), - }; - - nock.cleanAll(); - - Promise.props({ - user: userB, - site: factory.site(siteProps), - cookie: authenticatedSession(userA), - }) - .then(({ user, site, cookie }) => { - githubAPINocks.repo({ - owner: site.username, - repository: site.repo, - response: [ - 200, - { - permissions: { - admin: false, - push: false, - }, - }, - ], - }); - - return request(app) - .delete(`/v0/site/${site.id}/user/${user.id}`) - .set('x-csrf-token', csrfToken.getToken()) - .set('Cookie', cookie) - .expect(400); - }) - .then((response) => { - validateAgainstJSONSchema('DELETE', path, 400, response.body); - expect(response.body.message).to.equal(siteErrors.ADMIN_ACCESS_REQUIRED); - done(); - }) - .catch(done); - }); - - it(`should allow a user to remove themselves even - if they are not a repo write user`, (done) => { - const username = 'jane'; - const userA = factory.user(); - const userB = factory.user(); - const repo = 'whatever'; - const siteProps = { - owner: username, - repository: repo, - users: Promise.all([userA, userB]), - }; - let currentSite; - - nock.cleanAll(); - - Promise.props({ - user: userA, - site: factory.site(siteProps), - cookie: authenticatedSession(userA), - }) - .then(({ user, site, cookie }) => { - currentSite = site; - githubAPINocks.repo({ - owner: site.username, - repository: site.repo, - response: [ - 200, - { - permissions: { - admin: false, - push: false, - }, - }, - ], - }); - - return request(app) - .delete(requestPath(site.id, user.id)) - .set('x-csrf-token', csrfToken.getToken()) - .set('Cookie', cookie) - .expect(200); - }) - .then((response) => { - validateAgainstJSONSchema('DELETE', path, 200, response.body); - return Site.withUsers(currentSite.id); - }) - .then((fetchedSite) => { - expect(fetchedSite.Users).to.be.an('array'); - expect(fetchedSite.Users.length).to.equal(1); - done(); - }) - .catch(done); - }); - }); - - describe('DELETE /v0/site/:id', () => { - let queueDestroySiteInfra; - - beforeEach(() => { - queueDestroySiteInfra = sinon - .stub(SiteDestroyer, 'queueDestroySiteInfra') - .resolves(); - }); - - afterEach(() => { - sinon.restore(); - }); - - it('should require authentication', (done) => { - let site; - - factory - .site() - .then((model) => { - site = model; - nock.cleanAll(); - githubAPINocks.repo({ - owner: site.owner, - repository: site.repo, - response: [ - 200, - { - permissions: { - admin: true, - push: true, - }, - }, - ], - }); - return unauthenticatedSession(); - }) - .then((cookie) => - request(app) - .delete(`/v0/site/${site.id}`) - .set('x-csrf-token', csrfToken.getToken()) - .set('Cookie', cookie) - .expect(403), - ) - .then((response) => { - validateAgainstJSONSchema('DELETE', '/site/{id}', 403, response.body); - expect(response.body.message).to.equal(authErrorMessage); - done(); - }) - .catch(done); - }); - - it('should require a valid csrf token', (done) => { - let site; - - factory - .site() - .then((model) => { - site = model; - return authenticatedSession(); - }) - .then((cookie) => - request(app) - .delete(`/v0/site/${site.id}`) - .set('x-csrf-token', 'bad-token') - .set('Cookie', cookie) - .expect(403), - ) - .then((response) => { - validateAgainstJSONSchema('PUT', '/site/{id}', 403, response.body); - expect(response.body.message).to.equal('Invalid CSRF token'); - done(); - }) - .catch(done); - }); - - it('should allow a user to delete a site associated with their account', (done) => { - let site; - - factory - .site() - .then((s) => - Site.findByPk(s.id, { - include: [User], - }), - ) - .then((model) => { - site = model; - nock.cleanAll(); - githubAPINocks.repo({ - owner: site.owner, - repository: site.repo, - response: [ - 200, - { - permissions: { - admin: true, - push: true, - }, - }, - ], - }); - return authenticatedSession(site.Users[0]); - }) - .then((cookie) => - request(app) - .delete(`/v0/site/${site.id}`) - .set('x-csrf-token', csrfToken.getToken()) - .set('Cookie', cookie) - .expect(200), - ) - .then((response) => { - expect(response.body).to.deep.eq({}); - return Site.findAll({ - where: { - id: site.id, - }, - }); - }) - .then((sites) => { - expect(sites).to.be.empty; - done(); - }) - .catch(done); + expect(response.body).to.deep.eq({}); + const sites = await Site.findAll({ + where: { + id: site.id, + }, + }); + expect(sites).to.be.empty; }); it('does not destroy the site when the site has a domain', async () => { nock.cleanAll(); - const user = await factory.user(); - const site = await factory.site({ - users: [user], - }); + const { site, user } = await createSiteUserOrg(); const domain = await factory.domain.create({ siteId: site.id }); githubAPINocks.repo({ @@ -1618,127 +831,28 @@ describe('Site API', () => { }); expect(site.isSoftDeleted()).to.be.false; expect(response.body.message).to.have.string(domain.names); - }); - - it(`should not allow a user to delete a site - not associated with their account`, (done) => { - let site; - - factory - .site() - .then((s) => Site.findByPk(s.id)) - .then((model) => { - site = model; - return authenticatedSession(factory.user()); - }) - .then((cookie) => - request(app) - .delete(`/v0/site/${site.id}`) - .set('x-csrf-token', csrfToken.getToken()) - .set('Cookie', cookie) - .expect(404), - ) - .then((response) => { - validateAgainstJSONSchema('DELETE', '/site/{id}', 404, response.body); - return Site.findAll({ - where: { - id: site.id, - }, - }); - }) - .then((sites) => { - expect(sites).not.to.be.empty; - done(); - }) - .catch(done); - }); - - it("should plan to remove all of the site's data from S3", (done) => { - let site; - const userPromise = factory.user(); - const sitePromise = factory.site({ - users: Promise.all([userPromise]), - }); - const sessionPromise = authenticatedSession(userPromise); - - Promise.props({ - user: userPromise, - site: sitePromise, - cookie: sessionPromise, - }) - .then((results) => { - ({ site } = results); - nock.cleanAll(); - githubAPINocks.repo({ - owner: site.owner, - repository: site.repo, - response: [ - 200, - { - permissions: { - admin: true, - push: true, - }, - }, - ], - }); - - return request(app) - .delete(`/v0/site/${site.id}`) - .set('x-csrf-token', csrfToken.getToken()) - .set('Cookie', results.cookie) - .expect(200); - }) - .then(() => { - sinon.assert.calledOnce(queueDestroySiteInfra); - expect(queueDestroySiteInfra.firstCall.args[0].id).to.eq(site.id); - done(); - }) - .catch(done); - }); - - it(`should not allow a user to delete a site - associated with their account if not admin`, (done) => { - let site; - - factory - .site() - .then((s) => - Site.findByPk(s.id, { - include: [User], - }), - ) - .then((model) => { - site = model; - nock.cleanAll(); - githubAPINocks.repo({ - owner: site.owner, - repository: site.repo, - response: [ - 200, - { - permissions: { - admin: false, - push: true, - }, - }, - ], - }); + }); + + it(`should not allow a user to delete a site + not associated with their account`, (done) => { + let site; - return authenticatedSession(site.Users[0]); + factory + .site() + .then((s) => Site.findByPk(s.id)) + .then((model) => { + site = model; + return authenticatedSession(factory.user()); }) .then((cookie) => request(app) .delete(`/v0/site/${site.id}`) .set('x-csrf-token', csrfToken.getToken()) .set('Cookie', cookie) - .expect(403), + .expect(404), ) .then((response) => { - validateAgainstJSONSchema('DELETE', '/site/{id}', 403, response.body); - expect(response.body.message).to.equal( - 'You do not have administrative access to this repository', - ); + validateAgainstJSONSchema('DELETE', '/site/{id}', 404, response.body); return Site.findAll({ where: { id: site.id, @@ -1746,11 +860,79 @@ describe('Site API', () => { }); }) .then((sites) => { - expect(sites).to.not.be.empty; + expect(sites).not.to.be.empty; done(); }) .catch(done); }); + + it("should plan to remove all of the site's data from S3", async () => { + const { site, user } = await createSiteUserOrg(); + const cookie = await authenticatedSession(user); + + nock.cleanAll(); + githubAPINocks.repo({ + owner: site.owner, + repository: site.repo, + response: [ + 200, + { + permissions: { + admin: true, + push: true, + }, + }, + ], + }); + + await request(app) + .delete(`/v0/site/${site.id}`) + .set('x-csrf-token', csrfToken.getToken()) + .set('Cookie', cookie) + .expect(200); + + sinon.assert.calledOnce(queueDestroySiteInfra); + expect(queueDestroySiteInfra.firstCall.args[0].id).to.eq(site.id); + }); + + it(`should not allow a user to delete a site + associated with their account if not admin`, async () => { + const { site, user } = await createSiteUserOrg(); + + nock.cleanAll(); + githubAPINocks.repo({ + owner: site.owner, + repository: site.repo, + response: [ + 200, + { + permissions: { + admin: false, + push: true, + }, + }, + ], + }); + + const cookie = await authenticatedSession(user); + const response = await request(app) + .delete(`/v0/site/${site.id}`) + .set('x-csrf-token', csrfToken.getToken()) + .set('Cookie', cookie) + .expect(403); + + validateAgainstJSONSchema('DELETE', '/site/{id}', 403, response.body); + expect(response.body.message).to.equal( + 'You do not have administrative access to this repository', + ); + const sites = await Site.findAll({ + where: { + id: site.id, + }, + }); + + expect(sites).to.not.be.empty; + }); }); describe('PUT /v0/site/:id', () => { @@ -1841,150 +1023,29 @@ describe('Site API', () => { .catch(done); }); - it('should trigger a rebuild of the site', () => { - let siteModel; - factory - .site({ - repository: 'old-repo-name', - }) - .then((site) => - Site.findByPk(site.id, { - include: [User, Build], - }), - ) - .then((model) => { - siteModel = model; - expect(siteModel.Builds).to.have.length(0); - return authenticatedSession(siteModel.Users[0]); - }) - .then((cookie) => - request(app) - .put(`/v0/site/${siteModel.id}`) - .set('x-csrf-token', csrfToken.getToken()) - .send({ - repository: 'new-repo-name', - }) - .set('Cookie', cookie) - .expect(200), - ); - }); - - it('should trigger a rebuild of the demo branch if one is present', () => { - let siteModel; - factory - .site({ - repository: 'old-repo-name', - demoBranch: 'demo', - demoDomain: 'https://demo.example.gov', - }) - .then((site) => - Site.findByPk(site.id, { - include: [User, Build], - }), - ) - .then((model) => { - siteModel = model; - expect(siteModel.Builds).to.have.length(0); - return authenticatedSession(siteModel.Users[0]); - }) - .then((cookie) => - request(app) - .put(`/v0/site/${siteModel.id}`) - .set('x-csrf-token', csrfToken.getToken()) - .send({ - repository: 'new-repo-name', - }) - .set('Cookie', cookie) - .expect(200), - ); - }); - - it(`should not override existing attributes - if they are not present in the request body`, (done) => { - let site; - const userPromise = factory.user(); - const sitePromise = factory.site({ - users: Promise.all([userPromise]), - domain: 'https://example.com', - }); - const cookiePromise = authenticatedSession(userPromise); - - Promise.props({ - user: userPromise, - site: sitePromise, - cookie: cookiePromise, - }) - .then((results) => { - ({ site } = results); + it('should update engine params', async () => { + const { site, user } = await createSiteUserOrg(); + await site.update({ engine: 'jekyll' }); + const cookie = await authenticatedSession(user); - return request(app) - .put(`/v0/site/${site.id}`) - .set('x-csrf-token', csrfToken.getToken()) - .send({ - notAValue: 'new: true', - }) - .set('Cookie', results.cookie) - .expect(200); - }) - .then((response) => { - validateAgainstJSONSchema('PUT', '/site/{id}', 200, response.body); - return Site.findByPk(site.id); - }) - .then((foundSite) => { - expect(foundSite.domain).to.equal('https://example.com'); - done(); + const response = await request(app) + .put(`/v0/site/${site.id}`) + .set('x-csrf-token', csrfToken.getToken()) + .send({ + engine: 'hugo', }) - .catch(done); - }); - - it(`should ignore domain URLs in the request body - if the URLs are managed by an associated domain`, (done) => { - let site; - const userPromise = factory.user(); - const sitePromise = factory.site({ - users: Promise.all([userPromise]), - domain: 'https://example.com', - demoDomain: 'https://demo.example.com', - }); - const cookiePromise = authenticatedSession(userPromise); - sinon.stub(DomainService, 'isSiteUrlManagedByDomain').returns(true); + .set('Cookie', cookie) + .expect(200); - Promise.props({ - user: userPromise, - site: sitePromise, - cookie: cookiePromise, - }) - .then((results) => { - ({ site } = results); + validateAgainstJSONSchema('PUT', '/site/{id}', 200, response.body); + const foundSite = await Site.findByPk(site.id); - return request(app) - .put(`/v0/site/${site.id}`) - .set('x-csrf-token', csrfToken.getToken()) - .send({ - domain: 'https://changed.example.gov', - demoDomain: 'https://new.example.gov', - }) - .set('Cookie', results.cookie) - .expect(200); - }) - .then((response) => { - validateAgainstJSONSchema('PUT', '/site/{id}', 200, response.body); - return Site.findByPk(site.id); - }) - .then((foundSite) => { - expect(foundSite.domain).to.equal('https://example.com'); - expect(foundSite.demoDomain).to.equal('https://demo.example.com'); - done(); - }) - .catch(done); + expect(foundSite).to.have.property('engine', 'hugo'); }); it('should ignore non-engine params', async () => { - const user = await factory.user(); - const site = await factory.site({ - users: [user], - repository: 'original', - }); + const { site, user } = await createSiteUserOrg(); + await site.update({ repository: 'original' }); const cookie = await authenticatedSession(user); const response = await request(app) @@ -2003,309 +1064,99 @@ describe('Site API', () => { }); }); - describe('Site basic authentication API', () => { - describe('DELETE /v0/site/:site_id/basic-auth', () => { - describe('when the user is not authenticated', () => { - it('returns a 403', async () => { - const siteId = 1; - - const { body } = await request(app) - .delete(`/v0/site/${siteId}/basic-auth`) - .expect(403); - - validateAgainstJSONSchema('DELETE', '/site/{site_id}/basic-auth', 403, body); - }); - }); - - describe('when the site does not exist', () => { - it('returns a 404', async () => { - const siteId = 1; - const user = await factory.user(); - const cookie = await authenticatedSession(user); - - const { body } = await request(app) - .delete(`/v0/site/${siteId}/basic-auth`) - .set('Cookie', cookie) - .set('x-csrf-token', csrfToken.getToken()) - .type('json') - .expect(404); - - validateAgainstJSONSchema('DELETE', '/site/{site_id}/basic-auth', 404, body); - }); - }); - - describe('when the user is not authorized to see the site', () => { - it('returns a 404', async () => { - const [site, user] = await Promise.all([factory.site(), factory.user()]); - const cookie = await authenticatedSession(user); - - const { body } = await request(app) - .delete(`/v0/site/${site.id}/basic-auth`) - .set('Cookie', cookie) - .set('x-csrf-token', csrfToken.getToken()) - .type('json') - .expect(404); - - validateAgainstJSONSchema('DELETE', '/site/{site_id}/basic-auth', 404, body); - }); - }); + describe('GET /v0/site/:site_id/domains', () => { + it('should require authentication', async () => { + const siteId = 1; + const response = await request(app).get(`/v0/site/${siteId}/domains`).expect(403); - describe('when the parameters are valid', () => { - it('deletes basic auth from config and returns a 200', async () => { - const userPromise = await factory.user(); - const siteConfig = { - basicAuth: { - username: 'user', - password: 'password', - }, - blah: 'blahblah', - }; - let site = await factory.site({ - users: [userPromise], - config: siteConfig, - }); - - const cookie = await authenticatedSession(userPromise); - expect(site.config).to.deep.eq(siteConfig); - await request(app) - .delete(`/v0/site/${site.id}/basic-auth`) - .set('Cookie', cookie) - .set('x-csrf-token', csrfToken.getToken()) - .type('json') - .expect(200); - - site = await site.reload(); - expect(site.config).to.deep.equal({ - basicAuth: {}, - blah: 'blahblah', - }); - }); - }); + validateAgainstJSONSchema('GET', '/site/{site_id}/domains', 403, response.body); + expect(response.body.message).to.equal(authErrorMessage); }); - describe('POST /v0/site/:site_id/basic-auth', () => { - describe('when the user is not authenticated', () => { - it('returns a 403', async () => { - const siteId = 1; - - const { body } = await request(app) - .post(`/v0/site/${siteId}/basic-auth`) - .type('json') - .expect(403); - - validateAgainstJSONSchema('POST', '/site/{site_id}/basic-auth', 403, body); - }); - }); - - describe('when there is no csrf token', () => { - it('returns a 403', async () => { - const siteId = 1; - const user = await factory.user(); - const cookie = await authenticatedSession(user); - - const { body } = await request(app) - .post(`/v0/site/${siteId}/basic-auth`) - .set('Cookie', cookie) - .type('json') - .expect(403); - - validateAgainstJSONSchema('POST', '/site/{site_id}/basic-auth', 403, body); - }); - }); + it('should return 404 not found with a site that does not exist', async () => { + const siteId = 8675309; + const user = await factory.user(); - describe('when the site does not exist', () => { - it('returns a 404', async () => { - const siteId = 1; - const user = await factory.user(); - const cookie = await authenticatedSession(user); + const cookie = await authenticatedSession(user); + const response = await request(app) + .get(`/v0/site/${siteId}/domains`) + .set('Cookie', cookie) + .expect(404); - const { body } = await request(app) - .post(`/v0/site/${siteId}/basic-auth`) - .set('Cookie', cookie) - .set('x-csrf-token', csrfToken.getToken()) - .type('json') - .expect(404); + validateAgainstJSONSchema('GET', '/site/{site_id}/domains', 404, response.body); + }); - validateAgainstJSONSchema('POST', '/site/{site_id}/basic-auth', 404, body); - }); + it('should render a list of domains associated with a site', async () => { + const site = await factory.site({ + demoBranch: 'demo', }); + const { user } = await createSiteUserOrg({ site }); - describe('when the user is not authorized to see the site', () => { - it('returns a 404', async () => { - const [site, user] = await Promise.all([factory.site(), factory.user()]); - const cookie = await authenticatedSession(user); - - const { body } = await request(app) - .post(`/v0/site/${site.id}/basic-auth`) - .set('Cookie', cookie) - .set('x-csrf-token', csrfToken.getToken()) - .type('json') - .expect(404); - - validateAgainstJSONSchema('POST', '/site/{site_id}/basic-auth', 404, body); - }); + const domain1 = await factory.domain.create({ + siteId: site.id, + context: 'site', }); - - describe('when the parameters are not valid', () => { - it('returns a 400', async () => { - const userPromise = await factory.user(); - const site = await factory.site({ - users: [userPromise], - }); - const cookie = await authenticatedSession(userPromise); - const credentials = { - // invalid password - username: 'user', - password: 'password', - }; - - const { body } = await request(app) - .post(`/v0/site/${site.id}/basic-auth`) - .set('Cookie', cookie) - .set('x-csrf-token', csrfToken.getToken()) - .type('json') - .send(credentials) - .expect(400); - - validateAgainstJSONSchema('POST', '/site/{site_id}/basic-auth', 400, body); - }); + const domain2 = await factory.domain.create({ + siteId: site.id, + context: 'demo', }); + const cookie = await authenticatedSession(user); + const response = await request(app) + .get(`/v0/site/${site.id}/domains`) + .set('Cookie', cookie) + .expect(200); - describe('when the parameters are valid', () => { - it('sets username and password for basic authentication', async () => { - const userPromise = await factory.user(); - const site = await factory.site({ - users: [userPromise], - config: { - blah: 'blahblahblah', - }, - }); - const cookie = await authenticatedSession(userPromise); - const credentials = { - username: 'user', - password: 'paSsw0rd', - }; - - const { body } = await request(app) - .post(`/v0/site/${site.id}/basic-auth`) - .set('Cookie', cookie) - .set('x-csrf-token', csrfToken.getToken()) - .type('json') - .send(credentials) - .expect(200); - - validateAgainstJSONSchema('POST', '/site/{site_id}/basic-auth', 200, body); - await site.reload(); - expect(site.config).to.deep.equal({ - basicAuth: credentials, - blah: 'blahblahblah', - }); - }); + validateAgainstJSONSchema('GET', '/site/{site_id}/domains', 200, response.body); + expect(response.body).to.be.a('array'); + expect(response.body).to.have.length(2); + response.body.map((record) => { + const foundDomains = [domain1, domain2].find((domain) => record.id === domain.id); + expect(foundDomains).not.to.be.undefined; }); }); - describe('GET /v0/site/:site_id/domains', () => { - it('should require authentication', async () => { - const siteId = 1; - const response = await request(app).get(`/v0/site/${siteId}/domains`).expect(403); - - validateAgainstJSONSchema('GET', '/site/{site_id}/domains', 403, response.body); - expect(response.body.message).to.equal(authErrorMessage); - }); - - it('should return 404 not found with a site that does not exist', async () => { - const siteId = 8675309; - const user = await factory.user(); - - const cookie = await authenticatedSession(user); - const response = await request(app) - .get(`/v0/site/${siteId}/domains`) - .set('Cookie', cookie) - .expect(404); - - validateAgainstJSONSchema('GET', '/site/{site_id}/domains', 404, response.body); - }); + it(`should return an empty list + when no domains are associated with a site`, async () => { + const { site, user } = await createSiteUserOrg(); + const cookie = await authenticatedSession(user); + const response = await request(app) + .get(`/v0/site/${site.id}/domains`) + .set('Cookie', cookie) + .expect(200); - it('should render a list of domains associated with a site', async () => { - const user = await factory.user(); - const site = await factory.site({ - users: [user.id], - demoBranch: 'demo', - }); - const domain1 = await factory.domain.create({ - siteId: site.id, - context: 'site', - }); - const domain2 = await factory.domain.create({ - siteId: site.id, - context: 'demo', - }); - const cookie = await authenticatedSession(user); - const response = await request(app) - .get(`/v0/site/${site.id}/domains`) - .set('Cookie', cookie) - .expect(200); - - validateAgainstJSONSchema('GET', '/site/{site_id}/domains', 200, response.body); - expect(response.body).to.be.a('array'); - expect(response.body).to.have.length(2); - response.body.map((record) => { - const foundDomains = [domain1, domain2].find( - (domain) => record.id === domain.id, - ); - expect(foundDomains).not.to.be.undefined; - }); - }); + validateAgainstJSONSchema('GET', '/site/{site_id}/domains', 200, response.body); + expect(response.body).to.be.a('array'); + expect(response.body).to.have.length(0); + }); + }); - it(`should return an empty list - when no domains are associated with a site`, async () => { - const user = await factory.user(); - const site = await factory.site({ - users: [user.id], - }); - const cookie = await authenticatedSession(user); - const response = await request(app) - .get(`/v0/site/${site.id}/domains`) - .set('Cookie', cookie) - .expect(200); - - validateAgainstJSONSchema('GET', '/site/{site_id}/domains', 200, response.body); - expect(response.body).to.be.a('array'); - expect(response.body).to.have.length(0); + describe('GET /v0/site/:site_id/tasks', () => { + it('should return tasks for a site', async () => { + const { user, site } = await createSiteUserOrg(); + const build = await factory.build({ site }); + const buildTaskType = await factory.buildTaskType(); + const buildTask = await factory.buildTask({ + buildTaskType, + build, }); - }); - describe('GET /v0/site/:site_id/tasks', () => { - it('should return tasks for a site', async () => { - const user = await factory.user(); - const site = await factory.site({ - users: [user.id], - }); - const build = await factory.build({ site }); - const buildTaskType = await factory.buildTaskType(); - const buildTask = await factory.buildTask({ - buildTaskType, - build, - }); + const cookie = await authenticatedSession(user); + const response = await request(app) + .get(`/v0/site/${site.id}/tasks`) + .set('Cookie', cookie) + .expect(200); - const cookie = await authenticatedSession(user); - const response = await request(app) - .get(`/v0/site/${site.id}/tasks`) - .set('Cookie', cookie) - .expect(200); - - // TODO: update this validation for artifact (model is string, response is obj) - // validateAgainstJSONSchema( - // 'GET', - // '/site/{site_id}/tasks', - // 200, - // response.body - // ); - - expect(response.body).to.be.a('array'); - expect(response.body).to.have.length(1); - expect(response.body[0]).to.have.property('id', buildTask.id); - }); + // TODO: update this validation for artifact (model is string, response is obj) + // validateAgainstJSONSchema( + // 'GET', + // '/site/{site_id}/tasks', + // 200, + // response.body + // ); + + expect(response.body).to.be.a('array'); + expect(response.body).to.have.length(1); + expect(response.body[0]).to.have.property('id', buildTask.id); }); }); }); diff --git a/test/api/requests/user-action.test.js b/test/api/requests/user-action.test.js index fe62ed514..02bc86719 100644 --- a/test/api/requests/user-action.test.js +++ b/test/api/requests/user-action.test.js @@ -5,6 +5,7 @@ const { authenticatedSession } = require('../support/session'); const validateAgainstJSONSchema = require('../support/validateAgainstJSONSchema'); const factory = require('../support/factory'); const userActionFactory = require('../support/factory/user-action'); +const { createSiteUserOrg } = require('../support/site-user'); const version = '/v0'; const resource = 'site'; @@ -69,51 +70,32 @@ describe('UserAction API', () => { .catch(done); }); - it('returns a list of user actions associated with a site', (done) => { + it('returns a list of user actions associated with a site', async () => { const userActionCount = 3; - let currentUser; - let siteId; - factory - .user() - .then((user) => { - currentUser = user; - return userActionFactory.buildMany(userActionCount, { user }); - }) - .then((userActions) => { - siteId = userActions[0].siteId; - return buildAuthenticatedSession(currentUser); - }) - .then((cookie) => - makeGetRequest(200, { - id: siteId, - cookie, - }), - ) - .then((response) => { - const { body } = response; - - expect(body.length).to.equal(userActionCount); - - let lastCreatedAt = new Date().toISOString(); - - body.forEach((action) => { - expect(action.actionTarget).to.have.all.keys( - 'id', - 'username', - 'email', - 'createdAt', - ); - expect(action.actionType).to.have.all.keys('action'); - - // make sure they are in descending order by createdAt - expect(new Date(action.createdAt)).to.be.lte(new Date(lastCreatedAt)); - lastCreatedAt = action.createdAt; - }); - - done(); - }) - .catch(done); + const { site, user } = await createSiteUserOrg(); + + await userActionFactory.buildMany(userActionCount, { user, site }); + const cookie = await buildAuthenticatedSession(user); + + const { body } = await makeGetRequest(200, { + id: site.id, + cookie, + }); + expect(body.length).to.equal(userActionCount); + + body.reduce((lastCreatedAt, action) => { + expect(action.actionTarget).to.have.all.keys( + 'id', + 'username', + 'email', + 'createdAt', + ); + expect(action.actionType).to.have.all.keys('action'); + // make sure they are in descending order by createdAt + expect(new Date(action.createdAt)).to.be.lte(new Date(lastCreatedAt)); + return new Date(action.createdAt); + }, new Date().toISOString()); }); }); }); diff --git a/test/api/requests/user-environment-variable.test.js b/test/api/requests/user-environment-variable.test.js index 8c884d73b..1856bf23e 100644 --- a/test/api/requests/user-environment-variable.test.js +++ b/test/api/requests/user-environment-variable.test.js @@ -7,8 +7,9 @@ const { authenticatedSession } = require('../support/session'); const validateAgainstJSONSchema = require('../support/validateAgainstJSONSchema'); const app = require('../../../app'); const config = require('../../../config'); -const { Role, UserEnvironmentVariable } = require('../../../api/models'); +const { UserEnvironmentVariable } = require('../../../api/models'); const EventCreator = require('../../../api/services/EventCreator'); +const { createSiteUserOrg } = require('../support/site-user'); describe('User Environment Variable API', () => { beforeEach(() => { @@ -86,11 +87,7 @@ describe('User Environment Variable API', () => { describe('when the user environment variable does not exist', () => { it('returns a 404', async () => { const uevId = 1; - const userPromise = factory.user(); - const site = await factory.site({ - users: Promise.all([userPromise]), - }); - const user = await userPromise; + const { site, user } = await createSiteUserOrg(); const cookie = await authenticatedSession(user); const { body } = await request(app) @@ -110,45 +107,8 @@ describe('User Environment Variable API', () => { }); describe('when the parameters are valid', () => { - it('deletes the uev and returns a 200', async () => { - const userPromise = factory.user(); - const site = await factory.site({ - users: Promise.all([userPromise]), - }); - const [uev, user] = await Promise.all([ - factory.userEnvironmentVariable.create({ site }), - userPromise, - ]); - const cookie = await authenticatedSession(user); - - const beforeNumUEVs = await UserEnvironmentVariable.count(); - - await request(app) - .delete(`/v0/site/${site.id}/user-environment-variable/${uev.id}`) - .set('Cookie', cookie) - .set('x-csrf-token', csrfToken.getToken()) - .type('json') - .expect(200); - - const afterNumUEVs = await UserEnvironmentVariable.count(); - expect(afterNumUEVs).to.eq(beforeNumUEVs - 1); - }); - it('allows an org user to delete the uev and returns a 200', async () => { - const org = await factory.organization.create(); - const role = await Role.findOne({ - where: { - name: 'user', - }, - }); - const user = await factory.user(); - const site = await factory.site(); - await org.addUser(user, { - through: { - roleId: role.id, - }, - }); - await org.addSite(site); + const { site, user } = await createSiteUserOrg(); const uevs = await Promise.all([ factory.userEnvironmentVariable.create({ @@ -240,11 +200,7 @@ describe('User Environment Variable API', () => { describe('when there are no user environment variables for the site', () => { it('returns an empty array', async () => { - const userPromise = factory.user(); - const site = await factory.site({ - users: Promise.all([userPromise]), - }); - const user = await userPromise; + const { site, user } = await createSiteUserOrg(); const cookie = await authenticatedSession(user); const { body } = await request(app) @@ -265,10 +221,8 @@ describe('User Environment Variable API', () => { describe('when there are user environment variables for the site', () => { it('returns an array containing only the uevs for the site', async () => { - const userPromise = factory.user(); - const site = await factory.site({ - users: Promise.all([userPromise]), - }); + const { site, user } = await createSiteUserOrg(); + const { site: site2 } = await createSiteUserOrg({ user }); const uevs = await Promise.all([ factory.userEnvironmentVariable.create({ site, @@ -284,67 +238,7 @@ describe('User Environment Variable API', () => { factory.userEnvironmentVariable.create(), // Same user, different site factory.userEnvironmentVariable.create({ - site: await factory.site({ - users: Promise.all([userPromise]), - }), - }), - ]); - - const user = await userPromise; - const cookie = await authenticatedSession(user); - - const { body } = await request(app) - .get(`/v0/site/${site.id}/user-environment-variable`) - .set('Cookie', cookie) - .type('json') - .expect(200); - - validateAgainstJSONSchema( - 'GET', - '/site/{site_id}/user-environment-variable', - 200, - body, - ); - expect(body).to.have.length(uevs.length); - expect(body.map((uev) => uev.id)).to.have.members(uevs.map((uev) => uev.id)); - }); - - it(`returns an array containing only the uevs for a site - for an org user in the site org`, async () => { - const org = await factory.organization.create(); - const role = await Role.findOne({ - where: { - name: 'user', - }, - }); - const user = await factory.user(); - const site = await factory.site(); - await org.addUser(user, { - through: { - roleId: role.id, - }, - }); - await org.addSite(site); - - const uevs = await Promise.all([ - factory.userEnvironmentVariable.create({ - site, - name: 'foo', - }), - factory.userEnvironmentVariable.create({ - site, - name: 'bar', - }), - ]); - - await Promise.all([ - // Different user and site - factory.userEnvironmentVariable.create(), - // Same user, different site - factory.userEnvironmentVariable.create({ - site: await factory.site({ - users: Promise.all([user]), - }), + site: site2, }), ]); @@ -453,11 +347,7 @@ describe('User Environment Variable API', () => { describe('when the parameters are not valid', () => { it('returns a 400', async () => { - const userPromise = factory.user(); - const site = await factory.site({ - users: Promise.all([userPromise]), - }); - const user = await userPromise; + const { site, user } = await createSiteUserOrg(); const cookie = await authenticatedSession(user); const { body } = await request(app) @@ -479,11 +369,7 @@ describe('User Environment Variable API', () => { describe('when the name already exists', () => { it('returns a 400', async () => { - const userPromise = factory.user(); - const site = await factory.site({ - users: Promise.all([userPromise]), - }); - const user = await userPromise; + const { site, user } = await createSiteUserOrg(); const name = 'my-env-var'; await factory.userEnvironmentVariable.create({ name, @@ -529,11 +415,7 @@ describe('User Environment Variable API', () => { }); it('cannot encrypt without key and returns a 500', async () => { - const userPromise = factory.user(); - const site = await factory.site({ - users: Promise.all([userPromise]), - }); - const user = await userPromise; + const { site, user } = await createSiteUserOrg(); const cookie = await authenticatedSession(user); const name = 'my-env-var'; const value = 'secret1234'; @@ -553,57 +435,7 @@ describe('User Environment Variable API', () => { describe('when the parameters are valid', () => { it('creates and returns the uev', async () => { - const userPromise = factory.user(); - const site = await factory.site({ - users: Promise.all([userPromise]), - }); - const user = await userPromise; - const cookie = await authenticatedSession(user); - const name = 'my-env-var'; - const value = 'secret1234'; - - const beforeNumUEVs = await UserEnvironmentVariable.count(); - - const { body } = await request(app) - .post(`/v0/site/${site.id}/user-environment-variable`) - .set('Cookie', cookie) - .set('x-csrf-token', csrfToken.getToken()) - .type('json') - .send({ - name, - value, - }) - .expect(200); - - const afterNumUEVs = await UserEnvironmentVariable.count(); - - validateAgainstJSONSchema( - 'POST', - '/site/{site_id}/user-environment-variable', - 200, - body, - ); - expect(body.name).to.eq(name); - expect(body.hint).to.eq(''); - expect(afterNumUEVs).to.eq(beforeNumUEVs + 1); - }); - - it('allows an org user to create and return the uev', async () => { - const org = await factory.organization.create(); - const role = await Role.findOne({ - where: { - name: 'user', - }, - }); - const user = await factory.user(); - const site = await factory.site(); - await org.addUser(user, { - through: { - roleId: role.id, - }, - }); - await org.addSite(site); - + const { site, user } = await createSiteUserOrg(); const cookie = await authenticatedSession(user); const name = 'my-env-var'; const value = 'secret1234'; diff --git a/test/api/requests/webhook.test.js b/test/api/requests/webhook.test.js index 4829a90cc..0cc8b7791 100644 --- a/test/api/requests/webhook.test.js +++ b/test/api/requests/webhook.test.js @@ -4,9 +4,8 @@ const request = require('supertest'); const app = require('../../../app'); const config = require('../../../config'); const factory = require('../support/factory'); -const { User } = require('../../../api/models'); +const { createSiteUserOrg } = require('../support/site-user'); const EventCreator = require('../../../api/services/EventCreator'); - const Webhooks = require('../../../api/services/Webhooks'); describe('Webhook API', () => { @@ -66,12 +65,9 @@ describe('Webhook API', () => { }); it('should respond with a 400 if the signature is invalid', async () => { - const site = await factory.site(); - await site.reload({ - include: [User], - }); + const { site, user } = await createSiteUserOrg(); - const payload = buildWebhookPayload(site.Users[0], site); + const payload = buildWebhookPayload(user, site); const signature = '123abc'; await request(app) @@ -89,10 +85,7 @@ describe('Webhook API', () => { context('should call `pushWebhookRequest` with the payload if ok', () => { it('site is in not in an organization', async () => { - const user = await factory.user(); - const site = await factory.site({ - users: [user], - }); + const { site, user } = await createSiteUserOrg(); const payload = buildWebhookPayload(user, site); const signature = signWebhookPayload(payload); @@ -111,10 +104,7 @@ describe('Webhook API', () => { }); it('site does not exist', async () => { - const user = await factory.user(); - const site = await factory.site({ - users: [user], - }); + const { site, user } = await createSiteUserOrg(); const payload = buildWebhookPayload(user, site); const signature = signWebhookPayload(payload); @@ -134,12 +124,7 @@ describe('Webhook API', () => { }); it('site is in an active organization', async () => { - const org = await factory.organization.create(); - const user = await factory.user(); - const site = await factory.site({ - users: [user], - organizationId: org.id, - }); + const { site, user } = await createSiteUserOrg(); const payload = buildWebhookPayload(user, site); const signature = signWebhookPayload(payload); @@ -158,12 +143,8 @@ describe('Webhook API', () => { }); it('site is in an inactive organization', async () => { - const org = await factory.organization.create(); - const user = await factory.user(); - const site = await factory.site({ - users: [user], - organizationId: org.id, - }); + const org = await factory.organization.create({ isActive: false }); + const { site, user } = await createSiteUserOrg({ org }); const payload = buildWebhookPayload(user, site); const signature = signWebhookPayload(payload); @@ -182,11 +163,10 @@ describe('Webhook API', () => { }); it('site is inactive', async () => { - const user = await factory.user(); const site = await factory.site({ - users: [user], isActive: false, }); + const { user } = await createSiteUserOrg({ site }); const payload = buildWebhookPayload(user, site); const signature = signWebhookPayload(payload); diff --git a/test/api/support/factory/site.js b/test/api/support/factory/site.js index 351678803..2eabc08e0 100644 --- a/test/api/support/factory/site.js +++ b/test/api/support/factory/site.js @@ -1,4 +1,3 @@ -const userFactory = require('./user'); const { Site, SiteBranchConfig } = require('../../../../api/models'); const { generateSubdomain } = require('../../../../api/utils'); @@ -14,12 +13,6 @@ function generateUniqueAtts() { } function makeAttributes(overrides = {}) { - let { users } = overrides; - - if (users === undefined) { - users = Promise.all([userFactory()]); - } - const { owner, repository } = generateUniqueAtts(); return { @@ -31,7 +24,6 @@ function makeAttributes(overrides = {}) { awsBucketRegion: 'us-gov-west-1', defaultBranch: 'main', subdomain: generateSubdomain(owner, repository), - users, ...overrides, }; } @@ -78,22 +70,16 @@ async function addSiteBranchConfigs(site) { function site(overrides, options = {}) { let site; - let users; return Promise.props(makeAttributes(overrides)) .then((attributes) => { - users = attributes.users.slice(); - delete attributes.users; - return Site.create(attributes); }) .then(async (siteModel) => { site = siteModel; - const userPromises = users.map((user) => site.addUser(user)); if (!options.noSiteBranchConfig) { await addSiteBranchConfigs(site); } - return Promise.all(userPromises); }) .then(() => Site.findByPk(site.id, { diff --git a/test/api/support/site-user.js b/test/api/support/site-user.js new file mode 100644 index 000000000..dcac554e5 --- /dev/null +++ b/test/api/support/site-user.js @@ -0,0 +1,36 @@ +const factory = require('./factory'); +// previously we tracked whether users could access a site based on a simple relationship +// from Site to User. Now we use Organizations. This helper provides an easier way to +// setup the necessary structures to quickly test our access patterns. + +// operators can provide existing user/org/site models as needed and the user will +// still be added to the org, and the site will have it's organization set to the +// organization +async function createSiteUserOrg({ user = null, org = null, site = null } = {}) { + if (!user) { + // eslint-disable-next-line no-param-reassign + user = await factory.user(); + } + + if (!org) { + // eslint-disable-next-line no-param-reassign + org = await factory.organization.create(); + } + + await org.addRoleUser(user); + + if (!site) { + // eslint-disable-next-line no-param-reassign + site = await factory.site({ + organizationId: org.id, + }); + } else { + site.update({ + organizationId: org.id, + }); + } + + return { site, user, org }; +} + +module.exports = { createSiteUserOrg }; diff --git a/test/api/unit/authorizers/site.test.js b/test/api/unit/authorizers/site.test.js index fc17e630b..bae370564 100644 --- a/test/api/unit/authorizers/site.test.js +++ b/test/api/unit/authorizers/site.test.js @@ -6,6 +6,7 @@ const githubAPINocks = require('../../support/githubAPINocks'); const authorizer = require('../../../../api/authorizers/site'); const { Role } = require('../../../../api/models'); const siteErrors = require('../../../../api/responses/siteErrors'); +const { createSiteUserOrg } = require('../../support/site-user'); describe('Site authorizer', () => { describe('.create(user, params)', () => { @@ -27,15 +28,7 @@ describe('Site authorizer', () => { }); it('should resolve for user with organizations', async () => { - const [user, org, role] = await Promise.all([ - factory.user(), - factory.organization.create(), - Role.findOne({ - where: { - name: 'user', - }, - }), - ]); + const { user, org } = await createSiteUserOrg(); const params = { owner: crypto.randomBytes(3).toString('hex'), repository: crypto.randomBytes(3).toString('hex'), @@ -44,11 +37,6 @@ describe('Site authorizer', () => { organizationId: org.id, }; - await org.addUser(user, { - through: { - roleId: role.id, - }, - }); const expected = await authorizer.create(user, params); return expect(expected).to.be.undefined; @@ -74,15 +62,7 @@ describe('Site authorizer', () => { it(`should throw an error for user with organizations and no organizationId specified`, async () => { - const [user, org, role] = await Promise.all([ - factory.user(), - factory.organization.create(), - Role.findOne({ - where: { - name: 'user', - }, - }), - ]); + const { user } = await createSiteUserOrg(); const params = { owner: crypto.randomBytes(3).toString('hex'), repository: crypto.randomBytes(3).toString('hex'), @@ -90,11 +70,6 @@ describe('Site authorizer', () => { engine: 'jekyll', }; - await org.addUser(user, { - through: { - roleId: role.id, - }, - }); const error = await authorizer.create(user, params).catch((err) => err); expect(error).to.be.throw; @@ -134,16 +109,6 @@ describe('Site authorizer', () => { }); describe('.findOne(user, site)', () => { - it('should resolve if the user is associated with the site', async () => { - const user = await factory.user(); - const site = await factory.site({ - users: Promise.all([user]), - }); - const expected = await authorizer.findOne(user, site); - - return expect(expected.id).to.equal(site.id); - }); - it('should reject if the user is not associated with the site', async () => { const [user, site] = await Promise.all([factory.user(), factory.site()]); const error = await authorizer.findOne(user, site).catch((err) => err); @@ -154,12 +119,7 @@ describe('Site authorizer', () => { context('site that belongs to an inactive organization', () => { it(`should resolve if the site is associated with the active organization`, async () => { - const org = await factory.organization.create(); - const user = await factory.user(); - const site = await factory.site({ - users: Promise.all([user]), - organizationId: org.id, - }); + const { site, user } = await createSiteUserOrg(); const expected = await authorizer.findOne(user, site); return expect(expected.id).to.equal(site.id); @@ -167,15 +127,8 @@ describe('Site authorizer', () => { it(`should reject if the site is associated with the inactive organization`, async () => { - const org = await factory.organization.create({ - isActive: false, - }); - const [user, site] = await Promise.all([ - factory.user(), - factory.site({ - organizationId: org.id, - }), - ]); + const { site, user, org } = await createSiteUserOrg(); + await org.update({ isActive: false }); const error = await authorizer.findOne(user, site).catch((err) => err); expect(error).to.be.throw; @@ -184,22 +137,15 @@ describe('Site authorizer', () => { }); context('site is inactive', () => { it('should resolve if the site is active', async () => { - const user = await factory.user(); - const site = await factory.site({ - users: Promise.all([user]), - }); + const { site, user } = await createSiteUserOrg(); const expected = await authorizer.findOne(user, site); expect(expected.isActive).to.be.true; return expect(expected.id).to.equal(site.id); }); it('should reject if the site is inactive', async () => { - const [user, site] = await Promise.all([ - factory.user(), - factory.site({ - isActive: false, - }), - ]); + const { site, user } = await createSiteUserOrg(); + await site.update({ isActive: false }); const error = await authorizer.findOne(user, site).catch((err) => err); expect(error).to.be.throw; @@ -209,16 +155,6 @@ describe('Site authorizer', () => { }); describe('.update(user, site)', () => { - it('should resolve if the user is associated with the site', async () => { - const user = await factory.user(); - const site = await factory.site({ - users: Promise.all([user]), - }); - const expected = await authorizer.update(user, site); - - return expect(expected.id).to.equal(site.id); - }); - it('should reject if the user is not associated with the site', async () => { const [user, site] = await Promise.all([factory.user(), factory.site()]); const error = await authorizer.update(user, site).catch((err) => err); @@ -229,12 +165,7 @@ describe('Site authorizer', () => { context('site that belongs to an inactive organization', () => { it(`should resolve if the site is associated with the active organization`, async () => { - const org = await factory.organization.create(); - const user = await factory.user(); - const site = await factory.site({ - users: Promise.all([user]), - organizationId: org.id, - }); + const { site, user } = await createSiteUserOrg(); const expected = await authorizer.update(user, site); return expect(expected.id).to.equal(site.id); @@ -242,15 +173,9 @@ describe('Site authorizer', () => { it(`should reject if the site is associated with the inactive organization`, async () => { - const org = await factory.organization.create({ - isActive: false, - }); - const [user, site] = await Promise.all([ - factory.user(), - factory.site({ - organizationId: org.id, - }), - ]); + const { site, user, org } = await createSiteUserOrg(); + await org.update({ isActive: false }); + const error = await authorizer.update(user, site).catch((err) => err); expect(error).to.be.throw; @@ -259,10 +184,7 @@ describe('Site authorizer', () => { }); context('site is active', () => { it('should resolve if the site is active', async () => { - const user = await factory.user(); - const site = await factory.site({ - users: Promise.all([user]), - }); + const { site, user } = await createSiteUserOrg(); const expected = await authorizer.update(user, site); expect(expected.isActive).to.be.true; @@ -270,12 +192,8 @@ describe('Site authorizer', () => { }); it('should reject if the site is inactive', async () => { - const [user, site] = await Promise.all([ - factory.user(), - factory.site({ - isActive: false, - }), - ]); + const { site, user } = await createSiteUserOrg(); + await site.update({ isActive: false }); const error = await authorizer.update(user, site).catch((err) => err); expect(error).to.be.throw; @@ -292,10 +210,7 @@ describe('Site authorizer', () => { nock.cleanAll(); }); it('should resolve if the user is associated with the site', async () => { - const user = await factory.user(); - const site = await factory.site({ - users: Promise.all([user]), - }); + const { site, user } = await createSiteUserOrg(); githubAPINocks.repo({ owner: site.owner, @@ -326,10 +241,7 @@ describe('Site authorizer', () => { it(`should reject if the user is associated with the site but not an admin`, async () => { - const user = await factory.user(); - const site = await factory.site({ - users: Promise.all([user]), - }); + const { site, user } = await createSiteUserOrg(); githubAPINocks.repo({ owner: site.owner, @@ -354,10 +266,7 @@ describe('Site authorizer', () => { it(`should accept if the user is associated with the site but site does not exist`, async () => { - const user = await factory.user(); - const site = await factory.site({ - users: Promise.all([user]), - }); + const { site, user } = await createSiteUserOrg(); githubAPINocks.repo({ owner: site.owner, @@ -371,10 +280,7 @@ describe('Site authorizer', () => { it(`should reject if the user is associated with the site but returns error`, async () => { - const user = await factory.user(); - const site = await factory.site({ - users: Promise.all([user]), - }); + const { site, user } = await createSiteUserOrg(); githubAPINocks.repo({ owner: site.owner, diff --git a/test/api/unit/models/build.test.js b/test/api/unit/models/build.test.js index 91a2f89f2..e17b5f4e7 100644 --- a/test/api/unit/models/build.test.js +++ b/test/api/unit/models/build.test.js @@ -5,6 +5,7 @@ const QueueJobs = require('../../../../api/queue-jobs'); const factory = require('../../support/factory'); const { Build, Site } = require('../../../../api/models'); const config = require('../../../../config'); +const { createSiteUserOrg } = require('../../support/site-user'); describe('Build model', () => { afterEach(() => { @@ -545,35 +546,20 @@ describe('Build model', () => { }); describe('forSiteUser scope', () => { - it('returns the build for the associated user', async () => { - const user = await factory.user(); - const build = await factory.build({ - user, - }); - - const buildQuery = await Build.forSiteUser(user).findByPk(build.id); - - expect(buildQuery).to.not.be.null; - expect(buildQuery.id).to.equal(build.id); - }); - it('returns the build for any user who has access to the site', async () => { - const [user1, user2] = await Promise.all([factory.user(), factory.user()]); - const site = await factory.site({ - users: [user1, user2], - }); + const { site, user } = await createSiteUserOrg(); + const build = await factory.build({ - user: user1, site, }); - const buildQuery = await Build.forSiteUser(user2).findByPk(build.id); + const buildQuery = await Build.forSiteUser(user).findByPk(build.id); expect(buildQuery).to.not.be.null; expect(buildQuery.id).to.equal(build.id); }); - it('does not return the build for a different user', async () => { + it('does not return the build for an arbitrary user', async () => { const user = { id: 99999, }; @@ -584,4 +570,18 @@ describe('Build model', () => { expect(buildQuery).to.be.null; }); }); + + describe('getSiteOrgUsers instance method', () => { + it("returns users associated a Build's Site", async () => { + const { site, user } = await createSiteUserOrg(); + + const build = await factory.build({ + site, + }); + + expect(build).to.respondTo('getSiteOrgUsers'); + const users = await build.getSiteOrgUsers(); + expect(users[0].id).to.be.equal(user.id); + }); + }); }); diff --git a/test/api/unit/models/organization.test.js b/test/api/unit/models/organization.test.js index de8c19778..e40f8a647 100644 --- a/test/api/unit/models/organization.test.js +++ b/test/api/unit/models/organization.test.js @@ -299,4 +299,31 @@ describe('Organization model', () => { expect(org.daysUntilSandboxCleaning).to.equal(sandboxDays); }); }); + + describe('addRoleUser instance method', () => { + it('should add a user to an org', async () => { + const user = await createUser(); + const org = await orgFactory.create(); + expect(org).to.respondTo('addRoleUser'); + await org.addRoleUser(user); + + await org.reload({ include: OrganizationRole }); + const members = await OrganizationRole.forOrganization(org).findAll(); + expect(members[0].User.id).to.be.equal(user.id); + expect(members[0].Role.name).to.be.equal('user'); + }); + + it('should add a manager to an org', async () => { + const manager = 'manager'; + const user = await createUser(); + const org = await orgFactory.create(); + expect(org).to.respondTo('addRoleUser'); + await org.addRoleUser(user, manager); + + await org.reload({ include: OrganizationRole }); + const members = await OrganizationRole.forOrganization(org).findAll(); + expect(members[0].User.id).to.be.equal(user.id); + expect(members[0].Role.name).to.be.equal(manager); + }); + }); }); diff --git a/test/api/unit/models/site-user.test.js b/test/api/unit/models/site-user.test.js deleted file mode 100644 index 3122ee4e3..000000000 --- a/test/api/unit/models/site-user.test.js +++ /dev/null @@ -1,81 +0,0 @@ -const expect = require('chai').expect; -const factory = require('../../support/factory'); -const { User, Site, SiteUser } = require('../../../../api/models'); - -describe('SiteUser model', () => { - it('returns the site object with user association', (done) => { - let user1; - let user2; - let site1; - let site2; - - factory - .user() - .then((user) => { - user1 = user; - return factory.site({ - users: [user1], - }); - }) - .then((model) => { - site1 = model; - return SiteUser.findOne({ - where: { - site_users: site1.id, - user_sites: user1.id, - }, - include: [User, Site], - }); - }) - .then((siteUser) => { - expect(siteUser.site_users).to.equal(site1.id); - expect(siteUser.user_sites).to.equal(user1.id); - expect(siteUser.buildNotificationSetting).to.equal('site'); - - expect(siteUser.Site.id).to.equal(site1.id); - expect(siteUser.User.id).to.equal(user1.id); - expect(siteUser.buildNotificationSetting).to.equal('site'); - return factory.user(); - }) - .then((user) => { - user2 = user; - return Site.withUsers(site1.id); - }) - .then((siteWithUsers) => { - expect(siteWithUsers.Users).to.be.an('array'); - expect(siteWithUsers.Users.length).to.equal(1); - return factory.site({ - users: [user1, user2], - }); - }) - .then((site) => { - site2 = site; - return Site.withUsers(site2.id); - }) - .then((siteWithUsers) => { - expect(siteWithUsers.Users).to.be.an('array'); - expect(siteWithUsers.Users.length).to.equal(2); - - return Promise.all([ - User.findOne({ - where: { - id: user1.id, - }, - include: [Site], - }), - User.findOne({ - where: { - id: user2.id, - }, - include: [Site], - }), - ]); - }) - .then((usersWithSites) => { - expect(usersWithSites[0].Sites.length).to.equal(2); - expect(usersWithSites[1].Sites.length).to.equal(1); - done(); - }) - .catch(done); - }); -}); diff --git a/test/api/unit/models/site.test.js b/test/api/unit/models/site.test.js index 10386feaa..34a19a682 100644 --- a/test/api/unit/models/site.test.js +++ b/test/api/unit/models/site.test.js @@ -1,7 +1,8 @@ const { expect } = require('chai'); const factory = require('../../support/factory'); -const { Role, Site } = require('../../../../api/models'); +const { createSiteUserOrg } = require('../../support/site-user'); +const { Site } = require('../../../../api/models'); function clean() { return factory.organization.truncate(); @@ -28,19 +29,6 @@ describe('Site model', () => { }); }); - describe('.withUsers', () => { - it('returns the site object with user association', async () => { - const { id: siteId } = await factory.site({ - users: Promise.all([factory.user()]), - }); - - const site = await Site.withUsers(siteId); - - expect(site.Users).to.be.an('array'); - expect(site.Users.length).to.equal(1); - }); - }); - it('should not let the domain and demoDomain be equal', (done) => { factory .site({ @@ -299,62 +287,37 @@ describe('Site model', () => { }); describe('forUser scope', () => { - it('returns sites in the org of the user and non-org sites', async () => { - const [user1, user2, org1, org2, userRole] = await Promise.all([ + it('returns sites in the org of the user', async () => { + const [user1, user2, org1, org2] = await Promise.all([ factory.user(), factory.user(), factory.organization.create(), factory.organization.create(), - Role.findOne({ - where: { - name: 'user', - }, - }), ]); const [ - nonOrgSiteForUser, + _nonOrgSiteForUser, orgSiteForUser, nonOrgSiteForOtherUser, orgSiteForOtherUserOrg, orgSiteOnlyForUser, ] = await Promise.all([ - factory.site({ - users: [user1], - }), - factory.site({ - users: [user1], - }), - factory.site({ - users: [user2], - }), - factory.site({ - users: [user1, user2], - }), + factory.site(), + factory.site(), + factory.site(), + factory.site(), factory.site(), ]); await Promise.all([ - org1.addUser(user1, { - through: { - roleId: userRole.id, - }, - }), - org2.addUser(user2, { - through: { - roleId: userRole.id, - }, - }), + org1.addRoleUser(user1), + org2.addRoleUser(user2), org1.addSite(orgSiteForUser), org2.addSite(orgSiteForOtherUserOrg), org2.addSite(orgSiteOnlyForUser), ]); - const expectedMemberIds = [ - nonOrgSiteForUser, - orgSiteForUser, - orgSiteForOtherUserOrg, - ].map((site) => site.id); + const expectedMemberIds = [orgSiteForUser].map((site) => site.id); const expectedNonMemberIds = [nonOrgSiteForOtherUser].map((site) => site.id); @@ -368,7 +331,15 @@ describe('Site model', () => { const orgSite = sites.find((site) => site.id === orgSiteForUser.id); expect(orgSite.Organization.id).to.eq(org1.id); expect(orgSite.Organization.OrganizationRoles[0].userId).to.eq(user1.id); - expect(orgSite.Organization.OrganizationRoles[0].roleId).to.eq(userRole.id); + }); + }); + + describe('getOrgUsers instance method', () => { + it('returns users associated a Site', async () => { + const { site, user } = await createSiteUserOrg(); + expect(site).to.respondTo('getOrgUsers'); + const users = await site.getOrgUsers(); + expect(users[0].id).to.be.equal(user.id); }); }); }); diff --git a/test/api/unit/models/user.test.js b/test/api/unit/models/user.test.js index 2942bc840..4f3e3b658 100644 --- a/test/api/unit/models/user.test.js +++ b/test/api/unit/models/user.test.js @@ -1,7 +1,6 @@ const { expect } = require('chai'); const { OrganizationRole, Role, Site, User } = require('../../../../api/models'); const orgFactory = require('../../support/factory/organization'); -const createSite = require('../../support/factory/site'); const createUser = require('../../support/factory/user'); const { createUAAIdentity } = require('../../support/factory/uaa-identity'); @@ -176,28 +175,6 @@ describe('User model', () => { }); }); - describe('siteScope', () => { - it('filters users by site and includes Site', async () => { - const [, user1, user2] = await Promise.all([ - createUser(), - createUser(), - createUser(), - ]); - - const site = await createSite({ - users: [user1, user2], - }); - - const siteUsers = await User.scope(User.siteScope(site.id)).findAll(); - - expect(siteUsers.map((u) => u.id)).to.have.members([user1.id, user2.id]); - expect(siteUsers.flatMap((u) => u.Sites.map((s) => s.id))).to.have.members([ - site.id, - site.id, - ]); - }); - }); - describe('searchScope', () => { it('returns the user by id', async () => { const [user] = await Promise.all([createUser(), createUser()]); diff --git a/test/api/unit/serializers/site.test.js b/test/api/unit/serializers/site.test.js index 3ba1677e6..aeec6a74a 100644 --- a/test/api/unit/serializers/site.test.js +++ b/test/api/unit/serializers/site.test.js @@ -46,36 +46,8 @@ describe('SiteSerializer', () => { .catch(done); }); - it('excludes users without a federalist account', (done) => { - const authUserCount = 3; - const nonGithubUser = factory.user({ - githubAccessToken: null, - }); - const otherUsers = Array(authUserCount) - .fill(0) - .map(() => factory.user()); - - Promise.all(otherUsers.concat(nonGithubUser)) - .then((users) => - factory.site({ - users, - }), - ) - .then(SiteSerializer.serialize) - .then((object) => { - expect(object.users.length).to.equal(authUserCount); - done(); - }) - .catch(done); - }); - it('includes organization name when associated to site', async () => { - const site = await factory.site({ - basicAuth: { - username: 'username', - password: 'password', - }, - }); + const site = await factory.site(); const org = await factory.organization.create(); await org.addSite(site.id); await site.reload(); @@ -83,7 +55,6 @@ describe('SiteSerializer', () => { const result = validateJSONSchema(object, siteSchema); expect(result.errors).to.be.empty; - expect(object.basicAuth.password).to.eq('**********'); expect(object.organizationId).to.equal(org.id); }); diff --git a/test/api/unit/services/GitHub.test.js b/test/api/unit/services/GitHub.test.js index cb059d846..fb6768a7d 100644 --- a/test/api/unit/services/GitHub.test.js +++ b/test/api/unit/services/GitHub.test.js @@ -4,6 +4,7 @@ const config = require('../../../../config'); const factory = require('../../support/factory'); const GitHub = require('../../../../api/services/GitHub'); const githubAPINocks = require('../../support/githubAPINocks'); +const { createSiteUserOrg } = require('../../support/site-user'); describe('GitHub', () => { afterEach(() => expect(nock.isDone()).to.be.true); @@ -580,57 +581,42 @@ describe('GitHub', () => { }); describe('.getBranch', () => { - let promised; + let user; + let site; let mockGHRequest; - beforeEach(() => { + beforeEach(async () => { mockGHRequest = null; - const userPromise = factory.user(); - const sitePromise = factory.site({ - users: Promise.all([userPromise]), - }); + ({ site, user } = await createSiteUserOrg()); + }); - promised = Promise.props({ - user: userPromise, - site: sitePromise, + it('returns a branch based on the supplied parameters', async () => { + const { owner, repository } = site; + const branch = 'main'; + + mockGHRequest = githubAPINocks.getBranch({ + owner, + repo: repository, + branch, }); - }); - it('returns a branch based on the supplied parameters', (done) => { - promised - .then((values) => { - const { owner, repository } = values.site; - const branch = 'main'; - - mockGHRequest = githubAPINocks.getBranch({ - owner, - repo: repository, - branch, - }); + const branchInfo = await GitHub.getBranch(user, owner, repository, branch); - return GitHub.getBranch(values.user, owner, repository, branch); - }) - .then((branchInfo) => { - expect(branchInfo).to.exist; - expect(branchInfo.name).to.exist; - expect(branchInfo.commit).to.exist; - expect(mockGHRequest.isDone()).to.be.true; - done(); - }) - .catch(done); + expect(branchInfo).to.exist; + expect(branchInfo.name).to.exist; + expect(branchInfo.commit).to.exist; + expect(mockGHRequest.isDone()).to.be.true; }); it('returns an error if branch is not defined', (done) => { - promised - .then((values) => { - const { owner, repository } = values.site; - - return GitHub.getBranch(values.user, owner, repository).catch((err) => { - // octokit no longer validates the arguments client side so the error - // we are receiving here is actually the missing nock... - expect(err.status).to.equal(500); - done(); - }); + const { owner, repository } = site; + + GitHub.getBranch(user, owner, repository) + .catch((err) => { + // octokit no longer validates the arguments client side so the error + // we are receiving here is actually the missing nock... + expect(err.status).to.equal(500); + done(); }) .catch(done); }); diff --git a/test/api/unit/services/GithubBuildHelper.test.js b/test/api/unit/services/GithubBuildHelper.test.js index e74ac196e..1da146052 100644 --- a/test/api/unit/services/GithubBuildHelper.test.js +++ b/test/api/unit/services/GithubBuildHelper.test.js @@ -2,9 +2,10 @@ const { expect } = require('chai'); const sinon = require('sinon'); const nock = require('nock'); const config = require('../../../../config'); -const { Domain, Site, SiteBranchConfig, User } = require('../../../../api/models'); +const { Site, User, Organization, OrganizationRole } = require('../../../../api/models'); const factory = require('../../support/factory'); const githubAPINocks = require('../../support/githubAPINocks'); +const { createSiteUserOrg } = require('../../support/site-user'); const { buildUrl } = require('../../../../api/utils/build'); const GitHub = require('../../../../api/services/GitHub'); const GithubBuildHelper = require('../../../../api/services/GithubBuildHelper'); @@ -24,18 +25,22 @@ describe('GithubBuildHelper', () => { let site; let build; let users; + let org; + beforeEach(async () => { - user = await factory.user(); - site = await factory.site({ - users: [user], - }); + ({ site, user, org } = await createSiteUserOrg()); build = await factory.build({ state: 'processing', site, user, requestedCommitSha, }); - users = await site.getUsers(); + + const fullOrg = await Organization.findOne({ + where: { id: org.id }, + include: { model: OrganizationRole, include: User }, + }); + users = fullOrg.OrganizationRoles.map((role) => role.User); }); it("should report that the status is 'pending'", async () => { const repoNock = githubAPINocks.repo({ @@ -51,18 +56,7 @@ describe('GithubBuildHelper', () => { sha: requestedCommitSha, state: 'pending', }); - await build.reload({ - include: [ - { - model: Site, - include: [ - { - model: User, - }, - ], - }, - ], - }); + await build.reload({ include: Site }); await GithubBuildHelper.reportBuildStatus(build); expect(repoNock.isDone()).to.be.true; @@ -77,7 +71,6 @@ describe('GithubBuildHelper', () => { site, requestedCommitSha, }); - await site.addUser(build.user); let i; let options; @@ -107,62 +100,7 @@ describe('GithubBuildHelper', () => { state: 'pending', }); - await build.reload({ - include: [ - { - model: Site, - include: [ - { - model: User, - }, - ], - }, - ], - }); - await GithubBuildHelper.reportBuildStatus(build); - repoNocks.forEach((repoNock) => expect(repoNock.isDone()).to.be.true); - expect(statusNock.isDone()).to.be.true; - }); - - it('should report the status if the build initiated by a contributor', async () => { - const repoNocks = []; - await build.update({ - user: null, - username: 'contributor', - }); - users = await site.getUsers(); - let i; - let options; - for (i = 0; i < users.length; i += 1) { - options = { - accessToken: users[i].githubAccessToken, - owner: site.owner, - repo: site.repository, - username: users[i].username, - }; - repoNocks.push(githubAPINocks.repo(options)); - } - - const statusNock = githubAPINocks.status({ - owner: site.owner, - repo: site.repository, - repository: site.repository, - sha: requestedCommitSha, - state: 'pending', - }); - - await build.reload({ - include: [ - { - model: Site, - include: [ - { - model: User, - }, - ], - }, - ], - }); + await build.reload({ include: Site }); await GithubBuildHelper.reportBuildStatus(build); repoNocks.forEach((repoNock) => expect(repoNock.isDone()).to.be.true); expect(statusNock.isDone()).to.be.true; @@ -178,7 +116,7 @@ describe('GithubBuildHelper', () => { user: newUser.id, username: newUser.username, }); - users = await site.getUsers(); + let i; let options; for (i = 0; i < users.length; i += 1) { @@ -199,32 +137,20 @@ describe('GithubBuildHelper', () => { state: 'pending', }); - await build.reload({ - include: [ - { - model: Site, - include: [ - { - model: User, - }, - ], - }, - ], - }); + await build.reload({ include: Site }); await GithubBuildHelper.reportBuildStatus(build); repoNocks.forEach((repoNock) => expect(repoNock.isDone()).to.be.true); expect(statusNock.isDone()).to.be.true; }); - it(`should report that the status + it(`should report the status if the build user has invalid token'`, async () => { const repoNocks = []; await user.update({ signedInAt: '1999-01-01', }); const otherUser = await factory.user(); - await site.addUser(otherUser); - users = await site.getUsers(); + await org.addRoleUser(otherUser); repoNocks.push( githubAPINocks.repo({ @@ -254,18 +180,7 @@ describe('GithubBuildHelper', () => { sha: requestedCommitSha, state: 'pending', }); - await build.reload({ - include: [ - { - model: Site, - include: [ - { - model: User, - }, - ], - }, - ], - }); + await build.reload({ include: Site }); await GithubBuildHelper.reportBuildStatus(build); repoNocks.forEach((repoNock) => expect(repoNock.isDone()).to.be.true); expect(statusNock.isDone()).to.be.true; @@ -278,8 +193,7 @@ describe('GithubBuildHelper', () => { signedInAt: '1999-01-01', }); const otherUser = await factory.user(); - await site.addUser(otherUser); - users = await site.getUsers(); + await org.addRoleUser(otherUser); repoNocks.push( githubAPINocks.repo({ @@ -309,30 +223,18 @@ describe('GithubBuildHelper', () => { sha: requestedCommitSha, state: 'pending', }); - await build.reload({ - include: [ - { - model: Site, - include: [ - { - model: User, - }, - ], - }, - ], - }); + await build.reload({ include: Site }); await GithubBuildHelper.reportBuildStatus(build); repoNocks.forEach((repoNock) => expect(repoNock.isDone()).to.be.true); expect(statusNock.isDone()).to.be.true; }); - it(`should not report that the status + it(`should not report the status if the build user has invalid token'`, async () => { const repoNocks = []; const statusSpy = sinon.spy(GitHub, 'sendCreateGithubStatusRequest'); const recentUser = await factory.user(); - await site.addUser(recentUser); - users = await site.getUsers(); + await org.addRoleUser(recentUser); repoNocks.push( githubAPINocks.repo({ @@ -347,19 +249,6 @@ describe('GithubBuildHelper', () => { ], }), ); - repoNocks.push( - githubAPINocks.repo({ - owner: site.owner, - repo: site.repository, - username: user.username, - response: [ - 403, - { - permissions: {}, - }, - ], - }), - ); repoNocks.push( githubAPINocks.repo({ owner: site.owner, @@ -374,18 +263,7 @@ describe('GithubBuildHelper', () => { }), ); - await build.reload({ - include: [ - { - model: Site, - include: [ - { - model: User, - }, - ], - }, - ], - }); + await build.reload({ include: Site }); const err = await GithubBuildHelper.reportBuildStatus(build).catch((e) => e); repoNocks.forEach((repoNock) => expect(repoNock.isDone()).to.be.true); expect(statusSpy.called).to.be.false; @@ -394,13 +272,12 @@ describe('GithubBuildHelper', () => { ); }); - it(`should not report that the status if + it(`should not report the status if the build user does not have write permissions'`, async () => { const repoNocks = []; const statusSpy = sinon.spy(GitHub, 'sendCreateGithubStatusRequest'); const recentUser = await factory.user(); - await site.addUser(recentUser); - users = await site.getUsers(); + await org.addRoleUser(recentUser); repoNocks.push( githubAPINocks.repo({ @@ -415,19 +292,6 @@ describe('GithubBuildHelper', () => { ], }), ); - repoNocks.push( - githubAPINocks.repo({ - owner: site.owner, - repo: site.repository, - username: user.username, - response: [ - 201, - { - permissions: {}, - }, - ], - }), - ); repoNocks.push( githubAPINocks.repo({ owner: site.owner, @@ -442,18 +306,7 @@ describe('GithubBuildHelper', () => { }), ); - await build.reload({ - include: [ - { - model: Site, - include: [ - { - model: User, - }, - ], - }, - ], - }); + await build.reload({ include: Site }); const err = await GithubBuildHelper.reportBuildStatus(build).catch((e) => e); repoNocks.forEach((repoNock) => expect(repoNock.isDone()).to.be.true); expect(statusSpy.called).to.be.false; @@ -475,18 +328,7 @@ describe('GithubBuildHelper', () => { sha: requestedCommitSha, targetURL: `${config.app.hostname}/sites/${build.site}/builds/${build.id}/logs`, }); - await build.reload({ - include: [ - { - model: Site, - include: [ - { - model: User, - }, - ], - }, - ], - }); + await build.reload({ include: Site }); await GithubBuildHelper.reportBuildStatus(build); expect(repoNock.isDone()).to.be.true; expect(statusNock.isDone()).to.be.true; @@ -499,19 +341,17 @@ describe('GithubBuildHelper', () => { let build; beforeEach(async () => { - user = await factory.user(); site = await factory.site({ owner: 'test-owner', repository: 'test-repo', - users: [user], }); + ({ user } = await createSiteUserOrg({ site })); build = await factory.build({ state: 'created', site, requestedCommitSha, user, }); - await site.getUsers(); }); it("should report that the status is 'pending'", async () => { @@ -529,18 +369,7 @@ describe('GithubBuildHelper', () => { state: 'pending', }); - await build.reload({ - include: [ - { - model: Site, - include: [ - { - model: User, - }, - ], - }, - ], - }); + await build.reload({ include: Site }); await GithubBuildHelper.reportBuildStatus(build); expect(repoNock.isDone()).to.be.true; expect(statusNock.isDone()).to.be.true; @@ -560,18 +389,7 @@ describe('GithubBuildHelper', () => { targetURL: `${config.app.hostname}/sites/${build.site}/builds/${build.id}/logs`, }); - await build.reload({ - include: [ - { - model: Site, - include: [ - { - model: User, - }, - ], - }, - ], - }); + await build.reload({ include: Site }); await GithubBuildHelper.reportBuildStatus(build); expect(repoNock.isDone()).to.be.true; expect(statusNock.isDone()).to.be.true; @@ -584,19 +402,18 @@ describe('GithubBuildHelper', () => { let build; beforeEach(async () => { - user = await factory.user(); site = await factory.site({ owner: 'test-owner', repository: 'test-repo', - users: [user], }); + ({ user } = await createSiteUserOrg({ site })); + build = await factory.build({ state: 'created', site, requestedCommitSha, user, }); - await site.getUsers(); }); it("should report that the status is 'pending'", async () => { const repoNock = githubAPINocks.repo({ @@ -613,18 +430,7 @@ describe('GithubBuildHelper', () => { state: 'pending', }); - await build.reload({ - include: [ - { - model: Site, - include: [ - { - model: User, - }, - ], - }, - ], - }); + await build.reload({ include: Site }); await GithubBuildHelper.reportBuildStatus(build); expect(repoNock.isDone()).to.be.true; expect(statusNock.isDone()).to.be.true; @@ -644,18 +450,7 @@ describe('GithubBuildHelper', () => { targetURL: `${config.app.hostname}/sites/${build.site}/builds/${build.id}/logs`, }); - await build.reload({ - include: [ - { - model: Site, - include: [ - { - model: User, - }, - ], - }, - ], - }); + await build.reload({ include: Site }); await GithubBuildHelper.reportBuildStatus(build); expect(repoNock.isDone()).to.be.true; expect(statusNock.isDone()).to.be.true; @@ -674,12 +469,11 @@ describe('GithubBuildHelper', () => { }); beforeEach(async () => { - user = await factory.user(); site = await factory.site({ owner: 'test-owner', repository: 'test-repo', - users: [user], }); + ({ user } = await createSiteUserOrg({ site })); build = await factory.build({ state: 'success', site, @@ -687,7 +481,6 @@ describe('GithubBuildHelper', () => { clonedCommitSha, user, }); - await site.getUsers(); }); it(`should set status context to @@ -706,14 +499,7 @@ describe('GithubBuildHelper', () => { state: 'success', }); - await build.reload({ - include: [ - { - model: Site, - include: [User, Domain, SiteBranchConfig], - }, - ], - }); + await build.reload({ include: Site }); await GithubBuildHelper.reportBuildStatus(build); expect(repoNock.isDone()).to.be.true; expect(statusNock.isDone()).to.be.true; @@ -727,9 +513,7 @@ describe('GithubBuildHelper', () => { beforeEach(async () => { user = await factory.user(); - site = await factory.site({ - users: [user], - }); + ({ site, user } = await createSiteUserOrg()); build = await factory.build({ state: 'success', requestedCommitSha, @@ -737,7 +521,6 @@ describe('GithubBuildHelper', () => { user, site, }); - await site.getUsers(); }); it("should report that the status is 'success'", async () => { const repoNock = githubAPINocks.repo({ @@ -753,14 +536,7 @@ describe('GithubBuildHelper', () => { state: 'success', }); - await build.reload({ - include: [ - { - model: Site, - include: [User, Domain, SiteBranchConfig], - }, - ], - }); + await build.reload({ include: Site }); await GithubBuildHelper.reportBuildStatus(build); expect(repoNock.isDone()).to.be.true; expect(statusNock.isDone()).to.be.true; @@ -791,14 +567,7 @@ describe('GithubBuildHelper', () => { targetURL: buildUrl(build, site), }); - await build.reload({ - include: [ - { - model: Site, - include: [User, Domain, SiteBranchConfig], - }, - ], - }); + await build.reload({ include: Site }); await GithubBuildHelper.reportBuildStatus(build); expect(repoNock.isDone()).to.be.true; @@ -812,17 +581,13 @@ describe('GithubBuildHelper', () => { let build; beforeEach(async () => { - user = await factory.user(); - site = await factory.site({ - users: [user], - }); + ({ site, user } = await createSiteUserOrg()); build = await factory.build({ state: 'error', requestedCommitSha, user, site, }); - await site.getUsers(); }); it("should report that the status is 'error' with requestedCommitSha", async () => { const repoNock = githubAPINocks.repo({ @@ -838,18 +603,7 @@ describe('GithubBuildHelper', () => { state: 'error', }); - await build.reload({ - include: [ - { - model: Site, - include: [ - { - model: User, - }, - ], - }, - ], - }); + await build.reload({ include: Site }); await GithubBuildHelper.reportBuildStatus(build); expect(statusNock.isDone()).to.be.true; expect(repoNock.isDone()).to.be.true; @@ -874,18 +628,7 @@ describe('GithubBuildHelper', () => { targetURL: `${config.app.hostname}/sites/${build.site}/builds/${build.id}/logs`, }); - await build.reload({ - include: [ - { - model: Site, - include: [ - { - model: User, - }, - ], - }, - ], - }); + await build.reload({ include: Site }); await GithubBuildHelper.reportBuildStatus(build); expect(statusNock.isDone()).to.be.true; expect(repoNock.isDone()).to.be.true; @@ -912,18 +655,7 @@ describe('GithubBuildHelper', () => { targetURL: `${config.app.hostname}/sites/${build.site}/builds/${build.id}/logs`, }); - await build.reload({ - include: [ - { - model: Site, - include: [ - { - model: User, - }, - ], - }, - ], - }); + await build.reload({ include: Site }); await GithubBuildHelper.reportBuildStatus(build); expect(statusNock.isDone()).to.be.true; @@ -939,10 +671,7 @@ describe('GithubBuildHelper', () => { const path = 'filePath.txt'; beforeEach(async () => { - user = await factory.user(); - site = await factory.site({ - users: [user], - }); + ({ site, user } = await createSiteUserOrg()); build = await factory.build({ state: 'success', requestedCommitSha, @@ -950,7 +679,6 @@ describe('GithubBuildHelper', () => { user, site, }); - await site.getUsers(); }); it('should fetch the content requested', async () => { @@ -958,14 +686,7 @@ describe('GithubBuildHelper', () => { sinon.stub(GitHub, 'checkPermissions').resolves({ push: true, }); - await build.reload({ - include: [ - { - model: Site, - include: [{ model: User }], - }, - ], - }); + await build.reload({ include: Site }); const content = await GithubBuildHelper.fetchContent(build, path); expect(content).to.equal('testContent'); }); @@ -975,14 +696,7 @@ describe('GithubBuildHelper', () => { requestedCommitSha: null, clonedCommitSha: null, }); - await build.reload({ - include: [ - { - model: Site, - include: [{ model: User }], - }, - ], - }); + await build.reload({ include: Site }); const err = await GithubBuildHelper.fetchContent(build, path).catch((e) => e); expect(err.message).to.equal( `Build or commit sha undefined. Unable to fetch ${path} for build@id=${build.id}`, @@ -994,19 +708,14 @@ describe('GithubBuildHelper', () => { let user; let site; let build; + let org; beforeEach(async () => { - user = await factory.user({ - signedInAt: '1999-12-31', - }); - site = await factory.site({ - users: [user], - }); + ({ site, user, org } = await createSiteUserOrg()); build = await factory.build({ site, user, }); - await site.getUsers(); }); it('should fetch buildUser token', async () => { @@ -1016,24 +725,16 @@ describe('GithubBuildHelper', () => { repo: site.repository, username: user.username, }); - await build.reload({ - include: [ - { - model: Site, - include: [{ model: User }], - }, - ], - }); + await build.reload({ include: Site }); const githubAccessToken = await GithubBuildHelper.loadBuildUserAccessToken(build); expect(githubAccessToken).to.equal(user.githubAccessToken); expect(repoNock.isDone()).to.be.true; }); - it(`should fetch a siteUser token - - build user no access to private repo`, async () => { - const siteUser = await factory.user(); - await site.addUser(siteUser); - await site.getUsers(); + it(`should fetch another org user token + - build user has no access to private repo`, async () => { + const orgUser = await factory.user(); + await org.addRoleUser(orgUser); const repoNocks = []; repoNocks.push( githubAPINocks.repo({ @@ -1051,29 +752,21 @@ describe('GithubBuildHelper', () => { ); repoNocks.push( githubAPINocks.repo({ - accessToken: siteUser.githubAccessToken, + accessToken: orgUser.githubAccessToken, owner: site.owner, repo: site.repository, - username: siteUser.username, + username: orgUser.username, }), ); - await build.reload({ - include: [ - { - model: Site, - include: [{ model: User }], - }, - ], - }); + await build.reload({ include: Site }); const githubAccessToken = await GithubBuildHelper.loadBuildUserAccessToken(build); - expect(githubAccessToken).to.equal(siteUser.githubAccessToken); + expect(githubAccessToken).to.equal(orgUser.githubAccessToken); repoNocks.forEach((repoNock) => expect(repoNock.isDone()).to.be.true); }); - it('should fetch a siteUser token - build user no permission', async () => { - const siteUser = await factory.user(); - await site.addUser(siteUser); - await site.getUsers(); + it('should fetch an org user token - build user with no permission', async () => { + const orgUser = await factory.user(); + await org.addRoleUser(orgUser); const repoNocks = []; repoNocks.push( githubAPINocks.repo({ @@ -1091,29 +784,21 @@ describe('GithubBuildHelper', () => { ); repoNocks.push( githubAPINocks.repo({ - accessToken: siteUser.githubAccessToken, + accessToken: orgUser.githubAccessToken, owner: site.owner, repo: site.repository, - username: siteUser.username, + username: orgUser.username, }), ); - await build.reload({ - include: [ - { - model: Site, - include: [{ model: User }], - }, - ], - }); + await build.reload({ include: Site }); const githubAccessToken = await GithubBuildHelper.loadBuildUserAccessToken(build); - expect(githubAccessToken).to.equal(siteUser.githubAccessToken); + expect(githubAccessToken).to.equal(orgUser.githubAccessToken); repoNocks.forEach((repoNock) => expect(repoNock.isDone()).to.be.true); }); - it('all site user w/o permissions to private repo', async () => { - const siteUser = await factory.user(); - await site.addUser(siteUser); - await site.getUsers(); + it('all org users w/o permissions to private repo', async () => { + const orgUser = await factory.user(); + await org.addRoleUser(orgUser); const repoNocks = []; repoNocks.push( githubAPINocks.repo({ @@ -1131,24 +816,10 @@ describe('GithubBuildHelper', () => { ); repoNocks.push( githubAPINocks.repo({ - accessToken: siteUser.githubAccessToken, - owner: site.owner, - repo: site.repository, - username: siteUser.username, - response: [ - 403, - { - permissions: {}, - }, - ], - }), - ); - repoNocks.push( - githubAPINocks.repo({ - accessToken: user.githubAccessToken, + accessToken: orgUser.githubAccessToken, owner: site.owner, repo: site.repository, - username: user.username, + username: orgUser.username, response: [ 403, { @@ -1157,14 +828,7 @@ describe('GithubBuildHelper', () => { ], }), ); - await build.reload({ - include: [ - { - model: Site, - include: [{ model: User }], - }, - ], - }); + await build.reload({ include: Site }); const err = await GithubBuildHelper.loadBuildUserAccessToken(build).catch((e) => e); repoNocks.forEach((repoNock) => expect(repoNock.isDone()).to.be.true); expect(err.message).to.equal( @@ -1173,9 +837,8 @@ describe('GithubBuildHelper', () => { }); it('all site users withouth write access', async () => { - const siteUser = await factory.user(); - await site.addUser(siteUser); - await site.getUsers(); + const orgUser = await factory.user(); + await org.addRoleUser(orgUser); const repoNocks = []; repoNocks.push( githubAPINocks.repo({ @@ -1193,10 +856,10 @@ describe('GithubBuildHelper', () => { ); repoNocks.push( githubAPINocks.repo({ - accessToken: siteUser.githubAccessToken, + accessToken: orgUser.githubAccessToken, owner: site.owner, repo: site.repository, - username: siteUser.username, + username: orgUser.username, response: [ 201, { @@ -1205,28 +868,7 @@ describe('GithubBuildHelper', () => { ], }), ); - repoNocks.push( - githubAPINocks.repo({ - accessToken: user.githubAccessToken, - owner: site.owner, - repo: site.repository, - username: user.username, - response: [ - 201, - { - permissions: {}, - }, - ], - }), - ); - await build.reload({ - include: [ - { - model: Site, - include: [{ model: User }], - }, - ], - }); + await build.reload({ include: Site }); const err = await GithubBuildHelper.loadBuildUserAccessToken(build).catch((e) => e); repoNocks.forEach((repoNock) => expect(repoNock.isDone()).to.be.true); expect(err.message).to.equal( diff --git a/test/api/unit/services/RepositoryVerifier.test.js b/test/api/unit/services/RepositoryVerifier.test.js index 1e11b6d49..2ebaf8683 100644 --- a/test/api/unit/services/RepositoryVerifier.test.js +++ b/test/api/unit/services/RepositoryVerifier.test.js @@ -1,276 +1,77 @@ const { expect } = require('chai'); -const proxyquire = require('proxyquire').noCallThru(); -const moment = require('moment'); const factory = require('../../support/factory'); const RepositoryVerifier = require('../../../../api/services/RepositoryVerifier'); -const { Site, User } = require('../../../../api/models'); +const { Site } = require('../../../../api/models'); const githubAPINocks = require('../../support/githubAPINocks'); -const MockGitHub = require('../../support/mockGitHub'); +const { createSiteUserOrg } = require('../../support/site-user'); describe('RepositoryVerifier', () => { context('verifyRepos', () => { - it('verify site', (done) => { - let users; - let site; + it('verify site', async () => { + const { site, user } = await createSiteUserOrg(); - Promise.all([factory.user(), factory.user()]) - .then((_users) => { - users = _users; - return factory.site({ users }); - }) - .then((_site) => { - site = _site; - githubAPINocks.repo({ - accessToken: users[0].githubAccessToken, - owner: site.owner, - repo: site.repository, - }); - return RepositoryVerifier.verifyRepos(); - }) - .then(() => Site.findByPk(site.id)) - .then((_site) => { - expect(_site.repoLastVerified).gt(site.repoLastVerified); - done(); - }) - .catch(done); - }).timeout(10000); + githubAPINocks.repo({ + accessToken: user.githubAccessToken, + owner: site.owner, + repo: site.repository, + }); - it('verify site with second users', (done) => { - let users; - let site; - - Promise.all([factory.user(), factory.user(), factory.user()]) - .then((_users) => { - users = _users; - return factory.site({ users }); - }) - .then((_site) => { - site = _site; - githubAPINocks.repo({ - accessToken: users[0].githubAccessToken, - owner: site.owner, - repo: site.repository, - response: 404, - }); - githubAPINocks.repo({ - accessToken: users[1].githubAccessToken, - owner: site.owner, - repo: site.repository, - }); - return RepositoryVerifier.verifyRepos(); - }) - .then(() => Site.findByPk(site.id)) - .then((_site) => { - expect(_site.repoLastVerified).gt(site.repoLastVerified); - done(); - }) - .catch(done); - }).timeout(12000); + await RepositoryVerifier.verifyRepos(); + const latestSite = await Site.findByPk(site.id); + expect(latestSite.repoLastVerified).gt(site.repoLastVerified); + }); - it('not able to verify sites with users that cannot access repository', (done) => { - let users; - let site; + it('verify site with second users', async () => { + const { site, user: user1, org } = await createSiteUserOrg(); + const user2 = await factory.user(); + await org.addRoleUser(user2); - Promise.all([factory.user(), factory.user()]) - .then((_users) => { - users = _users; - return factory.site({ users }); - }) - .then((_site) => { - site = _site; - githubAPINocks.repo({ - accessToken: users[0].githubAccessToken, - owner: site.owner, - repo: site.repository, - response: 404, - }); - githubAPINocks.repo({ - accessToken: users[1].githubAccessToken, - owner: site.owner, - repo: site.repository, - response: 404, - }); - return RepositoryVerifier.verifyRepos(); - }) - .then(() => Site.findByPk(site.id)) - .then((_site) => { - expect(_site.repoLastVerified).deep.equal(site.repoLastVerified); - done(); - }) - .catch(done); + githubAPINocks.repo({ + accessToken: user1.githubAccessToken, + owner: site.owner, + repo: site.repository, + response: 404, + }); + githubAPINocks.repo({ + accessToken: user2.githubAccessToken, + owner: site.owner, + repo: site.repository, + }); + await RepositoryVerifier.verifyRepos(); + const latestSite = await Site.findByPk(site.id); + expect(latestSite.repoLastVerified).gt(site.repoLastVerified); }); - it('not able to verify sites with users without access tokens', (done) => { - let users; - let site; - Promise.all([ - factory.user({ - githubAccessToken: null, - }), - factory.user({ - githubAccessToken: null, - }), - ]) - .then((_users) => { - users = _users; - return factory.site({ users }); - }) - .then((_site) => { - site = _site; - githubAPINocks.repo({ - accessToken: users[0].githubAccessToken, - owner: site.owner, - repo: site.repository, - response: 404, - }); - githubAPINocks.repo({ - accessToken: users[1].githubAccessToken, - owner: site.owner, - repo: site.repository, - response: 404, - }); - return RepositoryVerifier.verifyRepos(); - }) - .then(() => Site.findByPk(site.id)) - .then((_site) => { - expect(_site.repoLastVerified).deep.equal(site.repoLastVerified); - done(); - }) - .catch(done); - }); + it('not able to verify sites with users that cannot access repository', async () => { + const { site, user } = await createSiteUserOrg(); - it('verify sites only with users that have githubAccessToken', (done) => { - let users; - const repoLastVerified = moment().subtract(1, 'day').toDate(); - Site.destroy({ - where: {}, - truncate: true, - }) - .then(() => - factory.site({ - users: [], - repoLastVerified, - }), - ) - .then(() => - factory.user({ - githubAccessToken: null, - repoLastVerified, - }), - ) - .then((user) => - factory.site({ - users: [user], - repoLastVerified, - }), - ) - .then(() => Promise.all([factory.user(), factory.user()])) - .then((_users) => { - users = _users; - return Promise.all([ - factory.site({ - users, - repoLastVerified, - }), - factory.site({ - users, - repoLastVerified, - }), - factory.site({ - users, - repoLastVerified, - }), - ]); - }) - .then((sites) => { - sites.forEach((site) => { - githubAPINocks.repo({ - accessToken: users[0].githubAccessToken, - owner: site.owner, - repo: site.repository, - response: 404, - }); - githubAPINocks.repo({ - accessToken: users[1].githubAccessToken, - owner: site.owner, - repo: site.repository, - }); - }); - return RepositoryVerifier.verifyRepos(); - }) - .then(() => - Site.findAll({ - include: User, - }), - ) - .then((sites) => { - expect(sites.length).to.equal(5); - expect(sites.filter((site) => site.Users.length > 0).length).to.equal(4); - expect( - sites.filter((site) => site.repoLastVerified > repoLastVerified).length, - ).to.equal(3); - done(); - }) - .catch(done); + githubAPINocks.repo({ + accessToken: user.githubAccessToken, + owner: site.owner, + repo: site.repository, + response: 404, + }); + await RepositoryVerifier.verifyRepos(); + + const latestSite = await Site.findByPk(site.id); + expect(latestSite.repoLastVerified).deep.equal(site.repoLastVerified); }); - }); - context('verifyUserRepos', () => { - it('verify sites only with users that have githubAccessToken', (done) => { - const MockRepositoryVerifier = proxyquire( - '../../../../api/services/RepositoryVerifier', - { - './GitHub': MockGitHub, - }, - ); - let user; - let sites; - const repoLastVerified = moment().subtract(1, 'day').toDate(); - factory - .user() - .then((model) => { - user = model; - const owner = 'owner'; - return Promise.all([ - factory.site({ - owner, - repository: 'repo-0', - users: [user], - repoLastVerified, - }), - factory.site({ - owner, - repository: 'repo-1', - users: [user], - repoLastVerified, - }), - factory.site({ - owner, - repository: 'repo-2', - repoLastVerified, - }), - ]); - }) - .then((models) => { - sites = models; - return MockRepositoryVerifier.verifyUserRepos(user); - }) - .then(() => - Site.findAll({ - where: { - id: sites.map((s) => s.id), - }, - }), - ) - .then((models) => { - sites = models; - expect(sites.length).equal(3); - expect( - sites.filter((s) => s.repoLastVerified > repoLastVerified).length, - ).to.equal(2); - done(); - }) - .catch(done); + it('not able to verify sites with users without access tokens', async () => { + const user = await factory.user({ + githubAccessToken: null, + }); + const { site } = await createSiteUserOrg({ user }); + + githubAPINocks.repo({ + accessToken: user.githubAccessToken, + owner: site.owner, + repo: site.repository, + response: 404, + }); + await RepositoryVerifier.verifyRepos(); + const latestSite = await Site.findByPk(site.id); + expect(latestSite.repoLastVerified).deep.equal(site.repoLastVerified); }); }); }); diff --git a/test/api/unit/services/SiteBuildQueue.test.js b/test/api/unit/services/SiteBuildQueue.test.js index 3df4a36b0..fc5ea9e41 100644 --- a/test/api/unit/services/SiteBuildQueue.test.js +++ b/test/api/unit/services/SiteBuildQueue.test.js @@ -5,6 +5,7 @@ const mockTokenRequest = require('../../support/cfAuthNock'); const apiNocks = require('../../support/cfAPINocks'); const config = require('../../../../config'); const factory = require('../../support/factory'); +const { createSiteUserOrg } = require('../../support/site-user'); const GithubBuildHelper = require('../../../../api/services/GithubBuildHelper'); const SiteBuildQueue = require('../../../../api/services/SiteBuildQueue'); const { @@ -577,7 +578,7 @@ describe('SiteBuildQueue', () => { { model: Site, required: true, - include: [User, SiteBranchConfig, Domain], + include: [SiteBranchConfig, Domain], }, User, ], @@ -622,7 +623,7 @@ describe('SiteBuildQueue', () => { { model: Site, required: true, - include: [User, SiteBranchConfig, Domain], + include: [SiteBranchConfig, Domain], }, User, ], @@ -693,52 +694,39 @@ describe('SiteBuildQueue', () => { }); it(`should set GITHUB_TOKEN in the - message to the user's GitHub access token`, (done) => { - let user; + message to the user's GitHub access token`, async () => { + const user = await factory.user({ + githubAccessToken: 'fake-github-token-123', + }); + const { site } = await createSiteUserOrg({ user }); + const build = await factory.build({ + user, + site, + }); - factory - .user({ - githubAccessToken: 'fake-github-token-123', - }) - .then((model) => { - user = model; - return factory.site({ users: [user] }); - }) - .then((site) => - factory.build({ - user, - site, - }), - ) - .then((build) => - Build.findByPk(build.id, { - include: [ - { - model: Site, - required: true, - include: [SiteBranchConfig], - }, - User, - ], - }), - ) - .then((build) => SiteBuildQueue.messageBodyForBuild(build)) - .then((message) => { - expect(messageEnv(message, 'GITHUB_TOKEN')).to.equal('fake-github-token-123'); - done(); - }) - .catch(done); + const fullBuild = await Build.findByPk(build.id, { + include: [ + { + model: Site, + required: true, + include: [SiteBranchConfig], + }, + User, + ], + }); + + const message = await SiteBuildQueue.messageBodyForBuild(fullBuild); + expect(messageEnv(message, 'GITHUB_TOKEN')).to.equal('fake-github-token-123'); }); it(`should find a github access token for a site user when the current user does not have one`, async () => { - const [user1, user2] = await Promise.all([factory.user(), factory.user()]); - await user1.update({ + const user1 = await factory.user({ githubAccessToken: null, }); - const site = await factory.site({ - users: [user1, user2], - }); + const { site, user: user2, org } = await createSiteUserOrg(); + await org.addRoleUser(user1); + const build = await factory.build({ user: user1, site, diff --git a/test/api/unit/services/SiteCreator.test.js b/test/api/unit/services/SiteCreator.test.js index f6eabf968..e2fb5ff46 100644 --- a/test/api/unit/services/SiteCreator.test.js +++ b/test/api/unit/services/SiteCreator.test.js @@ -9,7 +9,7 @@ const mockTokenRequest = require('../../support/cfAuthNock'); const apiNocks = require('../../support/cfAPINocks'); const SiteCreator = require('../../../../api/services/SiteCreator'); const TemplateResolver = require('../../../../api/services/TemplateResolver'); -const { Build, Site, User, SiteBranchConfig } = require('../../../../api/models'); +const { Build, Site, SiteBranchConfig } = require('../../../../api/models'); const QueueJobs = require('../../../../api/queue-jobs'); describe('SiteCreator', () => { @@ -39,8 +39,6 @@ describe('SiteCreator', () => { expect(site.s3ServiceName).to.equal(s3ServiceName); expect(site.awsBucketName).to.equal(awsBucketName); expect(site.awsBucketRegion).to.equal(awsBucketRegion); - expect(site.Users).to.have.length(1); - expect(site.Users[0].id).to.equal(user.id); expect(site.Builds).to.have.length(1); expect(site.Builds[0].user).to.equal(user.id); expect(site.SiteBranchConfigs[0].branch).to.equal(defaultBranch); @@ -53,7 +51,7 @@ describe('SiteCreator', () => { owner, repository, }, - include: [User, Build, SiteBranchConfig], + include: [Build, SiteBranchConfig], }); context('from a GitHub repo', () => { @@ -152,7 +150,7 @@ describe('SiteCreator', () => { context('when the owner of the repo is an authorized federalist org', () => { it(`creates new site record for the given repository, - adds the user, webhook, and build`, (done) => { + adds the webhook and build`, (done) => { const defaultBranch = 'myDefaultBranch'; factory @@ -458,7 +456,7 @@ describe('SiteCreator', () => { }); it(`should create a new site record - for the given repository and add the user`, (done) => { + for the given repository`, (done) => { factory .user() .then((model) => { @@ -481,13 +479,10 @@ describe('SiteCreator', () => { owner: siteParams.owner, repository: siteParams.repository, }, - include: [User], }); }) .then((site) => { expect(site).to.not.be.undefined; - expect(site.Users).to.have.length(1); - expect(site.Users[0].id).to.equal(user.id); done(); }) .catch(done); diff --git a/test/api/unit/services/SiteMembershipCreator.test.js b/test/api/unit/services/SiteMembershipCreator.test.js deleted file mode 100644 index 17f7b7ef2..000000000 --- a/test/api/unit/services/SiteMembershipCreator.test.js +++ /dev/null @@ -1,236 +0,0 @@ -const expect = require('chai').expect; -const nock = require('nock'); -const factory = require('../../support/factory'); -const githubAPINocks = require('../../support/githubAPINocks'); -const SiteMembershipCreator = require('../../../../api/services/SiteMembershipCreator'); -const config = require('../../../../config'); - -describe('SiteMembershipCreator', () => { - describe('.createSiteMembership({ siteParams, user })', () => { - beforeEach(() => { - githubAPINocks.repo({ - response: [ - 200, - { - permissions: { - admin: false, - push: true, - }, - }, - ], - }); - }); - - it('should add the user to the site', (done) => { - let site; - let user; - - Promise.props({ - userProm: factory.user(), - siteProm: factory.site(), - }) - .then(({ siteProm, userProm }) => { - user = userProm; - site = siteProm; - - return SiteMembershipCreator.createSiteMembership({ - user, - siteParams: { - owner: site.owner, - repository: site.repository, - }, - }); - }) - .then(() => - site.getUsers({ - where: { - id: user.id, - }, - }), - ) - .then((users) => { - expect(users).to.have.lengthOf(1); - done(); - }) - .catch(done); - }); - - it('should reject with 404 if the site does not exist', (done) => { - factory - .user() - .then((user) => - SiteMembershipCreator.createSiteMembership({ - user, - siteParams: { - owner: 'not-a', - repository: 'real-site', - }, - }), - ) - .catch((err) => { - expect(err.status).to.equal(404); - expect(err.message).to.equal('The site you are trying to add does not exist'); - done(); - }) - .catch(done); - }); - - it('should reject if the user does not have write access to the site', (done) => { - let site; - let user; - - Promise.props({ - userProm: factory.user({ - githubAccessToken: 'mytoken', - }), - siteProm: factory.site(), - }) - .then(({ siteProm, userProm }) => { - user = userProm; - site = siteProm; - - nock.cleanAll(); - githubAPINocks.repo({ - accessToken: user.githubAccessToken, - owner: site.owner, - repo: site.repository, - response: [ - 200, - { - permissions: { - admin: false, - push: false, - }, - }, - ], - }); - - githubAPINocks.getOrganizationMembers({ - accessToken: user.githubAccessToken, - organization: 'federalist-users', - role: 'admin', - }); - githubAPINocks.getOrganizationMembers({ - accessToken: user.githubAccessToken, - organization: 'federalist-users', - role: 'admin', - page: 2, - response: [], - }); - - return SiteMembershipCreator.createSiteMembership({ - user, - siteParams: { - owner: site.owner, - repository: site.repository, - }, - }); - }) - .catch((err) => { - expect(err.status).to.eq(400); - expect(err.message).to.equal('You do not have write access to this repository'); - done(); - }) - .catch(done); - }); - - it('should reject if the user has already added the site', (done) => { - const userProm = factory.user(); - const siteProm = factory.site({ - users: Promise.all([userProm]), - }); - - Promise.props({ - user: userProm, - site: siteProm, - }) - .then(({ user, site }) => - SiteMembershipCreator.createSiteMembership({ - user, - siteParams: { - owner: site.owner, - repository: site.repository, - }, - }), - ) - .catch((err) => { - expect(err.status).to.equal(400); - expect(err.message).to.equal( - `You've already added this site to ${config.app.appName}`, - ); - done(); - }) - .catch(done); - }); - - it(`should reject if the user has already added the site - and the name is different case`, (done) => { - const userProm = factory.user(); - const siteProm = factory.site({ - users: Promise.all([userProm]), - }); - - Promise.props({ - user: userProm, - site: siteProm, - }) - .then(({ user, site }) => - SiteMembershipCreator.createSiteMembership({ - user, - siteParams: { - owner: site.owner.toUpperCase(), - repository: site.repository.toUpperCase(), - }, - }), - ) - .catch((err) => { - expect(err.status).to.equal(400); - expect(err.message).to.equal( - `You've already added this site to ${config.app.appName}`, - ); - done(); - }) - .catch(done); - }); - - it('should reject if the repository does not exist', (done) => { - let site; - - Promise.props({ - user: factory.user(), - site: factory.site(), - }) - .then((models) => { - site = models.site; - - nock.cleanAll(); - githubAPINocks.repo({ - accessToken: models.user.accessToken, - owner: site.owner, - repo: site.repository, - response: [ - 404, - { - message: 'Not Found', - }, - ], - }); - return SiteMembershipCreator.createSiteMembership({ - user: models.user, - siteParams: { - owner: site.owner.toUpperCase(), - repository: site.repository.toUpperCase(), - }, - }); - }) - .catch((err) => { - expect(err.status).to.eq(400); - expect(err.message).to.eq( - `The repository ${site.owner}/${site.repository} does not exist.`, - ); - done(); - }) - .catch(done); - }); - }); -}); diff --git a/test/api/unit/services/SocketIOSubscriber.test.js b/test/api/unit/services/SocketIOSubscriber.test.js index 83ded724f..77c7d71f0 100644 --- a/test/api/unit/services/SocketIOSubscriber.test.js +++ b/test/api/unit/services/SocketIOSubscriber.test.js @@ -2,7 +2,8 @@ const { expect } = require('chai'); const factory = require('../../support/factory'); const SocketIOSubscriber = require('../../../../api/services/SocketIOSubscriber'); const MockSocket = require('../../support/mockSocket'); -const { Organization, Role, Site, SiteUser, User } = require('../../../../api/models'); +const { Organization, Site, User } = require('../../../../api/models'); +const { createSiteUserOrg } = require('../../support/site-user'); const times = (length, fn) => Array.from({ length }, fn); @@ -17,10 +18,6 @@ describe('SocketIOSubscriber', () => { force: true, cascade: true, }), - SiteUser.truncate({ - force: true, - cascade: true, - }), Organization.truncate({ force: true, cascade: true, @@ -32,13 +29,7 @@ describe('SocketIOSubscriber', () => { it('a user with sites joinsRooms(socket)', async () => { const numSites = 5; const user = await factory.user(); - await Promise.all( - times(numSites, () => - factory.site({ - users: [user], - }), - ), - ); + await Promise.all(times(numSites, () => createSiteUserOrg({ user }))); const socket = new MockSocket(user); await SocketIOSubscriber.joinRooms(socket); @@ -60,16 +51,8 @@ describe('SocketIOSubscriber', () => { const user2NumSites = 1; const [user1, user2] = await Promise.all(times(2, () => factory.user())); await Promise.all([ - ...times(user1NumSites, () => - factory.site({ - users: [user1], - }), - ), - ...times(user2NumSites, () => - factory.site({ - users: [user2], - }), - ), + ...times(user1NumSites, () => createSiteUserOrg({ user: user1 })), + ...times(user2NumSites, () => createSiteUserOrg({ user: user2 })), ]); const socket1 = new MockSocket(user1); const socket2 = new MockSocket(user2); @@ -89,21 +72,12 @@ describe('SocketIOSubscriber', () => { const bothNumSites = 2; const [user1, user2] = await Promise.all(times(2, () => factory.user())); await Promise.all([ - ...times(user1NumSites, () => - factory.site({ - users: [user1], - }), - ), - ...times(user2NumSites, () => - factory.site({ - users: [user2], - }), - ), - ...times(bothNumSites, () => - factory.site({ - users: [user1, user2], - }), - ), + ...times(user1NumSites, () => createSiteUserOrg({ user: user1 })), + ...times(user2NumSites, () => createSiteUserOrg({ user: user2 })), + ...times(bothNumSites, async () => { + const { org } = await createSiteUserOrg({ user: user1 }); + await org.addRoleUser(user2); + }), ]); const socket1 = new MockSocket(user1); const socket2 = new MockSocket(user2); @@ -122,13 +96,7 @@ describe('SocketIOSubscriber', () => { it('no user with sites joinsRooms(socket)', async () => { const numSites = 5; const user = await factory.user(); - await Promise.all( - times(numSites, () => - factory.site({ - users: [user], - }), - ), - ); + await Promise.all(times(numSites, () => createSiteUserOrg({ user }))); const socket = new MockSocket(); await SocketIOSubscriber.joinRooms(socket); @@ -140,11 +108,10 @@ describe('SocketIOSubscriber', () => { const numSites = 4; const user = await factory.user(); const [site1, site2] = await Promise.all( - times(numSites, () => - factory.site({ - users: [user], - }), - ), + times(numSites, async () => { + const { site } = await createSiteUserOrg({ user }); + return site; + }), ); await user.update({ buildNotificationSettings: { @@ -161,41 +128,5 @@ describe('SocketIOSubscriber', () => { [...socket.rooms].filter((room) => room.endsWith(`user-${user.id}`)).length, ).to.eql(1); }); - - it('organizations too!', async () => { - const [org, userRole, user] = await Promise.all([ - factory.organization.create(), - Role.findOne({ - where: { - name: 'user', - }, - }), - factory.user(), - ]); - const [site1, site2] = await Promise.all([ - factory.site({ - users: [user], - }), - factory.site(), - factory.site({ - users: [user], - }), - factory.site(), - ]); - await Promise.all([ - org.addSite(site1), - org.addSite(site2), - org.addUser(user, { - through: { - roleId: userRole.id, - }, - }), - ]); - const socket = new MockSocket(user); - - await SocketIOSubscriber.joinRooms(socket); - - expect(socket.rooms.size).to.eql(3 + 1); - }); }); }); diff --git a/test/api/unit/services/Webhooks.test.js b/test/api/unit/services/Webhooks.test.js index 0e833d148..02d70e3c9 100644 --- a/test/api/unit/services/Webhooks.test.js +++ b/test/api/unit/services/Webhooks.test.js @@ -9,6 +9,7 @@ const GithubBuildHelper = require('../../../../api/services/GithubBuildHelper'); const factory = require('../../support/factory'); const githubAPINocks = require('../../support/githubAPINocks'); +const { createSiteUserOrg } = require('../../support/site-user'); const Webhooks = require('../../../../api/services/Webhooks'); @@ -75,10 +76,7 @@ describe('Webhooks Service', () => { }); it('should create a new site build for the sender', async () => { - const user = await factory.user(); - const site = await factory.site({ - users: [user], - }); + const { site, user } = await createSiteUserOrg(); const numBuildsBefore = await Build.count({ where: { site: site.id, @@ -107,7 +105,7 @@ describe('Webhooks Service', () => { with the site for the sender if no user exists`, async () => { const username = crypto.randomBytes(3).toString('hex'); - const site = await factory.site(); + const { site } = await createSiteUserOrg(); const payload = buildWebhookPayload({ username }, site); await Webhooks.pushWebhookRequest(payload); @@ -126,10 +124,7 @@ describe('Webhooks Service', () => { it(`should find the site by the lowercased owner and repository and upper cased github user`, async () => { const reporterSpy = sinon.stub(GithubBuildHelper, 'reportBuildStatus').resolves(); - const user = await factory.user(); - const site = await factory.site({ - users: [user], - }); + const { site, user } = await createSiteUserOrg(); user.username = user.username.toUpperCase(); @@ -153,46 +148,30 @@ describe('Webhooks Service', () => { expect(reporterSpy.args[0][0].id).to.equal(build.id); }); - it('should report the status of the new build to GitHub', (done) => { + it('should report the status of the new build to GitHub', async () => { nock.cleanAll(); const statusNock = githubAPINocks.status({ state: 'pending', }); - const userProm = factory.user(); - const siteProm = factory.site({ - users: Promise.all([userProm]), + const { site, user } = await createSiteUserOrg(); + const payload = buildWebhookPayload(user, site); + const fullName = `${site.owner.toUpperCase()}/${site.repository.toUpperCase()}`; + payload.repository.full_name = fullName; + + githubAPINocks.repo({ + accessToken: user.githubAccessToken, + owner: site.owner, + repo: site.repository, + username: user.username, }); - Promise.props({ - user: userProm, - site: siteProm, - }) - .then(({ user, site }) => { - const payload = buildWebhookPayload(user, site); - const fullName = `${site.owner.toUpperCase()}/${site.repository.toUpperCase()}`; - payload.repository.full_name = fullName; - - githubAPINocks.repo({ - accessToken: user.githubAccessToken, - owner: site.owner, - repo: site.repository, - username: user.username, - }); - return Webhooks.pushWebhookRequest(payload); - }) - .then(() => { - expect(statusNock.isDone()).to.be.true; - done(); - }) - .catch(done); + await Webhooks.pushWebhookRequest(payload); + expect(statusNock.isDone()).to.be.true; }); it('should not schedule a build if there are no new commits', async () => { - const user = await factory.user(); - const site = await factory.site({ - users: [user], - }); + const { site, user } = await createSiteUserOrg(); const numBuildsBefore = await Build.count({ where: { site: site.id, @@ -230,11 +209,10 @@ describe('Webhooks Service', () => { }); it('sites should not build if site is inactive', async () => { - const user = await factory.user(); const site = await factory.site({ - users: [user], isActive: false, }); + const { user } = await createSiteUserOrg({ site }); const payload = buildWebhookPayload(user, { owner: site.owner, repository: site.repository, @@ -249,13 +227,9 @@ describe('Webhooks Service', () => { }); it('sites should build if organization is active', async () => { - const user = await factory.user(); - const org = await factory.organization.create(); + const { site, user, org } = await createSiteUserOrg(); expect(org.isActive).to.be.true; - const site = await factory.site({ - users: [user], - organizationId: org.id, - }); + const numBuildsBefore = await Build.count({ where: { site: site.id, @@ -281,13 +255,10 @@ describe('Webhooks Service', () => { }); it('sites should not build if organization is inactive', async () => { - const user = await factory.user(); const org = await factory.organization.create({ isActive: false }); + const { site, user } = await createSiteUserOrg({ org }); expect(org.isActive).to.be.false; - const site = await factory.site({ - users: [user], - organizationId: org.id, - }); + const numBuildsBefore = await Build.count({ where: { site: site.id, @@ -314,10 +285,7 @@ describe('Webhooks Service', () => { describe('when a queued build for the branch exists', () => { it('should not create a new build', async () => { - const user = await factory.user(); - const site = await factory.site({ - users: [user], - }); + const { site, user } = await createSiteUserOrg(); await Build.create( { site: site.id, @@ -357,11 +325,10 @@ describe('Webhooks Service', () => { it('should update the requestedCommitSha and user of build', async () => { const branch = 'main'; const origSha = 'aaa2b66a31319d456a448041a5b3c2a70c32d8b7'; - const userProms = Promise.all([factory.user(), factory.user()]); - const site = await factory.site({ - users: userProms, - }); - const [user1, user2] = await userProms; + const { site, user: user1, org } = await createSiteUserOrg(); + const user2 = await factory.user(); + await org.addRoleUser(user2); + await Build.create( { site: site.id, @@ -406,11 +373,7 @@ describe('Webhooks Service', () => { state: 'pending', }); - const userProm = factory.user(); - const site = await factory.site({ - users: Promise.all([userProm]), - }); - const user = await userProm; + const { site, user } = await createSiteUserOrg(); await Build.create( { site: site.id, @@ -443,11 +406,7 @@ describe('Webhooks Service', () => { describe('when a created build for the branch exists', () => { it('should not create a new build', async () => { - const userProm = factory.user(); - const site = await factory.site({ - users: Promise.all([userProm]), - }); - const user = await userProm; + const { site, user } = await createSiteUserOrg(); await Build.create( { site: site.id, @@ -487,11 +446,9 @@ describe('Webhooks Service', () => { it('should update the requestedCommitSha and user of build', async () => { const branch = 'main'; const origSha = 'aaa2b66a31319d456a448041a5b3c2a70c32d8b7'; - const userProms = Promise.all([factory.user(), factory.user()]); - const site = await factory.site({ - users: userProms, - }); - const [user1, user2] = await userProms; + const { site, user: user1, org } = await createSiteUserOrg(); + const user2 = await factory.user(); + await org.addRoleUser(user2); await Build.create( { site: site.id, @@ -536,11 +493,7 @@ describe('Webhooks Service', () => { state: 'pending', }); - const userProm = factory.user(); - const site = await factory.site({ - users: Promise.all([userProm]), - }); - const user = await userProm; + const { site, user } = await createSiteUserOrg(); await Build.create( { site: site.id, diff --git a/test/frontend/actions/actionCreators/siteActionsTest.js b/test/frontend/actions/actionCreators/siteActionsTest.js index 1a455b921..56f8a209b 100644 --- a/test/frontend/actions/actionCreators/siteActionsTest.js +++ b/test/frontend/actions/actionCreators/siteActionsTest.js @@ -112,74 +112,4 @@ describe('siteActions actionCreators', () => { expect(siteDeletedType).to.equal('SITE_DELETED'); }); }); - - describe('siteUserAdded', () => { - it('constructs properly', () => { - const site = { - id: 123, - owner: 'owner', - repository: 'a-repo', - }; - - const action = siteUserAdded(site); - expect(action.type).to.equal(siteUserAddedType); - expect(action.site).to.equal(site); - }); - - it('exports its type', () => { - expect(siteUserAddedType).to.equal('SITE_USER_ADDED'); - }); - }); - - describe('siteUserRemoved', () => { - it('constructs properly', () => { - const site = { - id: 1, - owner: 'someone', - repository: 'something', - }; - const action = siteUserRemoved(site); - expect(action.type).to.equal(siteUserRemovedType); - expect(action.site).to.equal(site); - }); - }); - - describe('basicAuthSaved', () => { - it('constructs properly', () => { - const site = { - id: 1, - owner: 'someone', - repository: 'something', - }; - const basicAuth = { - username: 'username', - password: 'password', - }; - - const actual = siteBasicAuthSaved(site, basicAuth); - expect(actual).to.deep.equal({ - type: siteBasicAuthSavedType, - siteId: site.id, - site, - }); - }); - }); - - describe('basicAuthRemoved', () => { - it('constructs properly', () => { - const site = { - id: 1, - owner: 'someone', - repository: 'something', - }; - - const actual = siteBasicAuthRemoved(site); - - expect(actual).to.deep.equal({ - type: siteBasicAuthRemovedType, - siteId: site.id, - site, - }); - }); - }); }); diff --git a/test/frontend/reducers/sitesTest.js b/test/frontend/reducers/sitesTest.js index 2fbfe86f7..ba2db3ee4 100644 --- a/test/frontend/reducers/sitesTest.js +++ b/test/frontend/reducers/sitesTest.js @@ -14,10 +14,6 @@ describe('sitesReducer', () => { const SITE_BRANCHES_RECEIVED = 'branches received'; const SITES_RECEIVED = 'hey, sites!'; const BUILD_RESTARTED = 'build restarted!'; - const SITE_USER_ADDED = 'site user added'; - const SITE_USER_REMOVED = 'SITE_USER_REMOVED'; - const SITE_BASIC_AUTH_SAVED = 'SITE_BASIC_AUTH_SAVED'; - const SITE_BASIC_AUTH_REMOVED = 'SITE_BASIC_AUTH_REMOVED'; const initialState = { isLoading: false, data: undefined, @@ -32,9 +28,6 @@ describe('sitesReducer', () => { siteUpdatedType: SITE_UPDATED, siteBranchesReceivedType: SITE_BRANCHES_RECEIVED, siteDeletedType: SITE_DELETED, - siteUserAddedType: SITE_USER_ADDED, - siteBasicAuthSavedType: SITE_BASIC_AUTH_SAVED, - siteBasicAuthRemovedType: SITE_BASIC_AUTH_REMOVED, }, '../actions/actionCreators/buildActions': { buildRestartedType: BUILD_RESTARTED, @@ -295,149 +288,4 @@ describe('sitesReducer', () => { expect(actual.data).to.deep.equal([siteOne]); }); - - it("adds site to state's data when SITE_USER_ADDED", () => { - const siteAdded = { - id: 55, - }; - const actual = fixture( - { - isLoading: false, - data: [], - }, - { - type: SITE_USER_ADDED, - site: siteAdded, - }, - ); - - expect(actual.isLoading).to.be.false; - expect(actual.data).to.deep.equal([siteAdded]); - }); - - it('returns existing state when SITE_USER_ADDED if action has no site', () => { - const actual = fixture( - { - isLoading: false, - data: [], - }, - { - type: SITE_USER_ADDED, - }, - ); - - expect(actual.isLoading).to.be.false; - expect(actual.data).to.deep.equal([]); - }); - - it('returns existing state with removed user information on SITE_USER_REMOVED', () => { - const oldSite = { - id: 1, - owner: 'person1', - repository: 'a', - users: [{ username: 'james' }, { username: 'jane' }], - }; - - const state = { - isLoading: false, - data: [ - oldSite, - { - id: 2, - owner: 'person2', - repository: 'b', - users: [], - }, - ], - }; - const updatedSite = { - ...oldSite, - users: [{ username: 'jane' }], - }; - const actual = siteReducer(state, { - type: SITE_USER_REMOVED, - site: updatedSite, - }); - - expect(actual.data.length).to.equal(2); - expect(actual.data[0].users[0].username).to.equal('jane'); - expect(actual.data[1]).to.deep.equal(state.data[1]); - }); - - it("updates existing site data when given an save basic auth action and the new site's id is found", () => { - const siteOne = { - id: 'siteToKeep', - oldData: true, - }; - - const siteTwo = { - id: 'anotherSiteToKeep', - oldData: true, - }; - - const existingSites = [siteOne, siteTwo]; - - const newSite = { - id: 'siteToKeep', - oldData: false, - hi: 'there', - basicAuth: { - username: 'username', - password: 'password', - }, - }; - - const actual = fixture( - { - isLoading: false, - data: existingSites, - }, - { - type: SITE_BASIC_AUTH_SAVED, - siteId: 'siteToKeep', - site: newSite, - }, - ); - - expect(actual.data).to.deep.equal([newSite, siteTwo]); - }); - - it("updates existing site data when given an save basic auth action and the new site's id is found", () => { - const siteOne = { - id: 'siteToKeep', - oldData: true, - basicAuth: { - username: 'username', - password: 'password', - }, - }; - - const siteTwo = { - id: 'anotherSiteToKeep', - oldData: true, - }; - - const existingSites = [siteOne, siteTwo]; - - const newSite = { - id: 'siteToKeep', - oldData: false, - hi: 'there', - basicAuth: {}, - }; - - const actual = fixture( - { - isLoading: false, - data: existingSites, - }, - { - type: SITE_BASIC_AUTH_REMOVED, - siteId: 'siteToKeep', - site: newSite, - }, - ); - - expect(actual.data).to.deep.equal([newSite, siteTwo]); - }); }); diff --git a/test/frontend/selectors/organizationTest.js b/test/frontend/selectors/organizationTest.js index 68322e288..fb3f1b5e9 100644 --- a/test/frontend/selectors/organizationTest.js +++ b/test/frontend/selectors/organizationTest.js @@ -72,16 +72,12 @@ describe('organzizationSelectors', () => { }); describe('.orgFilterOptions', () => { - it('returns array of objects with id and name while adding the "All" and "Unassociated" options', () => { + it('returns array of objects with id and name while adding the "All" option', () => { const grouped = orgFilterOptions(state); expect(grouped[0]).to.deep.equal({ id: 'all-options', name: 'All', }); - expect(grouped[grouped.length - 1]).to.deep.equal({ - id: 'unassociated', - name: 'Sites without an organization', - }); grouped.map((group) => expect(group).to.have.keys(['id', 'name'])); }); diff --git a/test/frontend/selectors/siteTest.js b/test/frontend/selectors/siteTest.js index 51ee8bc37..1189e4231 100644 --- a/test/frontend/selectors/siteTest.js +++ b/test/frontend/selectors/siteTest.js @@ -51,13 +51,6 @@ describe('siteSelectors', () => { expect(grouped.data).to.deep.equal(sites.data); }); - it('returns all sites without an organization Id associated', () => { - const orgId = 'unassociated'; - const grouped = groupSitesByOrg(sites, orgId); - expect(grouped.data).to.have.length(1); - expect(grouped.data).to.deep.equal([sites.data[3]]); - }); - it('returns an sites data as an empty array with organization Id is not associated to any sites', () => { const orgId = 100; const grouped = groupSitesByOrg(sites, orgId);