-
Notifications
You must be signed in to change notification settings - Fork 1
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
Lenient shield builder #315
Conversation
…to ac/mix_persona_and_accounts_in_factor_instances_provider
…allowed FactorSourceKind - add query methods which can query which kinds are supported/if a kind is supported
…r' into ac/shield_upgrade_validate_kind_for_rola_and_more_lenient_add_factors
{ | ||
return duplicates_err; | ||
} | ||
// but if threshold contains it, we're good (since mode is lenient) |
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.
With Lenient, we allow a factor which is in override
to be added to threshold
(and vice versa)
} | ||
} | ||
SecurityShieldBuilderMode::Lenient => {} |
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.
for Lenient
we do not count the number of Device
FactorSource, we allow many.
@@ -707,6 +829,10 @@ impl<const ROLE: u8> RoleBuilder<ROLE> { | |||
); | |||
} | |||
|
|||
if mode == SecurityShieldBuilderMode::Lenient { | |||
return Ok(()); |
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.
For Lenient
we allow password
by itself.
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.
Very nice!
You can merge it, and I'll handle the conflicts on my branch when I’m back.
pushing a fix for swift tests now |
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.
Perfect!
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #315 +/- ##
=======================================
- Coverage 93.3% 93.3% -0.1%
=======================================
Files 1111 1111
Lines 24064 24182 +118
Branches 79 79
=======================================
+ Hits 22464 22570 +106
- Misses 1585 1597 +12
Partials 15 15
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Allows for more lenient
SecurityShieldBuilder
, by passingSecurityShieldBuilderMode::Lenient
asmode
toSecurityShieldBuilder
.Changes
The
RoleBuilder
andMatrixBuilder
have got a new variant of almost all their methods, having the suffix_with_mode
which takes amode: SecurityShieldBuilderMode
. The reason why this is not a field on theMatrixBuilder
androle_builder
is because it requires bigger changes toAbstractRoleBuilderOrBuilt
andAbstractMatrixBuilderOrBuilt
- which we can do but a bit more intrusive change. We could potentially add a serde_skipped extra field which we set to()
forBuilt
but toSecurityShieldBuilderMode
(which we might rename toFactorBuilderMode
to fit more intoRoleBuilder
) for non-Built (i.e. builders).The SecurityShield builder than pass
self.mode
to all required methods.The RoleBuilder has been updated to make use of
mode
passed as method argument to be more lenient - when applicable.