fix: checkRepoInAuthorisedList push failure on URL mismatch

Author: andypols

src/db/file/index.ts

@@ -17,6 +17,7 @@ export const {
 export const {
   getRepos,
   getRepo,
+  getRepoByUrl,
   createRepo,
   addUserCanPush,
   addUserCanAuthorise,

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<Repo | null>((resolve, reject) => {
+    db.findOne({ url: url.toLowerCase().replace('.git', '') }, (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');

src/db/index.ts

@@ -71,6 +71,7 @@ export const {
   updateUser,
   getRepos,
   getRepo,
+  getRepoByUrl,
   createRepo,
   addUserCanPush,
   addUserCanAuthorise,

src/db/mongo/index.ts

@@ -20,6 +20,7 @@ export const {
 export const {
   getRepos,
   getRepo,
+  getRepoByUrl,
   createRepo,
   addUserCanPush,
   addUserCanAuthorise,

src/db/mongo/repo.ts

@@ -18,6 +18,12 @@ export const getRepo = async (name: string) => {
   return collection.findOne({ name: { $eq: name } });
 };
 
+export const getRepoByUrl = async (url: string) => {
+  url = url.toLowerCase().replace('.git', '');
+  const collection = await connect(collectionName);
+  return collection.findOne({ url: { $eq: url } });
+};
+
 export const createRepo = async (repo: Repo) => {
   repo.name = repo.name.toLowerCase();
   console.log(`creating new repo ${JSON.stringify(repo)}`);

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<Repo[]> = getRepos,
 ): Promise<Action> => {
   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;
 };

test/db/file/repo.test.js

@@ -0,0 +1,60 @@
+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',
+      };
+
+      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',
+      };
+
+      sandbox.stub(repoModule.db, 'findOne').callsFake((query, cb) => cb(null, repoData));
+
+      const result = await repoModule.getRepoByUrl('https://github.com/finos/git-proxy');
+      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',
+      };
+
+      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'}))).to.be.true;
+      expect(result).to.equal(repoData);
+    });
+  });
+});

test/db/mongo/repo.test.js

@@ -0,0 +1,85 @@
+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',
+      };
+      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',
+      };
+      repoCollection.findOne.resolves(repoData);
+
+      const result = await getRepoByUrl('https://github.com/finos/git-proxy');
+      expect(result).to.equal(repoData);
+      expect(connectionStub.calledWith('repos')).to.be.true;
+      expect(
+        repoCollection.findOne.calledWith({ url: { $eq: 'https://github.com/finos/git-proxy' } }),
+      ).to.be.true;
+    });
+
+    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',
+      };
+      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' } }),
+      ).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',
+      };
+      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' } }),
+      ).to.be.true;
+    });
+  });
+});

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');
+  });
+});