Logica di login del backend Node.js.

Question

Quindi ho scritto una logica di accesso piuttosto primitiva in Node.js, che autentica l'utente e gestisce JWT. È buono in termini di sicurezza, efficienza, creazione, asincronia / sincronizzazione, registrazione. LA SICUREZZA è la mia principale preoccupazione. In un formato più carino la domanda sarebbe:

• SICUREZZA : il mio sito web è sicuro in qualche modo, forma o forma? Mi chiedo se potrei implementare misure di sicurezza diverse da quelle integrate nei metodi forniti da Node.js. Inoltre, so che le password sono chiaramente ovvie da indovinare, ma sono così per garantire l'accesso quando diversi utenti lavorano.
• EFFICIENZA : il modo in cui controllo nomi utente e password è efficiente? C'è un modo migliore per farlo?
• COSTRUZIONE : Il modo in cui ho caricato il mio sito web è accettabile? Leggere da un file e poi terminare la risposta?
• ASYNC / SYNC : So di preformare asynce syncchiamare allo stesso tempo. C'è qualche problema a questo?
• REGISTRAZIONE : registro tutte le connessioni al server e tutti i tentativi di accesso. È una buona pratica o sto esagerando con ciò che dovrebbe eseguire la registrazione? (Fonte: Login Server con Node.js )

Il mio codice è:

// 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 ci sarà una domanda separata di follow-up con la logica del frontend.

Answer 1

Bug di accesso asincrono Il try/ catcharound User.findOnenon realizza nulla perché findOneè asincrono. Quando findOnefallisce, passerà un errore al callback, ma quando il callback lo fa throw err, non c'è niente per catturare l'errore asincrono, quindi nessuna risposta verrà inviata all'utente. Un altro problema è che non stai verificando se useresiste: in caso contrario , verrà generato un errore quando provi ad accedere alla sua passwordproprietà. (Inoltre, non stai verificando se si .signtraduce in un errore o meno)

Potresti anche prendere in considerazione l'utilizzo di Promises invece di callback: findOnerestituisce già una Promise e le Promises sono spesso preferibili perché sono più facilmente concatenabili e la gestione degli errori può essere più pulita.

La sicurezza sembra ragionevole, però:

• Assicurati che le connessioni siano consentite solo tramite HTTPS. Le richieste HTTP sono intercettabili.
• Anche se il bug di accesso sopra viene risolto, la tua attuale implementazione è in grado di informare chiunque se una determinata email è registrata. Ad esempio, se conosco il tuo indirizzo email, posso provare ad accedere come te ed esaminare la risposta per vedere se la password era sbagliata o se non esiste tale email. Questo potrebbe non essere desiderabile. Se vuoi risolverlo, fai in modo che quando la password non corrisponde inserisca lo stesso blocco di quando l'e-mail non viene trovata, per renderla indistinguibile per l'utente: dovresti inviare una risposta Bad loginin entrambi i casi.
• In una nota simile, quando l'autenticazione fallisce, il codice di stato appropriato da inviare è 401 (non autorizzato), non 200.

Il modo in cui controllo nomi utente e password è efficiente?

Mi sembra del tutto normale.

REGISTRAZIONE: registro tutte le connessioni al server e tutti i tentativi di accesso. È una buona pratica

Se uno sta registrando, i tentativi di accesso sono una delle cose più importanti da registrare. Ma console.lognon è il modo giusto per farlo, almeno non da solo - diciamo che alcuni utenti erano preoccupati per i loro accessi, come esaminereste i loro recenti tentativi di accesso? Control-F-ing attraverso lo stdout dell'applicazione non è un modo molto gestibile per farlo. Non sono sicuro di quale sia lo standard del settore per questo, ma potresti prendere in considerazione il salvataggio in un database di registrazione.

Email di accesso Fai:

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

Metti l'email di richiesta in una variabile denominata submittedUser, che non suona così intuitiva per me - meglio usare un nome variabile che indica che contiene una stringa email, non un utente, come email- potresti destrutturare entrambi contemporaneamente con:

const { password, email } = req.body;

Quindi, più tardi, usa quelle variabili invece di req.bodyripeterle.

Dopo il .findOne, non dovrebbe esserci alcuna necessità da fare user.eMail == submittedUser- quel controllo dovrebbe essere superfluo dato il findOnevincolo della chiamata.

Il corpo contiene email, ma il database contiene eMail. Le maiuscole sono diverse, il che è una strana incoerenza che potrebbe causare errori di battitura e bug. Consiglierei di utilizzare un unico nome di proprietà email, probabilmente , poiché tale uso delle maiuscole è molto più comune.

Piuttosto che confrontare la corrispondenza con === true, puoi semplicemente verificare se la corrispondenza è veritiera o, un'altra opzione, per ridurre il rientro non necessario sarebbe lanciare se la corrispondenza non è veritiera e gestire gli errori in un file .catch.

Dato che .signè basato su callback, ma vuoi lavorare con Promises, rendilo basato su Promise con util.promisify.

Nel complesso spero di rendere il codice simile a questo:

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

dove logLoginAttempte logServerErrorsalvare nei database di registrazione che possono essere esaminati.