SoftwareBrothers / adminjs

AdminJS is an admin panel for apps written in node.js
https://adminjs.co
MIT License
8.19k stars 661 forks source link

[Feature]: Add support for 2FA login #1483

Open WeeJeWel opened 1 year ago

WeeJeWel commented 1 year ago

Description

For a sensitive tool like an Admin Web UI, 2FA is a must-have for better security.

The custom buildAuthenticatedRouter should support 2FA, and the front-end should dynamically show a 2FA input field when this is requested.

Suggested Solution

For example:

const router = AdminBroExpress.buildAuthenticatedRouter(adminBro, {
    authenticate: async (email, password, otp) => {
        const user = await User.findOne({
            email,
        });

        if(!user.isValidPassword(password)) {
          throw new Error('invalid_password');
        }

        if(user.hasOTP() && !otp) {
            throw new Error('missing_otp');
        }

        if(user.hasOTP() && !user.isValidOTP(otp)) {
            throw new Error('invalid_otp');
        }

        return user;
    },
});

The front-end should then show the 2FA input field on missing_otp, and re-submit the credentials.

Industry standard seems to be to append the OTP to the password, that's also possible.

Alternatives

No response

Additional Context

No response

dziraf commented 1 year ago

@WeeJeWel While adding another argument to authenticate function would solve your particular use case, it isn't the best solution because we might want to add extra parameters later.

Currently there are email, password and context arguments in buildAuthenticatedRouter function and adding otp now will be messy since context is neither a form input nor it's required but it comes as a third argument, so otp as fourth would be odd.

Current email, password and context arguments are also bad semantically. I'd ideally go for something like:

export interface DefaultAuthenticationArguments {
  email: string;
  password: string;
}

export type AuthenticationOptions<T = DefaultAuthenticationArguments> = {
  cookiePassword: string;
  cookieName?: string;
  authenticate: (
    params: T,
    context?: AuthenticationContext
  ) => unknown | null;
  maxRetries?: number | AuthenticationMaxRetriesOptions;
};

export const buildAuthenticatedRouter = <T = DefaultAuthenticationArguments>(
  admin: AdminJS,
  auth: AuthenticationOptions<T>,
  predefinedRouter?: express.Router | null,
  sessionOptions?: session.SessionOptions,
  formidableOptions?: FormidableOptions
): Router => { /* ... */ }

Usage:

import { buildAuthenticatedRouter, type DefaultAuthenticationArguments } from '@adminjs/express'

// ...

interface AuthenticationArgumentsWithOtp extends DefaultAuthenticationArguments {
  otp?: string;
}

const router = buildAuthenticatedRouter<AuthenticationArgumentsWithOtp>({
  admin,
  {
     authenticate: async (params: AuthenticationArgumentsWithOtp) => { /* ... */ },
     // ...
  },
  // ...
})

The framework plugin would simply pass req.fields ?? req.body ?? {} as params and the frontend will just have to make sure to send all these fields. The downside is that this solution is a breaking change, we've just released a major version and I'm reluctant to go with another major release just for this. Additionally, @adminjs/nestjs relies on @adminjs/express and we'll have to make a breaking change there as well plus we should consider whether this should also be implemented in our other plugins: @adminjs/fastify, @adminjs/koa, @adminjs/hapijs

WeeJeWel commented 1 year ago

Gotcha, thanks for the reply @dziraf.

Alternatively, what most services use anyway, is just appending the otp code (e.g. 6 digits) to the password.

The only thing you would have to do then, is show an extra input field in the front-end when a special error appears (e.g. missing_otp) and append that to password. The back-end's custom method can then substring the code from it.