From b36e27a7f20cbe70024625d6583daa3827d97f49 Mon Sep 17 00:00:00 2001 From: Sven Sager Date: Tue, 10 Jan 2023 23:23:55 +0100 Subject: [PATCH] Clean up keyring when a connection is deleted --- src/revpicommander/helper.py | 4 ++++ src/revpicommander/revpiplclist.py | 21 +++++++++++++++++++++ src/revpicommander/sshauth.py | 19 ++++++++++++++++--- 3 files changed, 41 insertions(+), 3 deletions(-) diff --git a/src/revpicommander/helper.py b/src/revpicommander/helper.py index db60bed..d177dc5 100644 --- a/src/revpicommander/helper.py +++ b/src/revpicommander/helper.py @@ -62,6 +62,7 @@ class RevPiSettings: self.ssh_use_tunnel = True self.ssh_port = 22 self.ssh_user = "pi" + self.ssh_saved_password = False self.last_dir_upload = "." self.last_file_upload = "." @@ -96,6 +97,7 @@ class RevPiSettings: self.ssh_use_tunnel = self._settings.value("ssh_use_tunnel", True, type=bool) self.ssh_port = self._settings.value("ssh_port", 22, type=int) self.ssh_user = self._settings.value("ssh_user", "pi", type=str) + self.ssh_saved_password = self._settings.value("ssh_saved_password", False, type=bool) self.last_dir_upload = self._settings.value("last_dir_upload", ".", type=str) self.last_file_upload = self._settings.value("last_file_upload", ".", type=str) @@ -172,6 +174,7 @@ class RevPiSettings: self._settings.setValue("ssh_use_tunnel", self.ssh_use_tunnel) self._settings.setValue("ssh_port", self.ssh_port) self._settings.setValue("ssh_user", self.ssh_user) + self._settings.setValue("ssh_saved_password", self.ssh_saved_password) self._settings.setValue("last_dir_upload", self.last_dir_upload) self._settings.setValue("last_file_upload", self.last_file_upload) @@ -347,6 +350,7 @@ class ConnectionManager(QtCore.QThread): revpi_settings.address, revpi_settings.ssh_port ) + revpi_settings.ssh_saved_password = diag_ssh_auth.in_keyring try: ssh_tunnel_port = ssh_tunnel_server.connect_by_credentials(ssh_user, ssh_pass) break diff --git a/src/revpicommander/revpiplclist.py b/src/revpicommander/revpiplclist.py index f509a67..3e2ac7d 100644 --- a/src/revpicommander/revpiplclist.py +++ b/src/revpicommander/revpiplclist.py @@ -6,7 +6,9 @@ __license__ = "GPLv3" from enum import IntEnum +import keyring from PyQt5 import QtCore, QtGui, QtWidgets +from keyring.errors import KeyringError from . import helper from . import proginit as pi @@ -29,6 +31,7 @@ class RevPiPlcList(QtWidgets.QDialog, Ui_diag_connections): self.__current_item = QtWidgets.QTreeWidgetItem() # type: QtWidgets.QTreeWidgetItem self.changes = True + self._keyring_cleanup_id_user = [] self.tre_connections.setColumnWidth(0, 250) self.lbl_port.setText(self.lbl_port.text().format(self.__default_port)) @@ -79,6 +82,18 @@ class RevPiPlcList(QtWidgets.QDialog, Ui_diag_connections): def accept(self) -> None: pi.logger.debug("RevPiPlcList.accept") + for internal_id, ssh_user in self._keyring_cleanup_id_user: + service_name = "{0}.{1}_{2}".format( + helper.settings.applicationName(), + helper.settings.organizationName(), + internal_id + ) + try: + # Remove information from os keyring, which we collected on_btn_delete_clicked + keyring.delete_password(service_name, ssh_user) + except KeyringError as e: + pi.logger.error(e) + helper.settings.remove("connections") for i in range(self.tre_connections.topLevelItemCount()): @@ -242,6 +257,12 @@ class RevPiPlcList(QtWidgets.QDialog, Ui_diag_connections): """Remove selected entry.""" item = self.tre_connections.currentItem() if item and item.type() == NodeType.CON: + + revpi_settings = item.data(0, WidgetData.revpi_settings) # type: RevPiSettings + if revpi_settings.ssh_saved_password: + # Cleans up keyring in save function + self._keyring_cleanup_id_user.append((revpi_settings.internal_id, revpi_settings.ssh_user)) + dir_node = item.parent() if dir_node: dir_node.removeChild(item) diff --git a/src/revpicommander/sshauth.py b/src/revpicommander/sshauth.py index 23baaa0..2ee546a 100644 --- a/src/revpicommander/sshauth.py +++ b/src/revpicommander/sshauth.py @@ -34,6 +34,7 @@ class SSHAuth(QtWidgets.QDialog, Ui_diag_sshauth): super(SSHAuth, self).__init__(parent) self.setupUi(self) + self._in_keyring = False self._service_name = service_name self.cbx_save_password.setVisible(bool(service_name)) self.txt_username.setText(user_name) @@ -46,6 +47,7 @@ class SSHAuth(QtWidgets.QDialog, Ui_diag_sshauth): keyring.set_password(self._service_name, self.username, self.password) except KeyringError as e: log.error(e) + self._in_keyring = False QtWidgets.QMessageBox.warning( self, self.tr("Could not save password"), self.tr( "Could not save password to operating systems password save.\n\n" @@ -54,6 +56,9 @@ class SSHAuth(QtWidgets.QDialog, Ui_diag_sshauth): "This is not an error of RevPi Commander." ) ) + else: + self._in_keyring = True + super().accept() def exec(self) -> int: @@ -62,11 +67,14 @@ class SSHAuth(QtWidgets.QDialog, Ui_diag_sshauth): if self._service_name: try: saved_password = keyring.get_password(self._service_name, self.username) - if saved_password: - self.txt_password.setText(saved_password) - return QtWidgets.QDialog.Accepted except KeyringError as e: log.error(e) + self._in_keyring = False + else: + if saved_password: + self._in_keyring = True + self.txt_password.setText(saved_password) + return QtWidgets.QDialog.Accepted return super().exec() @@ -80,6 +88,11 @@ class SSHAuth(QtWidgets.QDialog, Ui_diag_sshauth): except KeyringError as e: log.error(e) + @property + def in_keyring(self) -> bool: + """True, if password is in keyring.""" + return self._in_keyring + @property def password(self) -> str: """Get the saved or entered password."""