-
Notifications
You must be signed in to change notification settings - Fork 38
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
Update okta auth to support 3-number challenge when Okta Verify mobile app forces extra verification #212
base: master
Are you sure you want to change the base?
Conversation
382af4e
to
0d49fed
Compare
@Pashtetollo this would be great to add but this new code will need unit tests. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #212 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 9 9
Lines 1131 1136 +5
=========================================
+ Hits 1131 1136 +5 ☔ View full report in Codecov by Sentry. |
@nathan-v Updated the test to cover the changes, not in the best way though as the request is updated with the number challenge data after you approve sign-in on your device (press Yes it's me), but that's better than nothing I suppose. Let me know if anything else is needed for this change to be included, |
@Pashtetollo Instead of re purposing an existing test a new test should be added that covers the alternate case of the phone number challenge. That way both potential branches are checked separately. :) After that it should be good. |
@nathan-v Rewrote the test as a separate one to check for display of a number |
This fixes #81 by pulling the correct number from request embedded data and displaying it in console once Okta Verify makes you pick one of 3 numbers