-
Notifications
You must be signed in to change notification settings - Fork 89
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
[4/X] [DXCDT-175] Add recovery code to guardian resource and refactor #216
Conversation
82ac5fb
to
dc34f25
Compare
6bdbca8
to
393aaaa
Compare
393aaaa
to
abf36b2
Compare
Codecov Report
@@ Coverage Diff @@
## feature/DXCDT-175-guardian++-part3 #216 +/- ##
======================================================================
+ Coverage 82.98% 83.52% +0.54%
======================================================================
Files 36 36
Lines 6541 6537 -4
======================================================================
+ Hits 5428 5460 +32
+ Misses 882 858 -24
+ Partials 231 219 -12
Continue to review full report at Codecov.
|
@@ -37,7 +37,6 @@ func newGuardian() *schema.Resource { | |||
Type: schema.TypeList, | |||
Optional: true, | |||
MaxItems: 1, | |||
MinItems: 0, |
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.
This gets ignored completely if optional is true.
multiFactorPolicies, err := api.Guardian.MultiFactor.Policy() | ||
flattenedPolicy, err := flattenMultiFactorPolicy(api) | ||
if err != nil { | ||
return diag.FromErr(err) | ||
} | ||
|
||
result := multierror.Append(d.Set("policy", "never")) | ||
if len(*multiFactorPolicies) > 0 { | ||
result = multierror.Append(result, d.Set("policy", (*multiFactorPolicies)[0])) | ||
} | ||
result := multierror.Append(d.Set("policy", flattenedPolicy)) |
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.
Extracting this logic somewhere else so it's easier to read through the readGuardian steps.
result := multierror.Append( | ||
updatePolicy(d, api), | ||
updateEmailFactor(d, api), | ||
updateOTPFactor(d, api), | ||
updateRecoveryCodeFactor(d, api), | ||
updatePhoneFactor(d, api), | ||
updateWebAuthnRoaming(d, api), | ||
updateWebAuthnPlatform(d, api), | ||
updateDUO(d, api), | ||
updatePush(d, api), | ||
) |
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.
It's better if we try al of these updates and report on any accumulated errors instead of failing at the first and not doing any of the others.
result := multierror.Append( | ||
api.Guardian.MultiFactor.Phone.Enable(false), | ||
api.Guardian.MultiFactor.Email.Enable(false), | ||
api.Guardian.MultiFactor.OTP.Enable(false), | ||
api.Guardian.MultiFactor.RecoveryCode.Enable(false), | ||
api.Guardian.MultiFactor.WebAuthnRoaming.Enable(false), | ||
api.Guardian.MultiFactor.WebAuthnPlatform.Enable(false), | ||
api.Guardian.MultiFactor.DUO.Enable(false), | ||
api.Guardian.MultiFactor.Push.Enable(false), |
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.
It's better if we try al of these updates and report on any accumulated errors instead of failing at the first and not doing any of the others.
resource.TestCheckResourceAttr("auth0_guardian.foo", "email", "true"), | ||
resource.TestCheckResourceAttr("auth0_guardian.foo", "otp", "false"), | ||
resource.TestCheckResourceAttr("auth0_guardian.foo", "phone.#", "0"), | ||
resource.TestCheckResourceAttr("auth0_guardian.foo", "duo.#", "0"), | ||
resource.TestCheckResourceAttr("auth0_guardian.foo", "webauthn_platform.#", "0"), | ||
resource.TestCheckResourceAttr("auth0_guardian.foo", "webauthn_roaming.#", "0"), | ||
resource.TestCheckResourceAttr("auth0_guardian.foo", "push.#", "0"), | ||
resource.TestCheckResourceAttr("auth0_guardian.foo", "recovery_code", "false"), |
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.
We're splitting the 1 big test into multiple ones that focus on distinct MFA types. Also we're now checking the full state for all the possible parameters for a thorough inspection.
Description
Add recovery code to guardian resource and refactor.
Checklist
Note: Checklist required to be completed before a PR is considered to be reviewable.
Auth0 Code of Conduct
Auth0 General Contribution Guidelines
Changes include test coverage?
Does the description provide the correct amount of context?
Have you updated the documentation?
Is this code ready for production?