Logika login backend Node.js

Question

Jadi saya menulis logika login yang agak primitif di Node.js, yang mengautentikasi pengguna dan menangani JWT. Apakah ada gunanya dalam hal keamanan, efisiensi, pembangunan, asinkron / sinkronisasi, pencatatan. KEAMANAN adalah perhatian utama saya. Dalam format yang lebih cantik, pertanyaannya adalah:

• KEAMANAN : Apakah situs web saya aman dengan cara, bentuk, atau bentuk apa pun? Saya bertanya-tanya apakah saya dapat menerapkan langkah-langkah keamanan apa pun selain yang dibangun pada metode yang disediakan oleh Node.js. Juga, saya tahu kata sandinya jelas untuk ditebak, tetapi mereka seperti itu untuk memastikan masuk saat pengguna yang berbeda bekerja.
• EFISIENSI : Apakah cara saya memeriksa nama pengguna dan kata sandi efisien? Apakah ada cara yang lebih baik untuk melakukan ini?
• MEMBANGUN : Apakah cara saya memuat situs web saya dapat diterima? Membaca dari file dan kemudian mengakhiri respons?
• ASYNC / SYNC : Saya tahu saya telah melakukan sebelumnya asyncdan syncmenelepon pada waktu yang sama. Apakah ada masalah dengan ini?
• LOGGING : Saya mencatat semua koneksi ke server, dan semua upaya login. Apakah ini praktik yang baik, atau apakah saya melakukan penebangan berlebihan secara berlebihan? (Sumber: Login Server dengan Node.js )

Kode saya adalah:

// 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 akan ada pertanyaan terpisah lanjutan dengan logika Frontend.

Answer 1

Bug login Async The try/ catcharound User.findOnetidak menyelesaikan apa pun karena findOneasinkron. Ketika findOnegagal, itu akan meneruskan kesalahan ke callback, tetapi ketika callback melakukannya throw err, tidak ada yang bisa menangkap kesalahan asinkron, jadi tidak ada tanggapan yang akan dikirim ke pengguna. Masalah lainnya adalah Anda tidak memeriksa apakah userada - jika tidak ada , kesalahan akan muncul saat Anda mencoba mengakses passwordpropertinya. (Anda juga tidak memeriksa apakah .signmenghasilkan kesalahan atau tidak)

Anda juga dapat mempertimbangkan untuk menggunakan Promises daripada callback - findOnesudah mengembalikan Promise, dan Promise sering kali lebih disukai karena lebih mudah dirantai dan penanganan errornya bisa lebih bersih.

Keamanan terlihat masuk akal, meskipun:

• Pastikan koneksi hanya diizinkan melalui HTTPS. Permintaan HTTP dapat dicegat.
• Bahkan jika bug login di atas diperbaiki, implementasi Anda saat ini mampu memberi tahu siapa pun apakah email tertentu terdaftar. Misalnya, jika saya mengetahui alamat email Anda, saya dapat mencoba masuk sebagai Anda dan memeriksa tanggapan untuk melihat apakah sandi salah atau jika tidak ada email semacam itu. Ini mungkin tidak diinginkan. Jika Anda ingin memperbaikinya, mintalah ketika kata sandi tidak cocok masukkan blok yang sama seperti ketika email tidak ditemukan, untuk membuatnya tidak dapat dibedakan oleh pengguna - Anda ingin mengirim balasan Bad logindalam kedua kasus tersebut.
• Pada catatan serupa, ketika otentikasi gagal, kode status yang sesuai untuk dikirim adalah 401 (tidak sah), bukan 200.

Apakah cara saya memeriksa nama pengguna dan kata sandi efisien?

Tampak sangat normal bagiku.

LOGGING: Saya mencatat semua koneksi ke server, dan semua upaya login. Apakah ini praktik yang baik

Jika seseorang sedang masuk, upaya masuk adalah salah satu hal terpenting untuk dicatat. Tetapi console.logbukankah cara yang tepat untuk melakukannya, setidaknya tidak sendirian - katakanlah bahwa beberapa pengguna mengkhawatirkan login mereka, bagaimana Anda memeriksa upaya login mereka baru-baru ini? Control-F-ing melalui application stdout bukanlah cara yang sangat mudah untuk dilakukan. Saya tidak yakin apa standar industri untuk ini, tetapi Anda dapat mempertimbangkan untuk menyimpan ke database logging.

Login email Anda melakukan:

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

Anda memasukkan email permintaan ke dalam variabel bernama submittedUser, yang tidak terdengar begitu intuitif bagi saya - lebih baik menggunakan nama variabel yang menunjukkan bahwa itu berisi string email, bukan pengguna, seperti email- Anda dapat merusak keduanya sekaligus dengan:

const { password, email } = req.body;

Kemudian, nanti, gunakan variabel itu alih-alih melalui req.bodylagi.

Setelah itu .findOne, seharusnya tidak ada yang perlu dilakukan user.eMail == submittedUser- pemeriksaan itu harus berlebihan mengingat findOnebatasan panggilan.

Tubuh berisi email, tetapi database berisi eMail. Kapitalisasi berbeda, yang merupakan inkonsistensi aneh yang dapat menyebabkan kesalahan ketik dan bug. Saya akan merekomendasikan menggunakan satu nama properti - mungkin email, karena kapitalisasi itu jauh lebih umum.

Daripada membandingkan pertandingan === true, Anda bisa memeriksa apakah pertandingan itu benar - atau, opsi lain, untuk mengurangi lekukan yang tidak perlu adalah melempar jika pertandingan tidak benar , dan menangani kesalahan dalam file .catch.

Karena .signberbasis panggilan balik, tetapi Anda ingin bekerja dengan Janji, jadikanlah itu berbasis Janji util.promisify.

Secara keseluruhan, saya berharap membuat kodenya terlihat seperti ini:

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

di mana logLoginAttemptdan logServerErrorsimpan ke database logging yang dapat diperiksa.