firebase / firebase-admin-node

Firebase Admin Node.js SDK
https://firebase.google.com/docs/admin/setup
Apache License 2.0
1.59k stars 359 forks source link

import HMAC1 users with different hash algorithm keys one after another not setting correct passwords #2590

Open jeffj6123 opened 1 month ago

jeffj6123 commented 1 month ago

[READ] Step 1: Are you in the right place?

[REQUIRED] Step 2: Describe your environment

[REQUIRED] Step 3: Describe the problem

When using the importUsers function, it is not correctly importing users where each one has a different hash key.

I am importing users where each user's password was created using HMAC sha1 but each user had its own key. I was trying to import 1 user per function call but when I import say 100 users one at a time, the majority of them import incorrectly(code below should make this easy to understand) . If I rerun a subset of around 10 users using the exact same information passed in they then will login correctly.

Is there some internal limitation or batching happening? I am importing users one at a time and respecting the API quotas and they are successful imports. This has been very confusing to understand the correct way to handle this scenario is and it feels difficult to understand a pattern.

Given they all import with no issues but only some actually log in correctly and when I rerun then more will work, tells me I am not passing in bad data but something about how they process is different each time.

Steps to reproduce:

What happened? How can we make the problem occur? This could be a description, log/console output, etc.

Relevant Code:

The code below will generate 100 users and import them one at a time and then when I attempt to log in, only the last 10 ish will even work, all others get incorrect password errors.

const crypto = require('crypto');
const admin = require('firebase-admin');

var serviceAccount = require(".\\key.json");
admin.initializeApp({
  credential: admin.credential.cert(serviceAccount),
});

const randomString = (length) => {
    return Math.random().toString(36).slice(2, length + 2)
}

const users = [];
for(let i = 0; i <= 100; i++) {
    const text = 'password'
    const key = randomString(12);
    const passwordHash = crypto.createHmac('sha1', key)
    .update(text)
    .digest('hex')

    users.push({
        plainPassword: text,
        passwordHash,
        key,
        email_address: `${i}@gmail.com`,
        id: i.toString()
    })
}

async function start() {
    for (const user of users) {        
        const result = await admin.auth().importUsers([
            {
                uid: user.id,
                email: user.email_address,
                passwordHash: Buffer.from(user.passwordHash, 'hex'),
                emailVerified: true
            }
        ], {
            hash: {
                algorithm: "HMAC_SHA1",
                key: Buffer.from(user.key)
            }
        })

        console.log(result.errors[0]);
    }
}

start();
google-oss-bot commented 1 month ago

I couldn't figure out how to label this issue, so I've labeled it for a human to triage. Hang tight.

jeffj6123 commented 1 month ago

Any updates on guidance for this? This is only needed for a one time operation so we don't need a robust solution.

jeffj6123 commented 3 weeks ago

Another point of data. If we attempt a login after each import with the correct password then all appear to import correctly. We tried using an incorrect password and that didnt seem to make a difference. This doesnt not help though given we dont know the plaintext passwords in the real production scenario, so we were only able to test this using the script above

jeffj6123 commented 3 weeks ago

A very interesting point of data, if I make all 100 calls with the same hash key then all the log ins will work. It almost feels like server side there is some degree of caching/handling state before the users get saved and when I give all my import calls the exact same hash it works well but if they're all different only 18 out of 100 will get set correctly. Specifically the last 18