ESD-25022: Redesign how mfa types get enabled within the guardian resource #423
Conversation
Codecov ReportBase: 86.79% // Head: 87.01% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #423 +/- ##
==========================================
+ Coverage 86.79% 87.01% +0.21%
==========================================
Files 41 41
Lines 9036 9116 +80
==========================================
+ Hits 7843 7932 +89
+ Misses 924 910 -14
- Partials 269 274 +5
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
| @@ -39,12 +39,18 @@ func newGuardian() *schema.Resource { | |||
| "webauthn_roaming": { | |||
| Type: schema.TypeList, | |||
| Optional: true, | |||
| Computed: true, | |||
There was a problem hiding this comment.
We are adding the computed property to all mfa objects as some of their attributes are already set by the API when a new tenant is created and we want to avoid needing to specify all of them just to get started.
| "provider": { | ||
| Type: schema.TypeString, | ||
| Required: true, | ||
| Optional: true, |
There was a problem hiding this comment.
We are changing some of the nested properties on mfa types from Required to Optional so we can allow simply setting an mfa type to disabled without needing to specify extra attributes.
| ), | ||
| }, | ||
| { | ||
| Config: testAccGuardianEmailDelete, | ||
| Check: resource.ComposeTestCheckFunc( | ||
| resource.TestCheckResourceAttr("auth0_guardian.foo", "policy", "all-applications"), | ||
| resource.TestCheckResourceAttr("auth0_guardian.foo", "email", "false"), | ||
| resource.TestCheckResourceAttr("auth0_guardian.foo", "otp", "false"), |
There was a problem hiding this comment.
These can be removed as the mfa types are computed, and values will get ready from the API.
🔧 Changes
In this PR we are redesigning how the enable/disable mechanism works for mfa types within the
auth0_guardianresource.Previously we used to toggle them on/off implicitly based purely on whether the mfa block was present or not.
E.g.
The above would enable WebAuthn with FIDO Device Biometrics MFA.
To provide a better developer experience and to fix a couple of obscure bugs related to the diffing mechanism when out of bounds changes are made (push-notification wasn't showing correctly diffs), we are introducing an
enabledelement to each mfa block.E.g.
Note: The high amount of lines changed shown is mostly due to the test recordings, so don't get alarmed! 😅
📚 References
🔬 Testing
📝 Checklist