Lógica de login de back-end Node.js

Question

Então, escrevi uma lógica de login bastante primitiva em Node.js, que autentica o usuário e lida com JWT. É bom em termos de segurança, eficiência, construção, async / sync, logging. SEGURANÇA é minha principal preocupação. Em um formato mais bonito, a pergunta seria:

• SEGURANÇA : Meu site é seguro de alguma forma, formato ou forma? Estou me perguntando se eu poderia implementar quaisquer medidas de segurança além daquelas que estão embutidas nos métodos fornecidos por Node.js. Além disso, sei que as senhas são obviamente óbvias de adivinhar, mas são assim para garantir que o login de diferentes usuários funcione.
• EFICIÊNCIA : a forma como estou verificando nomes de usuários e senhas é eficiente? Existe alguma maneira melhor de fazer isso?
• CONSTRUÇÃO : A forma como carreguei meu site é aceitável? Ler um arquivo e encerrar a resposta?
• ASSÍNC / SINCRONIZAÇÃO : Eu sei que faço apresentações asynce syncchamadas ao mesmo tempo. Existe algum problema nisso?
• LOGGING : Eu registro todas as conexões com o servidor e todas as tentativas de login. Esta é uma boa prática ou estou exagerando no que o log deve realizar? (Fonte: Servidor de Login com Node.js )

Meu código é:

// main login logic
app.post('/api/login', apiLimiter, async function (req, res) {
  // TODO: implement cookie check whether a still valid token is set and if so respond with cookie already set
  // TODO: add roles into jwt and add roles checking into other code
  // TODO: if wrong password send a response telling there's a wrong password/username
  try  {
    const pw = req.body.password;
    const submittedUser = req.body.email;
    
    User.findOne({eMail: req.body.email}, async function (err, user) {
      if (err) throw err;
      console.log(user);
      const match = await bcrypt.compare(pw, user.password);
      console.log(match);
      if (match === true && user.eMail == submittedUser) {
        jwt2.sign({user}, 'secrettokenhere', { expiresIn: '15min'}, (err, token) =>{
          res.cookie(
            'access_token', 'Bearer '+ token, {
              //domain: 'localhost',
              path: '/',
              expires: new Date(Date.now() + 900000), // cookie will be removed after 15 mins
              httpOnly: true // in production also add secure: true
            })
            .json(
              user
            );
        });
      }
      else {
        res.status(200).send("Bad login");
      }
    });    
  } catch (err) {
    res.status(500).send();
    console.log(err);
  }
});

PS: haverá uma pergunta separada de acompanhamento com a lógica do frontend.

Answer 1

Bug de login assíncrono O try/ catcharound User.findOnenão realiza nada porque findOneé assíncrono. Quando findOnefalhar, ele passará um erro para o retorno de chamada, mas quando o retorno de chamada o fizer throw err, nada estará lá para detectar o erro assíncrono, portanto nenhuma resposta será enviada ao usuário. Outro problema é que você não está verificando se userexiste - se não existir , um erro será gerado quando você tentar acessar sua passwordpropriedade. (Você também não está verificando se .signresulta em um erro ou não)

Você também pode considerar o uso de Promises em vez de callbacks - findOnejá retorna uma Promise, e Promises são geralmente preferíveis porque são mais fáceis de encadear e seu tratamento de erros pode ser mais limpo.

A segurança parece razoável, porém:

• Certifique-se de que as conexões sejam permitidas apenas por HTTPS. As solicitações HTTP são interceptáveis.
• Mesmo que o bug de login acima seja corrigido, sua implementação atual é capaz de informar a qualquer pessoa se um determinado e-mail está registrado. Por exemplo, se eu souber seu endereço de e-mail, posso tentar fazer o login como você e examinar a resposta para ver se a senha está errada ou se esse e-mail não existe. Isso pode não ser desejável. Se você quiser consertar, faça com que quando a senha não corresponder, entre no mesmo bloco de quando o e-mail não for encontrado, para torná-lo indistinguível para o usuário - você deseja enviar uma resposta de Bad loginem ambos os casos.
• Em uma observação semelhante, quando a autenticação falha, o código de status apropriado para enviar é 401 (não autorizado), não 200.

A forma como estou verificando nomes de usuário e senha é eficiente?

Parece completamente normal para mim.

LOGGING: Eu registro todas as conexões com o servidor e todas as tentativas de login. Esta é uma boa prática

Se alguém estiver logando, as tentativas de login são uma das coisas mais importantes a serem registradas. Mas console.lognão é a maneira certa de fazer isso, pelo menos não sozinho - digamos que algum usuário estivesse preocupado com seus logins, como você examinaria suas tentativas de login recentes? Control-F através do aplicativo stdout não é uma maneira muito gerenciável de fazer isso. Não tenho certeza de qual é o padrão da indústria para isso, mas você pode considerar salvar em um banco de dados de registro.

Login de e-mail Você faz:

const pw = req.body.password;
const submittedUser = req.body.email;

User.findOne({eMail: req.body.email}, async function (err, user) {
  if (err) throw err;
  console.log(user);
  const match = await bcrypt.compare(pw, user.password);
  console.log(match);
  if (match === true && user.eMail == submittedUser) {

Você coloca o e-mail de solicitação em uma variável chamada submittedUser, o que não parece muito intuitivo para mim - melhor usar um nome de variável que indica que contém uma string de e-mail, não um usuário, como email- você pode desestruturar ambos ao mesmo tempo com:

const { password, email } = req.body;

Então, mais tarde, use essas variáveis ​​em vez de percorrer req.bodynovamente.

Depois de .findOne, não deve haver nenhuma necessidade de fazer user.eMail == submittedUser- essa verificação deve ser supérflua, dada a findOnerestrição da chamada.

O corpo contém email, mas o banco de dados contém eMail. A capitalização é diferente, o que é uma inconsistência estranha que pode causar erros de digitação e erros. Eu recomendo usar um único nome de propriedade - provavelmente email, já que essa capitalização é muito mais comum.

Em vez de comparar a correspondência === true, você pode apenas verificar se a correspondência é verdadeira - ou, outra opção, para reduzir o recuo desnecessário seria lançar se a correspondência não for verdadeira e lidar com os erros em a .catch.

Uma vez que .signé baseado em callback, mas você quer trabalhar com Promises, torne-o baseado em Promise com util.promisify.

No geral , espero que o código se pareça com isto:

const jwtSignPromisified = util.promisify(jwt2.sign).bind(jwt2);
app.post('/api/login', apiLimiter, async function (req, res) {
    const failLogin = () => {
        logLoginAttempt(email, false); // or something - 2nd param indicates success
        // could also pass IP address
        res.status(401).send('Bad login');
    };
    const { password, email } = req.body;
    try {
        const user = await User.findOne({ email });
        if (!user) return failLogin();
        const match = await bcrypt.compare(password, user.password);
        if (!match) return failLogin();
        logLoginAttempt(email, true);
        const token = await jwtSignPromisified({ user }, 'secrettokenhere', { expiresIn: '15min' });
        res.cookie(
            'access_token', 'Bearer ' + token, {
            //domain: 'localhost',
            path: '/',
            expires: new Date(Date.now() + 900000), // cookie will be removed after 15 mins
            httpOnly: true // in production also add secure: true
        })
            .json(user);
    } catch (error) {
        // This should not be entered under normal circumstances:
        logServerError(error);
        res.status(500).send('Unexpected server error');
    }
});

onde logLoginAttempte logServerErrorsalvar em bancos de dados de registro que podem ser examinados.