Dj rest auth #187

BethanyG · 2020-09-24T06:01:47Z

What type of PR is this? (check all applicable)

Context

We wanted to implement a user registration process that incorporated an email and verification of that email.
See issue #110 and discussion #178 for additional context.

Closes #110, #113, #114, #118

Other Related Tickets & Documents (as needed)

Implementation Details

We had originally planned to use django-rest-auth, but it is no longer maintained, and doesn't play nice with django-allauth anymore. dj-rest-auth is the currently maintained fork of django-rest-auth. However, dj-rest-auth requires the django-simpleJWT library, so the current django-rest-framework-jwt library was replaced.

It was also decided that authorization and registration should live under the main API url instead of being "off to the side", so the URLs and code were refactored to support this. Additionally, settings changes were made to require a validated email upon user registration. Users will no longer be allowed to login without a "verified" email.

More refactor/changes may be needed as we refine user profiles and permissions.

Obtaining JWT tokens has moved to:

api/v1/auth/token (obtain an access and refresh token pair),
api/v1/auth/verify (verify the validity of a refresh or access token),
api/v1/refresh (obtain new access toke by using non-expired refresh token)

Registration/Login/Logout are now located at:

api/v1/auth/registration/ (now triggers a validation email to the email address a user enters.)
api/v1/auth/verify-email/ (to POST a user's HMC email key for validating their email)
api/v1/auth/login/ (Will require a validated email in order to sign in)
api/v1/auth/logout/ (Will clear tokens from currently logged in user)

User Details and current_user are located at:

api/v1/auth/user/ (to view currently logged in User Details)
api/v1/auth/current_user (to view currently logged in User without email address)

Since these two are redundant, one may be omitted or changed later in the project.

New Libraries/Dependancies Introduced (Fill out as needed)

If you have added libraries or other dependancies, please list them (and links to their repos) below:

djangorestframework-simplejwt
dj-rest-auth==1.1.1
django-rest-authtoken==2.1.3

I've updated project/requirements/base.txt

Any new migration files added?

👍 yes -- but these should happen in the course of installing the new apps/libraries.
🙅 no, because they aren't needed

Did you add tests?

Code added or changed without test coverage or good reason for exemption won't be approved.

👍 yes - more are needed, but the basics are covered.
🙋 no, because I need help
🙅 no, because they aren't needed

Did you add documentation?

📜 readme.md
📜 contributing.md
📜 wiki entry
🙋 I'd like some additional time to write documentation, and will file a new issue for it
🙅 no documentation needed

…llauth together.

…est-auth settings.

…rm emails and password reset.

…gin, logout, and user details. Reorganized.

…ckend.

…ation, etc.

codecov · 2020-09-24T06:04:30Z

Codecov Report

❗ No coverage uploaded for pull request base (main@6f9a92f). Click here to learn what that means.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #187   +/-   ##
=======================================
  Coverage        ?   81.56%           
=======================================
  Files           ?       33           
  Lines           ?      548           
  Branches        ?        0           
=======================================
  Hits            ?      447           
  Misses          ?      101           
  Partials        ?        0

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6f9a92f...604bac8. Read the comment docs.

lpatmo · 2020-09-24T16:01:57Z

Yay!!! 👏👏👏

Excellent work, Bethany! I started reviewing this morning but didn't get a chance to finish testing and looking at everything, but will try to finish up tonight/tomorrow.

One small detail that can be fixed right now: project/core/templates/account/email/email _confirmation_subject.txt has an extra space near the underscore.

BethanyG · 2020-09-24T20:06:19Z

Hey @lpatmo - thanks for the sharp-eyed copyedit! Could you make that suggestion in the PR directly, so we can just accept it here -- or give me a file and line to reference? Not sure where the URL is is mis-typed? Thanks!

EDIT: NM! Found it. Will push a renamed file.

oooofh. Not sure why, but changing that file causes 17 test failures and an endless recursion. Reverting.

…name." This reverts commit 2670780. This file rename breaks allauth and all auth test cases for some reason. It also causes a python failure due to an endless import recursion. Not sure why/how this came about, but the easiest thing is to leave it as-is, and revert the change.

… updating settings and tests.

… regex.

…es, it will fail intermittently - not consistantly.

… to exclude extra space.

…warnings.

BethanyG · 2020-09-25T00:06:32Z

OK. I think I've fixed both the intermittent test failure and the mis-named file. Test are passing.

lpatmo

OK. I think I've fixed both the intermittent test failure and the mis-named file. Test are passing.

Yay, thanks for fixing!

TIL having EMAIL_BACKEND = 'django.core.mail.backends.smtp.EmailBackend' means the email is "sent" via stdout if I run docker-compose up without the -d flag :D

We may need to look into updating the "from" line (a future issue), but I'm very happy with this:

Again, thanks for this big chunk of work! 👏 Feel free to merge in when you're ready. Separately, I filed a FE ticket to look into the changes that need to be made on the FE side: codebuddies/frontend#161

project/core/templates/account/email/email _confirmation_subject.txt

project/core/templates/account/email/email_confirmation_message.txt

project/config/settings/base.py

Co-authored-by: Linda <linda.peng@alumni.duke.edu>

project/userauth/tests.py

BethanyG · 2020-09-26T21:23:26Z

OK. Mashing the big, green button. 😄

BethanyG added 18 commits September 7, 2020 12:14

Added osprojects to admin interface.

dcbab36

Added osprojects to admin interface.

31edf0f

Added OSProjects to admin interface.

d585cdd

added django-res-auth and associated basic settings to test DRF and a…

606c871

…llauth together.

Working state Mon Sept 14 - dj-rest-auth and django-allauth

30936fe

Removed django-rest-jwt and django-rest-auth added simplejwt and dj-r…

cdadba1

…est-auth settings.

Added routes for registration and allauth.

782e05b

Added password reset email template.

040c6f3

Added confirm_email method. Will probably remove it as uneeded later.

ef3a5ad

Added CustomUserDetailsSerializer. Stubbed other serializers for conf…

7e6ff72

…rm emails and password reset.

Added routes for registration, email confirmation, password reset, lo…

e97153b

…gin, logout, and user details. Reorganized.

Added CustomVerifyEmailView to enable email verify emails from the ba…

b6c4ac2

…ckend.

Added tests for registration flow, token authorization, email verific…

f06a74c

…ation, etc.

Fixed tests that were failing due to auth change.

221b7b6

Added final simpleJWT tests for auth.

e8197e7

Added skips for User tests broken due to auth changes.

f220224

Removed django-rest_auth and django-rest-jwt from requirememts.

bb47eb0

Removed unused import that was causing an error.

9fd6eb7

BethanyG requested a review from lpatmo September 24, 2020 06:01

This was linked to issues Sep 24, 2020

[User Registration] JWT Serializer Changes in Support of User Email Validation #113

Closed

[User Registration] Changes to userauth App in Support of Email Validation #114

Closed

[User Registration] Need to Define JSON Spec for User Profile #118

Closed

BethanyG added 9 commits September 24, 2020 13:50

Removed excess space from email_confirmation_subject.txt filename.

2670780

Attempted to fix intermittent email verification post test failure by…

aa6218a

… updating settings and tests.

Changed order of included URLs.

f91a22d

Added specific path for email link verification post and changed path…

a2b2b4b

… regex.

Fun trick: If your regex fails to capture all the key generations cas…

569ee99

…es, it will fail intermittently - not consistantly.

Removed print statement from email post verificaton test case.

35c5610

Corrected circular reference that was breaking tests and renamed file…

75e9461

… to exclude extra space.

Removed regex from path url to avoid djang deprecation and migration …

b2ba2c2

…warnings.

BethanyG mentioned this pull request Sep 26, 2020

Issue 160 hangouts endpoint first pass #193

Closed

14 tasks

lpatmo mentioned this pull request Sep 26, 2020

Update user registration flow to support new backend auth endpoints codebuddies/frontend#161

Open

Merge branch 'main' into dj-rest-auth

b127b67

lpatmo approved these changes Sep 26, 2020

View reviewed changes

project/core/templates/account/email/email _confirmation_subject.txt Outdated Show resolved Hide resolved

project/core/templates/account/email/email_confirmation_message.txt Show resolved Hide resolved

project/config/settings/base.py Outdated Show resolved Hide resolved

Update project/config/settings/base.py

4a2bfd4

Co-authored-by: Linda <linda.peng@alumni.duke.edu>

BethanyG commented Sep 26, 2020

View reviewed changes

project/userauth/tests.py Outdated Show resolved Hide resolved

Update project/userauth/tests.py

604bac8

BethanyG merged commit 649c0c1 into codebuddies:main Sep 26, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Dj rest auth #187

Dj rest auth #187

BethanyG commented Sep 24, 2020

codecov bot commented Sep 24, 2020 •

edited

Loading

lpatmo commented Sep 24, 2020

BethanyG commented Sep 24, 2020 •

edited

Loading

BethanyG commented Sep 25, 2020

lpatmo left a comment

BethanyG commented Sep 26, 2020

Dj rest auth #187

Dj rest auth #187

Conversation

BethanyG commented Sep 24, 2020

What type of PR is this? (check all applicable)

Context

Other Related Tickets & Documents (as needed)

Implementation Details

New Libraries/Dependancies Introduced (Fill out as needed)

Any new migration files added?

Did you add tests?

Did you add documentation?

codecov bot commented Sep 24, 2020 • edited Loading

Codecov Report

lpatmo commented Sep 24, 2020

BethanyG commented Sep 24, 2020 • edited Loading

BethanyG commented Sep 25, 2020

lpatmo left a comment

Choose a reason for hiding this comment

BethanyG commented Sep 26, 2020

codecov bot commented Sep 24, 2020 •

edited

Loading

BethanyG commented Sep 24, 2020 •

edited

Loading