Skip to content

fix: return errors to the frontend if thrown in auth provider #117

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Dec 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 32 additions & 17 deletions src/authentication/login.handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import type {
AuthenticationMaxRetriesOptions,
AuthenticationOptions,
} from "../types.js";
import { INVALID_AUTH_CONFIG_ERROR, WrongArgumentError } from "../errors.js";

const getLoginPath = (admin: AdminJS): string => {
const { loginPath, rootPath } = admin.options;
Expand Down Expand Up @@ -100,23 +101,37 @@ export const withLogin = (
const context: AuthenticationContext = { req, res };

let adminUser;
if (provider) {
adminUser = await provider.handleLogin(
{
headers: req.headers,
query: req.query,
params: req.params,
data: req.fields ?? {},
},
context
);
} else {
const { email, password } = req.fields as {
email: string;
password: string;
};
// "auth.authenticate" must always be defined if "auth.provider" isn't
adminUser = await auth.authenticate!(email, password, context);
try {
if (provider) {
adminUser = await provider.handleLogin(
{
headers: req.headers,
query: req.query,
params: req.params,
data: req.fields ?? {},
},
context
);
} else if (auth.authenticate) {
const { email, password } = req.fields as {
email: string;
password: string;
};
// "auth.authenticate" must always be defined if "auth.provider" isn't
adminUser = await auth.authenticate(email, password, context);
} else {
throw new WrongArgumentError(INVALID_AUTH_CONFIG_ERROR);
}
} catch (error) {
const errorMessage = error.message || error.error || "invalidCredentials";

const loginPage = await admin.renderLogin({
action: admin.options.loginPath,
errorMessage,
...providerProps,
});

return res.status(400).send(loginPage);
}

if (adminUser) {
Expand Down
6 changes: 5 additions & 1 deletion src/authentication/logout.handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,11 @@ export const withLogout = (

router.get(logoutPath, async (request, response) => {
if (provider) {
await provider.handleLogout({ req: request, res: response });
try {
await provider.handleLogout({ req: request, res: response });
} catch (error) {
console.error(error); // fail silently and still logout
}
}

request.session.destroy(() => {
Expand Down
12 changes: 6 additions & 6 deletions src/buildAuthenticatedRouter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,15 @@
import { withLogout } from "./authentication/logout.handler.js";
import { withProtectedRoutesHandler } from "./authentication/protected-routes.handler.js";
import { buildAssets, buildRoutes, initializeAdmin } from "./buildRouter.js";
import { OldBodyParserUsedError, WrongArgumentError } from "./errors.js";
import {
INVALID_AUTH_CONFIG_ERROR,
MISSING_AUTH_CONFIG_ERROR,
OldBodyParserUsedError,
WrongArgumentError,
} from "./errors.js";
import { AuthenticationOptions, FormidableOptions } from "./types.js";
import { withRefresh } from "./authentication/refresh.handler.js";

const MISSING_AUTH_CONFIG_ERROR =
'You must configure either "authenticate" method or assign an auth "provider"';
const INVALID_AUTH_CONFIG_ERROR =
'You cannot configure both "authenticate" and "provider". "authenticate" will be removed in next major release.';

/**
* @typedef {Function} Authenticate
* @memberof module:@adminjs/express
Expand Down Expand Up @@ -80,7 +80,7 @@
}

router.use((req, _, next) => {
if ((req as any)._body) {

Check warning on line 83 in src/buildAuthenticatedRouter.ts

View workflow job for this annotation

GitHub Actions / Test

Unexpected any. Specify a different type
next(new OldBodyParserUsedError());
}
next();
Expand All @@ -92,9 +92,9 @@
...sessionOptions,
secret: auth.cookiePassword,
name: auth.cookieName || "adminjs",
}) as any

Check warning on line 95 in src/buildAuthenticatedRouter.ts

View workflow job for this annotation

GitHub Actions / Test

Unexpected any. Specify a different type
);
router.use(formidableMiddleware(formidableOptions) as any);

Check warning on line 97 in src/buildAuthenticatedRouter.ts

View workflow job for this annotation

GitHub Actions / Test

Unexpected any. Specify a different type

withLogin(router, admin, auth);
withLogout(router, admin, auth);
Expand Down
5 changes: 5 additions & 0 deletions src/errors.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
export const MISSING_AUTH_CONFIG_ERROR =
'You must configure either "authenticate" method or assign an auth "provider"';
export const INVALID_AUTH_CONFIG_ERROR =
'You cannot configure both "authenticate" and "provider". "authenticate" will be removed in next major release.';

export class WrongArgumentError extends Error {
constructor(message: string) {
super(message);
Expand Down
Loading