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

add okta email mfa support #1281

Merged
merged 4 commits into from
Jun 6, 2024
Merged

Conversation

tmesgong
Copy link
Contributor

@tmesgong tmesgong commented Jun 3, 2024

No description provided.

@codecov-commenter
Copy link

codecov-commenter commented Jun 4, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 42.23%. Comparing base (54e1e97) to head (37f0b63).
Report is 5 commits behind head on master.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1281      +/-   ##
==========================================
+ Coverage   42.08%   42.23%   +0.15%     
==========================================
  Files          54       54              
  Lines        6442     6442              
==========================================
+ Hits         2711     2721      +10     
+ Misses       3288     3274      -14     
- Partials      443      447       +4     
Flag Coverage Δ
unittests 42.23% <100.00%> (+0.15%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@mapkon
Copy link
Contributor

mapkon commented Jun 4, 2024

@tmesgong the linter is failing.

Copy link
Contributor

@mapkon mapkon left a comment

Choose a reason for hiding this comment

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

@tmesgong Do you mind adding some tests to this?

@tmesgong
Copy link
Contributor Author

tmesgong commented Jun 4, 2024

@mapkon I don't know how to test. It works for me. I follow the okta sms mfa code, but it doesn't have test. If there's similar test I can follow, I'd like add this later.

Copy link
Contributor

@mapkon mapkon left a comment

Choose a reason for hiding this comment

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

Ideally, you should look at how the okta_test.go is designed and add a test to exercise your case for the code. I would concentrate on using the test to document the scenarios and state needed for EmailMfa to function properly.

pkg/provider/okta/okta.go Outdated Show resolved Hide resolved
saml2aws.go Show resolved Hide resolved
@mapkon mapkon merged commit fff99ad into Versent:master Jun 6, 2024
8 checks passed
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.

3 participants