Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug: Valid Refresh Tokens despite user changing password #903

Open
epicadk opened this issue Sep 28, 2020 · 16 comments · May be fixed by #1001
Open

Bug: Valid Refresh Tokens despite user changing password #903

epicadk opened this issue Sep 28, 2020 · 16 comments · May be fixed by #1001
Assignees
Labels
Category: Coding Changes to code base or refactored code that doesn't fix a bug. Type: Bug Bug or Bug fixes.

Comments

@epicadk
Copy link
Member

epicadk commented Sep 28, 2020

Describe the bug

Refresh Tokens are still valid even after the user changes passwords.
To Reproduce

Steps to reproduce the behavior:

  1. Login and save the refresh token you get.
  2. Change password
  3. Scroll down to Refresh endpoint and use the old Refresh token.
  4. See error

Expected behavior

Refresh Tokens should not be valid after a user changes passwords.

Additional context

This can be done by using the users hashed password as the secret for the refresh tokens.

@epicadk epicadk closed this as completed Oct 22, 2020
@epicadk epicadk reopened this Oct 27, 2020
@epicadk epicadk changed the title Feature: Clearing Previous Auth Tokens when a user changes their password Feature: Valid Refresh Tokens despite user changing password Oct 27, 2020
@devkapilbansal
Copy link
Member

I don't know whether it is the case. Will verify today. Thanks @epicadk for opening this

@isabelcosta
Copy link
Member

@devkapilbansal if you can verify this, it would be amazing.

@epicadk can you please add more information to the ticket, for example how to reproduce this bug you found, so that whoever works on it, can have an example to follow. Also if you have any idea of a potential solution, even if its not the one being implemented, please put it in the alternatives :)

@epicadk epicadk changed the title Feature: Valid Refresh Tokens despite user changing password Bug: Valid Refresh Tokens despite user changing password Nov 24, 2020
@epicadk
Copy link
Member Author

epicadk commented Nov 24, 2020

@devkapilbansal if you can verify this, it would be amazing.

@epicadk can you please add more information to the ticket, for example how to reproduce this bug you found, so that whoever works on it, can have an example to follow. Also if you have any idea of a potential solution, even if its not the one being implemented, please put it in the alternatives :)

@isabelcosta done . @devkapilbansal I have added steps to reproduce the issues please let me know if I should elaborate any further.

@devkapilbansal
Copy link
Member

devkapilbansal commented Nov 29, 2020

Valid Issue ✔️

Thanks @epicadk to point out this security bug

Tested locally and able to get new access token using old refresh token that should not happen

TLDR

  • Changing Password Screenshot
    change_password

  • Successful Refresh
    successful_refresh

Access token generated is also valid and can be used after refresh.

@devkapilbansal
Copy link
Member

Also, I noticed that in the refresh endpoint it should mention refresh token instead of access token here

refresher

Therefore, opening an issue for this too

@devkapilbansal
Copy link
Member

@isabelcosta @vj-codes @rpattath please label this issue

@vj-codes vj-codes added Category: Coding Changes to code base or refactored code that doesn't fix a bug. Status: Available Issue was approved and available to claim or abandoned for over 3 days. Type: Bug Bug or Bug fixes. labels Nov 29, 2020
@isabelcosta
Copy link
Member

thank you for such a thorough test and showing the output here 🙌 @devkapilbansal

@isabelcosta
Copy link
Member

@devkapilbansal can you link up the issue here, in case you already created it?

@devkapilbansal
Copy link
Member

@devkapilbansal can you link up the issue here, in case you already created it?

The issue is #932

@tichnas
Copy link
Contributor

tichnas commented Dec 17, 2020

@isabelcosta @devkapilbansal Can I please get assigned as no one is working on this issue?
Thanks

@devkapilbansal
Copy link
Member

@tichnas consider asking it on zulip. You can work on this as soon as you are assigned to it

@gaurivn gaurivn removed the Status: Available Issue was approved and available to claim or abandoned for over 3 days. label Dec 17, 2020
@gaurivn
Copy link
Member

gaurivn commented Dec 17, 2020

Assigned @tichnas

@tichnas
Copy link
Contributor

tichnas commented Dec 17, 2020

Thanks a lot @gaurivn

@gaurivn
Copy link
Member

gaurivn commented Dec 17, 2020

You're welcome

tichnas added a commit to tichnas/mentorship-backend that referenced this issue Dec 28, 2020
tichnas added a commit to tichnas/mentorship-backend that referenced this issue Jan 2, 2021
tichnas added a commit to tichnas/mentorship-backend that referenced this issue Jan 4, 2021
@isabelcosta isabelcosta assigned epicadk and unassigned tichnas Feb 9, 2021
@epicadk
Copy link
Member Author

epicadk commented Feb 12, 2021

@isabelcosta an update working on the doc right now as discussed in the Mentorship system open session. Sorry it's taking so long.

@isabelcosta
Copy link
Member

Thank you for the update! That's ok @epicadk :)

@epicadk epicadk linked a pull request Feb 15, 2021 that will close this issue
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Category: Coding Changes to code base or refactored code that doesn't fix a bug. Type: Bug Bug or Bug fixes.
Projects
None yet
6 participants