Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implementing 2FA for NodeBB Forum posters #6452

Closed
RoiEXLab opened this issue May 9, 2020 · 7 comments
Closed

Implementing 2FA for NodeBB Forum posters #6452

RoiEXLab opened this issue May 9, 2020 · 7 comments
Assignees
Labels
Discussion team communication thread, meant for coordination and decision making

Comments

@RoiEXLab
Copy link
Member

RoiEXLab commented May 9, 2020

I thought of a "good way" to implement a mechanism that works for users that have 2FA enabled.

The current system works as follows whenever a post request is made:

  1. We lookup the user id for the username the user stored in the client settings.
  2. We create a login token using the user-id and the password stored in the client settings.
  3. We do all the posting using said login-token
  4. We remove/unregister the token again so it becomes invalid.

There are basically 3 options we can go here to implement 2FA:

  1. The most obvious one. We just add a prompt whenever the user tries to post something in case the user had a 2FA checkbox enabled somewhere.
    • Pro: Simple to implement (even though clearly separating the UI prompt from the logic could get tricky)
    • Con: One prompt per request with changing one time tokens.
  2. The easiest to code. We could just have the users create a login token themselves on the forum and just use that instead of the password. Almost no code changes required, we could simply drop step 1, 2 and 3 from the list above.
    • Pro: Easy to code, No actual password storage required. No additional prompts
    • Con: Probably a worse user experience, because we add some manual steps, not sure if this even works without an additional plugin.
  3. The best of both worlds. Instead of storing username + password, we retreive an auth-token on the fly and store that instead in the client settings store whenever the user tries to save a username + password combination.
    • Pro: No actual password storage required, No additional prompts.
    • Con: Hard to implement (details below)

Details on 3.

I tried giving 3. a shot but I soon realized that ClientSetting isn't really designed to handle this kind of storage.
There are 2 bigger problems I ran into that I could solve immediately which made me create this issue to discuss this approach in general:

  1. Value to encode != Value to decode:
    • ClientSetting expects that the value to decode and any encoded value have the same type. If you encode a boolean, the code expects to get a boolean when reading the setting again, which makes sense but doesn't work in our case. So the "workaround" would be to have an object that should really be 2 different objects, kind of ugly, but it would do its job even though it could get confusing at times.
  2. Token cleanup:
    • When moving the token creation to ClientSettings there's no easy system to implement token revocation once the user decides to reset their credentials. To revoke a token we need 3 things:
      1. The forum url obviously, we could pass that as parameter to the client setting though
      2. The token to revoke, which is already deleted when any "update" listener is fired, so I suppose we need to extend the ClientSetting class to pass any old value to the listeners.
      3. The users id because it's a user-specific token (there are master tokens as well), which we don't have once we got a token, which means we have to store the user id along with the token, which basically makes this client setting 2 client settings, which makes point 1 even uglier.

Thoughts

I'm interested in your thoughts about this. I think Option 3 would really be the way to go, but I just want to make sure you're thinking the same before doing those rather large code changes.

@RoiEXLab RoiEXLab added the Discussion team communication thread, meant for coordination and decision making label May 9, 2020
@DanVanAtta
Copy link
Member

(2) seems perhaps like an appropriate option given ROI.

TBH I would not invest too much in forum posting at this juncture. I'd like to see us get 2.0 launched (bugs found and bugs fixed), JavaFX launched first, and then enable HTTP server file uploads. That last capability opens a lot of possibilities. Once we have that, I feel like PBF is a very legacy tool (just strikes me as very late 90s) and could be replaced by posting to the server instead of to forum.

Having a save game option to post to server could be made mutually exclusive to PBF if we chose so, though I don't think the use-case for PBF is really all that there once we have post-to-server capability (benefits: one less login, one less integration, better and seamless integration with the core game). So I think it makes sense to deprecate PBF and just use post-to-server instead. There would be some design considerations for how to handle tournaments, but that is part and parcel to bringing tournaments in-game as well.

@panther2
Copy link
Contributor

panther2 commented May 9, 2020

@DanVanAtta I have no idea what post-to-server actually is. Is it only the savegame that is sent to a server instead to the forum? Will the written turn post be sent to the forum, still? That is what the players love about PBF - that they can read what happens/happened while they are not playing. That they can give OOL or scramble orders while not playing. That they can watch their or other people's games while not even having access to TripleA.
So how will that affect this very popular format (at least very popular on axisandallies.org)? Can you please elaborate a bit about that?

@DanVanAtta
Copy link
Member

There lots of options and very little designed; we are also premature here a bit as we've plenty to do before we can seriously start implementing 'post-to-server, AKA 'cloud-saves'. As mentioned, we can make it mutually exclusive to PBF or we could have it be a replacement. In essence we'd build in a forum like interface to view and post games, but into the game proper.

@panther2
Copy link
Contributor

panther2 commented May 10, 2020

I see. I just would not recommend to remove the Forum-feature, as this is a perfect way for TripleA players and watchers to dicuss moves and strategies independently from the client software. This feature has always been a huge advantage.

@RoiEXLab
Copy link
Member Author

#6487

@RoiEXLab
Copy link
Member Author

@panther2 I think I have been able to come up with a system that should support 2FA.
It will take a while until my PR is truly ready though, so stay tuned until then

@panther2
Copy link
Contributor

Great, thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Discussion team communication thread, meant for coordination and decision making
Projects
None yet
Development

No branches or pull requests

4 participants