From 4acf22d650b735ed453157364eb250e0ea7060da Mon Sep 17 00:00:00 2001 From: Thomas Sibley Date: Tue, 23 Aug 2022 15:26:58 -0700 Subject: [PATCH] Add RESTful API endpoints for managing Groups members Read-write access for group owners, read-only access for all other members of a group, and no access for non-members. Expands the server's AWS IAM policy to allow access to a few required Cognito user pool API actions. The policy files are .tftpl.json instead of .json.tftpl to indicate that although templated they must still remain valid JSON (at least for the time being due to automatic rewriting by scripts/migrate-group). --- aws/iam/main.tf | 6 +- ...NextstrainDotOrgServerInstance.tftpl.json} | 12 +++ ...tstrainDotOrgServerInstanceDev.tftpl.json} | 12 +++ aws/iam/variable-COGNITO_USER_POOL_ID.tf | 1 + aws/variable-COGNITO_USER_POOL_ID.tf | 4 + env/production/terraform.tf | 1 + env/testing/terraform.tf | 1 + scripts/migrate-group.js | 4 +- src/app.js | 15 ++++ src/cognito.js | 85 ++++++++++++++++++ src/endpoints/groups.js | 83 ++++++++++++++++++ src/groups.js | 87 +++++++++++++++++++ src/utils/iterators.js | 39 +++++++++ 13 files changed, 347 insertions(+), 3 deletions(-) rename aws/iam/policy/{NextstrainDotOrgServerInstance.json => NextstrainDotOrgServerInstance.tftpl.json} (87%) rename aws/iam/policy/{NextstrainDotOrgServerInstanceDev.json => NextstrainDotOrgServerInstanceDev.tftpl.json} (82%) create mode 120000 aws/iam/variable-COGNITO_USER_POOL_ID.tf create mode 100644 aws/variable-COGNITO_USER_POOL_ID.tf create mode 100644 src/cognito.js create mode 100644 src/utils/iterators.js diff --git a/aws/iam/main.tf b/aws/iam/main.tf index c79b22abc..313920748 100644 --- a/aws/iam/main.tf +++ b/aws/iam/main.tf @@ -42,7 +42,11 @@ resource "aws_iam_policy" "server" { name = local.server_policy_name description = local.server_policy_description - policy = file("${path.module}/policy/${local.server_policy_file_name}.json") + policy = templatefile( + "${path.module}/policy/${local.server_policy_file_name}.tftpl.json", { + COGNITO_USER_POOL_ID = var.COGNITO_USER_POOL_ID + } + ) } moved { diff --git a/aws/iam/policy/NextstrainDotOrgServerInstance.json b/aws/iam/policy/NextstrainDotOrgServerInstance.tftpl.json similarity index 87% rename from aws/iam/policy/NextstrainDotOrgServerInstance.json rename to aws/iam/policy/NextstrainDotOrgServerInstance.tftpl.json index 4d8916bee..730c0d347 100644 --- a/aws/iam/policy/NextstrainDotOrgServerInstance.json +++ b/aws/iam/policy/NextstrainDotOrgServerInstance.tftpl.json @@ -70,6 +70,18 @@ "Resource": [ "arn:aws:s3:::nextstrain-groups/*" ] + }, + { + "Sid": "CognitoUserPoolActions", + "Effect": "Allow", + "Action": [ + "cognito-idp:AdminAddUserToGroup", + "cognito-idp:AdminRemoveUserFromGroup", + "cognito-idp:ListUsersInGroup" + ], + "Resource": [ + "arn:aws:cognito-idp:us-east-1:827581582529:userpool/${COGNITO_USER_POOL_ID}" + ] } ] } diff --git a/aws/iam/policy/NextstrainDotOrgServerInstanceDev.json b/aws/iam/policy/NextstrainDotOrgServerInstanceDev.tftpl.json similarity index 82% rename from aws/iam/policy/NextstrainDotOrgServerInstanceDev.json rename to aws/iam/policy/NextstrainDotOrgServerInstanceDev.tftpl.json index 3495e82da..1d56a5757 100644 --- a/aws/iam/policy/NextstrainDotOrgServerInstanceDev.json +++ b/aws/iam/policy/NextstrainDotOrgServerInstanceDev.tftpl.json @@ -61,6 +61,18 @@ "arn:aws:s3:::nextstrain-groups/test/*", "arn:aws:s3:::nextstrain-groups/test-private/*" ] + }, + { + "Sid": "CognitoUserPoolActions", + "Effect": "Allow", + "Action": [ + "cognito-idp:AdminAddUserToGroup", + "cognito-idp:AdminRemoveUserFromGroup", + "cognito-idp:ListUsersInGroup" + ], + "Resource": [ + "arn:aws:cognito-idp:us-east-1:827581582529:userpool/${COGNITO_USER_POOL_ID}" + ] } ] } diff --git a/aws/iam/variable-COGNITO_USER_POOL_ID.tf b/aws/iam/variable-COGNITO_USER_POOL_ID.tf new file mode 120000 index 000000000..24e6467dd --- /dev/null +++ b/aws/iam/variable-COGNITO_USER_POOL_ID.tf @@ -0,0 +1 @@ +../variable-COGNITO_USER_POOL_ID.tf \ No newline at end of file diff --git a/aws/variable-COGNITO_USER_POOL_ID.tf b/aws/variable-COGNITO_USER_POOL_ID.tf new file mode 100644 index 000000000..fdff37a27 --- /dev/null +++ b/aws/variable-COGNITO_USER_POOL_ID.tf @@ -0,0 +1,4 @@ +variable "COGNITO_USER_POOL_ID" { + type = string + description = "Id of the Cognito user pool for this environment" +} diff --git a/env/production/terraform.tf b/env/production/terraform.tf index 32d7b2741..5f91023dc 100644 --- a/env/production/terraform.tf +++ b/env/production/terraform.tf @@ -52,6 +52,7 @@ module "iam" { } env = "production" + COGNITO_USER_POOL_ID = module.cognito.COGNITO_USER_POOL_ID } moved { diff --git a/env/testing/terraform.tf b/env/testing/terraform.tf index 8f092a05b..920d4cf9b 100644 --- a/env/testing/terraform.tf +++ b/env/testing/terraform.tf @@ -46,4 +46,5 @@ module "iam" { } env = "testing" + COGNITO_USER_POOL_ID = module.cognito.COGNITO_USER_POOL_ID } diff --git a/scripts/migrate-group.js b/scripts/migrate-group.js index c45d12050..7ba875b5a 100755 --- a/scripts/migrate-group.js +++ b/scripts/migrate-group.js @@ -175,8 +175,8 @@ async function s3ListObjects({group}) { async function updateServerPolicies({dryRun = true, oldBucket}) { const policyFiles = [ - "aws/iam/policy/NextstrainDotOrgServerInstance.json", - "aws/iam/policy/NextstrainDotOrgServerInstanceDev.json", + "aws/iam/policy/NextstrainDotOrgServerInstance.tftpl.json", + "aws/iam/policy/NextstrainDotOrgServerInstanceDev.tftpl.json", ]; const updatedFiles = await updatePolicyFiles({dryRun, policyFiles, oldBucket}); diff --git a/src/app.js b/src/app.js index 910f5ec46..8db9a8960 100644 --- a/src/app.js +++ b/src/app.js @@ -333,6 +333,21 @@ app.routeAsync("/groups/:groupName/settings/overview") .deleteAsync(deleteGroupOverview) ; +app.routeAsync("/groups/:groupName/settings/members") + .getAsync(endpoints.groups.listMembers); + +app.routeAsync("/groups/:groupName/settings/roles") + .getAsync(endpoints.groups.listRoles); + +app.routeAsync("/groups/:groupName/settings/roles/:roleName/members") + .getAsync(endpoints.groups.listRoleMembers); + +app.routeAsync("/groups/:groupName/settings/roles/:roleName/members/:username") + .getAsync(endpoints.groups.getRoleMember) + .putAsync(endpoints.groups.putRoleMember) + .deleteAsync(endpoints.groups.deleteRoleMember) +; + app.route("/groups/:groupName/settings/*") .all(() => { throw new NotFound(); }); diff --git a/src/cognito.js b/src/cognito.js new file mode 100644 index 000000000..47666ff34 --- /dev/null +++ b/src/cognito.js @@ -0,0 +1,85 @@ +/** + * Cognito user pool (IdP) management. + * + * @module cognito + */ +/* eslint-disable no-await-in-loop */ +import { + CognitoIdentityProviderClient, + AdminAddUserToGroupCommand, + AdminRemoveUserFromGroupCommand, + paginateListUsersInGroup, + UserNotFoundException, +} from "@aws-sdk/client-cognito-identity-provider"; + +import { COGNITO_USER_POOL_ID } from "./config.js"; +import { NotFound } from "./httpErrors.js"; + + +const REGION = COGNITO_USER_POOL_ID.split("_")[0]; + +const cognito = new CognitoIdentityProviderClient({ region: REGION }); + + +/** + * Retrieve AWS Cognito users in a Cognito group. + * + * @param {string} name - Name of the AWS Cognito group + * @yields {object} user, see + */ +export async function* listUsersInGroup(name) { + const paginator = paginateListUsersInGroup({client: cognito}, { + UserPoolId: COGNITO_USER_POOL_ID, + GroupName: name, + }); + + for await (const page of paginator) { + yield* page.Users; + } +} + + +/** + * Add an AWS Cognito user to a Cognito group. + * + * @param {string} username + * @param {string} group - Name of the AWS Cognito group + * @throws {NotFound} if username is unknown + */ +export async function addUserToGroup(username, group) { + try { + await cognito.send(new AdminAddUserToGroupCommand({ + UserPoolId: COGNITO_USER_POOL_ID, + Username: username, + GroupName: group, + })); + } catch (err) { + if (err instanceof UserNotFoundException) { + throw new NotFound(`unknown user: ${username}`); + } + throw err; + } +} + + +/** + * Remove an AWS Cognito user from a Cognito group. + * + * @param {string} username + * @param {string} group - Name of the AWS Cognito group + * @throws {NotFound} if username is unknown + */ +export async function removeUserFromGroup(username, group) { + try { + await cognito.send(new AdminRemoveUserFromGroupCommand({ + UserPoolId: COGNITO_USER_POOL_ID, + Username: username, + GroupName: group, + })); + } catch (err) { + if (err instanceof UserNotFoundException) { + throw new NotFound(`unknown user: ${username}`); + } + throw err; + } +} diff --git a/src/endpoints/groups.js b/src/endpoints/groups.js index 463122713..c2a029963 100644 --- a/src/endpoints/groups.js +++ b/src/endpoints/groups.js @@ -1,7 +1,9 @@ import * as authz from "../authz/index.js"; +import { NotFound } from "../httpErrors.js"; import { Group } from "../groups.js"; import {contentTypesProvided, contentTypesConsumed} from "../negotiate.js"; import {deleteByUrls, proxyFromUpstream, proxyToUpstream} from "../upstream.js"; +import { slurp } from "../utils/iterators.js"; const setGroup = (nameExtractor) => (req, res, next) => { @@ -149,12 +151,93 @@ async function receiveGroupLogo(req, res) { } +/* Members and roles + */ +const listMembers = async (req, res) => { + const group = req.context.group; + + authz.assertAuthorized(req.user, authz.actions.Read, group); + + return res.json(await group.members()); +}; + + +const listRoles = (req, res) => { + const group = req.context.group; + + authz.assertAuthorized(req.user, authz.actions.Read, group); + + const roles = [...group.membershipRoles.keys()]; + return res.json(roles.map(name => ({name}))); +}; + + +const listRoleMembers = async (req, res) => { + const group = req.context.group; + const {roleName} = req.params; + + authz.assertAuthorized(req.user, authz.actions.Read, group); + + return res.json(await slurp(group.membersWithRole(roleName))); +}; + + +const getRoleMember = async (req, res) => { + const group = req.context.group; + const {roleName, username} = req.params; + + authz.assertAuthorized(req.user, authz.actions.Read, group); + + for await (const member of group.membersWithRole(roleName)) { + if (member.username === username) { + return res.status(204).end(); + } + } + + throw new NotFound(`user ${username} does not have role ${roleName} in group ${group.name}`); +}; + + +const putRoleMember = async (req, res) => { + const group = req.context.group; + const {roleName, username} = req.params; + + authz.assertAuthorized(req.user, authz.actions.Write, group); + + await group.grantRole(roleName, username); + + return res.status(204).end(); +}; + + +const deleteRoleMember = async (req, res) => { + const group = req.context.group; + const {roleName, username} = req.params; + + authz.assertAuthorized(req.user, authz.actions.Write, group); + + await group.revokeRole(roleName, username); + + return res.status(204).end(); +}; + + export { setGroup, + getGroupLogo, putGroupLogo, deleteGroupLogo, + getGroupOverview, putGroupOverview, deleteGroupOverview, + + listMembers, + listRoles, + listRoleMembers, + + getRoleMember, + putRoleMember, + deleteRoleMember, }; diff --git a/src/groups.js b/src/groups.js index 4cab98791..c93dba49f 100644 --- a/src/groups.js +++ b/src/groups.js @@ -1,8 +1,11 @@ +/* eslint-disable no-await-in-loop */ import { strict as assert } from 'assert'; import * as authz from "./authz/index.js"; +import * as cognito from "./cognito.js"; import { PRODUCTION } from './config.js'; import { NotFound } from './httpErrors.js'; import { GroupSource } from "./sources/index.js"; +import { markUserStaleBeforeNow } from "./user.js"; /* eslint-disable-next-line import/first, import/newline-after-import */ import { readFile } from 'fs/promises'; @@ -119,6 +122,90 @@ class Group { get authzTags() { return new Set([authz.tags.Type.Group]); } + + + /** + * List all group members. + * + * @returns {Array.<{username: String, roles: Set.}>} + */ + async members() { + const members = new Map(); + + for (const role of this.membershipRoles.keys()) { + for await (const {username} of this.membersWithRole(role)) { + const member = members.get(username) ?? { + username, + roles: new Set([]), + }; + + member.roles.add(role); + members.set(username, member); + } + } + + return [...members.values()]; + } + + /** + * List group members with a given role. + * + * @param {String} role - e.g. viewers, editors, or owners + * @yields {{username: String}} + */ + async* membersWithRole(role) { + const cognitoGroup = this.membershipRoles.get(role); + if (!cognitoGroup) throw new NotFound(`unknown role: ${role}`); + + for await (const user of cognito.listUsersInGroup(cognitoGroup)) { + yield { + username: user.Username, + }; + } + } + + /** + * Grant a role in this group to a user. + * + * Makes the user a group member if they weren't already. + * + * Any existing authn tokens the user has will be considered stale after + * calling this function so that the tokens are automatically refreshed to + * reflect their new role. + * + * @param {String} role - e.g. viewers, editors, or owners + * @param {String} username + * @throws {NotFound} on an unknown role + */ + async grantRole(role, username) { + const cognitoGroup = this.membershipRoles.get(role); + if (!cognitoGroup) throw new NotFound(`unknown role: ${role}`); + + await cognito.addUserToGroup(username, cognitoGroup); + await markUserStaleBeforeNow(username); + } + + /** + * Revoke a role in this group from a user. + * + * Removes the user from group membership if the removed role was their sole + * one. + * + * Any existing authn tokens the user has will be considered stale after + * calling this function so that the tokens are automatically refreshed to + * reflect their removed role. + * + * @param {String} role - e.g. viewers, editors, or owners + * @param {String} username + * @throws {NotFound} on an unknown role + */ + async revokeRole(role, username) { + const cognitoGroup = this.membershipRoles.get(role); + if (!cognitoGroup) throw new NotFound(`unknown role: ${role}`); + + await cognito.removeUserFromGroup(username, cognitoGroup); + await markUserStaleBeforeNow(username); + } } diff --git a/src/utils/iterators.js b/src/utils/iterators.js new file mode 100644 index 000000000..5dd382121 --- /dev/null +++ b/src/utils/iterators.js @@ -0,0 +1,39 @@ +/** + * Iterator utilities. + * + * @module utils.iterators + */ + + +/** + * Maps a function over any iterable, including async ones. + * + * If the iterable has a "map" property, it's called directly. + * + * @param {Object} it - [Iterable object]{@link https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Iteration_protocols#the_iterable_protocol}, e.g. an array, Set, Map, generator, async generator, etc. + * @param {Function} fn - Synchronous mapping function taking an element and returning a mapped value. + * @returns {Array} + */ +export async function map(it, fn) { + if (it.map) return it.map(x => fn(x)); + + const array = []; + for await (const x of it) { + /* Apply fn() as we go to avoid keeping all of the untransformed and + * potentially large elements in memory at once. + */ + array.push(fn(x)); + } + return array; +} + + +/** + * Consume an potentially-async iterable into an array. + * + * @param {Object} it - [Iterable object]{@link https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Iteration_protocols#the_iterable_protocol}, e.g. an array, Set, Map, generator, async generator, etc. + * @returns {Array} + */ +export async function slurp(it) { + return await map(it, x => x); +}