Refactor/move identity to v2 endpoints (#368)

* Fix/retrieve last updated time (#354) * Fix: use pushed_at instead of updated_at * Fix: getLastUpdated endpoint * Fix: prepend unique prefix for release script (#370) * 0.1.0 * Refactor/auth (#328) * Feat: Add Auth service * Feat: add auth router * Feat: add auth endpoints * Feat: convert auth middleware to use dependency injection * feat: add tests for new auth router and service * Fix: add updated auth router endpoints to auth middleware * Refactor: verifyJwt into separate helper method * Nit: add exports for constants * Nit: add helper function for secure and comments for time * Style: early return * Style: test styling * Nit: adjust comments for importing error * Nit: adjust unknown route comment * Nit: return early * Nit: link issue * Refactor: move isSecure to utils * Style: early return * Nit: remove irrelevant comment * Refactor/separate auth initialisation (#377) * Chore: remove unused index router * Refactor: initialise middleware outside of server.js * Refactor: move unknown route check to server.js * Refactor: import auth middleware directly in routers * Chore: remove noverify method * Refactor/sites (#341) * Feat: add sitesService * Feat: add sites router * Feat: add site endpoints * Tests: add tests * Fix: use pushed_at instead of updated_at * Style: add spacing to tests * Style: object destructuring * Style: return type and map * Fix: return last updated time instead of string representing time and update tests * Feat: throw 404 error if no staging url found * Style: destructuring response * Feat: add access check in github service * Fix: tests * Fix: test comment * Rebase: use auth verify middleware in sites router * Fix: test for config priority * Fix: use originalUrl instead of url (#388) * fix: order of ping (#392) * 0.2.0 * chore(build): add mergify workflow (#385) * chore(add mergify): add mergify workflow * chore(mergify): remove extra build step * Fix: use existing githubservice instead of axiosinstance to check for user access * Refactor: move new logic from auth route into authService * refactor: shift auth middleware methods to authMiddlewareService * Fix: await method * Rebase: inject site token middleware into routers This commit also adds a path prefix for each router when using the middleware - I realised that we were previously hitting verifyJwt multiple times because a request would sequentially hit all routers until it reached the correct one - given that there was no check on the verifyJwt use in each router, it was called repeatedly * Fix: add auth middleware to users router * Fix: auth route and service tests * Feat: use dependency injection for middleware and identity services * Refactor: move v2 endpoints into subrouters * Refactor: use subrouters for v1 endpoints * Fix: tests * Fix: move identity constants to fixtures * Fix: test folder name * Nit: variable and function name * Nit: add comments * Chore: store repeated params in variable * Nit: test name * Nit: spacing * Nit: separate condition for readability * Chore: remove unnecessary error throwing * Chore: add comment on error transformation * Chore: add log statements for errors * Fix: set userId in res.locals instead of in req * Nit: add log statement for error * Nit: add proper error message for not logged in error * Fix: rebase errors * Chore: update comment * Chore: update error message * Fix: router path * Chore: update comment * Chore: update error messages * Fix: log error instead of providing information in error Co-authored-by: seaerchin <[email protected]>
isomerpages · Mar 28, 2022 · 40cffa8 · 40cffa8
1 parent 8a4057b
commit 40cffa8
Show file tree

Hide file tree

Showing 70 changed files with 2,099 additions and 556 deletions.
diff --git a/.github/mergify.yml b/.github/mergify.yml
@@ -0,0 +1,37 @@
+pull_request_rules:
+  - name: Approve and merge non-major version dependabot upgrades
+    conditions:
+      - author~=^dependabot\[bot\]$
+      - check-success~=lint
+      - check-success~=test
+      - title~=Bump [^\s]+ from ([\d]+)\..+ to \1\.
+    actions:
+      review:
+        type: APPROVE
+      merge:
+        method: squash
+
+  - name: Approve and merge Snyk.io upgrades
+    conditions:
+      - author=snyk-bot
+      - check-success~=lint
+      - check-success~=test
+      - title~=^\[Snyk\]
+    actions:
+      review:
+        type: APPROVE
+      merge:
+        method: squash
+
+  - name: Automatically delete branches after they have been merged
+    conditions:
+      - merged
+    actions:
+      delete_head_branch:
+
+  - name: Automatically mark a PR as draft if [WIP] is in the title
+    conditions:
+      - title~=(?i)\[wip\]
+    actions:
+      edit:
+        draft: True
diff --git a/CHANGELOG.md b/CHANGELOG.md
diff --git a/README b/README
@@ -11,6 +11,6 @@ export E2E_TEST_GH_TOKEN="" // this can be your own personal GH access token, or
 ### Release
 Run the following on the release branch to tag and push changes automatically:
 ```
-npm run release --update=<versionType>
+npm run release --isomer_update=<versionType>
 ```
 where versionType corresponds to npm version types. This only works on non-Windows platforms, for Windows, modify the release script to use %npm_config_update% instead of $npm_config_update.
diff --git a/tests/constants.ts → fixtures/identity.ts b/tests/constants.ts → fixtures/identity.ts
diff --git a/fixtures/repoInfo.js b/fixtures/repoInfo.js
@@ -0,0 +1,66 @@
+const repoInfo = {
+  name: "repo",
+  private: false,
+  description:
+    "Staging: https://repo-staging.netlify.app | Production: https://repo-prod.netlify.app",
+  pushed_at: "2021-09-09T02:41:37Z",
+  permissions: {
+    admin: true,
+    maintain: true,
+    push: true,
+    triage: true,
+    pull: true,
+  },
+}
+
+const repoInfo2 = {
+  name: "repo2",
+  private: false,
+  description:
+    "Staging: https://repo2-staging.netlify.app | Production: https://repo2-prod.netlify.app",
+  pushed_at: "2021-09-09T02:41:37Z",
+  permissions: {
+    admin: true,
+    maintain: true,
+    push: true,
+    triage: true,
+    pull: true,
+  },
+}
+
+const adminRepo = {
+  name: "isomercms-backend",
+  private: false,
+  description:
+    "Staging: https://isomercms-backend-staging.netlify.app | Production: https://isomercms-backend-prod.netlify.app",
+  pushed_at: "2021-09-09T02:41:37Z",
+  permissions: {
+    admin: true,
+    maintain: true,
+    push: true,
+    triage: true,
+    pull: true,
+  },
+}
+
+const noAccessRepo = {
+  name: "noaccess",
+  private: false,
+  description:
+    "Staging: https://noaccess-staging.netlify.app | Production: https://noaccess-prod.netlify.app",
+  pushed_at: "2021-09-09T02:41:37Z",
+  permissions: {
+    admin: false,
+    maintain: false,
+    push: false,
+    triage: false,
+    pull: true,
+  },
+}
+
+module.exports = {
+  repoInfo,
+  repoInfo2,
+  adminRepo,
+  noAccessRepo,
+}
diff --git a/integration/Users.spec.ts b/integration/Users.spec.ts
@@ -3,11 +3,10 @@ import mockAxios from "jest-mock-axios"
 import request from "supertest"
 
 import { User } from "@database/models"
+import { UsersRouter as _UsersRouter } from "@root/newroutes/authenticated/users"
 import { getUsersService } from "@services/identity"
 import { sequelize } from "@tests/database"
 
-import { UsersRouter as _UsersRouter } from "../newroutes/users"
-
 // NOTE: There is a module mock set up but as this is an integration test,
 // we try to avoid mocking as much as possible and use the actual module instead.
 // This is acceptable because, unlike axios, it does not hit the network.

diff --git a/newmiddleware/auth.js b/newmiddleware/auth.js
@@ -0,0 +1,51 @@
+const autoBind = require("auto-bind")
+
+class AuthMiddleware {
+  constructor({ authMiddlewareService }) {
+    this.authMiddlewareService = authMiddlewareService
+    // We need to bind all methods because we don't invoke them from the class directly
+    autoBind(this)
+  }
+
+  verifyJwt(req, res, next) {
+    const { cookies, originalUrl: url } = req
+    const { accessToken, userId } = this.authMiddlewareService.verifyJwt({
+      cookies,
+      url,
+    })
+    req.accessToken = accessToken
+    res.locals.userId = userId
+    return next()
+  }
+
+  whoamiAuth(req, res, next) {
+    const { cookies, originalUrl: url } = req
+    const { accessToken, userId } = this.authMiddlewareService.whoamiAuth({
+      cookies,
+      url,
+    })
+    req.accessToken = accessToken
+    if (userId) res.locals.userId = userId
+    return next()
+  }
+
+  // Replace access token with site access token if it is available
+  async useSiteAccessTokenIfAvailable(req, res, next) {
+    const {
+      accessToken: userAccessToken,
+      params: { siteName },
+    } = req
+    const {
+      locals: { userId },
+    } = res
+
+    const siteAccessToken = await this.authMiddlewareService.retrieveSiteAccessTokenIfAvailable(
+      { siteName, userAccessToken, userId }
+    )
+
+    if (siteAccessToken) req.accessToken = siteAccessToken
+
+    return next()
+  }
+}
+module.exports = { AuthMiddleware }
diff --git a/newmiddleware/index.js b/newmiddleware/index.js
@@ -0,0 +1,17 @@
+const {
+  AuthMiddlewareService,
+} = require("@services/middlewareServices/AuthMiddlewareService")
+
+const { AuthMiddleware } = require("./auth")
+
+const getAuthMiddleware = ({ identityAuthService }) => {
+  const authMiddlewareService = new AuthMiddlewareService({
+    identityAuthService,
+  })
+  const authMiddleware = new AuthMiddleware({ authMiddlewareService })
+  return authMiddleware
+}
+
+module.exports = {
+  getAuthMiddleware,
+}
diff --git a/newroutes/__tests__/Auth.spec.js b/newroutes/__tests__/Auth.spec.js
@@ -0,0 +1,130 @@
+const cookieParser = require("cookie-parser")
+const express = require("express")
+const request = require("supertest")
+
+const { errorHandler } = require("@middleware/errorHandler")
+const { attachReadRouteHandlerWrapper } = require("@middleware/routeHandler")
+
+const { CSRF_COOKIE_NAME, COOKIE_NAME, AuthRouter } = require("../auth")
+
+const { FRONTEND_URL } = process.env
+const csrfState = "csrfState"
+const cookieToken = "cookieToken"
+const accessToken = undefined
+
+describe("Unlinked Pages Router", () => {
+  const mockAuthService = {
+    getAuthRedirectDetails: jest.fn(),
+    getGithubAuthToken: jest.fn(),
+    getUserInfo: jest.fn(),
+  }
+
+  const router = new AuthRouter({
+    authService: mockAuthService,
+  })
+
+  const app = express()
+  app.use(express.json({ limit: "7mb" }))
+  app.use(express.urlencoded({ extended: false }))
+  app.use(cookieParser())
+
+  // We can use read route handler here because we don't need to lock the repo
+  app.get(
+    "/github-redirect",
+    attachReadRouteHandlerWrapper(router.authRedirect)
+  )
+  app.get("/", attachReadRouteHandlerWrapper(router.githubAuth))
+  app.delete("/logout", attachReadRouteHandlerWrapper(router.logout))
+  app.get("/whoami", attachReadRouteHandlerWrapper(router.whoami))
+  app.use(errorHandler)
+
+  beforeEach(() => {
+    jest.clearAllMocks()
+  })
+
+  describe("authRedirect", () => {
+    const redirectUrl = "redirectUrl"
+    it("redirects to the specified github url", async () => {
+      mockAuthService.getAuthRedirectDetails.mockResolvedValueOnce({
+        redirectUrl,
+        cookieToken,
+      })
+
+      const resp = await request(app).get(`/github-redirect`)
+
+      expect(mockAuthService.getAuthRedirectDetails).toHaveBeenCalledTimes(1)
+      expect(resp.status).toEqual(302)
+      expect(resp.headers.location).toContain(redirectUrl)
+      expect(resp.headers["set-cookie"]).toEqual(
+        expect.arrayContaining([expect.stringContaining(CSRF_COOKIE_NAME)])
+      )
+    })
+  })
+
+  describe("githubAuth", () => {
+    const code = "code"
+    const state = "state"
+    const token = "token"
+    it("retrieves the token and redirects back to the correct page after github auth", async () => {
+      mockAuthService.getGithubAuthToken.mockResolvedValueOnce({
+        token,
+      })
+
+      const resp = await request(app)
+        .get(`/?code=${code}&state=${state}`)
+        .set("Cookie", `${CSRF_COOKIE_NAME}=${csrfState};`)
+
+      expect(mockAuthService.getGithubAuthToken).toHaveBeenCalledWith({
+        csrfState,
+        code,
+        state,
+      })
+      expect(resp.status).toEqual(302)
+      expect(resp.headers.location).toContain(`${FRONTEND_URL}/sites`)
+      expect(resp.headers["set-cookie"]).toEqual(
+        expect.arrayContaining([expect.stringContaining(COOKIE_NAME)])
+      )
+    })
+  })
+  describe("logout", () => {
+    it("removes cookies on logout", async () => {
+      const resp = await request(app)
+        .delete(`/logout`)
+        .set(
+          "Cookie",
+          `${CSRF_COOKIE_NAME}=${csrfState};${COOKIE_NAME}=${cookieToken}`
+        )
+        .expect(200)
+
+      expect(resp.headers["set-cookie"]).toEqual(
+        expect.arrayContaining([
+          expect.stringContaining(`${CSRF_COOKIE_NAME}=;`),
+          expect.stringContaining(`${COOKIE_NAME}=;`),
+        ])
+      )
+    })
+  })
+
+  describe("whoami", () => {
+    const userId = "userId"
+    it("returns user info if found", async () => {
+      const expectedResponse = {
+        userId,
+      }
+      mockAuthService.getUserInfo.mockResolvedValueOnce(expectedResponse)
+
+      const resp = await request(app).get(`/whoami`).expect(200)
+
+      expect(resp.body).toStrictEqual(expectedResponse)
+      expect(mockAuthService.getUserInfo).toHaveBeenCalledWith({ accessToken })
+    })
+
+    it("sends a 401 if user not found", async () => {
+      mockAuthService.getUserInfo.mockResolvedValueOnce(undefined)
+
+      await request(app).get(`/whoami`).expect(401)
+
+      expect(mockAuthService.getUserInfo).toHaveBeenCalledWith({ accessToken })
+    })
+  })
+})