Add Two Factor Authentication #842

TillerBurr · 2019-05-20T16:50:51Z

Rebased #773 and #522
This PR solves: #496

Much of the CI Failures were due to formatting issues. Also contains the following:

Updated release.py to accommodate Python 3 versions (prevented CI tests from passing.)
Added Python 3.7 to the CI tests
Updated requirements due to a conflict in Werkzeug and Flask. (See Unable to send POST request using Flask test client pallets/flask#3148)

* supporting two factor authentication: * Support Mail, SMS, or Google Authenticator second factor authentication. * Ability to change second factor authentication to existing users * Provide rescue mail in case of lost phone * update docs with two factor authentication changes * updating requirements, authors * adding two_factor test * improving tests coverage

request.json->request.get_json update tests to use hash_password pep8 compliance Update dependencies for tests move from pyenchant to msgcheck

ghost · 2019-05-27T15:49:54Z

Thanks for this awesome PR.

Do you consider making this pull to the Flask-Security-Too fork at https://github.com/jwag956/flask-security?

The fork is more active. See #822.

ghost

Just some minor spelling errors.

There is also a mix between 'two factor' and 'two-factor'.

docs/configuration.rst

ghost · 2019-05-27T16:00:04Z

docs/configuration.rst

+                                               the verify code page for the two factor
+                                               authentication process. Defaults to
+                                               ``security/two_factor_verify_code
+                                               .html``.


There is a space too much which breaks the line.

ghost · 2019-05-27T16:00:13Z

docs/configuration.rst

+``SECURITY_TWO_FACTOR_CHOOSE_METHOD_TEMPLT``   Specifies the path to the template for
+                                               the choose method page for the two
+                                               factor authentication process. Defaults
+                                               to ``security/two_factor_choose_method


There is a space too much which breaks the line.

ghost · 2019-05-27T16:00:17Z

docs/configuration.rst

+``SECURITY_TWO_FACTOR_CHANGE_METHOD_TEMPLATE`` Specifies the path to the template for
+                                               the change method page for the two
+                                               factor authentication process. Defaults
+                                               to ``security/two_factor_change_method_


There is a space too much which breaks the line.

Thanks. I didn't change much on the docs, mostly just to make sure that the CI tests pass. I've made some changes to the codebase.

Co-Authored-By: malware-watch <malware-watch@users.noreply.github.com>

TillerBurr · 2019-05-28T13:42:25Z

The main reason that I did not make a pull request to https://github.com/jwag956/flask-security was that in #822 @lnielsen was added as an admin/maintainter on the various sites. I can definitely make a pull request to Flask-Security-Too.

jwag956 · 2019-05-28T16:58:26Z

I would be happy to help get your change into flask-security-too. I have been trying to work with current flask-security maintainers but so far to no avail - so I have decided at least for now to go off and formalize my fork.
I am working on a 3.2.0 release that will contain a few feature improvements - in particular support for Single-Page-Applications (more complete json support) as well as documentation improvements, and a contributed authentication token cache.
Adding 2-factor to 3.2.0 should be doable and I hope to get that release out in the next couple weeks.

Please be aware that on my fork - I need PRs against MASTER (not develop). Hopefully most of non-feature related code changes won't be necessary.

I have a rc1 for 3.2.0 on pypi:

https://pypi.org/project/Flask-Security-Too/#history

which currently is based off of master branch.

jwag956

One thing I am not completely sure about is how/what needs to be done to update translations - obviously you cant do all languages - but I think you need to update flask_security.pot at least with your new messages.

jwag956 · 2019-05-28T17:02:02Z

flask_security/twofactor.py

+
+
+def generate_totp():
+    return base64.b32encode(os.urandom(10)).decode('utf-8')


I don't think this should be in code - different applications will have different requirements here. For example in 3.6 there is now a secrets module that is probably better...

All I did was rebase the old pull requests to get them working and passing the CI tests. I can work on this at some point, but not sure when I will be able to get to them.

jwag956 · 2019-05-28T17:02:56Z

flask_security/twofactor.py

+    """generate the qrcode for the two-factor authentication process"""
+    if 'google_authenticator' not in\
+            config_value('TWO_FACTOR_ENABLED_METHODS'):
+        return abort(404)


In general I think that utility functions like this should NOT be assuming a request context - rather throw an exception to the caller?

jwag956 · 2019-05-28T17:04:05Z

flask_security/twofactor.py

+        pass
+
+
+def get_totp_uri(username, totp_secret):


Question - did you consider using passlib.totp classes rather than rolling your own?

jwag956 · 2019-05-28T17:05:39Z

flask_security/views.py

+        # if user's two-factor properties are not configured
+        if user.two_factor_primary_method is None or user.totp_secret is None:
+            session['has_two_factor'] = False
+            return redirect(url_for('two_factor_setup_function'))


This isn't JSON friendly!

jwag956 · 2019-05-28T17:06:47Z

requirements.txt

@@ -1,3 +1,11 @@
-# Trick for ReadTheDocs to install all requirements:
+Flask>=0.9


these shouldn't be necessary.

I thought I had changed that back. I'm reverting it

TillerBurr · 2019-05-28T21:05:43Z

Thank you for your help. This is not my initial code and I am kind of jumping into it without understanding what the original author was trying to do.

jwag956 · 2019-05-28T22:40:01Z

Let me know when you believe you are done - and I will cherry-pick that over to my fork. It will be a bit of a mess due to all the non-2FA changes - but shouldn't be too hard.
I assume you have an app that actually uses this so you can test that it actually works?

TillerBurr · 2019-05-29T21:00:46Z

@jwag956 I've got it passing all of the CI tests and I've tested it in an app that isn't a SPA. The force push from earlier was just to squash a bunch of smaller commits that I was working through with the CI.

I was trying to locally merge your fork with my current branch, but at this point there is too much of a difference to try to merge without stepping on your toes. If this gets merged into your fork, I will try to keep an eye on it. There is much that can be improved on the backend, e.g. hashing the totp_secret, not storing it on the user model etc. but I think this is a good start.

jwag956 · 2019-05-30T01:41:02Z

That was painful - doing a rebase rather than cherry-pick - in any case - I have a PR up - made minor changes, and had to fix some doc stuff since my CI actually verifies that the docs build!

In my PR I added a bunch of questions that I could use your help on. I am mostly concerned about things that if we change later would not be backwards compatible.

Also - please look at:
https://coveralls.io/builds/23685029

looks like we need some more unit tests!

TillerBurr · 2019-05-30T14:10:39Z

I can do this on your fork as well. As far as syntax checking, I am currently using vscode and it sometimes throws a bunch of unresolved imports that aren't true. I try to catch them, but apparently I seem to enjoy missing them.

jwag956 · 2019-05-31T07:48:05Z

Thanks - that would be great. I use Pycharm and it also sometimes makes mistakes - but if you run the code - doesn't it work/throw an error? I guess I am just asking why commit/push just to check if it runs? I both run from PyCharm - but also, to check pep8 etc - I run python setup.py test before committing... let me know if you run into any issues on my fork. Just work on the same branch (baurt) - I think it will let you - I just added you as a collaborator.

…

On Thu, May 30, 2019 at 7:10 AM Tyler Baur ***@***.***> wrote: I can do this on your fork as well. As far as syntax checking, I am currently using vscode and it sometimes throws a bunch of unresolved imports that aren't true. I try to catch them, but apparently I seem to enjoy missing them. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#842>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAHU2T5P6RQP7IHIVJVKL2DPX7N6DANCNFSM4HODXFRQ> .

-- Chris Wagner The land embracing Point Lobos was bought for inclusion in the State Park System (a) because the very peculiar physical characteristics of the locality have enabled many people to obtain here personal satisfactions of peculiar kinds which they have valued very highly, and of which the most highly valued are unobtainable elsewhere in like degree if at all, ... Point Lobos Reserve Master Plan Report - Olmsted Brothers, November, 1935

* docs: release date and block fixes * i18n: added Chinese-Simple translation * Update messages.po change language from 'zh_Hans_CN' to 'zh_CN' * rename nl_NL/LC_MESSAGES/messages.po and zh_Hans_CN/LC_MESSAGES/messages.po * Security two factor authentication feature * supporting two factor authentication: * Support Mail, SMS, or Google Authenticator second factor authentication. * Ability to change second factor authentication to existing users * Provide rescue mail in case of lost phone * update docs with two factor authentication changes * updating requirements, authors * adding two_factor test * improving tests coverage * fix double import and unused import, revert get_user * fix missing setup.py install_requires * fix TestMail (remove __init_) * formatting * more formatting * too many blank lines * formatting forms.py * signals line length * twofactor formatting * utils formatting * update twilio client import * missing import * blueprint line length (twofactor) * line too long/import/authors inadv. deleted * conftest.py allow nulls in sqlalchemy * Spelling, Update Functions and Tests Python 3.7 Support request.json->request.get_json update tests to use hash_password pep8 compliance Update dependencies for tests move from pyenchant to msgcheck * Update docs/configuration.rst Co-Authored-By: malware-watch <malware-watch@users.noreply.github.com> * consistent two-factor * .gitignore .venv/ * fixes. passlib.totp, if not request.is_json * translation stubs for new messages * Make two-factor login more JSON friendly * feature - merge in two factor auth. These changes are mostly docs and cleanup. * remove pyqrcode, onetimepass from install requires check imports if TWO_FACTOR is True * Minor doc fixes. Increase sizes of examples for 2FA phone and secret. * whitespace * feature - merge in two factor auth. These changes are mostly docs and cleanup. * Fix formatting.

miceno

LGTM 👍

miceno

👍

posix4e · 2020-02-20T05:31:47Z

what's going on here?

TillerBurr · 2020-02-20T15:58:38Z

Project is pretty much abandoned. An updated fork is here: https://github.com/jwag956/flask-security.
It contains the above pull request and many other improvements.

katesinclair91 · 2020-11-15T13:19:42Z

Project is pretty much abandoned. An updated fork is here: https://github.com/jwag956/flask-security.
It contains the above pull request and many other improvements.

Luckily. i read the pull requests and found this comment. Else i will be stuck on old. :D

Although OWASP still recommends that reset password and confirmation links have the no-referrer header option set - this causes issues with HTTPS and Flask-WTF that requires a referrer header. Also - for the past 5 years, the browser default for Referrer-Policy is 'strict-origin-when-cross-origin' which should be enough to mitigate any possible Referrer leakage. closes pallets-eco#829

galstat88 and others added 25 commits May 7, 2019 16:47

fix double import and unused import, revert get_user

eb6e673

Merge branch 'develop1' into develop

8befa63

syntax error

794faea

fix missing setup.py install_requires

3eebb5f

fix TestMail (remove __init_)

ab07db1

verify_and_update_password fix

4e1b9cb

formatting

cf617fd

more formatting

0f23a50

too many blank lines

09e9c69

double imports in forms.py

9cfa47d

formatting forms.py

6a932b0

signals line length

9fa6005

twofactor formatting

117a0e1

utils formatting

46c77af

update twilio client import

0944672

missing import

d1f854d

blueprint line length (twofactor)

f8c00ec

trailing whitespace...

1d1578c

line too long/import/authors inadv. deleted

47a5cd5

compat: release.py python3

948d248

update install_requires in setup.py

a2c82c8

bump flask_sqlalchemy

d7d0cb4

conftest.py allow nulls in sqlalchemy

5076a3c

Spelling, Update Functions and Tests Python 3.7 Support

176b8ca

request.json->request.get_json update tests to use hash_password pep8 compliance Update dependencies for tests move from pyenchant to msgcheck

ghost reviewed May 27, 2019

View reviewed changes

TillerBurr and others added 2 commits May 28, 2019 08:25

Update docs/configuration.rst

a9feee7

Co-Authored-By: malware-watch <malware-watch@users.noreply.github.com>

consistent two-factor

185fcbc

.gitignore .venv/

148f866

jwag956 reviewed May 28, 2019

View reviewed changes

TillerBurr added 2 commits May 28, 2019 14:06

fixes. passlib.totp, if not request.is_json

3b90197

translation stubs for new messages

0b0c366

Make two-factor login more JSON friendly

4a766a1

TillerBurr force-pushed the develop branch from 9ce6989 to 4a766a1 Compare May 29, 2019 15:38

TillerBurr added 2 commits May 30, 2019 08:56

make pyqrcode and onetimepass optional, check imps

4f909eb

fix function def and import

7ef9003

TillerBurr force-pushed the develop branch from 68118ac to 7ef9003 Compare June 6, 2019 01:40

miceno reviewed Jun 15, 2019

View reviewed changes

miceno approved these changes Jun 15, 2019

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Two Factor Authentication #842

Add Two Factor Authentication #842

TillerBurr commented May 20, 2019

ghost commented May 27, 2019

ghost left a comment

ghost May 27, 2019

ghost May 27, 2019

ghost May 27, 2019

TillerBurr May 28, 2019 •

edited

Loading

TillerBurr commented May 28, 2019

jwag956 commented May 28, 2019

jwag956 left a comment

jwag956 May 28, 2019

TillerBurr May 28, 2019

jwag956 May 28, 2019 •

edited

Loading

jwag956 May 28, 2019

jwag956 May 28, 2019

jwag956 May 28, 2019

TillerBurr May 28, 2019

TillerBurr commented May 28, 2019

jwag956 commented May 28, 2019

TillerBurr commented May 29, 2019 •

edited

Loading

jwag956 commented May 30, 2019 •

edited

Loading

TillerBurr commented May 30, 2019

jwag956 commented May 31, 2019 via email

miceno left a comment

miceno left a comment

posix4e commented Feb 20, 2020

TillerBurr commented Feb 20, 2020

katesinclair91 commented Nov 15, 2020 •

edited

Loading



		def generate_totp():
		return base64.b32encode(os.urandom(10)).decode('utf-8')

		@@ -1,3 +1,11 @@
		# Trick for ReadTheDocs to install all requirements:
		Flask>=0.9

Add Two Factor Authentication #842

Are you sure you want to change the base?

Add Two Factor Authentication #842

Conversation

TillerBurr commented May 20, 2019

ghost commented May 27, 2019

ghost 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

TillerBurr May 28, 2019 • edited Loading

Choose a reason for hiding this comment

TillerBurr commented May 28, 2019

jwag956 commented May 28, 2019

jwag956 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jwag956 May 28, 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

Choose a reason for hiding this comment

TillerBurr commented May 28, 2019

jwag956 commented May 28, 2019

TillerBurr commented May 29, 2019 • edited Loading

jwag956 commented May 30, 2019 • edited Loading

TillerBurr commented May 30, 2019

jwag956 commented May 31, 2019 via email

miceno left a comment

Choose a reason for hiding this comment

miceno left a comment

Choose a reason for hiding this comment

posix4e commented Feb 20, 2020

TillerBurr commented Feb 20, 2020

katesinclair91 commented Nov 15, 2020 • edited Loading

TillerBurr May 28, 2019 •

edited

Loading

jwag956 May 28, 2019 •

edited

Loading

TillerBurr commented May 29, 2019 •

edited

Loading

jwag956 commented May 30, 2019 •

edited

Loading

katesinclair91 commented Nov 15, 2020 •

edited

Loading