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

[PM-15359] Consider Credential Exchange import policies #1262

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
typealias Services = HasConfigService
& HasErrorReporter
& HasImportCiphersRepository
& HasPolicyService
& HasStateService

// MARK: Private Properties
Expand Down Expand Up @@ -68,6 +69,10 @@
state.status = .failure(message: Localizations.importingFromAnotherProviderIsNotAvailableForThisDevice)
return
}
if await services.policyService.policyAppliesToUser(.personalOwnership) {
state.isFeatureUnavailable = true
state.status = .failure(message: Localizations.personalOwnershipPolicyInEffect)
}

Check warning on line 75 in BitwardenShared/UI/Tools/ImportCXP/ImportCXP/ImportCXPProcessor.swift

View check run for this annotation

Codecov / codecov/patch

BitwardenShared/UI/Tools/ImportCXP/ImportCXP/ImportCXPProcessor.swift#L72-L75

Added lines #L72 - L75 were not covered by tests
}

/// Starts the import process.
Expand Down Expand Up @@ -110,7 +115,7 @@

/// Shows the alert confirming the user wants to import logins later.
private func cancelWithConfirmation() {
guard !state.isFeatureUnvailable else {
guard !state.isFeatureUnavailable else {
coordinator.navigate(to: .dismiss)
return
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ class ImportCXPProcessorTests: BitwardenTestCase {
var coordinator: MockCoordinator<ImportCXPRoute, Void>!
var errorReporter: MockErrorReporter!
var importCiphersRepository: MockImportCiphersRepository!
var policyService: MockPolicyService!
var state: ImportCXPState!
var stateService: MockStateService!
var subject: ImportCXPProcessor!
Expand All @@ -24,6 +25,7 @@ class ImportCXPProcessorTests: BitwardenTestCase {
coordinator = MockCoordinator<ImportCXPRoute, Void>()
errorReporter = MockErrorReporter()
importCiphersRepository = MockImportCiphersRepository()
policyService = MockPolicyService()
state = ImportCXPState()
stateService = MockStateService()
subject = ImportCXPProcessor(
Expand All @@ -32,6 +34,7 @@ class ImportCXPProcessorTests: BitwardenTestCase {
configService: configService,
errorReporter: errorReporter,
importCiphersRepository: importCiphersRepository,
policyService: policyService,
stateService: stateService
),
state: state
Expand All @@ -45,6 +48,7 @@ class ImportCXPProcessorTests: BitwardenTestCase {
coordinator = nil
errorReporter = nil
importCiphersRepository = nil
policyService = nil
state = nil
stateService = nil
subject = nil
Expand All @@ -65,7 +69,30 @@ class ImportCXPProcessorTests: BitwardenTestCase {
}

/// `perform(_:)` with `.appeared` sets the status as `.failure` with a message
/// when the feature flag `.cxpImportMobile` is not enabled.
/// when the feature flag `.cxpImportMobile` is enabled. but `.personalOwnership`
/// policy applies to user.
@MainActor
func test_perform_appearedPersonalOwnership() async throws {
guard #available(iOS 18.2, *) else {
throw XCTSkip("CXP Import feature is not available on this device")
}

configService.featureFlagsBool[.cxpImportMobile] = true
policyService.policyAppliesToUserResult[.personalOwnership] = true

await subject.perform(.appeared)

guard case let .failure(message) = subject.state.status else {
XCTFail("Status should be failure")
return
}
XCTAssertEqual(message, Localizations.personalOwnershipPolicyInEffect)
XCTAssertTrue(subject.state.isFeatureUnavailable)
}

/// `perform(_:)` with `.appeared` doesn't set the status as `.failure`
/// when the feature flag `.cxpImportMobile` is enabled and `.personalOwnership`
/// policy doesn't apply to user.
@MainActor
func test_perform_appearedFeatureFlagEnabled() async throws {
guard #available(iOS 18.2, *) else {
Expand All @@ -82,7 +109,7 @@ class ImportCXPProcessorTests: BitwardenTestCase {
/// `perform(_:)` with `.cancel` with feature available shows confirmation and navigates to dismiss.
@MainActor
func test_perform_cancel() async throws {
subject.state.isFeatureUnvailable = false
subject.state.isFeatureUnavailable = false
let task = Task {
await subject.perform(.cancel)
}
Expand All @@ -108,7 +135,7 @@ class ImportCXPProcessorTests: BitwardenTestCase {
/// doesn't navigate to dismiss if the user cancels the confirmation dialog.
@MainActor
func test_perform_cancelNoConfirmation() async throws {
subject.state.isFeatureUnvailable = false
subject.state.isFeatureUnavailable = false
let task = Task {
await subject.perform(.cancel)
}
Expand All @@ -128,7 +155,7 @@ class ImportCXPProcessorTests: BitwardenTestCase {
/// `perform(_:)` with `.cancel` with feature unavailable navigates to dismiss.
@MainActor
func test_perform_cancelFeatureUnavailable() async throws {
subject.state.isFeatureUnvailable = true
subject.state.isFeatureUnavailable = true
let task = Task {
await subject.perform(.cancel)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ struct ImportCXPState: Equatable, Sendable {
var credentialImportToken: UUID?

/// Whether the CXP import feature is available.
var isFeatureUnvailable: Bool = false
var isFeatureUnavailable: Bool = false

/// The title of the main button.
var mainButtonTitle: String {
Expand Down Expand Up @@ -83,7 +83,7 @@ struct ImportCXPState: Equatable, Sendable {
case .success:
Localizations.importSuccessful
case .failure:
isFeatureUnvailable ? Localizations.importNotAvailable : Localizations.importFailed
isFeatureUnavailable ? Localizations.importNotAvailable : Localizations.importFailed
}
}

Expand All @@ -99,7 +99,7 @@ struct ImportCXPState: Equatable, Sendable {

/// Whether to show the main button.
var showMainButton: Bool {
status != .importing || isFeatureUnvailable
status != .importing && !isFeatureUnavailable
}

/// The current status of the import process.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ class ImportCXPStateTests: BitwardenTestCase {
subject.status = .failure(message: "Something went wrong")
XCTAssertEqual(subject.title, Localizations.importFailed)

subject.isFeatureUnvailable = true
subject.isFeatureUnavailable = true
XCTAssertEqual(subject.title, Localizations.importNotAvailable)
}

Expand Down Expand Up @@ -117,4 +117,20 @@ class ImportCXPStateTests: BitwardenTestCase {
subject.status = .failure(message: "Something went wrong")
XCTAssertTrue(subject.showMainButton)
}

/// `getter:showMainButton` returns `false` when feature unavailable.
func test_showMainButton_featureUnavailable() {
subject.isFeatureUnavailable = true
subject.status = .start
XCTAssertFalse(subject.showMainButton)

subject.status = .importing
XCTAssertFalse(subject.showMainButton)

subject.status = .success(totalImportedCredentials: 1, importedResults: [])
XCTAssertFalse(subject.showMainButton)

subject.status = .failure(message: "Something went wrong")
XCTAssertFalse(subject.showMainButton)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ class ImportCXPCoordinator: Coordinator, HasStackNavigator {
typealias Services = HasConfigService
& HasErrorReporter
& HasImportCiphersRepository
& HasPolicyService
& HasStateService

// MARK: Private Properties
Expand Down
Loading