Email 2fa #423

pickfire · 2021-07-28T04:25:13Z

Revive #267 thanks to @Atterratio for the initial patch. I did some changes based on the reviews on the original pull requests and some doc style fixes.

Description

Make possible two-factor authorization using emails.

Motivation and Context

Although this way of two-factor authentication is not very reliable, it can be preferable for some users.

How Has This Been Tested?

Tested in a local project. Not fully tested but I tried it out. Didn't look closely at old PR.

Screenshots (if appropriate):

Content-Type: multipart/alternative;
 boundary="===============1209054917869947523=="
MIME-Version: 1.0
Subject: Authentication token email
From: hello@test.org
To: hello@example.com
Date: Wed, 28 Jul 2021 04:20:20 -0000
Message-ID: <162744602025.73619.15432270616108599468@mi.localdomain>

--===============1209054917869947523==
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit

Hello,
Your email address has been given for two-factor authorization on the our website.
If you did't do this, just ignore this message.

Authentication token for user hello is 487008.
--===============1209054917869947523==
Content-Type: text/html; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit



<p>Hello,</p>
<p>
    
        Your email address has been given for two-factor authorization on the our website.
        If you did't do this, just ignore this message.
    
</p>
<p>Authentication token for user hello is 487008.</p>
--===============1209054917869947523==--

-------------------------------------------------------------------------------

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

My code follows the code style of this project.
My change requires a change to the documentation.
I have updated the documentation accordingly.
I have added tests to cover my changes.
All new and existing tests passed.

Fix and update tests Fixes after code review Move user.save() in rigth place Remove unnecessary import

WayneLambert · 2021-07-28T14:50:00Z

Hi @pickfire,

Thanks for reviving this and continuing the effort from #267. I am in the midst of considering bolting on a custom implementation using email, however would prefer to adjust/extend the use of this package, so if this is merged, it would greatly help.

It looks like a review is required and is the blocker at the moment. Perhaps you could add @Atterratio as the reviewer?

Thanks again!

pickfire · 2021-07-29T00:40:50Z

It looks like a review is required and is the blocker at the moment. Perhaps you could add @Atterratio as the reviewer?

I cannot since I don't have permission but I don't think he will review it since he is not active in the original patch as well.

WayneLambert · 2021-07-29T08:44:01Z

I cannot since I don't have permission but I don't think he will review it since he is not active in the original patch as well.

Hi @pickfire, thanks for this.

Perhaps @moggers87 could take a look at this since it is a continuation from the previous PRs work (#267) and seems the most likely contributor to know what is required to get this over the line.

moggers87 · 2021-07-30T10:56:03Z

I have requested a review from @moggers87

daronzwink · 2021-08-14T02:41:22Z

This is awesome, and exactly what we were looking for! Thank you very much for picking this enhancement back up and running with it. We would love to apply this to our production environment and wanted to see if there is any chance this could be reviewed and merged?

pickfire · 2021-08-16T01:29:58Z

I am waiting for reviewer, not sure if the maintainers are still active.

moggers87

A few changes required, otherwise good 👍

moggers87 · 2021-08-16T05:41:13Z

two_factor/models.py


-    number = PhoneNumberField()
+    drift_range = ()


drift_range should be set to None here. That way if someone forgets to set it, they'll get an error.

moggers87 · 2021-08-16T05:42:58Z

two_factor/models.py

            if totp(self.bin_key, drift=drift, digits=totp_digits()) == token:
                return True
        return False

+    def get_throttle_factor(self):
+        return getattr(settings, 'TWO_FACTOR_PHONE_THROTTLE_FACTOR', 1)


This should not be in the mixin and there should be a TWO_FACTOR_EMAIL_THROTTLE_FACTOR setting for EmailDevice

claudep · 2021-08-16T06:18:20Z

I hoped that #331 would be merged before any new capability addition. The more we wait the more it will be difficult to change the module structure.

moggers87 · 2021-08-16T10:34:21Z

@claudep that's a slightly bigger change, which is why I keep delaying merging it 😆

moggers87 · 2021-08-16T10:36:06Z

Actually nothing can get merged yet anyway because Travis is down and I need to complete #425 to fix that.

Suggested by moggers87

chuichin · 2021-08-27T08:49:47Z

Hello, I'm following this pull request as I'm looking to use emails for the authorization too. Are there any updates on this? :) Thanks!

J4bbi · 2021-11-19T17:38:41Z

I am also interested in email support. And I am guessing it's a feature that would be in demand.

Can I ask what is holding this PR up at the moment? Original PR is from 2018.

@moggers87 it looks like @pickfire has applied your requested changes (admit I haven't looked at the code)

Do you have time for this?

As things stand we might try to implement the PR directly ourselves but we would be more relaxed about it if this PR was merged :)

Thanks to everyone for their work on this.

giovannicimolin · 2021-12-16T12:44:32Z

@moggers87 (CC @pickfire) I'm also interested in having this merged.
Is there anything I can do to help speed up this process?
Are there any blockers on this?

claudep · 2022-03-04T16:54:08Z

FYI I'm currently trying to refresh this patch based on the new plugin-based method.

claudep · 2022-03-04T18:02:35Z

The new version is now in #475.

Add email 2fa method

b362d06

Fix and update tests Fixes after code review Move user.save() in rigth place Remove unnecessary import

pickfire force-pushed the email_2fa branch from f1e113d to b362d06 Compare July 28, 2021 06:53

pickfire mentioned this pull request Jul 28, 2021

Email 2fa #267

Closed

8 tasks

moggers87 self-requested a review July 30, 2021 10:55

moggers87 requested changes Aug 16, 2021

View reviewed changes

Add throttle for email and hard error on missing drift_range

f1c5cf6

Suggested by moggers87

pickfire requested a review from moggers87 August 27, 2021 01:43

claudep closed this Mar 4, 2022

pickfire deleted the email_2fa branch March 5, 2022 07:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Email 2fa #423

Email 2fa #423

pickfire commented Jul 28, 2021 •

edited

WayneLambert commented Jul 28, 2021

pickfire commented Jul 29, 2021 •

edited

WayneLambert commented Jul 29, 2021

moggers87 commented Jul 30, 2021

daronzwink commented Aug 14, 2021

pickfire commented Aug 16, 2021

moggers87 left a comment

moggers87 Aug 16, 2021

moggers87 Aug 16, 2021

claudep commented Aug 16, 2021

moggers87 commented Aug 16, 2021

moggers87 commented Aug 16, 2021

chuichin commented Aug 27, 2021

J4bbi commented Nov 19, 2021

giovannicimolin commented Dec 16, 2021

claudep commented Mar 4, 2022

claudep commented Mar 4, 2022

Email 2fa #423

Email 2fa #423

Conversation

pickfire commented Jul 28, 2021 • edited

Description

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

Checklist:

WayneLambert commented Jul 28, 2021

pickfire commented Jul 29, 2021 • edited

WayneLambert commented Jul 29, 2021

moggers87 commented Jul 30, 2021

daronzwink commented Aug 14, 2021

pickfire commented Aug 16, 2021

moggers87 left a comment

Choose a reason for hiding this comment

moggers87 Aug 16, 2021

Choose a reason for hiding this comment

moggers87 Aug 16, 2021

Choose a reason for hiding this comment

claudep commented Aug 16, 2021

moggers87 commented Aug 16, 2021

moggers87 commented Aug 16, 2021

chuichin commented Aug 27, 2021

J4bbi commented Nov 19, 2021

giovannicimolin commented Dec 16, 2021

claudep commented Mar 4, 2022

claudep commented Mar 4, 2022

pickfire commented Jul 28, 2021 •

edited

pickfire commented Jul 29, 2021 •

edited