Skip to content

Commit

Permalink
Fix a regression where the forced logout indicator was presented on t…
Browse files Browse the repository at this point in the history
…he hidden overlay window. (#2063)
  • Loading branch information
pixlwave authored Nov 10, 2023
1 parent 5adbb37 commit b5cc2cb
Show file tree
Hide file tree
Showing 20 changed files with 88 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -49,13 +49,6 @@ class AppLockFlowCoordinator: CoordinatorProtocol {
// Set the initial background state.
showPlaceholder()

appLockService.disabledPublisher
.sink {
// When the service is disabled via a force logout, we need to remove the activity indicator.
ServiceLocator.shared.userIndicatorController.retractAllIndicators()
}
.store(in: &cancellables)

notificationCenter.publisher(for: UIApplication.didEnterBackgroundNotification)
.sink { [weak self] _ in
self?.applicationDidEnterBackground()
Expand Down Expand Up @@ -131,7 +124,6 @@ class AppLockFlowCoordinator: CoordinatorProtocol {
case .appUnlocked:
actionsSubject.send(.unlockApp)
case .forceLogout:
ServiceLocator.shared.userIndicatorController.submitIndicator(UserIndicator(type: .modal, title: L10n.commonSigningOut, persistent: true))
actionsSubject.send(.forceLogout)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ struct AppLockScreenViewState: BindableState {

/// The number of times the user attempted to enter their PIN.
var numberOfPINAttempts = 0
/// An overlay indicator shown when the user is being logged out.
var forcedLogoutIndicator: UserIndicator?

var bindings: AppLockScreenViewStateBindings

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ class AppLockScreenViewModel: AppLockScreenViewModelType, AppLockScreenViewModel
state.bindings.alertInfo = .init(id: .confirmResetPIN,
title: L10n.screenAppLockSignoutAlertTitle,
message: L10n.screenAppLockSignoutAlertMessage,
primaryButton: .init(title: L10n.actionOk) { self.actionsSubject.send(.forceLogout) },
primaryButton: .init(title: L10n.actionOk) { self.forceLogout() },
secondaryButton: .init(title: L10n.actionCancel, role: .cancel, action: nil))
}

Expand All @@ -90,7 +90,12 @@ class AppLockScreenViewModel: AppLockScreenViewModelType, AppLockScreenViewModel
state.bindings.alertInfo = .init(id: .forcedLogout,
title: L10n.screenAppLockSignoutAlertTitle,
message: L10n.screenAppLockSignoutAlertMessage,
primaryButton: .init(title: L10n.actionOk) { self.actionsSubject.send(.forceLogout) })
primaryButton: .init(title: L10n.actionOk) { self.forceLogout() })
}
}

private func forceLogout() {
state.forcedLogoutIndicator = UserIndicator(type: .modal, title: L10n.commonSigningOut, persistent: true)
actionsSubject.send(.forceLogout)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,11 @@ struct AppLockScreen: View {
}
.background()
.environment(\.backgroundStyle, AnyShapeStyle(Color.compound.bgCanvasDefault))
.disabled(context.viewState.forcedLogoutIndicator != nil)
.overlay {
context.viewState.forcedLogoutIndicator.map(UserIndicatorModalView.init)
.animation(.elementDefault, value: context.viewState.forcedLogoutIndicator)
}
.alert(item: $context.alertInfo)
}

Expand Down
4 changes: 0 additions & 4 deletions ElementX/Sources/Services/AppLock/AppLockService.swift
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,6 @@ class AppLockService: AppLockServiceProtocol {

var numberOfPINAttempts: AnyPublisher<Int, Never> { appSettings.$appLockNumberOfPINAttempts }

private var disabledSubject: PassthroughSubject<Void, Never> = .init()
var disabledPublisher: AnyPublisher<Void, Never> { disabledSubject.eraseToAnyPublisher() }

init(keychainController: KeychainControllerProtocol, appSettings: AppSettings, context: LAContext = .init()) {
self.keychainController = keychainController
self.appSettings = appSettings
Expand Down Expand Up @@ -108,7 +105,6 @@ class AppLockService: AppLockServiceProtocol {
keychainController.removePINCode()
keychainController.removePINCodeBiometricState()
appSettings.appLockNumberOfPINAttempts = 0
disabledSubject.send()
}

func applicationDidEnterBackground() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,6 @@ protocol AppLockServiceProtocol: AnyObject {
/// to re-enter their PIN code to re-enable the feature (i.e. to accept a new face or fingerprint).
var biometricUnlockTrusted: Bool { get }

/// A publisher that advertises when the service has been disabled.
var disabledPublisher: AnyPublisher<Void, Never> { get }

/// Sets the user's PIN code used to unlock the app.
func setupPINCode(_ pinCode: String) -> Result<Void, AppLockServiceError>
/// Validates the supplied PIN code is long enough, only contains digits and isn't a weak choice.
Expand Down
37 changes: 37 additions & 0 deletions UITests/Sources/AppLockUITests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@ class AppLockUITests: XCTestCase {
enum Step {
static let placeholder = 0
static let lockScreen = 1
static let failedUnlock = 2
static let logoutAlert = 3
static let forcedLogout = 4
static let unlocked = 99
}

Expand Down Expand Up @@ -76,6 +79,33 @@ class AppLockUITests: XCTestCase {
try await app.assertScreenshot(.appLockFlow, step: Step.unlocked)
}

func testWrongPIN() async throws {
// Given an app with screen lock enabled that is ready to unlock.
let client = try UITestsSignalling.Client(mode: .tests)
app = Application.launch(.appLockFlow)
await client.waitForApp()

try await app.assertScreenshot(.appLockFlow, step: Step.unlocked)
try client.send(.notification(name: UIApplication.didEnterBackgroundNotification))
try client.send(.notification(name: UIApplication.willEnterForegroundNotification))
try await app.assertScreenshot(.appLockFlow, step: Step.lockScreen)

// When entering an incorrect PIN
enterWrongPIN()

// Then the app should remain locked with a warning.
try await app.assertScreenshot(.appLockFlow, step: Step.failedUnlock)

// When entering it incorrectly twice more.
enterWrongPIN()
enterWrongPIN()

// Then then the app should sign the user out.
try await app.assertScreenshot(.appLockFlow, step: Step.logoutAlert)
app.alerts.element.buttons[A11yIdentifiers.alertInfo.primaryButton].tap()
try await app.assertScreenshot(.appLockFlow, step: Step.forcedLogout)
}

// MARK: - Helpers

func enterPIN() {
Expand All @@ -84,4 +114,11 @@ class AppLockUITests: XCTestCase {
app.buttons[A11yIdentifiers.appLockScreen.numpad(2)].tap()
app.buttons[A11yIdentifiers.appLockScreen.numpad(3)].tap()
}

func enterWrongPIN() {
app.buttons[A11yIdentifiers.appLockScreen.numpad(0)].tap()
app.buttons[A11yIdentifiers.appLockScreen.numpad(0)].tap()
app.buttons[A11yIdentifiers.appLockScreen.numpad(0)].tap()
app.buttons[A11yIdentifiers.appLockScreen.numpad(0)].tap()
}
}
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
1 change: 1 addition & 0 deletions changelog.d/pr-2063.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix a regression where the forced logout indicator was presented on the hidden overlay window.

0 comments on commit b5cc2cb

Please sign in to comment.