From 0cf0f20f9d76fe711faaf7c4b245bf14f6e702ed Mon Sep 17 00:00:00 2001 From: Johannes May Date: Sat, 13 May 2017 22:56:26 +0200 Subject: [PATCH 01/19] Added /api/user/boards --- models/boards.js | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/models/boards.js b/models/boards.js index 879dde845..a0c0c8bf4 100644 --- a/models/boards.js +++ b/models/boards.js @@ -556,6 +556,18 @@ if (Meteor.isServer) { //BOARDS REST API if (Meteor.isServer) { + JsonRoutes.add('GET', '/api/user/boards', function (req, res, next) { + // TODO: This should be changed to be less restrictive! + Authentication.checkUserId(req.userId); + + return Boards.find({ + archived: false, + 'members.userId': req.userId, // TODO: How does the current authentication system work? Can we rely on req.userId to be correct? + }, { + sort: ['title'], + }); + }); + JsonRoutes.add('GET', '/api/boards', function (req, res, next) { Authentication.checkUserId(req.userId); JsonRoutes.sendResult(res, { From 5be30a2cd83ba15f88551314531ef45f4e05b352 Mon Sep 17 00:00:00 2001 From: mayjs Date: Mon, 15 May 2017 18:20:07 +0200 Subject: [PATCH 02/19] Added my local test http file to .gitignore --- .gitignore | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.gitignore b/.gitignore index 257d8a853..b3ffa20c2 100644 --- a/.gitignore +++ b/.gitignore @@ -5,4 +5,5 @@ tmp/ node_modules/ .vscode/ -.build/* \ No newline at end of file +.build/* +wekan.http From a472986d0e763cbedf62a3b09fa9869bd00ba26a Mon Sep 17 00:00:00 2001 From: mayjs Date: Mon, 15 May 2017 18:20:51 +0200 Subject: [PATCH 03/19] Fixed the errors existing in /api/user/boards --- models/boards.js | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/models/boards.js b/models/boards.js index a0c0c8bf4..6959d653c 100644 --- a/models/boards.js +++ b/models/boards.js @@ -557,17 +557,25 @@ if (Meteor.isServer) { //BOARDS REST API if (Meteor.isServer) { JsonRoutes.add('GET', '/api/user/boards', function (req, res, next) { + console.log("Running"); // TODO: This should be changed to be less restrictive! Authentication.checkUserId(req.userId); - - return Boards.find({ - archived: false, - 'members.userId': req.userId, // TODO: How does the current authentication system work? Can we rely on req.userId to be correct? - }, { - sort: ['title'], + + let data = Boards.find({ + archived: false, + 'members.userId': req.userId, // TODO: How does the current authentication system work? Can we rely on req.userId to be correct? + }, { + sort: ['title'], + }).map(function(board) { + return { + _id: board._id, + title: board._title + } }); + + JsonRoutes.sendResult(res, {code: 200, data}); }); - + JsonRoutes.add('GET', '/api/boards', function (req, res, next) { Authentication.checkUserId(req.userId); JsonRoutes.sendResult(res, { @@ -624,5 +632,4 @@ if (Meteor.isServer) { }, }); }); - } From adc6a45185be52ae4d6d4796a07c36859a9fd464 Mon Sep 17 00:00:00 2001 From: mayjs Date: Mon, 15 May 2017 18:31:10 +0200 Subject: [PATCH 04/19] Removed old debug output --- models/boards.js | 1 - 1 file changed, 1 deletion(-) diff --git a/models/boards.js b/models/boards.js index 6959d653c..e0fd81dd0 100644 --- a/models/boards.js +++ b/models/boards.js @@ -557,7 +557,6 @@ if (Meteor.isServer) { //BOARDS REST API if (Meteor.isServer) { JsonRoutes.add('GET', '/api/user/boards', function (req, res, next) { - console.log("Running"); // TODO: This should be changed to be less restrictive! Authentication.checkUserId(req.userId); From 609a8894ece85ab36dc5ef88db5f290484dce9ff Mon Sep 17 00:00:00 2001 From: mayjs Date: Mon, 15 May 2017 18:40:45 +0200 Subject: [PATCH 05/19] Added a non restrictive authentication function --- server/authentication.js | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/server/authentication.js b/server/authentication.js index 816c4d4c0..6fee86493 100644 --- a/server/authentication.js +++ b/server/authentication.js @@ -17,5 +17,15 @@ Meteor.startup(() => { }; + // This will only check if the user is logged in. + // The authorization checks for the user will have to be done inside each API endpoint + Authentication.checkLoggedIn = function(userId) { + if(userId === undefined) { + const error = new Meteor.Error('Unauthorized', 'Unauthorized'); + error.statusCode = 401; + throw error; + } + }; + }); From fa928956c2e9bfd21b18f973eded49775ce33e86 Mon Sep 17 00:00:00 2001 From: mayjs Date: Mon, 15 May 2017 18:44:54 +0200 Subject: [PATCH 06/19] Added an endpoint to get the user referenced by the given token --- models/users.js | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/models/users.js b/models/users.js index aa870dca3..55d0095e7 100644 --- a/models/users.js +++ b/models/users.js @@ -527,6 +527,16 @@ if (Meteor.isServer) { // USERS REST API if (Meteor.isServer) { + JsonRoutes.add('GET', '/api/user', function(req, res, next) { + Authentication.checkLoggedIn(req.userId); + let data = Meteor.users.findOne({ _id: req.userId}); + delete data.services; + JsonRoutes.sendResult(res, { + code: 200, + data + }); + }); + JsonRoutes.add('GET', '/api/users', function (req, res, next) { Authentication.checkUserId( req.userId); JsonRoutes.sendResult(res, { From 158f19b67cfcc62e67927c4cbe75f5732e672a3c Mon Sep 17 00:00:00 2001 From: mayjs Date: Mon, 15 May 2017 18:45:24 +0200 Subject: [PATCH 07/19] let -> const --- models/boards.js | 2 +- models/users.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/models/boards.js b/models/boards.js index e0fd81dd0..914ded934 100644 --- a/models/boards.js +++ b/models/boards.js @@ -560,7 +560,7 @@ if (Meteor.isServer) { // TODO: This should be changed to be less restrictive! Authentication.checkUserId(req.userId); - let data = Boards.find({ + const data = Boards.find({ archived: false, 'members.userId': req.userId, // TODO: How does the current authentication system work? Can we rely on req.userId to be correct? }, { diff --git a/models/users.js b/models/users.js index 55d0095e7..01709f491 100644 --- a/models/users.js +++ b/models/users.js @@ -529,7 +529,7 @@ if (Meteor.isServer) { if (Meteor.isServer) { JsonRoutes.add('GET', '/api/user', function(req, res, next) { Authentication.checkLoggedIn(req.userId); - let data = Meteor.users.findOne({ _id: req.userId}); + const data = Meteor.users.findOne({ _id: req.userId}); delete data.services; JsonRoutes.sendResult(res, { code: 200, From 452901d3b1578da35ba6c1caa50a640a9fcfccf7 Mon Sep 17 00:00:00 2001 From: mayjs Date: Mon, 15 May 2017 19:20:37 +0200 Subject: [PATCH 08/19] Changed /api/user/boards to only check if the user is logged in --- models/boards.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/boards.js b/models/boards.js index 914ded934..5df342150 100644 --- a/models/boards.js +++ b/models/boards.js @@ -558,7 +558,7 @@ if (Meteor.isServer) { if (Meteor.isServer) { JsonRoutes.add('GET', '/api/user/boards', function (req, res, next) { // TODO: This should be changed to be less restrictive! - Authentication.checkUserId(req.userId); + Authentication.checkLoggedIn(req.userId); const data = Boards.find({ archived: false, From 1bdc28bf9cffa311ba0955760c3a2013f23dce83 Mon Sep 17 00:00:00 2001 From: mayjs Date: Mon, 15 May 2017 19:20:55 +0200 Subject: [PATCH 09/19] Fixed a typo and removed an old todo comment --- models/boards.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/models/boards.js b/models/boards.js index 5df342150..f2ac794ed 100644 --- a/models/boards.js +++ b/models/boards.js @@ -562,13 +562,13 @@ if (Meteor.isServer) { const data = Boards.find({ archived: false, - 'members.userId': req.userId, // TODO: How does the current authentication system work? Can we rely on req.userId to be correct? + 'members.userId': req.userId, }, { sort: ['title'], }).map(function(board) { return { _id: board._id, - title: board._title + title: board.title } }); From ef6f2e8d62a2322b9172edf0f7d07e2fe66b85c9 Mon Sep 17 00:00:00 2001 From: mayjs Date: Mon, 15 May 2017 19:43:15 +0200 Subject: [PATCH 10/19] Added a simple authorization function --- server/authentication.js | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/server/authentication.js b/server/authentication.js index 6fee86493..a67b64aa0 100644 --- a/server/authentication.js +++ b/server/authentication.js @@ -27,5 +27,17 @@ Meteor.startup(() => { } }; + // An admin should be authorized to access everything, so we use a separate check for admins + // This throws an error if otherReq is false and the user is not an admin + Authentication.checkAdminOrCondition = function(userId, otherReq) { + if(otherReq) return; + const admin = Users.findOne({ _id: userId, isAdmin: true }); + if (admin === undefined) { + const error = new Meteor.Error('Forbidden', 'Forbidden'); + error.statusCode = 403; + throw error; + } + } + }); From 95e2025ff9ac07644175689b873749fc2087eef2 Mon Sep 17 00:00:00 2001 From: mayjs Date: Mon, 15 May 2017 19:43:53 +0200 Subject: [PATCH 11/19] Changed GET /api/boards/:id to allow access by the normally also allowed users. --- models/boards.js | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/models/boards.js b/models/boards.js index f2ac794ed..3778963f9 100644 --- a/models/boards.js +++ b/models/boards.js @@ -557,7 +557,6 @@ if (Meteor.isServer) { //BOARDS REST API if (Meteor.isServer) { JsonRoutes.add('GET', '/api/user/boards', function (req, res, next) { - // TODO: This should be changed to be less restrictive! Authentication.checkLoggedIn(req.userId); const data = Boards.find({ @@ -589,8 +588,12 @@ if (Meteor.isServer) { }); JsonRoutes.add('GET', '/api/boards/:id', function (req, res, next) { - Authentication.checkUserId( req.userId); + Authentication.checkLoggedIn( req.userId); const id = req.params.id; + const board = Boards.findOne({ _id: id }); + const normalAccess = board.permission === 'public' || board.members.some(e => e._id === req.userId); + Authentication.checkAdminOrCondition(req.userId, normalAccess); + JsonRoutes.sendResult(res, { code: 200, data: Boards.findOne({ _id: id }), From 1e8d9f02f32a83bc3514330be53f7bd21156142b Mon Sep 17 00:00:00 2001 From: mayjs Date: Mon, 15 May 2017 21:02:31 +0200 Subject: [PATCH 12/19] Extracted board access check function --- models/boards.js | 5 +---- server/authentication.js | 9 +++++++++ 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/models/boards.js b/models/boards.js index 3778963f9..57493fd32 100644 --- a/models/boards.js +++ b/models/boards.js @@ -588,11 +588,8 @@ if (Meteor.isServer) { }); JsonRoutes.add('GET', '/api/boards/:id', function (req, res, next) { - Authentication.checkLoggedIn( req.userId); const id = req.params.id; - const board = Boards.findOne({ _id: id }); - const normalAccess = board.permission === 'public' || board.members.some(e => e._id === req.userId); - Authentication.checkAdminOrCondition(req.userId, normalAccess); + Authentication.checkBoardAccess( req.userId, id); JsonRoutes.sendResult(res, { code: 200, diff --git a/server/authentication.js b/server/authentication.js index a67b64aa0..14e9d1c43 100644 --- a/server/authentication.js +++ b/server/authentication.js @@ -39,5 +39,14 @@ Meteor.startup(() => { } } + // Helper function. Will throw an error if the user does not have read only access to the given board + Authentication.checkBoardAccess = function(userId, boardId) { + Authentication.checkLoggedIn(userId); + + const board = Boards.findOne({ _id: boardId }); + const normalAccess = board.permission === 'public' || board.members.some(e => e.userId === userId); + Authentication.checkAdminOrCondition(userId, normalAccess); + } + }); From 066593f9c3b103aee134f4fd26e913021d82749c Mon Sep 17 00:00:00 2001 From: mayjs Date: Mon, 15 May 2017 21:03:42 +0200 Subject: [PATCH 13/19] Modified access to GET /api/boards/:boardId/lists --- models/lists.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/models/lists.js b/models/lists.js index 7dbdc9f21..8114ff5cb 100644 --- a/models/lists.js +++ b/models/lists.js @@ -132,8 +132,9 @@ if (Meteor.isServer) { //LISTS REST API if (Meteor.isServer) { JsonRoutes.add('GET', '/api/boards/:boardId/lists', function (req, res, next) { - Authentication.checkUserId( req.userId); const paramBoardId = req.params.boardId; + Authentication.checkBoardAccess( req.userId, paramBoardId); + JsonRoutes.sendResult(res, { code: 200, data: Lists.find({ boardId: paramBoardId, archived: false }).map(function (doc) { From cb99fc582ef50a4b6dfbbabdcf93998bc1478496 Mon Sep 17 00:00:00 2001 From: mayjs Date: Mon, 15 May 2017 21:06:52 +0200 Subject: [PATCH 14/19] Modified access to GET /api/boards/:boardId/lists/:listId --- models/lists.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/lists.js b/models/lists.js index 8114ff5cb..d9a5b8e2a 100644 --- a/models/lists.js +++ b/models/lists.js @@ -147,9 +147,9 @@ if (Meteor.isServer) { }); JsonRoutes.add('GET', '/api/boards/:boardId/lists/:listId', function (req, res, next) { - Authentication.checkUserId( req.userId); const paramBoardId = req.params.boardId; const paramListId = req.params.listId; + Authentication.checkBoardAccess( req.userId, paramBoardId); JsonRoutes.sendResult(res, { code: 200, data: Lists.findOne({ _id: paramListId, boardId: paramBoardId, archived: false }), From c59891d44b09af1ed2112b1f524046376167dbed Mon Sep 17 00:00:00 2001 From: mayjs Date: Mon, 15 May 2017 21:41:21 +0200 Subject: [PATCH 15/19] Added readonly user access to cards --- models/cards.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/models/cards.js b/models/cards.js index bbe46b55a..c48b4845f 100644 --- a/models/cards.js +++ b/models/cards.js @@ -373,9 +373,9 @@ if (Meteor.isServer) { //LISTS REST API if (Meteor.isServer) { JsonRoutes.add('GET', '/api/boards/:boardId/lists/:listId/cards', function (req, res, next) { - Authentication.checkUserId( req.userId); const paramBoardId = req.params.boardId; const paramListId = req.params.listId; + Authentication.checkBoardAccess( req.userId, paramBoardId); JsonRoutes.sendResult(res, { code: 200, data: Cards.find({ boardId: paramBoardId, listId: paramListId, archived: false }).map(function (doc) { @@ -389,10 +389,10 @@ if (Meteor.isServer) { }); JsonRoutes.add('GET', '/api/boards/:boardId/lists/:listId/cards/:cardId', function (req, res, next) { - Authentication.checkUserId( req.userId); const paramBoardId = req.params.boardId; const paramListId = req.params.listId; const paramCardId = req.params.cardId; + Authentication.checkBoardAccess( req.userId, paramBoardId); JsonRoutes.sendResult(res, { code: 200, data: Cards.findOne({ _id: paramCardId, listId: paramListId, boardId: paramBoardId, archived: false }), From 25b26657da71da05b09fff48f1329ce936ae022b Mon Sep 17 00:00:00 2001 From: mayjs Date: Mon, 15 May 2017 22:10:46 +0200 Subject: [PATCH 16/19] Fixed eslint errors --- models/boards.js | 18 +++++++++--------- models/users.js | 2 +- server/authentication.js | 6 +++--- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/models/boards.js b/models/boards.js index 57493fd32..366a680a3 100644 --- a/models/boards.js +++ b/models/boards.js @@ -560,15 +560,15 @@ if (Meteor.isServer) { Authentication.checkLoggedIn(req.userId); const data = Boards.find({ - archived: false, - 'members.userId': req.userId, - }, { - sort: ['title'], - }).map(function(board) { - return { - _id: board._id, - title: board.title - } + archived: false, + 'members.userId': req.userId, + }, { + sort: ['title'], + }).map(function(board) { + return { + _id: board._id, + title: board.title, + }; }); JsonRoutes.sendResult(res, {code: 200, data}); diff --git a/models/users.js b/models/users.js index 01709f491..29504aa8f 100644 --- a/models/users.js +++ b/models/users.js @@ -533,7 +533,7 @@ if (Meteor.isServer) { delete data.services; JsonRoutes.sendResult(res, { code: 200, - data + data, }); }); diff --git a/server/authentication.js b/server/authentication.js index 14e9d1c43..23ed8f562 100644 --- a/server/authentication.js +++ b/server/authentication.js @@ -37,16 +37,16 @@ Meteor.startup(() => { error.statusCode = 403; throw error; } - } + }; // Helper function. Will throw an error if the user does not have read only access to the given board Authentication.checkBoardAccess = function(userId, boardId) { Authentication.checkLoggedIn(userId); const board = Boards.findOne({ _id: boardId }); - const normalAccess = board.permission === 'public' || board.members.some(e => e.userId === userId); + const normalAccess = board.permission === 'public' || board.members.some((e) => e.userId === userId); Authentication.checkAdminOrCondition(userId, normalAccess); - } + }; }); From 4ff906bd7b18d279d93b6c82c722c14c0c8d1f3a Mon Sep 17 00:00:00 2001 From: mayjs Date: Mon, 15 May 2017 22:35:53 +0200 Subject: [PATCH 17/19] Revert "Added my local test http file to .gitignore" This reverts commit 5be30a2cd83ba15f88551314531ef45f4e05b352. --- .gitignore | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.gitignore b/.gitignore index b3ffa20c2..257d8a853 100644 --- a/.gitignore +++ b/.gitignore @@ -5,5 +5,4 @@ tmp/ node_modules/ .vscode/ -.build/* -wekan.http +.build/* \ No newline at end of file From 058aabff68ceb822edb74248f1263da6590cbe1e Mon Sep 17 00:00:00 2001 From: mayjs Date: Tue, 16 May 2017 10:52:55 +0200 Subject: [PATCH 18/19] Implemented the change of /api/user/boards as proposed by huneau --- models/boards.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/models/boards.js b/models/boards.js index 366a680a3..5908dee9a 100644 --- a/models/boards.js +++ b/models/boards.js @@ -556,8 +556,12 @@ if (Meteor.isServer) { //BOARDS REST API if (Meteor.isServer) { - JsonRoutes.add('GET', '/api/user/boards', function (req, res, next) { + JsonRoutes.add('GET', '/api/user/:userId/boards', function (req, res, next) { Authentication.checkLoggedIn(req.userId); + const paramUserId = req.params.userId; + // A normal user should be able to see their own boards, + // admins can access boards of any user + Authentication.checkAdminOrCondition(req.userId, req.userId === paramUserId); const data = Boards.find({ archived: false, From e3fab5380690b955db7408f32b98b97f46497034 Mon Sep 17 00:00:00 2001 From: mayjs Date: Tue, 16 May 2017 17:35:14 +0200 Subject: [PATCH 19/19] Fixed a typo.. --- models/boards.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/boards.js b/models/boards.js index 5908dee9a..8a7844e2f 100644 --- a/models/boards.js +++ b/models/boards.js @@ -556,7 +556,7 @@ if (Meteor.isServer) { //BOARDS REST API if (Meteor.isServer) { - JsonRoutes.add('GET', '/api/user/:userId/boards', function (req, res, next) { + JsonRoutes.add('GET', '/api/users/:userId/boards', function (req, res, next) { Authentication.checkLoggedIn(req.userId); const paramUserId = req.params.userId; // A normal user should be able to see their own boards,