diff --git a/Cargo.lock b/Cargo.lock index 719a7dba9..430168b9f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2761,7 +2761,7 @@ dependencies = [ [[package]] name = "sargon" -version = "1.1.94" +version = "1.1.95" dependencies = [ "actix-rt", "aes-gcm", @@ -2816,7 +2816,7 @@ dependencies = [ [[package]] name = "sargon-uniffi" -version = "1.1.94" +version = "1.1.95" dependencies = [ "actix-rt", "assert-json-diff", diff --git a/apple/Tests/TestCases/Profile/MFA/SecurityShieldsBuilderTests.swift b/apple/Tests/TestCases/Profile/MFA/SecurityShieldsBuilderTests.swift index 7061ba22b..93f195649 100644 --- a/apple/Tests/TestCases/Profile/MFA/SecurityShieldsBuilderTests.swift +++ b/apple/Tests/TestCases/Profile/MFA/SecurityShieldsBuilderTests.swift @@ -96,7 +96,9 @@ struct ShieldTests { builder.addFactorSourceToPrimaryThreshold(factorSourceId: .sampleDevice) builder.addFactorSourceToPrimaryThreshold(factorSourceId: .sampleDevice) // did not get added, duplicates are not allowed #expect(builder.primaryRoleThresholdFactors == [.sampleDevice]) - builder.addFactorSourceToPrimaryThreshold(factorSourceId: .sampleDeviceOther) + builder.addFactorSourceToPrimaryThreshold(factorSourceId: .sampleDeviceOther) // actually this is added + #expect(builder.validate() == .PrimaryCannotHaveMultipleDevices) + builder.removeFactorFromPrimary(factorSourceId: .sampleDeviceOther) #expect(builder.validate() == .RecoveryRoleMustHaveAtLeastOneFactor) builder.removeFactorFromPrimary(factorSourceId: .sampleDeviceOther) @@ -139,7 +141,7 @@ struct ShieldTests { #expect(builder.confirmationRoleFactors.isEmpty) } - @Test("Primary can only contain one DeviceFactorSource") + @Test("Primary can contain two DeviceFactorSource while building - but is never valid") func primaryCanOnlyContainOneDeviceFactorSourceThreshold() throws { let builder = SecurityShieldBuilder() let factor = FactorSourceId.sampleDevice @@ -147,14 +149,18 @@ struct ShieldTests { builder.addFactorSourceToPrimaryThreshold(factorSourceId: factor) builder.addFactorSourceToPrimaryOverride(factorSourceId: other) #expect(builder.primaryRoleThresholdFactors == [factor]) - #expect(builder.primaryRoleOverrideFactors == []) + #expect(builder.primaryRoleOverrideFactors == [other]) builder.removeFactorFromPrimary(factorSourceId: factor) builder.addFactorSourceToPrimaryOverride(factorSourceId: factor) builder.addFactorSourceToPrimaryThreshold(factorSourceId: other) - #expect(builder.primaryRoleThresholdFactors == []) - #expect(builder.primaryRoleOverrideFactors == [factor]) + #expect(builder.primaryRoleThresholdFactors == [other]) + #expect(builder.primaryRoleOverrideFactors == [other, factor]) + + // But when validated/built is err + #expect(builder.validate() != nil) + #expect((try? builder.build()) == nil) } @Test("Primary password never alone") @@ -164,7 +170,8 @@ struct ShieldTests { #expect(builder.primaryRoleOverrideFactors.isEmpty) builder.addFactorSourceToPrimaryThreshold(factorSourceId: .samplePassword) - #expect(builder.validate() == .PrimaryRoleWithThresholdFactorsCannotHaveAThresholdValueOfZero) + #expect(builder.validate() == .PrimaryRoleWithPasswordInThresholdListMustHaveAnotherFactor) + builder.threshold = 0 #expect(builder.validate() == .PrimaryRoleWithThresholdFactorsCannotHaveAThresholdValueOfZero) builder.threshold = 1 diff --git a/crates/sargon-uniffi/Cargo.toml b/crates/sargon-uniffi/Cargo.toml index f6305e707..1a0e1ea39 100644 --- a/crates/sargon-uniffi/Cargo.toml +++ b/crates/sargon-uniffi/Cargo.toml @@ -1,7 +1,7 @@ [package] name = "sargon-uniffi" # Don't forget to update version in crates/sargon/Cargo.toml -version = "1.1.94" +version = "1.1.95" edition = "2021" build = "build.rs" diff --git a/crates/sargon-uniffi/src/profile/mfa/security_structures/security_shield_builder.rs b/crates/sargon-uniffi/src/profile/mfa/security_structures/security_shield_builder.rs index 4504cdfa3..2da5abacf 100644 --- a/crates/sargon-uniffi/src/profile/mfa/security_structures/security_shield_builder.rs +++ b/crates/sargon-uniffi/src/profile/mfa/security_structures/security_shield_builder.rs @@ -28,7 +28,7 @@ impl SecurityShieldBuilder { #[uniffi::constructor] pub fn new() -> Arc { Arc::new(Self { - wrapped: RwLock::new(sargon::SecurityShieldBuilder::new()), + wrapped: RwLock::new(sargon::SecurityShieldBuilder::lenient()), }) } } @@ -749,7 +749,7 @@ mod tests { ]) ); - assert_ne!( + assert_eq!( // we use lenient builder, so we say state has not changed sim_prim_threshold, sut.validation_for_addition_of_factor_source_to_primary_threshold_for_each( vec![ diff --git a/crates/sargon/Cargo.toml b/crates/sargon/Cargo.toml index 622891933..6a458775a 100644 --- a/crates/sargon/Cargo.toml +++ b/crates/sargon/Cargo.toml @@ -1,7 +1,7 @@ [package] name = "sargon" # Don't forget to update version in crates/sargon-uniffi/Cargo.toml -version = "1.1.94" +version = "1.1.95" edition = "2021" build = "build.rs" diff --git a/crates/sargon/src/profile/mfa/security_structures/automatic_shield_builder/automatic_shield_builder.rs b/crates/sargon/src/profile/mfa/security_structures/automatic_shield_builder/automatic_shield_builder.rs index b3b727b50..a4f5e27f7 100644 --- a/crates/sargon/src/profile/mfa/security_structures/automatic_shield_builder/automatic_shield_builder.rs +++ b/crates/sargon/src/profile/mfa/security_structures/automatic_shield_builder/automatic_shield_builder.rs @@ -464,7 +464,7 @@ mod tests { SecurityStructureOfFactorSourceIDs, AutoBuildOutcomeForTesting, )> { - let shield_builder = SecurityShieldBuilder::new(); + let shield_builder = SecurityShieldBuilder::default(); shield_builder.set_threshold(pick_primary_role_factors.len() as u8); pick_primary_role_factors.into_iter().for_each(|f| { shield_builder.add_factor_source_to_primary_threshold(f); diff --git a/crates/sargon/src/profile/mfa/security_structures/matrices/builder/matrix_builder.rs b/crates/sargon/src/profile/mfa/security_structures/matrices/builder/matrix_builder.rs index 2f962bd6f..12951a625 100644 --- a/crates/sargon/src/profile/mfa/security_structures/matrices/builder/matrix_builder.rs +++ b/crates/sargon/src/profile/mfa/security_structures/matrices/builder/matrix_builder.rs @@ -90,64 +90,118 @@ impl MatrixBuilder { pub fn validation_for_addition_of_factor_source_to_primary_threshold_for_each( &self, factor_sources: &IndexSet, + ) -> IndexSet { + self.validation_for_addition_of_factor_source_to_primary_threshold_for_each_with_mode(factor_sources, SecurityShieldBuilderMode::Strict) + } + + pub fn validation_for_addition_of_factor_source_to_primary_threshold_for_each_with_mode( + &self, + factor_sources: &IndexSet, + mode: SecurityShieldBuilderMode, ) -> IndexSet { self.primary_role - .validation_for_addition_of_factor_source_for_each( + .validation_for_addition_of_factor_source_for_each_with_mode( FactorListKind::Threshold, factor_sources, + mode, ) } pub fn validation_for_addition_of_factor_source_to_primary_override_for_each( &self, factor_sources: &IndexSet, + ) -> IndexSet { + self.validation_for_addition_of_factor_source_to_primary_override_for_each_with_mode(factor_sources, SecurityShieldBuilderMode::Strict) + } + + pub fn validation_for_addition_of_factor_source_to_primary_override_for_each_with_mode( + &self, + factor_sources: &IndexSet, + mode: SecurityShieldBuilderMode, ) -> IndexSet { self.primary_role - .validation_for_addition_of_factor_source_for_each( + .validation_for_addition_of_factor_source_for_each_with_mode( FactorListKind::Override, factor_sources, + mode, ) } pub fn validation_for_addition_of_factor_source_of_kind_to_recovery_override( &self, factor_source_kind: FactorSourceKind, + ) -> RoleBuilderMutateResult { + self.validation_for_addition_of_factor_source_of_kind_to_recovery_override_with_mode(factor_source_kind, SecurityShieldBuilderMode::Strict) + } + + pub fn validation_for_addition_of_factor_source_of_kind_to_recovery_override_with_mode( + &self, + factor_source_kind: FactorSourceKind, + mode: SecurityShieldBuilderMode, ) -> RoleBuilderMutateResult { self.recovery_role - .validation_for_addition_of_factor_source_of_kind_to_override( + .validation_for_addition_of_factor_source_of_kind_to_override_with_mode( factor_source_kind, + mode ) } pub fn validation_for_addition_of_factor_source_to_recovery_override_for_each( &self, factor_sources: &IndexSet, + ) -> IndexSet { + self.validation_for_addition_of_factor_source_to_recovery_override_for_each_with_mode(factor_sources, SecurityShieldBuilderMode::Strict) + } + + pub fn validation_for_addition_of_factor_source_to_recovery_override_for_each_with_mode( + &self, + factor_sources: &IndexSet, + mode: SecurityShieldBuilderMode, ) -> IndexSet { self.recovery_role - .validation_for_addition_of_factor_source_for_each( + .validation_for_addition_of_factor_source_for_each_with_mode( FactorListKind::Override, factor_sources, + mode, ) } pub fn validation_for_addition_of_factor_source_of_kind_to_confirmation_override( &self, factor_source_kind: FactorSourceKind, + ) -> RoleBuilderMutateResult { + self.validation_for_addition_of_factor_source_of_kind_to_confirmation_override_with_mode(factor_source_kind, SecurityShieldBuilderMode::Strict) + } + + pub fn validation_for_addition_of_factor_source_of_kind_to_confirmation_override_with_mode( + &self, + factor_source_kind: FactorSourceKind, + mode: SecurityShieldBuilderMode, ) -> RoleBuilderMutateResult { self.confirmation_role - .validation_for_addition_of_factor_source_of_kind_to_override( + .validation_for_addition_of_factor_source_of_kind_to_override_with_mode( factor_source_kind, + mode ) } pub fn validation_for_addition_of_factor_source_to_confirmation_override_for_each( &self, factor_sources: &IndexSet, + ) -> IndexSet { + self.validation_for_addition_of_factor_source_to_confirmation_override_for_each_with_mode(factor_sources, SecurityShieldBuilderMode::Strict) + } + + pub fn validation_for_addition_of_factor_source_to_confirmation_override_for_each_with_mode( + &self, + factor_sources: &IndexSet, + mode: SecurityShieldBuilderMode, ) -> IndexSet { self.confirmation_role - .validation_for_addition_of_factor_source_for_each( + .validation_for_addition_of_factor_source_for_each_with_mode( FactorListKind::Override, factor_sources, + mode, ) } @@ -198,9 +252,20 @@ impl MatrixBuilder { pub fn add_factor_source_to_primary_threshold( &mut self, factor_source_id: FactorSourceID, + ) -> MatrixBuilderMutateResult { + self.add_factor_source_to_primary_threshold_with_mode( + factor_source_id, + SecurityShieldBuilderMode::Strict, + ) + } + + pub fn add_factor_source_to_primary_threshold_with_mode( + &mut self, + factor_source_id: FactorSourceID, + mode: SecurityShieldBuilderMode, ) -> MatrixBuilderMutateResult { self.primary_role - .add_factor_source_to_threshold(factor_source_id) + .add_factor_source_to_threshold_with_mode(factor_source_id, mode) .into_matrix_err(RoleKind::Primary) } @@ -218,27 +283,61 @@ impl MatrixBuilder { pub fn add_factor_source_to_primary_override( &mut self, factor_source_id: FactorSourceID, + ) -> MatrixBuilderMutateResult { + self.add_factor_source_to_primary_override_with_mode( + factor_source_id, + SecurityShieldBuilderMode::Strict, + ) + } + + /// Adds the factor source to the primary role override list. + pub fn add_factor_source_to_primary_override_with_mode( + &mut self, + factor_source_id: FactorSourceID, + mode: SecurityShieldBuilderMode, ) -> MatrixBuilderMutateResult { self.primary_role - .add_factor_source_to_override(factor_source_id) + .add_factor_source_to_override_with_mode(factor_source_id, mode) .into_matrix_err(RoleKind::Primary) } pub fn add_factor_source_to_recovery_override( &mut self, factor_source_id: FactorSourceID, + ) -> MatrixBuilderMutateResult { + self.add_factor_source_to_recovery_override_with_mode( + factor_source_id, + SecurityShieldBuilderMode::Strict, + ) + } + + pub fn add_factor_source_to_recovery_override_with_mode( + &mut self, + factor_source_id: FactorSourceID, + mode: SecurityShieldBuilderMode, ) -> MatrixBuilderMutateResult { self.recovery_role - .add_factor_source_to_override(factor_source_id) + .add_factor_source_to_override_with_mode(factor_source_id, mode) .into_matrix_err(RoleKind::Recovery) } pub fn add_factor_source_to_confirmation_override( &mut self, factor_source_id: FactorSourceID, + ) -> MatrixBuilderMutateResult { + self.add_factor_source_to_confirmation_override_with_mode( + factor_source_id, + SecurityShieldBuilderMode::Strict, + ) + } + + pub fn add_factor_source_to_confirmation_override_with_mode( + &mut self, + factor_source_id: FactorSourceID, + mode: SecurityShieldBuilderMode, ) -> MatrixBuilderMutateResult { self.confirmation_role - .add_factor_source_to_override(factor_source_id) + .add_factor_source_to_override_with_mode(factor_source_id, mode) .into_matrix_err(RoleKind::Confirmation) } diff --git a/crates/sargon/src/profile/mfa/security_structures/roles/builder/roles_builder.rs b/crates/sargon/src/profile/mfa/security_structures/roles/builder/roles_builder.rs index 468f3a828..c3a651f1d 100644 --- a/crates/sargon/src/profile/mfa/security_structures/roles/builder/roles_builder.rs +++ b/crates/sargon/src/profile/mfa/security_structures/roles/builder/roles_builder.rs @@ -217,10 +217,21 @@ where pub(crate) fn add_factor_source_to_threshold( &mut self, factor_source_id: FactorSourceID, + ) -> RoleBuilderMutateResult { + self.add_factor_source_to_threshold_with_mode( + factor_source_id, + SecurityShieldBuilderMode::Strict, + ) + } + + pub(crate) fn add_factor_source_to_threshold_with_mode( + &mut self, + factor_source_id: FactorSourceID, + mode: SecurityShieldBuilderMode, ) -> RoleBuilderMutateResult { let should_set_threshold_to_one = self.get_threshold() == 0 && self.get_threshold_factors().is_empty(); - self._add_factor_source_to_list(factor_source_id, Threshold) + self._add_factor_source_to_list(factor_source_id, Threshold, mode) .inspect(|_| { if should_set_threshold_to_one { let _ = self.set_threshold(1); @@ -234,7 +245,15 @@ where &self, factor_source_kind: FactorSourceKind, ) -> RoleBuilderMutateResult { - self._validation_add(factor_source_kind, Threshold) + self.validation_for_addition_of_factor_source_of_kind_to_threshold_with_mode(factor_source_kind, SecurityShieldBuilderMode::Strict) + } + + pub(crate) fn validation_for_addition_of_factor_source_of_kind_to_threshold_with_mode( + &self, + factor_source_kind: FactorSourceKind, + mode: SecurityShieldBuilderMode, + ) -> RoleBuilderMutateResult { + self._validation_add(factor_source_kind, Threshold, mode) } #[cfg(test)] @@ -243,7 +262,11 @@ where factor_source_kind: FactorSourceKind, list: FactorListKind, ) -> RoleBuilderMutateResult { - self._validation_add(factor_source_kind, list) + self._validation_add( + factor_source_kind, + list, + SecurityShieldBuilderMode::Strict, + ) } } @@ -272,7 +295,18 @@ impl RoleBuilder { &mut self, factor_source_id: FactorSourceID, ) -> RoleBuilderMutateResult { - self._add_factor_source_to_list(factor_source_id, Override) + self.add_factor_source_to_override_with_mode( + factor_source_id, + SecurityShieldBuilderMode::Strict, + ) + } + + pub(crate) fn add_factor_source_to_override_with_mode( + &mut self, + factor_source_id: FactorSourceID, + mode: SecurityShieldBuilderMode, + ) -> RoleBuilderMutateResult { + self._add_factor_source_to_list(factor_source_id, Override, mode) } /// ```ignore @@ -283,10 +317,12 @@ impl RoleBuilder { &mut self, factor_source_id: FactorSourceID, factor_list_kind: FactorListKind, + mode: SecurityShieldBuilderMode, ) -> RoleBuilderMutateResult { let validation = self.validation_for_addition_of_factor_source_to_list( &factor_source_id, factor_list_kind, + mode, ); match validation.as_ref() { Ok(()) | Err(NotYetValid(_)) => { @@ -306,7 +342,15 @@ impl RoleBuilder { &self, factor_source_kind: FactorSourceKind, ) -> RoleBuilderMutateResult { - self._validation_add(factor_source_kind, Override) + self.validation_for_addition_of_factor_source_of_kind_to_override_with_mode(factor_source_kind, SecurityShieldBuilderMode::Strict) + } + + pub(crate) fn validation_for_addition_of_factor_source_of_kind_to_override_with_mode( + &self, + factor_source_kind: FactorSourceKind, + mode: SecurityShieldBuilderMode, + ) -> RoleBuilderMutateResult { + self._validation_add(factor_source_kind, Override, mode) } /// If we would add a factor of kind `factor_source_kind` to the list of kind `factor_list_kind` @@ -315,12 +359,14 @@ impl RoleBuilder { &self, factor_source_kind: FactorSourceKind, factor_list_kind: FactorListKind, + mode: SecurityShieldBuilderMode, ) -> RoleBuilderMutateResult { match self.role() { RoleKind::Primary => { - self.validation_for_addition_of_factor_source_of_kind_to_list_for_primary( + self.validation_for_addition_of_factor_source_of_kind_to_list_for_primary_with_mode( factor_source_kind, factor_list_kind, + mode ) } RoleKind::Recovery | RoleKind::Confirmation => match factor_list_kind { @@ -332,6 +378,7 @@ impl RoleBuilder { Override => { self.validation_for_addition_of_factor_source_of_kind_to_override_for_non_primary_role( factor_source_kind, + mode ) } }, @@ -446,8 +493,11 @@ impl RoleBuilder { // Validate threshold factors for f in self.get_threshold_factors() { - let validation = - simulation._add_factor_source_to_list(*f, Threshold); + let validation = simulation._add_factor_source_to_list( + *f, + Threshold, + SecurityShieldBuilderMode::Strict, + ); match validation.as_ref() { Ok(()) | Err(NotYetValid(_)) => continue, Err(ForeverInvalid(_)) | Err(BasicViolation(_)) => { @@ -495,6 +545,7 @@ impl RoleBuilder { fn validation_for_addition_of_factor_source_of_kind_to_override_for_non_primary_role( &self, factor_source_kind: FactorSourceKind, + mode: SecurityShieldBuilderMode, ) -> RoleBuilderMutateResult { match self.role() { RoleKind::Primary => { @@ -502,21 +553,38 @@ impl RoleBuilder { } RoleKind::Confirmation => self .validation_for_addition_of_factor_source_of_kind_to_override_for_confirmation( - factor_source_kind, + factor_source_kind, mode ), RoleKind::Recovery => self .validation_for_addition_of_factor_source_of_kind_to_override_for_recovery( - factor_source_kind, + factor_source_kind, mode ), } } /// For each factor source in the given set, return a validation status /// for adding it to factor list of the given kind (`factor_list_kind`) + #[allow(dead_code)] + #[cfg(test)] pub(crate) fn validation_for_addition_of_factor_source_for_each( &self, factor_list_kind: FactorListKind, factor_sources: &IndexSet, + ) -> IndexSet { + self.validation_for_addition_of_factor_source_for_each_with_mode( + factor_list_kind, + factor_sources, + SecurityShieldBuilderMode::Strict, + ) + } + + /// For each factor source in the given set, return a validation status + /// for adding it to factor list of the given kind (`factor_list_kind`) + pub(crate) fn validation_for_addition_of_factor_source_for_each_with_mode( + &self, + factor_list_kind: FactorListKind, + factor_sources: &IndexSet, + mode: SecurityShieldBuilderMode, ) -> IndexSet { factor_sources .iter() @@ -525,6 +593,7 @@ impl RoleBuilder { .validation_for_addition_of_factor_source_to_list( factor_source_id, factor_list_kind, + mode, ); FactorSourceInRoleBuilderValidationStatus::new( self.role(), @@ -539,14 +608,40 @@ impl RoleBuilder { &self, factor_source_id: &FactorSourceID, factor_list_kind: FactorListKind, + mode: SecurityShieldBuilderMode, ) -> RoleBuilderMutateResult { - if self.contains_factor_source(factor_source_id) { - return RoleBuilderMutateResult::forever_invalid( - FactorSourceAlreadyPresent, - ); + let duplicates_err = RoleBuilderMutateResult::forever_invalid( + FactorSourceAlreadyPresent, + ); + + match mode { + SecurityShieldBuilderMode::Strict => { + if self.contains_factor_source(factor_source_id) { + return duplicates_err; + } + } + SecurityShieldBuilderMode::Lenient => { + match factor_list_kind { + Override => { + if self + .override_contains_factor_source(factor_source_id) + { + return duplicates_err; + } + // but if threshold contains it, we're good (since mode is lenient) + } + Threshold => { + if self + .threshold_contains_factor_source(factor_source_id) + { + return duplicates_err; + } + } // but if override contains it, we're good (since mode is lenient) + } + } } let factor_source_kind = factor_source_id.get_factor_source_kind(); - self._validation_add(factor_source_kind, factor_list_kind) + self._validation_add(factor_source_kind, factor_list_kind, mode) } fn contains_factor_source( @@ -607,17 +702,28 @@ impl RoleBuilder { Ok(()) } - #[cfg(not(tarpaulin_include))] // false negative + #[allow(dead_code)] fn validation_for_addition_of_factor_source_of_kind_to_list_for_primary( &self, factor_source_kind: FactorSourceKind, factor_list_kind: FactorListKind, + ) -> RoleBuilderMutateResult { + self.validation_for_addition_of_factor_source_of_kind_to_list_for_primary_with_mode(factor_source_kind, factor_list_kind, SecurityShieldBuilderMode::Strict) + } + + fn validation_for_addition_of_factor_source_of_kind_to_list_for_primary_with_mode( + &self, + factor_source_kind: FactorSourceKind, + factor_list_kind: FactorListKind, + mode: SecurityShieldBuilderMode, ) -> RoleBuilderMutateResult { match factor_source_kind { FactorSourceKind::Password => { - return self.validation_for_addition_of_password_to_primary( - factor_list_kind, - ) + return self + .validation_for_addition_of_password_to_primary_with_mode( + factor_list_kind, + mode, + ) } FactorSourceKind::SecurityQuestions => { return RoleBuilderMutateResult::forever_invalid( @@ -629,14 +735,18 @@ impl RoleBuilder { PrimaryCannotContainTrustedContact, ); } - FactorSourceKind::Device => { - if self.contains_factor_source_of_kind(FactorSourceKind::Device) - { - return RoleBuilderMutateResult::forever_invalid( - PrimaryCannotHaveMultipleDevices, - ); + FactorSourceKind::Device => match mode { + SecurityShieldBuilderMode::Strict => { + if self.contains_factor_source_of_kind( + FactorSourceKind::Device, + ) { + return RoleBuilderMutateResult::forever_invalid( + PrimaryCannotHaveMultipleDevices, + ); + } } - } + SecurityShieldBuilderMode::Lenient => {} + }, FactorSourceKind::LedgerHQHardwareWallet | FactorSourceKind::ArculusCard | FactorSourceKind::OffDeviceMnemonic => {} @@ -648,6 +758,7 @@ impl RoleBuilder { fn validation_for_addition_of_factor_source_of_kind_to_override_for_recovery( &self, factor_source_kind: FactorSourceKind, + _mode: SecurityShieldBuilderMode, ) -> RoleBuilderMutateResult { assert_eq!(self.role(), RoleKind::Recovery); match factor_source_kind { @@ -673,6 +784,7 @@ impl RoleBuilder { fn validation_for_addition_of_factor_source_of_kind_to_override_for_confirmation( &self, factor_source_kind: FactorSourceKind, + _mode: SecurityShieldBuilderMode, ) -> RoleBuilderMutateResult { assert_eq!(self.role(), RoleKind::Confirmation); match factor_source_kind { @@ -698,6 +810,16 @@ impl RoleBuilder { fn validation_for_addition_of_password_to_primary( &self, factor_list_kind: FactorListKind, + ) -> RoleBuilderMutateResult { + self.validation_for_addition_of_password_to_primary_with_mode( + factor_list_kind, + SecurityShieldBuilderMode::Strict, + ) + } + fn validation_for_addition_of_password_to_primary_with_mode( + &self, + factor_list_kind: FactorListKind, + mode: SecurityShieldBuilderMode, ) -> RoleBuilderMutateResult { assert_eq!(self.role(), RoleKind::Primary); @@ -707,6 +829,10 @@ impl RoleBuilder { ); } + if mode == SecurityShieldBuilderMode::Lenient { + return Ok(()); + } + let factor_source_kind = FactorSourceKind::Password; let is_alone = self @@ -758,13 +884,18 @@ pub(crate) fn test_duplicates_not_allowed( // Arrange let mut sut = sut; - sut._add_factor_source_to_list(factor_source_id, list) - .unwrap(); + sut._add_factor_source_to_list( + factor_source_id, + list, + SecurityShieldBuilderMode::Strict, + ) + .unwrap(); // Act let res = sut._add_factor_source_to_list( factor_source_id, // oh no, duplicate! list, + SecurityShieldBuilderMode::Strict, ); // Assert @@ -819,6 +950,7 @@ mod tests { let res = sut._add_factor_source_to_list( FactorSourceID::sample_ledger(), Threshold, + SecurityShieldBuilderMode::Strict, ); assert_eq!( res, @@ -832,6 +964,7 @@ mod tests { let res = sut._add_factor_source_to_list( FactorSourceID::sample_ledger(), Threshold, + SecurityShieldBuilderMode::Strict, ); assert_eq!( res, @@ -844,7 +977,11 @@ mod tests { #[test] fn recovery_validation_add_is_err_for_threshold() { let sut = RecoveryRoleBuilder::new(); - let res = sut._validation_add(FactorSourceKind::Device, Threshold); + let res = sut._validation_add( + FactorSourceKind::Device, + Threshold, + SecurityShieldBuilderMode::Strict, + ); assert_eq!( res, RoleBuilderMutateResult::basic_violation( @@ -858,7 +995,11 @@ mod tests { #[test] fn confirmation_validation_add_is_err_for_threshold() { let sut = ConfirmationRoleBuilder::new(); - let res = sut._validation_add(FactorSourceKind::Device, Threshold); + let res = sut._validation_add( + FactorSourceKind::Device, + Threshold, + SecurityShieldBuilderMode::Strict, + ); assert_eq!( res, RoleBuilderMutateResult::basic_violation( diff --git a/crates/sargon/src/profile/mfa/security_structures/security_shield_builder.rs b/crates/sargon/src/profile/mfa/security_structures/security_shield_builder.rs index 799ef8618..8f367a6ca 100644 --- a/crates/sargon/src/profile/mfa/security_structures/security_shield_builder.rs +++ b/crates/sargon/src/profile/mfa/security_structures/security_shield_builder.rs @@ -1,13 +1,47 @@ use crate::prelude::*; +/// The mode of the shield builder, either `Lenient` or `Strict`, this has +/// no effect on the validation or building of the shield, which is always +/// strict. It only affects the incremental changing of the state before +/// the builder tries to build the shield (or validate the builders state). +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum SecurityShieldBuilderMode { + /// The most strict mode of the shield builder, which forbids many + /// mutations, e.g. adding same factor two primary threshold and override, + /// or adding multiple device factor sources to the primary etc. + Strict, + + /// The lenient mode of the shield builder, which allows many otherwise + /// forbidden mutations. Host might wanna use this if they wanna defer + /// validation of the shield to the last point. + /// + /// This does NOT mean that the shield will be lenient, only that the + /// incremental changes to the builder are lenient. + Lenient, +} + #[derive(Debug)] pub struct SecurityShieldBuilder { + /// If the builder is acting in lenient or strict mode. This does not + /// have any affect on the validation or building of the shield, which is + /// always strict. It only affects the incremental changing of the state. + mode: SecurityShieldBuilderMode, + + /// The underlying matrix builder matrix_builder: RwLock, + + /// The "ROLA" factor, used to sign authentication requests, it is always single + /// factor - therefore not part of the matrix_builder. When built it must be set, + /// and adhere to validation of `Self::is_allowed_factor_source_kind_for_authentication_signing`. authentication_signing_factor: RwLock>, + + /// The name of the shield, defaults to some valid value name: RwLock, + // We eagerly set this, and we use it inside the `build` method, ensuring // that for the same *state* of `MatrixBuilder` we always have the same shield! shield_id: SecurityStructureID, + // We eagerly set this, and we use it inside the `build` method, ensuring // that for the same *state* of `MatrixBuilder` we always have the same shield! created_on: Timestamp, @@ -15,15 +49,16 @@ pub struct SecurityShieldBuilder { impl Default for SecurityShieldBuilder { fn default() -> Self { - Self::new() + Self::lenient() } } impl SecurityShieldBuilder { - pub fn new() -> Self { + pub fn new(mode: SecurityShieldBuilderMode) -> Self { let matrix_builder = MatrixBuilder::new(); let name = RwLock::new("My Shield".to_owned()); Self { + mode, matrix_builder: RwLock::new(matrix_builder), name, authentication_signing_factor: RwLock::new(None), @@ -152,8 +187,10 @@ impl SecurityShieldBuilder { factor_source_id: FactorSourceID, ) -> &Self { self.set(|builder| { - let res = builder - .add_factor_source_to_primary_threshold(factor_source_id); + let res = builder.add_factor_source_to_primary_threshold_with_mode( + factor_source_id, + self.mode, + ); debug!( "Add FactorSource to PrimaryRole (threshold) result: {:?}", res @@ -166,8 +203,10 @@ impl SecurityShieldBuilder { factor_source_id: FactorSourceID, ) -> &Self { self.set(|builder| { - let res = - builder.add_factor_source_to_primary_override(factor_source_id); + let res = builder.add_factor_source_to_primary_override_with_mode( + factor_source_id, + self.mode, + ); debug!( "Add FactorSource to PrimaryRole (override) result: {:?}", res @@ -233,8 +272,10 @@ impl SecurityShieldBuilder { factor_source_id: FactorSourceID, ) -> &Self { self.set(|builder| { - let res = builder - .add_factor_source_to_recovery_override(factor_source_id); + let res = builder.add_factor_source_to_recovery_override_with_mode( + factor_source_id, + self.mode, + ); debug!("Add FactorSource to RecoveryRole result: {:?}", res); }) } @@ -245,7 +286,10 @@ impl SecurityShieldBuilder { ) -> &Self { self.set(|builder| { let res = builder - .add_factor_source_to_confirmation_override(factor_source_id); + .add_factor_source_to_confirmation_override_with_mode( + factor_source_id, + self.mode, + ); debug!("Add FactorSource to ConfirmationRole result: {:?}", res); }) } @@ -423,7 +467,7 @@ impl SecurityShieldBuilder { self.validation_for_addition_of_factor_source_by_calling( factor_sources, |builder, input| { - builder.validation_for_addition_of_factor_source_to_primary_threshold_for_each(input) + builder.validation_for_addition_of_factor_source_to_primary_threshold_for_each_with_mode(input, self.mode) }, ) } @@ -435,7 +479,7 @@ impl SecurityShieldBuilder { self.validation_for_addition_of_factor_source_by_calling( factor_sources, |builder, input| { - builder.validation_for_addition_of_factor_source_to_primary_override_for_each(input) + builder.validation_for_addition_of_factor_source_to_primary_override_for_each_with_mode(input, self.mode) }, ) } @@ -448,7 +492,7 @@ impl SecurityShieldBuilder { factor_sources, |builder, input| { builder - .validation_for_addition_of_factor_source_to_recovery_override_for_each(input) + .validation_for_addition_of_factor_source_to_recovery_override_for_each_with_mode(input, self.mode) }, ) } @@ -460,8 +504,9 @@ impl SecurityShieldBuilder { self.validation_for_addition_of_factor_source_by_calling( factor_sources, |builder, input| { - builder.validation_for_addition_of_factor_source_to_confirmation_override_for_each( + builder.validation_for_addition_of_factor_source_to_confirmation_override_for_each_with_mode( input, + self.mode ) }, ) @@ -604,6 +649,16 @@ impl SecurityShieldBuilder { } } +impl SecurityShieldBuilder { + pub fn strict() -> Self { + Self::new(SecurityShieldBuilderMode::Strict) + } + + pub fn lenient() -> Self { + Self::new(SecurityShieldBuilderMode::Lenient) + } +} + #[cfg(test)] mod tests { @@ -612,9 +667,24 @@ mod tests { #[allow(clippy::upper_case_acronyms)] type SUT = SecurityShieldBuilder; + #[test] + fn default_is_lenient() { + assert_eq!(SUT::default().mode, SecurityShieldBuilderMode::Lenient); + } + + #[test] + fn mode_of_lenient() { + assert_eq!(SUT::lenient().mode, SecurityShieldBuilderMode::Lenient); + } + + #[test] + fn mode_of_strict() { + assert_eq!(SUT::strict().mode, SecurityShieldBuilderMode::Strict); + } + #[test] fn add_factor_to_primary_threshold_does_not_change_already_set_threshold() { - let sut = SUT::new(); + let sut = SUT::strict(); sut.set_threshold(42); sut.add_factor_source_to_primary_threshold( FactorSourceID::sample_device(), @@ -648,7 +718,7 @@ mod tests { #[test] fn test_invalid_rola_kind_does_not_change_rola() { - let sut = SUT::new(); + let sut = SUT::strict(); assert!(sut.get_authentication_signing_factor().is_none()); let valid = FactorSourceID::sample_device(); sut.set_authentication_signing_factor(valid); @@ -729,7 +799,7 @@ mod tests { can_be: impl Fn(&SUT, FactorSourceKind) -> bool, add: impl Fn(&SUT, FactorSourceID) -> &SUT, ) { - let sut_owned = SUT::new(); + let sut_owned = SUT::strict(); let sut = &sut_owned; assert!(can_be(sut, FactorSourceKind::Device)); @@ -785,7 +855,7 @@ mod tests { #[test] fn test_addition_of_factor_source_of_kind_to_recovery_is_fully_valid() { - let sut = SUT::new(); + let sut = SUT::strict(); let result = sut .addition_of_factor_source_of_kind_to_recovery_is_fully_valid( @@ -802,7 +872,7 @@ mod tests { #[test] fn test_addition_of_factor_source_of_kind_to_confirmation_is_fully_valid() { - let sut = SUT::new(); + let sut = SUT::strict(); let result = sut .addition_of_factor_source_of_kind_to_confirmation_is_fully_valid( @@ -820,7 +890,7 @@ mod tests { #[test] fn test_validation_for_addition_of_factor_source_to_primary_threshold_for_each( ) { - let sut = SUT::new(); + let sut = SUT::strict(); sut.add_factor_source_to_primary_threshold( FactorSourceID::sample_device(), @@ -853,7 +923,7 @@ mod tests { #[test] fn test_validation_for_addition_of_factor_source_to_recovery_override_for_each( ) { - let sut = SUT::new(); + let sut = SUT::strict(); let xs = sut.validation_for_addition_of_factor_source_to_recovery_override_for_each( vec![ @@ -890,7 +960,7 @@ mod tests { #[test] fn test_validation_for_addition_of_factor_source_to_confirmation_override_for_each( ) { - let sut = SUT::new(); + let sut = SUT::strict(); let xs = sut .validation_for_addition_of_factor_source_to_confirmation_override_for_each( vec![ @@ -918,7 +988,7 @@ mod tests { #[test] fn test_sorted_factor_sources_for_primary_threshold_selection() { - let sut = SUT::new(); + let sut = SUT::strict(); let factor_sources = FactorSource::sample_values_all(); let expected = vec![ FactorSource::sample_device_babylon(), @@ -988,7 +1058,7 @@ mod test_invalid { #[test] fn primary_role_must_have_at_least_one_factor() { - let sut = SUT::new(); + let sut = SUT::strict(); assert_eq!( sut.validate().unwrap(), SecurityShieldBuilderInvalidReason::PrimaryRoleMustHaveAtLeastOneFactor @@ -1001,7 +1071,7 @@ mod test_invalid { #[test] fn primary_role_with_threshold_cannot_be_zero_with_factors() { - let sut = SUT::new(); + let sut = SUT::strict(); sut.add_factor_source_to_primary_threshold( // bumped threshold FactorSourceID::sample_device(), @@ -1020,7 +1090,7 @@ mod test_invalid { #[test] fn recovery_role_must_have_at_least_one_factor() { - let sut = SUT::new(); + let sut = SUT::strict(); sut.add_factor_source_to_primary_override( FactorSourceID::sample_device(), ); @@ -1036,7 +1106,7 @@ mod test_invalid { #[test] fn confirmation_role_must_have_at_least_one_factor() { - let sut = SUT::new(); + let sut = SUT::strict(); sut.add_factor_source_to_primary_override( FactorSourceID::sample_device(), ); @@ -1055,7 +1125,7 @@ mod test_invalid { #[test] fn valid_is_none() { - let sut = SUT::new(); + let sut = SUT::strict(); sut.set_authentication_signing_factor(FactorSourceID::sample_device()); sut.add_factor_source_to_primary_override( FactorSourceID::sample_device(), @@ -1070,7 +1140,7 @@ mod test_invalid { } fn valid() -> SUT { - let sut = SUT::new(); + let sut = SUT::strict(); sut.add_factor_source_to_primary_override( FactorSourceID::sample_device(), ); @@ -1121,7 +1191,7 @@ mod test_invalid { #[test] fn recovery_and_confirmation_factors_overlap() { - let sut = SUT::new(); + let sut = SUT::strict(); sut.add_factor_source_to_primary_override( FactorSourceID::sample_device(), ); @@ -1140,7 +1210,7 @@ mod test_invalid { #[test] fn single_factor_used_in_primary_must_not_be_used_in_any_other_role_in_recovery( ) { - let sut = SUT::new(); + let sut = SUT::strict(); let same = FactorSourceID::sample_ledger(); sut.add_factor_source_to_primary_override(same); @@ -1159,7 +1229,7 @@ mod test_invalid { #[test] fn single_factor_used_in_primary_must_not_be_used_in_any_other_role_in_confirmation( ) { - let sut = SUT::new(); + let sut = SUT::strict(); let same = FactorSourceID::sample_ledger(); sut.add_factor_source_to_primary_override(same); @@ -1178,7 +1248,7 @@ mod test_invalid { #[test] fn primary_role_with_password_in_threshold_list_must_threshold_greater_than_one( ) { - let sut = SUT::new(); + let sut = SUT::strict(); sut.add_factor_source_to_recovery_override( FactorSourceID::sample_ledger(), @@ -1207,7 +1277,7 @@ mod test_invalid { #[test] fn primary_role_with_password_in_threshold_list_must_have_another_factor() { - let sut = SUT::new(); + let sut = SUT::strict(); sut.add_factor_source_to_recovery_override( FactorSourceID::sample_ledger(), @@ -1233,7 +1303,7 @@ mod test_invalid { #[test] fn two_different_password_only_not_valid_for_primary() { - let sut = SUT::new(); + let sut = SUT::strict(); sut.add_factor_source_to_recovery_override( FactorSourceID::sample_ledger(), @@ -1258,7 +1328,7 @@ mod test_invalid { #[test] fn primary_role_with_password_in_override_does_not_get_added() { - let sut = SUT::new(); + let sut = SUT::strict(); sut.add_factor_source_to_recovery_override( FactorSourceID::sample_ledger(), @@ -1321,3 +1391,111 @@ mod test_invalid { ); } } + +#[cfg(test)] +mod lenient { + use super::*; + #[allow(clippy::upper_case_acronyms)] + type SUT = SecurityShieldBuilder; + + #[test] + fn same_factor_in_primary_roles_both_lists() { + let test = |sut: &SUT, expected_threshold_count: usize| { + let x = FactorSourceID::sample_ledger(); + sut.add_factor_source_to_primary_override(x); + + sut.add_factor_source_to_primary_threshold(x); + assert_eq!( + sut.get_primary_threshold_factors().len(), + expected_threshold_count + ); + }; + let strict = SUT::strict(); + let lenient = SUT::lenient(); + + test(&strict, 0); + test(&lenient, 1); + } + + #[test] + fn primary_allows_two_device_factor_sources() { + let test = |sut: &SUT, expected_threshold_count: usize| { + sut.add_factor_source_to_primary_threshold( + FactorSourceID::sample_device(), + ); + sut.add_factor_source_to_primary_threshold( + FactorSourceID::sample_device_other(), + ); + + assert_eq!( + sut.get_primary_threshold_factors().len(), + expected_threshold_count + ); + }; + let strict = SUT::strict(); + let lenient = SUT::lenient(); + + test(&strict, 1); + test(&lenient, 2); + } + + #[test] + fn same_factor_allowed_in_primary_and_then_in_recovery_and_confirmation() { + let test = |sut: &SUT, expected_factors_in_non_primary: usize| { + let x = FactorSourceID::sample_device(); + + sut.add_factor_source_to_primary_override(x); + + sut.add_factor_source_to_recovery_override(x); + sut.add_factor_source_to_confirmation_override(x); + + assert_eq!( + sut.get_recovery_factors().len(), + expected_factors_in_non_primary + ); + + assert_eq!( + sut.get_confirmation_factors().len(), + expected_factors_in_non_primary + ); + }; + let strict = SUT::strict(); + let lenient = SUT::lenient(); + + test(&lenient, 1); + test(&strict, 1); // actually it was allowed for strict too... + } + + #[test] + fn same_factor_to_same_primary_list_not_allowed_even_for_lenient() { + let test = |factor_list: FactorListKind| { + let do_test = |sut: &SUT| { + let x = FactorSourceID::sample_ledger(); + match factor_list { + FactorListKind::Override => { + sut.add_factor_source_to_primary_override(x); + sut.add_factor_source_to_primary_override(x); + assert_eq!(sut.get_primary_override_factors().len(), 1); + } + FactorListKind::Threshold => { + sut.add_factor_source_to_primary_threshold(x); + sut.add_factor_source_to_primary_threshold(x); + assert_eq!( + sut.get_primary_threshold_factors().len(), + 1 + ); + } + } + }; + + let strict = SUT::strict(); + let lenient = SUT::lenient(); + + do_test(&strict); + do_test(&lenient); + }; + + test(FactorListKind::Threshold); + test(FactorListKind::Override); + } +}