From 70376d3508d24ea4e439c1363e99cc382c8ae215 Mon Sep 17 00:00:00 2001 From: Joey Wilhelm Date: Mon, 22 Feb 2021 17:29:28 -0700 Subject: [PATCH 1/5] Add the ability to load application installations --- src/lib/loaders.js | 32 ++++++++++++++++++++++++++++++-- src/lib/typedefs.js | 17 +++++++++++++++++ 2 files changed, 47 insertions(+), 2 deletions(-) diff --git a/src/lib/loaders.js b/src/lib/loaders.js index 3d70ad2..3f87207 100644 --- a/src/lib/loaders.js +++ b/src/lib/loaders.js @@ -3,6 +3,7 @@ const fs = require('fs').promises; const { graphql } = require('@octokit/graphql'); +const { request } = require('@octokit/request'); const TOML = require('@iarna/toml'); const typedefs = require('./typedefs'); @@ -94,12 +95,39 @@ async function retrieveOrgInfo(orgName, token) { // eslint-disable-next-line id-length requiresTwoFactorAuthentication: organization.requiresTwoFactorAuthentication, twitterUsername: organization.twitterUsername, - websiteUrl: organization.websiteUrl + websiteUrl: organization.websiteUrl, + applications: await retrieveOrgApplications(orgName, token) }; console.log(`${Object.keys(result.members).length} total members retrieved.`); return result; } +/** + * Retrieve a list of all applications installed to the requested org + * + * @param {string} orgName - The login name of the org to retrieve applications for + * @param {string} token - A personal access token for interacting with the API + * @returns {typedefs.AppSet} - A list of all applications installed on the organization + */ +async function retrieveOrgApplications(orgName, token) { + const response = await request('GET /orgs/{org}/installations', { + headers: { authorization: `token ${token}` }, + org: orgName + }); + const result = response.data.installations.reduce((apps, app) => { + apps.push({ + appId: app.app_id, + appSlug: app.app_slug, + repositorySelection: app.repository_selection, + permissions: app.permissions, + events: app.events + }); + return apps; + }, []); + console.log(`Loaded ${result.length} application installations for ${orgName}.`); + return result; +} + /** * Load an org's expected configuration from a TOML config file * @@ -132,4 +160,4 @@ async function loadMembershipConfig(fileName) { return config; } -module.exports = { retrieveOrgInfo, loadMembershipConfig }; +module.exports = { retrieveOrgInfo, retrieveOrgApplications, loadMembershipConfig }; diff --git a/src/lib/typedefs.js b/src/lib/typedefs.js index fbbbd0b..5ad6bac 100644 --- a/src/lib/typedefs.js +++ b/src/lib/typedefs.js @@ -14,6 +14,23 @@ * @typedef {object.} MemberSet */ +/** + * A normalized application installation record retrieved from the GitHub REST API + * + * @typedef {object} AppRecord + * @property {number} appId - The global database identifier for the application + * @property {string} appSlug - A unique slug for identifying the application + * @property {string} repositorySelection - Whether this is installed for "all" or "selected" repositories + * @property {object.} permissions - The set of permissions granted to this application + * @property {Array.} events - A list of events that are received by this application + */ + +/** + * A set of application installation records retrieved from the GitHub REST API + * + * @typedef {Array.} AppSet + */ + /** * A normalized org record retrieved from the GitHub GraphQL API * From 41b11633f8506a4728ffd85fc0c4e147fad8ca0e Mon Sep 17 00:00:00 2001 From: Joey Wilhelm Date: Mon, 22 Feb 2021 17:30:15 -0700 Subject: [PATCH 2/5] Add tests for retrieveOrgApplications --- package-lock.json | 68 +++++++++++++++++++++++++++- package.json | 1 + test/loaders.spec.js | 104 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 172 insertions(+), 1 deletion(-) create mode 100644 test/loaders.spec.js diff --git a/package-lock.json b/package-lock.json index 32cb1eb..74bfe3b 100644 --- a/package-lock.json +++ b/package-lock.json @@ -24,6 +24,7 @@ "eslint-plugin-mocha": "^6.3.0", "eslint-plugin-node": "^11.1.0", "mocha": "^8.2.1", + "nock": "^13.0.7", "nyc": "^15.1.0", "sinon": "^9.2.4" }, @@ -787,7 +788,6 @@ "dependencies": { "anymatch": "~3.1.1", "braces": "~3.0.2", - "fsevents": "~2.1.2", "glob-parent": "~5.1.0", "is-binary-path": "~2.1.0", "is-glob": "~4.0.1", @@ -2077,6 +2077,12 @@ "integrity": "sha1-nbe1lJatPzz+8wp1FC0tkwrXJlE=", "dev": true }, + "node_modules/json-stringify-safe": { + "version": "5.0.1", + "resolved": "https://registry.npmjs.org/json-stringify-safe/-/json-stringify-safe-5.0.1.tgz", + "integrity": "sha1-Epai1Y/UXxmg9s4B1lcB4sc1tus=", + "dev": true + }, "node_modules/json5": { "version": "2.1.3", "resolved": "https://registry.npmjs.org/json5/-/json5-2.1.3.tgz", @@ -2157,6 +2163,12 @@ "integrity": "sha1-LRd/ZS+jHpObRDjVNBSZ36OCXpk=", "dev": true }, + "node_modules/lodash.set": { + "version": "4.3.2", + "resolved": "https://registry.npmjs.org/lodash.set/-/lodash.set-4.3.2.tgz", + "integrity": "sha1-2HV7HagH3eJIFrDWqEvqGnYjCyM=", + "dev": true + }, "node_modules/log-symbols": { "version": "4.0.0", "resolved": "https://registry.npmjs.org/log-symbols/-/log-symbols-4.0.0.tgz", @@ -2368,6 +2380,21 @@ "path-to-regexp": "^1.7.0" } }, + "node_modules/nock": { + "version": "13.0.7", + "resolved": "https://registry.npmjs.org/nock/-/nock-13.0.7.tgz", + "integrity": "sha512-WBz73VYIjdbO6BwmXODRQLtn7B5tldA9pNpWJe5QTtTEscQlY5KXU4srnGzBOK2fWakkXj69gfTnXGzmrsaRWw==", + "dev": true, + "dependencies": { + "debug": "^4.1.0", + "json-stringify-safe": "^5.0.1", + "lodash.set": "^4.3.2", + "propagate": "^2.0.0" + }, + "engines": { + "node": ">= 10.13" + } + }, "node_modules/node-fetch": { "version": "2.6.1", "resolved": "https://registry.npmjs.org/node-fetch/-/node-fetch-2.6.1.tgz", @@ -2852,6 +2879,15 @@ "node": ">=0.4.0" } }, + "node_modules/propagate": { + "version": "2.0.1", + "resolved": "https://registry.npmjs.org/propagate/-/propagate-2.0.1.tgz", + "integrity": "sha512-vGrhOavPSTz4QVNuBNdcNXePNdNMaO1xj9yBeH1ScQPjk/rhg9sSlCXPhMkFuaNNW/syTvYqsnbIJxMBfRbbag==", + "dev": true, + "engines": { + "node": ">= 8" + } + }, "node_modules/propget": { "version": "1.1.0", "resolved": "https://registry.npmjs.org/propget/-/propget-1.1.0.tgz", @@ -5449,6 +5485,12 @@ "integrity": "sha1-nbe1lJatPzz+8wp1FC0tkwrXJlE=", "dev": true }, + "json-stringify-safe": { + "version": "5.0.1", + "resolved": "https://registry.npmjs.org/json-stringify-safe/-/json-stringify-safe-5.0.1.tgz", + "integrity": "sha1-Epai1Y/UXxmg9s4B1lcB4sc1tus=", + "dev": true + }, "json5": { "version": "2.1.3", "resolved": "https://registry.npmjs.org/json5/-/json5-2.1.3.tgz", @@ -5513,6 +5555,12 @@ "integrity": "sha1-LRd/ZS+jHpObRDjVNBSZ36OCXpk=", "dev": true }, + "lodash.set": { + "version": "4.3.2", + "resolved": "https://registry.npmjs.org/lodash.set/-/lodash.set-4.3.2.tgz", + "integrity": "sha1-2HV7HagH3eJIFrDWqEvqGnYjCyM=", + "dev": true + }, "log-symbols": { "version": "4.0.0", "resolved": "https://registry.npmjs.org/log-symbols/-/log-symbols-4.0.0.tgz", @@ -5669,6 +5717,18 @@ "path-to-regexp": "^1.7.0" } }, + "nock": { + "version": "13.0.7", + "resolved": "https://registry.npmjs.org/nock/-/nock-13.0.7.tgz", + "integrity": "sha512-WBz73VYIjdbO6BwmXODRQLtn7B5tldA9pNpWJe5QTtTEscQlY5KXU4srnGzBOK2fWakkXj69gfTnXGzmrsaRWw==", + "dev": true, + "requires": { + "debug": "^4.1.0", + "json-stringify-safe": "^5.0.1", + "lodash.set": "^4.3.2", + "propagate": "^2.0.0" + } + }, "node-fetch": { "version": "2.6.1", "resolved": "https://registry.npmjs.org/node-fetch/-/node-fetch-2.6.1.tgz", @@ -6034,6 +6094,12 @@ "integrity": "sha512-7PiHtLll5LdnKIMw100I+8xJXR5gW2QwWYkT6iJva0bXitZKa/XMrSbdmg3r2Xnaidz9Qumd0VPaMrZlF9V9sA==", "dev": true }, + "propagate": { + "version": "2.0.1", + "resolved": "https://registry.npmjs.org/propagate/-/propagate-2.0.1.tgz", + "integrity": "sha512-vGrhOavPSTz4QVNuBNdcNXePNdNMaO1xj9yBeH1ScQPjk/rhg9sSlCXPhMkFuaNNW/syTvYqsnbIJxMBfRbbag==", + "dev": true + }, "propget": { "version": "1.1.0", "resolved": "https://registry.npmjs.org/propget/-/propget-1.1.0.tgz", diff --git a/package.json b/package.json index 4a945c6..5993ec7 100644 --- a/package.json +++ b/package.json @@ -32,6 +32,7 @@ "eslint-plugin-mocha": "^6.3.0", "eslint-plugin-node": "^11.1.0", "mocha": "^8.2.1", + "nock": "^13.0.7", "nyc": "^15.1.0", "sinon": "^9.2.4" }, diff --git a/test/loaders.spec.js b/test/loaders.spec.js new file mode 100644 index 0000000..66bd13e --- /dev/null +++ b/test/loaders.spec.js @@ -0,0 +1,104 @@ +'use strict'; + +const assume = require('assume'); +const loaders = require('../src/lib/loaders'); +const nock = require('nock'); +const sinon = require('sinon'); + + +describe('Loaders', function () { + + before(function () { + sinon.stub(console, 'log'); + }); + + after(function () { + sinon.restore(); + }); + + describe('retrieveOrgApplications', function () { + let scope; + + before(function () { + scope = nock('https://api.github.com/').get('/orgs/foo/installations').reply(200, { + total_count: 2, + installations: [ + { + id: 1, + account: {}, + repository_selection: 'all', + access_tokens_url: 'https://foo/bar/tokens', + repositories_url: 'https://foo/bar/repos/', + html_url: 'https://foo/bar/html/', + app_id: 12, + app_slug: 'heart-of-gold', + target_id: 42, + target_type: 'Organization', + permissions: { + foo: 'read', + bar: 'write', + baz: 'read' + }, + events: ['push', 'release'] + }, + { + id: 2, + account: {}, + repository_selection: 'selected', + access_tokens_url: 'https://foo/bar/tokens', + repositories_url: 'https://foo/bar/repos/', + html_url: 'https://foo/bar/html/', + app_id: 13, + app_slug: 'infinite-improbability-drive', + target_id: 42, + target_type: 'Organization', + permissions: { + foo: 'write', + bar: 'read', + baz: 'write' + }, + events: ['issue', 'discussion', 'pull_request'] + } + ] + }).persist(); + }); + + after(function () { + nock.cleanAll(); + }); + + it('calls the proper REST endpoint', async function () { + await loaders.retrieveOrgApplications('foo', '12345'); + scope.done(); + }); + + it('returns only the relevant information', async function () { + const apps = await loaders.retrieveOrgApplications('foo', '12345'); + + assume(apps).eqls([ + { + appId: 12, + appSlug: 'heart-of-gold', + repositorySelection: 'all', + permissions: { + foo: 'read', + bar: 'write', + baz: 'read' + }, + events: ['push', 'release'] + }, + { + appId: 13, + appSlug: 'infinite-improbability-drive', + repositorySelection: 'selected', + permissions: { + foo: 'write', + bar: 'read', + baz: 'write' + }, + events: ['issue', 'discussion', 'pull_request'] + } + ]); + }); + }); +}); From 8f2007e964e0431eab1800d9aa907854a7c6c7f4 Mon Sep 17 00:00:00 2001 From: Joey Wilhelm Date: Tue, 23 Feb 2021 08:33:58 -0700 Subject: [PATCH 3/5] Move the API response out to a separate JSON file --- test/loaders.spec.js | 47 ++------------ test/responses/installations.json | 100 ++++++++++++++++++++++++++++++ 2 files changed, 105 insertions(+), 42 deletions(-) create mode 100644 test/responses/installations.json diff --git a/test/loaders.spec.js b/test/loaders.spec.js index 66bd13e..36ced9b 100644 --- a/test/loaders.spec.js +++ b/test/loaders.spec.js @@ -1,8 +1,10 @@ 'use strict'; const assume = require('assume'); +const fs = require('fs').promises; const loaders = require('../src/lib/loaders'); const nock = require('nock'); +const path = require('path'); const sinon = require('sinon'); @@ -19,48 +21,9 @@ describe('Loaders', function () { describe('retrieveOrgApplications', function () { let scope; - before(function () { - scope = nock('https://api.github.com/').get('/orgs/foo/installations').reply(200, { - total_count: 2, - installations: [ - { - id: 1, - account: {}, - repository_selection: 'all', - access_tokens_url: 'https://foo/bar/tokens', - repositories_url: 'https://foo/bar/repos/', - html_url: 'https://foo/bar/html/', - app_id: 12, - app_slug: 'heart-of-gold', - target_id: 42, - target_type: 'Organization', - permissions: { - foo: 'read', - bar: 'write', - baz: 'read' - }, - events: ['push', 'release'] - }, - { - id: 2, - account: {}, - repository_selection: 'selected', - access_tokens_url: 'https://foo/bar/tokens', - repositories_url: 'https://foo/bar/repos/', - html_url: 'https://foo/bar/html/', - app_id: 13, - app_slug: 'infinite-improbability-drive', - target_id: 42, - target_type: 'Organization', - permissions: { - foo: 'write', - bar: 'read', - baz: 'write' - }, - events: ['issue', 'discussion', 'pull_request'] - } - ] - }).persist(); + before(async function () { + const response = JSON.parse(await fs.readFile(path.resolve(__dirname, 'responses/installations.json'))); + scope = nock('https://api.github.com/').get('/orgs/foo/installations').reply(200, response).persist(); }); after(function () { diff --git a/test/responses/installations.json b/test/responses/installations.json new file mode 100644 index 0000000..04536d5 --- /dev/null +++ b/test/responses/installations.json @@ -0,0 +1,100 @@ +{ + "total_count": 2, + "installations": [ + { + "id": 1, + "account": { + "login": "foo", + "id": 42, + "node_id": "", + "avatar_url": "https://avatars.githubusercontent.com/u/42?v=4", + "gravatar_id": "", + "url": "https://api.github.com/users/foo", + "html_url": "https://github.com/foo", + "followers_url": "https://api.github.com/users/foo/followers", + "following_url": "https://api.github.com/users/foo/following{/other_user}", + "gists_url": "https://api.github.com/users/foo/gists{/gist_id}", + "starred_url": "https://api.github.com/users/foo/starred{/owner}{/repo}", + "subscriptions_url": "https://api.github.com/users/foo/subscriptions", + "organizations_url": "https://api.github.com/users/foo/orgs", + "repos_url": "https://api.github.com/users/foo/repos", + "events_url": "https://api.github.com/users/foo/events{/privacy}", + "received_events_url": "https://api.github.com/users/foo/received_events", + "type": "Organization", + "site_admin": false + }, + "repository_selection": "all", + "access_tokens_url": "https://api.github.com/app/installations/1/access_tokens", + "repositories_url": "https://api.github.com/installation/repositories", + "html_url": "https://github.com/organizations/foo/settings/installations/1", + "app_id": 12, + "app_slug": "heart-of-gold", + "target_id": 42, + "target_type": "Organization", + "permissions": { + "foo": "read", + "bar": "write", + "baz": "read" + }, + "events": [ + "push", + "release" + ], + "created_at": "2017-08-31T16:13:51.000-04:00", + "updated_at": "2021-02-15T17:09:55.000-05:00", + "single_file_name": null, + "has_multiple_single_files": false, + "single_file_paths": [], + "suspended_by": null, + "suspended_at": null + }, + { + "id": 2, + "account": { + "login": "foo", + "id": 42, + "node_id": "", + "avatar_url": "https://avatars.githubusercontent.com/u/42?v=4", + "gravatar_id": "", + "url": "https://api.github.com/users/foo", + "html_url": "https://github.com/foo", + "followers_url": "https://api.github.com/users/foo/followers", + "following_url": "https://api.github.com/users/foo/following{/other_user}", + "gists_url": "https://api.github.com/users/foo/gists{/gist_id}", + "starred_url": "https://api.github.com/users/foo/starred{/owner}{/repo}", + "subscriptions_url": "https://api.github.com/users/foo/subscriptions", + "organizations_url": "https://api.github.com/users/foo/orgs", + "repos_url": "https://api.github.com/users/foo/repos", + "events_url": "https://api.github.com/users/foo/events{/privacy}", + "received_events_url": "https://api.github.com/users/foo/received_events", + "type": "Organization", + "site_admin": false + }, + "repository_selection": "selected", + "access_tokens_url": "https://api.github.com/app/installations/2/access_tokens", + "repositories_url": "https://api.github.com/installation/repositories", + "html_url": "https://github.com/organizations/foo/settings/installations/2", + "app_id": 13, + "app_slug": "infinite-improbability-drive", + "target_id": 42, + "target_type": "Organization", + "permissions": { + "foo": "write", + "bar": "read", + "baz": "write" + }, + "events": [ + "issue", + "discussion", + "pull_request" + ], + "created_at": "2018-01-04T19:32:28.000-05:00", + "updated_at": "2021-02-11T18:15:47.000-05:00", + "single_file_name": null, + "has_multiple_single_files": false, + "single_file_paths": [], + "suspended_by": null, + "suspended_at": null + } + ] +} From 0cfa87d7d7c32500792d9596f3f14658570f3f55 Mon Sep 17 00:00:00 2001 From: Joey Wilhelm Date: Tue, 23 Feb 2021 08:38:52 -0700 Subject: [PATCH 4/5] Ensure the authorization header is sent --- test/loaders.spec.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/test/loaders.spec.js b/test/loaders.spec.js index 36ced9b..7e09f32 100644 --- a/test/loaders.spec.js +++ b/test/loaders.spec.js @@ -23,7 +23,10 @@ describe('Loaders', function () { before(async function () { const response = JSON.parse(await fs.readFile(path.resolve(__dirname, 'responses/installations.json'))); - scope = nock('https://api.github.com/').get('/orgs/foo/installations').reply(200, response).persist(); + scope = nock( + 'https://api.github.com/', + { reqheaders: { authorization: 'token 12345' } } + ).get('/orgs/foo/installations').reply(200, response).persist(); }); after(function () { From c684dd9f0c731aa63a1b3dcf223839dfa1ebfdb6 Mon Sep 17 00:00:00 2001 From: Joey Wilhelm Date: Tue, 23 Feb 2021 09:03:19 -0700 Subject: [PATCH 5/5] Add documentation for the new feature --- CHANGELOG.md | 1 + README.md | 41 +++++++++++++++++++++++++++++++++++++++++ example.toml | 13 +++++++++++++ 3 files changed, 55 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index ba7fb39..76c88f2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ - Fixed #16 - New users were causing an error in findPromotions - Fixed #20 - Added full JSDoc coverage across the code +- Fixed #23 - Added auditing of application installations ## 0.1.0 diff --git a/README.md b/README.md index d10a94d..d5721c6 100644 --- a/README.md +++ b/README.md @@ -124,6 +124,47 @@ does have the side effect that, if an internal username is connected to multiple GitHub usernames, as demonstrated above, all of those GitHub usernames will be made admins of the org. +### [[applications]] + +Each `[[applications]]` section represents one org-level application +installation. These entries are used to ensure that no applications get added to +or removed from the org unexpectedly. You can also ensure that they only have +the permissions you are expecting, and receive the events you are expecting. + +Similar to the `[[teams]]` above, applications are specified as an [array of +tables]. In addition, the applications use a sub-table for the permissions, +making a single application specification look like the following: + +```toml +[[applications]] +appId = 42 +appSlug = "infinite-improbability-drive" +repositorySelection = "selected" +events = ["issues", "discussion", "pull_request"] + +[applications.permissions] +# This applies to the application directly above, infinite-improbability-drive +checks = "read" +issues = "read" +metadata = "read" +deployments = "write" +``` + +Given the flexibility of TOML, you can also specify the `permissions` table +in-line, depending on your preference. That would look like this: + +```toml +[[applications]] +appId = 42 +appSlug = "infinite-improbability-drive" +repositorySelection = "selected" +events = ["issues", "discussion", "pull_request"] +permissions = {checks = "read", issues = "read", metadata = "read", deployments = "write"} +``` + +Both of the above are wholly equivalent; it's simply a matter of preference +which way you would like to specify them. + ## Development ### Running Tests diff --git a/example.toml b/example.toml index db94cda..e0de3e0 100644 --- a/example.toml +++ b/example.toml @@ -19,3 +19,16 @@ name = "Heart of Gold Crew" default_org_role = "ADMIN" privacy = "SECRET" members = ["mandroid", "tmcmillan", "zbeeblebrox"] + +[[applications]] +appId = 42 +appSlug = "infinite-improbability-drive" +repositorySelection = "selected" +events = ["issues", "discussion", "pull_request"] + +[applications.permissions] +# This applies to the application directly above, infinite-improbability-drive +checks = "read" +issues = "read" +metadata = "read" +deployments = "write"