Philipp Hörist pushed to branch master at gajim / gajim

Commits: cb89f598 by lovetox at 2022-07-05T21:37:01+02:00 refactor: Remove side effects of password module

Fixes: #11032

- - - - -

5 changed files:

- gajim/common/application.py - gajim/common/passwords.py - gajim/gtk/accounts.py - gajim/gtk/features.py - gajim/gtk/password_dialog.py

Changes:

===================================== gajim/common/application.py ===================================== @@ -36,6 +36,7 @@ from gajim.common import ged from gajim.common import configpaths from gajim.common import logging_helpers +from gajim.common import passwords from gajim.common.dbus import logind from gajim.common.events import AccountDisonnected from gajim.common.events import AllowGajimUpdateCheck @@ -61,6 +62,8 @@ def _init_core(self) -> None: app.detect_dependencies() configpaths.create_paths()

+ passwords.init() + app.settings = Settings() app.settings.init()

===================================== gajim/common/passwords.py ===================================== @@ -23,6 +23,7 @@ from __future__ import annotations

from typing import Optional +from typing import cast

import logging

@@ -31,21 +32,34 @@ from gajim.common import app

__all__ = [ + 'init', + 'is_keyring_available', 'get_password', 'save_password', 'delete_password' ]

-log = logging.getLogger('gajim.password') +log = logging.getLogger('gajim.c.passwords')

-backends = keyring.backend.get_all_keyring() -for backend in backends: - log.info('Found keyring backend: %s', backend)

-keyring_backend = keyring.get_keyring() -log.info('Select %s backend', keyring_backend) +class Interface: + def __init__(self): + self.backend = cast(keyring.backend.KeyringBackend, None) + self._is_keyring_available = False

-KEYRING_AVAILABLE = keyring.core.recommended(keyring_backend) + @property + def is_keyring_available(self): + return self._is_keyring_available + + def init(self) -> None: + backends = keyring.backend.get_all_keyring() + for backend in backends: + log.info('Found keyring backend: %s', backend) + + self.backend = keyring.get_keyring() + log.info('Select %s backend', self.backend) + + self._is_keyring_available = keyring.core.recommended(self.backend)

class SecretPasswordStorage: @@ -55,13 +69,13 @@ class SecretPasswordStorage:

@staticmethod def save_password(account_name: str, password: str) -> bool: - if not KEYRING_AVAILABLE: + if not is_keyring_available(): log.warning('No recommended keyring backend available.' 'Passwords cannot be stored.') return True try: log.info('Save password to keyring') - keyring_backend.set_password('gajim', account_name, password) + _interface.backend.set_password('gajim', account_name, password) return True except Exception: log.exception('Save password failed') @@ -69,25 +83,28 @@ def save_password(account_name: str, password: str) -> bool:

@staticmethod def get_password(account_name: str) -> Optional[str]: - log.info('Request password from keyring') - if not KEYRING_AVAILABLE: + if not is_keyring_available(): return + + log.info('Request password from keyring') + try: # For security reasons remove clear-text password ConfigPasswordStorage.delete_password(account_name) - return keyring_backend.get_password('gajim', account_name) + return _interface.backend.get_password('gajim', account_name) except Exception: log.exception('Request password failed') return

@staticmethod def delete_password(account_name: str) -> None: - log.info('Remove password from keyring') - if not KEYRING_AVAILABLE: + if not is_keyring_available(): return

+ log.info('Remove password from keyring') + try: - return keyring_backend.delete_password('gajim', account_name) + return _interface.backend.delete_password('gajim', account_name) except keyring.errors.PasswordDeleteError as error: log.warning('Removing password failed: %s', error) except Exception: @@ -113,6 +130,14 @@ def delete_password(account_name: str) -> None: app.settings.set_account_setting(account_name, 'password', '')

+def init() -> None: + _interface.init() + + +def is_keyring_available() -> bool: + return _interface.is_keyring_available + + def get_password(account_name: str) -> Optional[str]: if app.settings.get('use_keyring'): return SecretPasswordStorage.get_password(account_name) @@ -138,3 +163,6 @@ def delete_password(account_name: str) -> None: if app.settings.get('use_keyring'): return SecretPasswordStorage.delete_password(account_name) return ConfigPasswordStorage.delete_password(account_name) + + +_interface = Interface()

===================================== gajim/gtk/accounts.py ===================================== @@ -1084,7 +1084,7 @@ def __init__(self, account: str, parent: Gtk.Window) -> None: SettingType.ACCOUNT_CONFIG, 'savepass', enabled_func=(lambda: not app.settings.get('use_keyring') or - passwords.KEYRING_AVAILABLE)), + passwords.is_keyring_available())),

Setting(SettingKind.CHANGEPASSWORD, _('Change Password'),

===================================== gajim/gtk/features.py ===================================== @@ -29,6 +29,7 @@ from gi.repository import Gdk

from gajim.common import app +from gajim.common import passwords from gajim.common.i18n import _

@@ -125,7 +126,7 @@ def _get_features(self) -> list[Feature]: _('No additional requirements'), notification_sounds_enabled), Feature(_('Secure Password Storage'), - self._some_keyring_available(), + passwords.is_keyring_available(), _('Enables Gajim to store Passwords securely instead of ' 'storing them in plaintext'), _('Requires: gnome-keyring or kwallet'), @@ -148,11 +149,6 @@ def _get_features(self) -> list[Feature]: None) ]

- @staticmethod - def _some_keyring_available() -> bool: - from gajim.common import passwords - return passwords.KEYRING_AVAILABLE - @staticmethod def _idle_available() -> bool: from gajim.common import idle

===================================== gajim/gtk/password_dialog.py ===================================== @@ -18,10 +18,9 @@ from gi.repository import Gdk

from gajim.common import app +from gajim.common import passwords from gajim.common.events import PasswordRequired from gajim.common.i18n import _ -from gajim.common.passwords import save_password -from gajim.common.passwords import KEYRING_AVAILABLE

from .builder import get_builder

@@ -69,9 +68,12 @@ def _process_event(self) -> None: 'jid': own_jid, 'account': account_name}) self._ui.save_pass_checkbutton.show() + + is_keyring_available = passwords.is_keyring_available() self._ui.save_pass_checkbutton.set_sensitive( - not app.settings.get('use_keyring') or KEYRING_AVAILABLE) - if not KEYRING_AVAILABLE: + not app.settings.get('use_keyring') or + is_keyring_available) + if not is_keyring_available: self._ui.keyring_hint.show()

def _on_ok(self, _button: Gtk.Button) -> None: @@ -81,7 +83,7 @@ def _on_ok(self, _button: Gtk.Button) -> None: self.account, 'savepass', self._ui.save_pass_checkbutton.get_active()) - save_password(self.account, password) + passwords.save_password(self.account, password) self._event.on_password(password) self.destroy()

View it on GitLab: https://dev.gajim.org/gajim/gajim/-/commit/cb89f598e4cfc68a17e4de5f86d66de0c...