feat: merge users with username

[nesro]

Hi Julian,

we are upgrading NodeBB from version 1.17.1 to 2.2.0. I found out that new users created with api are missing the 'email:uid' record in database which breaks logging in with session sharing plugin.

As a workaround, I thought it would be cool to associate users not only by the email, but username too.

It's possible that I missed something from changelogs and there is a better way how to do it.

Thanks, Tomas

[julianlam]

Hi @nesro — the reason why new users are not added to the email:uid sorted set is because NodeBB no longer allows for this by default unless the email is verified (that is, the user steps through the email verification flow.)

Of course, for something like session-sharing, that is undesirable, so it looks like this plugin needs to be updated to automatically add users to that zset.

The rationale behind the change above is because without it, it is easy to impersonate someone by inputting their email. A low-level security concern exists in that via session sharing, a user could conceivably take over a NodeBB account by using an email on the connected service (assuming that service does not validate emails), and then stepping through the session-sharing flow.

So knowing that, the session-sharing plugin is actually more secure for it... with only the unintended downside that you can no longer merge user accounts together via email.

In your specific case, can you not specify a unique id for your user? That way, they will continue to be able to be logged in. Using email to log in the user is not preferable.

[julianlam]

Separately, using username to log in via session-sharing is a larger security risk. If sites A and B are connected via session sharing, site A has an admin barnes, and site B does not, a malicious user could register barnes on site B, step through session sharing, and have elevated privileges on Site A.

For that reason, I would be hesitant to merge any logic allowing for this... sorry!

[julianlam]

superceded by #123

[nesro]

@julianlam You are right, it's a security risk in the setup you described and I understand that now.

Logging in with user id is a good idea but there is another problem. As I am looking into the code, I see that it search for plugin.settings.name + ':uid'. But this sorted set is added only when merging, which I cannot use, and in plugin.createUser function which I cannot use either, because we have set up noRegistration as on. How can I make this work? One solution could be to look for _key:'user:${id}' in the database.

[julianlam]

But this sorted set is added only when merging, which I cannot use

I would keep the email merging functionality, it would just be disabled by default. If that were the case your integration would continue to work, yes?

[nesro]

Yes, that would work.

I think the best solution for us would be: 1) a flag that allows users merging with an email 2) being able to tell in the session sharing cookie if the email is confirmed or not

[julianlam]

The second point would add complexity... unless it is important for you, I would rather just either blanket auto-confirm all emails, or not at all.

[nesro]

Ok, let's not add complexity. I can confirm emails with api.

Thank you for all your comments and time. :)