Authmereloaded: 2FA: Make two-factor optional

Created on 24 Mar 2017  路  27Comments  路  Source: AuthMe/AuthMeReloaded

Follow-up of #87, we currently have a TWO_FACTOR implementation but it forces everyone to use it. In this issue, we want to make it optional:

  • if enabled, the player should be able to _decide_ whether he wants to use a password, or 2FA -> requires a new command
  • internally, this means the player can decide between two hash algorithms. For this reason, the easiest way to keep track of this would be to add a new hash_algorithm column to the database.

Open question

With this new setup it should not be possible for plugin admins to configure TWO_FACTOR as hash algorithm. What happens if someone currently has this? This requires some smart migration...

enhancement

Most helpful comment

Yeah.

All 27 comments

I think 2nd factor authentication should be "separate" from login.

Internally, each user would have a column for OTP secret. If _null_, he's authenticated by password (token parameter is ignored), otherwise he's authenticated with both, password and token.

Scenario - User logs in without 2FA:

1) User joins
2) User types: /login "validPassword"
3) User is logged in

Scenario - User logs in with 2FA:

1) User joins
2) User types: /login "validPassword" "validToken"
3) User is logged in

Scenario - User turns 2FA on:

1) User is logged in
2) User types: /2fasetup
3) Server responses with OTP secret (qrcode, link to website, etc.)
4) User types: /2fasetup "validToken"
5) User now have 2FA active

Scenario - User turns 2FA off:

1) User is logged in
2) User types: /2faremove "validToken"
3) User now have 2FA inactive

Changing/recovering passwords would not be affected by 2FA.

@Jamaic230 Thanks for your input, this is all fine by me. With an admin command to turn 2FA off for a user we cover everything.

I move this to 5.5 with a somewhat "heavy heart" since I know that people do want this, but we need to eventually come to a closure with 5.4 and this requires major changes. We'll make this a priority in 5.5.

Analysis based on @Jamaic230's comment: 2FA should be an additional layer of authentication to the regular password. This means:

  • new field on PlayerAuth and in DB

    • If column is NULL player doesn't have 2fa, otherwise yes

    • Could have a follow-up issue to make the column optional maybe. I think once we figure out the specifics regarding configurations / permissions we'll see whether this makes sense or not.

  • new command /2fa to enable and disable 2 factor authentication

    • requires a permission node to control its use

  • extend /login to accept (and expect) a second argument if a user has 2fa enabled

    • I expect difficulties here to get the error messages and stuff like "number of failed login attempts" etc. still correct and intuitive

    • As a nice follow-up (as issue or later development step) would be to accept /login and then prompt the user to supply the token in an additional step maybe

  • admin command to turn 2fa off for a user -> separate issue
  • later we should disable the 2FA hash algorithm -> separate issue
  • email recovery -> require at least password or 2fa? Just turn 2fa off and allow to reset the password? My proposition would be to allow to change the password via email but to still keep 2FA, i.e. to make 2FA an entirely separate thing which can only be turned off by admins for now. Better propositions are very welcome.

Paging (alphabetically) @games647 @HexelDev @krusic22 @mraureliusr @sgdc3 @Stefatorus for any inputs you may have.

  • Optional: After the user enters his password for login, extend the Login Timeout for n-seconds (or just reset the timer) to make it possible for the user to open his 2FA app of choice.
  • Optional: Ability to require a set email to enable 2FA. Make it easier for admins to verify possible 2FA reset/toggle request.

Hi @krusic22 thanks for your feedback! I'm not sure I understand your second point, is it about forcing players (configurable) to set an email before they can turn 2FA on?

Yeah.

Current state (still on a branch):

  • /2fa add will generate a code and requires the user to confirm with /2fa confirm <code> before the code is actually saved for the account
  • /2fa remove <code> allows a user to remove the TOTP code again
  • For accounts with 2FA, login must be /login <password> <totpCode>

Outlook:

  • I should probably split the permissions for /totp add and /totp remove into two nodes? Right now they are guarded by the same permission node.
  • I don't know if it's interesting to keep the /login <pw> <totpCode> option or if it's OK to just split it into /login <password> and subsequently requiring /2fa <code>. The latter definitely needs to be implemented.
  • Tests / translatable messages
  • Make 2fa tokens one-use only -> #1219
  • Admin command to delete 2FA from a user -> #1523
  • Migration of our current TWO_FACTOR hash -> #1524

it would great if tokencode will be required only if IP will change and not every time

That's a nice idea! Could you create an issue for that so we can track these 2FA things into smaller components? :)

Done :)
And +1 for the separate commands from /login and use /2fa

hi, a base implementation of this is now available. In config.yml be sure to add /totp (and its alias, /2fa) to the allowCommands option under "restrictions"

I'd be thankful for some feedback. Please note that within this issue we've done a base implementation and that various follow-up issues exist to tackle some of the finer details (see https://github.com/AuthMe/AuthMeReloaded/issues/1141#issuecomment-371894607)

Sorry for reporting this a bit late but still:

  1. We need to add /2fa and /topt to AuthMeBungee command whitelist by default.
  2. The code is slightly desynced (+/-250ms).

Sorry could you elaborate on the second point? Desynced in comparison to what?

When the code in the 2FA app changes it's sometimes not the same as on the server. You need to wait additional 200ms after the change.

I鈥榣l look into this; thanks for the info!

when this will be made this "2FA" thing

It's already made. You can use it just type /2fa
Still some bugs to fix but otherwise it works.

ok 馃槂

but which version?

AuthMe-5.5.0-SNAPSHOT

and download link???????

put it on bukkit cuz i want it to go to aternos

When the Stable version is released.

fine

Given the long thread and the milestone I'd like to close this issue now. Please open a new issue for the desync issue if it still persists. Probably because the server's clock is somehow behind the real time.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

yannicnoller picture yannicnoller  路  4Comments

netherstar picture netherstar  路  5Comments

ITZVGcGPmO picture ITZVGcGPmO  路  3Comments

Kolosus990 picture Kolosus990  路  3Comments

Ahryzth picture Ahryzth  路  8Comments