CDM-243: Add MFA to TemporarySessionData LOGINIDENTS operation#503
CDM-243: Add MFA to TemporarySessionData LOGINIDENTS operation#503MrCreosote merged 1 commit intodevelopfrom
Conversation
All the other changes in this PR flow out of that single change. eesh. TODO: test the UITokens for MFA status once it's exposed in the API layer.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #503 +/- ##
==========================================
Coverage 93.41% 93.42%
- Complexity 2167 2169 +2
==========================================
Files 129 129
Lines 7626 7636 +10
Branches 1181 1184 +3
==========================================
+ Hits 7124 7134 +10
Misses 459 459
Partials 43 43 🚀 New features to boost your workflow:
|
| assertThat("incorrect state", ti.getOAuth2State(), is(ES)); | ||
| assertThat("incorrect pkce", ti.getPKCECodeVerifier(), is(ES)); | ||
| assertThat("incorrect user", ti.getUser(), is(Optional.empty())); | ||
| assertThat("incorrect user", ti.getUser(), is(opt())); |
There was a problem hiding this comment.
I would not immediately know opt() vs Optional.empty meant an empty optional, but I also am not steeped in Java norms.
There was a problem hiding this comment.
Yeah, it's just personal short hand since it's used over and over in these tests
dauglyon
left a comment
dauglyon
left a comment
There was a problem hiding this comment.
The test changes seem good, I didn't see anything unexpected except the test looping logic but that makes sense. Non-test changes are as expected. Not sure about the LoginState comment but assume it's a good idea given mfa state is indeed a type of login state. Should be good to merge!
|
The login state is just referring to the endpoint where the user gets to choose what account to log into (or for KBase, affirms they're logging into the right account. As such, MFA status doesn't really matter unless we wanted to stop the login flow right there or something |
|
It doesn't hurt to have it though, could be used to give feedback to the user that their MFA state is as expected (or not) |
2 participants
All the other changes in this PR flow out of that single change. eesh.
TODO: test the UITokens for MFA status once it's exposed in the API layer.
Turning whitespace off makes the review a lot simpler