From 2e830c520888c6628f2fc8756ebe6be807178b66 Mon Sep 17 00:00:00 2001 From: rogup Date: Tue, 30 Jul 2024 15:16:34 +0100 Subject: [PATCH 1/4] Allow user groups to be public --- src/queries/query.ts | 52 ++++++++++++++++++++++++-------------------- 1 file changed, 29 insertions(+), 23 deletions(-) diff --git a/src/queries/query.ts b/src/queries/query.ts index cb6be70..01a8bd6 100644 --- a/src/queries/query.ts +++ b/src/queries/query.ts @@ -19,6 +19,7 @@ import { getMapMarkers, getMapPolygonsAndLines } from "../queries/map"; import { hashPassword } from "./helper"; import bcrypt from "bcrypt"; import axios from "axios"; +import { Op } from "sequelize"; export const getUserById = async (id: number): Promise => { return await User.findOne({ where: { id } }); @@ -268,55 +269,60 @@ export const searchOwner = async (proprietorName: string) => { export const findAllDataGroupContentForUser = async (userId: number) => { const userGroupMemberships = await UserGroupMembership.findAll({ where: { - user_id: userId, + user_id: { [Op.or]: [userId, -1] }, // Include public user groups }, }); const userGroups: any[] = []; - for (let membership of userGroupMemberships) { - userGroups.push( - await UserGroup.findOne({ - where: { - iduser_groups: membership.user_group_id, - }, - }) - ); + for (const membership of userGroupMemberships) { + const userGroup = await UserGroup.findOne({ + where: { + iduser_groups: membership.user_group_id, + }, + }); + // Check that user group actually exists + if (userGroup) { + userGroups.push(userGroup); + } } const userGroupsAndData: any[] = []; - for (let group of userGroups) { + for (const group of userGroups) { const dataGroupMemberships = await DataGroupMembership.findAll({ where: { user_group_id: group.iduser_groups, }, }); - let dataGroups: any[] = []; + const dataGroups: any[] = []; - for (let membership of dataGroupMemberships) { - dataGroups = dataGroups.concat( - await DataGroup.findAll({ - where: { - iddata_groups: membership.data_group_id, - }, - }) - ); + for (const membership of dataGroupMemberships) { + const dataGroup = await DataGroup.findOne({ + where: { + iddata_groups: membership.data_group_id, + }, + raw: true, + }); + // Check that data group actually exists + if (dataGroup) { + dataGroups.push(dataGroup); + } } - for (let dataGroup of dataGroups) { - dataGroup.dataValues.markers = await Marker.findAll({ + for (const dataGroup of dataGroups) { + dataGroup.markers = await Marker.findAll({ where: { data_group_id: dataGroup.iddata_groups, }, }); - dataGroup.dataValues.polygons = await Polygon.findAll({ + dataGroup.polygons = await Polygon.findAll({ where: { data_group_id: dataGroup.iddata_groups, }, }); - dataGroup.dataValues.lines = await Line.findAll({ + dataGroup.lines = await Line.findAll({ where: { data_group_id: dataGroup.iddata_groups, }, From bf25adf8ff3c6a87d672e1808e9663fad99020ce Mon Sep 17 00:00:00 2001 From: rogup Date: Tue, 30 Jul 2024 15:16:47 +0100 Subject: [PATCH 2/4] Add UT --- src/queries/query.test.ts | 198 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 198 insertions(+) diff --git a/src/queries/query.test.ts b/src/queries/query.test.ts index 72643f0..4dc370f 100644 --- a/src/queries/query.test.ts +++ b/src/queries/query.test.ts @@ -136,3 +136,201 @@ describe("Check and return user", () => { }); }); }); + +describe("findAllDataGroupContentForUser", () => { + const testUserId = 123; + + afterEach(() => { + sandbox.restore(); + }); + + context( + "User is in 1 user group and there is 1 public user group, each associated with 1 data group", + () => { + const testMarker = { + idmarkers: 1, + name: "Test Marker", + description: "This is a datagroup marker", + data_group_id: 1, + location: { + type: "Feature", + geometry: { + type: "Point", + coordinates: [53.6, 12.1], + }, + }, + uuid: "abc-001", + }; + + const testPolygon = { + idpolygons: 1, + name: "Test Polygon", + description: "This is a datagroup polygon", + data_group_id: 2, + vertices: { + type: "Feature", + geometry: { + type: "Polygon", + coordinates: [ + [ + [125.6, 10.1], + [125.7, 10.2], + [125.6, 10.1], + ], + ], + }, + }, + center: { + type: "Feature", + geometry: { + type: "Point", + coordinates: [125.63, 10.13], + }, + }, + length: 100, + area: 1000, + uuid: "fgh-002", + }; + + const testUserGroup1 = { + iduser_groups: 1, + name: "Test User Group 1 (Public)", + }; + + const testDataGroup1 = { + iddata_groups: 1, + title: "Test Data Group 1 (Public)", + hex_colour: "#FF0001", + }; + + const testUserGroup2 = { + iduser_groups: 2, + name: "Test User Group 2", + }; + + const testDataGroup2 = { + iddata_groups: 2, + title: "Test Data Group 2", + hex_colour: "#FF0002", + }; + + it("returns user groups and associated data groups with markers, polygons, and lines", async () => { + // Arrange + + sandbox.replace( + Model.UserGroupMembership, + "findAll", + fake.resolves([ + { + iduser_group_memberships: 1, + user_id: -1, // -1 means public + user_group_id: testUserGroup1.iduser_groups, + }, + { + iduser_group_memberships: 2, + user_id: testUserId, + user_group_id: testUserGroup2.iduser_groups, + }, + ]) + ); + + sandbox.replace( + Model.UserGroup, + "findOne", + fake((options) => { + return options.where.iduser_groups === testUserGroup1.iduser_groups + ? testUserGroup1 + : testUserGroup2; + }) + ); + + sandbox.replace( + Model.DataGroupMembership, + "findAll", + fake((options) => { + return options.where.user_group_id === testUserGroup1.iduser_groups + ? [ + { + iddata_group_memberships: 1, + data_group_id: testDataGroup1.iddata_groups, + user_group_id: testUserGroup1.iduser_groups, + }, + ] + : [ + { + iddata_group_memberships: 2, + data_group_id: testDataGroup2.iddata_groups, + user_group_id: testUserGroup2.iduser_groups, + }, + ]; + }) + ); + + sandbox.replace( + Model.DataGroup, + "findOne", + fake((options) => { + return options.where.iddata_groups === testDataGroup1.iddata_groups + ? testDataGroup1 + : testDataGroup2; + }) + ); + + sandbox.replace( + Model.Marker, + "findAll", + fake((options) => { + return options.where.data_group_id === testDataGroup1.iddata_groups + ? [testMarker] + : []; + }) + ); + sandbox.replace( + Model.Polygon, + "findAll", + fake((options) => { + return options.where.data_group_id === testDataGroup2.iddata_groups + ? [testPolygon] + : []; + }) + ); + sandbox.replace(Model.Line, "findAll", fake.resolves([])); + + // Act + + const result = await query.findAllDataGroupContentForUser(testUserId); + + // Assert + + const expectedContent = [ + { + name: testUserGroup1.name, + id: testUserGroup1.iduser_groups, + dataGroups: [ + { + ...testDataGroup1, + markers: [testMarker], + polygons: [], + lines: [], + }, + ], + }, + { + name: testUserGroup2.name, + id: testUserGroup2.iduser_groups, + dataGroups: [ + { + ...testDataGroup2, + markers: [], + polygons: [testPolygon], + lines: [], + }, + ], + }, + ]; + + expect(result).to.deep.equal(expectedContent); + }); + } + ); +}); From f1c5deeff77982d1a033a4c27470980ab733ed7a Mon Sep 17 00:00:00 2001 From: rogup Date: Mon, 12 Aug 2024 00:32:07 +0100 Subject: [PATCH 3/4] Add access level to user group memberships --- ...10000-add-user_group_memberships-access.js | 21 ++++ src/queries/database.ts | 12 ++- src/queries/query.test.ts | 97 +++++++++++++------ src/queries/query.ts | 21 +++- src/routes/datagroups.ts | 14 +-- 5 files changed, 127 insertions(+), 38 deletions(-) create mode 100644 migrations/2024080611510000-add-user_group_memberships-access.js diff --git a/migrations/2024080611510000-add-user_group_memberships-access.js b/migrations/2024080611510000-add-user_group_memberships-access.js new file mode 100644 index 0000000..f88eab9 --- /dev/null +++ b/migrations/2024080611510000-add-user_group_memberships-access.js @@ -0,0 +1,21 @@ +"use strict"; + +module.exports = { + async up(queryInterface, Sequelize) { + // set access to ReadWrite for all current user_group_memberships + await queryInterface.sequelize.query( + `ALTER TABLE user_group_memberships + ADD access int(11) DEFAULT 3;` + ); + await queryInterface.sequelize.query( + `ALTER TABLE user_group_memberships + MODIFY COLUMN access int(11) NOT NULL;` + ); + }, + async down(queryInterface, Sequelize) { + await queryInterface.sequelize.query( + `ALTER TABLE user_group_memberships + DROP access;` + ); + }, +}; diff --git a/src/queries/database.ts b/src/queries/database.ts index 84f8c7b..2006567 100644 --- a/src/queries/database.ts +++ b/src/queries/database.ts @@ -43,7 +43,7 @@ const UserModel = sequelize.define( username: { type: DataTypes.STRING, allowNull: false }, password: { type: DataTypes.STRING, allowNull: false }, - access: DataTypes.INTEGER, // 1 = member, 2 = admin + access: DataTypes.INTEGER, enabled: DataTypes.INTEGER, is_super_user: { type: DataTypes.BOOLEAN, allowNull: false }, @@ -275,6 +275,10 @@ const UserGroupMembershipModel = sequelize.define( references: { model: UserGroupModel, key: "iduser_groups" }, allowNull: false, }, + access: { + type: DataTypes.INTEGER, + allowNull: false, + }, }, { tableName: "user_group_memberships", @@ -420,6 +424,12 @@ export enum UserMapAccess { Readwrite = 3, } +/* The access values in the UserGroupMembership table */ +export enum UserGroupAccess { + Readonly = 1, + Readwrite = 3, +} + /* All the possible values of the iditem_types column in the ItemType table */ export enum ItemTypeId { Marker = 0, diff --git a/src/queries/query.test.ts b/src/queries/query.test.ts index 4dc370f..bcaac64 100644 --- a/src/queries/query.test.ts +++ b/src/queries/query.test.ts @@ -145,7 +145,7 @@ describe("findAllDataGroupContentForUser", () => { }); context( - "User is in 1 user group and there is 1 public user group, each associated with 1 data group", + "There is a user group (1) associated with 1 data group containing 1 marker, and a user group (2) associated with a data group containing 1 polygon", () => { const testMarker = { idmarkers: 1, @@ -194,46 +194,27 @@ describe("findAllDataGroupContentForUser", () => { const testUserGroup1 = { iduser_groups: 1, - name: "Test User Group 1 (Public)", + name: "User Group (1)", }; const testDataGroup1 = { iddata_groups: 1, - title: "Test Data Group 1 (Public)", + title: "Data Group (1)", hex_colour: "#FF0001", }; const testUserGroup2 = { iduser_groups: 2, - name: "Test User Group 2", + name: "User Group (2)", }; const testDataGroup2 = { iddata_groups: 2, - title: "Test Data Group 2", + title: "Data Group (2)", hex_colour: "#FF0002", }; - it("returns user groups and associated data groups with markers, polygons, and lines", async () => { - // Arrange - - sandbox.replace( - Model.UserGroupMembership, - "findAll", - fake.resolves([ - { - iduser_group_memberships: 1, - user_id: -1, // -1 means public - user_group_id: testUserGroup1.iduser_groups, - }, - { - iduser_group_memberships: 2, - user_id: testUserId, - user_group_id: testUserGroup2.iduser_groups, - }, - ]) - ); - + beforeEach(() => { sandbox.replace( Model.UserGroup, "findOne", @@ -295,17 +276,35 @@ describe("findAllDataGroupContentForUser", () => { }) ); sandbox.replace(Model.Line, "findAll", fake.resolves([])); + }); - // Act + it("Returns the datagroups for a usergroup with readwrite access and a public usergroup with readonly access", async () => { + sandbox.replace( + Model.UserGroupMembership, + "findAll", + fake.resolves([ + { + iduser_group_memberships: 1, + user_id: -1, // -1 means public + user_group_id: testUserGroup1.iduser_groups, + access: 1, // readonly access + }, + { + iduser_group_memberships: 2, + user_id: testUserId, + user_group_id: testUserGroup2.iduser_groups, + access: 3, // readwrite access + }, + ]) + ); const result = await query.findAllDataGroupContentForUser(testUserId); - // Assert - const expectedContent = [ { name: testUserGroup1.name, id: testUserGroup1.iduser_groups, + access: 1, dataGroups: [ { ...testDataGroup1, @@ -318,6 +317,7 @@ describe("findAllDataGroupContentForUser", () => { { name: testUserGroup2.name, id: testUserGroup2.iduser_groups, + access: 3, dataGroups: [ { ...testDataGroup2, @@ -331,6 +331,47 @@ describe("findAllDataGroupContentForUser", () => { expect(result).to.deep.equal(expectedContent); }); + + it("Returns the higher access level if a user has readwrite access to a public usergroup", async () => { + sandbox.replace( + Model.UserGroupMembership, + "findAll", + fake.resolves([ + { + iduser_group_memberships: 1, + user_id: -1, // -1 means public + user_group_id: testUserGroup1.iduser_groups, + access: 1, // readonly access + }, + { + iduser_group_memberships: 2, + user_id: testUserId, + user_group_id: testUserGroup1.iduser_groups, + access: 3, // readwrite access + }, + ]) + ); + + const result = await query.findAllDataGroupContentForUser(testUserId); + + const expectedContent = [ + { + name: testUserGroup1.name, + id: testUserGroup1.iduser_groups, + access: 3, + dataGroups: [ + { + ...testDataGroup1, + markers: [testMarker], + polygons: [], + lines: [], + }, + ], + }, + ]; + + expect(result).to.deep.equal(expectedContent); + }); } ); }); diff --git a/src/queries/query.ts b/src/queries/query.ts index 01a8bd6..1a548d6 100644 --- a/src/queries/query.ts +++ b/src/queries/query.ts @@ -280,10 +280,25 @@ export const findAllDataGroupContentForUser = async (userId: number) => { where: { iduser_groups: membership.user_group_id, }, + raw: true, }); + userGroup.access = membership.access; + // Check that user group actually exists if (userGroup) { - userGroups.push(userGroup); + const existingUserGroup = userGroups.find( + (group) => group.iduser_groups === userGroup.iduser_groups + ); + + // If the user group is already in the list, use the highest access level + if (existingUserGroup) { + existingUserGroup.access = Math.max( + existingUserGroup.access, + userGroup.access + ); + } else { + userGroups.push(userGroup); + } } } @@ -332,6 +347,7 @@ export const findAllDataGroupContentForUser = async (userId: number) => { userGroupsAndData.push({ name: group.name, id: group.iduser_groups, + access: group.access, dataGroups, }); } @@ -339,13 +355,14 @@ export const findAllDataGroupContentForUser = async (userId: number) => { return userGroupsAndData; }; -export const hasAccessToDataGroup = async ( +export const hasWriteAccessToDataGroup = async ( userId: number, dataGroupId: number ): Promise => { const userGroupMemberships = await UserGroupMembership.findAll({ where: { user_id: userId, + access: 3, }, }); diff --git a/src/routes/datagroups.ts b/src/routes/datagroups.ts index c18674f..71d06eb 100644 --- a/src/routes/datagroups.ts +++ b/src/routes/datagroups.ts @@ -8,7 +8,7 @@ import { updateLine, } from "../queries/object"; import { - hasAccessToDataGroup, + hasWriteAccessToDataGroup, findAllDataGroupContentForUser, } from "../queries/query"; import { v4 as uuidv4 } from "uuid"; @@ -58,7 +58,7 @@ async function saveDataGroupMarker( ): Promise { const { object, dataGroupId } = request.payload; - const hasAccess = await hasAccessToDataGroup( + const hasAccess = await hasWriteAccessToDataGroup( request.auth.credentials.user_id, dataGroupId ); @@ -84,7 +84,7 @@ async function saveDataGroupPolygon( ): Promise { const { object, dataGroupId } = request.payload; - const hasAccess = await hasAccessToDataGroup( + const hasAccess = await hasWriteAccessToDataGroup( request.auth.credentials.user_id, dataGroupId ); @@ -113,7 +113,7 @@ async function saveDataGroupLine( ): Promise { const { object, dataGroupId } = request.payload; - const hasAccess = await hasAccessToDataGroup( + const hasAccess = await hasWriteAccessToDataGroup( request.auth.credentials.user_id, dataGroupId ); @@ -160,7 +160,7 @@ async function editDataGroupMarker( ): Promise { const { uuid, name, description, dataGroupId } = request.payload; - const hasAccess = await hasAccessToDataGroup( + const hasAccess = await hasWriteAccessToDataGroup( request.auth.credentials.user_id, dataGroupId ); @@ -179,7 +179,7 @@ async function editDataGroupPolygon( ): Promise { const { uuid, name, description, dataGroupId } = request.payload; - const hasAccess = await hasAccessToDataGroup( + const hasAccess = await hasWriteAccessToDataGroup( request.auth.credentials.user_id, dataGroupId ); @@ -198,7 +198,7 @@ async function editDataGroupLine( ): Promise { const { uuid, name, description, dataGroupId } = request.payload; - const hasAccess = await hasAccessToDataGroup( + const hasAccess = await hasWriteAccessToDataGroup( request.auth.credentials.user_id, dataGroupId ); From e7048de91c65d97071e4be8aead7d2e2bac5a6c2 Mon Sep 17 00:00:00 2001 From: rogup Date: Fri, 6 Sep 2024 10:02:07 +0100 Subject: [PATCH 4/4] Use named const rather than hardcoded integer --- src/queries/query.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/queries/query.ts b/src/queries/query.ts index 1a548d6..a9cf550 100644 --- a/src/queries/query.ts +++ b/src/queries/query.ts @@ -14,6 +14,7 @@ import { UserGroup, UserGroupMembership, UserFeedback, + UserGroupAccess, } from "./database"; import { getMapMarkers, getMapPolygonsAndLines } from "../queries/map"; import { hashPassword } from "./helper"; @@ -362,7 +363,7 @@ export const hasWriteAccessToDataGroup = async ( const userGroupMemberships = await UserGroupMembership.findAll({ where: { user_id: userId, - access: 3, + access: UserGroupAccess.Readwrite, }, });