From 4d82959e91f64922a3e579c009eceb5629c7beed 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 | 2 + env/testing/terraform.tf | 2 + 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, 349 insertions(+), 3 deletions(-) rename aws/iam/policy/{NextstrainDotOrgServerInstance.json => NextstrainDotOrgServerInstance.tftpl.json} (68%) rename aws/iam/policy/{NextstrainDotOrgServerInstanceDev.json => NextstrainDotOrgServerInstanceDev.tftpl.json} (75%) 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 68% rename from aws/iam/policy/NextstrainDotOrgServerInstance.json rename to aws/iam/policy/NextstrainDotOrgServerInstance.tftpl.json index 53718c05e..3d423d6b5 100644 --- a/aws/iam/policy/NextstrainDotOrgServerInstance.json +++ b/aws/iam/policy/NextstrainDotOrgServerInstance.tftpl.json @@ -32,6 +32,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 75% rename from aws/iam/policy/NextstrainDotOrgServerInstanceDev.json rename to aws/iam/policy/NextstrainDotOrgServerInstanceDev.tftpl.json index 1e9a114dd..b6f210543 100644 --- a/aws/iam/policy/NextstrainDotOrgServerInstanceDev.json +++ b/aws/iam/policy/NextstrainDotOrgServerInstanceDev.tftpl.json @@ -43,6 +43,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..fd966e7da 100644 --- a/env/production/terraform.tf +++ b/env/production/terraform.tf @@ -52,6 +52,8 @@ 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..36b30aca9 100644 --- a/env/testing/terraform.tf +++ b/env/testing/terraform.tf @@ -46,4 +46,6 @@ 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 ea9775a0b..68fe8f977 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 5bef58b13..465f959e9 100644 --- a/src/app.js +++ b/src/app.js @@ -337,6 +337,21 @@ app.routeAsync("/groups/:groupName/settings/overview") .optionsAsync(optionsGroupSettings) ; +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 ef0fee55b..651e8353d 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) => { @@ -161,13 +163,94 @@ 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, optionsGroupSettings, + getGroupLogo, putGroupLogo, deleteGroupLogo, + getGroupOverview, putGroupOverview, deleteGroupOverview, + + listMembers, + listRoles, + listRoleMembers, + + getRoleMember, + putRoleMember, + deleteRoleMember, }; diff --git a/src/groups.js b/src/groups.js index d69a09f59..dda2a0143 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 { GROUPS_DATA_FILE } from './config.js'; +import * as cognito from "./cognito.js"; import { NotFound } from './httpErrors.js'; import { GroupSource } from "./sources/index.js"; +import { markUserStaleBeforeNow } from "./user.js"; import { readFile } from 'fs/promises'; const GROUPS_DATA = JSON.parse(await readFile(GROUPS_DATA_FILE)); @@ -113,6 +116,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); +}