[WIP] #996 Add two-factor authentication

[asfaltboy]

EuroPython 2018 collaborators working on #996

[di]

Thanks for the PR! I think we'll probably want to hold off on merging this until we have the rest of #996 implemented as well.

[Sparkycz]

Printscreen of the verification page http://prntscr.com/kc4x8e

[Sparkycz]

@asfaltboy Could you please edit the title of the PR? This one is a little bit confusing.. Maybe to #996 add two-factor authentication

[asfaltboy]

Good point @Sparkycz done that!

[Sparkycz]

I think we should add one column more into the table AccountUser to indicate that the user has enabled two-factor auth.

Something like..

op.add_column(
    "accounts_user", sa.Column("2fa_enable", sa.Boolean(), nullable=False, default=False)
)

However, i'm still working on the login presenters..

[sergeykolosov]

@Sparkycz I'd like to make a number of clean-ups, commit squashes, etc, which basically involves rewriting branch history. Please notify me when it's an appropriate moment to do so, so that it won't conflict with your work on presenters.

[sergeykolosov]

Updated the branch as follows:

• fixed requirements ordering;
• renamed OTP secret to TOTP secret (to avoid collisions as soon as other OTP methods are added);
• fixed migration not being regenerated after field length change;
• reordered and squashed commits into logically consistent groups;
• implemented TOTP primitives and documented design decisions around those (see docstings);
• covered TOTP primitives with basic unit-tests.

@nlhkabu suggested teaming up with others involved in #996 to avoid putting effort into duplicate artifacts.

[sergeykolosov]

@Sparkycz please address unittest & linting issues.

[pradyunsg]

Could someone add the EuroPython label on this? :)

[sergeykolosov]

Those who had already applied DB migrations, before pulling the updated branch, make sure to downgrade first:

docker-compose run web python -m warehouse db downgrade 263b37556f74-1

[dstufft]

FWIW, we do git merge --squash, so you don't have to worry at all about keeping a clean history.

[dstufft]

Make sure that Not only do we guard the web based login, but we also need to figure out a strategy for requiring 2fa through basic auth as well. The easiest is probably going to be that whenever a user has 2fa, they have to send a password like <password>:<otp>. Alternatively we could accept a header like Warehouse-OTP, but that would require changes in twine.

[dstufft]

Looking at this function, it's not actually being base32 encoded, it's just generating a 16 length random string which just happens to also be valid base32. If this just needs a be a base32 encoded string, then I'd prefer to do something like:

import base64
import os

def generate_totp_secret():
    return base64.b32encode(os.urandom(32)).decode("ascii")

This matches how we've done things in warehouse/utils/crypto.py.

[dstufft]

Can we add a note here as to what exactly the valid window translate into? Is it an extra second? 6?, 30?

[Sparkycz]

I amended fix of the routes test into the commit with template. However there is still output from black linter (would reformat .. ) but i wouldn't like to reformat before the end of the changes in acounts/views.py

Meanwhile i'm finishing first version of the presenters (1-2 unit-tests left) despite We'll have to discuss the behavior of sessions etc..

[Sparkycz]

i guess you'll have a lot of requests for changes but somewhere we've gotta start ;-)

todo:

1. add enable/disable option of 2FA to profile settings
2. implement calling of utils/otp functions to user_service

[woodruffw]

I've restarted work on this in master...trailofbits:tob/996-add-2fa-support. Will open a PR shortly so that others can review progress in real time.

[di]

Closing in favor of #5567, but thank you for your contribution!