From c3b4d2e52593fd304fab298f2d23e3a4bf692d96 Mon Sep 17 00:00:00 2001 From: "Robert St. John" Date: Thu, 7 Nov 2024 09:44:37 -0700 Subject: [PATCH] refactor(service): users/auth: pass only user id vs user instance to event model filtering --- .../adapters.events.controllers.web.legacy.ts | 27 +++++---- service/src/models/event.d.ts | 6 +- service/src/models/event.js | 55 ++++++------------- 3 files changed, 35 insertions(+), 53 deletions(-) diff --git a/service/src/adapters/events/adapters.events.controllers.web.legacy.ts b/service/src/adapters/events/adapters.events.controllers.web.legacy.ts index f345364d8..96846a04d 100644 --- a/service/src/adapters/events/adapters.events.controllers.web.legacy.ts +++ b/service/src/adapters/events/adapters.events.controllers.web.legacy.ts @@ -4,22 +4,23 @@ import async from 'async' import util from 'util' import fileType from 'file-type' import mongoose from 'mongoose' -import EventModel, { FormDocument, MageEventDocument } from '../models/event' +import EventModel, { FormDocument, MageEventDocument } from '../../models/event' import express from 'express' -import access from '../access' -import { AnyPermission, MageEventPermission } from '../entities/authorization/entities.permissions' -import { JsonObject } from '../entities/entities.json_types' +import access from '../../access' +import { AnyPermission, MageEventPermission } from '../../entities/authorization/entities.permissions' +import { JsonObject } from '../../entities/entities.json_types' import fs from 'fs-extra' -import { EventAccessType, MageEvent } from '../entities/events/entities.events' -import { defaultHandler as upload } from '../upload' -import { defaultEventPermissionsService } from '../permissions/permissions.events' -import { LineStyle, PagingParameters } from '../entities/entities.global' +import { EventAccessType, MageEvent } from '../../entities/events/entities.events' +import { defaultHandler as upload } from '../../upload' +import { defaultEventPermissionsService } from '../../permissions/permissions.events' +import { LineStyle, PagingParameters } from '../../entities/entities.global' +import { UserId } from '../../entities/users/entities.users' declare module 'express-serve-static-core' { export interface Request { event?: EventModel.MageEventDocument eventEntity?: MageEvent - access?: { user: express.Request['user'], permission: EventAccessType } + access?: { userId: UserId, permission: EventAccessType } parameters?: EventQueryParams form?: FormJson team?: any @@ -27,10 +28,11 @@ declare module 'express-serve-static-core' { } function determineReadAccess(req: express.Request, res: express.Response, next: express.NextFunction): void { - if (!access.userHasPermission(req.user, MageEventPermission.READ_EVENT_ALL)) { - req.access = { user: req.user, permission: EventAccessType.Read }; + const requestingUser = req.user?.from === 'sessionToken' ? req.user.account : null + if (requestingUser && !access.userHasPermission(requestingUser, MageEventPermission.READ_EVENT_ALL)) { + req.access = { userId: requestingUser.id, permission: EventAccessType.Read } } - next(); + next() } /** @@ -251,6 +253,7 @@ function EventRoutes(): express.Router { if (err) { return next(err); } + // TODO: scale: should avoid this for large user sets new api.Form(event).populateUserFields(function(err: any) { if (err) { return next(err); diff --git a/service/src/models/event.d.ts b/service/src/models/event.d.ts index 664c5f0de..de117218a 100644 --- a/service/src/models/event.d.ts +++ b/service/src/models/event.d.ts @@ -1,12 +1,13 @@ import mongoose, { ToObjectOptions } from 'mongoose' -import { UserDocument } from './user' +import { UserDocument } from '../adapters/users/adapters.users.db.mongoose' import { MageEventId, MageEventAttrs, MageEventCreateAttrs, EventAccessType, EventRole } from '../entities/events/entities.events' import { Team, TeamMemberRole } from '../entities/teams/entities.teams' import { Form, FormField, FormFieldChoice } from '../entities/events/entities.events.forms' import { PageInfo } from '../utilities/paging'; +import { UserId } from '../entities/users/entities.users' export interface MageEventDocumentToObjectOptions extends ToObjectOptions { - access: { user: UserDocument, permission: EventAccessType } + access: { userId: UserId, permission: EventAccessType } projection: any } @@ -67,7 +68,6 @@ export type Callback = (err: Error | null, result?: Result) => export declare function count(options: TODO, callback: Callback): void export declare function getEvents(options: TODO, callback: Callback): void export declare function getById(id: MageEventId, options: TODO, callback: Callback): void -export declare function filterEventsByUserId(events: MageEventDocument[], userId: string, callback: Callback): void export declare function create(event: MageEventCreateAttrs, user: Partial & Pick, callback: Callback): void export declare function addForm(eventId: MageEventId, form: any, callback: Callback): void export declare function addLayer(event: MageEventDocument, layer: any, callback: Callback): void diff --git a/service/src/models/event.js b/service/src/models/event.js index 44cf590ee..b1fcb1e87 100644 --- a/service/src/models/event.js +++ b/service/src/models/event.js @@ -208,11 +208,11 @@ function transform(event, ret, options) { // if read only permissions in event acl, only return users acl // TODO: move this business logic if (options.access) { - const roleOfUserOnEvent = ret.acl[options.access.user._id]; + const roleOfUserOnEvent = ret.acl[options.access.userId]; const rolesThatCanModify = rolesWithPermission('update').concat(rolesWithPermission('delete')); if (!roleOfUserOnEvent || rolesThatCanModify.indexOf(roleOfUserOnEvent) === -1) { const acl = {}; - acl[options.access.user._id] = ret.acl[options.access.user._id]; + acl[options.access.userId] = ret.acl[options.access.userId]; ret.acl = acl; } } @@ -268,18 +268,14 @@ function filterEventsByUserId(events, userId, callback) { if (err) return callback(err); const filteredEvents = events.filter(function (event) { - // Check if user has read access to the event based on - // being on a team that is in the event + // Check if user has read access to the event based on being on a team that is in the event if (event.teamIds.some(function (team) { return team.userIds.indexOf(userId) !== -1; })) { return true; } - - // Check if user has read access to the event based on - // being in the events access control list + // Check if user has read access to the event based on being in the event access control list if (event.acl[userId] && rolesWithPermission('read').some(function (role) { return role === event.acl[userId]; })) { return true; } - return false; }); @@ -297,9 +293,7 @@ exports.count = function (options, callback) { if (options.access) { const accesses = []; rolesWithPermission(options.access.permission).forEach(function (role) { - const access = {}; - access['acl.' + options.access.user._id.toString()] = role; - accesses.push(access); + accesses.push({ [`acl.${options.access.userId}`]: role }); }); conditions['$or'] = accesses; } @@ -335,9 +329,9 @@ exports.getEvents = function (options, callback) { const filters = []; // First filter out events user cannot access - if (options.access && options.access.user) { + if (options.access && options.access.userId) { filters.push(function (done) { - filterEventsByUserId(events, options.access.user._id, function (err, filteredEvents) { + filterEventsByUserId(events, options.access.userId, function (err, filteredEvents) { if (err) return done(err); events = filteredEvents; @@ -411,9 +405,6 @@ exports.getById = function (id, options, callback) { }); }; -// TODO probably should live in event api -exports.filterEventsByUserId = filterEventsByUserId; - function createObservationCollection(event) { log.info("Creating observation collection: " + event.collectionName + ' for event ' + event.name); mongoose.connection.db.createCollection(event.collectionName).then(() => { @@ -549,22 +540,16 @@ exports.updateForm = function (event, form, callback) { exports.getMembers = async function (eventId, options) { const query = { _id: eventId }; if (options.access) { - const accesses = [{ - userIds: { - '$in': [options.access.user._id] - } - }]; - + const accesses = [ + { userIds: { '$in': [ new mongoose.Types.ObjectId(options.access.userId) ] } } + ]; rolesWithPermission(options.access.permission).forEach(role => { - const access = {}; - access['acl.' + options.access.user._id.toString()] = role; - accesses.push(access); + accesses.push({ [`acl.${options.access.userId}`]: role }); }); - query['$or'] = accesses; } - const event = await Event.findOne(query) + const event = await Event.findOne(query) if (event) { const { searchTerm } = options || {} const searchRegex = new RegExp(searchTerm, 'i') @@ -609,22 +594,16 @@ exports.getMembers = async function (eventId, options) { exports.getNonMembers = async function (eventId, options) { const query = { _id: eventId }; if (options.access) { - const accesses = [{ - userIds: { - '$in': [options.access.user._id] - } - }]; - + const accesses = [ + { userIds: { '$in': [ new mongoose.Types.ObjectId(options.access.userId) ] } } + ]; rolesWithPermission(options.access.permission).forEach(role => { - const access = {}; - access['acl.' + options.access.user._id.toString()] = role; - accesses.push(access); + accesses.push({ [`acl.${options.access.userId}`]: role }); }); - query['$or'] = accesses; } - const event = await Event.findOne(query) + const event = await Event.findOne(query) if (event) { const { searchTerm } = options || {} const searchRegex = new RegExp(searchTerm, 'i')