From cb89f598e4cfc68a17e4de5f86d66de0c1a414e8 Mon Sep 17 00:00:00 2001
From: lovetox <philipp@hoerist.com>
Date: Tue, 5 Jul 2022 21:36:57 +0200
Subject: [PATCH] refactor: Remove side effects of password module
Fixes: #11032
---
gajim/common/application.py | 3 ++
gajim/common/passwords.py | 58 ++++++++++++++++++++++++++----------
gajim/gtk/accounts.py | 2 +-
gajim/gtk/features.py | 8 ++---
gajim/gtk/password_dialog.py | 12 ++++----
5 files changed, 56 insertions(+), 27 deletions(-)
diff --git a/gajim/common/application.py b/gajim/common/application.py
index b10c7e8d82..2a58f80f0e 100644
--- a/gajim/common/application.py
+++ b/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()
diff --git a/gajim/common/passwords.py b/gajim/common/passwords.py
index 5e8e1ba975..6dad6c5428 100644
--- a/gajim/common/passwords.py
+++ b/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()
diff --git a/gajim/gtk/accounts.py b/gajim/gtk/accounts.py
index 4e179fd2b0..bd68ff1896 100644
--- a/gajim/gtk/accounts.py
+++ b/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'),
diff --git a/gajim/gtk/features.py b/gajim/gtk/features.py
index de8be1c6a1..e2fea2d2a4 100644
--- a/gajim/gtk/features.py
+++ b/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
diff --git a/gajim/gtk/password_dialog.py b/gajim/gtk/password_dialog.py
index 190a3dcaf7..a0a4dca2a8 100644
--- a/gajim/gtk/password_dialog.py
+++ b/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()
--
GitLab