Skip to content

Commit

Permalink
Feat/swap jwt to session (#619)
Browse files Browse the repository at this point in the history
* Chore: install new dependencies

* Chore: add migration

* Feat: add session middleware

* feat: replace jwt with session

* feat: update middleware

* feat: update auth routes

* chore: update method names

* Fix: tests

* chore: update .env-example

* chore: rename session middleware

* fix: use lodash isempty

* fix: .env-example

* chore: add logging to login and logout endpoints

* Fix: remove log on logout

Cookie may no longer exist

* fix: tests

* chore: fix rebase errors
  • Loading branch information
alexanderleegs authored Feb 24, 2023
1 parent 94a9daf commit 7578a8e
Show file tree
Hide file tree
Showing 17 changed files with 242 additions and 95 deletions.
3 changes: 2 additions & 1 deletion .env-example
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
export CLIENT_ID=""
export CLIENT_SECRET=""
export REDIRECT_URI="http://localhost:8081/auth"
export REDIRECT_URI="http://localhost:8081/v1/auth"
export NODE_ENV="LOCAL_DEV"
export COOKIE_DOMAIN="localhost"
export AUTH_TOKEN_EXPIRY_DURATION_IN_MILLISECONDS=3600000
export SESSION_SECRET=mysessionsecretblah
export JWT_SECRET=mysecretblah
export ENCRYPTION_SECRET=anothersecretblah
export FRONTEND_URL='http://localhost:8081'
Expand Down
77 changes: 77 additions & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
"bluebird": "^3.7.2",
"body-parser": "^1.19.0",
"cloudmersive-virus-api-client": "^1.2.7",
"connect-session-sequelize": "^7.1.5",
"cookie-parser": "~1.4.5",
"cors": "^2.8.5",
"crypto-js": "^4.1.1",
Expand All @@ -43,6 +44,7 @@
"dotenv": "^16.0.1",
"exponential-backoff": "^3.1.0",
"express": "~4.16.1",
"express-session": "^1.17.3",
"file-type": "^16.5.4",
"helmet": "^4.6.0",
"http-errors": "~1.8.0",
Expand Down Expand Up @@ -86,6 +88,7 @@
"@swc/helpers": "^0.3.8",
"@tsconfig/recommended": "^1.0.1",
"@types/express": "^4.17.13",
"@types/express-session": "^1.17.5",
"@types/jest": "^27.4.1",
"@types/lodash": "^4.14.186",
"@types/node": "^17.0.21",
Expand Down
28 changes: 28 additions & 0 deletions src/database/migrations/20230125033437-add-sessions.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
module.exports = {
async up(queryInterface, Sequelize) {
await queryInterface.createTable("sessions", {
sid: {
primaryKey: true,
type: Sequelize.STRING(36),
},
expires: {
type: Sequelize.DATE,
},
data: {
type: Sequelize.TEXT,
},
created_at: {
allowNull: false,
type: Sequelize.DATE,
},
updated_at: {
allowNull: false,
type: Sequelize.DATE,
},
})
},

async down(queryInterface) {
await queryInterface.dropTable("sessions")
},
}
13 changes: 10 additions & 3 deletions src/middleware/authentication.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,14 @@
import autoBind from "auto-bind"
import { NextFunction, Request, Response } from "express"
import { Session } from "express-session"

import UserSessionData from "@root/classes/UserSessionData"
import AuthenticationMiddlewareService from "@root/services/middlewareServices/AuthenticationMiddlewareService"
import { SessionData } from "@root/types/express/session"

interface RequestWithSession extends Request {
session: Session & SessionData
}

export class AuthenticationMiddleware {
private readonly authenticationMiddlewareService: AuthenticationMiddlewareService
Expand All @@ -17,16 +23,17 @@ export class AuthenticationMiddleware {
autoBind(this)
}

verifyJwt(req: Request, res: Response, next: NextFunction) {
const { cookies, originalUrl: url } = req
verifyAccess(req: RequestWithSession, res: Response, next: NextFunction) {
const { cookies, originalUrl: url, session } = req
const {
accessToken,
githubId,
isomerUserId,
email,
} = this.authenticationMiddlewareService.verifyJwt({
} = this.authenticationMiddlewareService.verifyAccess({
cookies,
url,
userInfo: session.userInfo,
})
const userSessionData = new UserSessionData({
accessToken,
Expand Down
32 changes: 11 additions & 21 deletions src/routes/v1/auth.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ const express = require("express")
const queryString = require("query-string")
const uuid = require("uuid/v4")

const logger = require("@logger/logger")

// Import error
const { AuthError } = require("@errors/AuthError")
const { ForbiddenError } = require("@errors/ForbiddenError")
Expand Down Expand Up @@ -111,32 +113,20 @@ async function githubAuth(req, res) {
const user = await identityServices.usersService.login(githubId)
if (!user) throw Error("Failed to create user")

const authTokenExpiry = new Date()
authTokenExpiry.setTime(authTokenExpiry.getTime() + AUTH_TOKEN_EXPIRY_MS)

const cookieSettings = {
path: "/",
expires: authTokenExpiry,
httpOnly: true,
sameSite: true,
secure:
process.env.NODE_ENV !== "DEV" &&
process.env.NODE_ENV !== "LOCAL_DEV" &&
process.env.NODE_ENV !== "test",
const userInfo = {
accessToken: jwtUtils.encryptToken(accessToken),
githubId,
isomerUserId: user.id,
}

const token = jwtUtils.signToken({
access_token: jwtUtils.encryptToken(accessToken),
user_id: githubId,
isomer_user_id: user.id,
})

res.cookie(COOKIE_NAME, token, cookieSettings)
Object.assign(req.session, { userInfo })
logger.info(`User ${userInfo.email} successfully logged in`)
return res.redirect(`${FRONTEND_URL}/sites`)
}

async function logout(req, res) {
clearAllCookies(res)
req.session.destroy()
logger.info(`User ${userInfo.email} successfully logged out`)
return res.sendStatus(200)
}

Expand Down Expand Up @@ -173,7 +163,7 @@ router.get("/", attachReadRouteHandlerWrapper(githubAuth))
router.delete("/logout", attachReadRouteHandlerWrapper(logout))
router.get(
"/whoami",
authenticationMiddleware.verifyJwt,
authenticationMiddleware.verifyAccess,
attachReadRouteHandlerWrapper(whoami)
)

Expand Down
2 changes: 1 addition & 1 deletion src/routes/v1/authenticated/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ const getAuthenticatedSubrouter = ({

const authenticatedSubrouter = express.Router({ mergeParams: true })

authenticatedSubrouter.use(authenticationMiddleware.verifyJwt)
authenticatedSubrouter.use(authenticationMiddleware.verifyAccess)

authenticatedSubrouter.use("/sites", sitesRouter)
authenticatedSubrouter.use("/user", usersRouter.getRouter())
Expand Down
2 changes: 1 addition & 1 deletion src/routes/v1/authenticatedSites/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ const getAuthenticatedSitesSubrouter = ({
}) => {
const authenticatedSitesSubrouter = express.Router({ mergeParams: true })

authenticatedSitesSubrouter.use(authenticationMiddleware.verifyJwt)
authenticatedSitesSubrouter.use(authenticationMiddleware.verifyAccess)
authenticatedSitesSubrouter.use(attachSiteHandler)
authenticatedSitesSubrouter.use(authorizationMiddleware.verifySiteMember)

Expand Down
27 changes: 21 additions & 6 deletions src/routes/v2/__tests__/Auth.spec.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
const express = require("express")
const session = require("express-session")
const request = require("supertest")

const { attachReadRouteHandlerWrapper } = require("@middleware/routeHandler")
Expand All @@ -13,9 +14,13 @@ const csrfState = "csrfState"
const cookieToken = "cookieToken"

describe("Unlinked Pages Router", () => {
jest.mock("@logger/logger", {
info: jest.fn(),
})

const mockAuthService = {
getAuthRedirectDetails: jest.fn(),
getGithubAuthToken: jest.fn(),
getUserInfoFromGithubAuth: jest.fn(),
getUserInfo: jest.fn(),
sendOtp: jest.fn(),
verifyOtp: jest.fn(),
Expand All @@ -26,6 +31,15 @@ describe("Unlinked Pages Router", () => {
})

const subrouter = express()
const options = {
resave: true,
saveUninitialized: true,
secret: "blah",
cookie: {
maxAge: 1209600000,
},
}
subrouter.use(session(options))

// We can use read route handler here because we don't need to lock the repo
subrouter.get(
Expand Down Expand Up @@ -67,24 +81,22 @@ describe("Unlinked Pages Router", () => {
const state = "state"
const token = "token"
it("retrieves the token and redirects back to the correct page after github auth", async () => {
mockAuthService.getGithubAuthToken.mockResolvedValueOnce({
mockAuthService.getUserInfoFromGithubAuth.mockResolvedValueOnce({
token,
})

const resp = await request(app)
.get(`/?code=${code}&state=${state}`)
.set("Cookie", `${CSRF_COOKIE_NAME}=${csrfState};`)

expect(mockAuthService.getGithubAuthToken).toHaveBeenCalledWith({
expect(mockAuthService.getUserInfoFromGithubAuth).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)])
)
expect(resp.headers["set-cookie"]).toBeTruthy()
})
})
describe("login", () => {
Expand All @@ -97,6 +109,9 @@ describe("Unlinked Pages Router", () => {
})
describe("verify", () => {
const mockOtp = "123456"
mockAuthService.verifyOtp.mockImplementationOnce(() => ({
email: mockEmail,
}))
it("adds the cookie on login", async () => {
mockAuthService.getAuthRedirectDetails.mockResolvedValueOnce(cookieToken)
await request(app)
Expand Down
Loading

0 comments on commit 7578a8e

Please sign in to comment.