diff --git a/src/db/file/index.ts b/src/db/file/index.ts index 6ac1c2088..18c36a06b 100644 --- a/src/db/file/index.ts +++ b/src/db/file/index.ts @@ -17,6 +17,7 @@ export const { export const { getRepos, getRepo, + getRepoByUrl, createRepo, addUserCanPush, addUserCanAuthorise, diff --git a/src/db/file/repo.ts b/src/db/file/repo.ts index fd7218c15..d07e1a2d0 100644 --- a/src/db/file/repo.ts +++ b/src/db/file/repo.ts @@ -10,7 +10,8 @@ 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 }); +// export for testing purposes +export const db = new Datastore({ filename: './.data/db/repos.db', autoload: true }); db.ensureIndex({ fieldName: 'name', unique: false }); db.setAutocompactionInterval(COMPACTION_INTERVAL); @@ -49,6 +50,20 @@ export const getRepo = async (name: string) => { }); }; +export const getRepoByUrl = async (url: string) => { + return new Promise((resolve, reject) => { + db.findOne({ url: url.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 { + resolve(doc); + } + }); + }); +}; + export const createRepo = async (repo: Repo) => { if (isBlank(repo.project)) { throw new Error('Project name cannot be empty'); diff --git a/src/db/index.ts b/src/db/index.ts index ff1189f1b..19a79b098 100644 --- a/src/db/index.ts +++ b/src/db/index.ts @@ -71,6 +71,7 @@ export const { updateUser, getRepos, getRepo, + getRepoByUrl, createRepo, addUserCanPush, addUserCanAuthorise, diff --git a/src/db/mongo/index.ts b/src/db/mongo/index.ts index a6d7ce6b2..b190bed49 100644 --- a/src/db/mongo/index.ts +++ b/src/db/mongo/index.ts @@ -20,6 +20,7 @@ export const { export const { getRepos, getRepo, + getRepoByUrl, createRepo, addUserCanPush, addUserCanAuthorise, diff --git a/src/db/mongo/repo.ts b/src/db/mongo/repo.ts index f299f907a..ed1c26535 100644 --- a/src/db/mongo/repo.ts +++ b/src/db/mongo/repo.ts @@ -18,6 +18,11 @@ export const getRepo = async (name: string) => { return collection.findOne({ name: { $eq: name } }); }; +export const getRepoByUrl = async (url: string) => { + const collection = await connect(collectionName); + return collection.findOne({ url: { $eq: url.toLowerCase() } }); +}; + export const createRepo = async (repo: Repo) => { repo.name = repo.name.toLowerCase(); console.log(`creating new repo ${JSON.stringify(repo)}`); diff --git a/src/proxy/processors/push-action/checkRepoInAuthorisedList.ts b/src/proxy/processors/push-action/checkRepoInAuthorisedList.ts index c3768888b..5f67f4e50 100644 --- a/src/proxy/processors/push-action/checkRepoInAuthorisedList.ts +++ b/src/proxy/processors/push-action/checkRepoInAuthorisedList.ts @@ -1,40 +1,22 @@ import { Action, Step } from '../../actions'; -import { getRepos } from '../../../db'; -import { Repo } from '../../../db/types'; -import { trimTrailingDotGit } from '../../../db/helper'; +import { getRepoByUrl } from '../../../db'; // Execute if the repo is approved const exec = async ( req: any, action: Action, - authorisedList: () => Promise = getRepos, ): Promise => { const step = new Step('checkRepoInAuthorisedList'); - const list = await authorisedList(); - console.log(list); - - const found = list.find((x: Repo) => { - const targetName = trimTrailingDotGit(action.repo.toLowerCase()); - const allowedName = trimTrailingDotGit(`${x.project}/${x.name}`.toLowerCase()); - console.log(`${targetName} = ${allowedName}`); - return targetName === allowedName; - }); - - console.log(found); - - if (!found) { - console.log('not found'); + const found = await getRepoByUrl(action.url); + if (found) { + step.log(`repo ${action.repo} is in the authorisedList`); + } else { step.error = true; - step.log(`repo ${action.repo} is not in the authorisedList, ending`); - console.log('setting error'); - step.setError(`Rejecting repo ${action.repo} not in the authorisedList`); - action.addStep(step); - return action; + step.log(`repo ${action.repo} is not in the authorised whitelist, ending`); + step.setError(`Rejecting repo ${action.repo} not in the authorised whitelist`); } - console.log('found'); - step.log(`repo ${action.repo} is in the authorisedList`); action.addStep(step); return action; }; diff --git a/test/db/file/repo.test.js b/test/db/file/repo.test.js new file mode 100644 index 000000000..d96e37cc4 --- /dev/null +++ b/test/db/file/repo.test.js @@ -0,0 +1,94 @@ +const { expect } = require('chai'); +const sinon = require('sinon'); +const repoModule = require('../../../src/db/file/repo'); + +describe('File DB', () => { + let sandbox; + + beforeEach(() => { + sandbox = sinon.createSandbox(); + }); + + afterEach(() => { + sandbox.restore(); + }); + + describe('getRepo', () => { + it('should get the repo using the name', async () => { + const repoData = { + name: 'sample', + users: { canPush: [] }, + url: 'http://example.com/sample-repo.git', + }; + + sandbox.stub(repoModule.db, 'findOne').callsFake((query, cb) => cb(null, repoData)); + + const result = await repoModule.getRepo('Sample'); + expect(result).to.equal(repoData); + }); + }); + + describe('getRepoByUrl', () => { + it('should get the repo using the url', async () => { + const repoData = { + name: 'sample', + users: { canPush: [] }, + url: 'https://github.com/finos/git-proxy.git', + }; + + sandbox.stub(repoModule.db, 'findOne').callsFake((query, cb) => cb(null, repoData)); + + const result = await repoModule.getRepoByUrl('https://github.com/finos/git-proxy.git'); + expect(result).to.equal(repoData); + }); + + it('should get the repo using the url, stripping off the .git', async () => { + const repoData = { + name: 'sample', + users: { canPush: [] }, + url: 'https://github.com/finos/git-proxy.git', + }; + + sandbox.stub(repoModule.db, 'findOne').callsFake((query, cb) => cb(null, repoData)); + + const result = await repoModule.getRepoByUrl('https://github.com/finos/git-proxy.git'); + + expect(repoModule.db.findOne.calledWith(sinon.match({ url: 'https://github.com/finos/git-proxy.git'}))).to.be.true; + expect(result).to.equal(repoData); + }); + + it('should get the repo using the url, ignoring the case', async () => { + const repoData = { + name: 'sample', + users: { canPush: [] }, + url: 'https://github.com/finos/git-proxy.git', + }; + + sandbox.stub(repoModule.db, 'findOne').callsFake((query, cb) => cb(null, repoData)); + + const result = await repoModule.getRepoByUrl('https://github.com/Finos/Git-Proxy.git'); + expect(result).to.equal(repoData); + expect(repoModule.db.findOne.calledWith(sinon.match({ url: 'https://github.com/finos/git-proxy.git' }))).to.be.true; + }); + + it('should return null if the repo is not found', async () => { + sandbox.stub(repoModule.db, 'findOne').callsFake((query, cb) => cb(null, null)); + + const result = await repoModule.getRepoByUrl('https://github.com/finos/missing-repo.git'); + expect(result).to.be.null; + expect(repoModule.db.findOne.calledWith(sinon.match({ url: 'https://github.com/finos/missing-repo.git' })), + ).to.be.true; + }); + + it('should reject if the database returns an error', async () => { + sandbox.stub(repoModule.db, 'findOne').callsFake((query, cb) => cb(new Error('DB error'))); + + try { + await repoModule.getRepoByUrl('https://github.com/finos/git-proxy.git'); + expect.fail('Expected promise to be rejected'); + } catch (err) { + expect(err.message).to.equal('DB error'); + } + }); + }); +}); diff --git a/test/db/mongo/repo.test.js b/test/db/mongo/repo.test.js new file mode 100644 index 000000000..187792686 --- /dev/null +++ b/test/db/mongo/repo.test.js @@ -0,0 +1,69 @@ +const { expect } = require('chai'); +const sinon = require('sinon'); +const proxyqquire = require('proxyquire'); + +const repoCollection = { + findOne: sinon.stub(), +}; + +const connectionStub = sinon.stub().returns(repoCollection); + +const { getRepo, getRepoByUrl } = proxyqquire('../../../src/db/mongo/repo', { + './helper': { connect: connectionStub }, +}); + +describe('MongoDB', () => { + afterEach(function () { + sinon.restore(); + }); + + describe('getRepo', () => { + it('should get the repo using the name', async () => { + const repoData = { + name: 'sample', + users: { canPush: [] }, + url: 'http://example.com/sample-repo.git', + }; + repoCollection.findOne.resolves(repoData); + + const result = await getRepo('Sample'); + expect(result).to.equal(repoData); + expect(connectionStub.calledWith('repos')).to.be.true; + expect(repoCollection.findOne.calledWith({ name: { $eq: 'sample' } })).to.be.true; + }); + }); + + describe('getRepoByUrl', () => { + it('should get the repo using the url', async () => { + const repoData = { + name: 'sample', + users: { canPush: [] }, + url: 'https://github.com/finos/git-proxy.git', + }; + repoCollection.findOne.resolves(repoData); + + const result = await getRepoByUrl('https://github.com/finos/git-proxy.git'); + expect(result).to.equal(repoData); + expect(connectionStub.calledWith('repos')).to.be.true; + expect( + repoCollection.findOne.calledWith({ url: { $eq: 'https://github.com/finos/git-proxy.git' } }), + ).to.be.true; + }); + + it('should get the repo using the url, ignoring the case', async () => { + const repoData = { + name: 'sample', + users: { canPush: [] }, + url: 'https://github.com/finos/git-proxy.git', + }; + repoCollection.findOne.resolves(repoData); + + const result = await getRepoByUrl('https://github.com/Finos/Git-Proxy.git'); + expect(result).to.equal(repoData); + expect(connectionStub.calledWith('repos')).to.be.true; + expect( + repoCollection.findOne.calledWith({ url: { $eq: 'https://github.com/finos/git-proxy.git' } }), + ).to.be.true; + }); + }); +}); diff --git a/test/processors/testCheckRepoInAuthList.test.js b/test/processors/testCheckRepoInAuthList.test.js new file mode 100644 index 000000000..f280e844a --- /dev/null +++ b/test/processors/testCheckRepoInAuthList.test.js @@ -0,0 +1,34 @@ +const chai = require('chai'); +const sinon = require('sinon'); +const actions = require('../../src/proxy/actions/Action'); +const processor = require('../../src/proxy/processors/push-action/checkRepoInAuthorisedList'); +const expect = chai.expect; +const db = require('../../src/db'); + +describe('Check a Repo is in the authorised list', async () => { + afterEach(() => { + sinon.restore(); + }); + + it('accepts the action if the repository is whitelisted in the db', async () => { + sinon.stub(db, 'getRepoByUrl').resolves({ + name: 'repo-is-ok', + project: 'thisproject', + url: 'https://github.com/thisproject/repo-is-ok', + }); + + const action = new actions.Action('123', 'type', 'get', 1234, 'thisproject/repo-is-ok'); + const result = await processor.exec(null, action); + expect(result.error).to.be.false; + expect(result.steps[0].logs[0]).to.eq('checkRepoInAuthorisedList - repo thisproject/repo-is-ok is in the authorisedList'); + }); + + it('rejects the action if repository not in the db', async () => { + sinon.stub(db, 'getRepoByUrl').resolves(null); + + const action = new actions.Action('123', 'type', 'get', 1234, 'thisproject/repo-is-not-ok'); + const result = await processor.exec(null, action); + expect(result.error).to.be.true; + expect(result.steps[0].logs[0]).to.eq('checkRepoInAuthorisedList - repo thisproject/repo-is-not-ok is not in the authorised whitelist, ending'); + }); +}); diff --git a/test/testCheckRepoInAuthList.test.js b/test/testCheckRepoInAuthList.test.js deleted file mode 100644 index 19d161c12..000000000 --- a/test/testCheckRepoInAuthList.test.js +++ /dev/null @@ -1,27 +0,0 @@ -const chai = require('chai'); -const actions = require('../src/proxy/actions/Action'); -const processor = require('../src/proxy/processors/push-action/checkRepoInAuthorisedList'); -const expect = chai.expect; - -const authList = () => { - return [ - { - name: 'repo-is-ok', - project: 'thisproject', - }, - ]; -}; - -describe('Check a Repo is in the authorised list', async () => { - it('Should set ok=true if repo in whitelist', async () => { - const action = new actions.Action('123', 'type', 'get', 1234, 'thisproject/repo-is-ok'); - const result = await processor.exec(null, action, authList); - expect(result.error).to.be.false; - }); - - it('Should set ok=false if not in authorised', async () => { - const action = new actions.Action('123', 'type', 'get', 1234, 'thisproject/repo-is-not-ok'); - const result = await processor.exec(null, action, authList); - expect(result.error).to.be.true; - }); -});