Logika logowania do zaplecza Node.js.

Question

Dlatego napisałem dość prymitywną logikę logowania w Node.js, która uwierzytelnia użytkownika i obsługuje JWT. Czy to coś dobrego pod względem bezpieczeństwa, wydajności, budowania, asynchronizacji / synchronizacji, logowania. BEZPIECZEŃSTWO to mój główny problem. W ładniejszym formacie pytanie brzmiałoby:

• BEZPIECZEŃSTWO : Czy moja witryna internetowa jest w jakikolwiek sposób, w jakimś kształcie lub formie bezpieczna? Zastanawiam się, czy mógłbym zaimplementować inne środki bezpieczeństwa niż te, które są wbudowane w metody dostarczane przez Node.js. Wiem też, że hasła są oczywiste do odgadnięcia, ale są takie, aby zapewnić logowanie, gdy działają różni użytkownicy.
• WYDAJNOŚĆ : Czy sposób, w jaki sprawdzam nazwy użytkownika i hasło, jest skuteczny? Czy jest lepszy sposób, aby to zrobić?
• BUDYNEK : Czy sposób wczytania mojej witryny jest akceptowalny? Czytanie z pliku, a następnie kończenie odpowiedzi?
• ASYNC / SYNC : Wiem preform asynci syncpołączeń w tym samym czasie. Czy jest z tym jakiś problem?
• LOGOWANIE : Loguję wszystkie połączenia z serwerem i wszystkie próby logowania. Czy to dobra praktyka, czy też przesadzam z tym, co ma przynieść logowanie? (Źródło: Login Server with Node.js )

Mój kod to:

// 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. Kolejne, oddzielne pytanie z logiką Frontendu.

Answer 1

Błąd logowania asynchronicznego Opcja try/ catcharound User.findOnenic nie daje, ponieważ findOnejest asynchroniczna. Gdy się findOnenie powiedzie, przekaże błąd do wywołania zwrotnego, ale gdy to zadziała throw err, nie ma nic, co mogłoby przechwycić błąd asynchroniczny, więc żadna odpowiedź nie zostanie wysłana do użytkownika. Inną kwestią jest to, że nie sprawdzasz, czy useristnieje - jeśli tak nie jest , podczas próby uzyskania dostępu do jego passwordwłaściwości zostanie wyrzucony błąd . (Nie sprawdzasz również, czy .signskutkuje to błędem, czy nie)

Możesz również rozważyć użycie Obietnic zamiast wywołań zwrotnych - findOnejuż zwraca Obietnicę, a Obietnice są często preferowane, ponieważ są łatwiejsze do łączenia w łańcuch, a ich obsługa błędów może być czystsza.

Bezpieczeństwo wygląda jednak rozsądnie:

• Upewnij się, że połączenia są dozwolone tylko przez HTTPS. Żądania HTTP są przechwytywane.
• Nawet jeśli powyższy błąd logowania zostanie naprawiony, Twoja obecna implementacja jest w stanie poinformować każdego, czy dany e-mail jest zarejestrowany. Na przykład, jeśli znam Twój adres e-mail, mogę spróbować zalogować się jako Ty i sprawdzić odpowiedź, aby sprawdzić, czy hasło jest błędne lub czy taki e-mail nie istnieje. To może nie być pożądane. Jeśli chcesz to naprawić, jeśli hasło nie pasuje, wprowadź ten sam blok, co w przypadku, gdy wiadomość e-mail nie zostanie znaleziona, aby użytkownik nie mógł go odróżnić - Bad loginw obu przypadkach chcesz wysłać odpowiedź .
• Podobnie, gdy uwierzytelnianie się nie powiedzie, odpowiedni kod statusu do wysłania to 401 (nieautoryzowany), a nie 200.

Czy sposób, w jaki sprawdzam nazwy użytkownika i hasło, jest skuteczny?

Dla mnie wygląda zupełnie normalnie.

LOGOWANIE: Loguję wszystkie połączenia z serwerem i wszystkie próby logowania. Czy to dobra praktyka

Jeśli się logujesz, próby logowania są jedną z najważniejszych rzeczy do logowania. Ale console.logczy nie jest to właściwy sposób, przynajmniej nie sam - powiedz, że jakiś użytkownik był zaniepokojony swoimi logowaniami, jak przeanalizowałbyś ich ostatnie próby logowania? Sterowanie poprzez stdout aplikacji nie jest bardzo łatwym sposobem na zrobienie tego. Nie jestem pewien, jaki jest tego standard branżowy, ale możesz rozważyć zapisanie w bazie danych logowania.

Logowanie przez e-mail Robisz:

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

Umieszczasz wiadomość e-mail z żądaniem w zmiennej o nazwie submittedUser, która nie brzmi dla mnie zbyt intuicyjnie - lepiej użyć nazwy zmiennej, która wskazuje, że zawiera ciąg wiadomości e-mail, a nie użytkownika, na przykład email- możesz zniszczyć oba naraz za pomocą:

const { password, email } = req.body;

Później użyj tych zmiennych, zamiast przechodzić req.bodyponownie.

Po zakończeniu .findOnenie powinno być potrzeby user.eMail == submittedUser- to sprawdzenie powinno być zbędne, biorąc pod uwagę findOneograniczenie połączenia.

Treść zawiera email, ale baza danych zawiera eMail. Wielkość liter jest inna, co jest dziwną niespójnością, która może powodować literówki i błędy. Zalecałbym użycie jednej nazwy właściwości - prawdopodobnie emaildlatego, że ta wielkość liter jest znacznie bardziej powszechna.

Zamiast porównywać dopasowanie z === truewynikami, możesz po prostu sprawdzić, czy dopasowanie jest zgodne z prawdą - lub inną opcją, aby zmniejszyć niepotrzebne wcięcia, byłoby wyrzucenie, jeśli dopasowanie nie jest zgodne z prawdą, i obsłużenie błędów w pliku .catch.

Ponieważ .signjest oparty na wywołaniach zwrotnych, ale chcesz pracować z Promises, ustaw go na Promise util.promisify.

Ogólnie mam nadzieję, że kod będzie wyglądał mniej więcej tak:

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

gdzie logLoginAttempti logServerErrorzapisać w rejestrowanych bazach danych, które można sprawdzić.