From dd98e2501c213b433366e800887555d536fe05d6 Mon Sep 17 00:00:00 2001 From: Alexander Lee Date: Fri, 9 Sep 2022 10:41:55 +0800 Subject: [PATCH] Feat/site member verification for email (#479) * Feat: add IsomerAdmins database table and migrations * Feat: add access token via interceptor if missing * Feat: add isomerAdminsService * Feat: add hasAccessToSite to usersService * Feat: shift site membership check to authorizationMiddlewareService * Chore: replace authMiddleware.checkHasAccess with authorizationMiddleware.checkIsSiteMember * Chore: migrate authmiddlewareservice to typescript * Fix: rename auth middleware to authentication middleware * Fix: move e2e_isomer_id into constants * Chore: add cookie types * Fix: more concise check for isSiteMember * FIx: rebase errors * Fix: remove unused identityAuthService dependency * Fix: rename AuthService import as identityAuthService * Nit: separate type definition * Feat/email login flow (#480) * build(deps): bump file-type from 16.5.3 to 16.5.4 (#475) Bumps [file-type](https://github.com/sindresorhus/file-type) from 16.5.3 to 16.5.4. - [Release notes](https://github.com/sindresorhus/file-type/releases) - [Commits](https://github.com/sindresorhus/file-type/compare/v16.5.3...v16.5.4) --- updated-dependencies: - dependency-name: file-type dependency-type: direct:production ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * fix: package.json & package-lock.json to reduce vulnerabilities (#476) The following vulnerabilities are fixed with an upgrade: - https://snyk.io/vuln/SNYK-JS-SEQUELIZE-2959225 * build(deps): bump vm2 from 3.9.5 to 3.9.7 (#350) Bumps [vm2](https://github.com/patriksimek/vm2) from 3.9.5 to 3.9.7. - [Release notes](https://github.com/patriksimek/vm2/releases) - [Changelog](https://github.com/patriksimek/vm2/blob/master/CHANGELOG.md) - [Commits](https://github.com/patriksimek/vm2/compare/3.9.5...3.9.7) --- updated-dependencies: - dependency-name: vm2 dependency-type: indirect ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Chore: remove site links from description (#482) * Fix: update resource room (#481) * 0.10.0 * fix: remove unnecessary update step (#487) * 0.10.1 * Chore: update commit message to include user id * Feat: add login and verify endpoints * Fix: model relations and alias * Feat: add findSitesByUserId * Feat: add site retrieval for email and admin users * Fix: hasAccessToSite * Fix: update email/mobile by isomer id * Chore: update error message * Fix: await check for whitelist * Chore: add mockSessionData for email login * Fix: SiteService behaviour for email users with no whitelisted sites * Test: update sitesservice tests * Test: add new authservice tests and fix existing tests * Fix: update user model to allow null in github field * Fix: update test fixture * Fix: update user test suite * Chore: remove unused endpoint * Fix: rebase errors * Chore: remove unnecessary message in test * Chore: remove unnecessary userId field * Nit: rename variable * Refactor: shift site retrieval for email users into helper method * Chore: spacing and remove unused var * Fix: tests * Tests: add new authorizationMiddlewareService test * fix: remove resources_name and add support for url (#490) * fix: remove resources_name and add support for url * fix: display url parameter as domain but store with https scheme * fix: resolve failing tests * Chore: flip conditional * Refactor: shift order of getSites to make it easier to understand * Test: add new auth router tests * Feat: add integration tests for getSites * Fix: failing requests for getLastUpdated and getStagingUrl * Nit: add comment * Nit: test name and var name * chore(mocks/axios): remove extra stuff * test(sites.spec): refactor specs for clarity * Fix: update settings * Nit: update comment * Fix: tests Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Snyk bot Co-authored-by: Hsu Zhong Jun <27919917+dcshzj@users.noreply.github.com> Co-authored-by: seaerchin Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Snyk bot Co-authored-by: Hsu Zhong Jun <27919917+dcshzj@users.noreply.github.com> Co-authored-by: seaerchin --- CHANGELOG.md | 18 ++ package-lock.json | 70 ++++--- package.json | 6 +- src/__mocks__/axios.ts | 3 - src/constants/constants.ts | 2 + .../20220726094614-create-isomer-admin.js | 36 ++++ ...03091224-change-users-github-allow-null.js | 23 +++ src/database/models/IsomerAdmin.ts | 32 +++ src/database/models/Site.ts | 5 +- src/database/models/User.ts | 9 +- src/database/models/index.ts | 1 + src/fixtures/config.js | 3 +- src/fixtures/sessionData.ts | 11 +- src/integration/Sites.spec.ts | 192 ++++++++++++++++++ src/integration/Users.spec.ts | 41 ++-- src/middleware/{auth.ts => authentication.ts} | 23 +-- src/middleware/authorization.ts | 38 ++++ src/middleware/index.ts | 52 +++-- src/routes/v1/auth.js | 4 +- src/routes/v1/authenticated/index.js | 7 +- src/routes/v1/authenticatedSites/index.js | 9 +- src/routes/v2/__tests__/Auth.spec.js | 25 ++- src/routes/v2/auth.js | 35 +++- .../v2/authenticated/__tests__/Sites.spec.js | 43 +--- src/routes/v2/authenticated/index.js | 12 +- src/routes/v2/authenticated/sites.js | 45 ++-- src/routes/v2/authenticated/users.ts | 4 +- src/routes/v2/authenticatedSites/index.js | 7 +- src/routes/v2/authenticatedSites/settings.js | 3 +- src/server.js | 29 ++- src/services/api/AxiosInstance.ts | 16 +- .../configServices/SettingsService.js | 33 +-- .../__tests__/SettingsService.spec.js | 18 +- src/services/db/GitHubService.js | 39 ++-- .../db/__tests__/GitHubService.spec.js | 32 ++- .../ResourceRoomDirectoryService.js | 12 +- src/services/identity/IsomerAdminsService.ts | 27 +++ src/services/identity/ReposService.ts | 6 - src/services/identity/UsersService.ts | 60 +++++- src/services/identity/index.ts | 7 +- ....js => AuthenticationMiddlewareService.ts} | 66 +++--- .../AuthorizationMiddlewareService.ts | 58 ++++++ .../AuthorizationMiddlewareService.spec.ts | 122 +++++++++++ src/services/utilServices/AuthService.js | 54 ++++- src/services/utilServices/SitesService.js | 37 +++- .../__tests__/AuthService.spec.js | 89 +++++++- .../__tests__/SitesService.spec.js | 135 +++++++++++- src/tests/database.ts | 2 + src/validators/RequestSchema.js | 1 + 49 files changed, 1315 insertions(+), 287 deletions(-) create mode 100644 src/database/migrations/20220726094614-create-isomer-admin.js create mode 100644 src/database/migrations/20220803091224-change-users-github-allow-null.js create mode 100644 src/database/models/IsomerAdmin.ts create mode 100644 src/integration/Sites.spec.ts rename src/middleware/{auth.ts => authentication.ts} (52%) create mode 100644 src/middleware/authorization.ts create mode 100644 src/services/identity/IsomerAdminsService.ts rename src/services/middlewareServices/{AuthMiddlewareService.js => AuthenticationMiddlewareService.ts} (66%) create mode 100644 src/services/middlewareServices/AuthorizationMiddlewareService.ts create mode 100644 src/services/middlewareServices/__tests__/AuthorizationMiddlewareService.spec.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index fc6f62a3f..fc0822d32 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,8 +4,26 @@ All notable changes to this project will be documented in this file. Dates are d Generated by [`auto-changelog`](https://github.com/CookPete/auto-changelog). +#### [v0.10.1](https://github.com/isomerpages/isomercms-backend/compare/v0.10.0...v0.10.1) + +- fix: remove unnecessary update step [`#487`](https://github.com/isomerpages/isomercms-backend/pull/487) +- 0.10.0 [`#484`](https://github.com/isomerpages/isomercms-backend/pull/484) + +#### [v0.10.0](https://github.com/isomerpages/isomercms-backend/compare/v0.9.0...v0.10.0) + +> 11 August 2022 + +- Fix: update resource room [`#481`](https://github.com/isomerpages/isomercms-backend/pull/481) +- Chore: remove site links from description [`#482`](https://github.com/isomerpages/isomercms-backend/pull/482) +- build(deps): bump vm2 from 3.9.5 to 3.9.7 [`#350`](https://github.com/isomerpages/isomercms-backend/pull/350) +- fix: package.json & package-lock.json to reduce vulnerabilities [`#476`](https://github.com/isomerpages/isomercms-backend/pull/476) +- build(deps): bump file-type from 16.5.3 to 16.5.4 [`#475`](https://github.com/isomerpages/isomercms-backend/pull/475) +- 0.9.0 [`#473`](https://github.com/isomerpages/isomercms-backend/pull/473) + #### [v0.9.0](https://github.com/isomerpages/isomercms-backend/compare/v0.8.0...v0.9.0) +> 14 July 2022 + - Misc Backend Cleanup 1 [`#470`](https://github.com/isomerpages/isomercms-backend/pull/470) - 0.8.0 [`#468`](https://github.com/isomerpages/isomercms-backend/pull/468) diff --git a/package-lock.json b/package-lock.json index 46b226cc5..e84efaf5c 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,6 +1,6 @@ { "name": "isomercms", - "version": "0.9.0", + "version": "0.10.1", "lockfileVersion": 1, "requires": true, "dependencies": { @@ -7147,9 +7147,9 @@ } }, "file-type": { - "version": "16.5.3", - "resolved": "https://registry.npmjs.org/file-type/-/file-type-16.5.3.tgz", - "integrity": "sha512-uVsl7iFhHSOY4bEONLlTK47iAHtNsFHWP5YE4xJfZ4rnX7S1Q3wce09XgqSC7E/xh8Ncv/be1lNoyprlUH/x6A==", + "version": "16.5.4", + "resolved": "https://registry.npmjs.org/file-type/-/file-type-16.5.4.tgz", + "integrity": "sha512-/yFHK0aGjFEgDJjEKP0pWCplsPFPhwyfwevf/pVxiN0tmE4L9LmwWxWukdJSHdoCli4VgQLehjJtwQBnqmsKcw==", "requires": { "readable-web-to-node-stream": "^3.0.0", "strtok3": "^6.2.4", @@ -11494,9 +11494,9 @@ "dev": true }, "peek-readable": { - "version": "4.0.1", - "resolved": "https://registry.npmjs.org/peek-readable/-/peek-readable-4.0.1.tgz", - "integrity": "sha512-7qmhptnR0WMSpxT5rMHG9bW/mYSR1uqaPFj2MHvT+y/aOUu6msJijpKt5SkTDKySwg65OWG2JwTMBlgcbwMHrQ==" + "version": "4.1.0", + "resolved": "https://registry.npmjs.org/peek-readable/-/peek-readable-4.1.0.tgz", + "integrity": "sha512-ZI3LnwUv5nOGbQzD9c2iDG6toheuXSZP5esSHBjopsXH4dg19soufvpUGA3uohi5anFtGb2lhAVdHzH6R/Evvg==" }, "performance-now": { "version": "2.1.0", @@ -12094,9 +12094,9 @@ } }, "sequelize": { - "version": "6.17.0", - "resolved": "https://registry.npmjs.org/sequelize/-/sequelize-6.17.0.tgz", - "integrity": "sha512-AZus+0YZDq91Zg0hzDaO5atTzHgJruI23V8nBlAhkLuI81Z53nSRdAe/4R1A6vGOZ/RfCLP9idF4tfQnoAsM5A==", + "version": "6.21.2", + "resolved": "https://registry.npmjs.org/sequelize/-/sequelize-6.21.2.tgz", + "integrity": "sha512-K0c6j/Y6yfucBL9XYHMVWqYGFShPsj6ZzMrQcOAjqzyE+a1XMBOoTXXjRvJS+fz6cKeh2D3ZqhYDRwN8nfvOMQ==", "requires": { "@types/debug": "^4.1.7", "@types/validator": "^13.7.1", @@ -12117,9 +12117,9 @@ }, "dependencies": { "debug": { - "version": "4.3.3", - "resolved": "https://registry.npmjs.org/debug/-/debug-4.3.3.tgz", - "integrity": "sha512-/zxw5+vh1Tfv+4Qn7a5nsbcJKPaSvCDhojn6FEl9vupwK2VCSDtEiEtqr8DFtzYFOdz63LBkxec7DYuc2jon6Q==", + "version": "4.3.4", + "resolved": "https://registry.npmjs.org/debug/-/debug-4.3.4.tgz", + "integrity": "sha512-PRWFHuSU3eDtQJPvnNY7Jcket1j0t5OuOsFzPPzsekD52Zl8qUfFIPEiswXqIvHWGVHOgX+7G/vCNNhehwxfkQ==", "requires": { "ms": "2.1.2" } @@ -12138,9 +12138,9 @@ "integrity": "sha512-sGkPx+VjMtmA6MX27oA4FBFELFCZZ4S4XqeGOXCv68tT+jb3vk/RyaKWP0PTKyWtmLSM0b+adUTEvbs1PEaH2w==" }, "semver": { - "version": "7.3.5", - "resolved": "https://registry.npmjs.org/semver/-/semver-7.3.5.tgz", - "integrity": "sha512-PoeGJYh8HK4BTO/a9Tf6ZG3veo/A7ZVsYrSA6J8ny9nb3B1VrpkuN+z9OE5wfE5p6H4LchYZsegiQgbJD94ZFQ==", + "version": "7.3.7", + "resolved": "https://registry.npmjs.org/semver/-/semver-7.3.7.tgz", + "integrity": "sha512-QlYTucUYOews+WeEujDoEGziz4K6c47V/Bd+LjSSYcA94p+DmINdf7ncaUinThfvZyu13lN9OY1XDxt8C0Tw0g==", "requires": { "lru-cache": "^6.0.0" } @@ -12621,12 +12621,12 @@ "dev": true }, "strtok3": { - "version": "6.2.4", - "resolved": "https://registry.npmjs.org/strtok3/-/strtok3-6.2.4.tgz", - "integrity": "sha512-GO8IcFF9GmFDvqduIspUBwCzCbqzegyVKIsSymcMgiZKeCfrN9SowtUoi8+b59WZMAjIzVZic/Ft97+pynR3Iw==", + "version": "6.3.0", + "resolved": "https://registry.npmjs.org/strtok3/-/strtok3-6.3.0.tgz", + "integrity": "sha512-fZtbhtvI9I48xDSywd/somNqgUHl2L2cstmXCCif0itOf96jeW18MBSyrLuNicYQVkvpOxkZtkzujiTJ9LW5Jw==", "requires": { "@tokenizer/token": "^0.3.0", - "peek-readable": "^4.0.1" + "peek-readable": "^4.1.0" } }, "superagent": { @@ -12951,9 +12951,9 @@ "integrity": "sha512-yaOH/Pk/VEhBWWTlhI+qXxDFXlejDGcQipMlyxda9nthulaxLZUNcUqFxokp0vcYnvteJln5FNQDRrxj3YcbVw==" }, "token-types": { - "version": "4.1.1", - "resolved": "https://registry.npmjs.org/token-types/-/token-types-4.1.1.tgz", - "integrity": "sha512-hD+QyuUAyI2spzsI0B7gf/jJ2ggR4RjkAo37j3StuePhApJUwcWDjnHDOFdIWYSwNR28H14hpwm4EI+V1Ted1w==", + "version": "4.2.0", + "resolved": "https://registry.npmjs.org/token-types/-/token-types-4.2.0.tgz", + "integrity": "sha512-P0rrp4wUpefLncNamWIef62J0v0kQR/GfDVji9WKY7GDCWy5YbVSrKUTam07iWPZQGy0zWNOfstYTykMmPNR7w==", "requires": { "@tokenizer/token": "^0.3.0", "ieee754": "^1.2.1" @@ -12974,7 +12974,7 @@ "toposort-class": { "version": "1.0.1", "resolved": "https://registry.npmjs.org/toposort-class/-/toposort-class-1.0.1.tgz", - "integrity": "sha1-f/0feMi+KMO6Rc1OGj9e4ZO9mYg=" + "integrity": "sha512-OsLcGGbYF3rMjPUf8oKktyvCiUxSbqMMS39m33MAjLTC1DVIH6x3WSt63/M77ihI09+Sdfk1AXvfhCEeUmC7mg==" }, "tough-cookie": { "version": "4.0.0", @@ -13433,9 +13433,25 @@ } }, "vm2": { - "version": "3.9.5", - "resolved": "https://registry.npmjs.org/vm2/-/vm2-3.9.5.tgz", - "integrity": "sha512-LuCAHZN75H9tdrAiLFf030oW7nJV5xwNMuk1ymOZwopmuK3d2H4L1Kv4+GFHgarKiLfXXLFU+7LDABHnwOkWng==" + "version": "3.9.7", + "resolved": "https://registry.npmjs.org/vm2/-/vm2-3.9.7.tgz", + "integrity": "sha512-g/GZ7V0Mlmch3eDVOATvAXr1GsJNg6kQ5PjvYy3HbJMCRn5slNbo/u73Uy7r5yUej1cRa3ZjtoVwcWSQuQ/fow==", + "requires": { + "acorn": "^8.7.0", + "acorn-walk": "^8.2.0" + }, + "dependencies": { + "acorn": { + "version": "8.7.0", + "resolved": "https://registry.npmjs.org/acorn/-/acorn-8.7.0.tgz", + "integrity": "sha512-V/LGr1APy+PXIwKebEWrkZPwoeoF+w1jiOBUmuxuiUIaOHtob8Qc9BTrYo7VuI5fR8tqsy+buA2WFooR5olqvQ==" + }, + "acorn-walk": { + "version": "8.2.0", + "resolved": "https://registry.npmjs.org/acorn-walk/-/acorn-walk-8.2.0.tgz", + "integrity": "sha512-k+iyHEuPgSw6SbuDpGQM+06HQUa04DZ3o+F6CSzXMvvI5KMvnaEqXe+YVe555R9nn6GPt404fos4wcgpw12SDA==" + } + } }, "w3c-hr-time": { "version": "1.0.2", diff --git a/package.json b/package.json index e75e81532..b28cc7279 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "isomercms", - "version": "0.9.0", + "version": "0.10.1", "private": true, "scripts": { "build": "tsc -p tsconfig.build.json", @@ -41,7 +41,7 @@ "dotenv": "^16.0.1", "exponential-backoff": "^3.1.0", "express": "~4.16.1", - "file-type": "^16.5.3", + "file-type": "^16.5.4", "helmet": "^4.6.0", "http-errors": "~1.8.0", "is-svg": "^4.3.1", @@ -60,7 +60,7 @@ "pg-connection-string": "^2.5.0", "query-string": "^6.14.1", "reflect-metadata": "^0.1.13", - "sequelize": "^6.17.0", + "sequelize": "^6.21.2", "sequelize-typescript": "^2.1.3", "serialize-error": "^7.0.1", "slugify": "^1.6.0", diff --git a/src/__mocks__/axios.ts b/src/__mocks__/axios.ts index 4f63804a7..e6c7a7f22 100644 --- a/src/__mocks__/axios.ts +++ b/src/__mocks__/axios.ts @@ -1,6 +1,3 @@ import mockAxios from "jest-mock-axios" -mockAxios.interceptors.request.use(jest.fn()) -mockAxios.interceptors.response.use(jest.fn()) - export default mockAxios diff --git a/src/constants/constants.ts b/src/constants/constants.ts index 0f453726d..32fceecce 100644 --- a/src/constants/constants.ts +++ b/src/constants/constants.ts @@ -9,3 +9,5 @@ export enum SiteStatus { Initialized = "INITIALIZED", Launched = "LAUNCHED", } + +export const E2E_ISOMER_ID = "e2e-id" diff --git a/src/database/migrations/20220726094614-create-isomer-admin.js b/src/database/migrations/20220726094614-create-isomer-admin.js new file mode 100644 index 000000000..c47293c5c --- /dev/null +++ b/src/database/migrations/20220726094614-create-isomer-admin.js @@ -0,0 +1,36 @@ +module.exports = { + up: async (queryInterface, Sequelize) => { + await queryInterface.createTable("isomer_admins", { + id: { + allowNull: false, + autoIncrement: true, + primaryKey: true, + type: Sequelize.BIGINT, + }, + user_id: { + allowNull: false, + type: Sequelize.BIGINT, + references: { + model: "users", + key: "id", + }, + onUpdate: "CASCADE", + onDelete: "CASCADE", + }, + created_at: { + type: Sequelize.DATE, + allowNull: false, + defaultValue: Sequelize.fn("NOW"), + }, + updated_at: { + type: Sequelize.DATE, + allowNull: false, + defaultValue: Sequelize.fn("NOW"), + }, + }) + }, + + down: async (queryInterface) => { + await queryInterface.dropTable("isomer_admins") + }, +} diff --git a/src/database/migrations/20220803091224-change-users-github-allow-null.js b/src/database/migrations/20220803091224-change-users-github-allow-null.js new file mode 100644 index 000000000..3ee65c69f --- /dev/null +++ b/src/database/migrations/20220803091224-change-users-github-allow-null.js @@ -0,0 +1,23 @@ +module.exports = { + async up(queryInterface, Sequelize) { + await queryInterface.changeColumn("users", "github_id", { + allowNull: true, + unique: true, + type: Sequelize.TEXT, + validate: { + notEmpty: true, + }, + }) + }, + + async down(queryInterface, Sequelize) { + await queryInterface.changeColumn("users", "github_id", { + allowNull: false, + unique: true, + type: Sequelize.TEXT, + validate: { + notEmpty: true, + }, + }) + }, +} diff --git a/src/database/models/IsomerAdmin.ts b/src/database/models/IsomerAdmin.ts new file mode 100644 index 000000000..a6ee22c1b --- /dev/null +++ b/src/database/models/IsomerAdmin.ts @@ -0,0 +1,32 @@ +import { + Column, + CreatedAt, + DataType, + ForeignKey, + Model, + Table, + UpdatedAt, +} from "sequelize-typescript" + +import { User } from "@database/models/User" + +@Table({ tableName: "isomer_admins" }) +export class IsomerAdmin extends Model { + @Column({ + autoIncrement: true, + primaryKey: true, + allowNull: false, + type: DataType.BIGINT, + }) + id!: number + + @ForeignKey(() => User) + @Column + userId!: number + + @CreatedAt + createdAt!: Date + + @UpdatedAt + updatedAt!: Date +} diff --git a/src/database/models/Site.ts b/src/database/models/Site.ts index 030634e2c..8ab687cec 100644 --- a/src/database/models/Site.ts +++ b/src/database/models/Site.ts @@ -68,6 +68,7 @@ export class Site extends Model { onUpdate: "CASCADE", onDelete: "CASCADE", through: () => SiteMember, + as: "site_members", }) users!: User[] @@ -80,6 +81,8 @@ export class Site extends Model { @ForeignKey(() => User) creatorId!: number - @BelongsTo(() => User) + @BelongsTo(() => User, { + as: "site_creator", + }) creator!: User } diff --git a/src/database/models/User.ts b/src/database/models/User.ts index ae56766f3..e5eb64c6f 100644 --- a/src/database/models/User.ts +++ b/src/database/models/User.ts @@ -31,7 +31,7 @@ export class User extends Model { email?: string | null @Column({ - allowNull: false, + allowNull: true, unique: true, type: DataType.TEXT, validate: { @@ -61,13 +61,16 @@ export class User extends Model { @DeletedAt deletedAt?: Date - @BelongsToMany(() => User, { + @BelongsToMany(() => Site, { onUpdate: "CASCADE", onDelete: "CASCADE", through: () => SiteMember, + as: "site_members", }) sites!: Site[] - @HasMany(() => Site) + @HasMany(() => Site, { + as: "sites_created", + }) sitesCreated?: Site[] } diff --git a/src/database/models/index.ts b/src/database/models/index.ts index 630e4c4a9..db6d5e783 100644 --- a/src/database/models/index.ts +++ b/src/database/models/index.ts @@ -5,3 +5,4 @@ export * from "@database/models/Whitelist" export * from "@database/models/AccessToken" export * from "@database/models/Repo" export * from "@database/models/Deployment" +export * from "@database/models/IsomerAdmin" diff --git a/src/fixtures/config.js b/src/fixtures/config.js index f4f51ac2d..23c30a7e2 100644 --- a/src/fixtures/config.js +++ b/src/fixtures/config.js @@ -118,7 +118,7 @@ const configContent = { const configSha = "configsha" const configResponse = { - url: configContent.url, + url: configContent.url.replace("https://", ""), title: configContent.title, description: configContent.description, favicon: configContent.favicon, @@ -127,7 +127,6 @@ const configResponse = { facebook_pixel: configContent["facebook-pixel"], google_analytics: configContent.google_analytics, linkedin_insights: configContent["linkedin-insights"], - resources_name: configContent.resources_name, colors: configContent.colors, } diff --git a/src/fixtures/sessionData.ts b/src/fixtures/sessionData.ts index 1ff193dd0..756ddc107 100644 --- a/src/fixtures/sessionData.ts +++ b/src/fixtures/sessionData.ts @@ -4,7 +4,7 @@ import UserWithSiteSessionData from "@root/classes/UserWithSiteSessionData" export const mockAccessToken = "mockAccessToken" export const mockGithubId = "mockGithubId" -export const mockIsomerUserId = "mockIsomerUserId" +export const mockIsomerUserId = "1" export const mockEmail = "mockEmail" export const mockTreeSha = "mockTreeSha" export const mockCurrentCommitSha = "mockCurrentCommitSha" @@ -34,3 +34,12 @@ export const mockGithubSessionData = new GithubSessionData({ treeSha: mockTreeSha, currentCommitSha: mockCurrentCommitSha, }) +export const mockSessionDataEmailUser = new UserSessionData({ + isomerUserId: mockIsomerUserId, + email: mockEmail, +}) +export const mockSessionDataEmailUserWithSite = new UserWithSiteSessionData({ + isomerUserId: mockIsomerUserId, + email: mockEmail, + siteName: mockSiteName, +}) diff --git a/src/integration/Sites.spec.ts b/src/integration/Sites.spec.ts new file mode 100644 index 000000000..1b65302b3 --- /dev/null +++ b/src/integration/Sites.spec.ts @@ -0,0 +1,192 @@ +import express from "express" +import mockAxios from "jest-mock-axios" +import request from "supertest" + +import { IsomerAdmin, Repo, Site, SiteMember, User } from "@database/models" +import { generateRouter } from "@fixtures/app" +import UserSessionData from "@root/classes/UserSessionData" +import { mockEmail, mockIsomerUserId } from "@root/fixtures/sessionData" +import { SitesRouter as _SitesRouter } from "@root/routes/v2/authenticated/sites" +import { GitHubService } from "@root/services/db/GitHubService" +import { ConfigYmlService } from "@root/services/fileServices/YmlFileServices/ConfigYmlService" +import IsomerAdminsService from "@root/services/identity/IsomerAdminsService" +import { SitesService } from "@root/services/utilServices/SitesService" +import { getUsersService } from "@services/identity" +import { sequelize } from "@tests/database" + +const mockSite = "mockSite" +const mockSiteId = "1" +const mockAdminSite = "adminOnly" +const mockUpdatedAt = "now" +const mockPermissions = { push: true } +const mockPrivate = true + +const gitHubService = new GitHubService({ axiosInstance: mockAxios.create() }) +const configYmlService = new ConfigYmlService({ gitHubService }) +const usersService = getUsersService(sequelize) +const isomerAdminsService = new IsomerAdminsService({ repository: IsomerAdmin }) +const sitesService = new SitesService({ + gitHubService, + configYmlService, + usersService, + isomerAdminsService, +}) + +const SitesRouter = new _SitesRouter({ sitesService }) +const sitesSubrouter = SitesRouter.getRouter() + +// Set up express with defaults and use the router under test +const subrouter = express() +// As we set certain properties on res.locals when the user signs in using github +// In order to do integration testing, we must expose a middleware +// that allows us to set this properties also +subrouter.use((req, res, next) => { + const userSessionData = new UserSessionData({ + isomerUserId: mockIsomerUserId, + email: mockEmail, + }) + res.locals.userSessionData = userSessionData + next() +}) +subrouter.use(sitesSubrouter) +const app = generateRouter(subrouter) + +const mockGenericAxios = mockAxios.create() +mockGenericAxios.get.mockResolvedValue({ + data: [], +}) + +describe("Sites Router", () => { + beforeAll(() => { + // NOTE: Because SitesService uses an axios instance, + // we need to mock the axios instance using es5 named exports + // to ensure that the calls for .get() on the instance + // will actually return a value and not fail. + jest.mock("../services/api/AxiosInstance.ts", () => ({ + __esModule: true, // this property makes it work + genericGitHubAxiosInstance: mockGenericAxios, + })) + }) + + describe("/", () => { + beforeAll(async () => { + // Set up User and Site table entries + await User.create({ + id: mockIsomerUserId, + }) + await Site.create({ + id: mockSiteId, + name: mockSite, + apiTokenName: "token", + jobStatus: "READY", + siteStatus: "LAUNCHED", + creatorId: mockIsomerUserId, + }) + await Site.create({ + id: "200", + name: mockAdminSite, + apiTokenName: "token", + jobStatus: "READY", + siteStatus: "LAUNCHED", + creatorId: mockIsomerUserId, + }) + await SiteMember.create({ + userId: mockIsomerUserId, + siteId: mockSiteId, + role: "ADMIN", + }) + await Repo.create({ + name: mockSite, + url: "url", + siteId: mockSiteId, + }) + }) + afterEach(async () => { + // Clean up so that different tests using + // the same mock user don't interfere with each other + await IsomerAdmin.destroy({ + where: { userId: mockIsomerUserId }, + }) + }) + it("should return list of only sites available to email user", async () => { + // Arrange + const expected = { + siteNames: [ + { + lastUpdated: mockUpdatedAt, + repoName: mockSite, + isPrivate: mockPrivate, + permissions: mockPermissions, + }, + ], + } + + mockGenericAxios.get.mockResolvedValueOnce({ + data: [ + { + pushed_at: mockUpdatedAt, + permissions: mockPermissions, + name: mockSite, + private: mockPrivate, + }, + { + pushed_at: mockUpdatedAt, + permissions: mockPermissions, + name: mockAdminSite, + private: mockPrivate, + }, + ], + }) + + // Act + const actual = await request(app).get("/") + + // Assert + expect(actual.body).toMatchObject(expected) + }) + it("should return list of all sites available for admin", async () => { + // Arrange + await IsomerAdmin.create({ + userId: mockIsomerUserId, + }) + const expected = { + siteNames: [ + { + lastUpdated: mockUpdatedAt, + repoName: mockSite, + isPrivate: mockPrivate, + permissions: mockPermissions, + }, + { + lastUpdated: mockUpdatedAt, + repoName: mockAdminSite, + isPrivate: mockPrivate, + permissions: mockPermissions, + }, + ], + } + mockGenericAxios.get.mockResolvedValueOnce({ + data: [ + { + pushed_at: mockUpdatedAt, + permissions: mockPermissions, + name: mockSite, + private: mockPrivate, + }, + { + pushed_at: mockUpdatedAt, + permissions: mockPermissions, + name: mockAdminSite, + private: mockPrivate, + }, + ], + }) + + // Act + const actual = await request(app).get("/") + + // Assert + expect(actual.body).toMatchObject(expected) + }) + }) +}) diff --git a/src/integration/Users.spec.ts b/src/integration/Users.spec.ts index 17372535e..cb638e3a1 100644 --- a/src/integration/Users.spec.ts +++ b/src/integration/Users.spec.ts @@ -5,6 +5,7 @@ import request from "supertest" import { User, Whitelist } from "@database/models" import { generateRouter } from "@fixtures/app" import UserSessionData from "@root/classes/UserSessionData" +import { mockIsomerUserId } from "@root/fixtures/sessionData" import { UsersRouter as _UsersRouter } from "@root/routes/v2/authenticated/users" import { getUsersService } from "@services/identity" import { sequelize } from "@tests/database" @@ -136,7 +137,7 @@ describe("Users Router", () => { // Clean up so that different tests using // the same mock user don't interfere with each other await User.destroy({ - where: { githubId: mockGithubId }, + where: { id: mockIsomerUserId }, force: true, // hard delete user record to prevent the unique constraint from being violated }) await Whitelist.destroy({ @@ -152,7 +153,7 @@ describe("Users Router", () => { otp = extractEmailOtp(email.body) return email }) - await User.create({ githubId: mockGithubId }) + await User.create({ id: mockIsomerUserId }) await Whitelist.create({ email: mockWhitelistedDomain }) await request(app).post("/email/otp").send({ email: mockValidEmail, @@ -162,11 +163,11 @@ describe("Users Router", () => { const actual = await request(app).post("/email/verifyOtp").send({ email: mockValidEmail, otp, - userId: mockGithubId, + userId: mockIsomerUserId, }) const updatedUser = await User.findOne({ where: { - githubId: mockGithubId, + id: mockIsomerUserId, }, }) @@ -180,7 +181,7 @@ describe("Users Router", () => { const expected = 400 const wrongOtp = 123456 mockAxios.post.mockResolvedValueOnce(200) - await User.create({ githubId: mockGithubId }) + await User.create({ id: mockIsomerUserId }) await request(app).post("/email/otp").send({ email: mockValidEmail, }) @@ -189,7 +190,7 @@ describe("Users Router", () => { const actual = await request(app).post("/email/verifyOtp").send({ email: mockValidEmail, otp: wrongOtp, - userId: mockGithubId, + userId: mockIsomerUserId, }) // Assert @@ -200,7 +201,7 @@ describe("Users Router", () => { // Arrange const expected = 400 mockAxios.post.mockResolvedValueOnce(200) - await User.create({ githubId: mockGithubId }) + await User.create({ id: mockIsomerUserId }) await request(app).post("/email/otp").send({ email: mockValidEmail, }) @@ -209,7 +210,7 @@ describe("Users Router", () => { const actual = await request(app).post("/email/verifyOtp").send({ email: mockValidEmail, otp: "", - userId: mockGithubId, + userId: mockIsomerUserId, }) // Assert @@ -220,7 +221,7 @@ describe("Users Router", () => { // Arrange const expected = 400 mockAxios.post.mockResolvedValueOnce(200) - await User.create({ githubId: mockGithubId }) + await User.create({ id: mockIsomerUserId }) await request(app).post("/email/otp").send({ email: mockValidEmail, }) @@ -229,7 +230,7 @@ describe("Users Router", () => { const actual = await request(app).post("/email/verifyOtp").send({ email: mockValidEmail, otp: undefined, - userId: mockGithubId, + userId: mockIsomerUserId, }) // Assert @@ -293,7 +294,7 @@ describe("Users Router", () => { // Clean up so that different tests using // the same mock user don't interfere with each other await User.destroy({ - where: { githubId: mockGithubId }, + where: { id: mockIsomerUserId }, force: true, // hard delete user record to prevent the unique constraint from being violated }) }) @@ -306,7 +307,7 @@ describe("Users Router", () => { otp = extractMobileOtp(sms.body) return sms }) - await User.create({ githubId: mockGithubId }) + await User.create({ id: mockIsomerUserId }) await request(app).post("/mobile/otp").send({ mobile: mockValidNumber, }) @@ -315,11 +316,11 @@ describe("Users Router", () => { const actual = await request(app).post("/mobile/verifyOtp").send({ mobile: mockValidNumber, otp, - userId: mockGithubId, + userId: mockIsomerUserId, }) const updatedUser = await User.findOne({ where: { - githubId: mockGithubId, + id: mockIsomerUserId, }, }) @@ -333,7 +334,7 @@ describe("Users Router", () => { const expected = 400 const wrongOtp = 123456 mockAxios.post.mockResolvedValueOnce(200) - await User.create({ githubId: mockGithubId }) + await User.create({ id: mockIsomerUserId }) await request(app).post("/mobile/otp").send({ mobile: mockValidNumber, }) @@ -342,7 +343,7 @@ describe("Users Router", () => { const actual = await request(app).post("/mobile/verifyOtp").send({ mobile: mockValidNumber, otp: wrongOtp, - userId: mockGithubId, + userId: mockIsomerUserId, }) // Assert @@ -353,7 +354,7 @@ describe("Users Router", () => { // Arrange const expected = 400 mockAxios.post.mockResolvedValueOnce(200) - await User.create({ githubId: mockGithubId }) + await User.create({ id: mockIsomerUserId }) await request(app).post("/mobile/otp").send({ mobile: mockValidNumber, }) @@ -362,7 +363,7 @@ describe("Users Router", () => { const actual = await request(app).post("/mobile/verifyOtp").send({ mobile: mockValidNumber, otp: "", - userId: mockGithubId, + userId: mockIsomerUserId, }) // Assert @@ -373,7 +374,7 @@ describe("Users Router", () => { // Arrange const expected = 400 mockAxios.post.mockResolvedValueOnce(200) - await User.create({ githubId: mockGithubId }) + await User.create({ id: mockIsomerUserId }) await request(app).post("/mobile/otp").send({ mobile: mockValidNumber, }) @@ -382,7 +383,7 @@ describe("Users Router", () => { const actual = await request(app).post("/mobile/verifyOtp").send({ mobile: mockValidNumber, otp: undefined, - userId: mockGithubId, + userId: mockIsomerUserId, }) // Assert diff --git a/src/middleware/auth.ts b/src/middleware/authentication.ts similarity index 52% rename from src/middleware/auth.ts rename to src/middleware/authentication.ts index 0d17eb3db..cfbb36b9a 100644 --- a/src/middleware/auth.ts +++ b/src/middleware/authentication.ts @@ -2,17 +2,17 @@ import autoBind from "auto-bind" import { NextFunction, Request, Response } from "express" import UserSessionData from "@root/classes/UserSessionData" -import { AuthMiddlewareService } from "@root/services/middlewareServices/AuthMiddlewareService" +import AuthenticationMiddlewareService from "@root/services/middlewareServices/AuthenticationMiddlewareService" -export class AuthMiddleware { - private readonly authMiddlewareService: AuthMiddlewareService +export class AuthenticationMiddleware { + private readonly authenticationMiddlewareService: AuthenticationMiddlewareService constructor({ - authMiddlewareService, + authenticationMiddlewareService, }: { - authMiddlewareService: AuthMiddlewareService + authenticationMiddlewareService: AuthenticationMiddlewareService }) { - this.authMiddlewareService = authMiddlewareService + this.authenticationMiddlewareService = authenticationMiddlewareService // We need to bind all methods because we don't invoke them from the class directly autoBind(this) } @@ -24,7 +24,7 @@ export class AuthMiddleware { githubId, isomerUserId, email, - } = this.authMiddlewareService.verifyJwt({ + } = this.authenticationMiddlewareService.verifyJwt({ cookies, url, }) @@ -37,13 +37,4 @@ export class AuthMiddleware { res.locals.userSessionData = userSessionData return next() } - - // Replace access token with site access token if it is available - async checkHasAccess(req: Request, res: Response, next: NextFunction) { - const { userSessionData } = res.locals - - await this.authMiddlewareService.checkHasAccess(userSessionData) - - return next() - } } diff --git a/src/middleware/authorization.ts b/src/middleware/authorization.ts new file mode 100644 index 000000000..777409aa2 --- /dev/null +++ b/src/middleware/authorization.ts @@ -0,0 +1,38 @@ +import autoBind from "auto-bind" +import { NextFunction, Request, Response } from "express" + +import UserWithSiteSessionData from "@classes/UserWithSiteSessionData" + +import { RequestHandler } from "@root/types" +import AuthorizationMiddlewareService from "@services/middlewareServices/AuthorizationMiddlewareService" + +export class AuthorizationMiddleware { + private readonly authorizationMiddlewareService: AuthorizationMiddlewareService + + constructor({ + authorizationMiddlewareService, + }: { + authorizationMiddlewareService: AuthorizationMiddlewareService + }) { + this.authorizationMiddlewareService = authorizationMiddlewareService + // We need to bind all methods because we don't invoke them from the class directly + autoBind(this) + } + + // Check whether a user is a site member + checkIsSiteMember: RequestHandler< + never, + unknown, + unknown, + never, + { userWithSiteSessionData: UserWithSiteSessionData } + > = async (req, res, next) => { + const { userWithSiteSessionData } = res.locals + + await this.authorizationMiddlewareService.checkIsSiteMember( + userWithSiteSessionData + ) + + return next() + } +} diff --git a/src/middleware/index.ts b/src/middleware/index.ts index c6de13d83..ea6a7fbf6 100644 --- a/src/middleware/index.ts +++ b/src/middleware/index.ts @@ -1,31 +1,46 @@ import FormSG from "@opengovsg/formsg-sdk" -import express, { - NextFunction, - Request, - Response, - RequestHandler as ExpressRequestHandler, -} from "express" +import express, { RequestHandler as ExpressRequestHandler } from "express" -import { AuthMiddleware } from "@middleware/auth" +import { AuthenticationMiddleware } from "@middleware/authentication" +import { AuthorizationMiddleware } from "@middleware/authorization" import UserSessionData from "@classes/UserSessionData" +import UserWithSiteSessionData from "@classes/UserWithSiteSessionData" -import UserWithSiteSessionData from "@root/classes/UserWithSiteSessionData" import { RequestHandler } from "@root/types" -import AuthService from "@services/identity/AuthService" -import { AuthMiddlewareService } from "@services/middlewareServices/AuthMiddlewareService" +import IdentityAuthService from "@services/identity/AuthService" +import IsomerAdminsService from "@services/identity/IsomerAdminsService" +import UsersService from "@services/identity/UsersService" +import AuthenticationMiddlewareService from "@services/middlewareServices/AuthenticationMiddlewareService" +import AuthorizationMiddlewareService from "@services/middlewareServices/AuthorizationMiddlewareService" import FormsProcessingService from "@services/middlewareServices/FormsProcessingService" -const getAuthMiddleware = ({ +const getAuthenticationMiddleware = () => { + const authenticationMiddlewareService = new AuthenticationMiddlewareService() + const authenticationMiddleware = new AuthenticationMiddleware({ + authenticationMiddlewareService, + }) + return authenticationMiddleware +} + +const getAuthorizationMiddleware = ({ identityAuthService, + usersService, + isomerAdminsService, }: { - identityAuthService: AuthService + identityAuthService: IdentityAuthService + usersService: UsersService + isomerAdminsService: IsomerAdminsService }) => { - const authMiddlewareService = new AuthMiddlewareService({ + const authorizationMiddlewareService = new AuthorizationMiddlewareService({ identityAuthService, + usersService, + isomerAdminsService, }) - const authMiddleware = new AuthMiddleware({ authMiddlewareService }) - return authMiddleware + const authorizationMiddleware = new AuthorizationMiddleware({ + authorizationMiddlewareService, + }) + return authorizationMiddleware } const formsg = FormSG() @@ -71,4 +86,9 @@ const attachSiteHandler: RequestHandler< return next() } -export { getAuthMiddleware, attachFormSGHandler, attachSiteHandler } +export { + getAuthenticationMiddleware, + getAuthorizationMiddleware, + attachFormSGHandler, + attachSiteHandler, +} diff --git a/src/routes/v1/auth.js b/src/routes/v1/auth.js index 38ed8b4c7..93e370696 100644 --- a/src/routes/v1/auth.js +++ b/src/routes/v1/auth.js @@ -13,7 +13,7 @@ const { attachReadRouteHandlerWrapper } = require("@middleware/routeHandler") const validateStatus = require("@utils/axios-utils") const jwtUtils = require("@utils/jwt-utils") -const { authMiddleware } = require("@root/middleware") +const { authenticationMiddleware } = require("@root/middleware") // Import services const identityServices = require("@services/identity") @@ -173,7 +173,7 @@ router.get("/", attachReadRouteHandlerWrapper(githubAuth)) router.delete("/logout", attachReadRouteHandlerWrapper(logout)) router.get( "/whoami", - authMiddleware.verifyJwt, + authenticationMiddleware.verifyJwt, attachReadRouteHandlerWrapper(whoami) ) diff --git a/src/routes/v1/authenticated/index.js b/src/routes/v1/authenticated/index.js index 8701aa02e..f6d70e893 100644 --- a/src/routes/v1/authenticated/index.js +++ b/src/routes/v1/authenticated/index.js @@ -3,13 +3,16 @@ const express = require("express") const sitesRouter = require("@routes/v1/authenticated/sites") const { UsersRouter } = require("@routes/v2/authenticated/users") -const getAuthenticatedSubrouter = ({ authMiddleware, usersService }) => { +const getAuthenticatedSubrouter = ({ + authenticationMiddleware, + usersService, +}) => { // Workaround - no v1 users router exists const usersRouter = new UsersRouter({ usersService }) const authenticatedSubrouter = express.Router({ mergeParams: true }) - authenticatedSubrouter.use(authMiddleware.verifyJwt) + authenticatedSubrouter.use(authenticationMiddleware.verifyJwt) authenticatedSubrouter.use("/sites", sitesRouter) authenticatedSubrouter.use("/user", usersRouter.getRouter()) diff --git a/src/routes/v1/authenticatedSites/index.js b/src/routes/v1/authenticatedSites/index.js index 901d0a5d9..908fe4ed1 100644 --- a/src/routes/v1/authenticatedSites/index.js +++ b/src/routes/v1/authenticatedSites/index.js @@ -18,12 +18,15 @@ const resourceRoomRouter = require("@routes/v1/authenticatedSites/resourceRoom") const resourcesRouter = require("@routes/v1/authenticatedSites/resources") const settingsRouter = require("@routes/v1/authenticatedSites/settings") -const getAuthenticatedSitesSubrouter = ({ authMiddleware }) => { +const getAuthenticatedSitesSubrouter = ({ + authenticationMiddleware, + authorizationMiddleware, +}) => { const authenticatedSitesSubrouter = express.Router({ mergeParams: true }) - authenticatedSitesSubrouter.use(authMiddleware.verifyJwt) + authenticatedSitesSubrouter.use(authenticationMiddleware.verifyJwt) authenticatedSitesSubrouter.use(attachSiteHandler) - authenticatedSitesSubrouter.use(authMiddleware.checkHasAccess) + authenticatedSitesSubrouter.use(authorizationMiddleware.checkIsSiteMember) authenticatedSitesSubrouter.use("/pages", pagesRouter) authenticatedSitesSubrouter.use("/collections", collectionsRouter) diff --git a/src/routes/v2/__tests__/Auth.spec.js b/src/routes/v2/__tests__/Auth.spec.js index 0a33ab5eb..845d3c73a 100644 --- a/src/routes/v2/__tests__/Auth.spec.js +++ b/src/routes/v2/__tests__/Auth.spec.js @@ -4,7 +4,7 @@ const request = require("supertest") const { attachReadRouteHandlerWrapper } = require("@middleware/routeHandler") const { generateRouter } = require("@fixtures/app") -const { mockUserSessionData } = require("@fixtures/sessionData") +const { mockUserSessionData, mockEmail } = require("@fixtures/sessionData") const { CSRF_COOKIE_NAME, COOKIE_NAME, AuthRouter } = require("../auth") @@ -17,6 +17,8 @@ describe("Unlinked Pages Router", () => { getAuthRedirectDetails: jest.fn(), getGithubAuthToken: jest.fn(), getUserInfo: jest.fn(), + sendOtp: jest.fn(), + verifyOtp: jest.fn(), } const router = new AuthRouter({ @@ -31,6 +33,8 @@ describe("Unlinked Pages Router", () => { attachReadRouteHandlerWrapper(router.authRedirect) ) subrouter.get("/", attachReadRouteHandlerWrapper(router.githubAuth)) + subrouter.post("/login", attachReadRouteHandlerWrapper(router.login)) + subrouter.post("/verify", attachReadRouteHandlerWrapper(router.verify)) subrouter.delete("/logout", attachReadRouteHandlerWrapper(router.logout)) subrouter.get("/whoami", attachReadRouteHandlerWrapper(router.whoami)) const app = generateRouter(subrouter) @@ -83,6 +87,25 @@ describe("Unlinked Pages Router", () => { ) }) }) + describe("login", () => { + it("calls the service to send otp", async () => { + await request(app).post(`/login`).send({ email: mockEmail }).expect(200) + expect(mockAuthService.sendOtp).toHaveBeenCalledWith( + mockEmail.toLowerCase() + ) + }) + }) + describe("verify", () => { + const mockOtp = "123456" + it("adds the cookie on login", async () => { + mockAuthService.getAuthRedirectDetails.mockResolvedValueOnce(cookieToken) + await request(app) + .post(`/verify`) + .send({ email: mockEmail, otp: mockOtp }) + .set("Cookie", `${COOKIE_NAME}=${cookieToken}`) + .expect(200) + }) + }) describe("logout", () => { it("removes cookies on logout", async () => { const resp = await request(app) diff --git a/src/routes/v2/auth.js b/src/routes/v2/auth.js index 188455aca..a314d9a33 100644 --- a/src/routes/v2/auth.js +++ b/src/routes/v2/auth.js @@ -7,6 +7,8 @@ const { attachReadRouteHandlerWrapper } = require("@middleware/routeHandler") const { FRONTEND_URL } = process.env const { isSecure } = require("@utils/auth-utils") +const { AuthError } = require("@errors/AuthError") + const AUTH_TOKEN_EXPIRY_MS = parseInt( process.env.AUTH_TOKEN_EXPIRY_DURATION_IN_MILLISECONDS, 10 @@ -16,9 +18,9 @@ const CSRF_COOKIE_NAME = "isomer-csrf" const COOKIE_NAME = "isomercms" class AuthRouter { - constructor({ authService, authMiddleware }) { + constructor({ authService, authenticationMiddleware }) { this.authService = authService - this.authMiddleware = authMiddleware + this.authenticationMiddleware = authenticationMiddleware // We need to bind all methods because we don't invoke them from the class directly autoBind(this) } @@ -72,6 +74,31 @@ class AuthRouter { return res.redirect(`${FRONTEND_URL}/sites`) } + async login(req, res) { + const { email: rawEmail } = req.body + const email = rawEmail.toLowerCase() + await this.authService.sendOtp(email) + return res.sendStatus(200) + } + + async verify(req, res) { + const { email: rawEmail, otp } = req.body + const email = rawEmail.toLowerCase() + const token = await this.authService.verifyOtp({ email, otp }) + const authTokenExpiry = new Date() + // getTime allows this to work across timezones + authTokenExpiry.setTime(authTokenExpiry.getTime() + AUTH_TOKEN_EXPIRY_MS) + const cookieSettings = { + path: "/", + expires: authTokenExpiry, + httpOnly: true, + sameSite: true, + secure: isSecure(), + } + res.cookie(COOKIE_NAME, token, cookieSettings) + return res.sendStatus(200) + } + async logout(req, res) { this.clearIsomerCookies(res) return res.sendStatus(200) @@ -96,10 +123,12 @@ class AuthRouter { attachReadRouteHandlerWrapper(this.authRedirect) ) router.get("/", attachReadRouteHandlerWrapper(this.githubAuth)) + router.post("/login", attachReadRouteHandlerWrapper(this.login)) + router.post("/verify", attachReadRouteHandlerWrapper(this.verify)) router.delete("/logout", attachReadRouteHandlerWrapper(this.logout)) router.get( "/whoami", - this.authMiddleware.verifyJwt, + this.authenticationMiddleware.verifyJwt, attachReadRouteHandlerWrapper(this.whoami) ) diff --git a/src/routes/v2/authenticated/__tests__/Sites.spec.js b/src/routes/v2/authenticated/__tests__/Sites.spec.js index 62afed667..9bf1e6a65 100644 --- a/src/routes/v2/authenticated/__tests__/Sites.spec.js +++ b/src/routes/v2/authenticated/__tests__/Sites.spec.js @@ -1,18 +1,17 @@ const express = require("express") const request = require("supertest") -const { NotFoundError } = require("@errors/NotFoundError") - const { attachReadRouteHandlerWrapper } = require("@middleware/routeHandler") const { generateRouter } = require("@fixtures/app") -const { mockUserSessionData } = require("@fixtures/sessionData") +const { + mockSiteName, + mockUserSessionData, + mockUserWithSiteSessionData, +} = require("@fixtures/sessionData") const { SitesRouter } = require("../sites") -// Can't set request fields - will always be undefined -const siteName = "siteName" - describe("Sites Router", () => { const mockSitesService = { getSites: jest.fn(), @@ -61,40 +60,18 @@ describe("Sites Router", () => { }) }) - describe("checkHasAccess", () => { - it("rejects if user has no access to a site", async () => { - mockSitesService.checkHasAccess.mockRejectedValueOnce( - new NotFoundError("") - ) - - await request(app).get(`/${siteName}`).expect(404) - - expect(mockSitesService.checkHasAccess).toHaveBeenCalledWith( - mockUserSessionData - ) - }) - - it("allows if user has access to a site", async () => { - await request(app).get(`/${siteName}`).expect(200) - - expect(mockSitesService.checkHasAccess).toHaveBeenCalledWith( - mockUserSessionData - ) - }) - }) - describe("getLastUpdated", () => { it("returns the last updated time", async () => { const lastUpdated = "last-updated" mockSitesService.getLastUpdated.mockResolvedValueOnce(lastUpdated) const resp = await request(app) - .get(`/${siteName}/lastUpdated`) + .get(`/${mockSiteName}/lastUpdated`) .expect(200) expect(resp.body).toStrictEqual({ lastUpdated }) expect(mockSitesService.getLastUpdated).toHaveBeenCalledWith( - mockUserSessionData + mockUserWithSiteSessionData ) }) }) @@ -104,11 +81,13 @@ describe("Sites Router", () => { const stagingUrl = "staging-url" mockSitesService.getStagingUrl.mockResolvedValueOnce(stagingUrl) - const resp = await request(app).get(`/${siteName}/stagingUrl`).expect(200) + const resp = await request(app) + .get(`/${mockSiteName}/stagingUrl`) + .expect(200) expect(resp.body).toStrictEqual({ stagingUrl }) expect(mockSitesService.getStagingUrl).toHaveBeenCalledWith( - mockUserSessionData + mockUserWithSiteSessionData ) }) }) diff --git a/src/routes/v2/authenticated/index.js b/src/routes/v2/authenticated/index.js index b118eb102..89a2f93c4 100644 --- a/src/routes/v2/authenticated/index.js +++ b/src/routes/v2/authenticated/index.js @@ -12,12 +12,18 @@ const { SitesRouter } = require("./sites") const { UsersRouter } = require("./users") const getAuthenticatedSubrouter = ({ - authMiddleware, + authenticationMiddleware, gitHubService, configYmlService, usersService, + isomerAdminsService, }) => { - const sitesService = new SitesService({ gitHubService, configYmlService }) + const sitesService = new SitesService({ + gitHubService, + configYmlService, + usersService, + isomerAdminsService, + }) const netlifyTomlService = new NetlifyTomlService() const sitesV2Router = new SitesRouter({ sitesService }) @@ -26,7 +32,7 @@ const getAuthenticatedSubrouter = ({ const authenticatedSubrouter = express.Router({ mergeParams: true }) - authenticatedSubrouter.use(authMiddleware.verifyJwt) + authenticatedSubrouter.use(authenticationMiddleware.verifyJwt) authenticatedSubrouter.use("/sites", sitesV2Router.getRouter()) authenticatedSubrouter.use("/user", usersRouter.getRouter()) diff --git a/src/routes/v2/authenticated/sites.js b/src/routes/v2/authenticated/sites.js index c90ee08ed..217bee704 100644 --- a/src/routes/v2/authenticated/sites.js +++ b/src/routes/v2/authenticated/sites.js @@ -4,6 +4,10 @@ const express = require("express") // Import middleware const { attachReadRouteHandlerWrapper } = require("@middleware/routeHandler") +const { + default: UserWithSiteSessionData, +} = require("@classes/UserWithSiteSessionData") + const { attachSiteHandler } = require("@root/middleware") class SitesRouter { @@ -13,29 +17,47 @@ class SitesRouter { autoBind(this) } + addSiteNameToSessionData(userSessionData, siteName) { + const { githubId, accessToken, isomerUserId, email } = userSessionData + return new UserWithSiteSessionData({ + githubId, + accessToken, + isomerUserId, + email, + siteName, + }) + } + async getSites(req, res) { const { userSessionData } = res.locals const siteNames = await this.sitesService.getSites(userSessionData) return res.status(200).json({ siteNames }) } - async checkHasAccess(req, res) { - const { userSessionData } = res.locals - - await this.sitesService.checkHasAccess(userSessionData) - return res.status(200).send("OK") - } - async getLastUpdated(req, res) { const { userSessionData } = res.locals - const lastUpdated = await this.sitesService.getLastUpdated(userSessionData) + const { siteName } = req.params + const userWithSiteSessionData = this.addSiteNameToSessionData( + userSessionData, + siteName + ) + const lastUpdated = await this.sitesService.getLastUpdated( + userWithSiteSessionData + ) return res.status(200).json({ lastUpdated }) } async getStagingUrl(req, res) { const { userSessionData } = res.locals - const stagingUrl = await this.sitesService.getStagingUrl(userSessionData) + const { siteName } = req.params + const userWithSiteSessionData = this.addSiteNameToSessionData( + userSessionData, + siteName + ) + const stagingUrl = await this.sitesService.getStagingUrl( + userWithSiteSessionData + ) return res.status(200).json({ stagingUrl }) } @@ -43,11 +65,6 @@ class SitesRouter { const router = express.Router({ mergeParams: true }) router.get("/", attachReadRouteHandlerWrapper(this.getSites)) - router.get( - "/:siteName", - attachSiteHandler, - attachReadRouteHandlerWrapper(this.checkHasAccess) - ) router.get( "/:siteName/lastUpdated", attachSiteHandler, diff --git a/src/routes/v2/authenticated/users.ts b/src/routes/v2/authenticated/users.ts index 7a78937a6..fba0d19e4 100644 --- a/src/routes/v2/authenticated/users.ts +++ b/src/routes/v2/authenticated/users.ts @@ -72,7 +72,7 @@ export class UsersRouter { throw new BadRequestError("Invalid OTP") } - await this.usersService.updateUserByGitHubId(userId, { email }) + await this.usersService.updateUserByIsomerId(userId, { email }) return res.sendStatus(200) } @@ -104,7 +104,7 @@ export class UsersRouter { throw new BadRequestError("Invalid OTP") } - await this.usersService.updateUserByGitHubId(userId, { + await this.usersService.updateUserByIsomerId(userId, { contactNumber: mobile, }) return res.sendStatus(200) diff --git a/src/routes/v2/authenticatedSites/index.js b/src/routes/v2/authenticatedSites/index.js index 7d117d63a..f7c40716d 100644 --- a/src/routes/v2/authenticatedSites/index.js +++ b/src/routes/v2/authenticatedSites/index.js @@ -84,7 +84,8 @@ const { const { MoverService } = require("@services/moverServices/MoverService") const getAuthenticatedSitesSubrouter = ({ - authMiddleware, + authenticationMiddleware, + authorizationMiddleware, gitHubService, configYmlService, }) => { @@ -186,9 +187,9 @@ const getAuthenticatedSitesSubrouter = ({ const authenticatedSitesSubrouter = express.Router({ mergeParams: true }) - authenticatedSitesSubrouter.use(authMiddleware.verifyJwt) + authenticatedSitesSubrouter.use(authenticationMiddleware.verifyJwt) authenticatedSitesSubrouter.use(attachSiteHandler) - authenticatedSitesSubrouter.use(authMiddleware.checkHasAccess) + authenticatedSitesSubrouter.use(authorizationMiddleware.checkIsSiteMember) authenticatedSitesSubrouter.use( "/collections/:collectionName", diff --git a/src/routes/v2/authenticatedSites/settings.js b/src/routes/v2/authenticatedSites/settings.js index 6585ca03a..7f1ed377e 100644 --- a/src/routes/v2/authenticatedSites/settings.js +++ b/src/routes/v2/authenticatedSites/settings.js @@ -64,8 +64,7 @@ class SettingsRouter { navigationContent: updatedNavigationContent, } = SettingsService.retrieveSettingsFields(settings) - await this.settingsService.updateSettingsFiles({ - userWithSiteSessionData, + await this.settingsService.updateSettingsFiles(userWithSiteSessionData, { config, homepage, footer, diff --git a/src/server.js b/src/server.js index a57e95d73..f444e90c7 100644 --- a/src/server.js +++ b/src/server.js @@ -12,14 +12,19 @@ import { AccessToken, Repo, Deployment, + IsomerAdmin, } from "@database/models" import bootstrap from "@root/bootstrap" -import { getAuthMiddleware } from "@root/middleware" +import { + getAuthenticationMiddleware, + getAuthorizationMiddleware, +} from "@root/middleware" import { isomerRepoAxiosInstance } from "@services/api/AxiosInstance" import { getIdentityAuthService, getUsersService, sitesService, + isomerAdminsService, } from "@services/identity" import DeploymentsService from "@services/identity/DeploymentsService" import ReposService from "@services/identity/ReposService" @@ -40,6 +45,7 @@ const sequelize = initSequelize([ AccessToken, Repo, Deployment, + IsomerAdmin, ]) const usersService = getUsersService(sequelize) @@ -80,32 +86,41 @@ const gitHubService = new GitHubService({ axiosInstance: isomerRepoAxiosInstance, }) const identityAuthService = getIdentityAuthService(gitHubService) + const configYmlService = new ConfigYmlService({ gitHubService }) -const authMiddleware = getAuthMiddleware({ identityAuthService }) +const authenticationMiddleware = getAuthenticationMiddleware() +const authorizationMiddleware = getAuthorizationMiddleware({ + identityAuthService, + usersService, + isomerAdminsService, +}) const authenticatedSubrouterV1 = getAuthenticatedSubrouterV1({ - authMiddleware, + authenticationMiddleware, usersService, }) const authenticatedSitesSubrouterV1 = getAuthenticatedSitesSubrouterV1({ - authMiddleware, + authenticationMiddleware, + authorizationMiddleware, }) const authenticatedSubrouterV2 = getAuthenticatedSubrouter({ - authMiddleware, + authenticationMiddleware, gitHubService, configYmlService, usersService, reposService, deploymentsService, + isomerAdminsService, }) const authenticatedSitesSubrouterV2 = getAuthenticatedSitesSubrouter({ - authMiddleware, + authorizationMiddleware, + authenticationMiddleware, gitHubService, configYmlService, }) -const authV2Router = new AuthRouter({ authMiddleware, authService }) +const authV2Router = new AuthRouter({ authenticationMiddleware, authService }) const formsgRouter = new FormsgRouter({ usersService, infraService }) const app = express() diff --git a/src/services/api/AxiosInstance.ts b/src/services/api/AxiosInstance.ts index 7950d372f..4e60511aa 100644 --- a/src/services/api/AxiosInstance.ts +++ b/src/services/api/AxiosInstance.ts @@ -2,11 +2,25 @@ import axios, { AxiosRequestConfig, AxiosResponse } from "axios" import logger from "@logger/logger" +import { getAccessToken } from "@utils/token-retrieval-utils" + // Env vars const { GITHUB_ORG_NAME } = process.env -const requestFormatter = (config: AxiosRequestConfig) => { +const requestFormatter = async (config: AxiosRequestConfig) => { logger.info("Making GitHub API call") + + const authMessage = config.headers.Authorization + + // If accessToken is missing, authMessage is `token ` + if ( + !authMessage || + authMessage === "token " || + authMessage === "token undefined" + ) { + const accessToken = await getAccessToken() + config.headers.Authorization = `token ${accessToken}` + } return { ...config, headers: { diff --git a/src/services/configServices/SettingsService.js b/src/services/configServices/SettingsService.js index 930c4096e..ce7bcba6c 100644 --- a/src/services/configServices/SettingsService.js +++ b/src/services/configServices/SettingsService.js @@ -43,21 +43,32 @@ class SettingsService { } } - async updateSettingsFiles({ + async updateSettingsFiles( sessionData, - config, - homepage, - footer, - navigation, - updatedConfigContent, - updatedFooterContent, - updatedNavigationContent, - }) { + { + config, + homepage, + footer, + navigation, + updatedConfigContent, + updatedFooterContent, + updatedNavigationContent, + } + ) { + console.log(sessionData) if (!_.isEmpty(updatedConfigContent)) { const mergedConfigContent = this.mergeUpdatedData( config.content, updatedConfigContent ) + // Prepend "https://" to url parameter if parameter is defined + if ( + mergedConfigContent.url !== "" && + !mergedConfigContent.url.startsWith("https://") + ) { + mergedConfigContent.url = `https://${mergedConfigContent.url}` + } + await this.configYmlService.update(sessionData, { fileContent: mergedConfigContent, sha: config.sha, @@ -152,7 +163,7 @@ class SettingsService { static extractConfigFields(config) { return { - url: config.content.url, + url: config.content.url.replace("https://", ""), description: config.content.description, title: config.content.title, favicon: config.content.favicon, @@ -161,7 +172,6 @@ class SettingsService { facebook_pixel: config.content["facebook-pixel"], google_analytics: config.content.google_analytics, linkedin_insights: config.content["linkedin-insights"], - resources_name: config.content.resources_name, colors: config.content.colors, } } @@ -187,7 +197,6 @@ class SettingsService { "facebook-pixel", "google_analytics", "linkedin-insights", - "resources_name", "colors", ] const footerParams = [ diff --git a/src/services/configServices/__tests__/SettingsService.spec.js b/src/services/configServices/__tests__/SettingsService.spec.js index 5c0a937f0..e26f9352e 100644 --- a/src/services/configServices/__tests__/SettingsService.spec.js +++ b/src/services/configServices/__tests__/SettingsService.spec.js @@ -118,8 +118,7 @@ describe("Settings Service", () => { } await expect( - service.updateSettingsFiles({ - sessionData: mockUserWithSiteSessionData, + service.updateSettingsFiles(mockUserWithSiteSessionData, { config, homepage, footer, @@ -162,8 +161,7 @@ describe("Settings Service", () => { } await expect( - service.updateSettingsFiles({ - sessionData: mockUserWithSiteSessionData, + service.updateSettingsFiles(mockUserWithSiteSessionData, { config, homepage, footer, @@ -209,8 +207,7 @@ describe("Settings Service", () => { } await expect( - service.updateSettingsFiles({ - sessionData: mockUserWithSiteSessionData, + service.updateSettingsFiles(mockUserWithSiteSessionData, { config, homepage, footer, @@ -248,8 +245,7 @@ describe("Settings Service", () => { } await expect( - service.updateSettingsFiles({ - sessionData: mockUserWithSiteSessionData, + service.updateSettingsFiles(mockUserWithSiteSessionData, { config, homepage, footer, @@ -284,8 +280,7 @@ describe("Settings Service", () => { } await expect( - service.updateSettingsFiles({ - sessionData: mockUserWithSiteSessionData, + service.updateSettingsFiles(mockUserWithSiteSessionData, { config, homepage, footer, @@ -349,8 +344,7 @@ describe("Settings Service", () => { } await expect( - service.updateSettingsFiles({ - sessionData: mockUserWithSiteSessionData, + service.updateSettingsFiles(mockUserWithSiteSessionData, { config, homepage, footer, diff --git a/src/services/db/GitHubService.js b/src/services/db/GitHubService.js index c86ef3765..394332995 100644 --- a/src/services/db/GitHubService.js +++ b/src/services/db/GitHubService.js @@ -43,15 +43,19 @@ class GitHubService { sessionData, { content, fileName, directoryName, isMedia = false } ) { - const { siteName } = sessionData - const { accessToken } = sessionData + const { accessToken, siteName, isomerUserId: userId } = sessionData try { const endpoint = this.getFilePath({ siteName, fileName, directoryName }) // Validation and sanitisation of media already done const encodedContent = isMedia ? content : Base64.encode(content) - const params = { + const message = JSON.stringify({ message: `Create file: ${fileName}`, + fileName, + userId, + }) + const params = { + message, content: encodedContent, branch: BRANCH_REF, } @@ -147,8 +151,7 @@ class GitHubService { } async update(sessionData, { fileContent, sha, fileName, directoryName }) { - const { accessToken } = sessionData - const { siteName } = sessionData + const { accessToken, siteName, isomerUserId: userId } = sessionData try { const endpoint = this.getFilePath({ siteName, fileName, directoryName }) const encodedNewContent = Base64.encode(fileContent) @@ -162,8 +165,13 @@ class GitHubService { fileSha = retrievedSha } - const params = { + const message = JSON.stringify({ message: `Update file: ${fileName}`, + fileName, + userId, + }) + const params = { + message, content: encodedNewContent, branch: BRANCH_REF, sha: fileSha, @@ -189,8 +197,7 @@ class GitHubService { } async delete(sessionData, { sha, fileName, directoryName }) { - const { accessToken } = sessionData - const { siteName } = sessionData + const { accessToken, siteName, isomerUserId: userId } = sessionData try { const endpoint = this.getFilePath({ siteName, fileName, directoryName }) @@ -204,8 +211,13 @@ class GitHubService { fileSha = retrievedSha } - const params = { + const message = JSON.stringify({ message: `Delete file: ${fileName}`, + fileName, + userId, + }) + const params = { + message, branch: BRANCH_REF, sha: fileSha, } @@ -295,8 +307,7 @@ class GitHubService { } async updateTree(sessionData, githubSessionData, { gitTree, message }) { - const { accessToken } = sessionData - const { siteName } = sessionData + const { accessToken, siteName, isomerUserId: userId } = sessionData const { treeSha, currentCommitSha } = githubSessionData.getGithubState() const url = `${siteName}/git/trees` @@ -319,10 +330,14 @@ class GitHubService { const commitEndpoint = `${siteName}/git/commits` + const stringifiedMessage = JSON.stringify({ + message: message || `isomerCMS updated ${siteName} state`, + userId, + }) const newCommitResp = await this.axiosInstance.post( commitEndpoint, { - message: message || `isomerCMS updated ${siteName} state`, + message: stringifiedMessage, tree: newTreeSha, parents: [currentCommitSha], }, diff --git a/src/services/db/__tests__/GitHubService.spec.js b/src/services/db/__tests__/GitHubService.spec.js index ae4f4493f..b89868003 100644 --- a/src/services/db/__tests__/GitHubService.spec.js +++ b/src/services/db/__tests__/GitHubService.spec.js @@ -11,6 +11,7 @@ const { mockGithubId, mockCurrentCommitSha, mockGithubSessionData, + mockIsomerUserId, } = require("@fixtures/sessionData") const { GitHubService } = require("@services/db/GitHubService") @@ -26,7 +27,7 @@ describe("Github Service", () => { const sha = "12345" const treeSha = mockTreeSha const content = "test-content" - const userId = mockGithubId + const userId = mockIsomerUserId const sessionData = mockUserWithSiteSessionData @@ -99,8 +100,13 @@ describe("Github Service", () => { const endpoint = `${siteName}/contents/${directoryName}/${fileName}` const encodedContent = Base64.encode(content) - const params = { + const message = JSON.stringify({ message: `Create file: ${fileName}`, + fileName, + userId, + }) + const params = { + message, content: encodedContent, branch: BRANCH_REF, } @@ -326,8 +332,13 @@ describe("Github Service", () => { describe("Update", () => { const endpoint = `${siteName}/contents/${directoryName}/${fileName}` const encodedContent = Base64.encode(content) - const params = { + const message = JSON.stringify({ message: `Update file: ${fileName}`, + fileName, + userId, + }) + const params = { + message, content: encodedContent, branch: BRANCH_REF, sha, @@ -447,8 +458,13 @@ describe("Github Service", () => { describe("Delete", () => { const endpoint = `${siteName}/contents/${directoryName}/${fileName}` - const params = { + const message = JSON.stringify({ message: `Delete file: ${fileName}`, + fileName, + userId, + }) + const params = { + message, branch: BRANCH_REF, sha, } @@ -601,6 +617,10 @@ describe("Github Service", () => { const secondSha = "second-sha" const gitTree = "git-tree" const message = "message" + const finalExpectedMessage = JSON.stringify({ + message, + userId, + }) const firstResp = { data: { sha: firstSha, @@ -632,7 +652,7 @@ describe("Github Service", () => { expect(mockAxiosInstance.post).toHaveBeenCalledWith( commitEndpoint, { - message: message || `isomerCMS updated ${siteName} state`, + message: finalExpectedMessage, tree: firstSha, parents: [mockCurrentCommitSha], }, @@ -655,7 +675,7 @@ describe("Github Service", () => { }) describe("checkHasAccess", () => { - const refEndpoint = `${siteName}/collaborators/${userId}` + const refEndpoint = `${siteName}/collaborators/${mockGithubId}` const headers = { Authorization: `token ${accessToken}`, "Content-Type": "application/json", diff --git a/src/services/directoryServices/ResourceRoomDirectoryService.js b/src/services/directoryServices/ResourceRoomDirectoryService.js index dd810892d..372820fd6 100644 --- a/src/services/directoryServices/ResourceRoomDirectoryService.js +++ b/src/services/directoryServices/ResourceRoomDirectoryService.js @@ -96,18 +96,18 @@ class ResourceRoomDirectoryService { const { frontMatter, pageContent } = retrieveDataFromMarkdown(rawContent) frontMatter.title = newDirectoryName const newContent = convertDataToMarkdown(frontMatter, pageContent) - await this.gitHubService.update(sessionData, { - fileContent: newContent, - sha, - fileName: INDEX_FILE_NAME, - directoryName: resourceRoomName, - }) await this.baseDirectoryService.rename(sessionData, githubSessionData, { oldDirectoryName: resourceRoomName, newDirectoryName: slugifiedNewResourceRoomName, message: `Renaming resource room from ${resourceRoomName} to ${slugifiedNewResourceRoomName}`, }) + await this.gitHubService.update(sessionData, { + fileContent: newContent, + sha, + fileName: INDEX_FILE_NAME, + directoryName: resourceRoomName, + }) const { content: configContent, diff --git a/src/services/identity/IsomerAdminsService.ts b/src/services/identity/IsomerAdminsService.ts new file mode 100644 index 000000000..4ac712814 --- /dev/null +++ b/src/services/identity/IsomerAdminsService.ts @@ -0,0 +1,27 @@ +import { ModelStatic } from "sequelize" + +import { IsomerAdmin } from "@database/models" + +interface IsomerAdminsServiceProps { + repository: ModelStatic +} + +class IsomerAdminsService { + // NOTE: Explicitly specifying using keyed properties to ensure + // that the types are synced. + private readonly repository: IsomerAdminsServiceProps["repository"] + + constructor({ repository }: IsomerAdminsServiceProps) { + this.repository = repository + } + + async getByUserId(userId: string): Promise { + const site = await this.repository.findOne({ + where: { userId }, + }) + + return site + } +} + +export default IsomerAdminsService diff --git a/src/services/identity/ReposService.ts b/src/services/identity/ReposService.ts index 788c5d90b..e89d1b528 100644 --- a/src/services/identity/ReposService.ts +++ b/src/services/identity/ReposService.ts @@ -77,12 +77,6 @@ export default class ReposService { productionUrl: string, stagingUrl: string ) => { - await octokit.repos.update({ - owner: ISOMER_GITHUB_ORGANIZATION_NAME, - repo: repoName, - description: `Staging: ${stagingUrl} | Production: ${productionUrl}`, - }) - const dir = this.getLocalRepoPath(repoName) // 1. Set URLs in local _config.yml diff --git a/src/services/identity/UsersService.ts b/src/services/identity/UsersService.ts index a49d7c7fd..0473e9f64 100644 --- a/src/services/identity/UsersService.ts +++ b/src/services/identity/UsersService.ts @@ -2,7 +2,7 @@ import { Op, ModelStatic } from "sequelize" import { Sequelize } from "sequelize-typescript" import { RequireAtLeastOne } from "type-fest" -import { User, Whitelist } from "@database/models" +import { Repo, Site, SiteMember, User, Whitelist } from "@database/models" import SmsClient from "@services/identity/SmsClient" import TotpGenerator from "@services/identity/TotpGenerator" import MailClient from "@services/utilServices/MailClient" @@ -55,6 +55,44 @@ class UsersService { return this.repository.findOne({ where: { githubId } }) } + async hasAccessToSite(userId: string, siteName: string): Promise { + const siteMember = await this.repository.findOne({ + where: { id: userId }, + include: [ + { + model: Site, + as: "site_members", + required: true, + include: [ + { + model: Repo, + required: true, + where: { + name: siteName, + }, + }, + ], + }, + ], + }) + + return !!siteMember + } + + async findSitesByUserId(isomerId: string) { + return this.repository.findOne({ + where: { id: isomerId }, + include: [ + { + model: Site, + as: "site_members", + required: true, + include: [{ model: Repo, required: true }], + }, + ], + }) + } + async updateUserByGitHubId( githubId: string, // NOTE: This ensures that the caller passes in at least 1 property of User @@ -63,6 +101,14 @@ class UsersService { await this.repository.update(user, { where: { githubId } }) } + async updateUserByIsomerId( + isomerId: string, + // NOTE: This ensures that the caller passes in at least 1 property of User + user: RequireAtLeastOne + ) { + await this.repository.update(user, { where: { id: isomerId } }) + } + async findOrCreate(githubId: string | undefined) { const [user] = await this.repository.findOrCreate({ where: { githubId }, @@ -82,6 +128,18 @@ class UsersService { }) } + async loginWithEmail(email: string): Promise { + return this.sequelize.transaction(async (transaction) => { + // NOTE: The service's findOrCreate is not being used here as this requires an explicit transaction + const [user] = await this.repository.findOrCreate({ + where: { email }, + transaction, + }) + user.lastLoggedIn = new Date() + return user.save({ transaction }) + }) + } + async canSendEmailOtp(email: string) { const whitelistEntries = await this.whitelist.findAll({ attributes: ["email"], diff --git a/src/services/identity/index.ts b/src/services/identity/index.ts index 3d08d6381..d8af5d8b2 100644 --- a/src/services/identity/index.ts +++ b/src/services/identity/index.ts @@ -2,13 +2,14 @@ import { Sequelize } from "sequelize-typescript" import logger from "@logger/logger" -import { User, Site, Whitelist } from "@database/models" +import { User, Site, Whitelist, IsomerAdmin } from "@database/models" import { GitHubService } from "@services/db/GitHubService" import SmsClient from "@services/identity/SmsClient" import TotpGenerator from "@services/identity/TotpGenerator" import { mailer } from "@services/utilServices/MailClient" import AuthService from "./AuthService" +import IsomerAdminsService from "./IsomerAdminsService" import SitesService from "./SitesService" import TokenStore from "./TokenStore" import UsersService from "./UsersService" @@ -64,3 +65,7 @@ export const getUsersService = (sequelize: Sequelize) => // the GithubService instance in... export const getIdentityAuthService = (gitHubService: GitHubService) => new AuthService({ gitHubService }) + +export const isomerAdminsService = new IsomerAdminsService({ + repository: IsomerAdmin, +}) diff --git a/src/services/middlewareServices/AuthMiddlewareService.js b/src/services/middlewareServices/AuthenticationMiddlewareService.ts similarity index 66% rename from src/services/middlewareServices/AuthMiddlewareService.js rename to src/services/middlewareServices/AuthenticationMiddlewareService.ts index ed665d6b2..2e506c729 100644 --- a/src/services/middlewareServices/AuthMiddlewareService.js +++ b/src/services/middlewareServices/AuthenticationMiddlewareService.ts @@ -1,27 +1,34 @@ // Import logger -const logger = require("@logger/logger") +import logger from "@logger/logger" // Import errors -const { AuthError } = require("@errors/AuthError") -const { NotFoundError } = require("@errors/NotFoundError") +import { AuthError } from "@errors/AuthError" -const jwtUtils = require("@utils/jwt-utils") +import jwtUtils from "@utils/jwt-utils" -const { BadRequestError } = require("@root/errors/BadRequestError") -const { sitesService } = require("@services/identity") +import { E2E_ISOMER_ID } from "@root/constants" +import { BadRequestError } from "@root/errors/BadRequestError" const { E2E_TEST_REPO, E2E_TEST_SECRET, E2E_TEST_GH_TOKEN } = process.env const E2E_TEST_USER = "e2e-test" -const E2E_ISOMER_ID = "e2e-id" const E2E_TEST_EMAIL = "test@e2e" -const GENERAL_ACCESS_PATHS = ["/v1/sites", "/v1/auth/whoami"] - -class AuthMiddlewareService { - constructor({ identityAuthService }) { - this.identityAuthService = identityAuthService +const GENERAL_ACCESS_PATHS = [ + "/v1/sites", + "/v1/auth/whoami", + "/v2/sites", + "/v2/auth/whoami", +] + +interface VerifyJwtProps { + cookies: { + isomercms: string + isomercmsE2E?: string } + url: string +} - verifyE2E({ cookies, url }) { +export default class AuthenticationMiddlewareService { + verifyE2E({ cookies, url }: VerifyJwtProps) { const { isomercmsE2E } = cookies const urlTokens = url.split("/") // urls take the form "/v1/sites//"" @@ -42,7 +49,7 @@ class AuthMiddlewareService { return true } - verifyJwt({ cookies, url }) { + verifyJwt({ cookies, url }: VerifyJwtProps) { const { isomercms } = cookies const isValidE2E = this.verifyE2E({ cookies, url }) @@ -69,9 +76,15 @@ class AuthMiddlewareService { notLoggedInError.name = "NotLoggedInError" throw notLoggedInError } - const accessToken = jwtUtils.decryptToken(retrievedToken) + const accessToken = retrievedToken + ? jwtUtils.decryptToken(retrievedToken) + : "" return { accessToken, githubId, isomerUserId, email } } catch (err) { + if (!(err instanceof Error)) { + // NOTE: If the error is of an unknown kind, we bubble it up the stack and block access. + throw err + } // NOTE: Cookies aren't being logged here because they get caught as "Object object", which is not useful // The cookies should be converted to a JSON struct before logging if (err.name === "NotLoggedInError") { @@ -92,27 +105,4 @@ class AuthMiddlewareService { throw err } } - - async checkHasAccess(sessionData) { - // Check if site is onboarded to Isomer identity, otherwise continue using user access token - const { siteName } = sessionData - const userId = sessionData.isomerUserId - - logger.info(`Verifying user's access to ${siteName}`) - - const hasAccessToSite = await this.identityAuthService.hasAccessToSite( - sessionData - ) - - const isE2EUser = userId === E2E_ISOMER_ID - if (!hasAccessToSite && !isE2EUser) { - throw new NotFoundError("Site does not exist") - } - - logger.info(`User ${userId} has access to ${siteName}`) - } -} - -module.exports = { - AuthMiddlewareService, } diff --git a/src/services/middlewareServices/AuthorizationMiddlewareService.ts b/src/services/middlewareServices/AuthorizationMiddlewareService.ts new file mode 100644 index 000000000..1a2594c7d --- /dev/null +++ b/src/services/middlewareServices/AuthorizationMiddlewareService.ts @@ -0,0 +1,58 @@ +import { NotFoundError } from "@errors/NotFoundError" + +import UserWithSiteSessionData from "@classes/UserWithSiteSessionData" + +import { E2E_ISOMER_ID } from "@root/constants" +import AuthService from "@services/identity/AuthService" +import IsomerAdminsService from "@services/identity/IsomerAdminsService" +import UsersService from "@services/identity/UsersService" + +// Import logger +const logger = require("@logger/logger") + +interface AuthorizationMiddlewareServiceProps { + identityAuthService: AuthService + usersService: UsersService + isomerAdminsService: IsomerAdminsService +} + +export default class AuthorizationMiddlewareService { + readonly identityAuthService: AuthorizationMiddlewareServiceProps["identityAuthService"] + + readonly usersService: AuthorizationMiddlewareServiceProps["usersService"] + + readonly isomerAdminsService: AuthorizationMiddlewareServiceProps["isomerAdminsService"] + + constructor({ + identityAuthService, + usersService, + isomerAdminsService, + }: AuthorizationMiddlewareServiceProps) { + this.identityAuthService = identityAuthService + this.usersService = usersService + this.isomerAdminsService = isomerAdminsService + } + + async checkIsSiteMember(sessionData: UserWithSiteSessionData) { + // Check if user has access to site + const { siteName, isomerUserId: userId } = sessionData + + // Should always be defined - authorization middleware only exists if siteName is defined + if (!siteName) throw Error("No site name in authorization middleware") + + logger.info(`Verifying user's access to ${siteName}`) + + const isSiteMember = await (sessionData.isEmailUser() + ? this.usersService.hasAccessToSite(userId, siteName) + : this.identityAuthService.hasAccessToSite(sessionData)) + + const isAdminUser = await this.isomerAdminsService.getByUserId(userId) + + const isE2EUser = userId === E2E_ISOMER_ID + if (!isSiteMember && !isAdminUser && !isE2EUser) { + throw new NotFoundError("Site does not exist") + } + + logger.info(`User ${userId} has access to ${siteName}`) + } +} diff --git a/src/services/middlewareServices/__tests__/AuthorizationMiddlewareService.spec.ts b/src/services/middlewareServices/__tests__/AuthorizationMiddlewareService.spec.ts new file mode 100644 index 000000000..64c28bed2 --- /dev/null +++ b/src/services/middlewareServices/__tests__/AuthorizationMiddlewareService.spec.ts @@ -0,0 +1,122 @@ +import { rejects } from "assert" + +import { NotFoundError } from "@errors/NotFoundError" + +import { + mockUserWithSiteSessionData, + mockIsomerUserId, + mockSessionDataEmailUserWithSite, + mockSiteName, +} from "@fixtures/sessionData" +import AuthService from "@root/services/identity/AuthService" +import IsomerAdminsService from "@root/services/identity/IsomerAdminsService" +import UsersService from "@root/services/identity/UsersService" + +import AuthorizationMiddlewareService from "../AuthorizationMiddlewareService" + +describe("Authorization Middleware Service", () => { + const mockIdentityAuthService = { + hasAccessToSite: jest.fn(), + } + + const mockUsersService = { + hasAccessToSite: jest.fn(), + } + + const mockIsomerAdminsService = { + getByUserId: jest.fn(), + } + + const service = new AuthorizationMiddlewareService({ + identityAuthService: (mockIdentityAuthService as unknown) as AuthService, + usersService: (mockUsersService as unknown) as UsersService, + isomerAdminsService: (mockIsomerAdminsService as unknown) as IsomerAdminsService, + }) + + beforeEach(() => { + jest.clearAllMocks() + }) + + describe("checkIsSiteMember", () => { + it("Allows access for email users with site access", async () => { + // Arrange + mockIsomerAdminsService.getByUserId.mockImplementationOnce(() => null) + mockUsersService.hasAccessToSite.mockImplementationOnce(() => true) + + // Act + const actual = service.checkIsSiteMember(mockSessionDataEmailUserWithSite) + + // Assert + await expect(actual).resolves.not.toThrow() + expect(mockIdentityAuthService.hasAccessToSite).toHaveBeenCalledTimes(0) + expect(mockUsersService.hasAccessToSite).toHaveBeenCalledWith( + mockIsomerUserId, + mockSiteName + ) + expect(mockIsomerAdminsService.getByUserId).toHaveBeenCalledWith( + mockIsomerUserId + ) + }) + + it("Allows access for github users with site access", async () => { + // Arrange + mockIsomerAdminsService.getByUserId.mockImplementationOnce(() => null) + mockIdentityAuthService.hasAccessToSite.mockImplementationOnce(() => true) + + // Act + const actual = service.checkIsSiteMember(mockUserWithSiteSessionData) + + // Assert + await expect(actual).resolves.not.toThrow() + expect(mockUsersService.hasAccessToSite).toHaveBeenCalledTimes(0) + expect(mockIdentityAuthService.hasAccessToSite).toHaveBeenCalledWith( + mockUserWithSiteSessionData + ) + expect(mockIsomerAdminsService.getByUserId).toHaveBeenCalledWith( + mockIsomerUserId + ) + }) + + it("Allows access for admin users even without site access", async () => { + // Arrange + mockIsomerAdminsService.getByUserId.mockImplementationOnce( + () => "adminObj" + ) + mockUsersService.hasAccessToSite.mockImplementationOnce(() => false) + + // Act + const actual = service.checkIsSiteMember(mockSessionDataEmailUserWithSite) + + // Assert + await expect(actual).resolves.not.toThrow() + expect(mockIdentityAuthService.hasAccessToSite).toHaveBeenCalledTimes(0) + expect(mockUsersService.hasAccessToSite).toHaveBeenCalledWith( + mockIsomerUserId, + mockSiteName + ) + expect(mockIsomerAdminsService.getByUserId).toHaveBeenCalledWith( + mockIsomerUserId + ) + }) + + it("Throws error for users without site access", async () => { + // Arrange + mockIsomerAdminsService.getByUserId.mockImplementationOnce(() => null) + mockUsersService.hasAccessToSite.mockImplementationOnce(() => false) + + // Act + const actual = service.checkIsSiteMember(mockSessionDataEmailUserWithSite) + + // Assert + await expect(actual).rejects.toThrowError(NotFoundError) + expect(mockIdentityAuthService.hasAccessToSite).toHaveBeenCalledTimes(0) + expect(mockUsersService.hasAccessToSite).toHaveBeenCalledWith( + mockIsomerUserId, + mockSiteName + ) + expect(mockIsomerAdminsService.getByUserId).toHaveBeenCalledWith( + mockIsomerUserId + ) + }) + }) +}) diff --git a/src/services/utilServices/AuthService.js b/src/services/utilServices/AuthService.js index 8d03d856f..08d3302e9 100644 --- a/src/services/utilServices/AuthService.js +++ b/src/services/utilServices/AuthService.js @@ -9,7 +9,9 @@ const { ForbiddenError } = require("@errors/ForbiddenError") const validateStatus = require("@utils/axios-utils") const jwtUtils = require("@utils/jwt-utils") +const { BadRequestError } = require("@root/errors/BadRequestError") const logger = require("@root/logger/logger") +const { isError } = require("@root/types") const { CLIENT_ID, CLIENT_SECRET, REDIRECT_URI } = process.env @@ -90,19 +92,49 @@ class AuthService { return token } - async getUserInfo(sessionData) { - const { accessToken } = sessionData - // Make a call to github - const endpoint = "https://api.github.com/user" + async sendOtp(email) { + const isValidEmail = await this.usersService.canSendEmailOtp(email) + if (!isValidEmail) + throw new AuthError( + "Please sign in with a gov.sg or other whitelisted email." + ) + try { + await this.usersService.sendEmailOtp(email) + } catch (err) { + if (isError(err)) { + logger.error(err.message) + throw new BadRequestError(err.message) + } else { + // If we encountered something that isn't an error but still ends up in the error branch, + // log this to cloudwatch with the relevant details + logger.error( + `Encountered unknown error type: ${err} when sendEmailOtp with email: ${email}` + ) + } + } + } + async verifyOtp({ email, otp }) { + if (!this.usersService.verifyOtp(email, otp)) { + throw new BadRequestError("You have entered an invalid OTP.") + } + // Create user if does not exists. Set last logged in to current time. + const user = await this.usersService.loginWithEmail(email) + const token = jwtUtils.signToken({ + isomer_user_id: user.id, + email: user.email, + }) + return token + } + + async getUserInfo(sessionData) { try { - const resp = await axios.get(endpoint, { - headers: { - Authorization: `token ${accessToken}`, - "Content-Type": "application/json", - }, - }) - const userId = resp.data.login + if (sessionData.isEmailUser()) { + const { email } = sessionData + const { contactNumber } = await this.usersService.findByEmail(email) + return { email, contactNumber } + } + const { githubId: userId } = sessionData const { email, contactNumber } = await this.usersService.findByGitHubId( userId diff --git a/src/services/utilServices/SitesService.js b/src/services/utilServices/SitesService.js index dc4435529..f88a8e15b 100644 --- a/src/services/utilServices/SitesService.js +++ b/src/services/utilServices/SitesService.js @@ -29,12 +29,33 @@ const ISOMER_ADMIN_REPOS = [ ] class SitesService { - constructor({ gitHubService, configYmlService }) { + constructor({ + gitHubService, + configYmlService, + usersService, + isomerAdminsService, + }) { this.githubService = gitHubService this.configYmlService = configYmlService + this.usersService = usersService + this.isomerAdminsService = isomerAdminsService + } + + async getEmailUserSites(userId) { + const user = await this.usersService.findSitesByUserId(userId) + if (!user) return [] + const { site_members: siteMemberEntries } = user + return siteMemberEntries.map((entry) => { + const repoData = entry.repo + return repoData.name + }) } async getSites(sessionData) { + const isEmailUser = sessionData.isEmailUser() + const { isomerUserId: userId } = sessionData + const isAdminUser = !!(await this.isomerAdminsService.getByUserId(userId)) + const { accessToken } = sessionData const endpoint = `https://api.github.com/orgs/${ISOMER_GITHUB_ORG_NAME}/repos` @@ -47,7 +68,7 @@ class SitesService { }) ) - const sites = await Bluebird.map(paramsArr, async (params) => { + const allSites = await Bluebird.map(paramsArr, async (params) => { const { data: respData } = await genericGitHubAxiosInstance.get( endpoint, { @@ -81,7 +102,17 @@ class SitesService { ) }) - return _.flatten(sites) + const flattenedAllSites = _.flatten(allSites) + // Github users are using their own access token, which already filters sites to only those they have write access to + // Admin users should have access to all sites regardless + if (isAdminUser || !isEmailUser) return flattenedAllSites + + // Email users need to have the list of sites filtered to those they have access to in our db, since our centralised token returns all sites + const retrievedSitesByEmail = await this.getEmailUserSites(userId) + + return flattenedAllSites.filter((repoData) => + retrievedSitesByEmail.includes(repoData.repoName) + ) } async checkHasAccess(sessionData) { diff --git a/src/services/utilServices/__tests__/AuthService.spec.js b/src/services/utilServices/__tests__/AuthService.spec.js index 93f184a2e..4c6fceabf 100644 --- a/src/services/utilServices/__tests__/AuthService.spec.js +++ b/src/services/utilServices/__tests__/AuthService.spec.js @@ -8,10 +8,20 @@ jest.mock("@utils/jwt-utils") const axios = require("axios") const uuid = require("uuid/v4") +const { AuthError } = require("@errors/AuthError") +const { BadRequestError } = require("@errors/BadRequestError") + const validateStatus = require("@utils/axios-utils") const jwtUtils = require("@utils/jwt-utils") -const { mockUserWithSiteSessionData } = require("@fixtures/sessionData") +const { + mockUserWithSiteSessionData, + mockGithubId, + mockEmail, + mockIsomerUserId, + mockGithubId: mockUserId, + mockSessionDataEmailUser, +} = require("@fixtures/sessionData") const { AuthService } = require("@services/utilServices/AuthService") describe("Auth Service", () => { @@ -23,11 +33,7 @@ describe("Auth Service", () => { const token = "token" const signedToken = "signedToken" const csrfState = "csrfState" - const userId = "user" - const mockEmail = "email" const mockContactNumber = "12345678" - const mockIsomerUserId = "isomer-user" - const mockUserId = "user" const mockUsersService = { login: jest.fn().mockImplementation(() => mockIsomerUserId), @@ -35,6 +41,15 @@ describe("Auth Service", () => { email: mockEmail, contactNumber: mockContactNumber, })), + findByEmail: jest + .fn() + .mockImplementation(() => ({ contactNumber: mockContactNumber })), + canSendEmailOtp: jest.fn(), + sendEmailOtp: jest.fn(), + verifyOtp: jest.fn(), + loginWithEmail: jest + .fn() + .mockImplementation(() => ({ id: mockIsomerUserId, email: mockEmail })), } const service = new AuthService({ usersService: mockUsersService }) @@ -101,21 +116,77 @@ describe("Auth Service", () => { }) }) + describe("sendOtp", () => { + it("should be able to send otp for whitelisted users", async () => { + mockUsersService.canSendEmailOtp.mockImplementationOnce(() => true) + + await expect(service.sendOtp(mockEmail)).resolves.not.toThrow() + expect(mockUsersService.canSendEmailOtp).toHaveBeenCalledWith(mockEmail) + expect(mockUsersService.sendEmailOtp).toHaveBeenCalledWith(mockEmail) + }) + + it("should throw an error for non-whitelisted users", async () => { + mockUsersService.canSendEmailOtp.mockImplementationOnce(() => false) + + await expect(service.sendOtp(mockEmail)).rejects.toThrow(AuthError) + expect(mockUsersService.canSendEmailOtp).toHaveBeenCalledWith(mockEmail) + }) + }) + + describe("verifyOtp", () => { + const mockOtp = "123456" + it("should be able to verify otp, login, and return token if correct", async () => { + mockUsersService.verifyOtp.mockImplementationOnce(() => true) + jwtUtils.signToken.mockImplementationOnce(() => signedToken) + + await expect( + service.verifyOtp({ email: mockEmail, otp: mockOtp }) + ).resolves.toEqual(signedToken) + expect(mockUsersService.verifyOtp).toHaveBeenCalledWith( + mockEmail, + mockOtp + ) + expect(mockUsersService.loginWithEmail).toHaveBeenCalledWith(mockEmail) + }) + + it("should throw an error if otp is incorrect", async () => { + mockUsersService.verifyOtp.mockImplementationOnce(() => false) + + await expect( + service.verifyOtp({ email: mockEmail, otp: mockOtp }) + ).rejects.toThrow(BadRequestError) + expect(mockUsersService.verifyOtp).toHaveBeenCalledWith( + mockEmail, + mockOtp + ) + }) + }) + describe("getUserInfo", () => { - it("should be able to retrieve user info", async () => { + it("should be able to retrieve user info for github users", async () => { axios.get.mockImplementation(() => ({ data: { - login: userId, + login: mockGithubId, }, })) await expect( service.getUserInfo(mockUserWithSiteSessionData) ).resolves.toEqual({ - userId, + userId: mockGithubId, + email: mockEmail, + contactNumber: mockContactNumber, + }) + expect(mockUsersService.findByGitHubId).toHaveBeenCalledWith(mockGithubId) + }) + + it("should be able to retrieve user info for email users", async () => { + await expect( + service.getUserInfo(mockSessionDataEmailUser) + ).resolves.toEqual({ email: mockEmail, contactNumber: mockContactNumber, }) - expect(mockUsersService.findByGitHubId).toHaveBeenCalledWith(userId) + expect(mockUsersService.findByEmail).toHaveBeenCalledWith(mockEmail) }) }) }) diff --git a/src/services/utilServices/__tests__/SitesService.spec.js b/src/services/utilServices/__tests__/SitesService.spec.js index d4064371e..003f8d466 100644 --- a/src/services/utilServices/__tests__/SitesService.spec.js +++ b/src/services/utilServices/__tests__/SitesService.spec.js @@ -6,13 +6,16 @@ const { adminRepo, noAccessRepo, } = require("@fixtures/repoInfo") -const { mockUserWithSiteSessionData } = require("@fixtures/sessionData") +const { + mockUserWithSiteSessionData, + mockSessionDataEmailUser, + mockIsomerUserId, +} = require("@fixtures/sessionData") const { genericGitHubAxiosInstance, } = require("@root/services/api/AxiosInstance") describe("Resource Page Service", () => { - const accessToken = "test-token" const userId = "userId" const mockGithubService = { @@ -24,10 +27,21 @@ describe("Resource Page Service", () => { read: jest.fn(), } + const mockUsersService = { + findSitesByUserId: jest.fn(), + getRepoInfo: jest.fn(), + } + + const mockIsomerAdminsService = { + getByUserId: jest.fn(), + } + const { SitesService } = require("@services/utilServices/SitesService") const service = new SitesService({ gitHubService: mockGithubService, configYmlService: mockConfigYmlService, + usersService: mockUsersService, + isomerAdminsService: mockIsomerAdminsService, }) beforeEach(() => { @@ -35,7 +49,118 @@ describe("Resource Page Service", () => { }) describe("getSites", () => { - it("Filters accessible sites correctly", async () => { + it("Filters accessible sites for github user correctly", async () => { + // Store the API key and set it later so that other tests are not affected + const currRepoCount = process.env.ISOMERPAGES_REPO_PAGE_COUNT + process.env.ISOMERPAGES_REPO_PAGE_COUNT = 3 + + const expectedResp = [ + { + lastUpdated: repoInfo.pushed_at, + permissions: repoInfo.permissions, + repoName: repoInfo.name, + isPrivate: repoInfo.private, + }, + { + lastUpdated: repoInfo2.pushed_at, + permissions: repoInfo2.permissions, + repoName: repoInfo2.name, + isPrivate: repoInfo2.private, + }, + ] + mockIsomerAdminsService.getByUserId.mockImplementationOnce(() => null) + genericGitHubAxiosInstance.get.mockImplementationOnce(() => ({ + data: [repoInfo, repoInfo2, adminRepo, noAccessRepo], + })) + genericGitHubAxiosInstance.get.mockImplementationOnce(() => ({ + data: [], + })) + genericGitHubAxiosInstance.get.mockImplementationOnce(() => ({ + data: [], + })) + + await expect( + service.getSites(mockUserWithSiteSessionData) + ).resolves.toMatchObject(expectedResp) + + expect(genericGitHubAxiosInstance.get).toHaveBeenCalledTimes(3) + process.env.ISOMERPAGES_REPO_PAGE_COUNT = currRepoCount + expect(process.env.ISOMERPAGES_REPO_PAGE_COUNT).toBe(currRepoCount) + }) + + it("Filters accessible sites for email user correctly", async () => { + // Store the API key and set it later so that other tests are not affected + const currRepoCount = process.env.ISOMERPAGES_REPO_PAGE_COUNT + process.env.ISOMERPAGES_REPO_PAGE_COUNT = 3 + + const expectedResp = [ + { + lastUpdated: repoInfo.pushed_at, + permissions: repoInfo.permissions, + repoName: repoInfo.name, + isPrivate: repoInfo.private, + }, + ] + mockIsomerAdminsService.getByUserId.mockImplementationOnce(() => null) + mockUsersService.findSitesByUserId.mockImplementationOnce(() => ({ + site_members: [{ repo: { name: repoInfo.name } }], + })) + genericGitHubAxiosInstance.get.mockImplementationOnce(() => ({ + data: [repoInfo, repoInfo2, adminRepo, noAccessRepo], + })) + genericGitHubAxiosInstance.get.mockImplementationOnce(() => ({ + data: [], + })) + genericGitHubAxiosInstance.get.mockImplementationOnce(() => ({ + data: [], + })) + + await expect( + service.getSites(mockSessionDataEmailUser) + ).resolves.toMatchObject(expectedResp) + + expect(mockIsomerAdminsService.getByUserId).toHaveBeenCalledWith( + mockIsomerUserId + ) + expect(genericGitHubAxiosInstance.get).toHaveBeenCalledTimes(3) + process.env.ISOMERPAGES_REPO_PAGE_COUNT = currRepoCount + expect(process.env.ISOMERPAGES_REPO_PAGE_COUNT).toBe(currRepoCount) + }) + + it("Filters accessible sites for email user with no sites correctly", async () => { + // Store the API key and set it later so that other tests are not affected + const currRepoCount = process.env.ISOMERPAGES_REPO_PAGE_COUNT + process.env.ISOMERPAGES_REPO_PAGE_COUNT = 3 + + const expectedResp = [] + mockIsomerAdminsService.getByUserId.mockImplementationOnce(() => null) + mockUsersService.findSitesByUserId.mockImplementationOnce(() => null) + genericGitHubAxiosInstance.get.mockImplementationOnce(() => ({ + data: [repoInfo, repoInfo2, adminRepo, noAccessRepo], + })) + genericGitHubAxiosInstance.get.mockImplementationOnce(() => ({ + data: [], + })) + genericGitHubAxiosInstance.get.mockImplementationOnce(() => ({ + data: [], + })) + + await expect( + service.getSites(mockSessionDataEmailUser) + ).resolves.toMatchObject(expectedResp) + + expect(mockIsomerAdminsService.getByUserId).toHaveBeenCalledWith( + mockIsomerUserId + ) + expect(mockUsersService.findSitesByUserId).toHaveBeenCalledWith( + mockIsomerUserId + ) + expect(genericGitHubAxiosInstance.get).toHaveBeenCalledTimes(3) + process.env.ISOMERPAGES_REPO_PAGE_COUNT = currRepoCount + expect(process.env.ISOMERPAGES_REPO_PAGE_COUNT).toBe(currRepoCount) + }) + + it("Returns all accessible sites for admin user correctly", async () => { // Store the API key and set it later so that other tests are not affected const currRepoCount = process.env.ISOMERPAGES_REPO_PAGE_COUNT process.env.ISOMERPAGES_REPO_PAGE_COUNT = 3 @@ -54,6 +179,7 @@ describe("Resource Page Service", () => { isPrivate: repoInfo2.private, }, ] + mockIsomerAdminsService.getByUserId.mockImplementationOnce(() => "user") genericGitHubAxiosInstance.get.mockImplementationOnce(() => ({ data: [repoInfo, repoInfo2, adminRepo, noAccessRepo], })) @@ -68,6 +194,9 @@ describe("Resource Page Service", () => { service.getSites(mockUserWithSiteSessionData) ).resolves.toMatchObject(expectedResp) + expect(mockIsomerAdminsService.getByUserId).toHaveBeenCalledWith( + mockIsomerUserId + ) expect(genericGitHubAxiosInstance.get).toHaveBeenCalledTimes(3) process.env.ISOMERPAGES_REPO_PAGE_COUNT = currRepoCount expect(process.env.ISOMERPAGES_REPO_PAGE_COUNT).toBe(currRepoCount) diff --git a/src/tests/database.ts b/src/tests/database.ts index 656b2722e..56a00bf05 100644 --- a/src/tests/database.ts +++ b/src/tests/database.ts @@ -9,6 +9,7 @@ import { AccessToken, Repo, Deployment, + IsomerAdmin, } from "@database/models" const sequelize = new Sequelize({ @@ -23,6 +24,7 @@ sequelize.addModels([ AccessToken, Repo, Deployment, + IsomerAdmin, ]) // eslint-disable-next-line import/prefer-default-export diff --git a/src/validators/RequestSchema.js b/src/validators/RequestSchema.js index fbc2d0bc8..0838a6409 100644 --- a/src/validators/RequestSchema.js +++ b/src/validators/RequestSchema.js @@ -236,6 +236,7 @@ const UpdateNavigationRequestSchema = Joi.object().keys({ }) const UpdateSettingsRequestSchema = Joi.object().keys({ + url: Joi.string().domain().allow(""), colors: Joi.object().keys({ "primary-color": Joi.string().required(), "secondary-color": Joi.string().required(),