From 29fa0eb10ac7ab5d7eddd5ff458f6c1dcaef5c05 Mon Sep 17 00:00:00 2001 From: ismailgulek Date: Thu, 9 Jun 2022 17:23:25 +0300 Subject: [PATCH 01/12] Add logoutDevices parameter to MXKAccount --- Riot/Modules/MatrixKit/Models/Account/MXKAccount.h | 8 ++++++-- Riot/Modules/MatrixKit/Models/Account/MXKAccount.m | 3 ++- Riot/Modules/Settings/SettingsViewController.m | 9 ++++++++- 3 files changed, 16 insertions(+), 4 deletions(-) diff --git a/Riot/Modules/MatrixKit/Models/Account/MXKAccount.h b/Riot/Modules/MatrixKit/Models/Account/MXKAccount.h index 785972fa5a..665f45ace8 100644 --- a/Riot/Modules/MatrixKit/Models/Account/MXKAccount.h +++ b/Riot/Modules/MatrixKit/Models/Account/MXKAccount.h @@ -287,11 +287,15 @@ typedef BOOL (^MXKAccountOnCertificateChange)(MXKAccount *mxAccount, NSData *cer @param oldPassword the old password. @param newPassword the new password. - + @param logoutDevices flag to logout from all devices. @param success A block object called when the operation succeeds. @param failure A block object called when the operation fails. */ -- (void)changePassword:(NSString*)oldPassword with:(NSString*)newPassword success:(void (^)(void))success failure:(void (^)(NSError *error))failure; +- (void)changePassword:(NSString*)oldPassword + with:(NSString*)newPassword + logoutDevices:(BOOL)logoutDevices + success:(void (^)(void))success + failure:(void (^)(NSError *error))failure; /** Load the 3PIDs linked to this account. diff --git a/Riot/Modules/MatrixKit/Models/Account/MXKAccount.m b/Riot/Modules/MatrixKit/Models/Account/MXKAccount.m index 9c35500d36..f9531c343a 100644 --- a/Riot/Modules/MatrixKit/Models/Account/MXKAccount.m +++ b/Riot/Modules/MatrixKit/Models/Account/MXKAccount.m @@ -582,12 +582,13 @@ - (void)setUserAvatarUrl:(NSString*)avatarUrl success:(void (^)(void))success fa } } -- (void)changePassword:(NSString*)oldPassword with:(NSString*)newPassword success:(void (^)(void))success failure:(void (^)(NSError *error))failure +- (void)changePassword:(NSString*)oldPassword with:(NSString*)newPassword logoutDevices:(BOOL)logoutDevices success:(void (^)(void))success failure:(void (^)(NSError *error))failure { if (mxSession) { [mxRestClient changePassword:oldPassword with:newPassword + logoutDevices:logoutDevices success:^{ if (success) { diff --git a/Riot/Modules/Settings/SettingsViewController.m b/Riot/Modules/Settings/SettingsViewController.m index a55366233d..d237154ba8 100644 --- a/Riot/Modules/Settings/SettingsViewController.m +++ b/Riot/Modules/Settings/SettingsViewController.m @@ -4130,7 +4130,7 @@ - (void)displayPasswordAlert MXKAccount* account = [MXKAccountManager sharedManager].activeAccounts.firstObject; - [account changePassword:self->currentPasswordTextField.text with:self->newPasswordTextField1.text success:^{ + [account changePassword:self->currentPasswordTextField.text with:self->newPasswordTextField1.text logoutDevices:YES success:^{ if (weakSelf) { @@ -4287,6 +4287,13 @@ - (void)displayPasswordAlert [resetPwdAlertController addAction:cancel]; [resetPwdAlertController addAction:savePasswordAction]; + + UIButton *logoutDevicesButton = [[UIButton alloc] initWithFrame:CGRectMake(0, 0, 100, 50)]; + [logoutDevicesButton setImage:AssetImages.selectionTick.image forState:UIControlStateSelected]; + [logoutDevicesButton setImage:AssetImages.selectionUntick.image forState:UIControlStateNormal]; + [logoutDevicesButton setTitle:VectorL10n.settingsChangePassword forState:UIControlStateNormal]; + [resetPwdAlertController.view addSubview:logoutDevicesButton]; + [self presentViewController:resetPwdAlertController animated:YES completion:nil]; } From 54b733366575bba9c16aa426d97d23eea7e24f44 Mon Sep 17 00:00:00 2001 From: ismailgulek Date: Thu, 9 Jun 2022 20:25:03 +0300 Subject: [PATCH 02/12] Make change password api async --- Riot/Categories/MXRestClient+Async.swift | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/Riot/Categories/MXRestClient+Async.swift b/Riot/Categories/MXRestClient+Async.swift index a0f996a71b..6d13c798fa 100644 --- a/Riot/Categories/MXRestClient+Async.swift +++ b/Riot/Categories/MXRestClient+Async.swift @@ -146,6 +146,15 @@ extension MXRestClient { resetPassword(parameters: parameters, completion: completion) } } + + // MARK: - Change Password + + /// An async version of `changePassword(from:to:logoutDevices:completion:)`. + func changePassword(from oldPassword: String, to newPassword: String, logoutDevices: Bool) async throws { + try await getResponse { completion in + changePassword(from: oldPassword, to: newPassword, logoutDevices: logoutDevices, completion: completion) + } + } // MARK: - Private From 1778608d95854768dd5f56a6696e0b66330eb001 Mon Sep 17 00:00:00 2001 From: ismailgulek Date: Thu, 9 Jun 2022 20:25:36 +0300 Subject: [PATCH 03/12] Create change password screen --- .../Modules/Common/Mock/MockAppScreens.swift | 1 + .../ChangePassword/ChangePasswordModels.swift | 65 ++++++++ .../ChangePasswordViewModel.swift | 69 +++++++++ .../ChangePasswordViewModelProtocol.swift | 26 ++++ .../ChangePasswordCoordinator.swift | 134 +++++++++++++++++ .../MockChangePasswordScreenState.swift | 64 ++++++++ .../Test/UI/ChangePasswordUITests.swift | 140 ++++++++++++++++++ .../Unit/ChangePasswordViewModelTests.swift | 50 +++++++ .../View/ChangePasswordScreen.swift | 136 +++++++++++++++++ 9 files changed, 685 insertions(+) create mode 100644 RiotSwiftUI/Modules/Settings/ChangePassword/ChangePasswordModels.swift create mode 100644 RiotSwiftUI/Modules/Settings/ChangePassword/ChangePasswordViewModel.swift create mode 100644 RiotSwiftUI/Modules/Settings/ChangePassword/ChangePasswordViewModelProtocol.swift create mode 100644 RiotSwiftUI/Modules/Settings/ChangePassword/Coordinator/ChangePasswordCoordinator.swift create mode 100644 RiotSwiftUI/Modules/Settings/ChangePassword/MockChangePasswordScreenState.swift create mode 100644 RiotSwiftUI/Modules/Settings/ChangePassword/Test/UI/ChangePasswordUITests.swift create mode 100644 RiotSwiftUI/Modules/Settings/ChangePassword/Test/Unit/ChangePasswordViewModelTests.swift create mode 100644 RiotSwiftUI/Modules/Settings/ChangePassword/View/ChangePasswordScreen.swift diff --git a/RiotSwiftUI/Modules/Common/Mock/MockAppScreens.swift b/RiotSwiftUI/Modules/Common/Mock/MockAppScreens.swift index 3a77f41b82..21762cb90a 100644 --- a/RiotSwiftUI/Modules/Common/Mock/MockAppScreens.swift +++ b/RiotSwiftUI/Modules/Common/Mock/MockAppScreens.swift @@ -51,6 +51,7 @@ enum MockAppScreens { MockSpaceCreationSettingsScreenState.self, MockSpaceCreationPostProcessScreenState.self, MockTimelinePollScreenState.self, + MockChangePasswordScreenState.self, MockTemplateSimpleScreenScreenState.self, MockTemplateUserProfileScreenState.self, MockTemplateRoomListScreenState.self, diff --git a/RiotSwiftUI/Modules/Settings/ChangePassword/ChangePasswordModels.swift b/RiotSwiftUI/Modules/Settings/ChangePassword/ChangePasswordModels.swift new file mode 100644 index 0000000000..513c00a080 --- /dev/null +++ b/RiotSwiftUI/Modules/Settings/ChangePassword/ChangePasswordModels.swift @@ -0,0 +1,65 @@ +// +// Copyright 2021 New Vector Ltd +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +import SwiftUI + +// MARK: View model + +enum ChangePasswordViewModelResult { + /// Submit with old and new passwords and sign out of all devices option + case submit(String, String, Bool) +} + +// MARK: View + +struct ChangePasswordViewState: BindableState { + /// View state that can be bound to from SwiftUI. + var bindings: ChangePasswordBindings + + /// Whether the user can submit the form: old password should be entered, and new passwords should match + var canSubmit: Bool { + !bindings.oldPassword.isEmpty + && !bindings.newPassword1.isEmpty + && bindings.newPassword1 == bindings.newPassword2 + } +} + +struct ChangePasswordBindings { + /// The password input by the user. + var oldPassword: String + /// The new password input by the user. + var newPassword1: String + /// The new password confirmation input by the user. + var newPassword2: String + /// The signout all devices checkbox status + var signoutAllDevices: Bool + /// Information describing the currently displayed alert. + var alertInfo: AlertInfo? +} + +enum ChangePasswordViewAction { + /// Send an email to the entered address. + case submit + /// Toggle sign out of all devices + case toggleSignoutAllDevices +} + +enum ChangePasswordErrorType: Hashable { + /// An error response from the homeserver. + case mxError(String) + /// An unknown error occurred. + case unknown +} diff --git a/RiotSwiftUI/Modules/Settings/ChangePassword/ChangePasswordViewModel.swift b/RiotSwiftUI/Modules/Settings/ChangePassword/ChangePasswordViewModel.swift new file mode 100644 index 0000000000..f5e1065c64 --- /dev/null +++ b/RiotSwiftUI/Modules/Settings/ChangePassword/ChangePasswordViewModel.swift @@ -0,0 +1,69 @@ +// +// Copyright 2021 New Vector Ltd +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +import SwiftUI + +typealias ChangePasswordViewModelType = StateStoreViewModel +class ChangePasswordViewModel: ChangePasswordViewModelType, ChangePasswordViewModelProtocol { + + // MARK: - Properties + + // MARK: Private + + // MARK: Public + + var callback: (@MainActor (ChangePasswordViewModelResult) -> Void)? + + // MARK: - Setup + + init(oldPassword: String = "", + newPassword1: String = "", + newPassword2: String = "", + signoutAllDevices: Bool = true) { + let bindings = ChangePasswordBindings(oldPassword: oldPassword, + newPassword1: newPassword1, + newPassword2: newPassword2, + signoutAllDevices: signoutAllDevices) + let viewState = ChangePasswordViewState(bindings: bindings) + super.init(initialViewState: viewState) + } + + // MARK: - Public + + override func process(viewAction: ChangePasswordViewAction) { + switch viewAction { + case .submit: + Task { await callback?(.submit(state.bindings.oldPassword, + state.bindings.newPassword1, + state.bindings.signoutAllDevices)) } + case .toggleSignoutAllDevices: + state.bindings.signoutAllDevices.toggle() + } + } + + @MainActor func displayError(_ type: ChangePasswordErrorType) { + switch type { + case .mxError(let message): + state.bindings.alertInfo = AlertInfo(id: type, + title: VectorL10n.error, + message: message) + case .unknown: + state.bindings.alertInfo = AlertInfo(id: type) + } + } +} diff --git a/RiotSwiftUI/Modules/Settings/ChangePassword/ChangePasswordViewModelProtocol.swift b/RiotSwiftUI/Modules/Settings/ChangePassword/ChangePasswordViewModelProtocol.swift new file mode 100644 index 0000000000..d6050d373e --- /dev/null +++ b/RiotSwiftUI/Modules/Settings/ChangePassword/ChangePasswordViewModelProtocol.swift @@ -0,0 +1,26 @@ +// +// Copyright 2021 New Vector Ltd +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +import Foundation + +protocol ChangePasswordViewModelProtocol { + + var callback: (@MainActor (ChangePasswordViewModelResult) -> Void)? { get set } + var context: ChangePasswordViewModelType.Context { get } + + /// Display an error to the user. + @MainActor func displayError(_ type: ChangePasswordErrorType) +} diff --git a/RiotSwiftUI/Modules/Settings/ChangePassword/Coordinator/ChangePasswordCoordinator.swift b/RiotSwiftUI/Modules/Settings/ChangePassword/Coordinator/ChangePasswordCoordinator.swift new file mode 100644 index 0000000000..d60d16a937 --- /dev/null +++ b/RiotSwiftUI/Modules/Settings/ChangePassword/Coordinator/ChangePasswordCoordinator.swift @@ -0,0 +1,134 @@ +// +// Copyright 2021 New Vector Ltd +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +import SwiftUI +import CommonKit + +struct ChangePasswordCoordinatorParameters { + let restClient: MXRestClient +} + +final class ChangePasswordCoordinator: Coordinator, Presentable { + + // MARK: - Properties + + // MARK: Private + + private let parameters: ChangePasswordCoordinatorParameters + private let changePasswordHostingController: VectorHostingController + private var changePasswordViewModel: ChangePasswordViewModelProtocol + + private var indicatorPresenter: UserIndicatorTypePresenterProtocol + private var loadingIndicator: UserIndicator? + + private var currentTask: Task? { + willSet { + currentTask?.cancel() + } + } + + // MARK: Public + + // Must be used only internally + var childCoordinators: [Coordinator] = [] + var callback: (@MainActor () -> Void)? + + // MARK: - Setup + + @MainActor init(parameters: ChangePasswordCoordinatorParameters) { + self.parameters = parameters + + let viewModel = ChangePasswordViewModel() + let view = ChangePasswordScreen(viewModel: viewModel.context) + changePasswordViewModel = viewModel + changePasswordHostingController = VectorHostingController(rootView: view) + changePasswordHostingController.vc_removeBackTitle() + changePasswordHostingController.enableNavigationBarScrollEdgeAppearance = true + + indicatorPresenter = UserIndicatorTypePresenter(presentingViewController: changePasswordHostingController) + } + + // MARK: - Public + + func start() { + MXLog.debug("[ChangePasswordCoordinator] did start.") + Task { await setupViewModel() } + } + + func toPresentable() -> UIViewController { + return self.changePasswordHostingController + } + + // MARK: - Private + + /// Set up the view model. This method is extracted from `start()` so it can run on the `MainActor`. + @MainActor private func setupViewModel() { + changePasswordViewModel.callback = { [weak self] result in + guard let self = self else { return } + MXLog.debug("[ChangePasswordCoordinator] ChangePasswordViewModel did complete with result: \(result).") + + switch result { + case .submit(let oldPassword, let newPassword, let signoutAllDevices): + self.changePassword(from: oldPassword, to: newPassword, signoutAllDevices: signoutAllDevices) + } + } + } + + /// Show an activity indicator whilst loading. + @MainActor private func startLoading() { + loadingIndicator = indicatorPresenter.present(.loading(label: VectorL10n.loading, isInteractionBlocking: true)) + } + + /// Hide the currently displayed activity indicator. + @MainActor private func stopLoading() { + loadingIndicator = nil + } + + /// Submits a reset password request with signing out of all devices option + @MainActor private func changePassword(from oldPassword: String, to newPassword: String, signoutAllDevices: Bool) { + startLoading() + + currentTask = Task { [weak self] in + do { + try await parameters.restClient.changePassword(from: oldPassword, to: newPassword, logoutDevices: signoutAllDevices) + + // Shouldn't be reachable but just in case, continue the flow. + + guard !Task.isCancelled else { return } + + self?.stopLoading() + self?.callback?() + } catch is CancellationError { + return + } catch { + self?.stopLoading() + self?.handleError(error) + } + } + } + + /// Processes an error to either update the flow or display it to the user. + @MainActor private func handleError(_ error: Error) { + if let mxError = MXError(nsError: error as NSError) { + changePasswordViewModel.displayError(.mxError(mxError.error)) + return + } + + // TODO: Handle another other error types as needed. + + changePasswordViewModel.displayError(.unknown) + } +} diff --git a/RiotSwiftUI/Modules/Settings/ChangePassword/MockChangePasswordScreenState.swift b/RiotSwiftUI/Modules/Settings/ChangePassword/MockChangePasswordScreenState.swift new file mode 100644 index 0000000000..6656e91585 --- /dev/null +++ b/RiotSwiftUI/Modules/Settings/ChangePassword/MockChangePasswordScreenState.swift @@ -0,0 +1,64 @@ +// +// Copyright 2021 New Vector Ltd +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +import Foundation +import SwiftUI + +/// Using an enum for the screen allows you define the different state cases with +/// the relevant associated data for each case. +@available(iOS 14.0, *) +enum MockChangePasswordScreenState: MockScreenState, CaseIterable { + // A case for each state you want to represent + // with specific, minimal associated data that will allow you + // mock that screen. + case allEmpty + case cannotSubmit + case canSubmit + case canSubmitAndSignoutAllDevicesChecked + + /// The associated screen + var screenType: Any.Type { + ChangePasswordScreen.self + } + + /// Generate the view struct for the screen state. + var screenView: ([Any], AnyView) { + let viewModel: ChangePasswordViewModel + switch self { + case .allEmpty: + viewModel = ChangePasswordViewModel() + case .cannotSubmit: + viewModel = ChangePasswordViewModel(oldPassword: "12345678", + newPassword1: "87654321", + newPassword2: "97654321") + case .canSubmit: + viewModel = ChangePasswordViewModel(oldPassword: "12345678", + newPassword1: "87654321", + newPassword2: "87654321") + case .canSubmitAndSignoutAllDevicesChecked: + viewModel = ChangePasswordViewModel(oldPassword: "12345678", + newPassword1: "87654321", + newPassword2: "87654321", + signoutAllDevices: true) + } + + // can simulate service and viewModel actions here if needs be. + + return ( + [viewModel], AnyView(ChangePasswordScreen(viewModel: viewModel.context)) + ) + } +} diff --git a/RiotSwiftUI/Modules/Settings/ChangePassword/Test/UI/ChangePasswordUITests.swift b/RiotSwiftUI/Modules/Settings/ChangePassword/Test/UI/ChangePasswordUITests.swift new file mode 100644 index 0000000000..fde678d983 --- /dev/null +++ b/RiotSwiftUI/Modules/Settings/ChangePassword/Test/UI/ChangePasswordUITests.swift @@ -0,0 +1,140 @@ +// +// Copyright 2021 New Vector Ltd +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +import XCTest +import RiotSwiftUI + +class ChangePasswordUITests: MockScreenTest { + + override class var screenType: MockScreenState.Type { + return MockChangePasswordScreenState.self + } + + override class func createTest() -> MockScreenTest { + return ChangePasswordUITests(selector: #selector(verifyChangePasswordScreen)) + } + + func verifyChangePasswordScreen() throws { + guard let screenState = screenState as? MockChangePasswordScreenState else { fatalError("no screen") } + switch screenState { + case .allEmpty: + verifyAllEmpty() + case .cannotSubmit: + verifyCannotSubmit() + case .canSubmit: + verifyCanSubmit() + case .canSubmitAndSignoutAllDevicesChecked: + verifyCanSubmitAndSignoutAllDevicesChecked() + } + } + + func verifyAllEmpty() { + XCTAssertTrue(app.staticTexts["titleLabel"].exists, "The title should be shown.") + + let oldPasswordTextField = app.secureTextFields["oldPasswordTextField"] + XCTAssertTrue(oldPasswordTextField.exists, "The text field should be shown.") + XCTAssertEqual(oldPasswordTextField.label, "old password", "The text field should be showing the placeholder before text is input.") + + let newPasswordTextField1 = app.secureTextFields["newPasswordTextField1"] + XCTAssertTrue(newPasswordTextField1.exists, "The text field should be shown.") + XCTAssertEqual(newPasswordTextField1.label, "new password", "The text field should be showing the placeholder before text is input.") + + let newPasswordTextField2 = app.secureTextFields["newPasswordTextField2"] + XCTAssertTrue(newPasswordTextField2.exists, "The text field should be shown.") + XCTAssertEqual(newPasswordTextField2.label, "confirm password", "The text field should be showing the placeholder before text is input.") + + let submitButton = app.buttons["submitButton"] + XCTAssertTrue(submitButton.exists, "The submit button should be shown.") + XCTAssertFalse(submitButton.isEnabled, "The submit button should be disabled when not able to submit.") + + let signoutAllDevicesToggle = app.switches["signoutAllDevicesToggle"] + XCTAssertTrue(signoutAllDevicesToggle.exists, "Sign out all devices toggle should exist") + XCTAssertTrue(signoutAllDevicesToggle.isOn, "Sign out all devices should be checked") + } + + func verifyCannotSubmit() { + XCTAssertTrue(app.staticTexts["titleLabel"].exists, "The title should be shown.") + + let oldPasswordTextField = app.secureTextFields["oldPasswordTextField"] + XCTAssertTrue(oldPasswordTextField.exists, "The text field should be shown.") + XCTAssertEqual(oldPasswordTextField.value as? String, "••••••••", "The text field should be showing the placeholder before text is input.") + + let newPasswordTextField1 = app.secureTextFields["newPasswordTextField1"] + XCTAssertTrue(newPasswordTextField1.exists, "The text field should be shown.") + XCTAssertEqual(newPasswordTextField1.value as? String, "••••••••", "The text field should be showing the placeholder before text is input.") + + let newPasswordTextField2 = app.secureTextFields["newPasswordTextField2"] + XCTAssertTrue(newPasswordTextField2.exists, "The text field should be shown.") + XCTAssertEqual(newPasswordTextField2.value as? String, "••••••••", "The text field should be showing the placeholder before text is input.") + + let submitButton = app.buttons["submitButton"] + XCTAssertTrue(submitButton.exists, "The submit button should be shown.") + XCTAssertFalse(submitButton.isEnabled, "The submit button should be disabled when not able to submit.") + + let signoutAllDevicesToggle = app.switches["signoutAllDevicesToggle"] + XCTAssertTrue(signoutAllDevicesToggle.exists, "Sign out all devices toggle should exist") + XCTAssertTrue(signoutAllDevicesToggle.isOn, "Sign out all devices should be checked") + } + + func verifyCanSubmit() { + XCTAssertTrue(app.staticTexts["titleLabel"].exists, "The title should be shown.") + + let oldPasswordTextField = app.secureTextFields["oldPasswordTextField"] + XCTAssertTrue(oldPasswordTextField.exists, "The text field should be shown.") + XCTAssertEqual(oldPasswordTextField.value as? String, "••••••••", "The text field should be showing the placeholder before text is input.") + + let newPasswordTextField1 = app.secureTextFields["newPasswordTextField1"] + XCTAssertTrue(newPasswordTextField1.exists, "The text field should be shown.") + XCTAssertEqual(newPasswordTextField1.value as? String, "••••••••", "The text field should be showing the placeholder before text is input.") + + let newPasswordTextField2 = app.secureTextFields["newPasswordTextField2"] + XCTAssertTrue(newPasswordTextField2.exists, "The text field should be shown.") + XCTAssertEqual(newPasswordTextField2.value as? String, "••••••••", "The text field should be showing the placeholder before text is input.") + + let submitButton = app.buttons["submitButton"] + XCTAssertTrue(submitButton.exists, "The submit button should be shown.") + XCTAssertTrue(submitButton.isEnabled, "The submit button should be enabled when able to submit.") + + let signoutAllDevicesToggle = app.switches["signoutAllDevicesToggle"] + XCTAssertTrue(signoutAllDevicesToggle.exists, "Sign out all devices toggle should exist") + XCTAssertTrue(signoutAllDevicesToggle.isOn, "Sign out all devices should be checked") + } + + func verifyCanSubmitAndSignoutAllDevicesChecked() { + XCTAssertTrue(app.staticTexts["titleLabel"].exists, "The title should be shown.") + + let oldPasswordTextField = app.secureTextFields["oldPasswordTextField"] + XCTAssertTrue(oldPasswordTextField.exists, "The text field should be shown.") + XCTAssertEqual(oldPasswordTextField.value as? String, "••••••••", "The text field should be showing the placeholder before text is input.") + + let newPasswordTextField1 = app.secureTextFields["newPasswordTextField1"] + XCTAssertTrue(newPasswordTextField1.exists, "The text field should be shown.") + XCTAssertEqual(newPasswordTextField1.value as? String, "••••••••", "The text field should be showing the placeholder before text is input.") + + let newPasswordTextField2 = app.secureTextFields["newPasswordTextField2"] + XCTAssertTrue(newPasswordTextField2.exists, "The text field should be shown.") + XCTAssertEqual(newPasswordTextField2.value as? String, "••••••••", "The text field should be showing the placeholder before text is input.") + + let submitButton = app.buttons["submitButton"] + XCTAssertTrue(submitButton.exists, "The submit button should be shown.") + XCTAssertTrue(submitButton.isEnabled, "The submit button should be enabled when able to submit.") + + let signoutAllDevicesToggle = app.switches["signoutAllDevicesToggle"] + XCTAssertTrue(signoutAllDevicesToggle.exists, "Sign out all devices toggle should exist") + XCTAssertTrue(signoutAllDevicesToggle.isOn, "Sign out all devices should be checked") + } + +} diff --git a/RiotSwiftUI/Modules/Settings/ChangePassword/Test/Unit/ChangePasswordViewModelTests.swift b/RiotSwiftUI/Modules/Settings/ChangePassword/Test/Unit/ChangePasswordViewModelTests.swift new file mode 100644 index 0000000000..a3a04d0114 --- /dev/null +++ b/RiotSwiftUI/Modules/Settings/ChangePassword/Test/Unit/ChangePasswordViewModelTests.swift @@ -0,0 +1,50 @@ +// +// Copyright 2021 New Vector Ltd +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +import XCTest + +@testable import RiotSwiftUI + +class ChangePasswordViewModelTests: XCTestCase { + + @MainActor func testEmptyState() async { + let viewModel = ChangePasswordViewModel() + let context = viewModel.context + + // Given an empty view model + XCTAssert(context.oldPassword.isEmpty, "The view model should start with an empty old password.") + XCTAssert(context.newPassword1.isEmpty, "The view model should start with an empty new password 1.") + XCTAssert(context.newPassword2.isEmpty, "The view model should start with an empty new password 2.") + XCTAssertFalse(context.viewState.canSubmit, "The view model should not be able to submit.") + XCTAssertTrue(context.signoutAllDevices, "The view model should start with sign out of all devices checked.") + } + + @MainActor func testValidState() async { + let viewModel = ChangePasswordViewModel(oldPassword: "12345678", + newPassword1: "87654321", + newPassword2: "87654321", + signoutAllDevices: false) + let context = viewModel.context + + // Given a filled view model in valid state + XCTAssertFalse(context.oldPassword.isEmpty, "The view model should start with an empty old password.") + XCTAssertFalse(context.newPassword1.isEmpty, "The view model should start with an empty new password 1.") + XCTAssertFalse(context.newPassword2.isEmpty, "The view model should start with an empty new password 2.") + XCTAssertTrue(context.viewState.canSubmit, "The view model should be able to submit.") + XCTAssertFalse(context.signoutAllDevices, "Sign out of all devices should be unchecked.") + } + +} diff --git a/RiotSwiftUI/Modules/Settings/ChangePassword/View/ChangePasswordScreen.swift b/RiotSwiftUI/Modules/Settings/ChangePassword/View/ChangePasswordScreen.swift new file mode 100644 index 0000000000..4224dabfdd --- /dev/null +++ b/RiotSwiftUI/Modules/Settings/ChangePassword/View/ChangePasswordScreen.swift @@ -0,0 +1,136 @@ +// +// Copyright 2021 New Vector Ltd +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +import SwiftUI + +struct ChangePasswordScreen: View { + + // MARK: - Properties + + // MARK: Private + + @Environment(\.theme) private var theme + + enum Field { case oldPassword, newPassword1, newPassword2 } + @State private var focusedField: Field? + + // MARK: Public + + @ObservedObject var viewModel: ChangePasswordViewModel.Context + + // MARK: Views + + var body: some View { + ScrollView { + VStack(spacing: 0) { + header + .padding(.top, 16) + .padding(.bottom, 36) + form + } + .readableFrame() + .padding(.horizontal, 16) + } + .background(theme.colors.background.ignoresSafeArea()) + .alert(item: $viewModel.alertInfo) { $0.alert } + .accentColor(theme.colors.accent) + } + + /// The title and icon at the top of the screen. + var header: some View { + VStack(spacing: 8) { + OnboardingIconImage(image: Asset.Images.authenticationPasswordIcon) + .padding(.bottom, 16) + + Text(VectorL10n.settingsChangePassword) + .font(theme.fonts.title2B) + .multilineTextAlignment(.center) + .foregroundColor(theme.colors.primaryContent) + .accessibilityIdentifier("titleLabel") + } + } + + /// The text fields and submit button. + var form: some View { + VStack(alignment: .leading, spacing: 12) { + RoundedBorderTextField(placeHolder: VectorL10n.settingsOldPassword, + text: $viewModel.oldPassword, + isFirstResponder: focusedField == .oldPassword, + configuration: UIKitTextInputConfiguration(returnKeyType: .next, + isSecureTextEntry: true), + onCommit: { focusedField = .newPassword1 }) + .accessibilityIdentifier("oldPasswordTextField") + + RoundedBorderTextField(placeHolder: VectorL10n.settingsNewPassword, + text: $viewModel.newPassword1, + isFirstResponder: focusedField == .newPassword1, + configuration: UIKitTextInputConfiguration(returnKeyType: .next, + isSecureTextEntry: true), + onCommit: { focusedField = .newPassword2 }) + .accessibilityIdentifier("newPasswordTextField1") + + RoundedBorderTextField(placeHolder: VectorL10n.settingsConfirmPassword, + text: $viewModel.newPassword2, + isFirstResponder: focusedField == .newPassword2, + configuration: UIKitTextInputConfiguration(returnKeyType: .done, + isSecureTextEntry: true), + onCommit: submit) + .accessibilityIdentifier("newPasswordTextField2") + + HStack(alignment: .center, spacing: 8) { + Toggle(VectorL10n.authenticationChoosePasswordSignoutAllDevices, isOn: $viewModel.signoutAllDevices) + .toggleStyle(AuthenticationTermsToggleStyle()) + .accessibilityIdentifier("signoutAllDevicesToggle") + Text(VectorL10n.authenticationChoosePasswordSignoutAllDevices) + .foregroundColor(theme.colors.secondaryContent) + } + .padding(.top, 8) + .padding(.bottom, 16) + .onTapGesture(perform: toggleSignoutAllDevices) + + Button(action: submit) { + Text(VectorL10n.save) + } + .buttonStyle(PrimaryActionButtonStyle()) + .disabled(!viewModel.viewState.canSubmit) + .accessibilityIdentifier("submitButton") + } + } + + /// Sends the `submit` view action if viewModel.viewState.canSubmit. + func submit() { + guard viewModel.viewState.canSubmit else { return } + viewModel.send(viewAction: .submit) + } + + /// Sends the `toggleSignoutAllDevices` view action. + func toggleSignoutAllDevices() { + viewModel.send(viewAction: .toggleSignoutAllDevices) + } +} + +// MARK: - Previews + +struct ChangePasswordScreen_Previews: PreviewProvider { + static let stateRenderer = MockChangePasswordScreenState.stateRenderer + static var previews: some View { + stateRenderer.screenGroup(addNavigation: true) + .navigationViewStyle(.stack) + stateRenderer.screenGroup(addNavigation: true) + .navigationViewStyle(.stack) + .theme(.dark).preferredColorScheme(.dark) + } +} From aa4e4dafdb618c5dc6ade743148790dcdace7109 Mon Sep 17 00:00:00 2001 From: ismailgulek Date: Thu, 9 Jun 2022 20:25:56 +0300 Subject: [PATCH 04/12] Create bridge presenter for change password coordinator --- .../ChangePasswordBridgePresenter.swift | 97 +++++++++++++++++++ 1 file changed, 97 insertions(+) create mode 100644 RiotSwiftUI/Modules/Settings/ChangePassword/Coordinator/ChangePasswordBridgePresenter.swift diff --git a/RiotSwiftUI/Modules/Settings/ChangePassword/Coordinator/ChangePasswordBridgePresenter.swift b/RiotSwiftUI/Modules/Settings/ChangePassword/Coordinator/ChangePasswordBridgePresenter.swift new file mode 100644 index 0000000000..4ef5583ba8 --- /dev/null +++ b/RiotSwiftUI/Modules/Settings/ChangePassword/Coordinator/ChangePasswordBridgePresenter.swift @@ -0,0 +1,97 @@ +// File created from FlowTemplate +// $ createRootCoordinator.sh Threads Threads ThreadList +/* + Copyright 2021 New Vector Ltd + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. + */ + +import Foundation +import MatrixSDK + +@objc protocol ChangePasswordCoordinatorBridgePresenterDelegate { + func changePasswordCoordinatorBridgePresenterDidCancel(_ bridgePresenter: ChangePasswordCoordinatorBridgePresenter) + func changePasswordCoordinatorBridgePresenterDidComplete(_ bridgePresenter: ChangePasswordCoordinatorBridgePresenter) +} + +/// ChangePasswordCoordinatorBridgePresenter enables to start ChangePasswordCoordinator from a view controller. +/// This bridge is used while waiting for global usage of coordinator pattern. +/// **WARNING**: This class breaks the Coordinator abstraction and it has been introduced for **Objective-C compatibility only** (mainly for integration in legacy view controllers). Each bridge should be removed +/// once the underlying Coordinator has been integrated by another Coordinator. +@objcMembers +final class ChangePasswordCoordinatorBridgePresenter: NSObject { + + // MARK: - Constants + + // MARK: - Properties + + // MARK: Private + + private let session: MXSession + private var coordinator: ChangePasswordCoordinator? + + // MARK: Public + + weak var delegate: ChangePasswordCoordinatorBridgePresenterDelegate? + + // MARK: - Setup + + /// Initializer + /// - Parameters: + /// - session: Session instance + init(session: MXSession) { + self.session = session + super.init() + } + + // MARK: - Public + + @MainActor func present(from viewController: UIViewController, animated: Bool) { + + let params = ChangePasswordCoordinatorParameters(restClient: self.session.matrixRestClient) + + let changePasswordCoordinator = ChangePasswordCoordinator(parameters: params) + changePasswordCoordinator.callback = { [weak self] in + guard let self = self else { return } + self.delegate?.changePasswordCoordinatorBridgePresenterDidComplete(self) + } + let presentable = changePasswordCoordinator.toPresentable() + let navController = RiotNavigationController(rootViewController: presentable.toPresentable()) + navController.navigationBar.topItem?.leftBarButtonItem = UIBarButtonItem(barButtonSystemItem: .cancel, + target: self, + action: #selector(cancelTapped)) + navController.isModalInPresentation = true + viewController.present(navController, animated: animated, completion: nil) + changePasswordCoordinator.start() + + self.coordinator = changePasswordCoordinator + } + + func dismiss(animated: Bool, completion: (() -> Void)?) { + guard let coordinator = self.coordinator else { + return + } + + // Dismiss modal + coordinator.toPresentable().dismiss(animated: animated) { + self.coordinator = nil + + completion?() + } + } + + @objc + private func cancelTapped() { + delegate?.changePasswordCoordinatorBridgePresenterDidCancel(self) + } +} From d8f24e9e23e11ecc437ce3a467578c5b0b13c292 Mon Sep 17 00:00:00 2001 From: ismailgulek Date: Thu, 9 Jun 2022 20:26:19 +0300 Subject: [PATCH 05/12] Use change password screen in settings --- .../Modules/Settings/SettingsViewController.m | 236 ++---------------- 1 file changed, 23 insertions(+), 213 deletions(-) diff --git a/Riot/Modules/Settings/SettingsViewController.m b/Riot/Modules/Settings/SettingsViewController.m index d237154ba8..3f2dd70488 100644 --- a/Riot/Modules/Settings/SettingsViewController.m +++ b/Riot/Modules/Settings/SettingsViewController.m @@ -189,7 +189,8 @@ @interface SettingsViewController () +ThreadsBetaCoordinatorBridgePresenterDelegate, +ChangePasswordCoordinatorBridgePresenterDelegate> { // Current alert (if any). __weak UIAlertController *currentAlert; @@ -211,12 +212,6 @@ @interface SettingsViewController () 0) && (newPasswordTextField1.text.length > 2) && [newPasswordTextField1.text isEqualToString:newPasswordTextField2.text]; -} - - (void)displayPasswordAlert { - __weak typeof(self) weakSelf = self; - [resetPwdAlertController dismissViewControllerAnimated:NO completion:nil]; - - resetPwdAlertController = [UIAlertController alertControllerWithTitle:[VectorL10n settingsChangePassword] message:nil preferredStyle:UIAlertControllerStyleAlert]; - resetPwdAlertController.accessibilityLabel=@"ChangePasswordAlertController"; - savePasswordAction = [UIAlertAction actionWithTitle:[VectorL10n save] style:UIAlertActionStyleDefault handler:^(UIAlertAction * action) { - - if (weakSelf) - { - typeof(self) self = weakSelf; - - self->resetPwdAlertController = nil; - - if ([MXKAccountManager sharedManager].activeAccounts.count > 0) - { - [self startActivityIndicator]; - self->isResetPwdInProgress = YES; - - MXKAccount* account = [MXKAccountManager sharedManager].activeAccounts.firstObject; - - [account changePassword:self->currentPasswordTextField.text with:self->newPasswordTextField1.text logoutDevices:YES success:^{ - - if (weakSelf) - { - typeof(self) self = weakSelf; - - self->isResetPwdInProgress = NO; - [self stopActivityIndicator]; - - // Display a successful message only if the settings screen is still visible (destroy is not called yet) - if (!self->onReadyToDestroyHandler) - { - [self->currentAlert dismissViewControllerAnimated:NO completion:nil]; - - UIAlertController *successAlert = [UIAlertController alertControllerWithTitle:nil message:[VectorL10n settingsPasswordUpdated] preferredStyle:UIAlertControllerStyleAlert]; - - [successAlert addAction:[UIAlertAction actionWithTitle:[VectorL10n ok] - style:UIAlertActionStyleDefault - handler:^(UIAlertAction * action) { - - if (weakSelf) - { - typeof(self) self = weakSelf; - self->currentAlert = nil; - - // Check whether destroy has been called durign pwd change - if (self->onReadyToDestroyHandler) - { - // Ready to destroy - self->onReadyToDestroyHandler(); - self->onReadyToDestroyHandler = nil; - } - } - - }]]; - - [successAlert mxk_setAccessibilityIdentifier:@"SettingsVCOnPasswordUpdatedAlert"]; - [self presentViewController:successAlert animated:YES completion:nil]; - self->currentAlert = successAlert; - } - else - { - // Ready to destroy - self->onReadyToDestroyHandler(); - self->onReadyToDestroyHandler = nil; - } - } - - } failure:^(NSError *error) { - - if (weakSelf) - { - typeof(self) self = weakSelf; - - self->isResetPwdInProgress = NO; - [self stopActivityIndicator]; - - // Display a failure message on the current screen - UIViewController *rootViewController = [AppDelegate theDelegate].window.rootViewController; - if (rootViewController) - { - [self->currentAlert dismissViewControllerAnimated:NO completion:nil]; - - UIAlertController *errorAlert = [UIAlertController alertControllerWithTitle:nil message:[VectorL10n settingsFailToUpdatePassword] preferredStyle:UIAlertControllerStyleAlert]; - - [errorAlert addAction:[UIAlertAction actionWithTitle:[VectorL10n ok] - style:UIAlertActionStyleDefault - handler:^(UIAlertAction * action) { - - if (weakSelf) - { - typeof(self) self = weakSelf; - - self->currentAlert = nil; - - // Check whether destroy has been called durign pwd change - if (self->onReadyToDestroyHandler) - { - // Ready to destroy - self->onReadyToDestroyHandler(); - self->onReadyToDestroyHandler = nil; - } - } - - }]]; - - [errorAlert mxk_setAccessibilityIdentifier:@"SettingsVCPasswordChangeFailedAlert"]; - [rootViewController presentViewController:errorAlert animated:YES completion:nil]; - self->currentAlert = errorAlert; - } - } - - }]; - } - } - - }]; - - // disable by default - // check if the textfields have the right value - savePasswordAction.enabled = NO; - - UIAlertAction* cancel = [UIAlertAction actionWithTitle:@"Cancel" style:UIAlertActionStyleCancel handler:^(UIAlertAction * action) { - - if (weakSelf) - { - typeof(self) self = weakSelf; - - self->resetPwdAlertController = nil; - } - - }]; - - [resetPwdAlertController addTextFieldWithConfigurationHandler:^(UITextField *textField) { - - if (weakSelf) - { - typeof(self) self = weakSelf; - - self->currentPasswordTextField = textField; - self->currentPasswordTextField.placeholder = [VectorL10n settingsOldPassword]; - self->currentPasswordTextField.secureTextEntry = YES; - [self->currentPasswordTextField addTarget:self action:@selector(passwordTextFieldDidChange:) forControlEvents:UIControlEventEditingChanged]; - } - - }]; - - [resetPwdAlertController addTextFieldWithConfigurationHandler:^(UITextField *textField) { - - if (weakSelf) - { - typeof(self) self = weakSelf; - - self->newPasswordTextField1 = textField; - self->newPasswordTextField1.placeholder = [VectorL10n settingsNewPassword]; - self->newPasswordTextField1.secureTextEntry = YES; - [self->newPasswordTextField1 addTarget:self action:@selector(passwordTextFieldDidChange:) forControlEvents:UIControlEventEditingChanged]; - } - - }]; - - [resetPwdAlertController addTextFieldWithConfigurationHandler:^(UITextField *textField) { - - if (weakSelf) - { - typeof(self) self = weakSelf; - - self->newPasswordTextField2 = textField; - self->newPasswordTextField2.placeholder = [VectorL10n settingsConfirmPassword]; - self->newPasswordTextField2.secureTextEntry = YES; - [self->newPasswordTextField2 addTarget:self action:@selector(passwordTextFieldDidChange:) forControlEvents:UIControlEventEditingChanged]; - } - }]; - - - [resetPwdAlertController addAction:cancel]; - [resetPwdAlertController addAction:savePasswordAction]; - - UIButton *logoutDevicesButton = [[UIButton alloc] initWithFrame:CGRectMake(0, 0, 100, 50)]; - [logoutDevicesButton setImage:AssetImages.selectionTick.image forState:UIControlStateSelected]; - [logoutDevicesButton setImage:AssetImages.selectionUntick.image forState:UIControlStateNormal]; - [logoutDevicesButton setTitle:VectorL10n.settingsChangePassword forState:UIControlStateNormal]; - [resetPwdAlertController.view addSubview:logoutDevicesButton]; + self.changePasswordBridgePresenter = [[ChangePasswordCoordinatorBridgePresenter alloc] initWithSession:self.mainSession]; + self.changePasswordBridgePresenter.delegate = self; - [self presentViewController:resetPwdAlertController animated:YES completion:nil]; + [self.changePasswordBridgePresenter presentFrom:self animated:YES]; } - #pragma mark - MXKCountryPickerViewControllerDelegate - (void)countryPickerViewController:(MXKCountryPickerViewController *)countryPickerViewController didSelectCountry:(NSString *)isoCountryCode @@ -4807,4 +4602,19 @@ - (void)threadsBetaCoordinatorBridgePresenterDelegateDidTapCancel:(ThreadsBetaCo }]; } +#pragma mark - ChangePasswordCoordinatorBridgePresenterDelegate + +- (void)changePasswordCoordinatorBridgePresenterDidComplete:(ChangePasswordCoordinatorBridgePresenter *)bridgePresenter +{ + [bridgePresenter dismissWithAnimated:YES completion:^{ + self.changePasswordBridgePresenter = nil; + }]; +} + +- (void)changePasswordCoordinatorBridgePresenterDidCancel:(ChangePasswordCoordinatorBridgePresenter *)bridgePresenter +{ + [bridgePresenter dismissWithAnimated:YES completion:nil]; + self.changePasswordBridgePresenter = nil; +} + @end From fc6bc078761f5c8ba4cd6a264149725afea11d07 Mon Sep 17 00:00:00 2001 From: ismailgulek Date: Thu, 9 Jun 2022 20:27:02 +0300 Subject: [PATCH 06/12] Add changelog --- changelog.d/6175.change | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/6175.change diff --git a/changelog.d/6175.change b/changelog.d/6175.change new file mode 100644 index 0000000000..2f281e62bf --- /dev/null +++ b/changelog.d/6175.change @@ -0,0 +1 @@ +Settings: Implement logging out all devices when changing password. From dae0ac5ace7bf17743a6f66ee82b6b3e225c1ef5 Mon Sep 17 00:00:00 2001 From: ismailgulek Date: Fri, 10 Jun 2022 14:28:00 +0300 Subject: [PATCH 07/12] Fix some of the PR remarks --- .../ChangePassword/ChangePasswordModels.swift | 8 ++++--- .../ChangePasswordViewModel.swift | 16 +++++++++---- .../ChangePasswordBridgePresenter.swift | 2 +- .../ChangePasswordCoordinator.swift | 6 +---- .../MockChangePasswordScreenState.swift | 3 +-- .../Test/UI/ChangePasswordUITests.swift | 24 +++++++++---------- 6 files changed, 32 insertions(+), 27 deletions(-) diff --git a/RiotSwiftUI/Modules/Settings/ChangePassword/ChangePasswordModels.swift b/RiotSwiftUI/Modules/Settings/ChangePassword/ChangePasswordModels.swift index 513c00a080..35df8396d4 100644 --- a/RiotSwiftUI/Modules/Settings/ChangePassword/ChangePasswordModels.swift +++ b/RiotSwiftUI/Modules/Settings/ChangePassword/ChangePasswordModels.swift @@ -20,7 +20,7 @@ import SwiftUI enum ChangePasswordViewModelResult { /// Submit with old and new passwords and sign out of all devices option - case submit(String, String, Bool) + case submit(oldPassword: String, newPassword: String, signoutAllDevices: Bool) } // MARK: View @@ -29,11 +29,11 @@ struct ChangePasswordViewState: BindableState { /// View state that can be bound to from SwiftUI. var bindings: ChangePasswordBindings - /// Whether the user can submit the form: old password should be entered, and new passwords should match + /// Whether the user can submit the form: old password and new passwords should be entered var canSubmit: Bool { !bindings.oldPassword.isEmpty && !bindings.newPassword1.isEmpty - && bindings.newPassword1 == bindings.newPassword2 + && !bindings.newPassword2.isEmpty } } @@ -60,6 +60,8 @@ enum ChangePasswordViewAction { enum ChangePasswordErrorType: Hashable { /// An error response from the homeserver. case mxError(String) + /// User entered new passwords do not match + case passwordsDontMatch /// An unknown error occurred. case unknown } diff --git a/RiotSwiftUI/Modules/Settings/ChangePassword/ChangePasswordViewModel.swift b/RiotSwiftUI/Modules/Settings/ChangePassword/ChangePasswordViewModel.swift index f5e1065c64..5ded9d127a 100644 --- a/RiotSwiftUI/Modules/Settings/ChangePassword/ChangePasswordViewModel.swift +++ b/RiotSwiftUI/Modules/Settings/ChangePassword/ChangePasswordViewModel.swift @@ -34,7 +34,7 @@ class ChangePasswordViewModel: ChangePasswordViewModelType, ChangePasswordViewMo init(oldPassword: String = "", newPassword1: String = "", newPassword2: String = "", - signoutAllDevices: Bool = true) { + signoutAllDevices: Bool = false) { let bindings = ChangePasswordBindings(oldPassword: oldPassword, newPassword1: newPassword1, newPassword2: newPassword2, @@ -48,9 +48,13 @@ class ChangePasswordViewModel: ChangePasswordViewModelType, ChangePasswordViewMo override func process(viewAction: ChangePasswordViewAction) { switch viewAction { case .submit: - Task { await callback?(.submit(state.bindings.oldPassword, - state.bindings.newPassword1, - state.bindings.signoutAllDevices)) } + guard state.bindings.newPassword1 == state.bindings.newPassword2 else { + Task { await displayError(.passwordsDontMatch) } + return + } + Task { await callback?(.submit(oldPassword: state.bindings.oldPassword, + newPassword: state.bindings.newPassword1, + signoutAllDevices: state.bindings.signoutAllDevices)) } case .toggleSignoutAllDevices: state.bindings.signoutAllDevices.toggle() } @@ -62,6 +66,10 @@ class ChangePasswordViewModel: ChangePasswordViewModelType, ChangePasswordViewMo state.bindings.alertInfo = AlertInfo(id: type, title: VectorL10n.error, message: message) + case .passwordsDontMatch: + state.bindings.alertInfo = AlertInfo(id: type, + title: VectorL10n.error, + message: VectorL10n.authPasswordDontMatch) case .unknown: state.bindings.alertInfo = AlertInfo(id: type) } diff --git a/RiotSwiftUI/Modules/Settings/ChangePassword/Coordinator/ChangePasswordBridgePresenter.swift b/RiotSwiftUI/Modules/Settings/ChangePassword/Coordinator/ChangePasswordBridgePresenter.swift index 4ef5583ba8..15c99ade34 100644 --- a/RiotSwiftUI/Modules/Settings/ChangePassword/Coordinator/ChangePasswordBridgePresenter.swift +++ b/RiotSwiftUI/Modules/Settings/ChangePassword/Coordinator/ChangePasswordBridgePresenter.swift @@ -56,7 +56,7 @@ final class ChangePasswordCoordinatorBridgePresenter: NSObject { // MARK: - Public - @MainActor func present(from viewController: UIViewController, animated: Bool) { + func present(from viewController: UIViewController, animated: Bool) { let params = ChangePasswordCoordinatorParameters(restClient: self.session.matrixRestClient) diff --git a/RiotSwiftUI/Modules/Settings/ChangePassword/Coordinator/ChangePasswordCoordinator.swift b/RiotSwiftUI/Modules/Settings/ChangePassword/Coordinator/ChangePasswordCoordinator.swift index d60d16a937..7274cc2915 100644 --- a/RiotSwiftUI/Modules/Settings/ChangePassword/Coordinator/ChangePasswordCoordinator.swift +++ b/RiotSwiftUI/Modules/Settings/ChangePassword/Coordinator/ChangePasswordCoordinator.swift @@ -48,7 +48,7 @@ final class ChangePasswordCoordinator: Coordinator, Presentable { // MARK: - Setup - @MainActor init(parameters: ChangePasswordCoordinatorParameters) { + init(parameters: ChangePasswordCoordinatorParameters) { self.parameters = parameters let viewModel = ChangePasswordViewModel() @@ -105,14 +105,10 @@ final class ChangePasswordCoordinator: Coordinator, Presentable { do { try await parameters.restClient.changePassword(from: oldPassword, to: newPassword, logoutDevices: signoutAllDevices) - // Shouldn't be reachable but just in case, continue the flow. - guard !Task.isCancelled else { return } self?.stopLoading() self?.callback?() - } catch is CancellationError { - return } catch { self?.stopLoading() self?.handleError(error) diff --git a/RiotSwiftUI/Modules/Settings/ChangePassword/MockChangePasswordScreenState.swift b/RiotSwiftUI/Modules/Settings/ChangePassword/MockChangePasswordScreenState.swift index 6656e91585..2e28720aa4 100644 --- a/RiotSwiftUI/Modules/Settings/ChangePassword/MockChangePasswordScreenState.swift +++ b/RiotSwiftUI/Modules/Settings/ChangePassword/MockChangePasswordScreenState.swift @@ -42,8 +42,7 @@ enum MockChangePasswordScreenState: MockScreenState, CaseIterable { viewModel = ChangePasswordViewModel() case .cannotSubmit: viewModel = ChangePasswordViewModel(oldPassword: "12345678", - newPassword1: "87654321", - newPassword2: "97654321") + newPassword1: "87654321") case .canSubmit: viewModel = ChangePasswordViewModel(oldPassword: "12345678", newPassword1: "87654321", diff --git a/RiotSwiftUI/Modules/Settings/ChangePassword/Test/UI/ChangePasswordUITests.swift b/RiotSwiftUI/Modules/Settings/ChangePassword/Test/UI/ChangePasswordUITests.swift index fde678d983..4bf4f94ca0 100644 --- a/RiotSwiftUI/Modules/Settings/ChangePassword/Test/UI/ChangePasswordUITests.swift +++ b/RiotSwiftUI/Modules/Settings/ChangePassword/Test/UI/ChangePasswordUITests.swift @@ -62,7 +62,7 @@ class ChangePasswordUITests: MockScreenTest { let signoutAllDevicesToggle = app.switches["signoutAllDevicesToggle"] XCTAssertTrue(signoutAllDevicesToggle.exists, "Sign out all devices toggle should exist") - XCTAssertTrue(signoutAllDevicesToggle.isOn, "Sign out all devices should be checked") + XCTAssertFalse(signoutAllDevicesToggle.isOn, "Sign out all devices should be unchecked") } func verifyCannotSubmit() { @@ -70,15 +70,15 @@ class ChangePasswordUITests: MockScreenTest { let oldPasswordTextField = app.secureTextFields["oldPasswordTextField"] XCTAssertTrue(oldPasswordTextField.exists, "The text field should be shown.") - XCTAssertEqual(oldPasswordTextField.value as? String, "••••••••", "The text field should be showing the placeholder before text is input.") + XCTAssertEqual(oldPasswordTextField.value as? String, "••••••••", "The text field should show the entered password secretly.") let newPasswordTextField1 = app.secureTextFields["newPasswordTextField1"] XCTAssertTrue(newPasswordTextField1.exists, "The text field should be shown.") - XCTAssertEqual(newPasswordTextField1.value as? String, "••••••••", "The text field should be showing the placeholder before text is input.") + XCTAssertEqual(newPasswordTextField1.value as? String, "••••••••", "The text field should show the entered password secretly.") let newPasswordTextField2 = app.secureTextFields["newPasswordTextField2"] XCTAssertTrue(newPasswordTextField2.exists, "The text field should be shown.") - XCTAssertEqual(newPasswordTextField2.value as? String, "••••••••", "The text field should be showing the placeholder before text is input.") + XCTAssertEqual(newPasswordTextField2.label, "confirm password", "The text field should be showing the placeholder before text is input.") let submitButton = app.buttons["submitButton"] XCTAssertTrue(submitButton.exists, "The submit button should be shown.") @@ -86,7 +86,7 @@ class ChangePasswordUITests: MockScreenTest { let signoutAllDevicesToggle = app.switches["signoutAllDevicesToggle"] XCTAssertTrue(signoutAllDevicesToggle.exists, "Sign out all devices toggle should exist") - XCTAssertTrue(signoutAllDevicesToggle.isOn, "Sign out all devices should be checked") + XCTAssertFalse(signoutAllDevicesToggle.isOn, "Sign out all devices should be unchecked") } func verifyCanSubmit() { @@ -94,15 +94,15 @@ class ChangePasswordUITests: MockScreenTest { let oldPasswordTextField = app.secureTextFields["oldPasswordTextField"] XCTAssertTrue(oldPasswordTextField.exists, "The text field should be shown.") - XCTAssertEqual(oldPasswordTextField.value as? String, "••••••••", "The text field should be showing the placeholder before text is input.") + XCTAssertEqual(oldPasswordTextField.value as? String, "••••••••", "The text field should show the entered password secretly.") let newPasswordTextField1 = app.secureTextFields["newPasswordTextField1"] XCTAssertTrue(newPasswordTextField1.exists, "The text field should be shown.") - XCTAssertEqual(newPasswordTextField1.value as? String, "••••••••", "The text field should be showing the placeholder before text is input.") + XCTAssertEqual(newPasswordTextField1.value as? String, "••••••••", "The text field should show the entered password secretly.") let newPasswordTextField2 = app.secureTextFields["newPasswordTextField2"] XCTAssertTrue(newPasswordTextField2.exists, "The text field should be shown.") - XCTAssertEqual(newPasswordTextField2.value as? String, "••••••••", "The text field should be showing the placeholder before text is input.") + XCTAssertEqual(newPasswordTextField2.value as? String, "••••••••", "The text field should show the entered password secretly.") let submitButton = app.buttons["submitButton"] XCTAssertTrue(submitButton.exists, "The submit button should be shown.") @@ -110,7 +110,7 @@ class ChangePasswordUITests: MockScreenTest { let signoutAllDevicesToggle = app.switches["signoutAllDevicesToggle"] XCTAssertTrue(signoutAllDevicesToggle.exists, "Sign out all devices toggle should exist") - XCTAssertTrue(signoutAllDevicesToggle.isOn, "Sign out all devices should be checked") + XCTAssertFalse(signoutAllDevicesToggle.isOn, "Sign out all devices should be unchecked") } func verifyCanSubmitAndSignoutAllDevicesChecked() { @@ -118,15 +118,15 @@ class ChangePasswordUITests: MockScreenTest { let oldPasswordTextField = app.secureTextFields["oldPasswordTextField"] XCTAssertTrue(oldPasswordTextField.exists, "The text field should be shown.") - XCTAssertEqual(oldPasswordTextField.value as? String, "••••••••", "The text field should be showing the placeholder before text is input.") + XCTAssertEqual(oldPasswordTextField.value as? String, "••••••••", "The text field should show the entered password secretly.") let newPasswordTextField1 = app.secureTextFields["newPasswordTextField1"] XCTAssertTrue(newPasswordTextField1.exists, "The text field should be shown.") - XCTAssertEqual(newPasswordTextField1.value as? String, "••••••••", "The text field should be showing the placeholder before text is input.") + XCTAssertEqual(newPasswordTextField1.value as? String, "••••••••", "The text field should show the entered password secretly.") let newPasswordTextField2 = app.secureTextFields["newPasswordTextField2"] XCTAssertTrue(newPasswordTextField2.exists, "The text field should be shown.") - XCTAssertEqual(newPasswordTextField2.value as? String, "••••••••", "The text field should be showing the placeholder before text is input.") + XCTAssertEqual(newPasswordTextField2.value as? String, "••••••••", "The text field should show the entered password secretly.") let submitButton = app.buttons["submitButton"] XCTAssertTrue(submitButton.exists, "The submit button should be shown.") From 4a4c15aed2c039061686f6175c418dca487049f3 Mon Sep 17 00:00:00 2001 From: ismailgulek Date: Fri, 10 Jun 2022 18:02:13 +0300 Subject: [PATCH 08/12] Create `PasswordValidator` --- Riot/Assets/en.lproj/Untranslated.strings | 10 ++ Riot/Generated/UntranslatedStrings.swift | 32 ++++++ Riot/Utils/PasswordValidator.swift | 118 ++++++++++++++++++++++ RiotTests/PasswordValidatorTests.swift | 98 ++++++++++++++++++ 4 files changed, 258 insertions(+) create mode 100644 Riot/Utils/PasswordValidator.swift create mode 100644 RiotTests/PasswordValidatorTests.swift diff --git a/Riot/Assets/en.lproj/Untranslated.strings b/Riot/Assets/en.lproj/Untranslated.strings index 5727d988db..b01125a78e 100644 --- a/Riot/Assets/en.lproj/Untranslated.strings +++ b/Riot/Assets/en.lproj/Untranslated.strings @@ -88,3 +88,13 @@ "leave_space_selection_title" = "SELECT ROOMS"; "leave_space_selection_all_rooms" = "Select all rooms"; "leave_space_selection_no_rooms" = "Select no rooms"; + +// MARK: Password Validation +"password_validation_info_header" = "Your password should meet the criteria below:"; +"password_validation_error_header" = "Given password does not meet the criteria below:"; +"password_validation_error_min_length" = "At least %d characters."; +"password_validation_error_max_length" = "Not exceed %d characters."; +"password_validation_error_contain_lowercase_letter" = "Contain a lower-case letter."; +"password_validation_error_contain_uppercase_letter" = "Contain an upper-case letter."; +"password_validation_error_contain_number" = "Contain a number."; +"password_validation_error_contain_symbol" = "Contain a symbol."; diff --git a/Riot/Generated/UntranslatedStrings.swift b/Riot/Generated/UntranslatedStrings.swift index 7d0c6ff7c2..b2aa475a5a 100644 --- a/Riot/Generated/UntranslatedStrings.swift +++ b/Riot/Generated/UntranslatedStrings.swift @@ -226,6 +226,38 @@ public extension VectorL10n { static var leaveSpaceSelectionTitle: String { return VectorL10n.tr("Untranslated", "leave_space_selection_title") } + /// Contain a lower-case letter. + static var passwordValidationErrorContainLowercaseLetter: String { + return VectorL10n.tr("Untranslated", "password_validation_error_contain_lowercase_letter") + } + /// Contain a number. + static var passwordValidationErrorContainNumber: String { + return VectorL10n.tr("Untranslated", "password_validation_error_contain_number") + } + /// Contain a symbol. + static var passwordValidationErrorContainSymbol: String { + return VectorL10n.tr("Untranslated", "password_validation_error_contain_symbol") + } + /// Contain an upper-case letter. + static var passwordValidationErrorContainUppercaseLetter: String { + return VectorL10n.tr("Untranslated", "password_validation_error_contain_uppercase_letter") + } + /// Given password does not meet the criteria below: + static var passwordValidationErrorHeader: String { + return VectorL10n.tr("Untranslated", "password_validation_error_header") + } + /// Not exceed %d characters. + static func passwordValidationErrorMaxLength(_ p1: Int) -> String { + return VectorL10n.tr("Untranslated", "password_validation_error_max_length", p1) + } + /// At least %d characters. + static func passwordValidationErrorMinLength(_ p1: Int) -> String { + return VectorL10n.tr("Untranslated", "password_validation_error_min_length", p1) + } + /// Your password should meet the criteria below: + static var passwordValidationInfoHeader: String { + return VectorL10n.tr("Untranslated", "password_validation_info_header") + } /// This feature isn't available here. For now, you can do this with %@ on your computer. static func spacesFeatureNotAvailable(_ p1: String) -> String { return VectorL10n.tr("Untranslated", "spaces_feature_not_available", p1) diff --git a/Riot/Utils/PasswordValidator.swift b/Riot/Utils/PasswordValidator.swift new file mode 100644 index 0000000000..59c6e15d44 --- /dev/null +++ b/Riot/Utils/PasswordValidator.swift @@ -0,0 +1,118 @@ +// +// Copyright 2022 New Vector Ltd +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +import Foundation + +struct PasswordValidatorError: LocalizedError { + /// Unmet rules + let unmetRules: [PasswordValidatorRule] + + /// Error description for the error + var errorDescription: String? { + var result = VectorL10n.passwordValidationErrorHeader + "\n" + result += unmetRules.map { $0.descriptionInList }.joined(separator: "\n") + return result + } +} + +/// Validation rule for a password +enum PasswordValidatorRule: CustomStringConvertible, Hashable { + case minLength(_ value: Int) + case maxLength(_ value: Int) + case containLowercaseLetter + case containUppercaseLetter + case containNumber + case containSymbol + + var description: String { + switch self { + case .minLength(let value): + return VectorL10n.passwordValidationErrorMinLength(value) + case .maxLength(let value): + return VectorL10n.passwordValidationErrorMaxLength(value) + case .containLowercaseLetter: + return VectorL10n.passwordValidationErrorContainLowercaseLetter + case .containUppercaseLetter: + return VectorL10n.passwordValidationErrorContainUppercaseLetter + case .containNumber: + return VectorL10n.passwordValidationErrorContainNumber + case .containSymbol: + return VectorL10n.passwordValidationErrorContainSymbol + } + } + + var descriptionInList: String { + return "• " + description + } + + func metBy(password: String) -> Bool { + switch self { + case .minLength(let value): + return password.count >= value + case .maxLength(let value): + return password.count <= value + case .containLowercaseLetter: + return password.range(of: "[a-z]", options: .regularExpression) != nil + case .containUppercaseLetter: + return password.range(of: "[A-Z]", options: .regularExpression) != nil + case .containNumber: + return password.range(of: "[0-9]", options: .regularExpression) != nil + case .containSymbol: + return password.range(of: "[!\"#$%&'()*+,-.:;<=>?@\\_`{|}~\\[\\]]", + options: .regularExpression) != nil + } + } +} + +/// A utility class to validate a password against some rules. +class PasswordValidator { + + /// Validation rules + let rules: [PasswordValidatorRule] + + /// Initializer + /// - Parameter rules: validation rules + init(withRules rules: [PasswordValidatorRule]) { + self.rules = rules + } + + /// Validate a given password. + /// - Parameter password: Password to be validated + func validate(password: String) throws { + var unmetRules: [PasswordValidatorRule] = [] + for rule in rules { + if !rule.metBy(password: password) { + unmetRules.append(rule) + } + } + if !unmetRules.isEmpty { + throw PasswordValidatorError(unmetRules: unmetRules) + } + } + + /// Creates a description text with current rules + /// - Parameter header: Header text to include in the result + /// - Returns: Description text containing `header` and rules + func description(with header: String) -> String { + var result = header + if !rules.isEmpty { + result += "\n" + } + result += rules.map { $0.descriptionInList }.joined(separator: "\n") + return result + } + +} diff --git a/RiotTests/PasswordValidatorTests.swift b/RiotTests/PasswordValidatorTests.swift new file mode 100644 index 0000000000..f48af6e2a4 --- /dev/null +++ b/RiotTests/PasswordValidatorTests.swift @@ -0,0 +1,98 @@ +// +// Copyright 2022 New Vector Ltd +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +import XCTest +@testable import Riot + +class PasswordValidatorTests: XCTestCase { + + func testOnlyLength() throws { + let minLengthRule = PasswordValidatorRule.minLength(8) + let validator = PasswordValidator(withRules: [minLengthRule]) + + // this should pass + try validator.validate(password: "abcdefgh") + + do { + // this should fail + try validator.validate(password: "abcdefg") + XCTFail("Should not pass") + } catch let error as PasswordValidatorError { + XCTAssertEqual(error.unmetRules.count, 1) + XCTAssertEqual(error.unmetRules.first, minLengthRule) + } + } + + func testComplexWithMinimumRequirements() throws { + let validator = PasswordValidator(withRules: [ + .minLength(4), + .maxLength(4), + .containUppercaseLetter, + .containLowercaseLetter, + .containNumber, + .containSymbol + ]) + + // this should pass + try validator.validate(password: "Ab1!") + + do { + // this should fail with only maxLength rule + try validator.validate(password: "Ab1!E") + XCTFail("Should fail with only maxLength rule") + } catch let error as PasswordValidatorError { + XCTAssertEqual(error.unmetRules.count, 1) + XCTAssertEqual(error.unmetRules.first, .maxLength(4)) + } + + do { + // this should fail with only uppercase rule + try validator.validate(password: "ab1!") + XCTFail("Should fail with only uppercase rule") + } catch let error as PasswordValidatorError { + XCTAssertEqual(error.unmetRules.count, 1) + XCTAssertEqual(error.unmetRules.first, .containUppercaseLetter) + } + + do { + // this should fail with only lowercase rule + try validator.validate(password: "AB1!") + XCTFail("Should fail with only lowercase rule") + } catch let error as PasswordValidatorError { + XCTAssertEqual(error.unmetRules.count, 1) + XCTAssertEqual(error.unmetRules.first, .containLowercaseLetter) + } + + do { + // this should fail with only number rule + try validator.validate(password: "Abc!") + XCTFail("Should fail with only number rule") + } catch let error as PasswordValidatorError { + XCTAssertEqual(error.unmetRules.count, 1) + XCTAssertEqual(error.unmetRules.first, .containNumber) + } + + do { + // this should fail with only symbol rule + try validator.validate(password: "Abc1") + XCTFail("Should fail with only symbol rule") + } catch let error as PasswordValidatorError { + XCTAssertEqual(error.unmetRules.count, 1) + XCTAssertEqual(error.unmetRules.first, .containSymbol) + } + } + +} From 1d78262f80ffeea47e2de57562853d0faacbb4ca Mon Sep 17 00:00:00 2001 From: ismailgulek Date: Fri, 10 Jun 2022 18:06:41 +0300 Subject: [PATCH 09/12] Add password validator to change password coordinator --- .../ChangePassword/ChangePasswordModels.swift | 2 ++ .../ChangePasswordViewModel.swift | 4 +++- .../ChangePasswordCoordinator.swift | 23 ++++++++++++++----- .../Test/UI/ChangePasswordUITests.swift | 4 ++++ .../View/ChangePasswordScreen.swift | 11 +++++++-- 5 files changed, 35 insertions(+), 9 deletions(-) diff --git a/RiotSwiftUI/Modules/Settings/ChangePassword/ChangePasswordModels.swift b/RiotSwiftUI/Modules/Settings/ChangePassword/ChangePasswordModels.swift index 35df8396d4..5911441999 100644 --- a/RiotSwiftUI/Modules/Settings/ChangePassword/ChangePasswordModels.swift +++ b/RiotSwiftUI/Modules/Settings/ChangePassword/ChangePasswordModels.swift @@ -26,6 +26,8 @@ enum ChangePasswordViewModelResult { // MARK: View struct ChangePasswordViewState: BindableState { + /// Requirements text for the new password + var passwordRequirements: String /// View state that can be bound to from SwiftUI. var bindings: ChangePasswordBindings diff --git a/RiotSwiftUI/Modules/Settings/ChangePassword/ChangePasswordViewModel.swift b/RiotSwiftUI/Modules/Settings/ChangePassword/ChangePasswordViewModel.swift index 5ded9d127a..b11309e9a7 100644 --- a/RiotSwiftUI/Modules/Settings/ChangePassword/ChangePasswordViewModel.swift +++ b/RiotSwiftUI/Modules/Settings/ChangePassword/ChangePasswordViewModel.swift @@ -34,12 +34,14 @@ class ChangePasswordViewModel: ChangePasswordViewModelType, ChangePasswordViewMo init(oldPassword: String = "", newPassword1: String = "", newPassword2: String = "", + passwordRequirements: String = "", signoutAllDevices: Bool = false) { let bindings = ChangePasswordBindings(oldPassword: oldPassword, newPassword1: newPassword1, newPassword2: newPassword2, signoutAllDevices: signoutAllDevices) - let viewState = ChangePasswordViewState(bindings: bindings) + let viewState = ChangePasswordViewState(passwordRequirements: passwordRequirements, + bindings: bindings) super.init(initialViewState: viewState) } diff --git a/RiotSwiftUI/Modules/Settings/ChangePassword/Coordinator/ChangePasswordCoordinator.swift b/RiotSwiftUI/Modules/Settings/ChangePassword/Coordinator/ChangePasswordCoordinator.swift index 7274cc2915..2c590a62ef 100644 --- a/RiotSwiftUI/Modules/Settings/ChangePassword/Coordinator/ChangePasswordCoordinator.swift +++ b/RiotSwiftUI/Modules/Settings/ChangePassword/Coordinator/ChangePasswordCoordinator.swift @@ -30,6 +30,13 @@ final class ChangePasswordCoordinator: Coordinator, Presentable { private let parameters: ChangePasswordCoordinatorParameters private let changePasswordHostingController: VectorHostingController private var changePasswordViewModel: ChangePasswordViewModelProtocol + private let passwordValidator = PasswordValidator(withRules: [ + .minLength(8), + .containUppercaseLetter, + .containLowercaseLetter, + .containNumber, + .containSymbol + ]) private var indicatorPresenter: UserIndicatorTypePresenterProtocol private var loadingIndicator: UserIndicator? @@ -50,8 +57,9 @@ final class ChangePasswordCoordinator: Coordinator, Presentable { init(parameters: ChangePasswordCoordinatorParameters) { self.parameters = parameters - - let viewModel = ChangePasswordViewModel() + + let requirements = passwordValidator.description(with: VectorL10n.passwordValidationInfoHeader) + let viewModel = ChangePasswordViewModel(passwordRequirements: requirements) let view = ChangePasswordScreen(viewModel: viewModel.context) changePasswordViewModel = viewModel changePasswordHostingController = VectorHostingController(rootView: view) @@ -103,6 +111,7 @@ final class ChangePasswordCoordinator: Coordinator, Presentable { currentTask = Task { [weak self] in do { + try passwordValidator.validate(password: newPassword) try await parameters.restClient.changePassword(from: oldPassword, to: newPassword, logoutDevices: signoutAllDevices) guard !Task.isCancelled else { return } @@ -122,9 +131,11 @@ final class ChangePasswordCoordinator: Coordinator, Presentable { changePasswordViewModel.displayError(.mxError(mxError.error)) return } - - // TODO: Handle another other error types as needed. - - changePasswordViewModel.displayError(.unknown) + + if let error = error as? PasswordValidatorError { + changePasswordViewModel.displayError(.mxError(error.localizedDescription)) + } else { + changePasswordViewModel.displayError(.unknown) + } } } diff --git a/RiotSwiftUI/Modules/Settings/ChangePassword/Test/UI/ChangePasswordUITests.swift b/RiotSwiftUI/Modules/Settings/ChangePassword/Test/UI/ChangePasswordUITests.swift index 4bf4f94ca0..5bdc3fddc2 100644 --- a/RiotSwiftUI/Modules/Settings/ChangePassword/Test/UI/ChangePasswordUITests.swift +++ b/RiotSwiftUI/Modules/Settings/ChangePassword/Test/UI/ChangePasswordUITests.swift @@ -43,6 +43,7 @@ class ChangePasswordUITests: MockScreenTest { func verifyAllEmpty() { XCTAssertTrue(app.staticTexts["titleLabel"].exists, "The title should be shown.") + XCTAssertTrue(app.staticTexts["passwordRequirementsLabel"].exists, "The password requirements label should be shown.") let oldPasswordTextField = app.secureTextFields["oldPasswordTextField"] XCTAssertTrue(oldPasswordTextField.exists, "The text field should be shown.") @@ -67,6 +68,7 @@ class ChangePasswordUITests: MockScreenTest { func verifyCannotSubmit() { XCTAssertTrue(app.staticTexts["titleLabel"].exists, "The title should be shown.") + XCTAssertTrue(app.staticTexts["passwordRequirementsLabel"].exists, "The password requirements label should be shown.") let oldPasswordTextField = app.secureTextFields["oldPasswordTextField"] XCTAssertTrue(oldPasswordTextField.exists, "The text field should be shown.") @@ -91,6 +93,7 @@ class ChangePasswordUITests: MockScreenTest { func verifyCanSubmit() { XCTAssertTrue(app.staticTexts["titleLabel"].exists, "The title should be shown.") + XCTAssertTrue(app.staticTexts["passwordRequirementsLabel"].exists, "The password requirements label should be shown.") let oldPasswordTextField = app.secureTextFields["oldPasswordTextField"] XCTAssertTrue(oldPasswordTextField.exists, "The text field should be shown.") @@ -115,6 +118,7 @@ class ChangePasswordUITests: MockScreenTest { func verifyCanSubmitAndSignoutAllDevicesChecked() { XCTAssertTrue(app.staticTexts["titleLabel"].exists, "The title should be shown.") + XCTAssertTrue(app.staticTexts["passwordRequirementsLabel"].exists, "The password requirements label should be shown.") let oldPasswordTextField = app.secureTextFields["oldPasswordTextField"] XCTAssertTrue(oldPasswordTextField.exists, "The text field should be shown.") diff --git a/RiotSwiftUI/Modules/Settings/ChangePassword/View/ChangePasswordScreen.swift b/RiotSwiftUI/Modules/Settings/ChangePassword/View/ChangePasswordScreen.swift index 4224dabfdd..ed7ca3dda9 100644 --- a/RiotSwiftUI/Modules/Settings/ChangePassword/View/ChangePasswordScreen.swift +++ b/RiotSwiftUI/Modules/Settings/ChangePassword/View/ChangePasswordScreen.swift @@ -97,9 +97,16 @@ struct ChangePasswordScreen: View { Text(VectorL10n.authenticationChoosePasswordSignoutAllDevices) .foregroundColor(theme.colors.secondaryContent) } - .padding(.top, 8) - .padding(.bottom, 16) .onTapGesture(perform: toggleSignoutAllDevices) + .padding(.top, 8) + + Text(viewModel.viewState.passwordRequirements) + .font(theme.fonts.body) + .multilineTextAlignment(.leading) + .foregroundColor(theme.colors.secondaryContent) + .accessibilityIdentifier("passwordRequirementsLabel") + .padding(.top, 8) + .padding(.bottom, 16) Button(action: submit) { Text(VectorL10n.save) From 44d54710ed311073f8d0e107fb14c36be72069ee Mon Sep 17 00:00:00 2001 From: ismailgulek Date: Fri, 10 Jun 2022 18:33:50 +0300 Subject: [PATCH 10/12] Fix tests --- .../Test/Unit/ChangePasswordViewModelTests.swift | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/RiotSwiftUI/Modules/Settings/ChangePassword/Test/Unit/ChangePasswordViewModelTests.swift b/RiotSwiftUI/Modules/Settings/ChangePassword/Test/Unit/ChangePasswordViewModelTests.swift index a3a04d0114..e279c6b530 100644 --- a/RiotSwiftUI/Modules/Settings/ChangePassword/Test/Unit/ChangePasswordViewModelTests.swift +++ b/RiotSwiftUI/Modules/Settings/ChangePassword/Test/Unit/ChangePasswordViewModelTests.swift @@ -29,14 +29,14 @@ class ChangePasswordViewModelTests: XCTestCase { XCTAssert(context.newPassword1.isEmpty, "The view model should start with an empty new password 1.") XCTAssert(context.newPassword2.isEmpty, "The view model should start with an empty new password 2.") XCTAssertFalse(context.viewState.canSubmit, "The view model should not be able to submit.") - XCTAssertTrue(context.signoutAllDevices, "The view model should start with sign out of all devices checked.") + XCTAssertFalse(context.signoutAllDevices, "The view model should start with sign out of all devices unchecked.") } @MainActor func testValidState() async { let viewModel = ChangePasswordViewModel(oldPassword: "12345678", newPassword1: "87654321", newPassword2: "87654321", - signoutAllDevices: false) + signoutAllDevices: true) let context = viewModel.context // Given a filled view model in valid state @@ -44,7 +44,7 @@ class ChangePasswordViewModelTests: XCTestCase { XCTAssertFalse(context.newPassword1.isEmpty, "The view model should start with an empty new password 1.") XCTAssertFalse(context.newPassword2.isEmpty, "The view model should start with an empty new password 2.") XCTAssertTrue(context.viewState.canSubmit, "The view model should be able to submit.") - XCTAssertFalse(context.signoutAllDevices, "Sign out of all devices should be unchecked.") + XCTAssertTrue(context.signoutAllDevices, "Sign out of all devices should be checked.") } } From 8f1eb8fd99c1e5fb0b561e80a6176afc3e3f5823 Mon Sep 17 00:00:00 2001 From: ismailgulek Date: Mon, 13 Jun 2022 00:15:53 +0300 Subject: [PATCH 11/12] Update strings --- Riot/Assets/en.lproj/Vector.strings | 8 ++++---- Riot/Generated/Strings.swift | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/Riot/Assets/en.lproj/Vector.strings b/Riot/Assets/en.lproj/Vector.strings index 6ffc16a165..e54b96cacc 100644 --- a/Riot/Assets/en.lproj/Vector.strings +++ b/Riot/Assets/en.lproj/Vector.strings @@ -597,7 +597,7 @@ Tap the + to start adding people."; "settings_add_email_address" = "Add email address"; "settings_phone_number" = "Phone"; "settings_add_phone_number" = "Add phone number"; -"settings_change_password" = "Change Matrix account password"; +"settings_change_password" = "Change password"; "settings_night_mode" = "Night Mode"; "settings_fail_to_update_profile" = "Fail to update profile"; "settings_three_pids_management_information_part1" = "Manage which email addresses or phone numbers you can use to log in or recover your account here. Control who can find you in "; @@ -685,9 +685,9 @@ Tap the + to start adding people."; "settings_analytics_and_crash_data" = "Send crash and analytics data"; "settings_enable_rageshake" = "Rage shake to report bug"; -"settings_old_password" = "old password"; -"settings_new_password" = "new password"; -"settings_confirm_password" = "confirm password"; +"settings_old_password" = "Old password"; +"settings_new_password" = "New password"; +"settings_confirm_password" = "Confirm password"; "settings_fail_to_update_password" = "Fail to update Matrix account password"; "settings_password_updated" = "Your Matrix account password has been updated"; diff --git a/Riot/Generated/Strings.swift b/Riot/Generated/Strings.swift index f1985bb8e4..88179a22de 100644 --- a/Riot/Generated/Strings.swift +++ b/Riot/Generated/Strings.swift @@ -6495,7 +6495,7 @@ public class VectorL10n: NSObject { public static func settingsCallsStunServerFallbackDescription(_ p1: String) -> String { return VectorL10n.tr("Vector", "settings_calls_stun_server_fallback_description", p1) } - /// Change Matrix account password + /// Change password public static var settingsChangePassword: String { return VectorL10n.tr("Vector", "settings_change_password") } @@ -6527,7 +6527,7 @@ public class VectorL10n: NSObject { public static var settingsConfirmMediaSizeDescription: String { return VectorL10n.tr("Vector", "settings_confirm_media_size_description") } - /// confirm password + /// Confirm password public static var settingsConfirmPassword: String { return VectorL10n.tr("Vector", "settings_confirm_password") } @@ -6947,7 +6947,7 @@ public class VectorL10n: NSObject { public static var settingsNewKeyword: String { return VectorL10n.tr("Vector", "settings_new_keyword") } - /// new password + /// New password public static var settingsNewPassword: String { return VectorL10n.tr("Vector", "settings_new_password") } @@ -6971,7 +6971,7 @@ public class VectorL10n: NSObject { public static var settingsNotifyMeFor: String { return VectorL10n.tr("Vector", "settings_notify_me_for") } - /// old password + /// Old password public static var settingsOldPassword: String { return VectorL10n.tr("Vector", "settings_old_password") } From e77fb6d0482487b704222881dc5ae9630f518676 Mon Sep 17 00:00:00 2001 From: ismailgulek Date: Mon, 13 Jun 2022 00:16:06 +0300 Subject: [PATCH 12/12] Remove sensitive data from logs --- .../Coordinator/ChangePasswordCoordinator.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/RiotSwiftUI/Modules/Settings/ChangePassword/Coordinator/ChangePasswordCoordinator.swift b/RiotSwiftUI/Modules/Settings/ChangePassword/Coordinator/ChangePasswordCoordinator.swift index 2c590a62ef..d582e87211 100644 --- a/RiotSwiftUI/Modules/Settings/ChangePassword/Coordinator/ChangePasswordCoordinator.swift +++ b/RiotSwiftUI/Modules/Settings/ChangePassword/Coordinator/ChangePasswordCoordinator.swift @@ -86,10 +86,10 @@ final class ChangePasswordCoordinator: Coordinator, Presentable { @MainActor private func setupViewModel() { changePasswordViewModel.callback = { [weak self] result in guard let self = self else { return } - MXLog.debug("[ChangePasswordCoordinator] ChangePasswordViewModel did complete with result: \(result).") - + switch result { case .submit(let oldPassword, let newPassword, let signoutAllDevices): + MXLog.debug("[ChangePasswordCoordinator] ChangePasswordViewModel did complete with result: submit.") self.changePassword(from: oldPassword, to: newPassword, signoutAllDevices: signoutAllDevices) } }