From e09e304131ea48770d86394b86447a40c82f9d56 Mon Sep 17 00:00:00 2001 From: JonaszJestem Date: Tue, 19 Jan 2021 14:05:58 +0100 Subject: [PATCH 1/4] fix: Do not protect non-admin routes --- src/authentication/protected-routes.handler.ts | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/src/authentication/protected-routes.handler.ts b/src/authentication/protected-routes.handler.ts index d40d573..f0871f3 100644 --- a/src/authentication/protected-routes.handler.ts +++ b/src/authentication/protected-routes.handler.ts @@ -8,7 +8,7 @@ export const withProtectedRoutesHandler = ( const { rootPath } = admin.options; router.use((req, res, next) => { - if (AdminRouter.assets.find((asset) => req.originalUrl.match(asset.path))) { + if (isAdminAsset(req.originalUrl)) { next(); } else if ( req.session.adminUser || @@ -17,18 +17,31 @@ export const withProtectedRoutesHandler = ( req.originalUrl.startsWith(admin.options.logoutPath) ) { next(); - } else { + } else if (isAdminRoute(req.originalUrl, rootPath)) { // If the redirection is caused by API call to some action just redirect to resource const [redirectTo] = req.originalUrl.split("/actions"); req.session.redirectTo = redirectTo.includes(`${rootPath}/api`) ? rootPath : redirectTo; + req.session.save((err) => { if (err) { next(err); } res.redirect(admin.options.loginPath); }); + } else { + next(); } }); }; + +const isAdminRoute = (url: string, adminRootUrl: string) => { + const adminRoutes = AdminRouter.routes.filter((route) => route.path !== ""); + const isAdminRootUrl = url === adminRootUrl; + + return isAdminRootUrl || adminRoutes.find((route) => route.path === url); +}; + +const isAdminAsset = (url: string) => + AdminRouter.assets.find((asset) => url.match(asset.path)); From e7c51d70fe88edda46727497e5843cc0e51bcbdf Mon Sep 17 00:00:00 2001 From: JonaszJestem Date: Wed, 20 Jan 2021 09:31:42 +0100 Subject: [PATCH 2/4] fix: Do not protect non-admin routes --- package.json | 1 + .../protected-routes.handler.ts | 13 +++++-- src/buildRouter.ts | 3 +- src/convertRoutes.ts | 2 ++ test/protected-routes.test.ts | 36 +++++++++++++++++++ yarn.lock | 5 +++ 6 files changed, 56 insertions(+), 4 deletions(-) create mode 100644 src/convertRoutes.ts create mode 100644 test/protected-routes.test.ts diff --git a/package.json b/package.json index 8878a47..fcdbe42 100644 --- a/package.json +++ b/package.json @@ -67,6 +67,7 @@ "jest": "^26.6.3", "mongoose": "^5.10.15", "nodemon": "^2.0.6", + "path-to-regexp": "^6.2.0", "prettier": "^2.2.0", "semantic-release": "^17.2.4", "semantic-release-jira-releases-sb": "^0.7.2", diff --git a/src/authentication/protected-routes.handler.ts b/src/authentication/protected-routes.handler.ts index f0871f3..5a58bd3 100644 --- a/src/authentication/protected-routes.handler.ts +++ b/src/authentication/protected-routes.handler.ts @@ -1,5 +1,7 @@ import AdminBro, { Router as AdminRouter } from "admin-bro"; import { Router } from "express"; +import { convertToExpressRoute } from "../convertRoutes"; +import { pathToRegexp } from "path-to-regexp"; export const withProtectedRoutesHandler = ( router: Router, @@ -36,11 +38,16 @@ export const withProtectedRoutesHandler = ( }); }; -const isAdminRoute = (url: string, adminRootUrl: string) => { - const adminRoutes = AdminRouter.routes.filter((route) => route.path !== ""); +export const isAdminRoute = (url: string, adminRootUrl: string): boolean => { + const adminRoutes = AdminRouter.routes + .map((route) => convertToExpressRoute(route.path)) + .filter((route) => route !== ""); const isAdminRootUrl = url === adminRootUrl; - return isAdminRootUrl || adminRoutes.find((route) => route.path === url); + return ( + isAdminRootUrl || + !!adminRoutes.find((route) => pathToRegexp(route).test(url)) + ); }; const isAdminAsset = (url: string) => diff --git a/src/buildRouter.ts b/src/buildRouter.ts index 44e0515..067e563 100644 --- a/src/buildRouter.ts +++ b/src/buildRouter.ts @@ -5,6 +5,7 @@ import path from "path"; import { WrongArgumentError } from "./errors"; import { log } from "./logger"; import { FormidableOptions } from "./types"; +import { convertToExpressRoute } from "./convertRoutes"; const INVALID_ADMIN_BRO_INSTANCE = "You have to pass an instance of AdminBro to the buildRouter() function"; @@ -28,7 +29,7 @@ export const buildRouter = ( routes.forEach((route) => { // we have to change routes defined in AdminBro from {recordId} to :recordId - const expressPath = route.path.replace(/{/g, ":").replace(/}/g, ""); + const expressPath = convertToExpressRoute(route.path); const handler: RequestHandler = async (req, res, next) => { try { diff --git a/src/convertRoutes.ts b/src/convertRoutes.ts new file mode 100644 index 0000000..847b788 --- /dev/null +++ b/src/convertRoutes.ts @@ -0,0 +1,2 @@ +export const convertToExpressRoute = (adminRoute: string): string => + adminRoute.replace(/{/g, ":").replace(/}/g, ""); diff --git a/test/protected-routes.test.ts b/test/protected-routes.test.ts new file mode 100644 index 0000000..0b626b1 --- /dev/null +++ b/test/protected-routes.test.ts @@ -0,0 +1,36 @@ +import { isAdminRoute } from "../src/authentication/protected-routes.handler"; + +describe("Protected routes", () => { + describe("#isAdminRoute", () => { + it("should detect admin routes", () => { + const adminRoutes = [ + "/", + "/resources/someResource", + "/api/resources/someResource/search/searchQuery", + "/resources/someResource/actions/someAction", + "/api/resources/someResource/actions/someAction", + "/api/resources/someResource/actions/someAction/searchQuery", + "/api/resources/someResource/actions/someAction", + "/resources/someResource/records/someRecordId/someAction", + "/api/resources/someResource/records/someRecordId/someAction", + "/api/resources/someResource/records/someRecordId/someAction", + "/resources/someResource/bulk/someAction", + "/api/resources/someResource/bulk/someAction", + "/api/resources/someResource/bulk/someAction", + "/api/resources/someResource/search/", + "/api/dashboard", + "/pages/somePage", + "/api/pages/somePage", + "/api/pages/somePage", + ]; + + adminRoutes.forEach((route) => { + expect(isAdminRoute(route, "/")).toBeTruthy(); + }); + }); + + it("should detect non-admin routes", () => { + expect(isAdminRoute("/api/my-endpoint", "/")).toBeFalsy(); + }); + }); +}); diff --git a/yarn.lock b/yarn.lock index d257ead..dc17de2 100644 --- a/yarn.lock +++ b/yarn.lock @@ -8382,6 +8382,11 @@ path-to-regexp@^1.7.0: dependencies: isarray "0.0.1" +path-to-regexp@^6.2.0: + version "6.2.0" + resolved "https://registry.yarnpkg.com/path-to-regexp/-/path-to-regexp-6.2.0.tgz#f7b3803336104c346889adece614669230645f38" + integrity sha512-f66KywYG6+43afgE/8j/GoiNyygk/bnoCbps++3ErRKsIYkGGupyv07R2Ok5m9i67Iqc+T2g1eAUGUPzWhYTyg== + path-type@^2.0.0: version "2.0.0" resolved "https://registry.yarnpkg.com/path-type/-/path-type-2.0.0.tgz#f012ccb8415b7096fc2daa1054c3d72389594c73" From d573e045111adc0f3d431a2eb939e4c78946f797 Mon Sep 17 00:00:00 2001 From: JonaszJestem Date: Wed, 20 Jan 2021 16:47:13 +0100 Subject: [PATCH 3/4] fix: Move dependency --- package.json | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/package.json b/package.json index fcdbe42..e62ba55 100644 --- a/package.json +++ b/package.json @@ -67,7 +67,6 @@ "jest": "^26.6.3", "mongoose": "^5.10.15", "nodemon": "^2.0.6", - "path-to-regexp": "^6.2.0", "prettier": "^2.2.0", "semantic-release": "^17.2.4", "semantic-release-jira-releases-sb": "^0.7.2", @@ -77,5 +76,8 @@ "ts-jest": "^26.4.4", "ts-node": "^9.0.0", "typescript": "^4.1.2" + }, + "dependencies": { + "path-to-regexp": "^6.2.0" } } From 1931b697cacda290a1a4de0e1b92a9c6f95a27c5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Dzi=C4=99gielewski?= Date: Fri, 11 Feb 2022 21:28:06 +0100 Subject: [PATCH 4/4] chore: rename AdminBro mention to AdminJS --- src/buildRouter.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/buildRouter.ts b/src/buildRouter.ts index a540233..e45e651 100644 --- a/src/buildRouter.ts +++ b/src/buildRouter.ts @@ -28,7 +28,7 @@ export const buildRouter = ( router.use(formidableMiddleware(formidableOptions)); routes.forEach((route) => { - // we have to change routes defined in AdminBro from {recordId} to :recordId + // we have to change routes defined in AdminJS from {recordId} to :recordId const expressPath = convertToExpressRoute(route.path); const handler: RequestHandler = async (req, res, next) => {