Skip to content

Commit

Permalink
Merge pull request #44 from SoftwareBrothers/fix/do-not-protect-non-a…
Browse files Browse the repository at this point in the history
…dmin-routes

fix: Do not protect non-admin routes
  • Loading branch information
dziraf authored Feb 11, 2022
2 parents eb17e5a + 1931b69 commit abb019d
Show file tree
Hide file tree
Showing 6 changed files with 70 additions and 3 deletions.
3 changes: 3 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -77,5 +77,8 @@
"ts-jest": "^26.4.4",
"ts-node": "^9.0.0",
"typescript": "^4.1.2"
},
"dependencies": {
"path-to-regexp": "^6.2.0"
}
}
24 changes: 22 additions & 2 deletions src/authentication/protected-routes.handler.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import AdminJS, { Router as AdminRouter } from "adminjs";
import { Router } from "express";
import { convertToExpressRoute } from "../convertRoutes";
import { pathToRegexp } from "path-to-regexp";

export const withProtectedRoutesHandler = (
router: Router,
Expand All @@ -8,7 +10,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 ||
Expand All @@ -17,18 +19,36 @@ 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();
}
});
};

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) => pathToRegexp(route).test(url))
);
};

const isAdminAsset = (url: string) =>
AdminRouter.assets.find((asset) => url.match(asset.path));
3 changes: 2 additions & 1 deletion src/buildRouter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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_ADMINJS_INSTANCE =
"You have to pass an instance of AdminJS to the buildRouter() function";
Expand All @@ -28,7 +29,7 @@ export const buildRouter = (

routes.forEach((route) => {
// we have to change routes defined in AdminJS 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 {
Expand Down
2 changes: 2 additions & 0 deletions src/convertRoutes.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
export const convertToExpressRoute = (adminRoute: string): string =>
adminRoute.replace(/{/g, ":").replace(/}/g, "");
36 changes: 36 additions & 0 deletions test/protected-routes.test.ts
Original file line number Diff line number Diff line change
@@ -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();
});
});
});
5 changes: 5 additions & 0 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -8431,6 +8431,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"
Expand Down

0 comments on commit abb019d

Please sign in to comment.