From 4ef686f5f908be126837f9a8889e8f5f6cdaa125 Mon Sep 17 00:00:00 2001 From: Kris West Date: Fri, 21 Mar 2025 17:09:25 +0000 Subject: [PATCH 01/25] fix: remove incorrect default query from repos page (that is ignored anyway) --- src/service/routes/repo.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/service/routes/repo.js b/src/service/routes/repo.js index 1f6698e3b..f43181a00 100644 --- a/src/service/routes/repo.js +++ b/src/service/routes/repo.js @@ -5,9 +5,7 @@ const { getProxyURL } = require('../urls'); router.get('/', async (req, res) => { const proxyURL = getProxyURL(req); - const query = { - type: 'push', - }; + const query = {}; for (const k in req.query) { if (!k) continue; From 232c466dd07dac4e406e893b67103cdc4661ed10 Mon Sep 17 00:00:00 2001 From: Kris West Date: Tue, 25 Mar 2025 10:58:09 +0000 Subject: [PATCH 02/25] fix: passthrough repository queries --- src/db/file/repo.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/db/file/repo.ts b/src/db/file/repo.ts index 8686899f5..db8480d1e 100644 --- a/src/db/file/repo.ts +++ b/src/db/file/repo.ts @@ -7,7 +7,7 @@ if (!fs.existsSync('./.data/db')) fs.mkdirSync('./.data/db'); const db = new Datastore({ filename: './.data/db/repos.db', autoload: true }); -export const getRepos = async (query: any = {}) => { +export const getRepos = async (query: any = {}) => { return new Promise((resolve, reject) => { db.find({}, (err: Error, docs: Repo[]) => { if (err) { From ab362e4ca83d519754f78074a43ad2c956a61ab9 Mon Sep 17 00:00:00 2001 From: Kris West Date: Tue, 1 Apr 2025 14:22:18 +0100 Subject: [PATCH 03/25] test: use unique emails for users in tests and remove afterwards --- test/addRepoTest.test.js | 5 ++++- test/testUserCreation.test.js | 3 +-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/test/addRepoTest.test.js b/test/addRepoTest.test.js index 04983f63c..b86fa222c 100644 --- a/test/addRepoTest.test.js +++ b/test/addRepoTest.test.js @@ -28,7 +28,7 @@ describe('add new repo', async () => { await db.deleteUser('u1'); await db.deleteUser('u2'); await db.createUser('u1', 'abc', 'test@test.com', 'test', true); - await db.createUser('u2', 'abc', 'test@test.com', 'test', true); + await db.createUser('u2', 'abc', 'test2@test.com', 'test', true); }); it('login', async function () { @@ -187,5 +187,8 @@ describe('add new repo', async () => { after(async function () { await service.httpServer.close(); + await db.deleteRepo('test-repo'); + await db.deleteUser('u1'); + await db.deleteUser('u2'); }); }); diff --git a/test/testUserCreation.test.js b/test/testUserCreation.test.js index c07dd0e7b..631be0069 100644 --- a/test/testUserCreation.test.js +++ b/test/testUserCreation.test.js @@ -55,8 +55,7 @@ describe('user creation', async () => { }); it('login as new user', async function () { - // we don't know the users tempoary password - so force update a - // pasword + // we don't know the users temporary password - so force update a password const user = await db.findUser('login-test-user'); await bcrypt.hash('test1234', 10, async function (err, hash) { From 12e7bfe62953f48aa87e2d957c46e25a40885f98 Mon Sep 17 00:00:00 2001 From: Kris West Date: Fri, 11 Apr 2025 16:00:07 +0100 Subject: [PATCH 04/25] test: don't clean-up test-repo as cypress tests rely on it --- test/addRepoTest.test.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/test/addRepoTest.test.js b/test/addRepoTest.test.js index b86fa222c..6df809a66 100644 --- a/test/addRepoTest.test.js +++ b/test/addRepoTest.test.js @@ -187,8 +187,10 @@ describe('add new repo', async () => { after(async function () { await service.httpServer.close(); - await db.deleteRepo('test-repo'); - await db.deleteUser('u1'); - await db.deleteUser('u2'); + + // don't clean up data as cypress tests rely on it being present + // await db.deleteRepo('test-repo'); + // await db.deleteUser('u1'); + // await db.deleteUser('u2'); }); }); From 200788c4ef84e163f617feff5076d73736c214ca Mon Sep 17 00:00:00 2001 From: Kris West Date: Tue, 25 Mar 2025 12:43:47 +0000 Subject: [PATCH 05/25] fix: add indexes, compaction and consistent lowercasing of inputs in file-based DB implementation --- src/db/file/pushes.ts | 6 +++++- src/db/file/repo.ts | 39 +++++++++++++++++++++++++++++++++++--- src/db/file/users.ts | 44 +++++++++++++++++++++++++++++++++++++++---- 3 files changed, 81 insertions(+), 8 deletions(-) diff --git a/src/db/file/pushes.ts b/src/db/file/pushes.ts index 9901940f7..bc2198d55 100644 --- a/src/db/file/pushes.ts +++ b/src/db/file/pushes.ts @@ -4,12 +4,16 @@ import Datastore from '@seald-io/nedb'; import { Action } from '../../proxy/actions/Action'; import { toClass } from '../helper'; import * as repo from './repo'; -import { PushQuery } from '../types' +import { PushQuery } from '../types'; + +const COMPACTION_INTERVAL = 1000 * 60 * 60 * 24; // once per day if (!fs.existsSync('./.data')) fs.mkdirSync('./.data'); if (!fs.existsSync('./.data/db')) fs.mkdirSync('./.data/db'); const db = new Datastore({ filename: './.data/db/pushes.db', autoload: true }); +db.ensureIndex({ fieldName: 'id', unique: true }); +db.setAutocompactionInterval(COMPACTION_INTERVAL); const defaultPushQuery: PushQuery = { error: false, diff --git a/src/db/file/repo.ts b/src/db/file/repo.ts index 8686899f5..b32542556 100644 --- a/src/db/file/repo.ts +++ b/src/db/file/repo.ts @@ -1,15 +1,23 @@ import fs from 'fs'; -import Datastore from '@seald-io/nedb' +import Datastore from '@seald-io/nedb'; import { Repo } from '../types'; +const COMPACTION_INTERVAL = 1000 * 60 * 60 * 24; // once per day + if (!fs.existsSync('./.data')) fs.mkdirSync('./.data'); if (!fs.existsSync('./.data/db')) fs.mkdirSync('./.data/db'); const db = new Datastore({ filename: './.data/db/repos.db', autoload: true }); +db.ensureIndex({ fieldName: 'name', unique: true }); +db.setAutocompactionInterval(COMPACTION_INTERVAL); + +const isBlank = (str: string) => { + return !str || /^\s*$/.test(str); +}; export const getRepos = async (query: any = {}) => { return new Promise((resolve, reject) => { - db.find({}, (err: Error, docs: Repo[]) => { + db.find(query, (err: Error, docs: Repo[]) => { if (err) { reject(err); } else { @@ -20,6 +28,7 @@ export const getRepos = async (query: any = {}) => { }; export const getRepo = async (name: string) => { + name = name.toLowerCase(); return new Promise((resolve, reject) => { db.findOne({ name }, (err: Error | null, doc: Repo) => { if (err) { @@ -31,8 +40,20 @@ export const getRepo = async (name: string) => { }); }; - export const createRepo = async (repo: Repo) => { + repo.name = repo.name.toLowerCase(); + console.log(`creating new repo ${JSON.stringify(repo)}`); + + if (isBlank(repo.project)) { + throw new Error('Project name cannot be empty'); + } + if (isBlank(repo.name)) { + throw new Error('Repository name cannot be empty'); + } + if (isBlank(repo.url)) { + throw new Error('URL cannot be empty'); + } + repo.users = { canPush: [], canAuthorise: [], @@ -43,6 +64,7 @@ export const createRepo = async (repo: Repo) => { if (err) { reject(err); } else { + console.log(`created new repo ${JSON.stringify(repo)}`); resolve(doc); } }); @@ -50,6 +72,8 @@ export const createRepo = async (repo: Repo) => { }; export const addUserCanPush = async (name: string, user: string) => { + name = name.toLowerCase(); + user = user.toLowerCase(); return new Promise(async (resolve, reject) => { const repo = await getRepo(name); if (!repo) { @@ -75,6 +99,8 @@ export const addUserCanPush = async (name: string, user: string) => { }; export const addUserCanAuthorise = async (name: string, user: string) => { + name = name.toLowerCase(); + user = user.toLowerCase(); return new Promise(async (resolve, reject) => { const repo = await getRepo(name); if (!repo) { @@ -101,6 +127,8 @@ export const addUserCanAuthorise = async (name: string, user: string) => { }; export const removeUserCanAuthorise = async (name: string, user: string) => { + name = name.toLowerCase(); + user = user.toLowerCase(); return new Promise(async (resolve, reject) => { const repo = await getRepo(name); if (!repo) { @@ -122,6 +150,8 @@ export const removeUserCanAuthorise = async (name: string, user: string) => { }; export const removeUserCanPush = async (name: string, user: string) => { + name = name.toLowerCase(); + user = user.toLowerCase(); return new Promise(async (resolve, reject) => { const repo = await getRepo(name); if (!repo) { @@ -143,6 +173,7 @@ export const removeUserCanPush = async (name: string, user: string) => { }; export const deleteRepo = async (name: string) => { + name = name.toLowerCase(); return new Promise((resolve, reject) => { db.remove({ name: name }, (err) => { if (err) { @@ -156,6 +187,7 @@ export const deleteRepo = async (name: string) => { export const isUserPushAllowed = async (name: string, user: string) => { name = name.toLowerCase(); + user = user.toLowerCase(); return new Promise(async (resolve) => { const repo = await getRepo(name); if (!repo) { @@ -176,6 +208,7 @@ export const isUserPushAllowed = async (name: string, user: string) => { export const canUserApproveRejectPushRepo = async (name: string, user: string) => { name = name.toLowerCase(); + user = user.toLowerCase(); console.log(`checking if user ${user} can approve/reject for ${name}`); return new Promise(async (resolve) => { const repo = await getRepo(name); diff --git a/src/db/file/users.ts b/src/db/file/users.ts index d72443c97..c258fd0f6 100644 --- a/src/db/file/users.ts +++ b/src/db/file/users.ts @@ -2,11 +2,18 @@ import fs from 'fs'; import Datastore from '@seald-io/nedb'; import { User } from '../types'; +const COMPACTION_INTERVAL = 1000 * 60 * 60 * 24; // once per day + if (!fs.existsSync('./.data')) fs.mkdirSync('./.data'); if (!fs.existsSync('./.data/db')) fs.mkdirSync('./.data/db'); const db = new Datastore({ filename: './.data/db/users.db', autoload: true }); +// Using a unique constraint with the index +db.ensureIndex({ fieldName: 'username', unique: true }); +db.ensureIndex({ fieldName: 'email', unique: true }); +db.setAutocompactionInterval(COMPACTION_INTERVAL); + export const findUser = (username: string) => { return new Promise((resolve, reject) => { db.findOne({ username: username }, (err: Error | null, doc: User) => { @@ -40,6 +47,8 @@ export const findUserByOIDC = function (oidcId: string) { }; export const createUser = function (user: User) { + user.username = user.username.toLowerCase(); + user.email = user.email.toLowerCase(); return new Promise((resolve, reject) => { db.insert(user, (err) => { if (err) { @@ -53,7 +62,7 @@ export const createUser = function (user: User) { export const deleteUser = (username: string) => { return new Promise((resolve, reject) => { - db.remove({ username: username }, (err) => { + db.remove({ username: username.toLowerCase() }, (err) => { if (err) { reject(err); } else { @@ -64,19 +73,46 @@ export const deleteUser = (username: string) => { }; export const updateUser = (user: User) => { + user.username = user.username.toLowerCase(); + if (user.email) { + user.email = user.email.toLowerCase(); + } return new Promise((resolve, reject) => { - const options = { multi: false, upsert: false }; - db.update({ username: user.username }, user, options, (err) => { + // The mongo db adaptor adds fields to existing documents, where this adaptor replaces the document + // hence, retrieve and merge documents to avoid dropping fields (such as the gitaccount) + let existingUser; + db.findOne({ username: user.username }, (err, doc) => { if (err) { reject(err); } else { - resolve(null); + if (!doc) { + existingUser = {}; + } else { + existingUser = doc; + } + + Object.assign(existingUser, user); + + const options = { multi: false, upsert: true }; + db.update({ username: user.username }, existingUser, options, (err) => { + if (err) { + reject(err); + } else { + resolve(null); + } + }); } }); }); }; export const getUsers = (query: any = {}) => { + if (query.username) { + query.username = query.username.toLowerCase(); + } + if (query.email) { + query.email = query.email.toLowerCase(); + } return new Promise((resolve, reject) => { db.find(query, (err: Error, docs: User[]) => { if (err) { From 9769be69bc4a676172d7bb180d16641a9ac580ba Mon Sep 17 00:00:00 2001 From: Kris West Date: Tue, 25 Mar 2025 12:50:53 +0000 Subject: [PATCH 06/25] fix: consistent lowercasing of inputs in mongoDB implementation --- src/db/mongo/repo.ts | 11 ++++++++++- src/db/mongo/users.ts | 16 +++++++++++++--- 2 files changed, 23 insertions(+), 4 deletions(-) diff --git a/src/db/mongo/repo.ts b/src/db/mongo/repo.ts index 5e55d3d71..f299f907a 100644 --- a/src/db/mongo/repo.ts +++ b/src/db/mongo/repo.ts @@ -1,4 +1,4 @@ -import { Repo } from "../types"; +import { Repo } from '../types'; const connect = require('./helper').connect; const collectionName = 'repos'; @@ -13,11 +13,13 @@ export const getRepos = async (query: any = {}) => { }; export const getRepo = async (name: string) => { + name = name.toLowerCase(); const collection = await connect(collectionName); return collection.findOne({ name: { $eq: name } }); }; export const createRepo = async (repo: Repo) => { + repo.name = repo.name.toLowerCase(); console.log(`creating new repo ${JSON.stringify(repo)}`); if (isBlank(repo.project)) { @@ -42,35 +44,41 @@ export const createRepo = async (repo: Repo) => { export const addUserCanPush = async (name: string, user: string) => { name = name.toLowerCase(); + user = user.toLowerCase(); const collection = await connect(collectionName); await collection.updateOne({ name: name }, { $push: { 'users.canPush': user } }); }; export const addUserCanAuthorise = async (name: string, user: string) => { name = name.toLowerCase(); + user = user.toLowerCase(); const collection = await connect(collectionName); await collection.updateOne({ name: name }, { $push: { 'users.canAuthorise': user } }); }; export const removeUserCanPush = async (name: string, user: string) => { name = name.toLowerCase(); + user = user.toLowerCase(); const collection = await connect(collectionName); await collection.updateOne({ name: name }, { $pull: { 'users.canPush': user } }); }; export const removeUserCanAuthorise = async (name: string, user: string) => { name = name.toLowerCase(); + user = user.toLowerCase(); const collection = await connect(collectionName); await collection.updateOne({ name: name }, { $pull: { 'users.canAuthorise': user } }); }; export const deleteRepo = async (name: string) => { + name = name.toLowerCase(); const collection = await connect(collectionName); await collection.deleteMany({ name: name }); }; export const isUserPushAllowed = async (name: string, user: string) => { name = name.toLowerCase(); + user = user.toLowerCase(); return new Promise(async (resolve) => { const repo = await exports.getRepo(name); console.log(repo.users.canPush); @@ -86,6 +94,7 @@ export const isUserPushAllowed = async (name: string, user: string) => { export const canUserApproveRejectPushRepo = async (name: string, user: string) => { name = name.toLowerCase(); + user = user.toLowerCase(); console.log(`checking if user ${user} can approve/reject for ${name}`); return new Promise(async (resolve) => { const repo = await exports.getRepo(name); diff --git a/src/db/mongo/users.ts b/src/db/mongo/users.ts index 0bfa1a941..5bacb245d 100644 --- a/src/db/mongo/users.ts +++ b/src/db/mongo/users.ts @@ -1,14 +1,20 @@ -import { User } from "../types"; +import { User } from '../types'; const connect = require('./helper').connect; const collectionName = 'users'; export const findUser = async function (username: string) { const collection = await connect(collectionName); - return collection.findOne({ username: { $eq: username } }); + return collection.findOne({ username: { $eq: username.toLowerCase() } }); }; export const getUsers = async function (query: any = {}) { + if (query.username) { + query.username = query.username.toLowerCase(); + } + if (query.email) { + query.email = query.email.toLowerCase(); + } console.log(`Getting users for query= ${JSON.stringify(query)}`); const collection = await connect(collectionName); return collection.find(query, { password: 0 }).toArray(); @@ -16,17 +22,21 @@ export const getUsers = async function (query: any = {}) { export const deleteUser = async function (username: string) { const collection = await connect(collectionName); - return collection.deleteOne({ username: username }); + return collection.deleteOne({ username: username.toLowerCase() }); }; export const createUser = async function (user: User) { user.username = user.username.toLowerCase(); + user.email = user.email.toLowerCase(); const collection = await connect(collectionName); return collection.insertOne(user); }; export const updateUser = async (user: User) => { user.username = user.username.toLowerCase(); + if (user.email) { + user.email = user.email.toLowerCase(); + } const options = { upsert: true }; const collection = await connect(collectionName); await collection.updateOne({ username: user.username }, { $set: user }, options); From 188d820b731e6a1fdf26bb25ecdfdd23dd466a6d Mon Sep 17 00:00:00 2001 From: kriswest Date: Tue, 25 Mar 2025 19:09:14 +0000 Subject: [PATCH 07/25] fix: make repo name index non-unique (currently assumed unique but won't be IRL) --- src/db/file/repo.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/db/file/repo.ts b/src/db/file/repo.ts index b32542556..f794e066c 100644 --- a/src/db/file/repo.ts +++ b/src/db/file/repo.ts @@ -8,7 +8,7 @@ if (!fs.existsSync('./.data')) fs.mkdirSync('./.data'); if (!fs.existsSync('./.data/db')) fs.mkdirSync('./.data/db'); const db = new Datastore({ filename: './.data/db/repos.db', autoload: true }); -db.ensureIndex({ fieldName: 'name', unique: true }); +db.ensureIndex({ fieldName: 'name', unique: false }); db.setAutocompactionInterval(COMPACTION_INTERVAL); const isBlank = (str: string) => { From f38c266b2d43c6e7778e541f4a65409708d6653a Mon Sep 17 00:00:00 2001 From: Kris West Date: Tue, 1 Apr 2025 14:22:18 +0100 Subject: [PATCH 08/25] test: use unique emails for users in tests and remove afterwards --- test/addRepoTest.test.js | 5 ++++- test/testUserCreation.test.js | 3 +-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/test/addRepoTest.test.js b/test/addRepoTest.test.js index 04983f63c..b86fa222c 100644 --- a/test/addRepoTest.test.js +++ b/test/addRepoTest.test.js @@ -28,7 +28,7 @@ describe('add new repo', async () => { await db.deleteUser('u1'); await db.deleteUser('u2'); await db.createUser('u1', 'abc', 'test@test.com', 'test', true); - await db.createUser('u2', 'abc', 'test@test.com', 'test', true); + await db.createUser('u2', 'abc', 'test2@test.com', 'test', true); }); it('login', async function () { @@ -187,5 +187,8 @@ describe('add new repo', async () => { after(async function () { await service.httpServer.close(); + await db.deleteRepo('test-repo'); + await db.deleteUser('u1'); + await db.deleteUser('u2'); }); }); diff --git a/test/testUserCreation.test.js b/test/testUserCreation.test.js index c07dd0e7b..631be0069 100644 --- a/test/testUserCreation.test.js +++ b/test/testUserCreation.test.js @@ -55,8 +55,7 @@ describe('user creation', async () => { }); it('login as new user', async function () { - // we don't know the users tempoary password - so force update a - // pasword + // we don't know the users temporary password - so force update a password const user = await db.findUser('login-test-user'); await bcrypt.hash('test1234', 10, async function (err, hash) { From 4632e86e87accc2faad5c44767072a3791e0e60e Mon Sep 17 00:00:00 2001 From: Kris West Date: Tue, 25 Mar 2025 12:43:47 +0000 Subject: [PATCH 09/25] fix: add indexes, compaction and consistent lowercasing of inputs in file-based DB implementation --- src/db/file/pushes.ts | 2 ++ src/db/file/users.ts | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/db/file/pushes.ts b/src/db/file/pushes.ts index bc2198d55..a67417ee1 100644 --- a/src/db/file/pushes.ts +++ b/src/db/file/pushes.ts @@ -8,6 +8,8 @@ import { PushQuery } from '../types'; const COMPACTION_INTERVAL = 1000 * 60 * 60 * 24; // once per day +const COMPACTION_INTERVAL = 1000 * 60 * 60 * 24; // once per day + if (!fs.existsSync('./.data')) fs.mkdirSync('./.data'); if (!fs.existsSync('./.data/db')) fs.mkdirSync('./.data/db'); diff --git a/src/db/file/users.ts b/src/db/file/users.ts index c258fd0f6..47e00de18 100644 --- a/src/db/file/users.ts +++ b/src/db/file/users.ts @@ -16,7 +16,7 @@ db.setAutocompactionInterval(COMPACTION_INTERVAL); export const findUser = (username: string) => { return new Promise((resolve, reject) => { - db.findOne({ username: username }, (err: Error | null, doc: User) => { + db.findOne({ username: username.toLowerCase() }, (err: Error | null, doc: User) => { if (err) { reject(err); } else { From 40a83b5c5e73f41ee1d5d4eceb96b9871d9d59c6 Mon Sep 17 00:00:00 2001 From: Kris West Date: Tue, 8 Apr 2025 09:43:35 +0100 Subject: [PATCH 10/25] chore: porting changes from users.js --- src/db/file/users.ts | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/db/file/users.ts b/src/db/file/users.ts index 47e00de18..46c6f30c3 100644 --- a/src/db/file/users.ts +++ b/src/db/file/users.ts @@ -107,12 +107,8 @@ export const updateUser = (user: User) => { }; export const getUsers = (query: any = {}) => { - if (query.username) { - query.username = query.username.toLowerCase(); - } - if (query.email) { - query.email = query.email.toLowerCase(); - } + if (query.username) { query.username = query.username.toLowerCase(); } + if (query.email) { query.email = query.email.toLowerCase(); } return new Promise((resolve, reject) => { db.find(query, (err: Error, docs: User[]) => { if (err) { From 6f082071c37dec675450c192126df820930b201b Mon Sep 17 00:00:00 2001 From: Kris West Date: Thu, 10 Apr 2025 14:02:12 +0100 Subject: [PATCH 11/25] fix: bad merge --- src/db/file/pushes.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/db/file/pushes.ts b/src/db/file/pushes.ts index a67417ee1..bc2198d55 100644 --- a/src/db/file/pushes.ts +++ b/src/db/file/pushes.ts @@ -8,8 +8,6 @@ import { PushQuery } from '../types'; const COMPACTION_INTERVAL = 1000 * 60 * 60 * 24; // once per day -const COMPACTION_INTERVAL = 1000 * 60 * 60 * 24; // once per day - if (!fs.existsSync('./.data')) fs.mkdirSync('./.data'); if (!fs.existsSync('./.data/db')) fs.mkdirSync('./.data/db'); From 092ec1323a991994b172fb80b6028932540c26ad Mon Sep 17 00:00:00 2001 From: Kris West Date: Fri, 11 Apr 2025 16:00:07 +0100 Subject: [PATCH 12/25] test: don't clean-up test-repo as cypress tests rely on it --- test/addRepoTest.test.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/test/addRepoTest.test.js b/test/addRepoTest.test.js index b86fa222c..6df809a66 100644 --- a/test/addRepoTest.test.js +++ b/test/addRepoTest.test.js @@ -187,8 +187,10 @@ describe('add new repo', async () => { after(async function () { await service.httpServer.close(); - await db.deleteRepo('test-repo'); - await db.deleteUser('u1'); - await db.deleteUser('u2'); + + // don't clean up data as cypress tests rely on it being present + // await db.deleteRepo('test-repo'); + // await db.deleteUser('u1'); + // await db.deleteUser('u2'); }); }); From 08a6c820bb152ed1c00ddd7de0c9d1a7c3a87bea Mon Sep 17 00:00:00 2001 From: Kris West Date: Tue, 25 Mar 2025 12:43:47 +0000 Subject: [PATCH 13/25] fix: add indexes, compaction and consistent lowercasing of inputs in file-based DB implementation --- src/db/file/pushes.ts | 2 ++ src/db/file/users.ts | 8 ++++++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/db/file/pushes.ts b/src/db/file/pushes.ts index bc2198d55..a67417ee1 100644 --- a/src/db/file/pushes.ts +++ b/src/db/file/pushes.ts @@ -8,6 +8,8 @@ import { PushQuery } from '../types'; const COMPACTION_INTERVAL = 1000 * 60 * 60 * 24; // once per day +const COMPACTION_INTERVAL = 1000 * 60 * 60 * 24; // once per day + if (!fs.existsSync('./.data')) fs.mkdirSync('./.data'); if (!fs.existsSync('./.data/db')) fs.mkdirSync('./.data/db'); diff --git a/src/db/file/users.ts b/src/db/file/users.ts index 46c6f30c3..47e00de18 100644 --- a/src/db/file/users.ts +++ b/src/db/file/users.ts @@ -107,8 +107,12 @@ export const updateUser = (user: User) => { }; export const getUsers = (query: any = {}) => { - if (query.username) { query.username = query.username.toLowerCase(); } - if (query.email) { query.email = query.email.toLowerCase(); } + if (query.username) { + query.username = query.username.toLowerCase(); + } + if (query.email) { + query.email = query.email.toLowerCase(); + } return new Promise((resolve, reject) => { db.find(query, (err: Error, docs: User[]) => { if (err) { From 1966088ef927135aabdb6da24589936416519e04 Mon Sep 17 00:00:00 2001 From: Kris West Date: Tue, 1 Apr 2025 14:22:18 +0100 Subject: [PATCH 14/25] test: use unique emails for users in tests and remove afterwards --- test/addRepoTest.test.js | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/test/addRepoTest.test.js b/test/addRepoTest.test.js index 6df809a66..b86fa222c 100644 --- a/test/addRepoTest.test.js +++ b/test/addRepoTest.test.js @@ -187,10 +187,8 @@ describe('add new repo', async () => { after(async function () { await service.httpServer.close(); - - // don't clean up data as cypress tests rely on it being present - // await db.deleteRepo('test-repo'); - // await db.deleteUser('u1'); - // await db.deleteUser('u2'); + await db.deleteRepo('test-repo'); + await db.deleteUser('u1'); + await db.deleteUser('u2'); }); }); From 8463c712851f48eee5048d593227b2590287c1c2 Mon Sep 17 00:00:00 2001 From: Kris West Date: Tue, 25 Mar 2025 12:43:47 +0000 Subject: [PATCH 15/25] fix: add indexes, compaction and consistent lowercasing of inputs in file-based DB implementation --- src/db/file/repo.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/db/file/repo.ts b/src/db/file/repo.ts index f794e066c..64576bd9c 100644 --- a/src/db/file/repo.ts +++ b/src/db/file/repo.ts @@ -8,7 +8,11 @@ if (!fs.existsSync('./.data')) fs.mkdirSync('./.data'); if (!fs.existsSync('./.data/db')) fs.mkdirSync('./.data/db'); const db = new Datastore({ filename: './.data/db/repos.db', autoload: true }); +<<<<<<< HEAD db.ensureIndex({ fieldName: 'name', unique: false }); +======= +db.ensureIndex({ fieldName: 'name', unique: true }); +>>>>>>> a7c80f0 (fix: add indexes, compaction and consistent lowercasing of inputs in file-based DB implementation) db.setAutocompactionInterval(COMPACTION_INTERVAL); const isBlank = (str: string) => { From fa5f2701e32f391d5e0dffbfd8b604bae85e1102 Mon Sep 17 00:00:00 2001 From: Kris West Date: Tue, 25 Mar 2025 12:50:53 +0000 Subject: [PATCH 16/25] fix: consistent lowercasing of inputs in mongoDB implementation --- src/db/file/repo.ts | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/db/file/repo.ts b/src/db/file/repo.ts index 64576bd9c..f794e066c 100644 --- a/src/db/file/repo.ts +++ b/src/db/file/repo.ts @@ -8,11 +8,7 @@ if (!fs.existsSync('./.data')) fs.mkdirSync('./.data'); if (!fs.existsSync('./.data/db')) fs.mkdirSync('./.data/db'); const db = new Datastore({ filename: './.data/db/repos.db', autoload: true }); -<<<<<<< HEAD db.ensureIndex({ fieldName: 'name', unique: false }); -======= -db.ensureIndex({ fieldName: 'name', unique: true }); ->>>>>>> a7c80f0 (fix: add indexes, compaction and consistent lowercasing of inputs in file-based DB implementation) db.setAutocompactionInterval(COMPACTION_INTERVAL); const isBlank = (str: string) => { From 4abbe28d60c70a1295d39183bfde95db359b6866 Mon Sep 17 00:00:00 2001 From: Kris West Date: Tue, 8 Apr 2025 09:43:35 +0100 Subject: [PATCH 17/25] chore: porting changes from users.js --- src/db/file/users.ts | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/db/file/users.ts b/src/db/file/users.ts index 47e00de18..46c6f30c3 100644 --- a/src/db/file/users.ts +++ b/src/db/file/users.ts @@ -107,12 +107,8 @@ export const updateUser = (user: User) => { }; export const getUsers = (query: any = {}) => { - if (query.username) { - query.username = query.username.toLowerCase(); - } - if (query.email) { - query.email = query.email.toLowerCase(); - } + if (query.username) { query.username = query.username.toLowerCase(); } + if (query.email) { query.email = query.email.toLowerCase(); } return new Promise((resolve, reject) => { db.find(query, (err: Error, docs: User[]) => { if (err) { From 8a73e5ae438f5f4461dc70c5ceb95b56e043eec9 Mon Sep 17 00:00:00 2001 From: Kris West Date: Thu, 10 Apr 2025 14:02:12 +0100 Subject: [PATCH 18/25] fix: bad merge --- src/db/file/pushes.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/db/file/pushes.ts b/src/db/file/pushes.ts index a67417ee1..bc2198d55 100644 --- a/src/db/file/pushes.ts +++ b/src/db/file/pushes.ts @@ -8,8 +8,6 @@ import { PushQuery } from '../types'; const COMPACTION_INTERVAL = 1000 * 60 * 60 * 24; // once per day -const COMPACTION_INTERVAL = 1000 * 60 * 60 * 24; // once per day - if (!fs.existsSync('./.data')) fs.mkdirSync('./.data'); if (!fs.existsSync('./.data/db')) fs.mkdirSync('./.data/db'); From 7aaf03c370827bec1faed4ce7d34fe58270485e4 Mon Sep 17 00:00:00 2001 From: Kris West Date: Fri, 11 Apr 2025 16:00:07 +0100 Subject: [PATCH 19/25] test: don't clean-up test-repo as cypress tests rely on it --- test/addRepoTest.test.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/test/addRepoTest.test.js b/test/addRepoTest.test.js index b86fa222c..6df809a66 100644 --- a/test/addRepoTest.test.js +++ b/test/addRepoTest.test.js @@ -187,8 +187,10 @@ describe('add new repo', async () => { after(async function () { await service.httpServer.close(); - await db.deleteRepo('test-repo'); - await db.deleteUser('u1'); - await db.deleteUser('u2'); + + // don't clean up data as cypress tests rely on it being present + // await db.deleteRepo('test-repo'); + // await db.deleteUser('u1'); + // await db.deleteUser('u2'); }); }); From 008e4faba7e3dbcc6247fef068260d38f0e24c25 Mon Sep 17 00:00:00 2001 From: Kris West Date: Tue, 15 Apr 2025 12:26:48 +0100 Subject: [PATCH 20/25] test: basic db test coverage --- test/testDb.test.js | 126 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 126 insertions(+) create mode 100644 test/testDb.test.js diff --git a/test/testDb.test.js b/test/testDb.test.js new file mode 100644 index 000000000..896be0362 --- /dev/null +++ b/test/testDb.test.js @@ -0,0 +1,126 @@ +// This test needs to run first +const chai = require('chai'); +const db = require('../src/db'); + +const { expect } = chai; + +const TEST_REPO = { + project: 'finos', + name: 'db-test-repo', + url: 'https://github.com/finos/db-test-repo.git', +}; + +const TEST_USER = { + username: 'db-u1', + password: 'abc', + gitAccount: 'db-test-user', + email: 'db-test@test.com', + admin: true, +}; + +const TEST_PUSH = { + steps: [], + error: false, + blocked: true, + allowPush: false, + authorised: false, + canceled: true, + rejected: false, + autoApproved: false, + autoRejected: false, + commitData: [], + id: '0000000000000000000000000000000000000000__1744380874110', + type: 'push', + method: 'get', + timestamp: 1744380903338, + project: 'finos', + repoName: 'db-test-repo.git', + url: 'https://github.com/finos/db-test-repo.git', + repo: 'finos/db-test-repo.git', + user: 'db-test-user', + userEmail: 'db-test@test.com', + lastStep: null, + blockedMessage: + '\n\n\nGitProxy has received your push:\n\nhttp://localhost:8080/requests/0000000000000000000000000000000000000000__1744380874110\n\n\n', + _id: 'GIMEz8tU2KScZiTz', + attestation: null, +}; + +/** + * Clean up response data from the DB by removing an extraneous properties, + * allowing comparison with expect. + * @param {object} example Example element from which columns to retain are extracted + * @param {array} responses Array of responses to clean. + * @return {array} Array of cleaned up responses. + */ +const cleanResponseData = (example, responses) => { + const columns = Object.keys(example); + return responses.map((response) => { + const cleanResponse = {}; + columns.forEach((col) => { + cleanResponse[col] = response[col]; + }); + return cleanResponse; + }); +}; + +// Use this test as a template +describe('Database client', async () => { + before(async function () {}); + + it('should be able to create a repo', async function () { + await db.createRepo(TEST_REPO); + const repos = await db.getRepos(); + const cleanRepos = cleanResponseData(TEST_REPO, repos); + expect(cleanRepos).to.deep.include(TEST_REPO); + }); + + it('should be able to delete a repo', async function () { + await db.deleteRepo(TEST_REPO.name); + const repos = await db.getRepos(); + const cleanRepos = cleanResponseData(TEST_REPO, repos); + expect(cleanRepos).to.not.deep.include(TEST_REPO); + }); + + it('should be able to create a user', async function () { + await db.createUser( + TEST_USER.username, + TEST_USER.password, + TEST_USER.email, + TEST_USER.gitAccount, + TEST_USER.admin, + ); + const users = await db.getUsers(); + console.log('TEST USER:', JSON.stringify(TEST_USER, null, 2)); + console.log('USERS:', JSON.stringify(users, null, 2)); + // remove password as it will have been hashed + // eslint-disable-next-line no-unused-vars + const { password: _, ...TEST_USER_CLEAN } = TEST_USER; + const cleanUsers = cleanResponseData(TEST_USER_CLEAN, users); + console.log('CLEAN USERS:', JSON.stringify(cleanUsers, null, 2)); + expect(cleanUsers).to.deep.include(TEST_USER_CLEAN); + }); + + it('should be able to delete a user', async function () { + await db.deleteUser(TEST_USER.username); + const users = await db.getUsers(); + const cleanUsers = cleanResponseData(TEST_USER, users); + expect(cleanUsers).to.not.deep.include(TEST_USER); + }); + + it('should be able to create a push', async function () { + await db.writeAudit(TEST_PUSH); + const pushes = await db.getPushes(); + const cleanPushes = cleanResponseData(TEST_PUSH, pushes); + expect(cleanPushes).to.deep.include(TEST_PUSH); + }); + + it('should be able to delete a push', async function () { + await db.deletePush(TEST_PUSH.id); + const pushes = await db.getPushes(); + const cleanPushes = cleanResponseData(TEST_PUSH, pushes); + expect(cleanPushes).to.not.deep.include(TEST_PUSH); + }); + + after(async function () {}); +}); From fa234f8d9f58f2ee1c5882eeb5ad8f6eecae0beb Mon Sep 17 00:00:00 2001 From: Kris West Date: Fri, 11 Apr 2025 11:03:27 +0100 Subject: [PATCH 21/25] fix: clean-up DB after CLI tests --- packages/git-proxy-cli/test/testCli.test.js | 442 +++++--------------- packages/git-proxy-cli/test/testCliUtils.js | 27 ++ src/db/file/index.ts | 10 +- src/db/file/pushes.ts | 12 + src/db/index.ts | 1 + src/db/mongo/index.ts | 15 +- src/db/mongo/pushes.ts | 11 +- 7 files changed, 155 insertions(+), 363 deletions(-) diff --git a/packages/git-proxy-cli/test/testCli.test.js b/packages/git-proxy-cli/test/testCli.test.js index 9f6052661..385e38d28 100644 --- a/packages/git-proxy-cli/test/testCli.test.js +++ b/packages/git-proxy-cli/test/testCli.test.js @@ -19,8 +19,8 @@ const GHOST_PUSH_ID = const TEST_REPO_CONFIG = { project: 'finos', name: 'git-proxy-test', - url: 'https://github.com/finos/git-proxy-test.git' -} + url: 'https://github.com/finos/git-proxy-test.git', +}; const TEST_REPO = 'finos/git-proxy-test.git'; describe('test git-proxy-cli', function () { @@ -36,12 +36,7 @@ describe('test git-proxy-cli', function () { 'Options:', 'You need at least one command before moving on', ]; - await helper.runCli( - cli, - expectedExitCode, - expectedMessages, - expectedErrorMessages, - ); + await helper.runCli(cli, expectedExitCode, expectedMessages, expectedErrorMessages); }); it(`print help if invalid command or option is given`, async function () { @@ -53,12 +48,7 @@ describe('test git-proxy-cli', function () { 'Options:', 'Unknown arguments: invalid, invalid', ]; - await helper.runCli( - cli, - expectedExitCode, - expectedMessages, - expectedErrorMessages, - ); + await helper.runCli(cli, expectedExitCode, expectedMessages, expectedErrorMessages); }); it(`print help if "--help" option is given`, async function () { @@ -66,12 +56,7 @@ describe('test git-proxy-cli', function () { const expectedExitCode = 0; const expectedMessages = ['Commands:', 'Options:']; const expectedErrorMessages = null; - await helper.runCli( - cli, - expectedExitCode, - expectedMessages, - expectedErrorMessages, - ); + await helper.runCli(cli, expectedExitCode, expectedMessages, expectedErrorMessages); }); }); @@ -83,12 +68,7 @@ describe('test git-proxy-cli', function () { const expectedExitCode = 0; const expectedMessages = ['0.1.0']; const expectedErrorMessages = null; - await helper.runCli( - cli, - expectedExitCode, - expectedMessages, - expectedErrorMessages, - ); + await helper.runCli(cli, expectedExitCode, expectedMessages, expectedErrorMessages); }); }); @@ -100,12 +80,7 @@ describe('test git-proxy-cli', function () { const expectedExitCode = 0; const expectedMessages = ['GitProxy URL:']; const expectedErrorMessages = null; - await helper.runCli( - cli, - expectedExitCode, - expectedMessages, - expectedErrorMessages, - ); + await helper.runCli(cli, expectedExitCode, expectedMessages, expectedErrorMessages); }); }); @@ -117,12 +92,11 @@ describe('test git-proxy-cli', function () { const testEmail = 'jane.doe@email.com'; before(async function () { - await helper.addUserToDb( - testUser, - testPassword, - testEmail, - 'testGitAccount', - ); + await helper.addUserToDb(testUser, testPassword, testEmail, 'testGitAccount'); + }); + + after(async function () { + await helper.removeUserFromDb(testUser); }); it('login shoud fail when server is down', async function () { @@ -132,12 +106,7 @@ describe('test git-proxy-cli', function () { const expectedExitCode = 2; const expectedMessages = null; const expectedErrorMessages = [`Error: Login '${username}':`]; - await helper.runCli( - cli, - expectedExitCode, - expectedMessages, - expectedErrorMessages, - ); + await helper.runCli(cli, expectedExitCode, expectedMessages, expectedErrorMessages); }); it('login shoud fail with invalid credentials', async function () { @@ -149,12 +118,7 @@ describe('test git-proxy-cli', function () { const expectedErrorMessages = [`Error: Login '${username}': '401'`]; try { await helper.startServer(service); - await helper.runCli( - cli, - expectedExitCode, - expectedMessages, - expectedErrorMessages, - ); + await helper.runCli(cli, expectedExitCode, expectedMessages, expectedErrorMessages); } finally { await helper.closeServer(service.httpServer); } @@ -165,18 +129,11 @@ describe('test git-proxy-cli', function () { const password = 'admin'; const cli = `npx -- @finos/git-proxy-cli login --username ${username} --password ${password}`; const expectedExitCode = 0; - const expectedMessages = [ - `Login "${username}" (admin): OK`, - ]; + const expectedMessages = [`Login "${username}" (admin): OK`]; const expectedErrorMessages = null; try { await helper.startServer(service); - await helper.runCli( - cli, - expectedExitCode, - expectedMessages, - expectedErrorMessages, - ); + await helper.runCli(cli, expectedExitCode, expectedMessages, expectedErrorMessages); } finally { await helper.closeServer(service.httpServer); } @@ -189,12 +146,7 @@ describe('test git-proxy-cli', function () { const expectedErrorMessages = null; try { await helper.startServer(service); - await helper.runCli( - cli, - expectedExitCode, - expectedMessages, - expectedErrorMessages, - ); + await helper.runCli(cli, expectedExitCode, expectedMessages, expectedErrorMessages); } finally { await helper.closeServer(service.httpServer); } @@ -211,20 +163,13 @@ describe('test git-proxy-cli', function () { const expectedExitCode = 0; const expectedMessages = [`Logout: OK`]; const expectedErrorMessages = null; - await helper.runCli( - cli, - expectedExitCode, - expectedMessages, - expectedErrorMessages, - ); + await helper.runCli(cli, expectedExitCode, expectedMessages, expectedErrorMessages); }); it('logout should succeed when server is down (but logged in before)', async function () { try { await helper.startServer(service); - await helper.runCli( - `npx -- @finos/git-proxy-cli login --username admin --password admin`, - ); + await helper.runCli(`npx -- @finos/git-proxy-cli login --username admin --password admin`); } finally { await helper.closeServer(service.httpServer); } @@ -233,12 +178,7 @@ describe('test git-proxy-cli', function () { const expectedExitCode = 0; const expectedMessages = [`Logout: OK`]; const expectedErrorMessages = null; - await helper.runCli( - cli, - expectedExitCode, - expectedMessages, - expectedErrorMessages, - ); + await helper.runCli(cli, expectedExitCode, expectedMessages, expectedErrorMessages); }); it('logout should succeed when not authenticated (server is up)', async function () { @@ -250,12 +190,7 @@ describe('test git-proxy-cli', function () { const expectedMessages = [`Logout: OK`]; const expectedErrorMessages = null; await helper.startServer(service); - await helper.runCli( - cli, - expectedExitCode, - expectedMessages, - expectedErrorMessages, - ); + await helper.runCli(cli, expectedExitCode, expectedMessages, expectedErrorMessages); } finally { await helper.closeServer(service.httpServer); } @@ -264,20 +199,13 @@ describe('test git-proxy-cli', function () { it('logout shoud be successful when authenticated (server is up)', async function () { try { await helper.startServer(service); - await helper.runCli( - `npx -- @finos/git-proxy-cli login --username admin --password admin`, - ); + await helper.runCli(`npx -- @finos/git-proxy-cli login --username admin --password admin`); const cli = `npx -- @finos/git-proxy-cli logout`; const expectedExitCode = 0; const expectedMessages = [`Logout: OK`]; const expectedErrorMessages = null; - await helper.runCli( - cli, - expectedExitCode, - expectedMessages, - expectedErrorMessages, - ); + await helper.runCli(cli, expectedExitCode, expectedMessages, expectedErrorMessages); } finally { await helper.closeServer(service.httpServer); } @@ -289,18 +217,21 @@ describe('test git-proxy-cli', function () { describe('test git-proxy-cli :: authorise', function () { const pushId = `auth000000000000000000000000000000000000__${Date.now()}`; - before(async function() { + before(async function () { await helper.addRepoToDb(TEST_REPO_CONFIG); await helper.addGitPushToDb(pushId, TEST_REPO); - }) + }); + + after(async function () { + await helper.removeGitPushFromDb(pushId); + await helper.removeRepoFromDb(TEST_REPO_CONFIG.name); + }); it('attempt to authorise should fail when server is down', async function () { try { // start server -> login -> stop server await helper.startServer(service); - await helper.runCli( - `npx -- @finos/git-proxy-cli login --username admin --password admin`, - ); + await helper.runCli(`npx -- @finos/git-proxy-cli login --username admin --password admin`); } finally { await helper.closeServer(service.httpServer); } @@ -310,12 +241,7 @@ describe('test git-proxy-cli', function () { const expectedExitCode = 2; const expectedMessages = null; const expectedErrorMessages = ['Error: Authorise:']; - await helper.runCli( - cli, - expectedExitCode, - expectedMessages, - expectedErrorMessages, - ); + await helper.runCli(cli, expectedExitCode, expectedMessages, expectedErrorMessages); }); it('attempt to authorise should fail when not authenticated', async function () { @@ -325,15 +251,8 @@ describe('test git-proxy-cli', function () { const cli = `npx -- @finos/git-proxy-cli authorise --id ${id}`; const expectedExitCode = 1; const expectedMessages = null; - const expectedErrorMessages = [ - 'Error: Authorise: Authentication required', - ]; - await helper.runCli( - cli, - expectedExitCode, - expectedMessages, - expectedErrorMessages, - ); + const expectedErrorMessages = ['Error: Authorise: Authentication required']; + await helper.runCli(cli, expectedExitCode, expectedMessages, expectedErrorMessages); }); it('attempt to authorise should fail when not authenticated (server restarted)', async function () { @@ -344,15 +263,8 @@ describe('test git-proxy-cli', function () { const cli = `npx -- @finos/git-proxy-cli authorise --id ${id}`; const expectedExitCode = 3; const expectedMessages = null; - const expectedErrorMessages = [ - 'Error: Authorise: Authentication required', - ]; - await helper.runCli( - cli, - expectedExitCode, - expectedMessages, - expectedErrorMessages, - ); + const expectedErrorMessages = ['Error: Authorise: Authentication required']; + await helper.runCli(cli, expectedExitCode, expectedMessages, expectedErrorMessages); } finally { await helper.closeServer(service.httpServer); } @@ -361,23 +273,14 @@ describe('test git-proxy-cli', function () { it('attempt to authorise should fail when git push ID not found', async function () { try { await helper.startServer(service); - await helper.runCli( - `npx -- @finos/git-proxy-cli login --username admin --password admin`, - ); + await helper.runCli(`npx -- @finos/git-proxy-cli login --username admin --password admin`); const id = GHOST_PUSH_ID; const cli = `npx -- @finos/git-proxy-cli authorise --id ${id}`; const expectedExitCode = 4; const expectedMessages = null; - const expectedErrorMessages = [ - `Error: Authorise: ID: '${id}': Not Found`, - ]; - await helper.runCli( - cli, - expectedExitCode, - expectedMessages, - expectedErrorMessages, - ); + const expectedErrorMessages = [`Error: Authorise: ID: '${id}': Not Found`]; + await helper.runCli(cli, expectedExitCode, expectedMessages, expectedErrorMessages); } finally { await helper.closeServer(service.httpServer); } @@ -389,18 +292,21 @@ describe('test git-proxy-cli', function () { describe('test git-proxy-cli :: cancel', function () { const pushId = `cancel0000000000000000000000000000000000__${Date.now()}`; - before(async function() { + before(async function () { await helper.addRepoToDb(TEST_REPO_CONFIG); await helper.addGitPushToDb(pushId, TEST_REPO); - }) - + }); + + after(async function () { + await helper.removeGitPushFromDb(pushId); + await helper.removeRepoFromDb(TEST_REPO_CONFIG.name); + }); + it('attempt to cancel should fail when server is down', async function () { try { // start server -> login -> stop server await helper.startServer(service); - await helper.runCli( - `npx -- @finos/git-proxy-cli login --username admin --password admin`, - ); + await helper.runCli(`npx -- @finos/git-proxy-cli login --username admin --password admin`); } finally { await helper.closeServer(service.httpServer); } @@ -410,12 +316,7 @@ describe('test git-proxy-cli', function () { const expectedExitCode = 2; const expectedMessages = null; const expectedErrorMessages = ['Error: Cancel:']; - await helper.runCli( - cli, - expectedExitCode, - expectedMessages, - expectedErrorMessages, - ); + await helper.runCli(cli, expectedExitCode, expectedMessages, expectedErrorMessages); }); it('attempt to cancel should fail when not authenticated', async function () { @@ -426,12 +327,7 @@ describe('test git-proxy-cli', function () { const expectedExitCode = 1; const expectedMessages = null; const expectedErrorMessages = ['Error: Cancel: Authentication required']; - await helper.runCli( - cli, - expectedExitCode, - expectedMessages, - expectedErrorMessages, - ); + await helper.runCli(cli, expectedExitCode, expectedMessages, expectedErrorMessages); }); it('attempt to cancel should fail when not authenticated (server restarted)', async function () { @@ -442,15 +338,8 @@ describe('test git-proxy-cli', function () { const cli = `npx -- @finos/git-proxy-cli cancel --id ${id}`; const expectedExitCode = 3; const expectedMessages = null; - const expectedErrorMessages = [ - 'Error: Cancel: Authentication required', - ]; - await helper.runCli( - cli, - expectedExitCode, - expectedMessages, - expectedErrorMessages, - ); + const expectedErrorMessages = ['Error: Cancel: Authentication required']; + await helper.runCli(cli, expectedExitCode, expectedMessages, expectedErrorMessages); // }); } finally { await helper.closeServer(service.httpServer); @@ -460,21 +349,14 @@ describe('test git-proxy-cli', function () { it('attempt to cancel should fail when git push ID not found', async function () { try { await helper.startServer(service); - await helper.runCli( - `npx -- @finos/git-proxy-cli login --username admin --password admin`, - ); + await helper.runCli(`npx -- @finos/git-proxy-cli login --username admin --password admin`); const id = GHOST_PUSH_ID; const cli = `npx -- @finos/git-proxy-cli cancel --id ${id}`; const expectedExitCode = 4; const expectedMessages = null; const expectedErrorMessages = [`Error: Cancel: ID: '${id}': Not Found`]; - await helper.runCli( - cli, - expectedExitCode, - expectedMessages, - expectedErrorMessages, - ); + await helper.runCli(cli, expectedExitCode, expectedMessages, expectedErrorMessages); } finally { await helper.closeServer(service.httpServer); } @@ -488,9 +370,7 @@ describe('test git-proxy-cli', function () { try { // start server -> login -> stop server await helper.startServer(service); - await helper.runCli( - `npx -- @finos/git-proxy-cli login --username admin --password admin`, - ); + await helper.runCli(`npx -- @finos/git-proxy-cli login --username admin --password admin`); } finally { await helper.closeServer(service.httpServer); } @@ -499,12 +379,7 @@ describe('test git-proxy-cli', function () { const expectedExitCode = 2; const expectedMessages = null; const expectedErrorMessages = ['Error: List:']; - await helper.runCli( - cli, - expectedExitCode, - expectedMessages, - expectedErrorMessages, - ); + await helper.runCli(cli, expectedExitCode, expectedMessages, expectedErrorMessages); }); it('attempt to ls should fail when not authenticated', async function () { @@ -514,31 +389,19 @@ describe('test git-proxy-cli', function () { const expectedExitCode = 1; const expectedMessages = null; const expectedErrorMessages = ['Error: List: Authentication required']; - await helper.runCli( - cli, - expectedExitCode, - expectedMessages, - expectedErrorMessages, - ); + await helper.runCli(cli, expectedExitCode, expectedMessages, expectedErrorMessages); }); it('attempt to ls should fail when invalid option given', async function () { try { await helper.startServer(service); - await helper.runCli( - `npx -- @finos/git-proxy-cli login --username admin --password admin`, - ); + await helper.runCli(`npx -- @finos/git-proxy-cli login --username admin --password admin`); const cli = `npx -- @finos/git-proxy-cli ls --invalid`; const expectedExitCode = 1; const expectedMessages = null; const expectedErrorMessages = ['Options:', 'Unknown argument: invalid']; - await helper.runCli( - cli, - expectedExitCode, - expectedMessages, - expectedErrorMessages, - ); + await helper.runCli(cli, expectedExitCode, expectedMessages, expectedErrorMessages); } finally { await helper.closeServer(service.httpServer); } @@ -550,18 +413,21 @@ describe('test git-proxy-cli', function () { describe('test git-proxy-cli :: reject', function () { const pushId = `reject0000000000000000000000000000000000__${Date.now()}`; - before(async function() { + before(async function () { await helper.addRepoToDb(TEST_REPO_CONFIG); await helper.addGitPushToDb(pushId, TEST_REPO); - }) - + }); + + after(async function () { + await helper.removeGitPushFromDb(pushId); + await helper.removeRepoFromDb(TEST_REPO_CONFIG.name); + }); + it('attempt to reject should fail when server is down', async function () { try { // start server -> login -> stop server await helper.startServer(service); - await helper.runCli( - `npx -- @finos/git-proxy-cli login --username admin --password admin`, - ); + await helper.runCli(`npx -- @finos/git-proxy-cli login --username admin --password admin`); } finally { await helper.closeServer(service.httpServer); } @@ -571,12 +437,7 @@ describe('test git-proxy-cli', function () { const expectedExitCode = 2; const expectedMessages = null; const expectedErrorMessages = ['Error: Reject:']; - await helper.runCli( - cli, - expectedExitCode, - expectedMessages, - expectedErrorMessages, - ); + await helper.runCli(cli, expectedExitCode, expectedMessages, expectedErrorMessages); }); it('attempt to reject should fail when not authenticated', async function () { @@ -587,12 +448,7 @@ describe('test git-proxy-cli', function () { const expectedExitCode = 1; const expectedMessages = null; const expectedErrorMessages = ['Error: Reject: Authentication required']; - await helper.runCli( - cli, - expectedExitCode, - expectedMessages, - expectedErrorMessages, - ); + await helper.runCli(cli, expectedExitCode, expectedMessages, expectedErrorMessages); }); it('attempt to reject should fail when not authenticated (server restarted)', async function () { @@ -603,15 +459,8 @@ describe('test git-proxy-cli', function () { const cli = `npx -- @finos/git-proxy-cli reject --id ${id}`; const expectedExitCode = 3; const expectedMessages = null; - const expectedErrorMessages = [ - 'Error: Reject: Authentication required', - ]; - await helper.runCli( - cli, - expectedExitCode, - expectedMessages, - expectedErrorMessages, - ); + const expectedErrorMessages = ['Error: Reject: Authentication required']; + await helper.runCli(cli, expectedExitCode, expectedMessages, expectedErrorMessages); } finally { await helper.closeServer(service.httpServer); } @@ -620,21 +469,14 @@ describe('test git-proxy-cli', function () { it('attempt to reject should fail when git push ID not found', async function () { try { await helper.startServer(service); - await helper.runCli( - `npx -- @finos/git-proxy-cli login --username admin --password admin`, - ); + await helper.runCli(`npx -- @finos/git-proxy-cli login --username admin --password admin`); const id = GHOST_PUSH_ID; const cli = `npx -- @finos/git-proxy-cli reject --id ${id}`; const expectedExitCode = 4; const expectedMessages = null; const expectedErrorMessages = [`Error: Reject: ID: '${id}': Not Found`]; - await helper.runCli( - cli, - expectedExitCode, - expectedMessages, - expectedErrorMessages, - ); + await helper.runCli(cli, expectedExitCode, expectedMessages, expectedErrorMessages); } finally { await helper.closeServer(service.httpServer); } @@ -653,12 +495,16 @@ describe('test git-proxy-cli', function () { await helper.addGitPushToDb(pushId, TEST_REPO, gitAccount); }); + after(async function () { + await helper.removeUserFromDb('testuser1'); + await helper.removeGitPushFromDb(pushId); + await helper.removeRepoFromDb(TEST_REPO_CONFIG.name); + }); + it('attempt to ls should list existing push', async function () { try { await helper.startServer(service); - await helper.runCli( - `npx -- @finos/git-proxy-cli login --username admin --password admin`, - ); + await helper.runCli(`npx -- @finos/git-proxy-cli login --username admin --password admin`); const cli = `npx -- @finos/git-proxy-cli ls --authorised false --blocked true --canceled false --rejected false`; const expectedExitCode = 0; @@ -672,12 +518,7 @@ describe('test git-proxy-cli', function () { 'rejected: false', ]; const expectedErrorMessages = null; - await helper.runCli( - cli, - expectedExitCode, - expectedMessages, - expectedErrorMessages, - ); + await helper.runCli(cli, expectedExitCode, expectedMessages, expectedErrorMessages); } finally { await helper.closeServer(service.httpServer); } @@ -686,20 +527,13 @@ describe('test git-proxy-cli', function () { it('attempt to ls should not list existing push when filtered for authorised', async function () { try { await helper.startServer(service); - await helper.runCli( - `npx -- @finos/git-proxy-cli login --username admin --password admin`, - ); + await helper.runCli(`npx -- @finos/git-proxy-cli login --username admin --password admin`); const cli = `npx -- @finos/git-proxy-cli ls --authorised true`; const expectedExitCode = 0; const expectedMessages = ['[]']; const expectedErrorMessages = null; - await helper.runCli( - cli, - expectedExitCode, - expectedMessages, - expectedErrorMessages, - ); + await helper.runCli(cli, expectedExitCode, expectedMessages, expectedErrorMessages); } finally { await helper.closeServer(service.httpServer); } @@ -708,20 +542,13 @@ describe('test git-proxy-cli', function () { it('attempt to ls should not list existing push when filtered for canceled', async function () { try { await helper.startServer(service); - await helper.runCli( - `npx -- @finos/git-proxy-cli login --username admin --password admin`, - ); + await helper.runCli(`npx -- @finos/git-proxy-cli login --username admin --password admin`); const cli = `npx -- @finos/git-proxy-cli ls --canceled true`; const expectedExitCode = 0; const expectedMessages = ['[]']; const expectedErrorMessages = null; - await helper.runCli( - cli, - expectedExitCode, - expectedMessages, - expectedErrorMessages, - ); + await helper.runCli(cli, expectedExitCode, expectedMessages, expectedErrorMessages); } finally { await helper.closeServer(service.httpServer); } @@ -730,20 +557,13 @@ describe('test git-proxy-cli', function () { it('attempt to ls should not list existing push when filtered for rejected', async function () { try { await helper.startServer(service); - await helper.runCli( - `npx -- @finos/git-proxy-cli login --username admin --password admin`, - ); + await helper.runCli(`npx -- @finos/git-proxy-cli login --username admin --password admin`); const cli = `npx -- @finos/git-proxy-cli ls --rejected true`; const expectedExitCode = 0; const expectedMessages = ['[]']; const expectedErrorMessages = null; - await helper.runCli( - cli, - expectedExitCode, - expectedMessages, - expectedErrorMessages, - ); + await helper.runCli(cli, expectedExitCode, expectedMessages, expectedErrorMessages); } finally { await helper.closeServer(service.httpServer); } @@ -752,20 +572,13 @@ describe('test git-proxy-cli', function () { it('attempt to ls should not list existing push when filtered for non-blocked', async function () { try { await helper.startServer(service); - await helper.runCli( - `npx -- @finos/git-proxy-cli login --username admin --password admin`, - ); + await helper.runCli(`npx -- @finos/git-proxy-cli login --username admin --password admin`); const cli = `npx -- @finos/git-proxy-cli ls --blocked false`; const expectedExitCode = 0; const expectedMessages = ['[]']; const expectedErrorMessages = null; - await helper.runCli( - cli, - expectedExitCode, - expectedMessages, - expectedErrorMessages, - ); + await helper.runCli(cli, expectedExitCode, expectedMessages, expectedErrorMessages); } finally { await helper.closeServer(service.httpServer); } @@ -774,42 +587,25 @@ describe('test git-proxy-cli', function () { it('authorise push and test if appears on authorised list', async function () { try { await helper.startServer(service); - await helper.runCli( - `npx -- @finos/git-proxy-cli login --username admin --password admin`, - ); + await helper.runCli(`npx -- @finos/git-proxy-cli login --username admin --password admin`); let cli = `npx -- @finos/git-proxy-cli ls --authorised true --canceled false --rejected false`; let expectedExitCode = 0; let expectedMessages = ['[]']; let expectedErrorMessages = null; - await helper.runCli( - cli, - expectedExitCode, - expectedMessages, - expectedErrorMessages, - ); + await helper.runCli(cli, expectedExitCode, expectedMessages, expectedErrorMessages); cli = `npx -- @finos/git-proxy-cli authorise --id ${pushId}`; expectedExitCode = 0; expectedMessages = [`Authorise: ID: '${pushId}': OK`]; expectedErrorMessages = null; - await helper.runCli( - cli, - expectedExitCode, - expectedMessages, - expectedErrorMessages, - ); + await helper.runCli(cli, expectedExitCode, expectedMessages, expectedErrorMessages); cli = `npx -- @finos/git-proxy-cli ls --authorised true --canceled false --rejected false`; expectedExitCode = 0; expectedMessages = [pushId, TEST_REPO]; expectedErrorMessages = null; - await helper.runCli( - cli, - expectedExitCode, - expectedMessages, - expectedErrorMessages, - ); + await helper.runCli(cli, expectedExitCode, expectedMessages, expectedErrorMessages); } finally { await helper.closeServer(service.httpServer); } @@ -818,42 +614,25 @@ describe('test git-proxy-cli', function () { it('reject push and test if appears on rejected list', async function () { try { await helper.startServer(service); - await helper.runCli( - `npx -- @finos/git-proxy-cli login --username admin --password admin`, - ); + await helper.runCli(`npx -- @finos/git-proxy-cli login --username admin --password admin`); let cli = `npx -- @finos/git-proxy-cli ls --authorised false --canceled false --rejected true`; let expectedExitCode = 0; let expectedMessages = ['[]']; let expectedErrorMessages = null; - await helper.runCli( - cli, - expectedExitCode, - expectedMessages, - expectedErrorMessages, - ); + await helper.runCli(cli, expectedExitCode, expectedMessages, expectedErrorMessages); cli = `npx -- @finos/git-proxy-cli reject --id ${pushId}`; expectedExitCode = 0; expectedMessages = [`Reject: ID: '${pushId}': OK`]; expectedErrorMessages = null; - await helper.runCli( - cli, - expectedExitCode, - expectedMessages, - expectedErrorMessages, - ); + await helper.runCli(cli, expectedExitCode, expectedMessages, expectedErrorMessages); cli = `npx -- @finos/git-proxy-cli ls --authorised false --canceled false --rejected true`; expectedExitCode = 0; expectedMessages = [pushId, TEST_REPO]; expectedErrorMessages = null; - await helper.runCli( - cli, - expectedExitCode, - expectedMessages, - expectedErrorMessages, - ); + await helper.runCli(cli, expectedExitCode, expectedMessages, expectedErrorMessages); } finally { await helper.closeServer(service.httpServer); } @@ -862,42 +641,25 @@ describe('test git-proxy-cli', function () { it('cancel push and test if appears on canceled list', async function () { try { await helper.startServer(service); - await helper.runCli( - `npx -- @finos/git-proxy-cli login --username admin --password admin`, - ); + await helper.runCli(`npx -- @finos/git-proxy-cli login --username admin --password admin`); let cli = `npx -- @finos/git-proxy-cli ls --authorised false --canceled true --rejected false`; let expectedExitCode = 0; let expectedMessages = ['[]']; let expectedErrorMessages = null; - await helper.runCli( - cli, - expectedExitCode, - expectedMessages, - expectedErrorMessages, - ); + await helper.runCli(cli, expectedExitCode, expectedMessages, expectedErrorMessages); cli = `npx -- @finos/git-proxy-cli cancel --id ${pushId}`; expectedExitCode = 0; expectedMessages = [`Cancel: ID: '${pushId}': OK`]; expectedErrorMessages = null; - await helper.runCli( - cli, - expectedExitCode, - expectedMessages, - expectedErrorMessages, - ); + await helper.runCli(cli, expectedExitCode, expectedMessages, expectedErrorMessages); cli = `npx -- @finos/git-proxy-cli ls --authorised false --canceled true --rejected false`; expectedExitCode = 0; expectedMessages = [pushId, TEST_REPO]; expectedErrorMessages = null; - await helper.runCli( - cli, - expectedExitCode, - expectedMessages, - expectedErrorMessages, - ); + await helper.runCli(cli, expectedExitCode, expectedMessages, expectedErrorMessages); } finally { await helper.closeServer(service.httpServer); await helper.removeCookiesFile(); diff --git a/packages/git-proxy-cli/test/testCliUtils.js b/packages/git-proxy-cli/test/testCliUtils.js index d9e633067..557857619 100644 --- a/packages/git-proxy-cli/test/testCliUtils.js +++ b/packages/git-proxy-cli/test/testCliUtils.js @@ -164,6 +164,14 @@ async function addRepoToDb(newRepo, debug = false) { } } +/** + * Removes a repo from the DB. + * @param {string} repoName The name of the repo to remove. + */ +async function removeRepoFromDb(repoName) { + await db.deleteRepo(repoName); +} + /** * Add a new git push record to the database. * @param {string} id The ID of the git push. @@ -205,6 +213,14 @@ async function addGitPushToDb(id, repo, user = null, debug = false) { } } +/** + * Removes a push from the DB + * @param {string} id + */ +async function removeGitPushFromDb(id) { + await db.deletePush(id); +} + /** * Add new user record to the database. * @param {string} username The user name. @@ -221,13 +237,24 @@ async function addUserToDb(username, password, email, gitAccount, admin = false, } } +/** + * Remove a user record from the database if present. + * @param {string} username The user name. + */ +async function removeUserFromDb(username) { + await db.deleteUser(username); +} + module.exports = { runCli: runCli, startServer: startServer, closeServer: closeServer, addRepoToDb: addRepoToDb, + removeRepoFromDb: removeRepoFromDb, addGitPushToDb: addGitPushToDb, + removeGitPushFromDb: removeGitPushFromDb, addUserToDb: addUserToDb, + removeUserFromDb: removeUserFromDb, createCookiesFileWithExpiredCookie: createCookiesFileWithExpiredCookie, removeCookiesFile: removeCookiesFile, }; diff --git a/src/db/file/index.ts b/src/db/file/index.ts index a31610173..6ac1c2088 100644 --- a/src/db/file/index.ts +++ b/src/db/file/index.ts @@ -6,6 +6,7 @@ export const { getPushes, writeAudit, getPush, + deletePush, authorise, cancel, reject, @@ -26,11 +27,4 @@ export const { canUserApproveRejectPushRepo, } = repo; -export const { - findUser, - findUserByOIDC, - getUsers, - createUser, - deleteUser, - updateUser, -} = users; +export const { findUser, findUserByOIDC, getUsers, createUser, deleteUser, updateUser } = users; diff --git a/src/db/file/pushes.ts b/src/db/file/pushes.ts index bc2198d55..b58aea4eb 100644 --- a/src/db/file/pushes.ts +++ b/src/db/file/pushes.ts @@ -55,6 +55,18 @@ export const getPush = async (id: string) => { }); }; +export const deletePush = async (id: string) => { + return new Promise((resolve, reject) => { + db.remove({ id }, (err) => { + if (err) { + reject(err); + } else { + resolve(); + } + }); + }); +}; + export const writeAudit = async (action: Action) => { return new Promise((resolve, reject) => { const options = { multi: false, upsert: true }; diff --git a/src/db/index.ts b/src/db/index.ts index 0fc681058..ff1189f1b 100644 --- a/src/db/index.ts +++ b/src/db/index.ts @@ -63,6 +63,7 @@ export const { getPushes, writeAudit, getPush, + deletePush, findUser, findUserByOIDC, getUsers, diff --git a/src/db/mongo/index.ts b/src/db/mongo/index.ts index 11f526c2a..a6d7ce6b2 100644 --- a/src/db/mongo/index.ts +++ b/src/db/mongo/index.ts @@ -1,16 +1,15 @@ import * as helper from './helper'; import * as pushes from './pushes'; -import * as repo from './repo'; +import * as repo from './repo'; import * as users from './users'; -export const { - getSessionStore, -} = helper; +export const { getSessionStore } = helper; export const { getPushes, writeAudit, getPush, + deletePush, authorise, cancel, reject, @@ -31,10 +30,4 @@ export const { canUserApproveRejectPushRepo, } = repo; -export const { - findUser, - getUsers, - createUser, - deleteUser, - updateUser, -} = users; +export const { findUser, getUsers, createUser, deleteUser, updateUser } = users; diff --git a/src/db/mongo/pushes.ts b/src/db/mongo/pushes.ts index 609db1252..8778bdf73 100644 --- a/src/db/mongo/pushes.ts +++ b/src/db/mongo/pushes.ts @@ -13,9 +13,7 @@ const defaultPushQuery: PushQuery = { authorised: false, }; -export const getPushes = async ( - query: PushQuery = defaultPushQuery -): Promise => { +export const getPushes = async (query: PushQuery = defaultPushQuery): Promise => { return findDocuments(collectionName, query, { projection: { _id: 0, @@ -44,7 +42,12 @@ export const getPushes = async ( export const getPush = async (id: string): Promise => { const doc = await findOneDocument(collectionName, { id }); - return doc ? toClass(doc, Action.prototype) as Action : null; + return doc ? (toClass(doc, Action.prototype) as Action) : null; +}; + +export const deletePush = async function (id: string) { + const collection = await connect(collectionName); + return collection.deleteOne({ id }); }; export const writeAudit = async (action: Action): Promise => { From 734fc30c870b33626ac63bbf569034a586ba3358 Mon Sep 17 00:00:00 2001 From: Kris West Date: Tue, 15 Apr 2025 13:56:45 +0100 Subject: [PATCH 22/25] test: more DB test coverage --- test/testDb.test.js | 65 ++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 61 insertions(+), 4 deletions(-) diff --git a/test/testDb.test.js b/test/testDb.test.js index 896be0362..740ad9669 100644 --- a/test/testDb.test.js +++ b/test/testDb.test.js @@ -91,16 +91,36 @@ describe('Database client', async () => { TEST_USER.admin, ); const users = await db.getUsers(); - console.log('TEST USER:', JSON.stringify(TEST_USER, null, 2)); - console.log('USERS:', JSON.stringify(users, null, 2)); // remove password as it will have been hashed // eslint-disable-next-line no-unused-vars const { password: _, ...TEST_USER_CLEAN } = TEST_USER; const cleanUsers = cleanResponseData(TEST_USER_CLEAN, users); - console.log('CLEAN USERS:', JSON.stringify(cleanUsers, null, 2)); expect(cleanUsers).to.deep.include(TEST_USER_CLEAN); }); + it('should be able to find a user', async function () { + const user = await db.findUser(TEST_USER.username); + // eslint-disable-next-line no-unused-vars + const { password: _, ...TEST_USER_CLEAN } = TEST_USER; + // eslint-disable-next-line no-unused-vars + const { password: _2, _id: _3, ...DB_USER_CLEAN } = user; + + expect(DB_USER_CLEAN).to.eql(TEST_USER_CLEAN); + }); + + it('should be able to filter getUsers', async function () { + // uppercase the filter value to confirm db client is lowercasing inputs + const users = await db.getUsers({ username: TEST_USER.username.toUpperCase() }); + // eslint-disable-next-line no-unused-vars + const { password: _, ...TEST_USER_CLEAN } = TEST_USER; + const cleanUsers = cleanResponseData(TEST_USER_CLEAN, users); + expect(cleanUsers[0]).to.eql(TEST_USER_CLEAN); + + const users2 = await db.getUsers({ email: TEST_USER.email.toUpperCase() }); + const cleanUsers2 = cleanResponseData(TEST_USER_CLEAN, users2); + expect(cleanUsers2[0]).to.eql(TEST_USER_CLEAN); + }); + it('should be able to delete a user', async function () { await db.deleteUser(TEST_USER.username); const users = await db.getUsers(); @@ -108,6 +128,39 @@ describe('Database client', async () => { expect(cleanUsers).to.not.deep.include(TEST_USER); }); + it('should be able to update a user', async function () { + await db.createUser( + TEST_USER.username, + TEST_USER.password, + TEST_USER.email, + TEST_USER.gitAccount, + TEST_USER.admin, + ); + + // has less properties to prove that records are merged + const updateToApply = { + username: TEST_USER.username, + gitAccount: 'updatedGitAccount', + admin: false, + }; + + const updatedUser = { + // remove password as it will have been hashed + username: TEST_USER.username, + email: TEST_USER.email, + gitAccount: 'updatedGitAccount', + admin: false, + }; + await db.updateUser(updateToApply); + + const users = await db.getUsers(); + console.log('TEST USER:', JSON.stringify(TEST_USER, null, 2)); + console.log('USERS:', JSON.stringify(users, null, 2)); + const cleanUsers = cleanResponseData(updatedUser, users); + console.log('CLEAN USERS:', JSON.stringify(cleanUsers, null, 2)); + expect(cleanUsers).to.deep.include(updatedUser); + }); + it('should be able to create a push', async function () { await db.writeAudit(TEST_PUSH); const pushes = await db.getPushes(); @@ -122,5 +175,9 @@ describe('Database client', async () => { expect(cleanPushes).to.not.deep.include(TEST_PUSH); }); - after(async function () {}); + after(async function () { + await db.deleteRepo(TEST_REPO.name); + await db.deleteUser(TEST_USER.username); + await db.deletePush(TEST_PUSH.id); + }); }); From 4ff22a308f1aa88b3c5f53cea43e7478d2403c8b Mon Sep 17 00:00:00 2001 From: Kris West Date: Wed, 16 Apr 2025 17:54:57 +0100 Subject: [PATCH 23/25] test: more code coverage in DB and service/routes/repo --- src/db/file/pushes.ts | 11 ++ src/db/file/repo.ts | 25 +++- src/db/file/users.ts | 25 +++- test/addRepoTest.test.js | 12 ++ test/testDb.test.js | 252 +++++++++++++++++++++++++++++++++++++-- 5 files changed, 310 insertions(+), 15 deletions(-) diff --git a/src/db/file/pushes.ts b/src/db/file/pushes.ts index b58aea4eb..5399e6dfa 100644 --- a/src/db/file/pushes.ts +++ b/src/db/file/pushes.ts @@ -8,7 +8,10 @@ import { PushQuery } from '../types'; const COMPACTION_INTERVAL = 1000 * 60 * 60 * 24; // once per day +// these don't get coverage in tests as they have already been run once before the test +/* istanbul ignore if */ if (!fs.existsSync('./.data')) fs.mkdirSync('./.data'); +/* istanbul ignore if */ if (!fs.existsSync('./.data/db')) fs.mkdirSync('./.data/db'); const db = new Datastore({ filename: './.data/db/pushes.db', autoload: true }); @@ -26,6 +29,8 @@ export const getPushes = (query: PushQuery) => { if (!query) query = defaultPushQuery; return new Promise((resolve, reject) => { db.find(query, (err: Error, docs: Action[]) => { + // ignore for code coverage as neDB rarely returns errors even for an invalid query + /* istanbul ignore if */ if (err) { reject(err); } else { @@ -42,6 +47,8 @@ export const getPushes = (query: PushQuery) => { export const getPush = async (id: string) => { return new Promise((resolve, reject) => { db.findOne({ id: id }, (err, doc) => { + // ignore for code coverage as neDB rarely returns errors even for an invalid query + /* istanbul ignore if */ if (err) { reject(err); } else { @@ -58,6 +65,8 @@ export const getPush = async (id: string) => { export const deletePush = async (id: string) => { return new Promise((resolve, reject) => { db.remove({ id }, (err) => { + // ignore for code coverage as neDB rarely returns errors even for an invalid query + /* istanbul ignore if */ if (err) { reject(err); } else { @@ -71,6 +80,8 @@ export const writeAudit = async (action: Action) => { return new Promise((resolve, reject) => { const options = { multi: false, upsert: true }; db.update({ id: action.id }, action, options, (err) => { + // ignore for code coverage as neDB rarely returns errors even for an invalid query + /* istanbul ignore if */ if (err) { reject(err); } else { diff --git a/src/db/file/repo.ts b/src/db/file/repo.ts index 7773e5f0d..ac684c552 100644 --- a/src/db/file/repo.ts +++ b/src/db/file/repo.ts @@ -4,7 +4,10 @@ import { Repo } from '../types'; const COMPACTION_INTERVAL = 1000 * 60 * 60 * 24; // once per day +// these don't get coverage in tests as they have already been run once before the test +/* istanbul ignore if */ if (!fs.existsSync('./.data')) fs.mkdirSync('./.data'); +/* istanbul ignore if */ if (!fs.existsSync('./.data/db')) fs.mkdirSync('./.data/db'); const db = new Datastore({ filename: './.data/db/repos.db', autoload: true }); @@ -15,9 +18,14 @@ const isBlank = (str: string) => { return !str || /^\s*$/.test(str); }; -export const getRepos = async (query: any = {}) => { +export const getRepos = async (query: any = {}) => { + if (query?.name) { + query.name = query.name.toLowerCase(); + } return new Promise((resolve, reject) => { db.find(query, (err: Error, docs: Repo[]) => { + // ignore for code coverage as neDB rarely returns errors even for an invalid query + /* istanbul ignore if */ if (err) { reject(err); } else { @@ -28,9 +36,10 @@ export const getRepos = async (query: any = {}) => { }; export const getRepo = async (name: string) => { - name = name.toLowerCase(); return new Promise((resolve, reject) => { - db.findOne({ name }, (err: Error | null, doc: Repo) => { + db.findOne({ name: name.toLowerCase() }, (err: Error | null, doc: Repo) => { + // ignore for code coverage as neDB rarely returns errors even for an invalid query + /* istanbul ignore if */ if (err) { reject(err); } else { @@ -42,7 +51,6 @@ export const getRepo = async (name: string) => { export const createRepo = async (repo: Repo) => { repo.name = repo.name.toLowerCase(); - console.log(`creating new repo ${JSON.stringify(repo)}`); if (isBlank(repo.project)) { throw new Error('Project name cannot be empty'); @@ -61,10 +69,11 @@ export const createRepo = async (repo: Repo) => { return new Promise((resolve, reject) => { db.insert(repo, (err, doc) => { + // ignore for code coverage as neDB rarely returns errors even for an invalid query + /* istanbul ignore if */ if (err) { reject(err); } else { - console.log(`created new repo ${JSON.stringify(repo)}`); resolve(doc); } }); @@ -89,6 +98,8 @@ export const addUserCanPush = async (name: string, user: string) => { const options = { multi: false, upsert: false }; db.update({ name: name }, repo, options, (err) => { + // ignore for code coverage as neDB rarely returns errors even for an invalid query + /* istanbul ignore if */ if (err) { reject(err); } else { @@ -117,6 +128,8 @@ export const addUserCanAuthorise = async (name: string, user: string) => { const options = { multi: false, upsert: false }; db.update({ name: name }, repo, options, (err) => { + // ignore for code coverage as neDB rarely returns errors even for an invalid query + /* istanbul ignore if */ if (err) { reject(err); } else { @@ -176,6 +189,8 @@ export const deleteRepo = async (name: string) => { name = name.toLowerCase(); return new Promise((resolve, reject) => { db.remove({ name: name }, (err) => { + // ignore for code coverage as neDB rarely returns errors even for an invalid query + /* istanbul ignore if */ if (err) { reject(err); } else { diff --git a/src/db/file/users.ts b/src/db/file/users.ts index 46c6f30c3..263c612f4 100644 --- a/src/db/file/users.ts +++ b/src/db/file/users.ts @@ -4,7 +4,10 @@ import { User } from '../types'; const COMPACTION_INTERVAL = 1000 * 60 * 60 * 24; // once per day +// these don't get coverage in tests as they have already been run once before the test +/* istanbul ignore if */ if (!fs.existsSync('./.data')) fs.mkdirSync('./.data'); +/* istanbul ignore if */ if (!fs.existsSync('./.data/db')) fs.mkdirSync('./.data/db'); const db = new Datastore({ filename: './.data/db/users.db', autoload: true }); @@ -17,6 +20,8 @@ db.setAutocompactionInterval(COMPACTION_INTERVAL); export const findUser = (username: string) => { return new Promise((resolve, reject) => { db.findOne({ username: username.toLowerCase() }, (err: Error | null, doc: User) => { + // ignore for code coverage as neDB rarely returns errors even for an invalid query + /* istanbul ignore if */ if (err) { reject(err); } else { @@ -33,6 +38,8 @@ export const findUser = (username: string) => { export const findUserByOIDC = function (oidcId: string) { return new Promise((resolve, reject) => { db.findOne({ oidcId: oidcId }, (err, doc) => { + // ignore for code coverage as neDB rarely returns errors even for an invalid query + /* istanbul ignore if */ if (err) { reject(err); } else { @@ -51,6 +58,8 @@ export const createUser = function (user: User) { user.email = user.email.toLowerCase(); return new Promise((resolve, reject) => { db.insert(user, (err) => { + // ignore for code coverage as neDB rarely returns errors even for an invalid query + /* istanbul ignore if */ if (err) { reject(err); } else { @@ -63,6 +72,8 @@ export const createUser = function (user: User) { export const deleteUser = (username: string) => { return new Promise((resolve, reject) => { db.remove({ username: username.toLowerCase() }, (err) => { + // ignore for code coverage as neDB rarely returns errors even for an invalid query + /* istanbul ignore if */ if (err) { reject(err); } else { @@ -82,6 +93,8 @@ export const updateUser = (user: User) => { // hence, retrieve and merge documents to avoid dropping fields (such as the gitaccount) let existingUser; db.findOne({ username: user.username }, (err, doc) => { + // ignore for code coverage as neDB rarely returns errors even for an invalid query + /* istanbul ignore if */ if (err) { reject(err); } else { @@ -95,6 +108,8 @@ export const updateUser = (user: User) => { const options = { multi: false, upsert: true }; db.update({ username: user.username }, existingUser, options, (err) => { + // ignore for code coverage as neDB rarely returns errors even for an invalid query + /* istanbul ignore if */ if (err) { reject(err); } else { @@ -107,10 +122,16 @@ export const updateUser = (user: User) => { }; export const getUsers = (query: any = {}) => { - if (query.username) { query.username = query.username.toLowerCase(); } - if (query.email) { query.email = query.email.toLowerCase(); } + if (query.username) { + query.username = query.username.toLowerCase(); + } + if (query.email) { + query.email = query.email.toLowerCase(); + } return new Promise((resolve, reject) => { db.find(query, (err: Error, docs: User[]) => { + // ignore for code coverage as neDB rarely returns errors even for an invalid query + /* istanbul ignore if */ if (err) { reject(err); } else { diff --git a/test/addRepoTest.test.js b/test/addRepoTest.test.js index 6df809a66..172074101 100644 --- a/test/addRepoTest.test.js +++ b/test/addRepoTest.test.js @@ -56,6 +56,18 @@ describe('add new repo', async () => { repo.users.canAuthorise.length.should.equal(0); }); + it('filter repos', async function () { + const res = await chai + .request(app) + .get('/api/v1/repo') + .set('Cookie', `${cookie}`) + .query({ name: 'test-repo' }); + res.should.have.status(200); + res.body[0].project.should.equal('finos'); + res.body[0].name.should.equal('test-repo'); + res.body[0].url.should.equal('https://github.com/finos/test-repo.git'); + }); + it('add 1st can push user', async function () { const res = await chai .request(app) diff --git a/test/testDb.test.js b/test/testDb.test.js index 740ad9669..83e213e21 100644 --- a/test/testDb.test.js +++ b/test/testDb.test.js @@ -50,22 +50,33 @@ const TEST_PUSH = { * Clean up response data from the DB by removing an extraneous properties, * allowing comparison with expect. * @param {object} example Example element from which columns to retain are extracted - * @param {array} responses Array of responses to clean. + * @param {array | object} responses Array of responses to clean. * @return {array} Array of cleaned up responses. */ const cleanResponseData = (example, responses) => { const columns = Object.keys(example); - return responses.map((response) => { + + if (Array.isArray(responses)) { + return responses.map((response) => { + const cleanResponse = {}; + columns.forEach((col) => { + cleanResponse[col] = response[col]; + }); + return cleanResponse; + }); + } else if (typeof responses === 'object') { const cleanResponse = {}; columns.forEach((col) => { - cleanResponse[col] = response[col]; + cleanResponse[col] = responses[col]; }); return cleanResponse; - }); + } else { + throw new Error(`Can only clean arrays or objects, but a ${typeof responses} was passed`); + } }; // Use this test as a template -describe('Database client', async () => { +describe('Database clients', async () => { before(async function () {}); it('should be able to create a repo', async function () { @@ -75,13 +86,101 @@ describe('Database client', async () => { expect(cleanRepos).to.deep.include(TEST_REPO); }); + it('should be able to filter repos', async function () { + // uppercase the filter value to confirm db client is lowercasing inputs + const repos = await db.getRepos({ name: TEST_REPO.name.toUpperCase() }); + const cleanRepos = cleanResponseData(TEST_REPO, repos); + expect(cleanRepos[0]).to.eql(TEST_REPO); + + const repos2 = await db.getRepos({ url: TEST_REPO.url }); + const cleanRepos2 = cleanResponseData(TEST_REPO, repos2); + expect(cleanRepos2[0]).to.eql(TEST_REPO); + }); + + it('should be able to retrieve a repo by name', async function () { + // uppercase the filter value to confirm db client is lowercasing inputs + const repo = await db.getRepo(TEST_REPO.name); + const cleanRepo = cleanResponseData(TEST_REPO, repo); + expect(cleanRepo).to.eql(TEST_REPO); + }); + it('should be able to delete a repo', async function () { await db.deleteRepo(TEST_REPO.name); const repos = await db.getRepos(); + const cleanRepos = cleanResponseData(TEST_REPO, repos); expect(cleanRepos).to.not.deep.include(TEST_REPO); }); + it('should NOT be able to create a repo with blank project, name or url', async function () { + // test with a null value + let threwError = false; + let testRepo = { + project: null, + name: TEST_REPO.name, + url: TEST_REPO.url, + }; + try { + await db.createRepo(testRepo); + } catch (e) { + threwError = true; + } + expect(threwError).to.be.true; + + // test with an empty string + threwError = false; + testRepo = { + project: '', + name: TEST_REPO.name, + url: TEST_REPO.url, + }; + try { + await db.createRepo(testRepo); + } catch (e) { + threwError = true; + } + expect(threwError).to.be.true; + + // test with an undefined property + threwError = false; + testRepo = { + name: TEST_REPO.name, + url: TEST_REPO.url, + }; + try { + await db.createRepo(testRepo); + } catch (e) { + threwError = true; + } + expect(threwError).to.be.true; + + // repeat tests for other fields, but don't both with all variations as they go through same fn + threwError = false; + testRepo = { + project: TEST_REPO.project, + name: null, + url: TEST_REPO.url, + }; + try { + await db.createRepo(testRepo); + } catch (e) { + threwError = true; + } + expect(threwError).to.be.true; + + testRepo = { + project: TEST_REPO.project, + name: TEST_REPO.name, + url: null, + }; + try { + await db.createRepo(testRepo); + } catch (e) { + threwError = true; + } + expect(threwError).to.be.true; + }); + it('should be able to create a user', async function () { await db.createUser( TEST_USER.username, @@ -154,11 +253,148 @@ describe('Database client', async () => { await db.updateUser(updateToApply); const users = await db.getUsers(); - console.log('TEST USER:', JSON.stringify(TEST_USER, null, 2)); - console.log('USERS:', JSON.stringify(users, null, 2)); const cleanUsers = cleanResponseData(updatedUser, users); - console.log('CLEAN USERS:', JSON.stringify(cleanUsers, null, 2)); expect(cleanUsers).to.deep.include(updatedUser); + await db.deleteUser(TEST_USER.username); + }); + + it('should be able to create a user via updateUser', async function () { + await db.updateUser(TEST_USER); + + const users = await db.getUsers(); + // remove password as it will have been hashed + // eslint-disable-next-line no-unused-vars + const { password: _, ...TEST_USER_CLEAN } = TEST_USER; + const cleanUsers = cleanResponseData(TEST_USER_CLEAN, users); + expect(cleanUsers).to.deep.include(TEST_USER_CLEAN); + // leave user in place for next test(s) + }); + + it('should be able to authorise a user to push and confirm that they can', async function () { + let threwError = false; + try { + // first create the repo and check that user is not allowed to push + await db.createRepo(TEST_REPO); + let allowed = await db.isUserPushAllowed(TEST_REPO.name, TEST_USER.username); + expect(allowed).to.be.false; + + // uppercase the filter value to confirm db client is lowercasing inputs + await db.addUserCanPush(TEST_REPO.name.toUpperCase(), TEST_USER.username.toUpperCase()); + + // repeat, should not throw an error if already set + await db.addUserCanPush(TEST_REPO.name.toUpperCase(), TEST_USER.username.toUpperCase()); + + // confirm the setting exists + allowed = await db.isUserPushAllowed(TEST_REPO.name, TEST_USER.username); + expect(allowed).to.be.true; + + // confirm that casing doesn't matter + allowed = await db.isUserPushAllowed( + TEST_REPO.name.toUpperCase(), + TEST_USER.username.toUpperCase(), + ); + expect(allowed).to.be.true; + } catch (e) { + console.error('Error thrown ', e); + threwError = true; + } + expect(threwError).to.be.false; + }); + + it("should be able to de-authorise a user to push and confirm that they can't", async function () { + let threwError = false; + try { + // repo should already exist with user able to push after previous test + let allowed = await db.isUserPushAllowed(TEST_REPO.name, TEST_USER.username); + expect(allowed).to.be.true; + + // uppercase the filter value to confirm db client is lowercasing inputs + await db.removeUserCanPush(TEST_REPO.name.toUpperCase(), TEST_USER.username.toUpperCase()); + + // repeat, should not throw an error if already unset + await db.removeUserCanPush(TEST_REPO.name.toUpperCase(), TEST_USER.username.toUpperCase()); + + // confirm the setting exists + allowed = await db.isUserPushAllowed(TEST_REPO.name, TEST_USER.username); + expect(allowed).to.be.false; + + // confirm that casing doesn't matter + allowed = await db.isUserPushAllowed( + TEST_REPO.name.toUpperCase(), + TEST_USER.username.toUpperCase(), + ); + expect(allowed).to.be.false; + } catch (e) { + console.error('Error thrown ', e); + threwError = true; + } + expect(threwError).to.be.false; + }); + + it('should be able to authorise a user to authorise and confirm that they can', async function () { + let threwError = false; + try { + // repo should already exist after a previous test + let allowed = await db.canUserApproveRejectPushRepo(TEST_REPO.name, TEST_USER.username); + expect(allowed).to.be.false; + + // uppercase the filter value to confirm db client is lowercasing inputs + await db.addUserCanAuthorise(TEST_REPO.name.toUpperCase(), TEST_USER.username.toUpperCase()); + + // repeat, should not throw an error if already set + await db.addUserCanAuthorise(TEST_REPO.name.toUpperCase(), TEST_USER.username.toUpperCase()); + + // confirm the setting exists + allowed = await db.canUserApproveRejectPushRepo(TEST_REPO.name, TEST_USER.username); + expect(allowed).to.be.true; + + // confirm that casing doesn't matter + allowed = await db.canUserApproveRejectPushRepo( + TEST_REPO.name.toUpperCase(), + TEST_USER.username.toUpperCase(), + ); + expect(allowed).to.be.true; + } catch (e) { + console.error('Error thrown ', e); + threwError = true; + } + expect(threwError).to.be.false; + }); + + it("should be able to de-authorise a user to authorise and confirm that they can't", async function () { + let threwError = false; + try { + // repo should already exist after a previous test and user should be an authoriser + let allowed = await db.canUserApproveRejectPushRepo(TEST_REPO.name, TEST_USER.username); + expect(allowed).to.be.true; + + // uppercase the filter value to confirm db client is lowercasing inputs + await db.removeUserCanAuthorise( + TEST_REPO.name.toUpperCase(), + TEST_USER.username.toUpperCase(), + ); + + // repeat, should not throw an error if already set + await db.removeUserCanAuthorise( + TEST_REPO.name.toUpperCase(), + TEST_USER.username.toUpperCase(), + ); + + // confirm the setting was removed + allowed = await db.canUserApproveRejectPushRepo(TEST_REPO.name, TEST_USER.username); + expect(allowed).to.be.false; + + // confirm that casing doesn't matter + allowed = await db.canUserApproveRejectPushRepo( + TEST_REPO.name.toUpperCase(), + TEST_USER.username.toUpperCase(), + ); + expect(allowed).to.be.false; + } catch (e) { + console.error('Error thrown ', e); + threwError = true; + } + expect(threwError).to.be.false; }); it('should be able to create a push', async function () { From fe64a9151f964f34981e922fb9f32e25e7f6e595 Mon Sep 17 00:00:00 2001 From: Kris West Date: Thu, 17 Apr 2025 11:33:25 +0100 Subject: [PATCH 24/25] test: more DB test coverage --- src/db/file/pushes.ts | 6 +- src/db/file/repo.ts | 8 +- test/testDb.test.js | 202 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 211 insertions(+), 5 deletions(-) diff --git a/src/db/file/pushes.ts b/src/db/file/pushes.ts index 5399e6dfa..2d8c41efb 100644 --- a/src/db/file/pushes.ts +++ b/src/db/file/pushes.ts @@ -130,7 +130,7 @@ export const cancel = async (id: string) => { return { message: `cancel ${id}` }; }; -export const canUserCancelPush = async (id: string, user: any) => { +export const canUserCancelPush = async (id: string, user: string) => { return new Promise(async (resolve) => { const pushDetail = await getPush(id); if (!pushDetail) { @@ -149,14 +149,14 @@ export const canUserCancelPush = async (id: string, user: any) => { }); }; -export const canUserApproveRejectPush = async (id: string, user: any) => { +export const canUserApproveRejectPush = async (id: string, user: string) => { return new Promise(async (resolve) => { const action = await getPush(id); if (!action) { resolve(false); return; } - const repoName = action?.repoName.replace('.git', ''); + const repoName = action.repoName.replace('.git', ''); const isAllowed = await repo.canUserApproveRejectPushRepo(repoName, user); resolve(isAllowed); diff --git a/src/db/file/repo.ts b/src/db/file/repo.ts index ac684c552..fd7218c15 100644 --- a/src/db/file/repo.ts +++ b/src/db/file/repo.ts @@ -50,13 +50,13 @@ export const getRepo = async (name: string) => { }; export const createRepo = async (repo: Repo) => { - repo.name = repo.name.toLowerCase(); - if (isBlank(repo.project)) { throw new Error('Project name cannot be empty'); } if (isBlank(repo.name)) { throw new Error('Repository name cannot be empty'); + } else { + repo.name = repo.name.toLowerCase(); } if (isBlank(repo.url)) { throw new Error('URL cannot be empty'); @@ -153,6 +153,8 @@ export const removeUserCanAuthorise = async (name: string, user: string) => { const options = { multi: false, upsert: false }; db.update({ name: name }, repo, options, (err) => { + // ignore for code coverage as neDB rarely returns errors even for an invalid query + /* istanbul ignore if */ if (err) { reject(err); } else { @@ -176,6 +178,8 @@ export const removeUserCanPush = async (name: string, user: string) => { const options = { multi: false, upsert: false }; db.update({ name: name }, repo, options, (err) => { + // ignore for code coverage as neDB rarely returns errors even for an invalid query + /* istanbul ignore if */ if (err) { reject(err); } else { diff --git a/test/testDb.test.js b/test/testDb.test.js index 83e213e21..45f329e77 100644 --- a/test/testDb.test.js +++ b/test/testDb.test.js @@ -95,6 +95,11 @@ describe('Database clients', async () => { const repos2 = await db.getRepos({ url: TEST_REPO.url }); const cleanRepos2 = cleanResponseData(TEST_REPO, repos2); expect(cleanRepos2[0]).to.eql(TEST_REPO); + + // passing an empty query should produce same results as no query + const repos3 = await db.getRepos(); + const repos4 = await db.getRepos({}); + expect(repos3).to.have.same.deep.members(repos4); }); it('should be able to retrieve a repo by name', async function () { @@ -270,6 +275,17 @@ describe('Database clients', async () => { // leave user in place for next test(s) }); + it('should throw an error when authorising a user to push on non-existent repo', async function () { + let threwError = false; + try { + // uppercase the filter value to confirm db client is lowercasing inputs + await db.addUserCanPush('non-existent-repo', TEST_USER.username); + } catch (e) { + threwError = true; + } + expect(threwError).to.be.true; + }); + it('should be able to authorise a user to push and confirm that they can', async function () { let threwError = false; try { @@ -301,6 +317,16 @@ describe('Database clients', async () => { expect(threwError).to.be.false; }); + it('should throw an error when de-authorising a user to push on non-existent repo', async function () { + let threwError = false; + try { + await db.removeUserCanPush('non-existent-repo', TEST_USER.username); + } catch (e) { + threwError = true; + } + expect(threwError).to.be.true; + }); + it("should be able to de-authorise a user to push and confirm that they can't", async function () { let threwError = false; try { @@ -331,6 +357,16 @@ describe('Database clients', async () => { expect(threwError).to.be.false; }); + it('should throw an error when authorising a user to authorise on non-existent repo', async function () { + let threwError = false; + try { + await db.addUserCanAuthorise('non-existent-repo', TEST_USER.username); + } catch (e) { + threwError = true; + } + expect(threwError).to.be.true; + }); + it('should be able to authorise a user to authorise and confirm that they can', async function () { let threwError = false; try { @@ -361,6 +397,17 @@ describe('Database clients', async () => { expect(threwError).to.be.false; }); + it('should throw an error when de-authorising a user to push on non-existent repo', async function () { + let threwError = false; + try { + // uppercase the filter value to confirm db client is lowercasing inputs + await db.removeUserCanAuthorise('non-existent-repo', TEST_USER.username); + } catch (e) { + threwError = true; + } + expect(threwError).to.be.true; + }); + it("should be able to de-authorise a user to authorise and confirm that they can't", async function () { let threwError = false; try { @@ -397,6 +444,33 @@ describe('Database clients', async () => { expect(threwError).to.be.false; }); + it('should NOT throw an error when checking whether a user can push on non-existent repo', async function () { + let threwError = false; + try { + // uppercase the filter value to confirm db client is lowercasing inputs + const allowed = await db.isUserPushAllowed('non-existent-repo', TEST_USER.username); + expect(allowed).to.be.false; + } catch (e) { + threwError = true; + } + expect(threwError).to.be.false; + }); + + it('should NOT throw an error when checking whether a user can authorise on non-existent repo', async function () { + let threwError = false; + try { + // uppercase the filter value to confirm db client is lowercasing inputs + const allowed = await db.canUserApproveRejectPushRepo( + 'non-existent-repo', + TEST_USER.username, + ); + expect(allowed).to.be.false; + } catch (e) { + threwError = true; + } + expect(threwError).to.be.false; + }); + it('should be able to create a push', async function () { await db.writeAudit(TEST_PUSH); const pushes = await db.getPushes(); @@ -411,6 +485,134 @@ describe('Database clients', async () => { expect(cleanPushes).to.not.deep.include(TEST_PUSH); }); + it('should be able to authorise a push', async function () { + // first create the push + await db.writeAudit(TEST_PUSH); + let threwError = false; + try { + const msg = await db.authorise(TEST_PUSH.id); + expect(msg).to.have.property('message'); + } catch (e) { + console.error('Error: ', e); + threwError = true; + } + expect(threwError).to.be.false; + // clean up + await db.deletePush(TEST_PUSH.id); + }); + + it('should throw an error when authorising a non-existent a push', async function () { + let threwError = false; + try { + await db.authorise(TEST_PUSH.id); + } catch (e) { + threwError = true; + } + expect(threwError).to.be.true; + }); + + it('should be able to reject a push', async function () { + // first create the push + await db.writeAudit(TEST_PUSH); + let threwError = false; + try { + const msg = await db.reject(TEST_PUSH.id); + expect(msg).to.have.property('message'); + } catch (e) { + threwError = true; + } + expect(threwError).to.be.false; + // clean up + await db.deletePush(TEST_PUSH.id); + }); + + it('should throw an error when rejecting a non-existent a push', async function () { + let threwError = false; + try { + await db.reject(TEST_PUSH.id); + } catch (e) { + threwError = true; + } + expect(threwError).to.be.true; + }); + + it('should be able to cancel a push', async function () { + // first create the push + await db.writeAudit(TEST_PUSH); + let threwError = false; + try { + const msg = await db.cancel(TEST_PUSH.id); + expect(msg).to.have.property('message'); + } catch (e) { + threwError = true; + } + expect(threwError).to.be.false; + // clean up + await db.deletePush(TEST_PUSH.id); + }); + + it('should throw an error when cancelling a non-existent a push', async function () { + let threwError = false; + try { + await db.cancel(TEST_PUSH.id); + } catch (e) { + threwError = true; + } + expect(threwError).to.be.true; + }); + + it('should be able to check if a user can cancel push', async function () { + let threwError = false; + const repoName = TEST_PUSH.repoName.replace('.git', ''); + try { + // push does not exist yet, should return false + let allowed = await db.canUserCancelPush(TEST_PUSH.id, TEST_USER.username); + expect(allowed).to.be.false; + + // create the push - user should already exist and not authorised to push + await db.writeAudit(TEST_PUSH); + allowed = await db.canUserCancelPush(TEST_PUSH.id, TEST_USER.username); + expect(allowed).to.be.false; + + // authorise user and recheck + await db.addUserCanPush(repoName, TEST_USER.username); + allowed = await db.canUserCancelPush(TEST_PUSH.id, TEST_USER.username); + expect(allowed).to.be.true; + } catch (e) { + threwError = true; + } + expect(threwError).to.be.false; + // clean up + await db.deletePush(TEST_PUSH.id); + await db.removeUserCanPush(repoName, TEST_USER.username); + }); + + it('should be able to check if a user can approve/reject push', async function () { + let threwError = false; + const repoName = TEST_PUSH.repoName.replace('.git', ''); + try { + // push does not exist yet, should return false + let allowed = await db.canUserApproveRejectPush(TEST_PUSH.id, TEST_USER.username); + expect(allowed).to.be.false; + + // create the push - user should already exist and not authorised to push + await db.writeAudit(TEST_PUSH); + allowed = await db.canUserApproveRejectPush(TEST_PUSH.id, TEST_USER.username); + expect(allowed).to.be.false; + + // authorise user and recheck + await db.addUserCanAuthorise(repoName, TEST_USER.username); + allowed = await db.canUserApproveRejectPush(TEST_PUSH.id, TEST_USER.username); + expect(allowed).to.be.true; + } catch (e) { + threwError = true; + } + expect(threwError).to.be.false; + // clean up + await db.deletePush(TEST_PUSH.id); + await db.removeUserCanAuthorise(repoName, TEST_USER.username); + }); + after(async function () { await db.deleteRepo(TEST_REPO.name); await db.deleteUser(TEST_USER.username); From b5cd0383839d5ea010e6ce53e337d47f4391cc96 Mon Sep 17 00:00:00 2001 From: kriswest Date: Wed, 7 May 2025 14:12:42 +0100 Subject: [PATCH 25/25] chore: extended prettier format command to package/git-proxy-cli and ran it --- package.json | 2 +- packages/git-proxy-cli/index.js | 3 +- .../test/testCli.proxy.config.json | 7 +- src/config/env.ts | 4 +- src/db/mongo/helper.ts | 4 +- src/db/types.ts | 6 +- src/plugin.ts | 59 +++++++------ src/proxy/actions/Action.ts | 10 +-- src/proxy/actions/Step.ts | 8 +- src/proxy/actions/autoActions.ts | 5 +- src/proxy/actions/index.ts | 5 +- src/proxy/chain.ts | 8 +- .../processors/pre-processor/parseAction.ts | 6 +- .../push-action/checkAuthorEmails.ts | 6 +- .../push-action/checkCommitMessages.ts | 2 +- .../push-action/checkUserPushPermission.ts | 4 +- src/proxy/processors/push-action/parsePush.ts | 18 ++-- .../processors/push-action/preReceive.ts | 2 +- .../processors/push-action/pullRemote.ts | 23 +++-- src/proxy/processors/push-action/scanDiff.ts | 84 +++++++++---------- src/proxy/processors/types.ts | 4 +- src/proxy/routes/index.ts | 7 +- src/service/passport/oidc.js | 17 ++-- src/service/routes/auth.js | 3 +- src/ui/components/Filtering/Filtering.css | 2 +- src/ui/components/Filtering/Filtering.jsx | 27 +++--- src/ui/components/Pagination/Pagination.jsx | 3 +- src/ui/components/Search/Search.css | 8 +- src/ui/components/Search/Search.jsx | 21 ++--- .../RepoList/Components/Repositories.jsx | 51 +++++------ src/ui/views/UserList/Components/UserList.jsx | 33 ++++---- test/fixtures/baz.js | 2 +- test/fixtures/proxy.config.invalid-1.json | 2 +- test/fixtures/proxy.config.invalid-2.json | 2 +- test/fixtures/proxy.config.valid-1.json | 2 +- test/fixtures/proxy.config.valid-2.json | 2 +- test/fixtures/test-package/multiple-export.js | 3 +- test/plugin/plugin.test.js | 18 ++-- test/scanDiff.test.js | 44 +++++----- test/testClearBareClone.test.js | 2 +- test/testRouteFilter.js | 11 +-- 41 files changed, 268 insertions(+), 262 deletions(-) diff --git a/package.json b/package.json index 2db0042be..e0567b623 100644 --- a/package.json +++ b/package.json @@ -16,7 +16,7 @@ "prepare": "node ./scripts/prepare.js", "lint": "eslint \"src/**/*.{js,jsx,ts,tsx,json}\" \"test/**/*.{js,jsx,ts,tsx,json}\"", "lint:fix": "eslint --fix \"src/**/*.{js,jsx,ts,tsx,json}\" \"test/**/*.{js,jsx,ts,tsx,json}\"", - "format": "prettier --write src/**/*.{js,jsx,ts,tsx,css,md,json,scss} test/**/*.{js,jsx,ts,tsx,json} --config ./.prettierrc", + "format": "prettier --write src/**/*.{js,jsx,ts,tsx,css,md,json,scss} test/**/*.{js,jsx,ts,tsx,json} packages/git-proxy-cli/test/**/*.{js,jsx,ts,tsx,json} packages/git-proxy-cli/index.js --config ./.prettierrc", "gen-schema-doc": "node ./scripts/doc-schema.js", "cypress:run": "cypress run" }, diff --git a/packages/git-proxy-cli/index.js b/packages/git-proxy-cli/index.js index b0090a4bf..35b488a07 100755 --- a/packages/git-proxy-cli/index.js +++ b/packages/git-proxy-cli/index.js @@ -7,7 +7,8 @@ const util = require('util'); const GIT_PROXY_COOKIE_FILE = 'git-proxy-cookie'; // GitProxy UI HOST and PORT (configurable via environment variable) -const { GIT_PROXY_UI_HOST: uiHost = 'http://localhost', GIT_PROXY_UI_PORT: uiPort = 8080 } = process.env; +const { GIT_PROXY_UI_HOST: uiHost = 'http://localhost', GIT_PROXY_UI_PORT: uiPort = 8080 } = + process.env; const baseUrl = `${uiHost}:${uiPort}`; diff --git a/packages/git-proxy-cli/test/testCli.proxy.config.json b/packages/git-proxy-cli/test/testCli.proxy.config.json index 48073ef57..95b3266ec 100644 --- a/packages/git-proxy-cli/test/testCli.proxy.config.json +++ b/packages/git-proxy-cli/test/testCli.proxy.config.json @@ -1,8 +1,7 @@ { "tempPassword": { "sendEmail": false, - "emailConfig": { - } + "emailConfig": {} }, "authorisedList": [ { @@ -22,9 +21,9 @@ { "type": "mongo", "connectionString": "mongodb://localhost:27017/gitproxy", - "options": { + "options": { "useUnifiedTopology": true - }, + }, "enabled": false } ], diff --git a/src/config/env.ts b/src/config/env.ts index 85b8475b5..9bb0bad55 100644 --- a/src/config/env.ts +++ b/src/config/env.ts @@ -9,12 +9,12 @@ const { GIT_PROXY_SERVER_PORT = 8000, GIT_PROXY_HTTPS_SERVER_PORT = 8443, GIT_PROXY_UI_HOST = 'http://localhost', - GIT_PROXY_UI_PORT = 8080 + GIT_PROXY_UI_PORT = 8080, } = process.env; export const serverConfig: ServerConfig = { GIT_PROXY_SERVER_PORT, GIT_PROXY_HTTPS_SERVER_PORT, GIT_PROXY_UI_HOST, - GIT_PROXY_UI_PORT + GIT_PROXY_UI_PORT, }; diff --git a/src/db/mongo/helper.ts b/src/db/mongo/helper.ts index 335434ebb..c4956de0f 100644 --- a/src/db/mongo/helper.ts +++ b/src/db/mongo/helper.ts @@ -25,7 +25,7 @@ export const connect = async (collectionName: string): Promise => { export const findDocuments = async ( collectionName: string, filter: Filter = {}, - options: FindOptions = {} + options: FindOptions = {}, ): Promise => { const collection = await connect(collectionName); return collection.find(filter, options).toArray() as Promise; @@ -34,7 +34,7 @@ export const findDocuments = async ( export const findOneDocument = async ( collectionName: string, filter: Filter = {}, - options: FindOptions = {} + options: FindOptions = {}, ): Promise => { const collection = await connect(collectionName); return (await collection.findOne(filter, options)) as T | null; diff --git a/src/db/types.ts b/src/db/types.ts index dba9bdf3a..04951a699 100644 --- a/src/db/types.ts +++ b/src/db/types.ts @@ -1,8 +1,8 @@ export type PushQuery = { error: boolean; - blocked: boolean, - allowPush: boolean, - authorised: boolean + blocked: boolean; + allowPush: boolean; + authorised: boolean; }; export type UserRole = 'canPush' | 'canAuthorise'; diff --git a/src/plugin.ts b/src/plugin.ts index f2bd8f26a..92fb9a99c 100644 --- a/src/plugin.ts +++ b/src/plugin.ts @@ -15,9 +15,11 @@ function isCompatiblePlugin(obj: any, propertyName: string = 'isGitProxyPlugin') // valid plugin objects will have the appropriate property set to true // if the prototype chain is exhausted, return false while (obj != null) { - if (Object.prototype.hasOwnProperty.call(obj, propertyName) && + if ( + Object.prototype.hasOwnProperty.call(obj, propertyName) && obj.isGitProxyPlugin && - Object.keys(obj).includes('exec')) { + Object.keys(obj).includes('exec') + ) { return true; } obj = Object.getPrototypeOf(obj); @@ -55,25 +57,28 @@ class PluginLoader { */ async load(): Promise { try { - const modulePromises = this.targets.map(target => - this._loadPluginModule(target).catch(error => { + const modulePromises = this.targets.map((target) => + this._loadPluginModule(target).catch((error) => { console.error(`Failed to load plugin: ${error}`); // TODO: log.error() return Promise.reject(error); // Or return an error object to handle it later - }) + }), ); const moduleResults = await Promise.allSettled(modulePromises); const loadedModules = moduleResults - .filter((result): result is PromiseFulfilledResult => result.status === 'fulfilled' && result.value !== null) - .map(result => result.value); + .filter( + (result): result is PromiseFulfilledResult => + result.status === 'fulfilled' && result.value !== null, + ) + .map((result) => result.value); console.log(`Found ${loadedModules.length} plugin modules`); // TODO: log.debug() - const pluginTypeResultPromises = loadedModules.map(mod => - this._getPluginObjects(mod).catch(error => { + const pluginTypeResultPromises = loadedModules.map((mod) => + this._getPluginObjects(mod).catch((error) => { console.error(`Failed to cast plugin objects: ${error}`); // TODO: log.error() return Promise.reject(error); // Or return an error object to handle it later - }) + }), ); const settledPluginTypeResults = await Promise.allSettled(pluginTypeResultPromises); @@ -81,16 +86,19 @@ class PluginLoader { * @type {PluginTypeResult[]} List of resolved PluginTypeResult objects */ const pluginTypeResults = settledPluginTypeResults - .filter((result): result is PromiseFulfilledResult => result.status === 'fulfilled' && result.value !== null) - .map(result => result.value); + .filter( + (result): result is PromiseFulfilledResult => + result.status === 'fulfilled' && result.value !== null, + ) + .map((result) => result.value); for (const result of pluginTypeResults) { - this.pushPlugins.push(...result.pushAction) - this.pullPlugins.push(...result.pullAction) + this.pushPlugins.push(...result.pushAction); + this.pullPlugins.push(...result.pullAction); } const combinedPlugins = [...this.pushPlugins, ...this.pullPlugins]; - combinedPlugins.forEach(plugin => { + combinedPlugins.forEach((plugin) => { console.log(`Loaded plugin: ${plugin.constructor.name}`); }); } catch (error) { @@ -128,7 +136,9 @@ class PluginLoader { console.log('found pull plugin', potentialModule.constructor.name); plugins.pullAction.push(potentialModule); } else { - console.error(`Error: Object ${potentialModule.constructor.name} does not seem to be a compatible plugin type`); + console.error( + `Error: Object ${potentialModule.constructor.name} does not seem to be a compatible plugin type`, + ); } } @@ -136,7 +146,7 @@ class PluginLoader { // `module.exports = new ProxyPlugin()` in CJS or `exports default new ProxyPlugin()` in ESM // the "module" is a single object that could be a plugin if (isCompatiblePlugin(pluginModule)) { - handlePlugin(pluginModule) + handlePlugin(pluginModule); } else { // handle the typical case of a module which exports multiple objects // module.exports = { x, y } (CJS) or multiple `export ...` statements (ESM) @@ -173,11 +183,11 @@ class PushActionPlugin extends ProxyPlugin { * Wrapper class which contains at least one function executed as part of the action chain for git push operations. * The function must be called `exec` and take in two parameters: an Express Request (req) and the current Action * executed in the chain (action). This function should return a Promise that resolves to an Action. - * + * * Optionally, child classes which extend this can simply define the `exec` function as their own property. * This is the preferred implementation when a custom plugin (subclass) has its own state or additional methods * that are required. - * + * * @param {function} exec - A function that: * - Takes in an Express Request object as the first parameter (`req`). * - Takes in an Action object as the second parameter (`action`). @@ -201,11 +211,11 @@ class PullActionPlugin extends ProxyPlugin { * Wrapper class which contains at least one function executed as part of the action chain for git pull operations. * The function must be called `exec` and take in two parameters: an Express Request (req) and the current Action * executed in the chain (action). This function should return a Promise that resolves to an Action. - * + * * Optionally, child classes which extend this can simply define the `exec` function as their own property. * This is the preferred implementation when a custom plugin (subclass) has its own state or additional methods * that are required. - * + * * @param {function} exec - A function that: * - Takes in an Express Request object as the first parameter (`req`). * - Takes in an Action object as the second parameter (`action`). @@ -218,9 +228,4 @@ class PullActionPlugin extends ProxyPlugin { } } -export { - PluginLoader, - PushActionPlugin, - PullActionPlugin, - isCompatiblePlugin, -} +export { PluginLoader, PushActionPlugin, PullActionPlugin, isCompatiblePlugin }; diff --git a/src/proxy/actions/Action.ts b/src/proxy/actions/Action.ts index 78dbc2ef0..b15b7c24c 100644 --- a/src/proxy/actions/Action.ts +++ b/src/proxy/actions/Action.ts @@ -1,5 +1,5 @@ -import { getProxyUrl } from "../../config"; -import { Step } from "./Step"; +import { getProxyUrl } from '../../config'; +import { Step } from './Step'; /** * Represents a commit. @@ -62,15 +62,15 @@ class Action { this.type = type; this.method = method; this.timestamp = timestamp; - this.project = repo.split("/")[0]; - this.repoName = repo.split("/")[1]; + this.project = repo.split('/')[0]; + this.repoName = repo.split('/')[1]; this.url = `${getProxyUrl()}/${repo}`; this.repo = repo; } /** * Add a step to the action. - * @param {Step} step + * @param {Step} step */ addStep(step: Step): void { this.steps.push(step); diff --git a/src/proxy/actions/Step.ts b/src/proxy/actions/Step.ts index cdb07bf95..6eb114e9c 100644 --- a/src/proxy/actions/Step.ts +++ b/src/proxy/actions/Step.ts @@ -1,4 +1,4 @@ -import { v4 as uuidv4 } from "uuid"; +import { v4 as uuidv4 } from 'uuid'; /** Class representing a Push Step. */ class Step { @@ -17,7 +17,7 @@ class Step { errorMessage: string | null = null, blocked: boolean = false, blockedMessage: string | null = null, - content: any = null + content: any = null, ) { this.id = uuidv4(); this.stepName = stepName; @@ -35,12 +35,12 @@ class Step { } setContent(content: any): void { - this.log("setting content"); + this.log('setting content'); this.content = content; } setAsyncBlock(message: string): void { - this.log("setting blocked"); + this.log('setting blocked'); this.blocked = true; this.blockedMessage = message; } diff --git a/src/proxy/actions/autoActions.ts b/src/proxy/actions/autoActions.ts index 03ed0529a..450c97d80 100644 --- a/src/proxy/actions/autoActions.ts +++ b/src/proxy/actions/autoActions.ts @@ -33,7 +33,4 @@ const attemptAutoRejection = async (action: Action) => { } }; -export { - attemptAutoApproval, - attemptAutoRejection, -}; +export { attemptAutoApproval, attemptAutoRejection }; diff --git a/src/proxy/actions/index.ts b/src/proxy/actions/index.ts index 72aa2918a..13f35276c 100644 --- a/src/proxy/actions/index.ts +++ b/src/proxy/actions/index.ts @@ -1,7 +1,4 @@ import { Action } from './Action'; import { Step } from './Step'; -export { - Action, - Step -} +export { Action, Step }; diff --git a/src/proxy/chain.ts b/src/proxy/chain.ts index 41a7cc495..5344df797 100644 --- a/src/proxy/chain.ts +++ b/src/proxy/chain.ts @@ -19,7 +19,9 @@ const pushActionChain: ((req: any, action: Action) => Promise)[] = [ proc.push.blockForAuth, ]; -const pullActionChain: ((req: any, action: Action) => Promise)[] = [proc.push.checkRepoInAuthorisedList]; +const pullActionChain: ((req: any, action: Action) => Promise)[] = [ + proc.push.checkRepoInAuthorisedList, +]; let pluginsInserted = false; @@ -57,7 +59,9 @@ export const executeChain = async (req: any, res: any): Promise => { */ let chainPluginLoader: PluginLoader; -const getChain = async (action: Action): Promise<((req: any, action: Action) => Promise)[]> => { +const getChain = async ( + action: Action, +): Promise<((req: any, action: Action) => Promise)[]> => { if (chainPluginLoader === undefined) { console.error( 'Plugin loader was not initialized! This is an application error. Please report it to the GitProxy maintainers. Skipping plugins...', diff --git a/src/proxy/processors/pre-processor/parseAction.ts b/src/proxy/processors/pre-processor/parseAction.ts index ed610d9d1..a9c332fdc 100644 --- a/src/proxy/processors/pre-processor/parseAction.ts +++ b/src/proxy/processors/pre-processor/parseAction.ts @@ -1,6 +1,10 @@ import { Action } from '../../actions'; -const exec = async (req: { originalUrl: string; method: string; headers: Record }) => { +const exec = async (req: { + originalUrl: string; + method: string; + headers: Record; +}) => { const id = Date.now(); const timestamp = id; const repoName = getRepoNameFromUrl(req.originalUrl); diff --git a/src/proxy/processors/push-action/checkAuthorEmails.ts b/src/proxy/processors/push-action/checkAuthorEmails.ts index a25a0e6c9..367514e16 100644 --- a/src/proxy/processors/push-action/checkAuthorEmails.ts +++ b/src/proxy/processors/push-action/checkAuthorEmails.ts @@ -27,14 +27,16 @@ const isEmailAllowed = (email: string): boolean => { } return true; -} +}; const exec = async (req: any, action: Action): Promise => { console.log({ req, action }); const step = new Step('checkAuthorEmails'); - const uniqueAuthorEmails = [...new Set(action.commitData?.map((commit: Commit) => commit.authorEmail))]; + const uniqueAuthorEmails = [ + ...new Set(action.commitData?.map((commit: Commit) => commit.authorEmail)), + ]; console.log({ uniqueAuthorEmails }); const illegalEmails = uniqueAuthorEmails.filter((email) => !isEmailAllowed(email)); diff --git a/src/proxy/processors/push-action/checkCommitMessages.ts b/src/proxy/processors/push-action/checkCommitMessages.ts index 577a572af..7b5db82db 100644 --- a/src/proxy/processors/push-action/checkCommitMessages.ts +++ b/src/proxy/processors/push-action/checkCommitMessages.ts @@ -47,7 +47,7 @@ const isMessageAllowed = (commitMessage: string): boolean => { } return true; -} +}; // Execute if the repo is approved const exec = async (req: any, action: Action): Promise => { diff --git a/src/proxy/processors/push-action/checkUserPushPermission.ts b/src/proxy/processors/push-action/checkUserPushPermission.ts index c712693e5..5ed660b44 100644 --- a/src/proxy/processors/push-action/checkUserPushPermission.ts +++ b/src/proxy/processors/push-action/checkUserPushPermission.ts @@ -30,8 +30,8 @@ const exec = async (req: any, action: Action): Promise => { step.setError( `Rejecting push as user ${action.user} ` + - `is not allowed to push on repo ` + - `${action.repo}`, + `is not allowed to push on repo ` + + `${action.repo}`, ); action.addStep(step); return action; diff --git a/src/proxy/processors/push-action/parsePush.ts b/src/proxy/processors/push-action/parsePush.ts index 42adcb48e..4fdf60215 100644 --- a/src/proxy/processors/push-action/parsePush.ts +++ b/src/proxy/processors/push-action/parsePush.ts @@ -50,7 +50,7 @@ async function exec(req: any, action: Action): Promise { action.addStep(step); } return action; -}; +} const getCommitData = (contents: CommitContent[]) => { console.log({ contents }); @@ -120,7 +120,15 @@ const getCommitData = (contents: CommitContent[]) => { authorEmail, }); - if (!tree || !parent || !author || !committer || !commitTimestamp || !message || !authorEmail) { + if ( + !tree || + !parent || + !author || + !committer || + !commitTimestamp || + !message || + !authorEmail + ) { throw new Error('Invalid commit data'); } @@ -256,8 +264,4 @@ const unpack = (buf: Buffer) => { exec.displayName = 'parsePush.exec'; -export { - exec, - getPackMeta, - unpack -}; +export { exec, getPackMeta, unpack }; diff --git a/src/proxy/processors/push-action/preReceive.ts b/src/proxy/processors/push-action/preReceive.ts index 1a8b23d89..1c3ad36b9 100644 --- a/src/proxy/processors/push-action/preReceive.ts +++ b/src/proxy/processors/push-action/preReceive.ts @@ -10,7 +10,7 @@ const sanitizeInput = (_req: any, action: Action): string => { const exec = async ( req: any, action: Action, - hookFilePath: string = './hooks/pre-receive.sh' + hookFilePath: string = './hooks/pre-receive.sh', ): Promise => { const step = new Step('executeExternalPreReceiveHook'); let stderrTrimmed = ''; diff --git a/src/proxy/processors/push-action/pullRemote.ts b/src/proxy/processors/push-action/pullRemote.ts index c7559643f..2f7c808a2 100644 --- a/src/proxy/processors/push-action/pullRemote.ts +++ b/src/proxy/processors/push-action/pullRemote.ts @@ -1,5 +1,5 @@ import { Action, Step } from '../../actions'; -import fs from 'fs' +import fs from 'fs'; import git from 'isomorphic-git'; import gitHttpClient from 'isomorphic-git/http/node'; @@ -29,17 +29,16 @@ const exec = async (req: any, action: Action): Promise => { .toString() .split(':'); - await git - .clone({ - fs, - http: gitHttpClient, - url: action.url, - onAuth: () => ({ - username, - password, - }), - dir: `${action.proxyGitPath}/${action.repoName}`, - }); + await git.clone({ + fs, + http: gitHttpClient, + url: action.url, + onAuth: () => ({ + username, + password, + }), + dir: `${action.proxyGitPath}/${action.repoName}`, + }); console.log('Clone Success: ', action.url); diff --git a/src/proxy/processors/push-action/scanDiff.ts b/src/proxy/processors/push-action/scanDiff.ts index 296c8b404..40b39627b 100644 --- a/src/proxy/processors/push-action/scanDiff.ts +++ b/src/proxy/processors/push-action/scanDiff.ts @@ -8,13 +8,13 @@ const privateOrganizations = getPrivateOrganizations(); const BLOCK_TYPE = { LITERAL: 'Offending Literal', PATTERN: 'Offending Pattern', - PROVIDER: 'PROVIDER' -} + PROVIDER: 'PROVIDER', +}; type CombinedMatch = { type: string; match: RegExp; -} +}; type RawMatch = { type: string; @@ -22,7 +22,7 @@ type RawMatch = { file?: string; lines: number[]; content: string; -} +}; type Match = { type: string; @@ -30,7 +30,7 @@ type Match = { file?: string; lines: string; content: string; -} +}; const getDiffViolations = (diff: string, organization: string): Match[] | string | null => { // Commit diff is empty, i.e. '', null or undefined @@ -53,7 +53,7 @@ const getDiffViolations = (diff: string, organization: string): Match[] | string if (res.length > 0) { console.log('Diff is blocked via configured literals/patterns/providers...'); // combining matches with file and line number - return res + return res; } return null; @@ -67,66 +67,64 @@ const combineMatches = (organization: string) => { const blockedPatterns: string[] = commitConfig.diff.block.patterns; // Configured blocked providers - const blockedProviders: [string, string][] = organization && privateOrganizations.includes(organization) ? [] : - Object.entries(commitConfig.diff.block.providers); + const blockedProviders: [string, string][] = + organization && privateOrganizations.includes(organization) + ? [] + : Object.entries(commitConfig.diff.block.providers); // Combine all matches (literals, paterns) const combinedMatches = [ - ...blockedLiterals.map(literal => ({ + ...blockedLiterals.map((literal) => ({ type: BLOCK_TYPE.LITERAL, - match: new RegExp(literal, 'gi') + match: new RegExp(literal, 'gi'), })), - ...blockedPatterns.map(pattern => ({ + ...blockedPatterns.map((pattern) => ({ type: BLOCK_TYPE.PATTERN, - match: new RegExp(pattern, 'gi') + match: new RegExp(pattern, 'gi'), })), ...blockedProviders.map(([key, value]) => ({ type: key, - match: new RegExp(value, 'gi') + match: new RegExp(value, 'gi'), })), ]; return combinedMatches; -} +}; -const collectMatches = ( - parsedDiff: File[], - combinedMatches: CombinedMatch[] -): Match[] => { +const collectMatches = (parsedDiff: File[], combinedMatches: CombinedMatch[]): Match[] => { const allMatches: Record = {}; - parsedDiff.forEach(file => { + parsedDiff.forEach((file) => { const fileName = file.to || file.from; - console.log("CHANGE", file.chunks) + console.log('CHANGE', file.chunks); - file.chunks.forEach(chunk => { - chunk.changes.forEach(change => { - console.log("CHANGE", change) + file.chunks.forEach((chunk) => { + chunk.changes.forEach((change) => { + console.log('CHANGE', change); if (change.type === 'add') { // store line number const lineNumber = change.ln; // Iterate through each match types - literal, patterns, providers combinedMatches.forEach(({ type, match }) => { // using Match all to find all occurences of the pattern in the line - const matches = [...change.content.matchAll(match)] + const matches = [...change.content.matchAll(match)]; - matches.forEach(matchInstance => { + matches.forEach((matchInstance) => { const matchLiteral = matchInstance[0]; const matchKey = `${type}_${matchLiteral}_${fileName}`; // unique key - if (!allMatches[matchKey]) { - // match entry + // match entry allMatches[matchKey] = { type, literal: matchLiteral, file: fileName, lines: [], - content: change.content.trim() + content: change.content.trim(), }; } // apend line numbers to the list of lines - allMatches[matchKey].lines.push(lineNumber) - }) + allMatches[matchKey].lines.push(lineNumber); + }); }); } }); @@ -134,13 +132,13 @@ const collectMatches = ( }); // convert matches into a final result array, joining line numbers - const result = Object.values(allMatches).map(match => ({ + const result = Object.values(allMatches).map((match) => ({ ...match, - lines: match.lines.join(',') // join the line numbers into a comma-separated string - })) + lines: match.lines.join(','), // join the line numbers into a comma-separated string + })); return result; -} +}; const formatMatches = (matches: Match[]) => { return matches.map((match, index) => { @@ -148,9 +146,9 @@ const formatMatches = (matches: Match[]) => { Policy Exception Type: ${match.type} DETECTED: ${match.literal} FILE(S) LOCATED: ${match.file} - Line(s) of code: ${match.lines}` + Line(s) of code: ${match.lines}`; }); -} +}; const exec = async (req: any, action: Action): Promise => { const step = new Step('scanDiff'); @@ -160,24 +158,24 @@ const exec = async (req: any, action: Action): Promise => { const diff = steps.find((s) => s.stepName === 'diff')?.content; - console.log(diff) + console.log(diff); const diffViolations = getDiffViolations(diff, action.project); if (diffViolations) { - const formattedMatches = Array.isArray(diffViolations) ? formatMatches(diffViolations).join('\n\n') : diffViolations; + const formattedMatches = Array.isArray(diffViolations) + ? formatMatches(diffViolations).join('\n\n') + : diffViolations; const errorMsg = []; errorMsg.push(`\n\n\n\nYour push has been blocked.\n`); errorMsg.push(`Please ensure your code does not contain sensitive information or URLs.\n\n`); - errorMsg.push(formattedMatches) - errorMsg.push('\n') + errorMsg.push(formattedMatches); + errorMsg.push('\n'); console.log(`The following diff is illegal: ${commitFrom}:${commitTo}`); step.error = true; step.log(`The following diff is illegal: ${commitFrom}:${commitTo}`); - step.setError( - errorMsg.join('\n') - ); + step.setError(errorMsg.join('\n')); action.addStep(step); return action; diff --git a/src/proxy/processors/types.ts b/src/proxy/processors/types.ts index bb267ce90..ae92ba48c 100644 --- a/src/proxy/processors/types.ts +++ b/src/proxy/processors/types.ts @@ -1,4 +1,4 @@ -import { Action } from "../actions"; +import { Action } from '../actions'; export interface Processor { exec(req: any, action: Action): Promise; @@ -17,4 +17,4 @@ export type CommitContent = { deflatedSize: number; objectRef: any; content: string; -} +}; diff --git a/src/proxy/routes/index.ts b/src/proxy/routes/index.ts index c49853376..973608169 100644 --- a/src/proxy/routes/index.ts +++ b/src/proxy/routes/index.ts @@ -132,9 +132,4 @@ const handleMessage = (message: string): string => { return packetMessage; }; -export { - router, - handleMessage, - validGitRequest, - stripGitHubFromGitPath, -}; +export { router, handleMessage, validGitRequest, stripGitHubFromGitPath }; diff --git a/src/service/passport/oidc.js b/src/service/passport/oidc.js index 904faff04..f81a8de06 100644 --- a/src/service/passport/oidc.js +++ b/src/service/passport/oidc.js @@ -10,7 +10,7 @@ const configure = async () => { const { issuer, clientID, clientSecret, callbackURL, scope } = oidcConfig; if (!oidcConfig || !oidcConfig.issuer) { - throw new Error('Missing OIDC issuer in configuration') + throw new Error('Missing OIDC issuer in configuration'); } const server = new URL(issuer); @@ -25,7 +25,7 @@ const configure = async () => { const userInfo = await fetchUserInfo(config, tokenSet.access_token, expectedSub); handleUserAuthentication(userInfo, done); }); - + // currentUrl must be overridden to match the callback URL strategy.currentUrl = function (request) { const callbackUrl = new URL(callbackURL); @@ -39,7 +39,7 @@ const configure = async () => { passport.serializeUser((user, done) => { done(null, user.oidcId || user.username); - }) + }); passport.deserializeUser(async (id, done) => { try { @@ -48,7 +48,7 @@ const configure = async () => { } catch (err) { done(err); } - }) + }); passport.type = server.host; return passport; @@ -56,14 +56,13 @@ const configure = async () => { console.error('OIDC configuration failed:', error); throw error; } -} - +}; module.exports.configure = configure; /** * Handles user authentication with OIDC. - * @param {Object} userInfo the OIDC user info object + * @param {Object} userInfo the OIDC user info object * @param {Function} done the callback function * @return {Promise} a promise with the authenticated user or an error */ @@ -98,7 +97,9 @@ const handleUserAuthentication = async (userInfo, done) => { * @return {string | null} the email address */ const safelyExtractEmail = (profile) => { - return profile.email || (profile.emails && profile.emails.length > 0 ? profile.emails[0].value : null); + return ( + profile.email || (profile.emails && profile.emails.length > 0 ? profile.emails[0].value : null) + ); }; /** diff --git a/src/service/routes/auth.js b/src/service/routes/auth.js index 92cd82e39..422a866a3 100644 --- a/src/service/routes/auth.js +++ b/src/service/routes/auth.js @@ -3,7 +3,8 @@ const router = new express.Router(); const passport = require('../passport').getPassport(); const db = require('../../db'); const passportType = passport.type; -const { GIT_PROXY_UI_HOST: uiHost = 'http://localhost', GIT_PROXY_UI_PORT: uiPort = 3000 } = process.env; +const { GIT_PROXY_UI_HOST: uiHost = 'http://localhost', GIT_PROXY_UI_PORT: uiPort = 3000 } = + process.env; router.get('/', (req, res) => { res.status(200).json({ diff --git a/src/ui/components/Filtering/Filtering.css b/src/ui/components/Filtering/Filtering.css index 84f9258e0..a83724cb2 100644 --- a/src/ui/components/Filtering/Filtering.css +++ b/src/ui/components/Filtering/Filtering.css @@ -52,4 +52,4 @@ .dropdown-item:hover { background-color: #f0f0f0; -} \ No newline at end of file +} diff --git a/src/ui/components/Filtering/Filtering.jsx b/src/ui/components/Filtering/Filtering.jsx index aa9d26c78..1c9e2fcc7 100644 --- a/src/ui/components/Filtering/Filtering.jsx +++ b/src/ui/components/Filtering/Filtering.jsx @@ -27,29 +27,34 @@ const Filtering = ({ onFilterChange }) => { }; return ( -
-
+
+
{/* Make the entire button clickable for toggling dropdown */} - {isOpen && ( -
-
handleOptionClick('Date Modified')} className="dropdown-item"> +
+
handleOptionClick('Date Modified')} className='dropdown-item'> Date Modified
-
handleOptionClick('Date Created')} className="dropdown-item"> +
handleOptionClick('Date Created')} className='dropdown-item'> Date Created
-
handleOptionClick('Alphabetical')} className="dropdown-item"> +
handleOptionClick('Alphabetical')} className='dropdown-item'> Alphabetical
@@ -60,7 +65,3 @@ const Filtering = ({ onFilterChange }) => { }; export default Filtering; - - - - diff --git a/src/ui/components/Pagination/Pagination.jsx b/src/ui/components/Pagination/Pagination.jsx index e87e43c17..777807343 100644 --- a/src/ui/components/Pagination/Pagination.jsx +++ b/src/ui/components/Pagination/Pagination.jsx @@ -1,8 +1,7 @@ import React from 'react'; -import './Pagination.css'; +import './Pagination.css'; export default function Pagination({ currentPage, totalItems = 0, itemsPerPage, onPageChange }) { - const totalPages = Math.ceil(totalItems / itemsPerPage); const handlePageClick = (page) => { diff --git a/src/ui/components/Search/Search.css b/src/ui/components/Search/Search.css index db87dc8c0..d4c650d13 100644 --- a/src/ui/components/Search/Search.css +++ b/src/ui/components/Search/Search.css @@ -1,7 +1,7 @@ .search-bar { width: 100%; - max-width:100%; - margin: 0 auto 20px auto; + max-width: 100%; + margin: 0 auto 20px auto; } .search-input { @@ -10,9 +10,9 @@ font-size: 16px; border: 1px solid #ccc; border-radius: 4px; - box-sizing: border-box; + box-sizing: border-box; } .search-input:focus { - border-color: #007bff; + border-color: #007bff; } diff --git a/src/ui/components/Search/Search.jsx b/src/ui/components/Search/Search.jsx index 5e1cbf6b4..e774ea0f2 100644 --- a/src/ui/components/Search/Search.jsx +++ b/src/ui/components/Search/Search.jsx @@ -2,27 +2,26 @@ import React from 'react'; import { TextField } from '@material-ui/core'; import './Search.css'; import InputAdornment from '@material-ui/core/InputAdornment'; -import SearchIcon from '@material-ui/icons/Search'; - +import SearchIcon from '@material-ui/icons/Search'; export default function Search({ onSearch }) { const handleSearchChange = (event) => { const query = event.target.value; - onSearch(query); + onSearch(query); }; return (
+ ), @@ -31,7 +30,3 @@ export default function Search({ onSearch }) {
); } - - - - diff --git a/src/ui/views/RepoList/Components/Repositories.jsx b/src/ui/views/RepoList/Components/Repositories.jsx index 4be87b36d..85b6e43c1 100644 --- a/src/ui/views/RepoList/Components/Repositories.jsx +++ b/src/ui/views/RepoList/Components/Repositories.jsx @@ -14,8 +14,7 @@ import { UserContext } from '../../../../context'; import PropTypes from 'prop-types'; import Search from '../../../components/Search/Search'; import Pagination from '../../../components/Pagination/Pagination'; -import Filtering from '../../../components/Filtering/Filtering'; - +import Filtering from '../../../components/Filtering/Filtering'; export default function Repositories(props) { const useStyles = makeStyles(styles); @@ -26,7 +25,7 @@ export default function Repositories(props) { const [isLoading, setIsLoading] = useState(false); const [isError, setIsError] = useState(false); const [currentPage, setCurrentPage] = useState(1); - const itemsPerPage = 5; + const itemsPerPage = 5; const navigate = useNavigate(); const { user } = useContext(UserContext); const openRepo = (repo) => navigate(`/admin/repo/${repo}`, { replace: true }); @@ -37,10 +36,16 @@ export default function Repositories(props) { if (!k) continue; query[k] = props[k]; } - getRepos(setIsLoading, (data) => { - setData(data); - setFilteredData(data); - }, setAuth, setIsError, query); + getRepos( + setIsLoading, + (data) => { + setData(data); + setFilteredData(data); + }, + setAuth, + setIsError, + query, + ); }, [props]); const refresh = async (repo) => { @@ -50,16 +55,17 @@ export default function Repositories(props) { }; const handleSearch = (query) => { - setCurrentPage(1); + setCurrentPage(1); if (!query) { setFilteredData(data); } else { const lowercasedQuery = query.toLowerCase(); setFilteredData( - data.filter(repo => - repo.name.toLowerCase().includes(lowercasedQuery) || - repo.project.toLowerCase().includes(lowercasedQuery) - ) + data.filter( + (repo) => + repo.name.toLowerCase().includes(lowercasedQuery) || + repo.project.toLowerCase().includes(lowercasedQuery), + ), ); } }; @@ -88,8 +94,7 @@ export default function Repositories(props) { setFilteredData(sortedData); }; - - const handlePageChange = (page) => setCurrentPage(page); + const handlePageChange = (page) => setCurrentPage(page); const startIdx = (currentPage - 1) * itemsPerPage; const paginatedData = filteredData.slice(startIdx, startIdx + itemsPerPage); @@ -109,14 +114,14 @@ export default function Repositories(props) { key='x' classes={classes} openRepo={openRepo} - data={paginatedData} + data={paginatedData} repoButton={addrepoButton} - onSearch={handleSearch} - currentPage={currentPage} - totalItems={filteredData.length} - itemsPerPage={itemsPerPage} - onPageChange={handlePageChange} - onFilterChange={handleFilterChange} // Pass handleFilterChange as prop + onSearch={handleSearch} + currentPage={currentPage} + totalItems={filteredData.length} + itemsPerPage={itemsPerPage} + onPageChange={handlePageChange} + onFilterChange={handleFilterChange} // Pass handleFilterChange as prop /> ); } @@ -138,9 +143,8 @@ function GetGridContainerLayOut(props) { {props.repoButton} - - {/* Include the Filtering component */} + {/* Include the Filtering component */} @@ -166,4 +170,3 @@ function GetGridContainerLayOut(props) { ); } - diff --git a/src/ui/views/UserList/Components/UserList.jsx b/src/ui/views/UserList/Components/UserList.jsx index f148a8384..5be4ce1c5 100644 --- a/src/ui/views/UserList/Components/UserList.jsx +++ b/src/ui/views/UserList/Components/UserList.jsx @@ -20,7 +20,6 @@ import Search from '../../../components/Search/Search'; const useStyles = makeStyles(styles); export default function UserList(props) { - const classes = useStyles(); const [data, setData] = useState([]); const [, setAuth] = useState(true); @@ -28,12 +27,11 @@ export default function UserList(props) { const [isError, setIsError] = useState(false); const navigate = useNavigate(); const [currentPage, setCurrentPage] = useState(1); - const itemsPerPage = 5; + const itemsPerPage = 5; const [searchQuery, setSearchQuery] = useState(''); const openUser = (username) => navigate(`/admin/user/${username}`, { replace: true }); - useEffect(() => { const query = {}; @@ -47,32 +45,30 @@ export default function UserList(props) { if (isLoading) return
Loading...
; if (isError) return
Something went wrong...
; - - const filteredUsers = data.filter(user => - user.displayName && user.displayName.toLowerCase().includes(searchQuery.toLowerCase()) || - user.username && user.username.toLowerCase().includes(searchQuery.toLowerCase()) -); + const filteredUsers = data.filter( + (user) => + (user.displayName && user.displayName.toLowerCase().includes(searchQuery.toLowerCase())) || + (user.username && user.username.toLowerCase().includes(searchQuery.toLowerCase())), + ); const indexOfLastItem = currentPage * itemsPerPage; const indexOfFirstItem = indexOfLastItem - itemsPerPage; const currentItems = filteredUsers.slice(indexOfFirstItem, indexOfLastItem); const totalItems = filteredUsers.length; - const handlePageChange = (page) => { setCurrentPage(page); }; - const handleSearch = (query) => { setSearchQuery(query); - setCurrentPage(1); + setCurrentPage(1); }; return ( - + @@ -94,12 +90,20 @@ export default function UserList(props) { {row.email} - + {row.gitAccount} - {row.admin ? : } + {row.admin ? ( + + ) : ( + + )}