Logique de connexion backend Node.js

Question

J'ai donc écrit une logique de connexion plutôt primitive dans Node.js, qui authentifie l'utilisateur et gère JWT. Est-ce que c'est bon en termes de sécurité, d'efficacité, de construction, d'async / sync, de journalisation. LA SÉCURITÉ est ma principale préoccupation. Dans un format plus joli, la question serait:

• SÉCURITÉ : Mon site Web est-il sécurisé de quelque manière, forme ou forme que ce soit? Je me demande si je pourrais mettre en œuvre des mesures de sécurité autres que celles qui sont intégrées aux méthodes fournies par Node.js. De plus, je sais que les mots de passe sont clairement évidents à deviner, mais ils le sont pour garantir la connexion lorsque différents utilisateurs fonctionnent.
• EFFICACITÉ : La façon dont je vérifie les noms d'utilisateur et les mots de passe est-elle efficace? Y a-t-il une meilleure façon de faire cela?
• BÂTIMENT : La façon dont j'ai chargé mon site Web est-elle acceptable? Lire à partir d'un fichier et terminer la réponse?
• ASYNC / SYNC : Je sais que je préforme asyncet syncappelle en même temps. Y a-t-il un problème à cela?
• LOGGING : J'enregistre toutes les connexions au serveur et toutes les tentatives de connexion. Est-ce une bonne pratique ou est-ce que j'exagère ce que la journalisation est censée accomplir? (Source: Serveur de connexion avec Node.js )

Mon code est:

// 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, il y aura une question distincte de suivi avec la logique Frontend.

Answer 1

Bogue de connexion asynchrone Le try/ catcharound User.findOnen'accomplit rien car il findOneest asynchrone. En cas d' findOneéchec, il transmettra une erreur au rappel, mais lorsque le rappel le fera throw err, rien n'est là pour attraper l'erreur asynchrone, donc aucune réponse ne sera envoyée à l'utilisateur. Un autre problème est que vous ne vérifiez pas s'il userexiste - si ce n'est pas le cas , une erreur sera générée lorsque vous essayez d'accéder à sa passwordpropriété. (Vous ne vérifiez pas non plus si cela .signentraîne une erreur ou non)

Vous pouvez également envisager d'utiliser des promesses au lieu de rappels - findOnerenvoie déjà une promesse, et les promesses sont souvent préférables car elles sont plus facilement chaînables et leur gestion des erreurs peut être plus propre.

La sécurité semble cependant raisonnable:

• Assurez-vous que les connexions ne sont autorisées que via HTTPS. Les requêtes HTTP sont interceptables.
• Même si le bogue de connexion ci-dessus est corrigé, votre implémentation actuelle est capable d'informer n'importe qui si un e-mail donné est enregistré. Par exemple, si je connais votre adresse e-mail, je peux essayer de me connecter en tant que vous et examiner la réponse pour voir si le mot de passe était incorrect ou si aucun e-mail n'existe. Cela peut ne pas être souhaitable. Si vous vouliez le réparer, lorsque le mot de passe ne correspond pas, entrez le même bloc que lorsque l'e-mail n'est pas trouvé, pour le rendre indiscernable pour l'utilisateur - vous voudrez envoyer une réponse Bad logindans les deux cas.
• De même, lorsque l'authentification échoue, le code d'état approprié à envoyer est 401 (non autorisé), et non 200.

La vérification des noms d'utilisateur et des mots de passe est-elle efficace?

Cela me semble tout à fait normal.

LOGGING: J'enregistre toutes les connexions au serveur et toutes les tentatives de connexion. Est-ce une bonne pratique

Si l'on se connecte, les tentatives de connexion sont l'une des choses les plus importantes à enregistrer. Mais ce console.logn'est pas la bonne façon de le faire, du moins pas seul - disons que certains utilisateurs étaient préoccupés par leurs connexions, comment examineriez-vous leurs récentes tentatives de connexion? Control-F-ing via l'application stdout n'est pas une façon très gérable de le faire. Je ne sais pas quelle est la norme de l'industrie à cet égard, mais vous pouvez envisager de sauvegarder dans une base de données de journalisation.

Connexion par e-mail Vous faites:

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) {

Vous mettez l'e-mail de demande dans une variable nommée submittedUser, ce qui ne me semble pas très intuitif - mieux vaut utiliser un nom de variable qui indique qu'elle contient une chaîne d'e-mail, pas un utilisateur, comme email- vous pouvez déstructurer les deux à la fois avec:

const { password, email } = req.body;

Puis, plus tard, utilisez ces variables au lieu de req.bodyrecommencer.

Après le .findOne, il ne devrait pas être nécessaire de le faire user.eMail == submittedUser- cette vérification devrait être superflue étant donné la findOnecontrainte de l' appel.

Le corps contient email, mais la base de données contient eMail. La capitalisation est différente, ce qui est une incohérence étrange qui pourrait provoquer des fautes de frappe et des bogues. Je recommanderais d'utiliser un seul nom de propriété - probablement email, car cette capitalisation est beaucoup plus courante.

Plutôt que de comparer la correspondance === true, vous pouvez simplement vérifier si la correspondance est véridique - ou, une autre option, pour réduire l'indentation inutile serait de lancer si la correspondance n'est pas véridique et de gérer les erreurs dans un fichier .catch.

Puisque .signest basé sur le rappel, mais que vous voulez travailler avec Promises, faites-le basé sur Promise avec util.promisify.

Dans l'ensemble, j'espère que le code ressemblera à ceci:

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');
    }
});

où logLoginAttemptet logServerErrorenregistrer dans les bases de données de journalisation qui peuvent être examinées.