PEPPER-1063: Cleanup and refactoring of authentication code #2658
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
PEPPER-1063
A recent change to the authentication code meant DSM was no longer returning a response body for expired tokens (so DSM requesters had no way of knowing why authentication failed). Also, the expired token error continued to alert on Prod.
This PR fixes that issue, but also expands to normalize DSM's response to authentication issues. The intent is to ensure errors are getting logged at the appropriate level, and requesters are getting the correct status code and a response body describing the problem. The approach was largely to detangle the exception handling, logging and return protocols into something more coherent and consistent, but also includes new/updated method Java doc, and some selective cleanup in code that was poorly structured and/or particularly difficult to read. (There is more cleanup to do, but it makes sense to let these changes settle first.)
Checklist
C-*
labels.R-*
labels.L-*
labels as needed.I-*
labels as neededIf unsure or need help with any of the above items, add the
help wanted
label. For items that starts withIf applicable
, if it is not applicable, check it off and addn/a
in front.FUD Score
Overall, how are you feeling about these changes?
How do we demo these changes?
How does one observe these changes in a deployed system? Note that user visible encompasses many personas--not just patients and study staff, but also ops duty, your fellow devs, compliance, etc.
Testing
Release