From 8af104a16e3961504e28de6242925db76cb10c17 Mon Sep 17 00:00:00 2001 From: Sale Djenic Date: Mon, 13 Sep 2021 13:51:47 +0200 Subject: [PATCH] feat(@desktop/onboarding): support (optionally) OS keychain to login password This feature works for MacOs only, for now. On login, whether new or already created user may select between options: "Store" - store password to the Keychain "Not now" - don't store it now, but ask next time again "Never" - don't store them ever and don't ask again Selected preference may be changed later in: `ProfileSettings > Privacy and security > Store pass to Keychain` On the next app run, if `Store` was selected, a user will be asked to confirm his identity using Touch Id in order to log in the app. If any error happens he will be able to login using password. Fixes: #2675 --- Makefile | 2 +- src/app/login/core.nim | 7 +- src/app/login/view.nim | 61 +++++++++++++- src/app/onboarding/view.nim | 10 ++- .../Profile/Sections/PrivacyContainer.qml | 30 +++++++ .../StoreToKeychainSelectionModal.qml | 77 ++++++++++++++++++ ui/imports/Constants.qml | 4 + ui/main.qml | 55 +++++++++++++ ui/onboarding/CreatePasswordModal.qml | 59 +++++++++----- ui/onboarding/Login.qml | 80 ++++++++++++++----- ui/shared/ConfirmationDialog.qml | 14 ++++ ui/shared/status/StatusRadioButtonRow.qml | 2 +- vendor/DOtherSide | 2 +- vendor/nimqml | 2 +- 14 files changed, 356 insertions(+), 49 deletions(-) create mode 100644 ui/app/AppLayouts/Profile/Sections/StoreToKeychainSelectionModal.qml diff --git a/Makefile b/Makefile index 37bcad331e..cb1252775e 100644 --- a/Makefile +++ b/Makefile @@ -129,7 +129,7 @@ ifneq ($(detected_OS),Windows) QT5_LIBDIR := $(QTDIR)/lib # some manually installed Qt5 instances have wrong paths in their *.pc files, so we pass the right one to the linker here ifeq ($(detected_OS),Darwin) - NIM_PARAMS += -L:"-framework Foundation -framework Security -framework IOKit -framework CoreServices" + NIM_PARAMS += -L:"-framework Foundation -framework Security -framework IOKit -framework CoreServices -framework LocalAuthentication" # Fix for failures due to 'can't allocate code signature data for' NIM_PARAMS += --passL:"-headerpad_max_install_names" NIM_PARAMS += --passL:"-F$(QT5_LIBDIR)" diff --git a/src/app/login/core.nim b/src/app/login/core.nim index e39f07fda5..dbc12ed538 100644 --- a/src/app/login/core.nim +++ b/src/app/login/core.nim @@ -1,19 +1,22 @@ import NimQml, chronicles, options, std/wrapnils import status/[signals, status] import status/types/[account, rpc_response] +import ../../app_service/[main] import view import eventemitter import ../../constants type LoginController* = ref object status*: Status + appService: AppService view*: LoginView variant*: QVariant -proc newController*(status: Status): LoginController = +proc newController*(status: Status, appService: AppService): LoginController = result = LoginController() result.status = status - result.view = newLoginView(status) + result.appService = appService + result.view = newLoginView(status, appService) result.variant = newQVariant(result.view) proc delete*(self: LoginController) = diff --git a/src/app/login/view.nim b/src/app/login/view.nim index 1a7101c6d6..7c05c1eecc 100644 --- a/src/app/login/view.nim +++ b/src/app/login/view.nim @@ -2,8 +2,15 @@ import NimQml, Tables, json, nimcrypto, strformat, json_serialization, chronicle import status/accounts as AccountModel import status/types/[account, rpc_response] import status/[status] +import ../../app_service/[main] import ../onboarding/views/account_info +const ERROR_TYPE_AUTHENTICATION = "authentication" +const ERROR_TYPE_KEYCHAIN = "keychain" + +logScope: + topics = "login-model" + type AccountRoles {.pure.} = enum Username = UserRole + 1 @@ -15,23 +22,32 @@ type QtObject: type LoginView* = ref object of QAbstractListModel status: Status + appService: AppService accounts: seq[NodeAccount] currentAccount*: AccountInfoView isCurrentFlow*: bool + keychainManager*: StatusKeychainManager proc setup(self: LoginView) = self.QAbstractListModel.setup + self.keychainManager = newStatusKeychainManager("StatusDesktop", "authenticate you") + signalConnect(self.keychainManager, "success(QString)", self, + "onKeychainManagerSuccess(QString)", 2) + signalConnect(self.keychainManager, "error(QString, int, QString)", self, + "onKeychainManagerError(QString, int, QString)", 2) proc delete*(self: LoginView) = self.currentAccount.delete self.accounts = @[] + self.keychainManager.delete self.QAbstractListModel.delete - proc newLoginView*(status: Status): LoginView = + proc newLoginView*(status: Status, appService: AppService): LoginView = new(result, delete) result.accounts = @[] result.currentAccount = newAccountInfoView() result.status = status + result.appService = appService result.isCurrentFlow = false result.setup @@ -139,3 +155,46 @@ QtObject: read = isCurrentFlow write = setCurrentFlow notify = currentFlowChanged + + proc storePassword*(self: LoginView, username: string, password: string) {.slot.} = + # The following check is commented out only because we maintaing a single file + # using two QSettings instances, one created in qml and one here in nim part. + # Once we move to maintain settings file only via nim part this condition need + # to be applied. The reason why it's commented out is, if you change something + # from the qml part and try in a next step to read that property from the nim + # part, that property won't be read correctly cause even 'sync' method is called + # we need to wait untill the event loop ends, cause data are flushed at regular + # intervals to the file. + # let value = self.appService.localSettingsService.getValue( + # LS_KEY_STORE_TO_KEYCHAIN).stringVal + # if (value == LS_VALUE_STORE): + + if (username.len > 0): + self.keychainManager.storeDataAsync(username, password) + + proc tryToObtainPassword*(self: LoginView) {.slot.} = + let value = self.appService.localSettingsService.getValue( + LS_KEY_STORE_TO_KEYCHAIN).stringVal + if (value == LS_VALUE_STORE): + self.keychainManager.readDataAsync(self.currentAccount.username) + + proc obtainingPasswordError*(self:LoginView, errorDescription: string) {.signal.} + proc obtainingPasswordSuccess*(self:LoginView, password: string) {.signal.} + + proc onKeychainManagerError*(self: LoginView, errType: string, code: int, + errorDescription: string) {.slot.} = + ## This slot is called in case an error occured while we're dealing with + ## KeychainManager. So far we're just logging the error. + info "KeychainManager stopped: ", msg = code, errorDescription + if (errType == ERROR_TYPE_AUTHENTICATION): + return + + # We are notifying user only about keychain errors. + self.appService.localSettingsService.removeValue(LS_KEY_STORE_TO_KEYCHAIN) + self.obtainingPasswordError(errorDescription) + + proc onKeychainManagerSuccess*(self: LoginView, data: string) {.slot.} = + ## This slot is called in case a password is successfully retrieved from the + ## Keychain. In this case @data contains required password. + self.obtainingPasswordSuccess(data) + diff --git a/src/app/onboarding/view.nim b/src/app/onboarding/view.nim index af03ff6624..7aad09fb8e 100644 --- a/src/app/onboarding/view.nim +++ b/src/app/onboarding/view.nim @@ -1,7 +1,8 @@ import NimQml, Tables, json, nimcrypto, strformat, json_serialization, strutils import status/accounts as AccountModel import status/[status, wallet] -import status/types/[account, rpc_response] +import status/types/[rpc_response] +import status/types/account as status_account_type import views/account_info type @@ -99,8 +100,13 @@ QtObject: result = self.status.wallet.validateMnemonic(mnemonic.strip()) proc storeDerivedAndLogin(self: OnboardingView, password: string): string {.slot.} = + # In this moment we're sure that new account will be logged in, and emit signal. + let genAcc = self.currentAccount.account + let acc = Account(name: genAcc.name, keyUid: genAcc.keyUid, identicon: genAcc.identicon, identityImage: genAcc.identityImage) + self.status.events.emit("currentAccountUpdated", status_account_type.AccountArgs(account: acc)) + try: - result = self.status.accounts.storeDerivedAndLogin(self.status.fleet.config, self.currentAccount.account, password).toJson + result = self.status.accounts.storeDerivedAndLogin(self.status.fleet.config, genAcc, password).toJson except StatusGoException as e: var msg = e.msg if e.msg.contains("account already exists"): diff --git a/ui/app/AppLayouts/Profile/Sections/PrivacyContainer.qml b/ui/app/AppLayouts/Profile/Sections/PrivacyContainer.qml index 1ae6716bb1..0984fa2d8c 100644 --- a/ui/app/AppLayouts/Profile/Sections/PrivacyContainer.qml +++ b/ui/app/AppLayouts/Profile/Sections/PrivacyContainer.qml @@ -5,6 +5,7 @@ import QtGraphicalEffects 1.13 import "../../../../imports" import "../../../../shared" import "../../../../shared/status" +import "../../../../onboarding/" as OnboardingComponents Item { id: privacyContainer @@ -45,6 +46,35 @@ Item { } } + StatusSettingsLineButton { + text: qsTr("Store pass to Keychain") + visible: Qt.platform.os == "osx" // For now, this is available only on MacOS + currentValue: { + let value = appSettings.storeToKeychain + if(value == Constants.storeToKeychainValueStore) + return qsTr("Store") + + if(value == Constants.storeToKeychainValueNever) + return qsTr("Never") + + return qsTr("Not now") + } + onClicked: openPopup(storeToKeychainSelectionModal) + + Component { + id: storePasswordModal + OnboardingComponents.CreatePasswordModal { + storingPasswordModal: true + height: 350 + } + } + + Component { + id: storeToKeychainSelectionModal + StoreToKeychainSelectionModal {} + } + } + BackupSeedModal { id: backupSeedModal } diff --git a/ui/app/AppLayouts/Profile/Sections/StoreToKeychainSelectionModal.qml b/ui/app/AppLayouts/Profile/Sections/StoreToKeychainSelectionModal.qml new file mode 100644 index 0000000000..b700a22b30 --- /dev/null +++ b/ui/app/AppLayouts/Profile/Sections/StoreToKeychainSelectionModal.qml @@ -0,0 +1,77 @@ +import QtQuick 2.13 +import QtQuick.Controls 2.13 +import "../../../../imports" +import "../../../../shared" +import "../../../../shared/status" + +ModalPopup { + id: popup + + title: qsTr("Store pass to Keychain") + + onClosed: { + destroy() + } + + Column { + anchors.top: parent.top + anchors.bottom: parent.bottom + anchors.left: parent.left + anchors.right: parent.right + anchors.rightMargin: Style.current.padding + anchors.leftMargin: Style.current.padding + + spacing: 0 + + ButtonGroup { + id: openLinksWithGroup + } + + StatusRadioButtonRow { + text: qsTr("Store") + buttonGroup: openLinksWithGroup + checked: appSettings.storeToKeychain === Constants.storeToKeychainValueStore + onRadioCheckedChanged: { + if (checked && appSettings.storeToKeychain !== Constants.storeToKeychainValueStore) { + var storePassPopup = openPopup(storePasswordModal) + if(storePassPopup) + { + storePassPopup.closed.connect(function(){ + if (appSettings.storeToKeychain === Constants.storeToKeychainValueStore) + popup.close() + else if (appSettings.storeToKeychain === Constants.storeToKeychainValueNotNow) + notNowBtn.checked = true + else if (appSettings.storeToKeychain === Constants.storeToKeychainValueNever) + neverBtn.checked = true + }) + } + } + } + } + + StatusRadioButtonRow { + id: notNowBtn + text: qsTr("Not now") + buttonGroup: openLinksWithGroup + checked: appSettings.storeToKeychain === Constants.storeToKeychainValueNotNow || + appSettings.storeToKeychain === "" + onRadioCheckedChanged: { + if (checked) { + appSettings.storeToKeychain = Constants.storeToKeychainValueNotNow + } + } + } + + StatusRadioButtonRow { + id: neverBtn + text: qsTr("Never") + buttonGroup: openLinksWithGroup + checked: appSettings.storeToKeychain === Constants.storeToKeychainValueNever + onRadioCheckedChanged: { + if (checked) { + appSettings.storeToKeychain = Constants.storeToKeychainValueNever + } + } + } + } +} diff --git a/ui/imports/Constants.qml b/ui/imports/Constants.qml index df6e3e4645..4d6943b1aa 100644 --- a/ui/imports/Constants.qml +++ b/ui/imports/Constants.qml @@ -163,6 +163,10 @@ QtObject { readonly property string ens_connected: "connected" readonly property string ens_connected_dkey: "connected-different-key" + readonly property string storeToKeychainValueStore: "store" + readonly property string storeToKeychainValueNotNow: "notNow" + readonly property string storeToKeychainValueNever: "never" + //% "(edited)" readonly property string editLabel: ` ` + qsTrId("-edited-") + `` diff --git a/ui/main.qml b/ui/main.qml index d1e75d7c9b..0f699e02bd 100644 --- a/ui/main.qml +++ b/ui/main.qml @@ -41,6 +41,7 @@ StatusWindow { Settings { id: appSettings fileName: profileModel.settingsFile + property string storeToKeychain: "" property var chatSplitView property var walletSplitView @@ -277,6 +278,60 @@ StatusWindow { } } + function checkForStoringPassToKeychain(username, password, clearStoredValue) { + if(Qt.platform.os == "osx") + { + if(clearStoredValue) + { + appSettings.storeToKeychain = "" + } + + if(appSettings.storeToKeychain === "" || + appSettings.storeToKeychain === Constants.storeToKeychainValueNotNow) + { + storeToKeychainConfirmationPopup.password = password + storeToKeychainConfirmationPopup.username = username + storeToKeychainConfirmationPopup.open() + } + } + } + + ConfirmationDialog { + id: storeToKeychainConfirmationPopup + property string password: "" + property string username: "" + height: 200 + confirmationText: qsTr("Would you like to store password to the Keychain?") + showRejectButton: true + showCancelButton: true + confirmButtonLabel: qsTr("Store") + rejectButtonLabel: qsTr("Not now") + cancelButtonLabel: qsTr("Never") + + function finish() + { + password = "" + username = "" + storeToKeychainConfirmationPopup.close() + } + + onConfirmButtonClicked: { + appSettings.storeToKeychain = Constants.storeToKeychainValueStore + loginModel.storePassword(username, password) + finish() + } + + onRejectButtonClicked: { + appSettings.storeToKeychain = Constants.storeToKeychainValueNotNow + finish() + } + + onCancelButtonClicked: { + appSettings.storeToKeychain = Constants.storeToKeychainValueNever + finish() + } + } + DSM.StateMachine { id: stateMachine initialState: onboardingState diff --git a/ui/onboarding/CreatePasswordModal.qml b/ui/onboarding/CreatePasswordModal.qml index 30806c6496..ea496e35b8 100644 --- a/ui/onboarding/CreatePasswordModal.qml +++ b/ui/onboarding/CreatePasswordModal.qml @@ -11,10 +11,13 @@ ModalPopup { property bool repeatPasswordFieldValid: false property string passwordValidationError: "" property string repeatPasswordValidationError: "" + property bool storingPasswordModal: false id: popup - //% "Create a password" - title: qsTrId("intro-wizard-title-alt4") + title: storingPasswordModal? + qsTr("Store password") : + //% "Create a password" + qsTrId("intro-wizard-title-alt4") height: 500 onOpened: { @@ -27,9 +30,11 @@ ModalPopup { anchors.rightMargin: 56 anchors.leftMargin: 56 anchors.top: parent.top - anchors.topMargin: 88 - //% "New password..." - placeholderText: qsTrId("new-password...") + anchors.topMargin: storingPasswordModal? Style.current.xlPadding : 88 + placeholderText: storingPasswordModal? + qsTr("Current password...") : + //% "New password..." + qsTrId("new-password...") textField.echoMode: TextInput.Password onTextChanged: { [firstPasswordFieldValid, passwordValidationError] = @@ -79,6 +84,7 @@ ModalPopup { } StyledText { + visible: !storingPasswordModal //% "At least 6 characters. You will use this password to unlock status on this device & sign transactions." text: qsTrId("at-least-6-characters-you-will-use-this-password-to-unlock-status-on-this-device-sign-transactions.") wrapMode: Text.WordWrap @@ -103,8 +109,11 @@ ModalPopup { anchors.topMargin: Style.current.padding anchors.right: parent.right state: loading ? "pending" : "default" - //% "Create password" - text: qsTrId("create-password") + + text: storingPasswordModal? + qsTr("Store password") : + //% "Create password" + qsTrId("create-password") enabled: firstPasswordFieldValid && repeatPasswordFieldValid && !loading @@ -146,23 +155,29 @@ ModalPopup { } onClicked: { - loading = true - loginModel.isCurrentFlow = false; - onboardingModel.isCurrentFlow = true; - const result = onboardingModel.storeDerivedAndLogin(repeatPasswordField.text); - const error = JSON.parse(result).error - if (error) { - importError.text += error - return importError.open() + if (storingPasswordModal) + { + appSettings.storeToKeychain = Constants.storeToKeychainValueStore + loginModel.storePassword(profileModel.profile.username, repeatPasswordField.text) + popup.close() + } + else + { + loading = true + loginModel.isCurrentFlow = false; + onboardingModel.isCurrentFlow = true; + const result = onboardingModel.storeDerivedAndLogin(repeatPasswordField.text); + const error = JSON.parse(result).error + if (error) { + importError.text += error + return importError.open() + } + onboardingModel.firstTimeLogin = true + + applicationWindow.checkForStoringPassToKeychain(onboardingModel.currentAccount.username, + repeatPasswordField.text, true) } - onboardingModel.firstTimeLogin = true } } } } - -/*##^## -Designer { - D{i:0;formeditorColor:"#ffffff";height:500;width:400} -} -##^##*/ diff --git a/ui/onboarding/Login.qml b/ui/onboarding/Login.qml index a4c2c052a3..a5e2a39691 100644 --- a/ui/onboarding/Login.qml +++ b/ui/onboarding/Login.qml @@ -21,8 +21,63 @@ Item { onboardingModel.isCurrentFlow = !isLogin; } + function doLogin(password) { + if (loading || password.length === 0) + return + + setCurrentFlow(true); + loading = true + loginModel.login(password) + applicationWindow.checkForStoringPassToKeychain(loginModel.currentAccount.username, password, false) + txtPassword.textField.clear() + } + + function resetLogin() { + if(appSettings.storeToKeychain === Constants.storeToKeychainValueStore) + { + connection.enabled = true + txtPassword.visible = false + loginModel.tryToObtainPassword() + } + else + { + txtPassword.visible = true + txtPassword.forceActiveFocus(Qt.MouseFocusReason) + } + } + Component.onCompleted: { - txtPassword.forceActiveFocus(Qt.MouseFocusReason) + resetLogin() + } + + Connections{ + id: connection + target: loginModel + + onObtainingPasswordError: { + enabled = false + obtainingPasswordErrorNotification.confirmationText = errorDescription + obtainingPasswordErrorNotification.open() + } + + onObtainingPasswordSuccess: { + enabled = false + doLogin(password) + } + } + + ConfirmationDialog { + id: obtainingPasswordErrorNotification + height: 270 + confirmButtonLabel: qsTr("Ok") + + onConfirmButtonClicked: { + close() + } + + onClosed: { + txtPassword.visible = true + } } Item { @@ -60,6 +115,7 @@ Item { id: selectAnotherAccountModal onAccountSelect: function (index) { loginModel.setCurrentAccount(index) + resetLogin() } onOpenModalClick: function () { setCurrentFlow(true); @@ -127,7 +183,7 @@ Item { textField.echoMode: TextInput.Password textField.focus: true Keys.onReturnPressed: { - submitBtn.clicked() + doLogin(textField.text) } onTextEdited: { errMsg.visible = false @@ -143,18 +199,12 @@ Item { icon.width: 18 icon.height: 14 opacity: (loading || txtPassword.text.length > 0) ? 1 : 0 - anchors.left: txtPassword.right + anchors.left: txtPassword.visible? txtPassword.right : changeAccountBtn.right anchors.leftMargin: (loading || txtPassword.text.length > 0) ? Style.current.padding : Style.current.smallPadding - anchors.verticalCenter: txtPassword.verticalCenter + anchors.verticalCenter: txtPassword.visible? txtPassword.verticalCenter : changeAccountBtn.verticalCenter state: loading ? "pending" : "default" onClicked: { - if (loading) { - return; - } - setCurrentFlow(true); - loading = true - loginModel.login(txtPassword.textField.text) - txtPassword.textField.clear() + doLogin(txtPassword.textField.text) } // https://www.figma.com/file/BTS422M9AkvWjfRrXED3WC/%F0%9F%91%8B-Onboarding%E2%8E%9CDesktop?node-id=6%3A0 @@ -194,7 +244,7 @@ Item { id: generateKeysLinkText //% "Generate new keys" text: qsTrId("generate-new-keys") - anchors.top: txtPassword.bottom + anchors.top: txtPassword.visible? txtPassword.bottom : changeAccountBtn.bottom anchors.topMargin: 16 anchors.horizontalCenter: parent.horizontalCenter font.pixelSize: 13 @@ -219,9 +269,3 @@ Item { } } } - -/*##^## -Designer { - D{i:0;autoSize:true;formeditorColor:"#ffffff";formeditorZoom:0.75;height:480;width:640} -} -##^##*/ diff --git a/ui/shared/ConfirmationDialog.qml b/ui/shared/ConfirmationDialog.qml index a908870742..b05b900ce0 100644 --- a/ui/shared/ConfirmationDialog.qml +++ b/ui/shared/ConfirmationDialog.qml @@ -14,11 +14,14 @@ StatusModal { property Popup parentPopup property var value property var executeConfirm + property var executeReject property var executeCancel property string btnType: "warn" property string confirmButtonLabel: qsTr("Confirm") + property string rejectButtonLabel: qsTr("Reject") property string cancelButtonLabel: qsTr("Cancel") property string confirmationText: qsTr("Are you sure you want to do this?") + property bool showRejectButton: false property bool showCancelButton: false property alias checkbox: checkbox @@ -27,6 +30,7 @@ StatusModal { focus: visible signal confirmButtonClicked() + signal rejectButtonClicked() signal cancelButtonClicked() @@ -83,6 +87,16 @@ StatusModal { confirmationDialog.cancelButtonClicked() } }, + StatusFlatButton { + visible: showRejectButton + text: confirmationDialog.rejectButtonLabel + onClicked: { + if (executeReject && typeof executeReject === "function") { + executeReject() + } + confirmationDialog.rejectButtonClicked() + } + }, StatusButton { id: confirmButton type: { diff --git a/ui/shared/status/StatusRadioButtonRow.qml b/ui/shared/status/StatusRadioButtonRow.qml index 1556bc2302..e467b6994b 100644 --- a/ui/shared/status/StatusRadioButtonRow.qml +++ b/ui/shared/status/StatusRadioButtonRow.qml @@ -7,7 +7,7 @@ import "." Rectangle { property alias text: textElement.text property var buttonGroup - property bool checked: false + property alias checked: radioButton.checked property bool isHovered: false signal radioCheckedChanged(checked: bool) diff --git a/vendor/DOtherSide b/vendor/DOtherSide index 0f8ed95fc7..b1e4d3b686 160000 --- a/vendor/DOtherSide +++ b/vendor/DOtherSide @@ -1 +1 @@ -Subproject commit 0f8ed95fc7a47e4d3efb218ef961d36e60610cb3 +Subproject commit b1e4d3b68629a101e21cfdfd448ef6f54364f235 diff --git a/vendor/nimqml b/vendor/nimqml index 00ee27ca52..4351b9a61f 160000 --- a/vendor/nimqml +++ b/vendor/nimqml @@ -1 +1 @@ -Subproject commit 00ee27ca52bcf5216c0de0e19e594ddfc1790452 +Subproject commit 4351b9a61f7ff2b6798cadd4151b34b3c0670a56