feat(aio): verify uploaded builds based on JWT from Travis

aio/aio-builds-setup/dockerbuild/Dockerfile

@@ -13,6 +13,8 @@ EXPOSE 80 443
 
 ENV AIO_BUILDS_DIR=/var/www/aio-builds             TEST_AIO_BUILDS_DIR=/tmp/aio-builds         \
     AIO_DOMAIN_NAME=ngbuilds.io                    TEST_AIO_DOMAIN_NAME=test-ngbuilds.io       \
+    AIO_GITHUB_ORGANIZATION=angular                TEST_AIO_GITHUB_ORGANIZATION=angular        \
+    AIO_GITHUB_TEAM_SLUGS=angular-core             TEST_AIO_GITHUB_TEAM_SLUGS=angular-core     \
     AIO_NGINX_HOSTNAME=nginx.localhost             TEST_AIO_NGINX_HOSTNAME=nginx.localhost     \
     AIO_NGINX_PORT_HTTP=80                         TEST_AIO_NGINX_PORT_HTTP=8080               \
     AIO_NGINX_PORT_HTTPS=443                       TEST_AIO_NGINX_PORT_HTTPS=4433              \

aio/aio-builds-setup/dockerbuild/scripts-js/lib/common/github-pull-requests.ts

@@ -3,8 +3,9 @@ import {assertNotMissingOrEmpty} from '../common/utils';
 import {GithubApi} from './github-api';
 
 // Interfaces - Types
-interface PullRequest  {
+export interface PullRequest  {
   number: number;
+  user: {login: string};
 }
 
 export type PullRequestState = 'all' | 'closed' | 'open';
@@ -28,6 +29,10 @@ export class GithubPullRequests extends GithubApi {
     return this.post<void>(`/repos/${this.repoSlug}/issues/${pr}/comments`, null, {body});
   }
 
+  public fetch(pr: number): Promise<PullRequest> {
+    return this.get<PullRequest>(`/repos/${this.repoSlug}/pulls/${pr}`);
+  }
+
   public fetchAll(state: PullRequestState = 'all'): Promise<PullRequest[]> {
     console.log(`Fetching ${state} pull requests...`);
 

aio/aio-builds-setup/dockerbuild/scripts-js/lib/upload-server/index.ts

@@ -2,15 +2,16 @@
 process.setuid('www-data');
 
 // Imports
-import {GithubPullRequests} from '../common/github-pull-requests';
 import {getEnvVar} from '../common/utils';
-import {CreatedBuildEvent} from './build-events';
 import {uploadServerFactory} from './upload-server-factory';
 
 // Constants
 const AIO_BUILDS_DIR = getEnvVar('AIO_BUILDS_DIR');
-const AIO_GITHUB_TOKEN = getEnvVar('AIO_GITHUB_TOKEN', true);
+const AIO_GITHUB_ORGANIZATION = getEnvVar('AIO_GITHUB_ORGANIZATION');
-const AIO_REPO_SLUG = getEnvVar('AIO_REPO_SLUG', true);
+const AIO_GITHUB_TEAM_SLUGS = getEnvVar('AIO_GITHUB_TEAM_SLUGS');
+const AIO_GITHUB_TOKEN = getEnvVar('AIO_GITHUB_TOKEN');
+const AIO_PREVIEW_DEPLOYMENT_TOKEN = getEnvVar('AIO_PREVIEW_DEPLOYMENT_TOKEN');
+const AIO_REPO_SLUG = getEnvVar('AIO_REPO_SLUG');
 const AIO_UPLOAD_HOSTNAME = getEnvVar('AIO_UPLOAD_HOSTNAME');
 const AIO_UPLOAD_PORT = +getEnvVar('AIO_UPLOAD_PORT');
 
@@ -20,23 +21,13 @@ _main();
 // Functions
 function _main() {
   uploadServerFactory.
-    create(AIO_BUILDS_DIR).
+    create({
-    on(CreatedBuildEvent.type, createOnBuildCreatedHanlder()).
+      buildsDir: AIO_BUILDS_DIR,
+      githubOrganization: AIO_GITHUB_ORGANIZATION,
+      githubTeamSlugs: AIO_GITHUB_TEAM_SLUGS.split(','),
+      githubToken: AIO_GITHUB_TOKEN,
+      repoSlug: AIO_REPO_SLUG,
+      secret: AIO_PREVIEW_DEPLOYMENT_TOKEN,
+    }).
     listen(AIO_UPLOAD_PORT, AIO_UPLOAD_HOSTNAME);
 }
-
-function createOnBuildCreatedHanlder() {
-  if (!AIO_REPO_SLUG) {
-    console.warn('No repo specified. Preview links will not be posted on PRs.');
-    return () => null;
-  }
-
-  const githubPullRequests = new GithubPullRequests(AIO_REPO_SLUG, AIO_GITHUB_TOKEN);
-
-  return ({pr, sha}: CreatedBuildEvent) => {
-    const body = `The angular.io preview for ${sha.slice(0, 7)} is available [here][1].\n\n` +
-                 `[1]: https://pr${pr}-${sha}.ngbuilds.io/`;
-
-    githubPullRequests.addComment(pr, body);
-  };
-}

aio/aio-builds-setup/dockerbuild/scripts-js/lib/upload-server/upload-server-factory.ts

@@ -1,25 +1,43 @@
 // Imports
 import * as express from 'express';
 import * as http from 'http';
-import {assertNotMissingOrEmpty} from '../common/utils';
+import {GithubPullRequests} from '../common/github-pull-requests';
 import {BuildCreator} from './build-creator';
 import {CreatedBuildEvent} from './build-events';
+import {BuildVerifier} from './build-verifier';
 import {UploadError} from './upload-error';
 
 // Constants
+const AUTHORIZATION_HEADER = 'AUTHORIZATION';
 const X_FILE_HEADER = 'X-FILE';
 
+// Interfaces - Types
+interface UploadServerConfig {
+  buildsDir: string;
+  githubOrganization: string;
+  githubTeamSlugs: string[];
+  githubToken: string;
+  repoSlug: string;
+  secret: string;
+}
+
 // Classes
 class UploadServerFactory {
   // Methods - Public
-  public create(buildsDir: string): http.Server {
+  public create({
-    assertNotMissingOrEmpty('buildsDir', buildsDir);
+    buildsDir,
+    githubOrganization,
+    githubTeamSlugs,
+    githubToken,
+    repoSlug,
+    secret,
+  }: UploadServerConfig): http.Server {
+    const buildVerifier = new BuildVerifier(secret, githubToken, repoSlug, githubOrganization, githubTeamSlugs);
+    const buildCreator = this.createBuildCreator(buildsDir, githubToken, repoSlug);
 
-    const buildCreator = new BuildCreator(buildsDir);
+    const middleware = this.createMiddleware(buildVerifier, buildCreator);
-    const middleware = this.createMiddleware(buildCreator);
     const httpServer = http.createServer(middleware);
 
-    buildCreator.on(CreatedBuildEvent.type, (data: CreatedBuildEvent) => httpServer.emit(CreatedBuildEvent.type, data));
     httpServer.on('listening', () => {
       const info = httpServer.address();
       console.info(`Up and running (and listening on ${info.address}:${info.port})...`);
@@ -29,20 +47,38 @@ class UploadServerFactory {
   }
 
   // Methods - Protected
-  protected createMiddleware(buildCreator: BuildCreator): express.Express {
+  protected createBuildCreator(buildsDir: string, githubToken: string, repoSlug: string): BuildCreator {
+    const buildCreator = new BuildCreator(buildsDir);
+    const githubPullRequests = new GithubPullRequests(githubToken, repoSlug);
+
+    buildCreator.on(CreatedBuildEvent.type, ({pr, sha}: CreatedBuildEvent) => {
+      const body = `The angular.io preview for ${sha.slice(0, 7)} is available [here][1].\n\n` +
+                   `[1]: https://pr${pr}-${sha}.ngbuilds.io/`;
+
+      githubPullRequests.addComment(pr, body);
+    });
+
+    return buildCreator;
+  }
+
+  protected createMiddleware(buildVerifier: BuildVerifier, buildCreator: BuildCreator): express.Express {
     const middleware = express();
 
     middleware.get(/^\/create-build\/([1-9][0-9]*)\/([0-9a-f]{40})\/?$/, (req, res) => {
       const pr = req.params[0];
       const sha = req.params[1];
       const archive = req.header(X_FILE_HEADER);
+      const authHeader = req.header(AUTHORIZATION_HEADER);
 
-      if (!archive) {
+      if (!authHeader) {
+        this.throwRequestError(401, `Missing or empty '${AUTHORIZATION_HEADER}' header`, req);
+      } else if (!archive) {
         this.throwRequestError(400, `Missing or empty '${X_FILE_HEADER}' header`, req);
       }
 
-      buildCreator.
+      buildVerifier.
-        create(pr, sha, archive).
+        verify(+pr, authHeader).
+        then(() => buildCreator.create(pr, sha, archive)).
         then(() => res.sendStatus(201)).
         catch(err => this.respondWithError(res, err));
     });

aio/aio-builds-setup/dockerbuild/scripts-js/test/upload-server/upload-server-factory.spec.ts

@@ -2,12 +2,27 @@
 import * as express from 'express';
 import * as http from 'http';
 import * as supertest from 'supertest';
+import {GithubPullRequests} from '../../lib/common/github-pull-requests';
 import {BuildCreator} from '../../lib/upload-server/build-creator';
 import {CreatedBuildEvent} from '../../lib/upload-server/build-events';
+import {BuildVerifier} from '../../lib/upload-server/build-verifier';
 import {uploadServerFactory as usf} from '../../lib/upload-server/upload-server-factory';
 
 // Tests
 describe('uploadServerFactory', () => {
+  const defaultConfig = {
+    buildsDir: 'builds/dir',
+    githubOrganization: 'organization',
+    githubTeamSlugs: ['team1', 'team2'],
+    githubToken: '12345',
+    repoSlug: 'repo/slug',
+    secret: 'secret',
+  };
+
+  // Helpers
+  const createUploadServer = (partialConfig: Partial<typeof defaultConfig> = {}) =>
+    usf.create({...defaultConfig, ...partialConfig});
+
 
   describe('create()', () => {
     let usfCreateMiddlewareSpy: jasmine.Spy;
@@ -17,55 +32,77 @@ describe('uploadServerFactory', () => {
     });
 
 
-    it('should throw if \'buildsDir\' is empty', () => {
+    it('should throw if \'secret\' is missing or empty', () => {
-      expect(() => usf.create('')).toThrowError('Missing or empty required parameter \'buildsDir\'!');
+      expect(() => createUploadServer({secret: ''})).
+        toThrowError('Missing or empty required parameter \'secret\'!');
+    });
+
+
+    it('should throw if \'githubToken\' is missing or empty', () => {
+      expect(() => createUploadServer({githubToken: ''})).
+        toThrowError('Missing or empty required parameter \'githubToken\'!');
+    });
+
+
+    it('should throw if \'githubOrganization\' is missing or empty', () => {
+      expect(() => createUploadServer({githubOrganization: ''})).
+        toThrowError('Missing or empty required parameter \'organization\'!');
+    });
+
+
+    it('should throw if \'githubTeamSlugs\' is missing or empty', () => {
+      expect(() => createUploadServer({githubTeamSlugs: []})).
+        toThrowError('Missing or empty required parameter \'allowedTeamSlugs\'!');
+    });
+
+
+    it('should throw if \'repoSlug\' is missing or empty', () => {
+      expect(() => createUploadServer({repoSlug: ''})).
+        toThrowError('Missing or empty required parameter \'repoSlug\'!');
+    });
+
+
+    it('should throw if \'secret\' is missing or empty', () => {
+      expect(() => createUploadServer({secret: ''})).
+        toThrowError('Missing or empty required parameter \'secret\'!');
     });
 
 
     it('should return an http.Server', () => {
       const httpCreateServerSpy = spyOn(http, 'createServer').and.callThrough();
-      const server = usf.create('builds/dir');
+      const server = createUploadServer();
 
       expect(server).toBe(httpCreateServerSpy.calls.mostRecent().returnValue);
     });
 
 
+    it('should create and use an appropriate BuildCreator', () => {
+      const usfCreateBuildCreatorSpy = spyOn(usf as any, 'createBuildCreator').and.callThrough();
+
+      createUploadServer();
+      const buildCreator: BuildCreator = usfCreateBuildCreatorSpy.calls.mostRecent().returnValue;
+
+      expect(usfCreateMiddlewareSpy).toHaveBeenCalledWith(jasmine.any(BuildVerifier), buildCreator);
+      expect(usfCreateBuildCreatorSpy).toHaveBeenCalledWith('builds/dir', '12345', 'repo/slug');
+    });
+
+
     it('should create and use an appropriate middleware', () => {
       const httpCreateServerSpy = spyOn(http, 'createServer').and.callThrough();
 
-      usf.create('builds/dir');
+      createUploadServer();
       const middleware: express.Express = usfCreateMiddlewareSpy.calls.mostRecent().returnValue;
+      const buildVerifier = jasmine.any(BuildVerifier);
+      const buildCreator = jasmine.any(BuildCreator);
 
-      expect(usfCreateMiddlewareSpy).toHaveBeenCalledWith(jasmine.any(BuildCreator));
       expect(httpCreateServerSpy).toHaveBeenCalledWith(middleware);
-    });
+      expect(usfCreateMiddlewareSpy).toHaveBeenCalledWith(buildVerifier, buildCreator);
-
-
-    it('should pass \'buildsDir\' to the created BuildCreator', () => {
-      usf.create('builds/dir');
-      const buildCreator: BuildCreator = usfCreateMiddlewareSpy.calls.argsFor(0)[0];
-
-      expect((buildCreator as any).buildsDir).toBe('builds/dir');
-    });
-
-
-    it('should pass CreatedBuildEvents emitted on BuildCreator through to the server', done => {
-      const server = usf.create('builds/dir');
-      const buildCreator: BuildCreator = usfCreateMiddlewareSpy.calls.argsFor(0)[0];
-      const evt = new CreatedBuildEvent(42, 'foo');
-
-      server.on(CreatedBuildEvent.type, (data: CreatedBuildEvent) => {
-        expect(data).toBe(evt);
-        done();
-      });
-
-      buildCreator.emit(CreatedBuildEvent.type, evt);
     });
 
 
     it('should log the server address info on \'listening\'', () => {
       const consoleInfoSpy = spyOn(console, 'info');
-      const server = usf.create('builds/dir');
+      const server = createUploadServer('builds/dir');
       server.address = () => ({address: 'foo', family: '', port: 1337});
 
       expect(consoleInfoSpy).not.toHaveBeenCalled();
@@ -79,7 +116,50 @@ describe('uploadServerFactory', () => {
 
   // Protected methods
 
+  describe('createBuildCreator()', () => {
+    let buildCreator: BuildCreator;
+
+    beforeEach(() => {
+      buildCreator = (usf as any).createBuildCreator(
+        defaultConfig.buildsDir,
+        defaultConfig.githubToken,
+        defaultConfig.repoSlug,
+      );
+    });
+
+
+    it('should pass the \'buildsDir\' to the BuildCreator', () => {
+      expect((buildCreator as any).buildsDir).toBe('builds/dir');
+    });
+
+
+    it('should post a comment on GitHub on \'build.created\'', () => {
+      const prsAddCommentSpy = spyOn(GithubPullRequests.prototype, 'addComment');
+      const commentBody = 'The angular.io preview for 1234567 is available [here][1].\n\n' +
+                          '[1]: https://pr42-1234567890.ngbuilds.io/';
+
+      buildCreator.emit(CreatedBuildEvent.type, {pr: 42, sha: '1234567890'});
+
+      expect(prsAddCommentSpy).toHaveBeenCalledWith(42, commentBody);
+    });
+
+
+    it('should pass the correct \'githubToken\' and \'repoSlug\' to GithubPullRequests', () => {
+      const prsAddCommentSpy = spyOn(GithubPullRequests.prototype, 'addComment');
+
+      buildCreator.emit(CreatedBuildEvent.type, {pr: 42, sha: '1234567890'});
+      const prs = prsAddCommentSpy.calls.mostRecent().object;
+
+      expect(prs).toEqual(jasmine.any(GithubPullRequests));
+      expect((prs as any).repoSlug).toBe('repo/slug');
+      expect((prs as any).requestHeaders.Authorization).toContain('12345');
+    });
+
+  });
+
+
   describe('createMiddleware()', () => {
+    let buildVerifier: BuildVerifier;
     let buildCreator: BuildCreator;
     let agent: supertest.SuperTest<supertest.Test>;
 
@@ -90,8 +170,15 @@ describe('uploadServerFactory', () => {
       Promise.all(reqs.map(promisifyRequest)).then(done, done.fail);
 
     beforeEach(() => {
-      buildCreator = new BuildCreator('builds/dir');
+      buildVerifier = new BuildVerifier(
-      agent = supertest.agent((usf as any).createMiddleware(buildCreator));
+        defaultConfig.secret,
+        defaultConfig.githubToken,
+        defaultConfig.repoSlug,
+        defaultConfig.githubOrganization,
+        defaultConfig.githubTeamSlugs,
+      );
+      buildCreator = new BuildCreator(defaultConfig.buildsDir);
+      agent = supertest.agent((usf as any).createMiddleware(buildVerifier, buildCreator));
 
       spyOn(console, 'error');
     });
@@ -100,14 +187,12 @@ describe('uploadServerFactory', () => {
     describe('GET /create-build/<pr>/<sha>', () => {
       const pr = '9';
       const sha = '9'.repeat(40);
+      let buildVerifierVerifySpy: jasmine.Spy;
       let buildCreatorCreateSpy: jasmine.Spy;
-      let deferred: {resolve: Function, reject: Function};
 
       beforeEach(() => {
-        const promise = new Promise((resolve, reject) => deferred = {resolve, reject});
+        buildVerifierVerifySpy = spyOn(buildVerifier, 'verify').and.returnValue(Promise.resolve());
-        promise.catch(() => null);   // Avoid "unhandled rejection" warnings.
+        buildCreatorCreateSpy = spyOn(buildCreator, 'create').and.returnValue(Promise.resolve());
-
-        buildCreatorCreateSpy = spyOn(buildCreator, 'create').and.returnValue(promise);
       });
 
 
@@ -121,13 +206,27 @@ describe('uploadServerFactory', () => {
       });
 
 
+      it('should respond with 401 for requests without an \'AUTHORIZATION\' header', done => {
+        const url = `/create-build/${pr}/${sha}`;
+        const responseBody = `Missing or empty 'AUTHORIZATION' header in request: GET ${url}`;
+
+        verifyRequests([
+          agent.get(url).expect(401, responseBody),
+          agent.get(url).set('AUTHORIZATION', '').expect(401, responseBody),
+        ], done);
+      });
+
+
       it('should respond with 400 for requests without an \'X-FILE\' header', done => {
         const url = `/create-build/${pr}/${sha}`;
         const responseBody = `Missing or empty 'X-FILE' header in request: GET ${url}`;
 
+        const request1 = agent.get(url).set('AUTHORIZATION', 'foo');
+        const request2 = agent.get(url).set('AUTHORIZATION', 'foo').set('X-FILE', '');
+
         verifyRequests([
-          agent.get(url).expect(400, responseBody),
+          request1.expect(400, responseBody),
-          agent.get(url).field('X-FILE', '').expect(400, responseBody),
+          request2.expect(400, responseBody),
         ], done);
       });
 
@@ -146,34 +245,68 @@ describe('uploadServerFactory', () => {
       });
 
 
-      it('should propagate errors from BuildCreator', done => {
+      it('should call \'BuildVerifier#verify()\' with the correct arguments', done => {
         const req = agent.
           get(`/create-build/${pr}/${sha}`).
-          set('X-FILE', 'foo').
+          set('AUTHORIZATION', 'foo').
+          set('X-FILE', 'bar');
+
+        promisifyRequest(req).
+          then(() => expect(buildVerifierVerifySpy).toHaveBeenCalledWith(9, 'foo')).
+          then(done, done.fail);
+      });
+
+
+      it('should propagate errors from BuildVerifier', done => {
+        buildVerifierVerifySpy.and.callFake(() => Promise.reject('Test'));
+
+        const req = agent.
+          get(`/create-build/${pr}/${sha}`).
+          set('AUTHORIZATION', 'foo').
+          set('X-FILE', 'bar').
+          expect(500, 'Test');
+
+        promisifyRequest(req).
+          then(() => {
+            expect(buildVerifierVerifySpy).toHaveBeenCalledWith(9, 'foo');
+            expect(buildCreatorCreateSpy).not.toHaveBeenCalled();
+          }).
+          then(done, done.fail);
+      });
+
+
+      it('should call \'BuildCreator#create()\' with the correct arguments', done => {
+        const req = agent.
+          get(`/create-build/${pr}/${sha}`).
+          set('AUTHORIZATION', 'foo').
+          set('X-FILE', 'bar');
+
+        promisifyRequest(req).
+          then(() => expect(buildCreatorCreateSpy).toHaveBeenCalledWith(pr, sha, 'bar')).
+          then(done, done.fail);
+      });
+
+
+      it('should propagate errors from BuildCreator', done => {
+        buildCreatorCreateSpy.and.callFake(() => Promise.reject('Test'));
+        const req = agent.
+          get(`/create-build/${pr}/${sha}`).
+          set('AUTHORIZATION', 'foo').
+          set('X-FILE', 'bar').
           expect(500, 'Test');
 
         verifyRequests([req], done);
-        deferred.reject('Test');
       });
 
 
       it('should respond with 201 on successful upload', done => {
         const req = agent.
           get(`/create-build/${pr}/${sha}`).
-          set('X-FILE', 'foo').
+          set('AUTHORIZATION', 'foo').
+          set('X-FILE', 'bar').
           expect(201, http.STATUS_CODES[201]);
 
         verifyRequests([req], done);
-        deferred.resolve();
-      });
-
-
-      it('should call \'BuildCreator#create()\' with appropriate arguments', done => {
-        promisifyRequest(agent.get(`/create-build/${pr}/${sha}`).set('X-FILE', 'foo').expect(201)).
-          then(() => expect(buildCreatorCreateSpy).toHaveBeenCalledWith(pr, sha, 'foo')).
-          then(done, done.fail);
-
-        deferred.resolve();
       });
 
 
@@ -183,16 +316,18 @@ describe('uploadServerFactory', () => {
 
 
       it('should accept SHAs with leading zeros (but not ignore them)', done => {
+        const sha41 = '0'.repeat(41);
         const sha40 = '0'.repeat(40);
-        const sha41 = `0${sha40}`;
+
+        const request41 = agent.get(`/create-build/${pr}/${sha41}`);
+        const request40 = agent.get(`/create-build/${pr}/${sha40}`).
+          set('AUTHORIZATION', 'foo').
+          set('X-FILE', 'bar');
 
         Promise.all([
-          promisifyRequest(agent.get(`/create-build/${pr}/${sha41}`).expect(404)),
+          promisifyRequest(request41.expect(404)),
-          promisifyRequest(agent.get(`/create-build/${pr}/${sha40}`).set('X-FILE', 'foo')).
+          promisifyRequest(request40.expect(201)),
-            then(() => expect(buildCreatorCreateSpy).toHaveBeenCalledWith(pr, sha40, 'foo')),
         ]).then(done, done.fail);
-
-        deferred.resolve();
       });
 
     });

aio/aio-builds-setup/dockerbuild/scripts-sh/upload-server-test.sh

@@ -3,6 +3,9 @@ set -e -o pipefail
 
 # Set up env variables for testing
 export AIO_BUILDS_DIR=$TEST_AIO_BUILDS_DIR
+export AIO_GITHUB_ORGANIZATION=$TEST_AIO_GITHUB_ORGANIZATION
+export AIO_GITHUB_TEAM_SLUGS=$TEST_AIO_GITHUB_TEAM_SLUGS
+export AIO_PREVIEW_DEPLOYMENT_TOKEN=$TEST_AIO_PREVIEW_DEPLOYMENT_TOKEN
 export AIO_REPO_SLUG=$TEST_AIO_REPO_SLUG
 export AIO_UPLOAD_HOSTNAME=$TEST_AIO_UPLOAD_HOSTNAME
 export AIO_UPLOAD_PORT=$TEST_AIO_UPLOAD_PORT