From 7843020524d716ec7512b934b516639891e6cca1 Mon Sep 17 00:00:00 2001 From: "Lin, Hung-Shin" Date: Wed, 11 Oct 2023 14:36:20 +0800 Subject: [PATCH 01/24] feat: disable global network in player containers --- docker-compose.yml | 1 + server/src/containers/docker.js | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/docker-compose.yml b/docker-compose.yml index 1508496..f52c403 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -93,3 +93,4 @@ networks: internal: players: name: linux-odyssey-players + internal: true diff --git a/server/src/containers/docker.js b/server/src/containers/docker.js index be652f5..551b1fc 100644 --- a/server/src/containers/docker.js +++ b/server/src/containers/docker.js @@ -12,6 +12,7 @@ const containerOptions = { OpenStdin: true, StdinOnce: false, HostConfig: { + NetworkMode: config.dockerNetwork, Binds: !config.isProduction && config.hostPwd ? [ @@ -22,14 +23,13 @@ const containerOptions = { }, } -const network = engine.getNetwork(config.dockerNetwork) +// const network = engine.getNetwork(config.dockerNetwork) export async function createContainer(name) { const container = await engine.createContainer({ ...containerOptions, name, }) - await network.connect({ Container: container.id }) return container } From 11bc793b125965babba8b4340f071360b9e83f12 Mon Sep 17 00:00:00 2001 From: wancat Date: Mon, 30 Oct 2023 22:10:25 +0800 Subject: [PATCH 02/24] refactor: separate createSession to sessionManager --- .eslintrc.js | 1 + docker-compose.prod.yml | 1 + docker-compose.testing.yml | 1 + .../src/api/controllers/sessionController.js | 52 +++---------------- server/src/containers/docker.js | 8 +-- server/src/game/sessionHandler.js | 4 ++ server/src/game/sessionManager.js | 50 ++++++++++++++++++ 7 files changed, 68 insertions(+), 49 deletions(-) create mode 100644 server/src/game/sessionManager.js diff --git a/.eslintrc.js b/.eslintrc.js index b9b5429..1d3cb3d 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -27,5 +27,6 @@ module.exports = { 'no-var': 'error', 'no-plusplus': 'off', 'prettier/prettier': ['error', { endOfLine: 'auto', semi: false }], + 'import/prefer-default-export': 'off', }, } diff --git a/docker-compose.prod.yml b/docker-compose.prod.yml index aeeeec4..44bb466 100644 --- a/docker-compose.prod.yml +++ b/docker-compose.prod.yml @@ -81,6 +81,7 @@ networks: internal: players: name: linux-odyssey-players + internal: true volumes: mongo: diff --git a/docker-compose.testing.yml b/docker-compose.testing.yml index cace521..100c9e1 100644 --- a/docker-compose.testing.yml +++ b/docker-compose.testing.yml @@ -76,3 +76,4 @@ networks: internal: players: name: linux-odyssey-players + internal: true diff --git a/server/src/api/controllers/sessionController.js b/server/src/api/controllers/sessionController.js index fd8564f..601a69c 100644 --- a/server/src/api/controllers/sessionController.js +++ b/server/src/api/controllers/sessionController.js @@ -1,6 +1,5 @@ -import { Session, Quest } from '@linux-odyssey/models' -import { createContainer, deleteContainer } from '../../containers/docker.js' -import SessionHandler from '../../game/sessionHandler.js' +import { Session } from '@linux-odyssey/models' +import { createNewSession } from '../../game/sessionManager.js' function sessionSummary(session) { return { @@ -37,53 +36,14 @@ export async function getSessionList(req, res) { } } -async function deactivateSessions(user, quest) { - const sessions = await Session.find({ - user, - quest, - status: 'active', - }) - return Promise.all( - sessions.map((session) => { - return deleteContainer(session.containerId).then(() => { - session.status = 'inactive' - session.containerId = null - return session.save() - }) - }) - ) -} - export async function createSession(req, res) { - const quest = await Quest.findById(req.body.quest_id) - if (!quest) { - res.status(400).json({ message: 'Quest not found.' }) - return - } - - // deactivate all active sessions - deactivateSessions(req.user._id, quest._id).catch((err) => { - console.error(err) - }) - + const { quest_id } = req.body try { - const container = await createContainer( - `quest-${quest.id}-${req.user.username}-${Date.now()}` - ) - const newSession = new Session({ - user: req.user, - quest, - containerId: container.id, - }) - - const session = new SessionHandler(newSession) - session.addNewTasks() - - await newSession.save() - res.status(201).json(sessionDetail(newSession)) + const session = await createNewSession(req.user, quest_id) + res.status(201).json(sessionDetail(session)) } catch (err) { console.error(err) - res.status(400).json({ message: err.message }) + res.status(500).json({ message: err.message }) } } diff --git a/server/src/containers/docker.js b/server/src/containers/docker.js index 551b1fc..5f21147 100644 --- a/server/src/containers/docker.js +++ b/server/src/containers/docker.js @@ -25,17 +25,19 @@ const containerOptions = { // const network = engine.getNetwork(config.dockerNetwork) -export async function createContainer(name) { - const container = await engine.createContainer({ +export function createContainer(name) { + return engine.createContainer({ ...containerOptions, name, }) - return container } export async function getAndStartContainer(id) { console.log(`Getting container: ${id}`) const container = engine.getContainer(id) + if (!container) { + throw new Error(`Container ${id} not found`) + } if (!(await container.inspect()).State.Running) { await container.start() } diff --git a/server/src/game/sessionHandler.js b/server/src/game/sessionHandler.js index 6107305..7b11e1e 100644 --- a/server/src/game/sessionHandler.js +++ b/server/src/game/sessionHandler.js @@ -6,6 +6,10 @@ export default class SessionHandler { this.quest = session.quest } + getSession() { + return this.session + } + getTask(stage) { return this.session.tasks.find((t) => t.id === stage.id) } diff --git a/server/src/game/sessionManager.js b/server/src/game/sessionManager.js new file mode 100644 index 0000000..211f11d --- /dev/null +++ b/server/src/game/sessionManager.js @@ -0,0 +1,50 @@ +import { Session, Quest } from '@linux-odyssey/models' +import { createContainer, deleteContainer } from '../containers/docker.js' +import SessionHandler from './sessionHandler.js' + +async function deactivateSessions(user, quest) { + const sessions = await Session.find({ + user, + quest, + status: 'active', + }) + return Promise.all( + sessions.map((session) => { + return deleteContainer(session.containerId).then(() => { + session.status = 'inactive' + session.containerId = null + return session.save() + }) + }) + ) +} + +export async function createNewSession(user, questId) { + const quest = await Quest.findById(questId) + if (!quest) { + throw new Error(`Quest ${questId} not found`) + } + + // deactivate all active sessions + deactivateSessions(user._id, quest._id).catch((err) => { + console.error(err) + }) + + const container = await createContainer( + `quest-${quest.id}-${user.username}-${Date.now()}` + ) + + const sessionHandler = new SessionHandler( + new Session({ + user, + quest, + containerId: container.id, + }) + ) + + sessionHandler.addNewTasks() + + const session = sessionHandler.getSession() + await session.save() + return session +} From 5a7c4da628ac32510b3e08b5153acef091d88fc5 Mon Sep 17 00:00:00 2001 From: wancat Date: Mon, 30 Oct 2023 22:43:27 +0800 Subject: [PATCH 03/24] refactor: add getOrCreateSession to simplify process --- app/src/utils/session.js | 30 +++--------------- app/src/views/GamePage.vue | 2 +- .../src/api/controllers/sessionController.js | 16 +++++++++- server/src/api/routes/sessions.js | 1 + server/src/game/sessionManager.js | 24 +++++++++++--- server/swagger.yaml | 31 +++++++++++++++++++ 6 files changed, 72 insertions(+), 32 deletions(-) diff --git a/app/src/utils/session.js b/app/src/utils/session.js index e659bff..a1317ee 100644 --- a/app/src/utils/session.js +++ b/app/src/utils/session.js @@ -58,34 +58,14 @@ class SessionManager { this.setSession(data) } - async getSessionList() { - const res = await api.get('/sessions', { - params: { quest_id: this.questId }, + async getActiveSession() { + const res = await api.post('/sessions/active', { + quest_id: this.questId, }) - const { data } = res - return data - } - - async lastSession() { - const sessions = await this.getSessionList() - if (sessions.length === 0) { - return - } - const { _id: sessionId } = sessions[sessions.length - 1] - const { data: session } = await api.get(`/sessions/${sessionId}`) - console.log( - `Last Session ID: ${session._id}, Quest: ${session.quest}, Created At: ${session.createdAt}` - ) + const session = res.data + console.log(session) this.setSession(session) } - - async lastOrCreate() { - await this.lastSession() - if (!this.session.value) { - await this.createSession() - } - console.log(this.session.value) - } } const sessionManager = new SessionManager('helloworld') diff --git a/app/src/views/GamePage.vue b/app/src/views/GamePage.vue index 6312731..e5550bc 100644 --- a/app/src/views/GamePage.vue +++ b/app/src/views/GamePage.vue @@ -16,7 +16,7 @@ const completed = computed(() => { }) onMounted(async () => { - await sessionManager.lastOrCreate() + await sessionManager.getActiveSession() socket.connect(sessionManager.getSession()) socket.on('connect', function open() { console.log('Connected to the server.') diff --git a/server/src/api/controllers/sessionController.js b/server/src/api/controllers/sessionController.js index 601a69c..43ea7ca 100644 --- a/server/src/api/controllers/sessionController.js +++ b/server/src/api/controllers/sessionController.js @@ -1,5 +1,8 @@ import { Session } from '@linux-odyssey/models' -import { createNewSession } from '../../game/sessionManager.js' +import { + createNewSession, + getOrCreateActiveSession, +} from '../../game/sessionManager.js' function sessionSummary(session) { return { @@ -82,3 +85,14 @@ export async function deleteSessionById(req, res) { res.status(500).json({ message: err.message }) } } + +export async function getActiveSession(req, res) { + const { quest_id } = req.body + try { + const session = await getOrCreateActiveSession(req.user, quest_id) + res.json(sessionDetail(session)) + } catch (err) { + console.error(err) + res.status(500).json({ message: err.message }) + } +} diff --git a/server/src/api/routes/sessions.js b/server/src/api/routes/sessions.js index a4d15eb..d8f0b5d 100644 --- a/server/src/api/routes/sessions.js +++ b/server/src/api/routes/sessions.js @@ -5,6 +5,7 @@ const router = Router() router.get('/', sessionController.getSessionList) router.post('/', sessionController.createSession) +router.post('/active', sessionController.getActiveSession) router.get('/:id', sessionController.getSessionById) router.delete('/:id', sessionController.deleteSessionById) diff --git a/server/src/game/sessionManager.js b/server/src/game/sessionManager.js index 211f11d..409f58e 100644 --- a/server/src/game/sessionManager.js +++ b/server/src/game/sessionManager.js @@ -10,11 +10,13 @@ async function deactivateSessions(user, quest) { }) return Promise.all( sessions.map((session) => { - return deleteContainer(session.containerId).then(() => { - session.status = 'inactive' - session.containerId = null - return session.save() - }) + return deleteContainer(session.containerId) + .catch(console.error) + .finally(() => { + session.status = 'inactive' + session.containerId = null + return session.save() + }) }) ) } @@ -48,3 +50,15 @@ export async function createNewSession(user, questId) { await session.save() return session } + +export async function getOrCreateActiveSession(user, questId) { + const session = await Session.findOne({ + user, + quest: questId, + status: 'active', + }) + if (session) { + return session + } + return createNewSession(user, questId) +} diff --git a/server/swagger.yaml b/server/swagger.yaml index d52d7bc..a90f34d 100644 --- a/server/swagger.yaml +++ b/server/swagger.yaml @@ -80,6 +80,37 @@ paths: message: type: string + /sessions/active: + post: + summary: 'Get or create an active session.' + tags: + - signed-in + description: 'This endpoint returns the latest active session for the user, or creates a new one if no active session exists.' + requestBody: + required: true + content: + application/json: + schema: + type: 'object' + required: + - quest_id + properties: + quest_id: + type: 'string' + description: 'The ID of the quest.' + example: 'helloworld' + responses: + 200: + description: 'Return a active session.' + content: + application/json: + schema: + $ref: '#/components/schemas/Session' + 400: + description: 'Bad Request. The request is invalid or malformed.' + 500: + description: 'Internal Server Error.' + /sessions/{session_id}: get: tags: From 9c3c6adb1663c1294e4ca27efe7379ee289ef1e6 Mon Sep 17 00:00:00 2001 From: wancat Date: Mon, 30 Oct 2023 23:12:50 +0800 Subject: [PATCH 04/24] refactor: centralize all variable to sessionManager --- app/src/components/ControlPalette.vue | 4 ---- app/src/components/TerminalPart.vue | 6 +++--- app/src/utils/session.js | 28 +++++++++++++++++++++++++++ app/src/utils/socket.js | 16 ++++++++++++++- app/src/utils/terminal.js | 18 ++++++++--------- app/src/views/GamePage.vue | 23 +--------------------- 6 files changed, 56 insertions(+), 39 deletions(-) diff --git a/app/src/components/ControlPalette.vue b/app/src/components/ControlPalette.vue index bc14d7d..d8ad208 100644 --- a/app/src/components/ControlPalette.vue +++ b/app/src/components/ControlPalette.vue @@ -30,13 +30,9 @@ diff --git a/app/src/components/TerminalPart.vue b/app/src/components/TerminalPart.vue index 3453db1..1bf86d8 100644 --- a/app/src/components/TerminalPart.vue +++ b/app/src/components/TerminalPart.vue @@ -10,12 +10,12 @@ diff --git a/app/src/utils/session.js b/app/src/utils/session.js index a1317ee..b5655f7 100644 --- a/app/src/utils/session.js +++ b/app/src/utils/session.js @@ -1,6 +1,8 @@ import { ref } from 'vue' import { FileGraph } from '@linux-odyssey/file-graph' import api from './api' +import Socket from './socket' +import SocketTerminal from './terminal' class SessionManager { constructor(questId) { @@ -16,6 +18,30 @@ class SessionManager { ) this.pwd = ref('') this.status = ref('inactive') + this.socket = new Socket() + this.term = new SocketTerminal(40, 80) + } + + init() { + this.socket.on('terminal', (data) => { + this.term.write(data) + }) + this.term.onData((data) => { + this.socket.emit('terminal', data) + }) + this.socket.on('graph', (event) => { + this.handleGraphUpdate(event) + }) + this.socket.on('hints', (event) => { + this.handleHintUpdate(event) + }) + this.socket.on('tasks', (tasks) => { + this.setTasks(tasks) + }) + this.socket.on('status', (event) => { + this.handleStatusUpdate(event) + }) + return this.getActiveSession() } getSession() { @@ -27,6 +53,8 @@ class SessionManager { this.graph.value = new FileGraph(session.graph) this.hints.value = session.hints this.status.value = session.status + this.term.clear() + this.socket.connect(session) } handleGraphUpdate(event) { diff --git a/app/src/utils/socket.js b/app/src/utils/socket.js index 69050ca..8f5334c 100644 --- a/app/src/utils/socket.js +++ b/app/src/utils/socket.js @@ -22,6 +22,14 @@ class SocketWrapper { console.log('Socket connected to session', session._id) }) + this.socket.on('disconnect', () => { + console.log('Socket disconnected from session', session._id) + }) + + this.socket.on('error', (err) => { + console.log('Socket error', err) + }) + this.bindListeners() } @@ -32,6 +40,12 @@ class SocketWrapper { } } + once(event, callback) { + if (this.socket) { + this.socket.once(event, callback) + } + } + bindListeners() { this.listeners.forEach(({ event, callback }) => { this.socket.on(event, callback) @@ -47,4 +61,4 @@ class SocketWrapper { } } -export default new SocketWrapper() +export default SocketWrapper diff --git a/app/src/utils/terminal.js b/app/src/utils/terminal.js index be6c381..d5cf72e 100644 --- a/app/src/utils/terminal.js +++ b/app/src/utils/terminal.js @@ -1,6 +1,5 @@ import { Terminal } from 'xterm' import { FitAddon } from 'xterm-addon-fit' -import socket from './socket' import 'xterm/css/xterm.css' class SocketTerminal { @@ -24,13 +23,14 @@ class SocketTerminal { this.fitAddon = new FitAddon() this.term.loadAddon(this.fitAddon) this.fitAddon.fit() - this.socket = socket - this.socket.on('terminal', (message) => { - this.term.write(message) - }) - this.term.onData((key) => { - this.socket.emit('terminal', key) - }) + } + + write(message) { + this.term.write(message) + } + + onData(callback) { + this.term.onData(callback) } resizeScreen() { @@ -48,4 +48,4 @@ class SocketTerminal { } } -export default new SocketTerminal(40, 100) +export default SocketTerminal diff --git a/app/src/views/GamePage.vue b/app/src/views/GamePage.vue index e5550bc..075472b 100644 --- a/app/src/views/GamePage.vue +++ b/app/src/views/GamePage.vue @@ -1,6 +1,5 @@ From 252781477a425a93251e79c274a52dcc47919bdd Mon Sep 17 00:00:00 2001 From: wancat Date: Mon, 30 Oct 2023 23:31:58 +0800 Subject: [PATCH 05/24] feat: socket error handling * create a new session when failed to connect to socket * reset termnal on connect --- app/src/utils/session.js | 14 +++++++++---- app/src/utils/socket.js | 42 +++++++++++++++++++-------------------- app/src/utils/terminal.js | 8 ++++++-- server/src/api/socket.js | 6 +++--- 4 files changed, 39 insertions(+), 31 deletions(-) diff --git a/app/src/utils/session.js b/app/src/utils/session.js index b5655f7..bbf0f3b 100644 --- a/app/src/utils/session.js +++ b/app/src/utils/session.js @@ -48,13 +48,14 @@ class SessionManager { return this.session.value } - setSession(session) { + async setSession(session) { this.session.value = session this.graph.value = new FileGraph(session.graph) this.hints.value = session.hints this.status.value = session.status - this.term.clear() - this.socket.connect(session) + this.term.reset() + await this.socket.connect(session) + this.term.focus() } handleGraphUpdate(event) { @@ -92,7 +93,12 @@ class SessionManager { }) const session = res.data console.log(session) - this.setSession(session) + try { + await this.setSession(session) + } catch (err) { + console.log(err) + this.createSession().catch(console.error) + } } } diff --git a/app/src/utils/socket.js b/app/src/utils/socket.js index 8f5334c..54fbb49 100644 --- a/app/src/utils/socket.js +++ b/app/src/utils/socket.js @@ -7,30 +7,28 @@ class SocketWrapper { } connect(session) { - if (this.socket) { - this.socket.off() - this.socket.disconnect() - } + return new Promise((resolve, reject) => { + if (this.socket) { + this.socket.off() + this.socket.disconnect() + } - this.socket = io('', { - query: { - session_id: session._id, - }, + this.socket = io('', { + query: { + session_id: session._id, + }, + }) + + this.socket.on('connect', () => { + console.log('Socket connected to session', session._id) + }) + + this.socket.once('terminal', () => { + resolve() + }) + this.socket.once('error', reject) + this.bindListeners() }) - - this.socket.on('connect', () => { - console.log('Socket connected to session', session._id) - }) - - this.socket.on('disconnect', () => { - console.log('Socket disconnected from session', session._id) - }) - - this.socket.on('error', (err) => { - console.log('Socket error', err) - }) - - this.bindListeners() } on(event, callback) { diff --git a/app/src/utils/terminal.js b/app/src/utils/terminal.js index d5cf72e..528dd8d 100644 --- a/app/src/utils/terminal.js +++ b/app/src/utils/terminal.js @@ -33,6 +33,10 @@ class SocketTerminal { this.term.onData(callback) } + focus() { + this.term.focus() + } + resizeScreen() { this.fitAddon.fit() } @@ -43,8 +47,8 @@ class SocketTerminal { this.resizeScreen() } - clear() { - this.term.clear() + reset() { + this.term.reset() } } diff --git a/server/src/api/socket.js b/server/src/api/socket.js index 1c09ce4..276970e 100644 --- a/server/src/api/socket.js +++ b/server/src/api/socket.js @@ -35,7 +35,7 @@ async function connectClient(socket) { } const sessionId = socket.handshake.query.session_id if (!sessionId) { - socket.send('Session ID not found.') + socket.emit('error', 'Session ID not found.') socket.disconnect() return } @@ -46,7 +46,7 @@ async function connectClient(socket) { status: 'active', }) if (!session) { - socket.send('Session not found.') + socket.emit('error', 'Session not found.') socket.disconnect() return } @@ -61,7 +61,7 @@ async function connectClient(socket) { container = await getAndStartContainer(session.containerId) stream = await attachContainer(container, { token }) } catch (err) { - socket.send(err.message) + socket.emit('error', 'Failed to start container.') socket.disconnect() return } From c10b05c401bfc3d885f47021c761ea3682f39f45 Mon Sep 17 00:00:00 2001 From: wancat Date: Mon, 30 Oct 2023 23:46:39 +0800 Subject: [PATCH 06/24] feat: add global and session rate limit --- server/package.json | 3 ++- server/src/api/routes/sessions.js | 3 ++- server/src/index.js | 2 ++ server/src/middleware/globalRateLimit.js | 6 ++++++ server/src/middleware/sessionRateLimit.js | 9 +++++++++ yarn.lock | 5 +++++ 6 files changed, 26 insertions(+), 2 deletions(-) create mode 100644 server/src/middleware/globalRateLimit.js create mode 100644 server/src/middleware/sessionRateLimit.js diff --git a/server/package.json b/server/package.json index 9e3b98a..e78fd12 100644 --- a/server/package.json +++ b/server/package.json @@ -11,13 +11,14 @@ }, "dependencies": { "@linux-odyssey/file-graph": "*", - "@linux-odyssey/utils": "*", "@linux-odyssey/models": "*", + "@linux-odyssey/utils": "*", "bcrypt": "^5.1.1", "commander": "^11.0.0", "connect-mongo": "^5.0.0", "dockerode": "^3.3.5", "express": "^4.18.2", + "express-rate-limit": "^7.1.3", "express-session": "^1.17.3", "jsonwebtoken": "^9.0.1", "minimist": "^1.2.8", diff --git a/server/src/api/routes/sessions.js b/server/src/api/routes/sessions.js index d8f0b5d..8962045 100644 --- a/server/src/api/routes/sessions.js +++ b/server/src/api/routes/sessions.js @@ -1,10 +1,11 @@ import { Router } from 'express' import * as sessionController from '../controllers/sessionController.js' +import sessionRateLimit from '../../middleware/sessionRateLimit.js' const router = Router() router.get('/', sessionController.getSessionList) -router.post('/', sessionController.createSession) +router.post('/', sessionRateLimit, sessionController.createSession) router.post('/active', sessionController.getActiveSession) router.get('/:id', sessionController.getSessionById) router.delete('/:id', sessionController.deleteSessionById) diff --git a/server/src/index.js b/server/src/index.js index 6eb5b92..6ee7fe2 100644 --- a/server/src/index.js +++ b/server/src/index.js @@ -12,6 +12,7 @@ import apiRouter from './api/routes/index.js' import loadAndUpdateQuests from './utils/quest.js' import config from './config.js' import errorHandler from './middleware/error.js' +import globalRateLimit from './middleware/globalRateLimit.js' import sessionMiddleware from './middleware/session.js' import expiryRemovalScheduler from './containers/expiryChecker.js' import { createTestUser } from './utils/auth.js' @@ -37,6 +38,7 @@ async function main() { const server = http.createServer(app) socketServer(server) app.use(errorHandler) + app.use(globalRateLimit) app.use(sessionMiddleware) app.use(passport.session()) app.use( diff --git a/server/src/middleware/globalRateLimit.js b/server/src/middleware/globalRateLimit.js new file mode 100644 index 0000000..85cd264 --- /dev/null +++ b/server/src/middleware/globalRateLimit.js @@ -0,0 +1,6 @@ +import { rateLimit } from 'express-rate-limit' + +export default rateLimit({ + windowMs: 60 * 1000, // 1 minute + max: 20, +}) diff --git a/server/src/middleware/sessionRateLimit.js b/server/src/middleware/sessionRateLimit.js new file mode 100644 index 0000000..be008e6 --- /dev/null +++ b/server/src/middleware/sessionRateLimit.js @@ -0,0 +1,9 @@ +import { rateLimit } from 'express-rate-limit' + +export default rateLimit({ + windowMs: 10 * 1000, + max: 1, + keyGenerator: (req) => { + return req.user ? req.user._id : req.ip + }, +}) diff --git a/yarn.lock b/yarn.lock index 9e4e5e4..4967bf3 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2695,6 +2695,11 @@ express-handlebars@^7.1.2: graceful-fs "^4.2.11" handlebars "^4.7.8" +express-rate-limit@^7.1.3: + version "7.1.3" + resolved "https://registry.yarnpkg.com/express-rate-limit/-/express-rate-limit-7.1.3.tgz#0eae6c7733316f3d9403a71ad488e31e94ca0aa4" + integrity sha512-BDes6WeNYSGRRGQU8QDNwUnwqaBro28HN/TTweM3RlxXRHDld8RLoH7tbfCxAc0hamQyn6aL0KrfR45+ZxknYg== + express-session@^1.17.3: version "1.17.3" resolved "https://registry.yarnpkg.com/express-session/-/express-session-1.17.3.tgz#14b997a15ed43e5949cb1d073725675dd2777f36" From 3b9ea9ea50f719e8e4ee9543224c2d6593b0e106 Mon Sep 17 00:00:00 2001 From: wancat Date: Tue, 31 Oct 2023 00:15:39 +0800 Subject: [PATCH 07/24] feat: setup proxy and x-forwarded-for header --- app/vite.config.js | 2 ++ deployment/nginx.conf | 2 ++ server/src/index.js | 1 + 3 files changed, 5 insertions(+) diff --git a/app/vite.config.js b/app/vite.config.js index 9f8466e..433a916 100644 --- a/app/vite.config.js +++ b/app/vite.config.js @@ -9,11 +9,13 @@ export default defineConfig({ '/api': { target: process.env.API_TARGET || 'http://localhost:3000', changeOrigin: false, + xfwd: true, }, '/socket.io': { target: process.env.API_TARGET || 'ws://localhost:3000', changeOrigin: false, ws: true, + xfwd: true, }, }, }, diff --git a/deployment/nginx.conf b/deployment/nginx.conf index 278bcb1..d4ac3df 100644 --- a/deployment/nginx.conf +++ b/deployment/nginx.conf @@ -9,6 +9,7 @@ server { location /api/ { proxy_pass http://backend:3000; + proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; } location /api-docs/ { @@ -18,6 +19,7 @@ server { location /socket.io/ { proxy_pass http://backend:3000; proxy_http_version 1.1; + proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; proxy_set_header Upgrade $http_upgrade; proxy_set_header Connection "upgrade"; proxy_set_header Host $host; diff --git a/server/src/index.js b/server/src/index.js index 6ee7fe2..83c7c1d 100644 --- a/server/src/index.js +++ b/server/src/index.js @@ -37,6 +37,7 @@ async function main() { const app = express() const server = http.createServer(app) socketServer(server) + app.set('trust proxy', true) app.use(errorHandler) app.use(globalRateLimit) app.use(sessionMiddleware) From b9a79eb435c1d338b271b1726d50e10185466618 Mon Sep 17 00:00:00 2001 From: wancat Date: Tue, 31 Oct 2023 00:27:06 +0800 Subject: [PATCH 08/24] feat: add get ip endpoint * rename handler * change rate limit --- server/src/api/controllers/sessionController.js | 2 +- server/src/api/routes/index.js | 4 ++++ server/src/api/routes/sessions.js | 2 +- server/src/index.js | 2 +- server/src/middleware/globalRateLimit.js | 2 +- server/swagger.yaml | 14 ++++++++++++++ 6 files changed, 22 insertions(+), 4 deletions(-) diff --git a/server/src/api/controllers/sessionController.js b/server/src/api/controllers/sessionController.js index 43ea7ca..8bfe782 100644 --- a/server/src/api/controllers/sessionController.js +++ b/server/src/api/controllers/sessionController.js @@ -86,7 +86,7 @@ export async function deleteSessionById(req, res) { } } -export async function getActiveSession(req, res) { +export async function getOrCreateSession(req, res) { const { quest_id } = req.body try { const session = await getOrCreateActiveSession(req.user, quest_id) diff --git a/server/src/api/routes/index.js b/server/src/api/routes/index.js index fde9932..56f3b88 100644 --- a/server/src/api/routes/index.js +++ b/server/src/api/routes/index.js @@ -12,6 +12,10 @@ router.get('/', (req, res) => { res.send('Hello API!') }) +router.get('/ip', (req, res) => { + res.json({ ip: req.ip }) +}) + router.use('/auth', authRouter) router.use('/quests', quests) diff --git a/server/src/api/routes/sessions.js b/server/src/api/routes/sessions.js index 8962045..d6b5f9d 100644 --- a/server/src/api/routes/sessions.js +++ b/server/src/api/routes/sessions.js @@ -6,7 +6,7 @@ const router = Router() router.get('/', sessionController.getSessionList) router.post('/', sessionRateLimit, sessionController.createSession) -router.post('/active', sessionController.getActiveSession) +router.post('/active', sessionController.getOrCreateSession) router.get('/:id', sessionController.getSessionById) router.delete('/:id', sessionController.deleteSessionById) diff --git a/server/src/index.js b/server/src/index.js index 83c7c1d..edad513 100644 --- a/server/src/index.js +++ b/server/src/index.js @@ -38,7 +38,6 @@ async function main() { const server = http.createServer(app) socketServer(server) app.set('trust proxy', true) - app.use(errorHandler) app.use(globalRateLimit) app.use(sessionMiddleware) app.use(passport.session()) @@ -56,6 +55,7 @@ async function main() { }) app.use('/api/v1', apiRouter) + app.use(errorHandler) server.listen(config.port, config.host, () => { console.log(`Server listening at ${config.baseUrl}`) diff --git a/server/src/middleware/globalRateLimit.js b/server/src/middleware/globalRateLimit.js index 85cd264..1ca314e 100644 --- a/server/src/middleware/globalRateLimit.js +++ b/server/src/middleware/globalRateLimit.js @@ -2,5 +2,5 @@ import { rateLimit } from 'express-rate-limit' export default rateLimit({ windowMs: 60 * 1000, // 1 minute - max: 20, + max: 100, }) diff --git a/server/swagger.yaml b/server/swagger.yaml index a90f34d..b5cc28a 100644 --- a/server/swagger.yaml +++ b/server/swagger.yaml @@ -519,6 +519,20 @@ paths: description: Invalid request body '401': description: Unauthorized + /ip: + get: + summary: Get client IP address + responses: + '200': + description: IP address + content: + application/json: + schema: + type: object + properties: + ip: + type: string + example: '127.0.0.1' components: schemas: From f18ec35a9f2ed147691f51c116297c2742b4c145 Mon Sep 17 00:00:00 2001 From: "Lin, Hung-Shin" Date: Wed, 1 Nov 2023 14:19:18 +0800 Subject: [PATCH 09/24] feat: generate secure secret key and prevent default --- docker-compose.prod.yml | 1 + docker-compose.testing.yml | 1 + docker-compose.yml | 1 + package.json | 1 + server/package.json | 2 ++ server/src/auth/jwtStrategy.js | 2 +- server/src/cli/setup.js | 9 +++++++++ server/src/config.js | 11 +++++++++-- server/src/middleware/session.js | 2 +- server/src/utils/env.js | 7 ++++++- yarn.lock | 13 +++++++++++++ 11 files changed, 45 insertions(+), 5 deletions(-) create mode 100644 server/src/cli/setup.js diff --git a/docker-compose.prod.yml b/docker-compose.prod.yml index 44bb466..9dba70a 100644 --- a/docker-compose.prod.yml +++ b/docker-compose.prod.yml @@ -27,6 +27,7 @@ services: environment: - MONGO_URL=mongodb://user:user-password@db:27017/odyssey-test - DOCKER_NETWORK=linux-odyssey-players + - SECRET_KEY=${SECRET_KEY} analytics: image: lancatlin/linux-odyssey:analytics diff --git a/docker-compose.testing.yml b/docker-compose.testing.yml index 100c9e1..9b60bed 100644 --- a/docker-compose.testing.yml +++ b/docker-compose.testing.yml @@ -46,6 +46,7 @@ services: - GOOGLE_CLIENT_SECRET=${GOOGLE_CLIENT_SECRET} - GITHUB_CLIENT_ID=${GITHUB_CLIENT_ID} - GITHUB_CLIENT_SECRET=${GITHUB_CLIENT_SECRET} + - SECRET_KEY=${SECRET_KEY} db-test: image: mongo diff --git a/docker-compose.yml b/docker-compose.yml index f52c403..8c88224 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -41,6 +41,7 @@ services: - GOOGLE_CLIENT_SECRET=${GOOGLE_CLIENT_SECRET} - GITHUB_CLIENT_ID=${GITHUB_CLIENT_ID} - GITHUB_CLIENT_SECRET=${GITHUB_CLIENT_SECRET} + - SECRET_KEY=${SECRET_KEY} analytics: build: diff --git a/package.json b/package.json index cb6fbb1..601f97b 100644 --- a/package.json +++ b/package.json @@ -22,6 +22,7 @@ "test": "yarn node --experimental-vm-modules $(yarn bin jest)", "test:e2e": "yarn workspace tests test", "analytics": "yarn workspace analytics dev", + "setup": "yarn workspace server setup", "clean": "yarn workspace server clean" }, "devDependencies": { diff --git a/server/package.json b/server/package.json index e78fd12..d744625 100644 --- a/server/package.json +++ b/server/package.json @@ -7,6 +7,7 @@ "start": "node src/index.js", "dev": "nodemon src/index.js", "test": "yarn node --experimental-vm-modules $(yarn bin jest)", + "setup": "node src/cli/setup.js", "clean": "node src/cli/clean.js" }, "dependencies": { @@ -20,6 +21,7 @@ "express": "^4.18.2", "express-rate-limit": "^7.1.3", "express-session": "^1.17.3", + "express-validator": "^7.0.1", "jsonwebtoken": "^9.0.1", "minimist": "^1.2.8", "mongoose": "^7.4.0", diff --git a/server/src/auth/jwtStrategy.js b/server/src/auth/jwtStrategy.js index f1b2cd4..446b852 100644 --- a/server/src/auth/jwtStrategy.js +++ b/server/src/auth/jwtStrategy.js @@ -3,7 +3,7 @@ import config from '../config.js' const jwtOptions = { jwtFromRequest: ExtractJwt.fromAuthHeaderAsBearerToken(), - secretOrKey: config.jwtSecret, + secretOrKey: config.secret, } function verifyJwt(jwtPayload, done) { diff --git a/server/src/cli/setup.js b/server/src/cli/setup.js new file mode 100644 index 0000000..b146e3a --- /dev/null +++ b/server/src/cli/setup.js @@ -0,0 +1,9 @@ +import { genSecret } from '../utils/env.js' + +function setup() { + console.log('Setting up .env file...') + const secret = genSecret() + console.log(`SECRET_KEY=${secret}`) +} + +setup() diff --git a/server/src/config.js b/server/src/config.js index e987cf4..d51a71e 100644 --- a/server/src/config.js +++ b/server/src/config.js @@ -1,10 +1,10 @@ -import { get } from './utils/env.js' +import { get, genSecret } from './utils/env.js' const config = { host: get('HOST', 'localhost'), port: get('PORT', 3000), db: get('MONGO_URL', 'mongodb://localhost:27017/odyssey-test'), - jwtSecret: get('JWT_SECRET', 'secret'), + secret: get('SECRET_KEY', ''), dockerNetwork: get('DOCKER_NETWORK', 'linux-odyssey-players'), isProduction: process.env.NODE_ENV === 'production', hostPwd: get('HOST_PWD', ''), @@ -22,4 +22,11 @@ const config = { config.baseUrl = get('BASE_URL', `http://${config.host}:${config.port}`) +if (!config.secret) { + config.secret = genSecret() + console.warn( + 'No SECRET_KEY found in .env! A temporary secret key has been generated. To set up a persistent key, please run the setup script.' + ) +} + export default config diff --git a/server/src/middleware/session.js b/server/src/middleware/session.js index 8822b4c..38ce0a2 100644 --- a/server/src/middleware/session.js +++ b/server/src/middleware/session.js @@ -4,7 +4,7 @@ import MongoStore from 'connect-mongo' import config from '../config.js' export default session({ - secret: config.jwtSecret, + secret: config.secret, resave: false, saveUninitialized: false, rolling: true, diff --git a/server/src/utils/env.js b/server/src/utils/env.js index 3195d31..6f2c883 100644 --- a/server/src/utils/env.js +++ b/server/src/utils/env.js @@ -1,4 +1,5 @@ -/* eslint-disable import/prefer-default-export */ +import crypto from 'crypto' + export function get(key, defaultValue) { const value = process.env[key] if (value != null) { @@ -6,3 +7,7 @@ export function get(key, defaultValue) { } return defaultValue } + +export function genSecret() { + return crypto.randomUUID() +} diff --git a/yarn.lock b/yarn.lock index 4967bf3..c5994d7 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2714,6 +2714,14 @@ express-session@^1.17.3: safe-buffer "5.2.1" uid-safe "~2.1.5" +express-validator@^7.0.1: + version "7.0.1" + resolved "https://registry.yarnpkg.com/express-validator/-/express-validator-7.0.1.tgz#435fac6b5fa838763f78eca05d2206317b92106e" + integrity sha512-oB+z9QOzQIE8FnlINqyIFA8eIckahC6qc8KtqLdLJcU3/phVyuhXH3bA4qzcrhme+1RYaCSwrq+TlZ/kAKIARA== + dependencies: + lodash "^4.17.21" + validator "^13.9.0" + express@^4.18.2: version "4.18.2" resolved "https://registry.yarnpkg.com/express/-/express-4.18.2.tgz#3fabe08296e930c796c19e3c516979386ba9fd59" @@ -6084,6 +6092,11 @@ v8-to-istanbul@^9.0.1: "@types/istanbul-lib-coverage" "^2.0.1" convert-source-map "^1.6.0" +validator@^13.9.0: + version "13.11.0" + resolved "https://registry.yarnpkg.com/validator/-/validator-13.11.0.tgz#23ab3fd59290c61248364eabf4067f04955fbb1b" + integrity sha512-Ii+sehpSfZy+At5nPdnyMhx78fEoPDkR2XW/zimHEL3MyGJQOCQ7WeP20jPYRz7ZCpcKLB21NxuXHF3bxjStBQ== + vary@^1, vary@~1.1.2: version "1.1.2" resolved "https://registry.yarnpkg.com/vary/-/vary-1.1.2.tgz#2299f02c6ded30d4a5961b0b9f74524a18f634fc" From c551f0c92b69232c2214185cbcaf5ff7b3ad21dc Mon Sep 17 00:00:00 2001 From: "Lin, Hung-Shin" Date: Wed, 1 Nov 2023 14:19:45 +0800 Subject: [PATCH 10/24] fix: remove default user --- server/src/index.js | 3 --- server/src/utils/auth.js | 17 ++--------------- 2 files changed, 2 insertions(+), 18 deletions(-) diff --git a/server/src/index.js b/server/src/index.js index edad513..8960767 100644 --- a/server/src/index.js +++ b/server/src/index.js @@ -15,7 +15,6 @@ import errorHandler from './middleware/error.js' import globalRateLimit from './middleware/globalRateLimit.js' import sessionMiddleware from './middleware/session.js' import expiryRemovalScheduler from './containers/expiryChecker.js' -import { createTestUser } from './utils/auth.js' async function main() { try { @@ -26,8 +25,6 @@ async function main() { process.exit(1) } - await createTestUser() - await loadAndUpdateQuests() expiryRemovalScheduler() diff --git a/server/src/utils/auth.js b/server/src/utils/auth.js index 4f215f6..2c87137 100644 --- a/server/src/utils/auth.js +++ b/server/src/utils/auth.js @@ -1,5 +1,4 @@ import jwt from 'jsonwebtoken' -import { User } from '@linux-odyssey/models' import bcrypt from 'bcrypt' import config from '../config.js' @@ -7,23 +6,11 @@ export function hashPassword(password) { return bcrypt.hash(password, 10) } -export async function createTestUser() { - let user = await User.findOne({ username: 'alex' }) - if (!user) { - user = new User({ - username: 'alex', - email: 'alex@example.com', - }) - user.hashedPassword = await hashPassword('123456') - await user.save() - } -} - export function genJWT(payload) { return new Promise((resolve, reject) => { jwt.sign( payload, - config.jwtSecret, + config.secret, { expiresIn: config.containerExpiry }, (err, token) => { if (err) { @@ -38,7 +25,7 @@ export function genJWT(payload) { export function verifyJWT(token) { return new Promise((resolve, reject) => { - jwt.verify(token, config.jwtSecret, (err, decoded) => { + jwt.verify(token, config.secret, (err, decoded) => { if (err) { reject(err) } else { From 1c9d69a8e00eb13ee4e14f231180dd569ab93110 Mon Sep 17 00:00:00 2001 From: "Lin, Hung-Shin" Date: Wed, 1 Nov 2023 15:02:30 +0800 Subject: [PATCH 11/24] feat: add validators for username, email, password * not passed yet --- server/src/api/controllers/authController.js | 74 +------------------- server/src/api/routes/auth.js | 28 ++++++-- server/src/api/validators/authValidator.js | 43 ++++++++++++ server/src/middleware/validator.js | 13 ++++ 4 files changed, 82 insertions(+), 76 deletions(-) create mode 100644 server/src/api/validators/authValidator.js create mode 100644 server/src/middleware/validator.js diff --git a/server/src/api/controllers/authController.js b/server/src/api/controllers/authController.js index 7a87a53..5dd0232 100644 --- a/server/src/api/controllers/authController.js +++ b/server/src/api/controllers/authController.js @@ -1,4 +1,4 @@ -import { isValidEmail, isValidUsername } from '@linux-odyssey/utils' +import { matchedData } from 'express-validator' import { User } from '@linux-odyssey/models' import { genJWT, hashPassword } from '../../utils/auth.js' @@ -12,67 +12,8 @@ export async function issueToken(req, res) { res.json({ token }) } -async function invalidUsername(username) { - return !isValidUsername(username) || User.exists({ username }) -} - -async function invalidEmail(email) { - return !isValidEmail(email) || User.exists({ email }) -} - -export async function checkUsername(req, res) { - const { username, email } = req.query - const cred = username || email - if (!cred) { - res.status(400).json({ - message: 'username or email is required', - }) - return - } - - if (isValidEmail(cred)) { - res.json({ - type: 'email', - available: !(await User.exists({ email: cred })), - }) - return - } - if (isValidUsername(cred)) { - res.json({ - type: 'username', - available: !(await User.exists({ username: cred })), - }) - return - } - res.status(400).json({ - message: 'invalid username or email', - }) -} - export async function register(req, res, next) { - const { username, password, email } = req.body - if (!(username && password && email)) { - res.status(400).json({ - message: 'username, password, and email are required', - }) - return - } - - if (await invalidUsername(username)) { - res.status(409).json({ - type: 'username', - message: 'Invalid username', - }) - return - } - - if (await invalidEmail(email)) { - res.status(409).json({ - type: 'email', - message: 'Invalid email', - }) - return - } + const { username, password, email } = matchedData(req) const user = new User({ username, @@ -81,7 +22,6 @@ export async function register(req, res, next) { try { user.hashedPassword = await hashPassword(password) await user.save() - req.user = user req.login(user, (err) => { if (err) { next(err) @@ -131,7 +71,7 @@ export async function socialLogin(req, res) { } export async function registerFromSession(req, res) { - const { username } = req.body + const { username } = matchedData(req) const { newUser } = req.session if (!newUser) { @@ -141,14 +81,6 @@ export async function registerFromSession(req, res) { return } - if (await invalidUsername(username)) { - res.status(409).json({ - type: 'username', - message: 'Invalid username', - }) - return - } - newUser.username = username const user = new User(newUser) diff --git a/server/src/api/routes/auth.js b/server/src/api/routes/auth.js index 8b9954d..85d9196 100644 --- a/server/src/api/routes/auth.js +++ b/server/src/api/routes/auth.js @@ -5,12 +5,17 @@ import enabledMethods from '../../auth/passport.js' import { register, - checkUsername, checkSession, logout, socialLogin, registerFromSession, } from '../controllers/authController.js' +import { + checkNewEmail, + checkNewUsername, + checkPassword, +} from '../validators/authValidator.js' +import { noError } from '../../middleware/validator.js' const router = Router() @@ -22,12 +27,20 @@ router.post( } ) -router.post('/register', register) +router.post( + '/register', + checkNewUsername(), + checkNewEmail(), + checkPassword(), + noError, + register +) router.post('/logout', logout) -router.get('/check-username', checkUsername) - +router.get('/check-username', checkNewUsername(), noError, (req, res) => + res.json({ available: true }) +) router.get('/check-session', checkSession) router.get('/available-methods', (req, res) => { @@ -44,6 +57,11 @@ if (enabledMethods.github) { router.get('/github/callback', passport.authenticate('github'), socialLogin) } -router.post('/register-from-session', registerFromSession) +router.post( + '/register-from-session', + checkNewUsername(), + noError, + registerFromSession +) export default router diff --git a/server/src/api/validators/authValidator.js b/server/src/api/validators/authValidator.js new file mode 100644 index 0000000..9ed7172 --- /dev/null +++ b/server/src/api/validators/authValidator.js @@ -0,0 +1,43 @@ +import { query } from 'express-validator' + +import { isValidUsername } from '@linux-odyssey/utils' +import { User } from '@linux-odyssey/models' + +export const checkUsername = () => + query('username') + .notEmpty() + .isString() + .custom((username) => { + if (!isValidUsername(username)) { + throw new Error('Invalid username') + } + return true + }) + .trim() + +export const checkNewUsername = () => + checkUsername().custom(async (username) => { + if (await User.exists({ username })) { + throw new Error('Username taken') + } + return true + }) + +export const checkEmail = () => query('email').notEmpty().isEmail().trim() + +export const checkNewEmail = () => + checkEmail().custom(async (email) => { + if (await User.exists({ email })) { + throw new Error('Email taken') + } + return true + }) + +export const checkPassword = () => + query('password').notEmpty().isStrongPassword({ + minLength: 8, + minNumbers: 1, + minLowercase: 1, + minUppercase: 1, + minSymbols: 0, + }) diff --git a/server/src/middleware/validator.js b/server/src/middleware/validator.js new file mode 100644 index 0000000..8cda838 --- /dev/null +++ b/server/src/middleware/validator.js @@ -0,0 +1,13 @@ +import { validationResult } from 'express-validator' + +export function noError(req, res, next) { + const result = validationResult(req) + if (result.isEmpty()) { + next() + return + } + console.log(result.array()) + res.status(400).json({ + errors: result.array(), + }) +} From fc6ac11c60ce0ff7daf5dba2e4f37e96d2e5fbe2 Mon Sep 17 00:00:00 2001 From: wancat Date: Wed, 1 Nov 2023 21:02:01 +0800 Subject: [PATCH 12/24] feat: complete register validation * add password policy in frontend --- app/Dockerfile | 2 ++ app/package.json | 6 ++++-- app/src/components/AuthForm.vue | 3 +++ app/src/views/RegisterPage.vue | 22 +++++++++++++++------- packages/constants/index.js | 11 +++++++++++ packages/constants/package.json | 9 +++++++++ server/Dockerfile | 2 ++ server/package.json | 1 + server/src/api/validators/authValidator.js | 15 +++++---------- server/src/config.js | 7 ++++--- yarn.lock | 2 +- 11 files changed, 57 insertions(+), 23 deletions(-) create mode 100644 packages/constants/index.js create mode 100644 packages/constants/package.json diff --git a/app/Dockerfile b/app/Dockerfile index eafe05a..df72388 100644 --- a/app/Dockerfile +++ b/app/Dockerfile @@ -5,6 +5,7 @@ COPY package.json yarn.lock ./ COPY app/package.json ./app/package.json COPY packages/file-graph/package.json ./packages/file-graph/package.json COPY packages/utils/package.json ./packages/utils/package.json +COPY packages/constants/package.json ./packages/constants/package.json RUN yarn install --frozen-lockfile # Dev stage @@ -16,6 +17,7 @@ CMD [ "yarn", "dev:docker", "--host", "0.0.0.0" ] FROM base as build COPY packages/file-graph ./packages/file-graph COPY packages/utils ./packages/utils +COPY packages/constants ./packages/constants COPY app ./app RUN yarn build diff --git a/app/package.json b/app/package.json index 9746862..7152e58 100644 --- a/app/package.json +++ b/app/package.json @@ -11,15 +11,17 @@ "preview": "vite preview" }, "dependencies": { - "@linux-odyssey/file-graph": "*", - "@linux-odyssey/utils": "*", "@fortawesome/fontawesome-svg-core": "^6.4.0", "@fortawesome/free-brands-svg-icons": "^6.4.0", "@fortawesome/free-regular-svg-icons": "^6.4.0", "@fortawesome/free-solid-svg-icons": "^6.4.0", "@fortawesome/vue-fontawesome": "^3.0.3", + "@linux-odyssey/file-graph": "*", + "@linux-odyssey/utils": "*", + "@linux-odyssey/constants": "*", "axios": "^1.4.0", "socket.io-client": "^4.7.1", + "validator": "^13.11.0", "vue": "^3.3.4", "vue-router": "^4.2.4", "xterm": "^5.2.1", diff --git a/app/src/components/AuthForm.vue b/app/src/components/AuthForm.vue index d8c2088..7438907 100644 --- a/app/src/components/AuthForm.vue +++ b/app/src/components/AuthForm.vue @@ -128,6 +128,7 @@ const handleChange = () => { class="my-4 bg-background-primary text-text-primary bg- rounded-md block w-full px-3 h-10 shadow-sm focus:outline-none placeholder:text-text-line focus:ring-2 focus:ring-text-primary ring-1 ring-background-secondary" :placeholder="type === 'login' ? 'Email / Username' : 'Username'" v-model="username" + required @input="handleChange()" autocomplete="username" /> @@ -147,6 +148,7 @@ const handleChange = () => { class="my-4 bg-background-primary text-text-primary bg- rounded-md block w-full px-3 h-10 shadow-sm focus:outline-none placeholder:text-text-line focus:ring-2 focus:ring-text-primary ring-1 ring-background-secondary" placeholder="Email" v-model="email" + required @input="handleChange()" autocomplete="email" /> @@ -158,6 +160,7 @@ const handleChange = () => { class="my-4 bg-background-primary text-text-primary bg- rounded-md block w-full px-3 h-10 shadow-sm focus:outline-none placeholder:text-text-line focus:ring-2 focus:ring-text-primary ring-1 ring-background-secondary" placeholder="Password" v-model="password" + required @input="handleChange()" /> diff --git a/app/src/views/RegisterPage.vue b/app/src/views/RegisterPage.vue index f12b16d..f8cb967 100644 --- a/app/src/views/RegisterPage.vue +++ b/app/src/views/RegisterPage.vue @@ -1,5 +1,7 @@ diff --git a/server/src/api/routes/auth.js b/server/src/api/routes/auth.js index 85d9196..eb42353 100644 --- a/server/src/api/routes/auth.js +++ b/server/src/api/routes/auth.js @@ -11,34 +11,28 @@ import { registerFromSession, } from '../controllers/authController.js' import { - checkNewEmail, - checkNewUsername, - checkPassword, + checkUsernameValidators, + loginValidators, + registerFromSessionValidators, + registerValidators, } from '../validators/authValidator.js' -import { noError } from '../../middleware/validator.js' const router = Router() router.post( '/login', + loginValidators, passport.authenticate('local', { failureMessage: true }), (req, res) => { res.json({ message: 'success' }) } ) -router.post( - '/register', - checkNewUsername(), - checkNewEmail(), - checkPassword(), - noError, - register -) +router.post('/register', registerValidators, register) router.post('/logout', logout) -router.get('/check-username', checkNewUsername(), noError, (req, res) => +router.get('/check-username', checkUsernameValidators, (req, res) => res.json({ available: true }) ) router.get('/check-session', checkSession) @@ -59,8 +53,7 @@ if (enabledMethods.github) { router.post( '/register-from-session', - checkNewUsername(), - noError, + registerFromSessionValidators, registerFromSession ) diff --git a/server/src/api/validators/authValidator.js b/server/src/api/validators/authValidator.js index dbb5b11..69ded44 100644 --- a/server/src/api/validators/authValidator.js +++ b/server/src/api/validators/authValidator.js @@ -1,10 +1,11 @@ import { check } from 'express-validator' -import { isValidUsername } from '@linux-odyssey/utils' +import { isValidUsername, isValidEmail } from '@linux-odyssey/utils' import { User } from '@linux-odyssey/models' import { passwordPolicy } from '@linux-odyssey/constants' +import { noError } from '../../middleware/validator.js' -export const checkUsername = () => +const checkUsername = () => check('username') .notEmpty() .isString() @@ -16,7 +17,7 @@ export const checkUsername = () => }) .trim() -export const checkNewUsername = () => +const checkNewUsername = () => checkUsername().custom(async (username) => { if (await User.exists({ username })) { throw new Error('Username taken') @@ -24,9 +25,9 @@ export const checkNewUsername = () => return true }) -export const checkEmail = () => check('email').notEmpty().isEmail().trim() +const checkEmail = () => check('email').notEmpty().isString().isEmail().trim() -export const checkNewEmail = () => +const checkNewEmail = () => checkEmail().custom(async (email) => { if (await User.exists({ email })) { throw new Error('Email taken') @@ -34,5 +35,30 @@ export const checkNewEmail = () => return true }) -export const checkPassword = () => +const checkPassword = () => check('password').notEmpty().isStrongPassword(passwordPolicy) + +const checkLogin = () => + check('username') + .isString() + .notEmpty() + .trim() + .custom((login) => { + if (!isValidUsername(login) && !isValidEmail(login)) { + throw new Error('Invalid login') + } + return true + }) + +export const registerValidators = [ + checkNewUsername(), + checkNewEmail(), + checkPassword(), + noError, +] + +export const registerFromSessionValidators = [checkNewUsername(), noError] + +export const loginValidators = [checkLogin(), noError] + +export const checkUsernameValidators = [checkUsername(), noError] diff --git a/server/src/auth/passwordStrategy.js b/server/src/auth/passwordStrategy.js index 0993a9b..6334178 100644 --- a/server/src/auth/passwordStrategy.js +++ b/server/src/auth/passwordStrategy.js @@ -8,7 +8,7 @@ async function verifyPassword(username, password, done) { $or: [{ username }, { email: username }], }) if (!user) { - return done(null, false, { message: 'Incorrect username.' }) + return done(null, false, { message: 'Incorrect username or password.' }) } const isPasswordValid = await bcrypt.compare(password, user.hashedPassword) diff --git a/server/src/middleware/error.js b/server/src/middleware/error.js index fc1be7e..5af9688 100644 --- a/server/src/middleware/error.js +++ b/server/src/middleware/error.js @@ -1,5 +1,5 @@ // eslint-disable-next-line no-unused-vars export default function errorHandler(err, req, res, next) { console.error(err.stack) // Log error stack trace to the console - res.status(500).send({ message: err.message }) // Send error response + res.status(500).send({ message: 'Something broke!' }) // Send error response } diff --git a/server/src/middleware/validator.js b/server/src/middleware/validator.js index 8cda838..891886b 100644 --- a/server/src/middleware/validator.js +++ b/server/src/middleware/validator.js @@ -6,8 +6,8 @@ export function noError(req, res, next) { next() return } - console.log(result.array()) res.status(400).json({ + message: 'Validation failed', errors: result.array(), }) } From f65b72c5b42f7996c227bd1a8efec4eaf1d12a22 Mon Sep 17 00:00:00 2001 From: wancat Date: Wed, 1 Nov 2023 22:11:17 +0800 Subject: [PATCH 14/24] feat: add rate limit for authentication * improve error handling in frontend --- app/src/utils/auth.js | 54 ++++++++++++++++++++--- app/src/utils/errors.js | 27 ++++++++++++ app/src/views/LoginPage.vue | 18 +++++++- app/src/views/RegisterPage.vue | 30 +++++++------ server/src/api/routes/auth.js | 21 +++++++-- server/src/api/routes/sessions.js | 2 +- server/src/index.js | 2 +- server/src/middleware/globalRateLimit.js | 6 --- server/src/middleware/rateLimiter.js | 19 ++++++++ server/src/middleware/sessionRateLimit.js | 9 ---- 10 files changed, 146 insertions(+), 42 deletions(-) create mode 100644 app/src/utils/errors.js delete mode 100644 server/src/middleware/globalRateLimit.js create mode 100644 server/src/middleware/rateLimiter.js delete mode 100644 server/src/middleware/sessionRateLimit.js diff --git a/app/src/utils/auth.js b/app/src/utils/auth.js index 8e6a478..d91e2b8 100644 --- a/app/src/utils/auth.js +++ b/app/src/utils/auth.js @@ -1,5 +1,11 @@ import api from './api' +import { + TooManyRequestsError, + UnauthorizedError, + ValidationError, +} from './errors' + export async function isLoggedIn() { const res = await api.get('/auth/check-session') const { loggedIn } = res.data @@ -12,18 +18,52 @@ export const login = async (username, password) => { return true } catch (err) { if (err.response?.status === 401) { - return false + throw new UnauthorizedError(err.response.data.message) + } + if (err.response?.status === 429) { + throw new TooManyRequestsError(err.response.data.message) + } + if (err.response?.status === 400) { + throw new ValidationError(err.response.data.message) } throw err } } -export const register = (username, email, password) => { - return api.post('/auth/register', { - username, - password, - email, - }) +export const register = async (username, email, password) => { + try { + await api.post('/auth/register', { + username, + password, + email, + }) + return true + } catch (err) { + const status = err.response?.status + if (status === 429) { + throw new TooManyRequestsError(err.response.data.message) + } + if (status === 401) { + throw new UnauthorizedError(err.response.data.message) + } + if (status === 400) { + const { errors } = err.response.data + if (errors?.length > 0) { + const { path, msg } = errors[0] + switch (path) { + case 'username': + throw new ValidationError('Username is already taken') + case 'email': + throw new ValidationError('Email is already taken') + case 'password': + throw new ValidationError('Password is too weak') + default: + throw new ValidationError(msg) + } + } + } + throw err + } } export const logout = () => { diff --git a/app/src/utils/errors.js b/app/src/utils/errors.js new file mode 100644 index 0000000..2fe0cbb --- /dev/null +++ b/app/src/utils/errors.js @@ -0,0 +1,27 @@ +/* eslint-disable max-classes-per-file */ +export class TooManyRequestsError extends Error { + constructor(message) { + super(message) + this.name = 'TooManyRequestsError' + } +} + +export class ValidationError extends Error { + constructor(message) { + super(message) + this.name = 'ValidationError' + } +} + +export class UnauthorizedError extends Error { + constructor(message) { + super(message) + this.name = 'UnauthorizedError' + } +} + +export default { + TooManyRequestsError, + ValidationError, + UnauthorizedError, +} diff --git a/app/src/views/LoginPage.vue b/app/src/views/LoginPage.vue index 35e354c..0c12075 100644 --- a/app/src/views/LoginPage.vue +++ b/app/src/views/LoginPage.vue @@ -2,14 +2,30 @@ import AuthForm from '../components/AuthForm.vue' import Background from '../components/DynamicBackground.vue' +import { + TooManyRequestsError, + UnauthorizedError, + ValidationError, +} from '../utils/errors' import { login } from '../utils/auth' const handleLogin = async ({ username, password, success, error }) => { try { const isSuccess = await login(username, password) if (isSuccess) success() - else error('Wrong username or password.') } catch (err) { + if (err instanceof TooManyRequestsError) { + error('Too many requests. Try again in 2 minutes.') + return + } + if (err instanceof UnauthorizedError) { + error('Wrong username or password.') + return + } + if (err instanceof ValidationError) { + error('Invalid username or password.') + return + } console.error(err) error('Something went wrong.') } diff --git a/app/src/views/RegisterPage.vue b/app/src/views/RegisterPage.vue index f8cb967..ab8e7a9 100644 --- a/app/src/views/RegisterPage.vue +++ b/app/src/views/RegisterPage.vue @@ -7,25 +7,29 @@ import Background from '../components/DynamicBackground.vue' import api from '../utils/api' import { register } from '../utils/auth' +import { + UnauthorizedError, + TooManyRequestsError, + ValidationError, +} from '../utils/errors' function handleRegister({ username, email, password, success, error }) { register(username, email, password) .then(success) .catch((err) => { - if (err.response?.status === 409) { - // Handle username or email already exists error here - if (err.response.data.type === 'username') { - error('Username already exists.') - } else if (err.response.data.type === 'email') { - error('Email already exists.') - } else { - error('Something went wrong.') - } - } else if (err.response?.status === 400) { - error('Invalid username or email.') - } else { - console.error(err) + if (err instanceof TooManyRequestsError) { + error('Too many requests. Try again in 2 minutes.') + return } + if (err instanceof UnauthorizedError) { + error('Invalid username or password.') + return + } + if (err instanceof ValidationError) { + error(err.message) + return + } + console.error(err) }) } diff --git a/server/src/api/routes/auth.js b/server/src/api/routes/auth.js index eb42353..8bea9a9 100644 --- a/server/src/api/routes/auth.js +++ b/server/src/api/routes/auth.js @@ -16,11 +16,13 @@ import { registerFromSessionValidators, registerValidators, } from '../validators/authValidator.js' +import { authenticateRateLimit } from '../../middleware/rateLimiter.js' const router = Router() router.post( '/login', + authenticateRateLimit, loginValidators, passport.authenticate('local', { failureMessage: true }), (req, res) => { @@ -28,9 +30,9 @@ router.post( } ) -router.post('/register', registerValidators, register) +router.post('/register', authenticateRateLimit, registerValidators, register) -router.post('/logout', logout) +router.post('/logout', authenticateRateLimit, logout) router.get('/check-username', checkUsernameValidators, (req, res) => res.json({ available: true }) @@ -43,16 +45,27 @@ router.get('/available-methods', (req, res) => { if (enabledMethods.google) { router.get('/google', passport.authenticate('google')) - router.get('/google/callback', passport.authenticate('google'), socialLogin) + router.get( + '/google/callback', + authenticateRateLimit, + passport.authenticate('google'), + socialLogin + ) } if (enabledMethods.github) { router.get('/github', passport.authenticate('github')) - router.get('/github/callback', passport.authenticate('github'), socialLogin) + router.get( + '/github/callback', + authenticateRateLimit, + passport.authenticate('github'), + socialLogin + ) } router.post( '/register-from-session', + authenticateRateLimit, registerFromSessionValidators, registerFromSession ) diff --git a/server/src/api/routes/sessions.js b/server/src/api/routes/sessions.js index d6b5f9d..86961d5 100644 --- a/server/src/api/routes/sessions.js +++ b/server/src/api/routes/sessions.js @@ -1,6 +1,6 @@ import { Router } from 'express' import * as sessionController from '../controllers/sessionController.js' -import sessionRateLimit from '../../middleware/sessionRateLimit.js' +import { sessionRateLimit } from '../../middleware/rateLimiter.js' const router = Router() diff --git a/server/src/index.js b/server/src/index.js index 8960767..67f4566 100644 --- a/server/src/index.js +++ b/server/src/index.js @@ -12,7 +12,7 @@ import apiRouter from './api/routes/index.js' import loadAndUpdateQuests from './utils/quest.js' import config from './config.js' import errorHandler from './middleware/error.js' -import globalRateLimit from './middleware/globalRateLimit.js' +import { globalRateLimit } from './middleware/rateLimiter.js' import sessionMiddleware from './middleware/session.js' import expiryRemovalScheduler from './containers/expiryChecker.js' diff --git a/server/src/middleware/globalRateLimit.js b/server/src/middleware/globalRateLimit.js deleted file mode 100644 index 1ca314e..0000000 --- a/server/src/middleware/globalRateLimit.js +++ /dev/null @@ -1,6 +0,0 @@ -import { rateLimit } from 'express-rate-limit' - -export default rateLimit({ - windowMs: 60 * 1000, // 1 minute - max: 100, -}) diff --git a/server/src/middleware/rateLimiter.js b/server/src/middleware/rateLimiter.js new file mode 100644 index 0000000..85b1f5d --- /dev/null +++ b/server/src/middleware/rateLimiter.js @@ -0,0 +1,19 @@ +import rateLimit from 'express-rate-limit' + +export const globalRateLimit = rateLimit({ + windowMs: 60 * 1000, // 1 minute + max: 100, +}) + +export const authenticateRateLimit = rateLimit({ + windowMs: 2 * 60 * 1000, + max: 5, +}) + +export const sessionRateLimit = rateLimit({ + windowMs: 10 * 1000, + max: 1, + keyGenerator: (req) => { + return req.user ? req.user._id : req.ip + }, +}) diff --git a/server/src/middleware/sessionRateLimit.js b/server/src/middleware/sessionRateLimit.js deleted file mode 100644 index be008e6..0000000 --- a/server/src/middleware/sessionRateLimit.js +++ /dev/null @@ -1,9 +0,0 @@ -import { rateLimit } from 'express-rate-limit' - -export default rateLimit({ - windowMs: 10 * 1000, - max: 1, - keyGenerator: (req) => { - return req.user ? req.user._id : req.ip - }, -}) From bcfdf155595514a6b4819e089a133d02c8aff919 Mon Sep 17 00:00:00 2001 From: wancat Date: Wed, 1 Nov 2023 22:44:32 +0800 Subject: [PATCH 15/24] feat: add error handling for choose username --- app/src/utils/auth.js | 60 ++++++++++++++++---- app/src/views/ChooseUsernamePage.vue | 56 +++++++++--------- app/src/views/RegisterPage.vue | 11 +--- server/src/api/controllers/authController.js | 2 +- 4 files changed, 81 insertions(+), 48 deletions(-) diff --git a/app/src/utils/auth.js b/app/src/utils/auth.js index d91e2b8..cd9ef75 100644 --- a/app/src/utils/auth.js +++ b/app/src/utils/auth.js @@ -17,16 +17,20 @@ export const login = async (username, password) => { await api.post('/auth/login', { username, password }) return true } catch (err) { - if (err.response?.status === 401) { - throw new UnauthorizedError(err.response.data.message) + const { status } = err.response + switch (status) { + case 401: + throw new UnauthorizedError(err.response.data.message) + + case 429: + throw new TooManyRequestsError(err.response.data.message) + + case 400: + throw new ValidationError(err.response.data.message) + + default: + throw err } - if (err.response?.status === 429) { - throw new TooManyRequestsError(err.response.data.message) - } - if (err.response?.status === 400) { - throw new ValidationError(err.response.data.message) - } - throw err } } @@ -39,7 +43,7 @@ export const register = async (username, email, password) => { }) return true } catch (err) { - const status = err.response?.status + const { status } = err.response if (status === 429) { throw new TooManyRequestsError(err.response.data.message) } @@ -48,7 +52,7 @@ export const register = async (username, email, password) => { } if (status === 400) { const { errors } = err.response.data - if (errors?.length > 0) { + if (Array.isArray(errors) && errors.length > 0) { const { path, msg } = errors[0] switch (path) { case 'username': @@ -61,11 +65,45 @@ export const register = async (username, email, password) => { throw new ValidationError(msg) } } + throw new ValidationError(err.response.data.message) } throw err } } +export const checkUsername = async (username) => { + try { + await api.get('/auth/check-username', { + params: { username }, + }) + return true + } catch (err) { + const { status } = err.response + if (status === 429) { + throw new TooManyRequestsError('Too many requests') + } + if (status === 409) { + throw new ValidationError('Username is already taken') + } + throw err + } +} + +export const chooseUsername = async (username) => { + try { + await api.post('/auth/register-from-session', { username }) + } catch (err) { + switch (err.response?.status) { + case 400: + throw new ValidationError('Username is already taken') + case 401: + throw new UnauthorizedError('You are not a new user') + default: + throw err + } + } +} + export const logout = () => { return api.post('/auth/logout') } diff --git a/app/src/views/ChooseUsernamePage.vue b/app/src/views/ChooseUsernamePage.vue index decc2b4..f165c78 100644 --- a/app/src/views/ChooseUsernamePage.vue +++ b/app/src/views/ChooseUsernamePage.vue @@ -2,44 +2,46 @@ import { isValidUsername } from '@linux-odyssey/utils' import AuthForm from '../components/AuthForm.vue' import Background from '../components/DynamicBackground.vue' +import { TooManyRequestsError, ValidationError } from '../utils/errors' +import { checkUsername, chooseUsername } from '../utils/auth' -import api from '../utils/api' - -async function checkUsername({ username, error }) { +async function check({ username, error }) { if (username && !isValidUsername(username)) { error('Invalid username.') return } if (username.length > 3) { try { - const { data } = await api.get('/auth/check-username', { - params: { username }, - }) - - const { available } = data - if (!available) { - error('Username is already taken.') + await checkUsername(username) + } catch (err) { + if (err instanceof TooManyRequestsError) { + error('Too many requests. Try again in 2 minutes.') + return } - } catch { - error('Invalid username.') + if (err instanceof ValidationError) { + error(err.message) + return + } + error('Something went wrong. Please try again later.') } } } -function handleSubmit({ username, success, error }) { - api - .post('/auth/register-from-session', { - username, - }) - .then(success) - .catch((err) => { - if (err.response?.status === 409) { - error('Username is already taken.') - } else { - console.error(err.message) - error('Something went wrong.') - } - }) +async function handleSubmit({ username, success, error }) { + try { + await chooseUsername(username) + success() + } catch (err) { + if (err instanceof TooManyRequestsError) { + error('Too many requests. Try again in 2 minutes.') + return + } + if (err instanceof ValidationError) { + error(err.message) + return + } + error(err.message) + } }