diff --git a/server/src/server/middleware/auth.ts b/server/src/server/middleware/auth.ts index 143fdb1b1..c6a5c68ef 100644 --- a/server/src/server/middleware/auth.ts +++ b/server/src/server/middleware/auth.ts @@ -111,6 +111,13 @@ export const RequirePermissions = }); } + if (!req[SYMBOL_TachiAPIAuth].userID) { + return res.status(401).json({ + success: false, + description: `You are not authorised to perform this action.`, + }); + } + const missingPerms = []; for (const perm of perms) { if (!req[SYMBOL_TachiAPIAuth]!.permissions[perm]) { diff --git a/server/src/server/middleware/require-logged-in.test.ts b/server/src/server/middleware/require-logged-in.test.ts deleted file mode 100644 index 7fef5696b..000000000 --- a/server/src/server/middleware/require-logged-in.test.ts +++ /dev/null @@ -1,36 +0,0 @@ -import { RequireLoggedInSession } from "./require-logged-in"; -import expMiddlewareMock from "express-request-mock"; -import t from "tap"; - -t.test("#RequireLoggedIn", (t) => { - t.test("Should reject users that are not logged in.", async (t) => { - const { res } = await expMiddlewareMock(RequireLoggedInSession, { - session: { - tachi: null, - }, - }); - - t.equal(res.statusCode, 401, "Should return 401."); - - const json = res._getJSONData(); - - t.strictSame(json, { - success: false, - description: "You are not authorised to perform this action.", - }); - }); - - t.test("Should allow users that are logged in.", async (t) => { - const { res } = await expMiddlewareMock(RequireLoggedInSession, { - session: { - tachi: { - userID: 1, - }, - }, - }); - - t.equal(res.statusCode, 200, "Request should still be 200."); - }); - - t.end(); -}); diff --git a/server/src/server/middleware/require-logged-in.ts b/server/src/server/middleware/require-logged-in.ts deleted file mode 100644 index d761682f1..000000000 --- a/server/src/server/middleware/require-logged-in.ts +++ /dev/null @@ -1,29 +0,0 @@ -import { RequestHandler } from "express"; -import CreateLogCtx from "../../lib/logger/logger"; - -const logger = CreateLogCtx(__filename); - -export const RequireLoggedInSession: RequestHandler = (req, res, next) => { - if (!req.session.tachi?.userID) { - logger.info(`Received unauthorised request from ${req.ip} from ${req.originalUrl}`); - - return res.status(401).json({ - success: false, - description: `You are not authorised to perform this action.`, - }); - } - - next(); -}; - -export const RequireNotLoggedInSession: RequestHandler = (req, res, next) => { - if (req.session.tachi?.userID) { - logger.info(`Dual log-in attempted from ${req.session.tachi.userID}`); - return res.status(409).json({ - success: false, - description: `You cannot perform this while logged in.`, - }); - } - - return next(); -}; diff --git a/server/src/server/router/api/v1/auth/router.ts b/server/src/server/router/api/v1/auth/router.ts index 877183c68..3efce8e48 100644 --- a/server/src/server/router/api/v1/auth/router.ts +++ b/server/src/server/router/api/v1/auth/router.ts @@ -15,7 +15,6 @@ import { import db from "../../../../../external/mongo/db"; import CreateLogCtx from "../../../../../lib/logger/logger"; import prValidate from "../../../../middleware/prudence-validate"; -import { RequireLoggedInSession } from "../../../../middleware/require-logged-in"; const logger = CreateLogCtx(__filename); @@ -226,7 +225,14 @@ router.post( * Logs out the requesting user. * @name POST /api/v1/auth/logout */ -router.post("/logout", RequireLoggedInSession, (req, res) => { +router.post("/logout", (req, res) => { + if (!req.session?.tachi?.userID) { + return res.status(409).json({ + success: false, + description: `You are not logged in.`, + }); + } + req.session.destroy(() => 0); return res.status(200).json({ diff --git a/server/src/server/router/api/v1/import/router.test.ts b/server/src/server/router/api/v1/import/router.test.ts index f15e41def..d64ace0f4 100644 --- a/server/src/server/router/api/v1/import/router.test.ts +++ b/server/src/server/router/api/v1/import/router.test.ts @@ -7,7 +7,7 @@ import { TestingIIDXEamusementCSV27, } from "../../../../../test-utils/test-data"; import { CloseAllConnections } from "../../../../../test-utils/close-connections"; -import { RequireNeutralAuthentication } from "../../../../../test-utils/api-common"; +import { RequireAuthPerms } from "../../../../../test-utils/api-common"; import { CreateFakeAuthCookie } from "../../../../../test-utils/fake-auth"; import ResetDBState, { SetIndexesForDB } from "../../../../../test-utils/resets"; import db from "../../../../../external/mongo/db"; @@ -18,6 +18,8 @@ t.test("POST /api/v1/import/file", async (t) => { t.before(SetIndexesForDB); t.beforeEach(ResetDBState); + RequireAuthPerms("/api/v1/import/file", "submit_score", "POST"); + t.test("file/eamusement-iidx-csv", (t) => { t.beforeEach(LoadKTBlackIIDXData); diff --git a/server/src/server/router/api/v1/import/router.ts b/server/src/server/router/api/v1/import/router.ts index 853bf6a62..e1392a7fe 100644 --- a/server/src/server/router/api/v1/import/router.ts +++ b/server/src/server/router/api/v1/import/router.ts @@ -4,7 +4,6 @@ import Prudence from "prudence"; import { GetUserWithIDGuaranteed } from "../../../../../utils/user"; import CreateLogCtx, { KtLogger } from "../../../../../lib/logger/logger"; import prValidate from "../../../../middleware/prudence-validate"; -import { RequireLoggedInSession } from "../../../../middleware/require-logged-in"; import ScoreImportFatalError from "../../../../../lib/score-import/framework/score-importing/score-import-error"; import { SIXTEEN_MEGABTYES } from "../../../../../lib/constants/filesize"; import { ExpressWrappedScoreImportMain } from "../../../../../lib/score-import/framework/express-wrapper"; @@ -16,6 +15,7 @@ import { ParseSolidStateXML } from "../../../../../lib/score-import/import-types import { ParseMerIIDX } from "../../../../../lib/score-import/import-types/file/mer-iidx/parser"; import ParsePLIIIDXCSV from "../../../../../lib/score-import/import-types/file/pli-iidx-csv/parser"; import { CONF_INFO } from "../../../../../lib/setup/config"; +import { RequirePermissions } from "../../../../middleware/auth"; const logger = CreateLogCtx(__filename); @@ -33,7 +33,7 @@ const ParseMultipartScoredata = CreateMulterSingleUploadMiddleware( */ router.post( "/file", - RequireLoggedInSession, + RequirePermissions("submit_score"), ParseMultipartScoredata, prValidate( { diff --git a/server/src/server/router/ir/direct-manual/router.ts b/server/src/server/router/ir/direct-manual/router.ts index aa25671ab..588eba5c4 100644 --- a/server/src/server/router/ir/direct-manual/router.ts +++ b/server/src/server/router/ir/direct-manual/router.ts @@ -1,6 +1,5 @@ import { Router } from "express"; import { GetUserWithIDGuaranteed } from "../../../../utils/user"; -import { RequireLoggedInSession } from "../../../middleware/require-logged-in"; import { ExpressWrappedScoreImportMain } from "../../../../lib/score-import/framework/express-wrapper"; import ParseDirectManual from "../../../../lib/score-import/import-types/ir/direct-manual/parser"; import { SYMBOL_TachiAPIAuth } from "../../../../lib/constants/tachi"; @@ -12,24 +11,19 @@ const router: Router = Router({ mergeParams: true }); * Imports scores in ir/json:direct-manual form. * @name POST /ir/direct-manual/import */ -router.post( - "/import", - RequirePermissions("submit_score"), - RequireLoggedInSession, - async (req, res) => { - const userDoc = await GetUserWithIDGuaranteed(req[SYMBOL_TachiAPIAuth].userID!); +router.post("/import", RequirePermissions("submit_score"), async (req, res) => { + const userDoc = await GetUserWithIDGuaranteed(req[SYMBOL_TachiAPIAuth].userID!); - const intent = req.header("X-User-Intent"); + const intent = req.header("X-User-Intent"); - const responseData = await ExpressWrappedScoreImportMain( - userDoc, - !!intent, - "ir/direct-manual", - (logger) => ParseDirectManual(req.body, logger) - ); + const responseData = await ExpressWrappedScoreImportMain( + userDoc, + !!intent, + "ir/direct-manual", + (logger) => ParseDirectManual(req.body, logger) + ); - return res.status(responseData.statusCode).json(responseData.body); - } -); + return res.status(responseData.statusCode).json(responseData.body); +}); export default router; diff --git a/server/src/test-utils/api-common.ts b/server/src/test-utils/api-common.ts index 021b0bc26..3d2b421ec 100644 --- a/server/src/test-utils/api-common.ts +++ b/server/src/test-utils/api-common.ts @@ -1,24 +1,48 @@ import t from "tap"; import mockApi from "./mock-api"; +import { APIPermissions } from "tachi-common"; +import db from "../external/mongo/db"; +import ResetDBState from "./resets"; -export function RequireNeutralAuthentication(url: string, method: "GET" | "POST" = "GET") { - t.test(`Testing authentication for ${method} ${url}.`, async (t) => { - let res; +export function RequireAuthPerms( + url: string, + perms: APIPermissions | APIPermissions[], + method: "GET" | "POST" | "PATCH" | "PUT" | "DELETE" = "GET" +) { + t.test(`Testing permissions for ${method} ${url} [${perms}]`, async (t) => { + const m = method.toLowerCase() as Lowercase; - if (method === "GET") { - res = await mockApi.get(url); - } else { - res = await mockApi.post(url); - } + const res = await mockApi[m](url); - t.equal(res.status, 403, "Should return 403 immediately."); - t.equal( - res.body.description, - "You are not authorised to perform this action.", - "Should return an appropriate error message." - ); + // 401 if no auth given + t.equal(res.statusCode, 401); - // CloseAllConnections(); + await db["api-tokens"].insert({ + identifier: "temp_auth_perms", + permissions: {}, + token: "temp_auth", + userID: 1, + }); + + const resAuth = await mockApi[m](url).set("Authorization", "Bearer temp_auth"); + + t.equal(resAuth.statusCode, 403); + + const prm = Array.isArray(perms) ? perms : [perms]; + + await db["api-tokens"].insert({ + identifier: "temp_auth_perms2", + permissions: Object.fromEntries(prm.map((e) => [e, true])), + token: "temp_auth2", + userID: 1, + }); + + const resAuthed = await mockApi[m](url).set("Authorization", "Bearer temp_auth2"); + + t.not(resAuthed.statusCode, 401); + t.not(resAuthed.statusCode, 403); + + await ResetDBState(); t.end(); });