From 42610d96427959d2e009e21df0f2dfba4d1334b8 Mon Sep 17 00:00:00 2001 From: John Supplee Date: Tue, 21 Dec 2021 02:39:16 +0200 Subject: [PATCH] More enhancements to Admin Reports and security fixes * update Boards Report * use Boards.userBoards() instead of Boards.find() to make sure user has access permission --- client/components/settings/adminReports.jade | 7 ++ client/components/settings/adminReports.js | 8 ++ models/boards.js | 22 +++-- models/users.js | 57 +++++------- server/publications/boards.js | 97 +++++++++++--------- 5 files changed, 107 insertions(+), 84 deletions(-) diff --git a/client/components/settings/adminReports.jade b/client/components/settings/adminReports.jade index c9df62892..7a1b8cb96 100644 --- a/client/components/settings/adminReports.jade +++ b/client/components/settings/adminReports.jade @@ -154,12 +154,19 @@ template(name="boardsReport") tr th Title th Id + th Permission + th Archived? th Members + th Organizations + th Teams each board in results tr td {{abbreviate board.title }} td {{abbreviate board._id }} + td {{ board.permission }} + td + = yesOrNo(board.archived) td {{userNames board.members }} else div {{_ 'no-results' }} diff --git a/client/components/settings/adminReports.js b/client/components/settings/adminReports.js index 68414fd9c..6dcbb0fc4 100644 --- a/client/components/settings/adminReports.js +++ b/client/components/settings/adminReports.js @@ -108,6 +108,14 @@ class AdminReport extends BlazeComponent { return this.collection.find(); } + yesOrNo(value) { + if (value) { + return TAPi18n.__('yes'); + } else { + return TAPi18n.__('no'); + } + } + resultsCount() { return this.collection.find().count(); } diff --git a/models/boards.js b/models/boards.js index e0dee3f7b..6244e5aca 100644 --- a/models/boards.js +++ b/models/boards.js @@ -5,6 +5,7 @@ import { TYPE_TEMPLATE_BOARD, TYPE_TEMPLATE_CONTAINER, } from '/config/const'; +import Users from "./users"; const escapeForRegex = require('escape-string-regexp'); Boards = new Mongo.Collection('boards'); @@ -1485,6 +1486,11 @@ Boards.userBoards = ( selector = {}, projection = {}, ) => { + const user = Users.findOne(userId); + if (!user) { + return []; + } + if (typeof archived === 'boolean') { selector.archived = archived; } @@ -1492,14 +1498,14 @@ Boards.userBoards = ( selector.type = 'board'; } - selector.$or = [{ permission: 'public' }]; - if (userId) { - selector.$or.push( - { members: { $elemMatch: { userId, isActive: true } } }, - projection, - ); - } - return Boards.find(selector); + selector.$or = [ + { permission: 'public' }, + { members: { $elemMatch: { userId, isActive: true } } }, + { 'orgs.orgId': { $in: user.orgIds() } }, + { 'teams.teamId': { $in : user.teamIds() } }, + ]; + + return Boards.find(selector, projection); }; Boards.userBoardIds = (userId, archived = false, selector = {}) => { diff --git a/models/users.js b/models/users.js index 70a5b141b..05354e045 100644 --- a/models/users.js +++ b/models/users.js @@ -519,6 +519,18 @@ Users.helpers({ } return ''; }, + teamIds() { + if (this.teams) { + return this.teams.map(team => { return team.teamId }); + } + return []; + }, + orgIds() { + if (this.orgs) { + return this.orgs.map(org => { return org.orgId }); + } + return []; + }, orgsUserBelongs() { if (this.orgs) { return this.orgs.map(function(org){return org.orgDisplayName}).sort().join(','); @@ -544,32 +556,16 @@ Users.helpers({ return ''; }, boards() { - return Boards.find( - { - 'members.userId': this._id, - }, - { - sort: { - sort: 1 /* boards default sorting */, - }, - }, - ); + return Boards.userBoards(this._id, null, {}, { sort: { sort: 1 } }) }, starredBoards() { const { starredBoards = [] } = this.profile || {}; - return Boards.find( - { - archived: false, - _id: { - $in: starredBoards, - }, - }, - { - sort: { - sort: 1 /* boards default sorting */, - }, - }, + return Boards.userBoards( + this._id, + false, + { _id: { $in: starredBoards } }, + { sort: { sort: 1 } } ); }, @@ -580,18 +576,11 @@ Users.helpers({ invitedBoards() { const { invitedBoards = [] } = this.profile || {}; - return Boards.find( - { - archived: false, - _id: { - $in: invitedBoards, - }, - }, - { - sort: { - sort: 1 /* boards default sorting */, - }, - }, + return Boards.userBoards( + this._id, + false, + { _id: { $in: invitedBoards } }, + { sort: { sort: 1 } } ); }, diff --git a/server/publications/boards.js b/server/publications/boards.js index 045c1c810..ef47e52bb 100644 --- a/server/publications/boards.js +++ b/server/publications/boards.js @@ -3,41 +3,46 @@ // 1. that the user is a member of // 2. the user has starred import Users from "../../models/users"; +import Org from "../../models/org"; +import Team from "../../models/team"; Meteor.publish('boards', function() { const userId = this.userId; // Ensure that the user is connected. If it is not, we need to return an empty // array to tell the client to remove the previously published docs. - if (!Match.test(userId, String) || !userId) return []; + if (!Match.test(userId, String) || !userId) { + return []; + } // Defensive programming to verify that starredBoards has the expected // format -- since the field is in the `profile` a user can modify it. - const { starredBoards = [] } = (Users.findOne(userId) || {}).profile || {}; - check(starredBoards, [String]); + // const { starredBoards = [] } = (Users.findOne(userId) || {}).profile || {}; + // check(starredBoards, [String]); - let currUser = Users.findOne(userId); - let orgIdsUserBelongs = currUser!== 'undefined' && currUser.teams !== 'undefined' ? currUser.orgIdsUserBelongs() : ''; - let teamIdsUserBelongs = currUser!== 'undefined' && currUser.teams !== 'undefined' ? currUser.teamIdsUserBelongs() : ''; - let orgsIds = []; - let teamsIds = []; - if(orgIdsUserBelongs && orgIdsUserBelongs != ''){ - orgsIds = orgIdsUserBelongs.split(','); - } - if(teamIdsUserBelongs && teamIdsUserBelongs != ''){ - teamsIds = teamIdsUserBelongs.split(','); - } + // let currUser = Users.findOne(userId); + // let orgIdsUserBelongs = currUser!== 'undefined' && currUser.teams !== 'undefined' ? currUser.orgIdsUserBelongs() : ''; + // let teamIdsUserBelongs = currUser!== 'undefined' && currUser.teams !== 'undefined' ? currUser.teamIdsUserBelongs() : ''; + // let orgsIds = []; + // let teamsIds = []; + // if(orgIdsUserBelongs && orgIdsUserBelongs != ''){ + // orgsIds = orgIdsUserBelongs.split(','); + // } + // if(teamIdsUserBelongs && teamIdsUserBelongs != ''){ + // teamsIds = teamIdsUserBelongs.split(','); + // } return Boards.find( { archived: false, - $or: [ - { - // _id: { $in: starredBoards }, // Commented out, to get a list of all public boards - permission: 'public', - }, - { members: { $elemMatch: { userId, isActive: true } } }, - {'orgs.orgId': {$in : orgsIds}}, - {'teams.teamId': {$in : teamsIds}}, - ], + _id: { $in: Boards.userBoardIds(userId, false) }, + // $or: [ + // { + // // _id: { $in: starredBoards }, // Commented out, to get a list of all public boards + // permission: 'public', + // }, + // { members: { $elemMatch: { userId, isActive: true } } }, + // {'orgs.orgId': {$in : orgsIds}}, + // {'teams.teamId': {$in : teamsIds}}, + // ], }, { fields: { @@ -66,16 +71,9 @@ Meteor.publish('boardsReport', function() { // array to tell the client to remove the previously published docs. if (!Match.test(userId, String) || !userId) return []; - boards = Boards.find( + const boards = Boards.find( { - archived: false, - $or: [ - { - // _id: { $in: starredBoards }, // Commented out, to get a list of all public boards - permission: 'public', - }, - { members: { $elemMatch: { userId, isActive: true } } }, - ], + _id: { $in: Boards.userBoardIds(userId, null) }, }, { fields: { @@ -97,18 +95,32 @@ Meteor.publish('boardsReport', function() { }, ); - const users = []; + const userIds = []; + const orgIds = []; + const teamIds = []; boards.forEach(board => { if (board.members) { board.members.forEach(member => { - users.push(member.userId); + userIds.push(member.userId); + }); + } + if (board.orgs) { + board.orgs.forEach(org => { + orgIds.push(org.orgId); + }); + } + if (board.teams) { + board.teams.forEach(team => { + teamIds.push(team.teamId); }); } }) return [ boards, - Users.find({ _id: { $in: users } }, { fields: Users.safeFields }), + Users.find({ _id: { $in: userIds } }, { fields: Users.safeFields }), + Team.find({ _id: { $in: teamIds } }), + Org.find({ _id: { $in: orgIds } }), ] }); @@ -118,13 +130,14 @@ Meteor.publish('archivedBoards', function() { return Boards.find( { - archived: true, - members: { - $elemMatch: { - userId, - isAdmin: true, - }, - }, + _id: { $in: Boards.userBoardIds(userId, true)}, + // archived: true, + // members: { + // $elemMatch: { + // userId, + // isAdmin: true, + // }, + // }, }, { fields: {