From cd67c9565a10242982ff51645dcd5e31f1ca32cb Mon Sep 17 00:00:00 2001 From: Kris West Date: Tue, 25 Mar 2025 12:43:47 +0000 Subject: [PATCH 01/35] fix: add indexes, compaction and consistent lowercasing of inputs in file-based DB implementation --- src/db/file/pushes.ts | 4 + src/db/file/repo.ts | 39 +++- src/db/file/users.ts | 44 ++++- test/testDb.test.js | 450 ++++++++++++++++++++++++++++++++++++++++-- 4 files changed, 519 insertions(+), 18 deletions(-) diff --git a/src/db/file/pushes.ts b/src/db/file/pushes.ts index 2a54314ea..b58aea4eb 100644 --- a/src/db/file/pushes.ts +++ b/src/db/file/pushes.ts @@ -6,10 +6,14 @@ import { toClass } from '../helper'; import * as repo from './repo'; 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) { diff --git a/test/testDb.test.js b/test/testDb.test.js index 896be0362..d290e496f 100644 --- a/test/testDb.test.js +++ b/test/testDb.test.js @@ -10,6 +10,13 @@ const TEST_REPO = { url: 'https://github.com/finos/db-test-repo.git', }; +const TEST_NONEXISTENT_REPO = { + project: 'MegaCorp', + name: 'repo', + url: 'https://example.com/MegaCorp/MegaGroup/repo.git', + _id: 'ABCDEFGHIJKLMNOP', +}; + const TEST_USER = { username: 'db-u1', password: 'abc', @@ -35,7 +42,7 @@ const TEST_PUSH = { timestamp: 1744380903338, project: 'finos', repoName: 'db-test-repo.git', - url: 'https://github.com/finos/db-test-repo.git', + url: TEST_REPO.url, repo: 'finos/db-test-repo.git', user: 'db-test-user', userEmail: 'db-test@test.com', @@ -50,22 +57,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 +93,114 @@ 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); + + // 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 url', async function () { + const repo = await db.getRepoByUrl(TEST_REPO.url); + const cleanRepo = cleanResponseData(TEST_REPO, repo); + expect(cleanRepo).to.eql(TEST_REPO); + }); + + it('should be able to retrieve a repo by id', async function () { + // _id is autogenerated by the DB so we need to retrieve it before we can use it + const repo = await db.getRepoByUrl(TEST_REPO.url); + const repoById = await db.getRepoById(repo._id); + const cleanRepo = cleanResponseData(TEST_REPO, repoById); + expect(cleanRepo).to.eql(TEST_REPO); + }); + it('should be able to delete a repo', async function () { - await db.deleteRepo(TEST_REPO.name); + // _id is autogenerated by the DB so we need to retrieve it before we can use it + const repo = await db.getRepoByUrl(TEST_REPO.url); + await db.deleteRepo(repo._id); 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, @@ -91,16 +210,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 +247,149 @@ 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(); + const cleanUsers = cleanResponseData(updatedUser, users); + 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 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(TEST_NONEXISTENT_REPO._id, 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 () { + // 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.url, TEST_USER.username); + expect(allowed).to.be.false; + + const repo = await db.getRepoByUrl(TEST_REPO.url); + + // uppercase the filter value to confirm db client is lowercasing inputs + await db.addUserCanPush(repo._id, TEST_USER.username.toUpperCase()); + + // repeat, should not throw an error if already set + await db.addUserCanPush(repo._id, TEST_USER.username.toUpperCase()); + + // confirm the setting exists + allowed = await db.isUserPushAllowed(TEST_REPO.url, TEST_USER.username); + expect(allowed).to.be.true; + + // confirm that casing doesn't matter + allowed = await db.isUserPushAllowed(TEST_REPO.url, TEST_USER.username.toUpperCase()); + expect(allowed).to.be.true; + }); + + 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(TEST_NONEXISTENT_REPO._id, 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 { + // repo should already exist with user able to push after previous test + let allowed = await db.isUserPushAllowed(TEST_REPO.url, TEST_USER.username); + expect(allowed).to.be.true; + + const repo = await db.getRepoByUrl(TEST_REPO.url); + + // uppercase the filter value to confirm db client is lowercasing inputs + await db.removeUserCanPush(repo._id, TEST_USER.username.toUpperCase()); + + // repeat, should not throw an error if already unset + await db.removeUserCanPush(repo._id, TEST_USER.username.toUpperCase()); + + // confirm the setting exists + allowed = await db.isUserPushAllowed(TEST_REPO.url, TEST_USER.username); + expect(allowed).to.be.false; + + // confirm that casing doesn't matter + allowed = await db.isUserPushAllowed(TEST_REPO.url, TEST_USER.username.toUpperCase()); + expect(allowed).to.be.false; + } catch (e) { + console.error('Error thrown at: ' + e.stack, e); + threwError = true; + } + 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(TEST_NONEXISTENT_REPO._id, TEST_USER.username); + } catch (e) { + threwError = true; + } + expect(threwError).to.be.true; + }); + + 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(TEST_NONEXISTENT_REPO._id, TEST_USER.username); + } catch (e) { + threwError = true; + } + expect(threwError).to.be.true; + }); + + it('should NOT throw an error when checking whether a user can push on non-existent repo', async function () { + const allowed = await db.isUserPushAllowed(TEST_NONEXISTENT_REPO.url, TEST_USER.username); + expect(allowed).to.be.false; + }); + it('should be able to create a push', async function () { await db.writeAudit(TEST_PUSH); const pushes = await db.getPushes(); @@ -122,5 +404,151 @@ describe('Database client', async () => { expect(cleanPushes).to.not.deep.include(TEST_PUSH); }); - after(async function () {}); + 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; + try { + const repo = await db.getRepoByUrl(TEST_REPO.url); + + // 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(repo._id, TEST_USER.username); + allowed = await db.canUserCancelPush(TEST_PUSH.id, TEST_USER.username); + expect(allowed).to.be.true; + + // deauthorise user and recheck + await db.removeUserCanPush(repo._id, TEST_USER.username); + allowed = await db.canUserCancelPush(TEST_PUSH.id, TEST_USER.username); + expect(allowed).to.be.false; + } catch (e) { + console.error(e); + threwError = true; + } + expect(threwError).to.be.false; + // clean up + await db.deletePush(TEST_PUSH.id); + }); + + it('should be able to check if a user can approve/reject push', async function () { + let threwError = false; + 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; + + const repo = await db.getRepoByUrl(TEST_REPO.url); + + // authorise user and recheck + await db.addUserCanAuthorise(repo._id, TEST_USER.username); + allowed = await db.canUserApproveRejectPush(TEST_PUSH.id, TEST_USER.username); + expect(allowed).to.be.true; + + // deauthorise user and recheck + await db.removeUserCanAuthorise(repo._id, TEST_USER.username); + allowed = await db.canUserApproveRejectPush(TEST_PUSH.id, TEST_USER.username); + expect(allowed).to.be.false; + } catch (e) { + threwError = true; + } + expect(threwError).to.be.false; + + // clean up + await db.deletePush(TEST_PUSH.id); + }); + + after(async function () { + // _id is autogenerated by the DB so we need to retrieve it before we can use it + const repo = await db.getRepoByUrl(TEST_REPO.url); + await db.deleteRepo(repo._id, true); + await db.deleteUser(TEST_USER.username); + await db.deletePush(TEST_PUSH.id); + }); }); From 1174c071459f362bb532c6b6902e671fa641602d Mon Sep 17 00:00:00 2001 From: Kris West Date: Tue, 25 Mar 2025 12:50:53 +0000 Subject: [PATCH 02/35] 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 c981a8f8ec724fc7085bc6826d564fc650d133c4 Mon Sep 17 00:00:00 2001 From: kriswest Date: Tue, 25 Mar 2025 19:09:14 +0000 Subject: [PATCH 03/35] 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 82ef44170936257783d70515c90bacbd60658cce Mon Sep 17 00:00:00 2001 From: Kris West Date: Tue, 25 Mar 2025 12:43:47 +0000 Subject: [PATCH 04/35] 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 b58aea4eb..3abfc1740 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 e129964ee5ef873a285fa3c54f0196d5fca36301 Mon Sep 17 00:00:00 2001 From: Kris West Date: Tue, 8 Apr 2025 09:43:35 +0100 Subject: [PATCH 05/35] 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 44b6c5108ea38710f575c773944ddee15b138402 Mon Sep 17 00:00:00 2001 From: Kris West Date: Thu, 10 Apr 2025 14:02:12 +0100 Subject: [PATCH 06/35] 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 3abfc1740..b58aea4eb 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 c81f829153005de93ebd4b398f6b2d157d58f0de Mon Sep 17 00:00:00 2001 From: Kris West Date: Tue, 25 Mar 2025 12:43:47 +0000 Subject: [PATCH 07/35] 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 b58aea4eb..3abfc1740 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 1847c0ab83832aa108afc288359eb66f34057a5f Mon Sep 17 00:00:00 2001 From: Kris West Date: Tue, 1 Apr 2025 14:22:18 +0100 Subject: [PATCH 08/35] 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 ab803386ffb9efc2dda5c4c7597cfbed8c89818a Mon Sep 17 00:00:00 2001 From: Kris West Date: Tue, 25 Mar 2025 12:43:47 +0000 Subject: [PATCH 09/35] 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 c778324e251fe0c35a17d65fc4f8dbe3804526c5 Mon Sep 17 00:00:00 2001 From: Kris West Date: Tue, 25 Mar 2025 12:50:53 +0000 Subject: [PATCH 10/35] 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 7040595ef0d9ccfdcb3fb3b2cf9532be04b01670 Mon Sep 17 00:00:00 2001 From: Kris West Date: Tue, 8 Apr 2025 09:43:35 +0100 Subject: [PATCH 11/35] 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 c76976919fc9185e0b771e77995b14af7b99fd3c Mon Sep 17 00:00:00 2001 From: Kris West Date: Thu, 10 Apr 2025 14:02:12 +0100 Subject: [PATCH 12/35] 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 3abfc1740..b58aea4eb 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 0edd4dbf832f5e53da5695555a273104b4e4bbb9 Mon Sep 17 00:00:00 2001 From: Kris West Date: Fri, 21 Mar 2025 17:09:25 +0000 Subject: [PATCH 13/35] 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 dd4fad029113092f84decd282be95fc6a7f71bd2 Mon Sep 17 00:00:00 2001 From: Kris West Date: Tue, 25 Mar 2025 10:58:09 +0000 Subject: [PATCH 14/35] 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 f794e066c..7773e5f0d 100644 --- a/src/db/file/repo.ts +++ b/src/db/file/repo.ts @@ -15,7 +15,7 @@ const isBlank = (str: string) => { return !str || /^\s*$/.test(str); }; -export const getRepos = async (query: any = {}) => { +export const getRepos = async (query: any = {}) => { return new Promise((resolve, reject) => { db.find(query, (err: Error, docs: Repo[]) => { if (err) { From ed578c181fa713ab3f70765cce423d0a8272865d Mon Sep 17 00:00:00 2001 From: Kris West Date: Wed, 16 Apr 2025 17:54:57 +0100 Subject: [PATCH 15/35] 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 | 1 + 5 files changed, 67 insertions(+), 7 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 b86fa222c..e242eec45 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 d290e496f..e295a3639 100644 --- a/test/testDb.test.js +++ b/test/testDb.test.js @@ -128,6 +128,7 @@ describe('Database clients', async () => { const repo = await db.getRepoByUrl(TEST_REPO.url); await db.deleteRepo(repo._id); const repos = await db.getRepos(); + const cleanRepos = cleanResponseData(TEST_REPO, repos); expect(cleanRepos).to.not.deep.include(TEST_REPO); }); From 22808113751bf33f59d6c6a2369b6a92d7a93fc1 Mon Sep 17 00:00:00 2001 From: Kris West Date: Thu, 17 Apr 2025 11:33:25 +0100 Subject: [PATCH 16/35] test: more DB test coverage --- src/db/file/pushes.ts | 6 +++--- src/db/file/repo.ts | 8 ++++++-- test/testDb.test.js | 37 +++++++++++++++++++++++++++++++++++++ 3 files changed, 46 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 e295a3639..80964cc0d 100644 --- a/test/testDb.test.js +++ b/test/testDb.test.js @@ -336,6 +336,16 @@ describe('Database clients', async () => { expect(threwError).to.be.true; }); + 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 { @@ -391,6 +401,33 @@ describe('Database clients', async () => { expect(allowed).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(); From 6ec806292ebdcbc5b40d4e7f8bccaafe7767bc2b Mon Sep 17 00:00:00 2001 From: kriswest Date: Wed, 7 May 2025 14:12:42 +0100 Subject: [PATCH 17/35] chore: extended prettier format command to package/git-proxy-cli and ran it --- package.json | 2 +- .../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 +- .../processors/pre-processor/parseAction.ts | 6 +- .../push-action/checkAuthorEmails.ts | 6 +- .../push-action/checkCommitMessages.ts | 2 +- .../push-action/checkUserPushPermission.ts | 4 +- .../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 | 20 +++-- src/service/routes/auth.js | 5 +- 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 +-- 38 files changed, 253 insertions(+), 253 deletions(-) diff --git a/package.json b/package.json index f05521aa0..64c9a0722 100644 --- a/package.json +++ b/package.json @@ -19,7 +19,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/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/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 7a95f6c12..01517539d 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/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 18fdf7de9..52928460b 100644 --- a/src/service/passport/oidc.js +++ b/src/service/passport/oidc.js @@ -7,11 +7,13 @@ const configure = async (passport) => { const { discovery, fetchUserInfo } = await import('openid-client'); const { Strategy } = await import('openid-client/passport'); const authMethods = require('../../config').getAuthMethods(); - const oidcConfig = authMethods.find((method) => method.type.toLowerCase() === "openidconnect")?.oidcConfig; + const oidcConfig = authMethods.find( + (method) => method.type.toLowerCase() === 'openidconnect', + )?.oidcConfig; 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); @@ -26,7 +28,7 @@ const configure = async (passport) => { 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); @@ -41,7 +43,7 @@ const configure = async (passport) => { passport.serializeUser((user, done) => { done(null, user.oidcId || user.username); - }) + }); passport.deserializeUser(async (id, done) => { try { @@ -50,18 +52,18 @@ const configure = async (passport) => { } catch (err) { done(err); } - }) + }); return passport; } catch (error) { console.error('OIDC configuration failed:', error); throw error; } -} +}; /** * 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 */ @@ -97,7 +99,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 aaf2efa26..9c5db2578 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 authStrategies = require('../passport').authStrategies; const db = require('../../db'); -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({ @@ -27,7 +28,7 @@ router.post('/login', passport.authenticate(authStrategies['local'].type), async const currentUser = { ...req.user }; delete currentUser.password; console.log( - `serivce.routes.auth.login: user logged in, username=${ + `service.routes.auth.login: user logged in, username=${ currentUser.username } profile=${JSON.stringify(currentUser)}`, ); 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 ac9663423..228190903 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(`/dashboard/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 ee6812485..36aef89e6 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(`/dashboard/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 ? ( + + ) : ( + + )} { - getRepo(setIsLoading, setData, setAuth, setIsError, repoName); + getRepo(setIsLoading, setData, setAuth, setIsError, repoId); }, []); const removeUser = async (userToRemove, action) => { - await deleteUser(userToRemove, repoName, action); - getRepo(setIsLoading, setData, setAuth, setIsError, repoName); + await deleteUser(userToRemove, repoId, action); + getRepo(setIsLoading, setData, setAuth, setIsError, repoId); }; - const removeRepository = async (name) => { - await deleteRepo(name); + const removeRepository = async (id) => { + await deleteRepo(id); navigate('/dashboard/repo', { replace: true }); }; - const refresh = () => getRepo(setIsLoading, setData, setAuth, setIsError, repoName); + const refresh = () => getRepo(setIsLoading, setData, setAuth, setIsError, repoId); if (isLoading) return
Loading...
; if (isError) return
Something went wrong ...
; - const { project: org, name, proxyURL } = data || {}; - const cloneURL = `${proxyURL}/${org}/${name}.git`; + const { url: remoteUrl, proxyURL } = data || {}; + const parsedUrl = new URL(remoteUrl); + const cloneURL = `${proxyURL}/${parsedUrl.host}${parsedUrl.port ? `:${parsedUrl.port}` : ''}${parsedUrl.pathname}`; return ( @@ -72,7 +73,7 @@ export default function RepoDetails() { @@ -134,7 +135,7 @@ export default function RepoDetails() { {user.admin && (
- +
)} @@ -179,7 +180,7 @@ export default function RepoDetails() { {user.admin && (
- +
)} diff --git a/src/ui/views/RepoList/Components/NewRepo.jsx b/src/ui/views/RepoList/Components/NewRepo.jsx index e1c912069..b5e93f1c4 100644 --- a/src/ui/views/RepoList/Components/NewRepo.jsx +++ b/src/ui/views/RepoList/Components/NewRepo.jsx @@ -53,8 +53,8 @@ function AddRepositoryDialog(props) { maxUser: 1, }; - if (data.project.trim().length == 0 || data.project.length > 100) { - setError('project name length unexpected'); + if (data.project.length > 100) { + setError('organisation name is too long'); return; } @@ -64,7 +64,11 @@ function AddRepositoryDialog(props) { } try { - new URL(data.url); + const parsedUrl = new URL(data.url); + if (!parsedUrl.pathname.endsWith('.git')) { + setError('Invalid git URL - Git URLs should end with .git'); + return; + } } catch { setError('Invalid URL'); return; @@ -73,6 +77,7 @@ function AddRepositoryDialog(props) { try { await addRepo(onClose, setError, data); handleSuccess(data); + handleClose(); } catch (e) { if (e.message) { @@ -124,7 +129,7 @@ function AddRepositoryDialog(props) { aria-describedby='project-helper-text' onChange={(e) => setProject(e.target.value)} /> - GitHub Organization + Organization or path @@ -136,7 +141,7 @@ function AddRepositoryDialog(props) { aria-describedby='name-helper-text' onChange={(e) => setName(e.target.value)} /> - GitHub Repository Name + Git Repository Name @@ -149,7 +154,7 @@ function AddRepositoryDialog(props) { aria-describedby='url-helper-text' onChange={(e) => setUrl(e.target.value)} /> - GitHub Repository URL + Git Repository URL diff --git a/src/ui/views/RepoList/Components/RepoOverview.jsx b/src/ui/views/RepoList/Components/RepoOverview.jsx index 826f78c97..a808b4054 100644 --- a/src/ui/views/RepoList/Components/RepoOverview.jsx +++ b/src/ui/views/RepoList/Components/RepoOverview.jsx @@ -578,6 +578,7 @@ export default function Repositories(props) { getGitHubRepository(); }, [props.data.project, props.data.name]); + // TODO add support for GitLab API: https://docs.gitlab.com/api/projects/#get-a-single-project const getGitHubRepository = async () => { await axios .get(`https://api.github.com/repos/${props.data.project}/${props.data.name}`) @@ -585,13 +586,16 @@ export default function Repositories(props) { setGitHub(res.data); }) .catch((error) => { - setErrorMessage(`Error fetching GitHub repository ${props.data.project}/${props.data.name}: ${error}`); + setErrorMessage( + `Error fetching GitHub repository ${props.data.project}/${props.data.name}: ${error}`, + ); setSnackbarOpen(true); }); }; - const { project: org, name, proxyURL } = props?.data || {}; - const cloneURL = `${proxyURL}/${org}/${name}.git`; + const { url: remoteUrl, proxyURL } = props?.data || {}; + const parsedUrl = new URL(remoteUrl); + const cloneURL = `${proxyURL}/${parsedUrl.host}${parsedUrl.port ? `:${parsedUrl.port}` : ''}${parsedUrl.pathname}`; return ( diff --git a/src/ui/views/RepoList/Components/Repositories.jsx b/src/ui/views/RepoList/Components/Repositories.jsx index 228190903..248a27ae5 100644 --- a/src/ui/views/RepoList/Components/Repositories.jsx +++ b/src/ui/views/RepoList/Components/Repositories.jsx @@ -151,7 +151,7 @@ function GetGridContainerLayOut(props) {
{props.data.map((row) => { - if (row.project && row.name) { + if (row.url) { return ; } })} From 4c82cb45acbc9f72affc7d51decc8948a348a665 Mon Sep 17 00:00:00 2001 From: Kris West Date: Fri, 30 May 2025 15:57:49 +0100 Subject: [PATCH 33/35] feat(key on repo url): fix issues in CLI tests related to change of key field --- packages/git-proxy-cli/test/testCli.test.js | 6 +++--- packages/git-proxy-cli/test/testCliUtils.js | 8 ++++---- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/packages/git-proxy-cli/test/testCli.test.js b/packages/git-proxy-cli/test/testCli.test.js index a872221c5..33d8d7917 100644 --- a/packages/git-proxy-cli/test/testCli.test.js +++ b/packages/git-proxy-cli/test/testCli.test.js @@ -219,7 +219,7 @@ describe('test git-proxy-cli', function () { before(async function () { await helper.addRepoToDb(TEST_REPO_CONFIG); - await helper.addGitPushToDb(pushId, TEST_REPO); + await helper.addGitPushToDb(pushId, TEST_REPO_CONFIG.url); }); after(async function () { @@ -415,7 +415,7 @@ describe('test git-proxy-cli', function () { before(async function () { await helper.addRepoToDb(TEST_REPO_CONFIG); - await helper.addGitPushToDb(pushId, TEST_REPO); + await helper.addGitPushToDb(pushId, TEST_REPO_CONFIG.url); }); after(async function () { @@ -492,7 +492,7 @@ describe('test git-proxy-cli', function () { before(async function () { await helper.addRepoToDb(TEST_REPO_CONFIG); await helper.addUserToDb('testuser1', 'testpassword', 'test@email.com', gitAccount); - await helper.addGitPushToDb(pushId, TEST_REPO, gitAccount); + await helper.addGitPushToDb(pushId, TEST_REPO_CONFIG.url, gitAccount); }); after(async function () { diff --git a/packages/git-proxy-cli/test/testCliUtils.js b/packages/git-proxy-cli/test/testCliUtils.js index a58ece747..e47a21fd3 100644 --- a/packages/git-proxy-cli/test/testCliUtils.js +++ b/packages/git-proxy-cli/test/testCliUtils.js @@ -30,7 +30,7 @@ async function runCli( expectedExitCode = 0, expectedMessages = null, expectedErrorMessages = null, - debug = false, + debug = true, ) { try { console.log(`cli: '${cli}'`); @@ -177,17 +177,17 @@ async function removeRepoFromDb(repoUrl) { /** * Add a new git push record to the database. * @param {string} id The ID of the git push. - * @param {string} repo The repository of the git push. + * @param {string} repoUrl The repository URL of the git push. * @param {string} user The user who pushed the git push. * @param {boolean} debug Flag to enable logging for debugging. */ -async function addGitPushToDb(id, repo, user = null, debug = false) { +async function addGitPushToDb(id, repoUrl, user = null, debug = false) { const action = new actions.Action( id, 'push', // type 'get', // method Date.now(), // timestamp - repo, + repoUrl, ); action.user = user; const step = new steps.Step( From 52dd89782e766816b4a75622e978e1b60d15e5aa Mon Sep 17 00:00:00 2001 From: Kris West Date: Tue, 3 Jun 2025 15:41:39 +0100 Subject: [PATCH 34/35] feat(key on repo url): fix CLI and cypress tests after change to proxy urls --- cypress/e2e/repo.cy.js | 2 +- package-lock.json | 47 ++++++++++++++++++++++++++++-------------- test/testParsePush.js | 6 +++--- 3 files changed, 35 insertions(+), 20 deletions(-) diff --git a/cypress/e2e/repo.cy.js b/cypress/e2e/repo.cy.js index 411397128..8b0ac1a21 100644 --- a/cypress/e2e/repo.cy.js +++ b/cypress/e2e/repo.cy.js @@ -10,7 +10,7 @@ describe('Repo', () => { describe('Code button for repo row', () => { it('Opens tooltip with correct content and can copy', () => { - const cloneURL = 'http://localhost:8000/finos/test-repo.git'; + const cloneURL = 'http://localhost:8000/github.com/finos/test-repo.git'; const tooltipQuery = 'div[role="tooltip"]'; cy diff --git a/package-lock.json b/package-lock.json index af178aa47..2919561cc 100644 --- a/package-lock.json +++ b/package-lock.json @@ -2715,6 +2715,19 @@ "eslint-scope": "5.1.1" } }, + "node_modules/@noble/hashes": { + "version": "1.8.0", + "resolved": "https://registry.npmjs.org/@noble/hashes/-/hashes-1.8.0.tgz", + "integrity": "sha512-jCs9ldd7NwzpgXDIf6P3+NrHh9/sD6CQdxHyjQI+h/6rDNo88ypBxxz45UDuZHz9r3tNz7N/VInSVoVdtXEI4A==", + "dev": true, + "license": "MIT", + "engines": { + "node": "^14.21.3 || >=16" + }, + "funding": { + "url": "https://paulmillr.com/funding/" + } + }, "node_modules/@nodelib/fs.scandir": { "version": "2.1.5", "resolved": "https://registry.npmjs.org/@nodelib/fs.scandir/-/fs.scandir-2.1.5.tgz", @@ -2864,6 +2877,16 @@ "node": "^14.17.0 || ^16.13.0 || >=18.0.0" } }, + "node_modules/@paralleldrive/cuid2": { + "version": "2.2.2", + "resolved": "https://registry.npmjs.org/@paralleldrive/cuid2/-/cuid2-2.2.2.tgz", + "integrity": "sha512-ZOBkgDwEdoYVlSeRbYYXs0S9MejQofiVYoTbKzy/6GQa39/q5tQU2IX46+shYnUkpEl3wc+J6wRlar7r2EK2xA==", + "dev": true, + "license": "MIT", + "dependencies": { + "@noble/hashes": "^1.1.5" + } + }, "node_modules/@pkgjs/parseargs": { "version": "0.11.0", "resolved": "https://registry.npmjs.org/@pkgjs/parseargs/-/parseargs-0.11.0.tgz", @@ -7492,13 +7515,14 @@ } }, "node_modules/formidable": { - "version": "2.1.2", - "resolved": "https://registry.npmjs.org/formidable/-/formidable-2.1.2.tgz", - "integrity": "sha512-CM3GuJ57US06mlpQ47YcunuUZ9jpm8Vx+P2CGt2j7HpgkKZO/DJYQ0Bobim8G6PFQmK5lOqOOdUXboU+h73A4g==", + "version": "2.1.5", + "resolved": "https://registry.npmjs.org/formidable/-/formidable-2.1.5.tgz", + "integrity": "sha512-Oz5Hwvwak/DCaXVVUtPn4oLMLLy1CdclLKO1LFgU7XzDpVMUU5UjlSLpGMocyQNNk8F6IJW9M/YdooSn2MRI+Q==", "dev": true, + "license": "MIT", "dependencies": { + "@paralleldrive/cuid2": "^2.2.2", "dezalgo": "^1.0.4", - "hexoid": "^1.0.0", "once": "^1.4.0", "qs": "^6.11.0" }, @@ -8053,15 +8077,6 @@ "he": "bin/he" } }, - "node_modules/hexoid": { - "version": "1.0.0", - "resolved": "https://registry.npmjs.org/hexoid/-/hexoid-1.0.0.tgz", - "integrity": "sha512-QFLV0taWQOZtvIRIAdBChesmogZrtuXvVWsFHZTk2SU+anspqZ2vMnoLg7IE1+Uk16N19APic1BuF8bC8c2m5g==", - "dev": true, - "engines": { - "node": ">=8" - } - }, "node_modules/highlight.js": { "version": "11.9.0", "resolved": "https://registry.npmjs.org/highlight.js/-/highlight.js-11.9.0.tgz", @@ -14029,9 +14044,9 @@ } }, "node_modules/vite": { - "version": "4.5.13", - "resolved": "https://registry.npmjs.org/vite/-/vite-4.5.13.tgz", - "integrity": "sha512-Hgp8IF/yZDzKsN1hQWOuQZbrKiaFsbQud+07jJ8h9m9PaHWkpvZ5u55Xw5yYjWRXwRQ4jwFlJvY7T7FUJG9MCA==", + "version": "4.5.14", + "resolved": "https://registry.npmjs.org/vite/-/vite-4.5.14.tgz", + "integrity": "sha512-+v57oAaoYNnO3hIu5Z/tJRZjq5aHM2zDve9YZ8HngVHbhk66RStobhb1sqPMIPEleV6cNKYK4eGrAbE9Ulbl2g==", "dev": true, "license": "MIT", "dependencies": { diff --git a/test/testParsePush.js b/test/testParsePush.js index a4106d6ce..5ce80cbfa 100644 --- a/test/testParsePush.js +++ b/test/testParsePush.js @@ -97,7 +97,7 @@ const actionData = { type: 'push', method: 'POST', timestamp: 1746542004301, - repo: 'kriswest/git-proxy.git', + repo: 'https://github.com/kriswest/git-proxy.git', }; // truncated request (should through an error and not parse) @@ -419,7 +419,7 @@ const actionData2 = { type: 'push', method: 'POST', timestamp: 1746612610060, - repo: 'kriswest/git-proxy.git', + repo: 'https://github.com/kriswest/git-proxy.git', }; // push with a commit message not terminated by a newline @@ -474,7 +474,7 @@ const actionDataNoNewLine = { type: 'push', method: 'POST', timestamp: 1746697059624, - repo: 'kriswest/git-proxy.git', + repo: 'https://github.com/kriswest/git-proxy.git', }; describe('Check that pushes can be parsed', async () => { From f1f1b9a4385ceab4fef2b12fe27fc874a3979beb Mon Sep 17 00:00:00 2001 From: Kris West Date: Tue, 3 Jun 2025 18:10:12 +0100 Subject: [PATCH 35/35] chore(key on repo url): clean-up unused helper function and test --- src/proxy/routes/helper.ts | 36 ------------------------------------ test/testRouteFilter.js | 8 -------- 2 files changed, 44 deletions(-) diff --git a/src/proxy/routes/helper.ts b/src/proxy/routes/helper.ts index dc0808760..1db075b9e 100644 --- a/src/proxy/routes/helper.ts +++ b/src/proxy/routes/helper.ts @@ -118,42 +118,6 @@ export const processGitURLForNameAndOrg = (gitUrl: string): GitNameBreakdown | n } }; -// /** Regex used to analyze legacy proxy paths to extract the repository name and -// * any path or organisation that proceeds it. */ -// const GIT_LEGACY_PATH_REGEX = /\/([^/]+)\/([^/]+)\.git/; - -// /** Type representing a breakdown of a legacy proxy path into repository name and organisation (project) -// * and a predicted GitHub URL. */ -// export type GitLegacyPathBreakdown = { project: string; repoName: string; url: string }; - -// /** Function that processes legacy proxy path string (which assumed GitHub as the repository host) to -// * extract the repository name, project (organisation) and to construct a predicted GitHub URL. -// * -// * E.g. Processing finos/git-proxy.git -// * would produce: -// * - project: finos -// * - repoName: git-proxy -// * - url: https://github.com/finos/git-proxy.git -// * -// * @param {string} requestPath The proxy path to process. -// * @return {GitLegacyPathBreakdown | null} A breakdown of the components of the URL. -// */ -// export const processLegacyProxyPathForNameAndOrg = ( -// requestPath: string, -// ): GitLegacyPathBreakdown | null => { -// const components = requestPath.match(GIT_LEGACY_PATH_REGEX); -// if (components && components.length >= 3) { -// return { -// project: components[1], -// repoName: components[2], -// url: `https://github.com/${components[1]}/${components[2]}.git`, -// }; -// } else { -// console.error(`Failed to parse git path: ${requestPath}`); -// return null; -// } -// }; - /** * Check whether an HTTP request has the expected properties of a * Git HTTP request. The URL is expected to be "sanitized", stripped of diff --git a/test/testRouteFilter.js b/test/testRouteFilter.js index 1102a7e41..f73f42565 100644 --- a/test/testRouteFilter.js +++ b/test/testRouteFilter.js @@ -79,14 +79,6 @@ describe('url helpers and filter functions used in the proxy', function () { }); }); - // it('processLegacyProxyPathForNameAndOrg should return breakdown of a legacy proxy path separating out the project (organisation), repository name and a predicted github URL', function () { - // expect(processLegacyProxyPathForNameAndOrg('/octocat/hello-world.git')).to.deep.eq({ - // project: 'octocat', - // repoName: 'hello-world', - // url: 'https://github.com/octocat/hello-world.git', - // }); - // }); - it('validGitRequest should return true for safe requests on expected URLs', function () { [ '/info/refs?service=git-upload-pack',