Add 2 factor authentication as optional feature #70

leportella · 2019-03-04T12:23:26Z

SignIn with 2fa allowed:

Signup with 2fa allowed:

Authorization area:

Not included but wanna do in another pr:

admin cleanup on 2fa in case user looses it
page for retrieving code/ unabling 2fa

yuvipanda · 2019-03-04T18:35:24Z

nativeauthenticator/nativeauthenticator.py

@@ -69,6 +69,11 @@ class NativeAuthenticator(Authenticator):
        default_value=False,
        help="Deletes FirstUse Authenticator database after the import"
    )
+    add_two_factor_authentication = Bool(


Can we split this into two parts:

Allow users to have 2FA

Require users to have 2FA

In the former, some users might choose to protect their own accounts with 2FA, but isn't enforced. (2) is more complicated - what happens to people who already had created accounts without 2FA? How can they log in?

So I think one PR should be to allow_two_factor_authentication (or just allow_2fa), and it makes it optional for users to set up and use 2FA. There should probably also be a button in the admin screen that shows admins if this user has 2fa enabled, and optionally allows them to reset it.

After that, we can figure out how to make it required.

Indeed this makes sense. I'll refactor this PR to make the 2fa an optional character within user and then make another one to make it required and how to deal with people that signed in without 2fa, ok?

yuvipanda · 2019-03-04T18:36:29Z

nativeauthenticator/orm.py

@@ -13,6 +16,12 @@ class UserInfo(Base):
    password = Column(LargeBinary, nullable=False)
    is_authorized = Column(Boolean, default=False)
    email = Column(String)
+    otp_secret = Column(String(10))


What does the '10' refer to here?

the token is created with size 10, so I limited here

yuvipanda · 2019-03-06T14:49:25Z

Instead of having two login pages, what do you think about the flow being:

Normal login page, you enter username and password
If 2fa is enabled, it takes you to another page asking for 2FA

This is similar to how other websites do it, and is least confusing (I think).

leportella · 2019-03-06T15:39:19Z

@yuvipanda I tried to figured it out how to do this but couldn't. Any suggestions?

yuvipanda · 2019-03-06T16:42:49Z

@leportella hmm, we'd probably have to use a secure cookie, and split the login logic into two pages. I tried to write out how exactly we would do it, and realized to do so securely will be quite complicated - probably too complicated for what we wanna do with NativeAuthenticator. I definitely don't trust myself to figure out how to do that safely right now... 😄

For now, let's just have the additional textbox in the original page if 2FA is enabled? I think that was in your original design. I mostly don't want us to have two different login pages.

Sorry for all the confusion.

leportella · 2019-03-07T22:52:34Z

@yuvipanda I just removed the second page login and added a more detailed description with photos here :)

leportella · 2019-03-24T21:26:59Z

@choldgraf if you want to take a look as well, feel free :)

yuvipanda · 2019-03-25T09:00:34Z

nativeauthenticator/templates/native-login.html

-                Don't have an user? <a href="/signup">Signup!</a>
+                <p>Don't have an user? <a href="/signup">Signup!</a></p>
+                {% if two_factor_auth %}
+                <p>Have Two Factor Authentication configured? <a href="/2fa">Sign In here!</a></p>


What do you think about just putting the 2FA text box here? Currently the entire sign-in page is duplicated, and changes to them will have to be done twice - people will always forget! Same for the two Login Handlers.

There can be text like 'Enter 2FA Code here if enabled' with this text box, and it can be left empty by the user if they don't have 2FA enabled. This way, we avoid lots of duplication, and provide a mostly sane experience without making our code super complex.

What do you think?

choldgraf · 2019-03-25T18:26:59Z

hiiii 👋 is there a place that I could demo what this looks like w/ images and such? I think I agree with @yuvipanda that in general it'd be good to avoid duplicating UI elements (if anything to keep things more maintainable, but also to avoid confusing users).

One thing we should make clear is that by 2FA we specifically mean "we'll give users a code that they must have to log in". I think nowadays many people assume that 2FA means something fancy like "we'll send you a text message to click a link" etc. Maybe add some extra wording to options.rst to make that clear

leportella · 2019-03-29T11:33:43Z

@yuvipanda I had already removed the 2nd login page but I had forgot to upload it. Now it is fixed.

@choldgraf I did two gifs showing how is the system working now:

No 2fa

With 2fa

(Gifs are too large to post here, but I left the link for you to take a look :) )

choldgraf · 2019-03-30T17:35:53Z

Seems good to me! Thanks for the improvement @leportella :-)

yuvipanda

Minor changes, otherwise LGTM!

yuvipanda · 2019-04-01T06:48:29Z

nativeauthenticator/nativeauthenticator.py

@@ -69,6 +69,11 @@ class NativeAuthenticator(Authenticator):
        default_value=False,
        help="Deletes FirstUse Authenticator database after the import"
    )
+    allow_2fa = Bool(
+        True,


Let's turn this off by default to begin with. We can turn it on later if we wish.

yuvipanda · 2019-04-01T06:49:00Z

nativeauthenticator/templates/native-login.html

@@ -33,6 +33,7 @@
            {% endif %}
            <label for="username_input">Username:</label>
            <input id="username_input" type="text" autocapitalize="off" autocorrect="off" class="form-control" name="username" val="{{username}}" tabindex="1" autofocus="autofocus" />
+            <p></p>


Why the empty
?

A hack in the design the give more space between things 😅 I can take it off

yuvipanda · 2019-04-13T19:34:20Z

I like it! <3

choldgraf · 2019-04-13T22:09:40Z

wooo, way to go!

leportella added 7 commits March 4, 2019 12:24

Add onetimepass as dependency to native auth

f13707c

Add onetimepass secret creation on UserInfo orm

9e52615

Add sn authentication on authenticate method

d7156ac

Add two factor auth code on signup template

1a4efcd

Add 2 factor auth on login template

0756327

Add two factor authentication documentation

215f527

Change name from second_factor to two_factor

a9f7057

leportella force-pushed the add-2-factor branch from d469368 to a9f7057 Compare March 4, 2019 12:26

Fix errors on nativeauth default variables

ab8b16e

yuvipanda reviewed Mar 4, 2019

View reviewed changes

Add option to allow users to have a 2fa

feb5f0e

Add authenticator input on login website

e8c47ed

yuvipanda requested changes Mar 25, 2019

View reviewed changes

leportella force-pushed the add-2-factor branch from 9dce809 to e8c47ed Compare March 29, 2019 11:15

yuvipanda requested changes Apr 1, 2019

View reviewed changes

Makind 2fa not allowed by default

458bf45

yuvipanda approved these changes Apr 13, 2019

View reviewed changes

yuvipanda merged commit 919a374 into master Apr 13, 2019

lambdaTotoro deleted the add-2-factor branch September 29, 2021 17:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add 2 factor authentication as optional feature #70

Add 2 factor authentication as optional feature #70

leportella commented Mar 4, 2019 •

edited

Loading

yuvipanda Mar 4, 2019

leportella Mar 5, 2019

yuvipanda Mar 4, 2019

leportella Mar 5, 2019

yuvipanda commented Mar 6, 2019

leportella commented Mar 6, 2019

yuvipanda commented Mar 6, 2019

leportella commented Mar 7, 2019

leportella commented Mar 24, 2019

yuvipanda Mar 25, 2019

choldgraf commented Mar 25, 2019

leportella commented Mar 29, 2019 •

edited

Loading

choldgraf commented Mar 30, 2019

yuvipanda left a comment

yuvipanda Apr 1, 2019

yuvipanda Apr 1, 2019

leportella Apr 9, 2019

yuvipanda commented Apr 13, 2019

choldgraf commented Apr 13, 2019

Add 2 factor authentication as optional feature #70

Add 2 factor authentication as optional feature #70

Conversation

leportella commented Mar 4, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yuvipanda commented Mar 6, 2019

leportella commented Mar 6, 2019

yuvipanda commented Mar 6, 2019

leportella commented Mar 7, 2019

leportella commented Mar 24, 2019

Choose a reason for hiding this comment

choldgraf commented Mar 25, 2019

leportella commented Mar 29, 2019 • edited Loading

choldgraf commented Mar 30, 2019

yuvipanda left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yuvipanda commented Apr 13, 2019

choldgraf commented Apr 13, 2019

leportella commented Mar 4, 2019 •

edited

Loading

leportella commented Mar 29, 2019 •

edited

Loading