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

[NEW] Two Factor Auth #6476

Merged
merged 24 commits into from
Mar 29, 2017
Merged

[NEW] Two Factor Auth #6476

merged 24 commits into from
Mar 29, 2017

Conversation

rodrigok
Copy link
Member

@rodrigok rodrigok commented Mar 24, 2017

@RocketChat/core

Closes #1034

captura de tela 2017-03-24 as 16 25 57
captura de tela 2017-03-24 as 17 09 47
captura de tela 2017-03-24 as 17 09 52
captura de tela 2017-03-24 as 17 10 18

@rodrigok rodrigok added this to the 0.55.0 milestone Mar 24, 2017
@engelgabriel engelgabriel temporarily deployed to rocket-chat-pr-6476 March 24, 2017 19:32 Inactive
@engelgabriel engelgabriel temporarily deployed to rocket-chat-pr-6476 March 24, 2017 19:43 Inactive
@rodrigok
Copy link
Member Author

rodrigok commented Mar 24, 2017

@rafaelks @laggedHero You guys will need to implement this.

  1. It will only work with password login now
  2. You will receive a new error on login {error: 'totp-required'}
  3. When you receive that error you should ask user for the 2FA code
  4. And submit the login again with the format:
{
	totp: {
		login: {
			user: SAME_USER_OBJECT,
			password: 'SAME_PASSWORD'
		},
		code: 'THE_CODE'
	}
}
  1. You will receive {error: 'totp-invalid'} if the code is invalid

@engelgabriel engelgabriel temporarily deployed to rocket-chat-pr-6476 March 24, 2017 19:51 Inactive
@engelgabriel engelgabriel temporarily deployed to rocket-chat-pr-6476 March 27, 2017 20:40 Inactive
@engelgabriel engelgabriel temporarily deployed to rocket-chat-pr-6476 March 27, 2017 20:41 Inactive
@engelgabriel engelgabriel temporarily deployed to rocket-chat-pr-6476 March 27, 2017 20:48 Inactive
@engelgabriel engelgabriel temporarily deployed to rocket-chat-pr-6476 March 27, 2017 21:15 Inactive
@engelgabriel engelgabriel temporarily deployed to rocket-chat-pr-6476 March 27, 2017 21:27 Inactive
Copy link
Contributor

@geekgonecrazy geekgonecrazy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

It tells you to make sure you have a copy of your backup codes but there don't appear to be backup codes at this point.

We should either Generate them right away and show them in the alert telling them to store them somewhere safe... Or change the wording initially from "Regenerate Backup codes" to "Generate Backup codes" Because until they have been generated once they cannot be "Regenerated"

Also even when I click Regenerate I get no codes.

<div class="section-content border-component-color">
<div class="alert pending-background pending-color pending-border">
<strong>
WARNING: Once you enable this, you will not be able to login on the native mobile apps (Rocket.Chat+) using your password until they implement the 2FA.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we change this a bit? Maybe something like:

WARNING: Please make sure your mobile app supports 2FA authentication before enabling. See <a href="https://github.com/RocketChat/Rocket.Chat.iOS/issues/375">Rocket.Chat+ iOS</a> or <a href="https://github.com/RocketChat/Rocket.Chat.Android/issues/248">Rocket.Chat+ Android</a> for current status.

WARNING: Please make sure your mobile app supports 2FA authentication before enabling. See Rocket.Chat+ iOS or Rocket.Chat+ Android for current status.

Feels less critical :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this was only meant to be temporary until the mobile applications actually support it, which is why it isn't translated and is only in english.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not too worried about the translation. If for what ever reason this makes it to a release before its supported. This is much better lingo to have its less critical toward the mobile guys :)

@sampaiodiego
Copy link
Member

@geekgonecrazy please try it clearing you .meteor/local folder (except the db).. because it shows up your backup codes, but as you can see you have i18n problems, that's why it doens't showed up to you.

if you clear your .meteor/local folder you should fix i18n problems 😉

@graywolf336
Copy link
Contributor

Can we get a server up somewhere with this enabled? That way we can all test it out easily and correctly? And that way the mobile developers can test it out

@sampaiodiego
Copy link
Member

@graywolf336 I'm trying to get it running on heroku with no success =(

geekgonecrazy
geekgonecrazy previously approved these changes Mar 28, 2017
@geekgonecrazy geekgonecrazy dismissed their stale review March 28, 2017 16:42

was trying to dismiss review :)

@geekgonecrazy
Copy link
Contributor

@sampaiodiego that was it. Works fine now. 😁

@geekgonecrazy
Copy link
Contributor

@graywolf336 / @sampaiodiego https://pr-6476.rocket.chat/ 😉

Copy link
Member

@ggazzo ggazzo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The "remaining backup" codes didn't appear at the first time after enabling 2fa.

image

@geekgonecrazy
Copy link
Contributor

Deployment updated: https://pr-6476.rocket.chat/ @ggazzo if you want to try and reproduce :)

@engelgabriel engelgabriel merged commit 7ff735d into develop Mar 29, 2017
@engelgabriel engelgabriel deleted the improvements/2fa-implementation branch March 29, 2017 21:18
@ulope
Copy link

ulope commented Mar 30, 2017

The screenshots contain this warning that login on the native mobile apps will not work once 2FA is enabled. Wouldn't it be possible to accept an append the 2FA token to the password as a workaround until the apps get updated?

@graywolf336
Copy link
Contributor

@rodrigok @sampaiodiego how does this work with the rest api?

@k0nsl
Copy link
Contributor

k0nsl commented Apr 30, 2017

I tried to enable 2FA on my production server and tested it both with Authy and Google Authenticator and neither with success; it just replies back invalid two factor code.

Sorry for the lack of details -- but I don't have anything more substantial than this at the moment.

PS: I am using version 0.55.1.

@geekgonecrazy
Copy link
Contributor

@k0nsl that's no good. I'll give a try and see if I can reproduce

@sampaiodiego
Copy link
Member

I have to say I've seen synchronization problems.. @k0nsl have sure both server and apps are synced (same date and hour)..

Maybe we should allow like 1 minute delay

@geekgonecrazy
Copy link
Contributor

This should maybe be a configurable setting? Some servers in business may want to keep the time delay very short. Others may want to allow a little longer grace period

@sampaiodiego
Copy link
Member

there you go #6859

@shakalandy
Copy link

Should this work whether LDAP authentication is enabled or not? Thanks!

@itzpraveen
Copy link

itzpraveen commented May 3, 2017 via email

@octanefilms
Copy link

Hi @sampaiodiego and @rodrigok,

I am evaluating RC. We have configured RC to our Atlassian Crowd server. This is working fine so far in our testing. I am also seeking to use 2FA which seems to not work for us for unknown reasons.

Steps

  1. Deployed RC server (on-premise); server boots appears to work correctly
  2. Configured Atlassian Crowd for user account management (username, password); working
  3. Configured 2FA via RC admin settings from the RC desktop client (MacOS); appears to work, no errors on save.
  4. Configure RC user settings for 2FA; worked and RC validated Google Authenticator 2FA sec string
  5. Log out of RC MacOS desktop client.
  6. Log into RC MacOS desktop client; no 2FA was required. Only username and password.
  7. Log out of RC MacOS desktop client; again.
  8. Log into RC web client, MacOS Chrome; no 2FA was required. Only username and password.

Questions

  1. What do we need to do to get 2FA working with RC server and RC clients? How do we triage the issue? Is this caused by having RC use Crowd for user management?
  2. Can we force all users to use 2FA on their RC clients (desktop and mobile) as 2FA currently seems to be "opt-in"? I saw there were some threads on this request in the RC Github...
  3. Is there a method to enable the use of security keys that use the FIDO U2F standard?

Thank you for considering my comment and questions.

@geekgonecrazy
Copy link
Contributor

I bet this is related to using crowd as authentication. Might be skipping 2FA code. Please open an issue with details.

Regarding Fido/U2F we don’t yet. But I personally think that would be amazing to add. Love my u2f key

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add two factor authentication