-
Notifications
You must be signed in to change notification settings - Fork 51
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
Refresh APS Token #982
Refresh APS Token #982
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couple of things to tweak. Consult with this documentation for addressing errors.
…ynthesis into jwrigh/1699/aps-refresh-token
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I managed to get the token to expire in such a way that resulted in a failure to login. This then followed a toast and this error:
Upon which the APS Login button seemed to be broken and would not let me login again.
Also there appears to be lots of error messages in the developer console while running this branch, things related to message ports closing before a expected response? Is this expected?
Edit - Steps to Reproduce:
- Spam the expire token button.
Not sure that this issue would ever be reproducible in production, just want to make sure that this is not a symptom of a different problem.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See previous review comment, lets not let this get in the way anymore. It has been 3 weeks...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry to backtrack on previous review.
@PepperLola I'm having issues with the code challenge endpoint. For some reason it's giving a 500 error |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good. Can't wait to see the further integration.
Chorus detected one or more security issues with this pull request. See the Checks tab for more details. As a reminder, please follow the secure code review process as part of the Secure Coding Non-Negotiable requirement. |
Changes:
Testing:
Important
The server has to be running when testing, which can be found in the website repo.
Important
Waiting on server update PR to mergeJIRA Issue