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

3137 UI refresh token logic #3188

Closed
wants to merge 2 commits into from

Conversation

ilija-lazoroski
Copy link
Contributor

@ilija-lazoroski ilija-lazoroski commented Apr 3, 2023

What does this PR do?

Fixes #3137 .

Add any further explanations here.

PR Checklist

  • Have you added an explanation of what your changes do and why you'd like to include them?
  • Is the TravisCI build passing?
  • Was the CHANGELOG.md updated to reflect the changes?
  • Was the documentation framework updated to reflect the changes?
  • Have you checked that you haven't introduced any duplicate code?

Testing Checklist

  • Added relevant unit tests?
  • Do all unit tests pass?
  • Do all end-to-end tests pass?
  • Any other testing performed?

    Tested by {Running the Monkey locally with relevant config/running Island/...}

  • If applicable, add screenshots or log transcripts of the feature working

@ilija-lazoroski ilija-lazoroski changed the base branch from develop to 3137-new-token-pair-endpoint April 3, 2023 13:11
@codecov
Copy link

codecov bot commented Apr 3, 2023

Codecov Report

❗ No coverage uploaded for pull request base (develop@e3f04ea). Click here to learn what that means.
Patch has no changes to coverable lines.

❗ Current head b2905ea differs from pull request most recent head df4fd60. Consider uploading reports for the commit df4fd60 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             develop    #3188   +/-   ##
==========================================
  Coverage           ?   72.75%           
==========================================
  Files              ?      469           
  Lines              ?    13424           
  Branches           ?        0           
==========================================
  Hits               ?     9767           
  Misses             ?     3657           
  Partials           ?        0           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@ilija-lazoroski ilija-lazoroski force-pushed the 3137-ui-refresh-token-logic branch 5 times, most recently from 1df8bd8 to 7630f4f Compare April 3, 2023 14:02
ilija-lazoroski added a commit that referenced this pull request Apr 3, 2023
@ilija-lazoroski ilija-lazoroski force-pushed the 3137-ui-refresh-token-logic branch from 7630f4f to 06d6eb7 Compare April 3, 2023 14:10
ilija-lazoroski added a commit that referenced this pull request Apr 3, 2023
@ilija-lazoroski ilija-lazoroski force-pushed the 3137-ui-refresh-token-logic branch from 06d6eb7 to 12a0fa2 Compare April 3, 2023 14:21
ilija-lazoroski added a commit that referenced this pull request Apr 3, 2023
@ilija-lazoroski ilija-lazoroski force-pushed the 3137-ui-refresh-token-logic branch from 12a0fa2 to 32012ef Compare April 3, 2023 14:30
ilija-lazoroski added a commit that referenced this pull request Apr 3, 2023
@ilija-lazoroski ilija-lazoroski force-pushed the 3137-ui-refresh-token-logic branch from 32012ef to 1e39f45 Compare April 3, 2023 14:56
ilija-lazoroski added a commit that referenced this pull request Apr 3, 2023
@ilija-lazoroski ilija-lazoroski force-pushed the 3137-ui-refresh-token-logic branch from 1e39f45 to 68a875a Compare April 3, 2023 15:01
@mssalvatore mssalvatore force-pushed the 3137-new-token-pair-endpoint branch from 315c23c to a2ce7d3 Compare April 3, 2023 15:39
Base automatically changed from 3137-new-token-pair-endpoint to develop April 3, 2023 15:41
mssalvatore pushed a commit that referenced this pull request Apr 3, 2023
@mssalvatore mssalvatore force-pushed the 3137-ui-refresh-token-logic branch from 68a875a to 2ec7554 Compare April 3, 2023 16:53
ilija-lazoroski added a commit that referenced this pull request Apr 4, 2023
@ilija-lazoroski ilija-lazoroski force-pushed the 3137-ui-refresh-token-logic branch from 2ec7554 to 66a25d0 Compare April 4, 2023 08:02
ilija-lazoroski added a commit that referenced this pull request Apr 4, 2023
@ilija-lazoroski ilija-lazoroski force-pushed the 3137-ui-refresh-token-logic branch from 66a25d0 to 5619e8c Compare April 4, 2023 09:32
ilija-lazoroski added a commit that referenced this pull request Apr 4, 2023
@ilija-lazoroski ilija-lazoroski force-pushed the 3137-ui-refresh-token-logic branch from 5619e8c to 43d9db5 Compare April 4, 2023 09:35
ilija-lazoroski added a commit that referenced this pull request Apr 4, 2023
@ilija-lazoroski ilija-lazoroski force-pushed the 3137-ui-refresh-token-logic branch from 43d9db5 to 5074344 Compare April 4, 2023 09:41
ilija-lazoroski added a commit that referenced this pull request Apr 4, 2023
@ilija-lazoroski ilija-lazoroski force-pushed the 3137-ui-refresh-token-logic branch from 5074344 to 1c66d99 Compare April 4, 2023 09:50
mssalvatore pushed a commit that referenced this pull request Apr 4, 2023
@mssalvatore mssalvatore force-pushed the 3137-ui-refresh-token-logic branch from b2905ea to d834c46 Compare April 4, 2023 11:43
Comment on lines +191 to +196
_removeTokens(tokenNameArray) {
tokenNameArray?.forEach(tokenName => {
localStorage.removeItem(tokenName);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there any reason we don't want to always remove the auth and refresh tokens together?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, there is. When we do a request and it is 401, we can immediately remove the authentication token but not the refresh token as we need it so we can generate a new token pair.

@ilija-lazoroski ilija-lazoroski force-pushed the 3137-ui-refresh-token-logic branch from d834c46 to df4fd60 Compare April 4, 2023 11:54
@mssalvatore
Copy link
Collaborator

We went a different direction.

@ilija-lazoroski ilija-lazoroski deleted the 3137-ui-refresh-token-logic branch May 3, 2023 14:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Generate refresh token
4 participants