Invalidate tokens on password change #1783

DavisRayM · 2020-02-17T12:11:13Z

Changes / Features

Invalidate a users api and temporary token when they're password
changes.
Add test to verify that the api and temporary tokens are renewed

Steps taken to verify this change does what is intended

Added tests

Pending:

Add new tests

Side effects of implementing this change

API Tokens will now need to be re-retrieved when one changes their password

Closes #1782

ivermac

lgtm!

ukanga · 2020-02-18T14:39:45Z

onadata/apps/api/viewsets/user_profile_viewset.py

                    metadata = user_profile.metadata or {}
                    metadata['last_password_edit'] = timezone.now().isoformat()
                    user_profile.user.set_password(new_password)
                    user_profile.metadata = metadata
                    user_profile.user.save()
                    user_profile.save()
+                    data.update(invalidate_and_regen_tokens(
+                        user=user_profile.user))


Do we need to return the access and temp token at this point? Could on the UI side simply make the request to the API/user endpoint to retrieve the new information from there as it already does on login?

The tokens that are in UI will already be invalidated so I don't think they can be used to make a request to the user endpoint unless the user logs out, no?

Yes, not necessarily logging out but would need to log in with the new credentials. Will we be terminating the existing session and creating a new one? Essentially as you say, log out and log in a fresh?

No much particular objection but simply wondering.

Should we update the documentation at https://api.ona.io/static/docs/profiles.html#change-authenticated-users-password to reflect that we also return the access and temp tokens?

We don't necessarily have to return the access and temp token at this point, returning them just makes it easier for the API consumer to continue making requesting (Just removes the added step of requesting for a new access token).

UI's will have to change / modify their stored token values. They can decide to terminate a session or just update it's values.

Yes, we should update the documentation. Thank you for bringing that up, let me make that change.

faith-mutua · 2020-03-17T09:16:12Z

@DavisRayM The API token is only updated when a user changes their password from the **Settings ** page. Changing passwords from the Forgot password link doesn't invalidate the API token.

DavisRayM · 2020-03-17T09:30:55Z

@DavisRayM The API token is only updated when a user changes their password from the **Settings ** page. Changing passwords from the Forgot password link doesn't invalidate the API token.

Thank you for noting this @faith-mutua. The latest commits should handle this now.

faith-mutua · 2020-03-20T13:52:34Z

@DavisRayM The API token is now being invalidated when a user changes their password via The Forgot password link or from the Edit Account Settings tab in the account settings page. This has passed QA.

ukanga · 2020-03-20T14:27:39Z

onadata/apps/api/viewsets/user_profile_viewset.py

@@ -142,6 +144,29 @@ def change_password_attempts(request):
    return 1


+def invalidate_and_regen_tokens(user):


Can we have this util function in onadata.libs.authentication module rather than a view set? Especially since it is also used by the CustomPasswordResetTokenGenerator class. I think it clutters the viewset.

Yes, I'll make that change.

Changed in the latest commits.

- Invalidate a users api and temporary token when they're password changes either through the password reset email or through an authenticated session. - Modify test to verify that the api and temporary tokens are renewed

Include information about information returned when an authenticated user changes their password

DavisRayM requested review from ukanga, ivermac, lincmba and WinnyTroy February 17, 2020 12:11

DavisRayM force-pushed the invalidate-session-on-password-reset branch from 6c763d1 to 32820c1 Compare February 17, 2020 12:13

ivermac previously approved these changes Feb 17, 2020

View reviewed changes

pld previously approved these changes Feb 17, 2020

View reviewed changes

WinnyTroy previously approved these changes Feb 17, 2020

View reviewed changes

DavisRayM dismissed stale reviews from WinnyTroy, pld, and ivermac via fcaf348 February 18, 2020 12:58

DavisRayM force-pushed the invalidate-session-on-password-reset branch from 32820c1 to fcaf348 Compare February 18, 2020 12:58

ivermac previously approved these changes Feb 18, 2020

View reviewed changes

lincmba previously approved these changes Feb 18, 2020

View reviewed changes

ukanga reviewed Feb 18, 2020

View reviewed changes

DavisRayM dismissed stale reviews from lincmba and ivermac via b1096ec February 18, 2020 15:44

DavisRayM changed the title ~~Invalidate tokens on password change~~ [DNM] Invalidate tokens on password change Feb 19, 2020

DavisRayM force-pushed the invalidate-session-on-password-reset branch from b1096ec to afcd54a Compare February 20, 2020 07:19

pld previously approved these changes Feb 20, 2020

View reviewed changes

DavisRayM dismissed pld’s stale review via 89a2045 March 3, 2020 08:04

DavisRayM force-pushed the invalidate-session-on-password-reset branch from afcd54a to 89a2045 Compare March 3, 2020 08:04

pld previously approved these changes Mar 3, 2020

View reviewed changes

DavisRayM changed the title ~~[DNM] Invalidate tokens on password change~~ Invalidate tokens on password change Mar 4, 2020

DavisRayM dismissed pld’s stale review via f419918 March 17, 2020 09:30

DavisRayM force-pushed the invalidate-session-on-password-reset branch from 89a2045 to f419918 Compare March 17, 2020 09:30

DavisRayM changed the title ~~Invalidate tokens on password change~~ [WIP] Invalidate tokens on password change Mar 17, 2020

pld previously approved these changes Mar 17, 2020

View reviewed changes

DavisRayM dismissed pld’s stale review via d5cc570 March 19, 2020 14:10

DavisRayM force-pushed the invalidate-session-on-password-reset branch from d5cc570 to 78245d2 Compare March 20, 2020 06:58

DavisRayM changed the title ~~[WIP] Invalidate tokens on password change~~ Invalidate tokens on password change Mar 20, 2020

DavisRayM changed the title ~~Invalidate tokens on password change~~ [Pending QA] Invalidate tokens on password change Mar 20, 2020

faith-mutua added the QA+ PR passed QA testing label Mar 20, 2020

DavisRayM changed the title ~~[Pending QA] Invalidate tokens on password change~~ Invalidate tokens on password change Mar 20, 2020

ukanga requested changes Mar 20, 2020

View reviewed changes

DavisRayM force-pushed the invalidate-session-on-password-reset branch from 78245d2 to d09ea1d Compare March 20, 2020 14:39

DavisRayM added 3 commits March 24, 2020 12:46

Invalidate tokens on password change

550bdc0

- Invalidate a users api and temporary token when they're password changes either through the password reset email or through an authenticated session. - Modify test to verify that the api and temporary tokens are renewed

Update profiles documentation

ff22509

Include information about information returned when an authenticated user changes their password

Return username after password reset

9441a45

DavisRayM force-pushed the invalidate-session-on-password-reset branch from d09ea1d to 9441a45 Compare March 24, 2020 09:46

ukanga approved these changes Mar 24, 2020

View reviewed changes

DavisRayM merged commit d6bd53b into master Mar 24, 2020

DavisRayM deleted the invalidate-session-on-password-reset branch March 24, 2020 12:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Invalidate tokens on password change #1783

Invalidate tokens on password change #1783

DavisRayM commented Feb 17, 2020 •

edited

Loading

ivermac left a comment

ukanga Feb 18, 2020

ivermac Feb 18, 2020

ukanga Feb 18, 2020

ukanga Feb 18, 2020

DavisRayM Feb 18, 2020

DavisRayM Feb 18, 2020

faith-mutua commented Mar 17, 2020

DavisRayM commented Mar 17, 2020

faith-mutua commented Mar 20, 2020

ukanga Mar 20, 2020

DavisRayM Mar 20, 2020

DavisRayM Mar 20, 2020

		@@ -142,6 +144,29 @@ def change_password_attempts(request):
		return 1


		def invalidate_and_regen_tokens(user):

Invalidate tokens on password change #1783

Invalidate tokens on password change #1783

Conversation

DavisRayM commented Feb 17, 2020 • edited Loading

Changes / Features

Steps taken to verify this change does what is intended

Side effects of implementing this change

ivermac 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

faith-mutua commented Mar 17, 2020

DavisRayM commented Mar 17, 2020

faith-mutua commented Mar 20, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DavisRayM commented Feb 17, 2020 •

edited

Loading