Skip to content
This repository has been archived by the owner on Oct 30, 2021. It is now read-only.

[BB-1718] Implement JWT token refreshing #24

Merged

Conversation

Agrendalath
Copy link
Member

@Agrendalath Agrendalath commented Oct 4, 2019

This implements JWT token refreshing. Access tokens will be valid for shorter (JWT_ACCESS_TOKEN_LIFETIME_HOURS) time and refresh tokens will be used to regenerate them. The refresh tokens are rotating, so we won't need to log in anymore as long as we visit the page once per JWT_REFRESH_TOKEN_LIFETIME_DAYS days.

I added one new line to envs on the ticket: JWT_ACCESS_TOKEN_LIFETIME_HOURS=0.0004. This will make access tokens last for less than 2 seconds, which will be enough for making singular requests and will speed up the testing.

Testing instructions:

  1. Set .env file (you can find it in a comment on the ticket).
  2. Start the backend with docker-compose -f local.yml up --build (build is important here; it will take a while, because our first layer of the Dockerfile has changed).
  3. Run migrations with docker-compose -f local.yml exec django python manage.py migrate.
  4. Create frontend/.env.local and set REACT_APP_GOOGLE_CLIENT_ID there.
  5. Optional (this will speed up testing, as the accounts' rework is not merged into the master yet): remove lines 82 and 83 from frontent/src/App.js.
  6. Navigate to frontend and run npm i && npm start.
  7. Open "Network" tab in developer tools.
  8. Log in.
  9. Refresh the page and see that user/ returned 401, was followed by refresh/, which returned 200, which was then followed by user/ returning 200.
  10. Click on the cell's name and observe the same pattern for ?board_id=<id> endpoint.

@Agrendalath Agrendalath requested a review from xitij2000 October 4, 2019 12:30
@Agrendalath Agrendalath self-assigned this Oct 4, 2019
Copy link
Member

@xitij2000 xitij2000 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

  • I tested this: followed testing instructions in PR
  • I read through the code

coreapi~=2.3.3 # https://github.com/core-api/python-client
drf-yasg~=1.15.0 # https://github.com/axnsan12/drf-yasg

# REST auth
# ------------------------------------------------------------------------------
django-rest-auth~=0.9.5 # https://github.com/Tivix/django-rest-auth
git+https://github.com/open-craft/django-rest-auth # fork of https://github.com/Tivix/django-rest-auth with simpleJWT support
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you pin this to a commit id please?
Why was this fork necessary?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you pin this to a commit id please?

Sure, that makes sense 👍

Why was this fork necessary?

The implementation of JWT used by django-rest-auth is no longer maintained . There is a fork of the django-rest-auth repository that adds SimpleJWT support (which has the token refreshing fixed).
I forked this to our GH organization, because forks tend to go missing sometimes and a couple year ago I had this exact problem with disappearing Python library (django-pagination), to which there were no mirrors at the point. So I just did this to keep it safe.

@Agrendalath Agrendalath force-pushed the agrendalath/bb-1718-implement_jwt_token_refreshing branch from 73c74d5 to 4926918 Compare October 5, 2019 14:51
@Agrendalath Agrendalath merged commit 590acc8 into master Oct 5, 2019
@Agrendalath Agrendalath deleted the agrendalath/bb-1718-implement_jwt_token_refreshing branch October 5, 2019 16:45
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants