From bff3ba577b8db2eb417d0c5501efddfc00b928f9 Mon Sep 17 00:00:00 2001 From: Jaime van Kessel Date: Mon, 21 Dec 2020 14:02:45 +0100 Subject: [PATCH 01/22] Store auth & refresh key in keyring instead of in preferences People tend to share configuration folders, which just isn't secure. CURA-7180 --- cura/OAuth2/AuthorizationService.py | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/cura/OAuth2/AuthorizationService.py b/cura/OAuth2/AuthorizationService.py index 9a5c81ae55..af9d884d6c 100644 --- a/cura/OAuth2/AuthorizationService.py +++ b/cura/OAuth2/AuthorizationService.py @@ -17,7 +17,7 @@ from UM.i18n import i18nCatalog from cura.OAuth2.AuthorizationHelpers import AuthorizationHelpers, TOKEN_TIMESTAMP_FORMAT from cura.OAuth2.LocalAuthorizationServer import LocalAuthorizationServer from cura.OAuth2.Models import AuthenticationResponse - +import keyring i18n_catalog = i18nCatalog("cura") if TYPE_CHECKING: @@ -229,6 +229,11 @@ class AuthorizationService: return try: preferences_data = json.loads(self._preferences.getValue(self._settings.AUTH_DATA_PREFERENCE_KEY)) + + # Since we stored all the sensitive stuff in the keyring, restore that now. + preferences_data["access_token"] = keyring.get_password("cura", "access_token") + preferences_data["refresh_token"] = keyring.get_password("cura", "refresh_token") + if preferences_data: self._auth_data = AuthenticationResponse(**preferences_data) # Also check if we can actually get the user profile information. @@ -255,6 +260,15 @@ class AuthorizationService: self._auth_data = auth_data if auth_data: self._user_profile = self.getUserProfile() + + # Store all the sensitive stuff in the keyring + keyring.set_password("cura", "access_token", auth_data.access_token) + keyring.set_password("cura", "refresh_token", auth_data.refresh_token) + + # And remove that data again so it isn't stored in the preferences. + auth_data.access_token = None + auth_data.refresh_token = None + self._preferences.setValue(self._settings.AUTH_DATA_PREFERENCE_KEY, json.dumps(vars(auth_data))) else: self._user_profile = None From a25a51eddbce84c68fbe8a520b54ea1c0f07241d Mon Sep 17 00:00:00 2001 From: Remco Burema Date: Thu, 24 Dec 2020 14:39:22 +0100 Subject: [PATCH 02/22] Windows workaround for OAuth data removal from config. Windows won't allow long keys in the backend the keyring python package uses as a backend. This means the access_token part can't be stored in the obvious way. Timeboxed some attempts at working around this limitation, but couldn't make it work within the time set. As this is mostly an extra precaustion protecting users that share config folders around against themselves (in other words, if this goes wrong it's not unreasonable to blame the user) it's not top critical, and the important part of that (the refresh_token) can proceed, giving any potential attacker only a 10 minute window from the moment any user shares their %appdata%/cura files (again, this is not how we intent for users to behave, but they can and will do it this way). CURA-7180 --- cura/OAuth2/AuthorizationService.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cura/OAuth2/AuthorizationService.py b/cura/OAuth2/AuthorizationService.py index af9d884d6c..a71468a157 100644 --- a/cura/OAuth2/AuthorizationService.py +++ b/cura/OAuth2/AuthorizationService.py @@ -231,7 +231,7 @@ class AuthorizationService: preferences_data = json.loads(self._preferences.getValue(self._settings.AUTH_DATA_PREFERENCE_KEY)) # Since we stored all the sensitive stuff in the keyring, restore that now. - preferences_data["access_token"] = keyring.get_password("cura", "access_token") + # Don't store the access_token, as it's very long and that (or tried workarounds) causes issues on Windows. preferences_data["refresh_token"] = keyring.get_password("cura", "refresh_token") if preferences_data: @@ -262,11 +262,11 @@ class AuthorizationService: self._user_profile = self.getUserProfile() # Store all the sensitive stuff in the keyring - keyring.set_password("cura", "access_token", auth_data.access_token) + # Don't store the access_token, as it's very long and that (or tried workarounds) causes issues on Windows. keyring.set_password("cura", "refresh_token", auth_data.refresh_token) # And remove that data again so it isn't stored in the preferences. - auth_data.access_token = None + # Keep the access_token, as it's very long and that (or tried workarounds) causes issues on Windows. auth_data.refresh_token = None self._preferences.setValue(self._settings.AUTH_DATA_PREFERENCE_KEY, json.dumps(vars(auth_data))) From 47df060beef382e02c6146a792f507b66636ca56 Mon Sep 17 00:00:00 2001 From: Jelle Spijker Date: Thu, 11 Mar 2021 14:21:51 +0100 Subject: [PATCH 03/22] Added fundaments of SecretStorage vault This class will handle the storing and processing of secrets. Such as tokens. It will try to use the system keyring by default. Falling back to less secure methods, if the user doesn't allow access to the keyring or if the back-end is unsupported. CURA-7180 keyring storage --- cura/OAuth2/AuthorizationService.py | 11 +++++++---- cura/OAuth2/SecretStorage.py | 20 ++++++++++++++++++++ 2 files changed, 27 insertions(+), 4 deletions(-) create mode 100644 cura/OAuth2/SecretStorage.py diff --git a/cura/OAuth2/AuthorizationService.py b/cura/OAuth2/AuthorizationService.py index df1c47f279..d0077cb9a6 100644 --- a/cura/OAuth2/AuthorizationService.py +++ b/cura/OAuth2/AuthorizationService.py @@ -17,7 +17,8 @@ from UM.i18n import i18nCatalog from cura.OAuth2.AuthorizationHelpers import AuthorizationHelpers, TOKEN_TIMESTAMP_FORMAT from cura.OAuth2.LocalAuthorizationServer import LocalAuthorizationServer from cura.OAuth2.Models import AuthenticationResponse -import keyring +from cura.OAuth2.SecretStorage import SecretStorage + i18n_catalog = i18nCatalog("cura") if TYPE_CHECKING: @@ -52,6 +53,8 @@ class AuthorizationService: self.onAuthStateChanged.connect(self._authChanged) + self._secret_storage = SecretStorage() + def _authChanged(self, logged_in): if logged_in and self._unable_to_get_data_message is not None: self._unable_to_get_data_message.hide() @@ -232,7 +235,7 @@ class AuthorizationService: # Since we stored all the sensitive stuff in the keyring, restore that now. # Don't store the access_token, as it's very long and that (or tried workarounds) causes issues on Windows. - preferences_data["refresh_token"] = keyring.get_password("cura", "refresh_token") + preferences_data["refresh_token"] = self._secret_storage["refresh_token"] if preferences_data: self._auth_data = AuthenticationResponse(**preferences_data) @@ -263,7 +266,8 @@ class AuthorizationService: # Store all the sensitive stuff in the keyring # Don't store the access_token, as it's very long and that (or tried workarounds) causes issues on Windows. - keyring.set_password("cura", "refresh_token", auth_data.refresh_token) + self._secret_storage["refresh_token"] = auth_data.refresh_token + # And remove that data again so it isn't stored in the preferences. # Keep the access_token, as it's very long and that (or tried workarounds) causes issues on Windows. @@ -275,4 +279,3 @@ class AuthorizationService: self._preferences.resetPreference(self._settings.AUTH_DATA_PREFERENCE_KEY) self.accessTokenChanged.emit() - diff --git a/cura/OAuth2/SecretStorage.py b/cura/OAuth2/SecretStorage.py new file mode 100644 index 0000000000..42f6164be2 --- /dev/null +++ b/cura/OAuth2/SecretStorage.py @@ -0,0 +1,20 @@ +import keyring + + +class SecretStorage: + def __init__(self): + self._stored_secrets = [] + + def __delitem__(self, key): + if key in self._stored_secrets: + del self._stored_secrets[key] + keyring.delete_password("cura", key) + + def __setitem__(self, key, value): + self._stored_secrets.append(key) + keyring.set_password("cura", key, value) + + def __getitem__(self, key): + if key in self._stored_secrets: + return keyring.get_password("cura", key) + return None From b604bbd255956e2473db925eeb0b803221e9a93d Mon Sep 17 00:00:00 2001 From: Jelle Spijker Date: Mon, 15 Mar 2021 11:48:42 +0100 Subject: [PATCH 04/22] Store secrets as securely as possible Use the keyring if allowed, available, otherwise use preference CURA-7180 keyring storage --- cura/OAuth2/AuthorizationService.py | 6 +++- cura/OAuth2/SecretStorage.py | 43 +++++++++++++++++++++++------ 2 files changed, 40 insertions(+), 9 deletions(-) diff --git a/cura/OAuth2/AuthorizationService.py b/cura/OAuth2/AuthorizationService.py index d0077cb9a6..dd63cf384e 100644 --- a/cura/OAuth2/AuthorizationService.py +++ b/cura/OAuth2/AuthorizationService.py @@ -53,7 +53,7 @@ class AuthorizationService: self.onAuthStateChanged.connect(self._authChanged) - self._secret_storage = SecretStorage() + self._secret_storage = None # type: Optional[SecretStorage] def _authChanged(self, logged_in): if logged_in and self._unable_to_get_data_message is not None: @@ -62,6 +62,7 @@ class AuthorizationService: def initialize(self, preferences: Optional["Preferences"] = None) -> None: if preferences is not None: self._preferences = preferences + self._secret_storage = SecretStorage(preferences) if self._preferences: self._preferences.addPreference(self._settings.AUTH_DATA_PREFERENCE_KEY, "{}") @@ -236,6 +237,7 @@ class AuthorizationService: # Since we stored all the sensitive stuff in the keyring, restore that now. # Don't store the access_token, as it's very long and that (or tried workarounds) causes issues on Windows. preferences_data["refresh_token"] = self._secret_storage["refresh_token"] + preferences_data["access_token"] = self._secret_storage["access_token"] if preferences_data: self._auth_data = AuthenticationResponse(**preferences_data) @@ -267,11 +269,13 @@ class AuthorizationService: # Store all the sensitive stuff in the keyring # Don't store the access_token, as it's very long and that (or tried workarounds) causes issues on Windows. self._secret_storage["refresh_token"] = auth_data.refresh_token + self._secret_storage["access_token"] = auth_data.access_token # And remove that data again so it isn't stored in the preferences. # Keep the access_token, as it's very long and that (or tried workarounds) causes issues on Windows. auth_data.refresh_token = None + auth_data.access_token = None self._preferences.setValue(self._settings.AUTH_DATA_PREFERENCE_KEY, json.dumps(vars(auth_data))) else: diff --git a/cura/OAuth2/SecretStorage.py b/cura/OAuth2/SecretStorage.py index 42f6164be2..c083aa8a0c 100644 --- a/cura/OAuth2/SecretStorage.py +++ b/cura/OAuth2/SecretStorage.py @@ -1,20 +1,47 @@ -import keyring +from typing import Optional + +import keyring # TODO: Add to about as dependency + +from UM.Logger import Logger class SecretStorage: - def __init__(self): - self._stored_secrets = [] + def __init__(self, preferences: Optional["Preferences"] = None): + self._stored_secrets = {} + if preferences: + self._preferences = preferences + keys = self._preferences.getValue("general/keyring") + if keys is not None and keys != '': + self._stored_secrets = set(keys.split(";")) def __delitem__(self, key): if key in self._stored_secrets: - del self._stored_secrets[key] + self._stored_secrets.remove(key) + self._preferences.setValue("general/keyring", ";".join(self._stored_secrets)) keyring.delete_password("cura", key) + else: + # TODO: handle removal of secret from preferences + pass def __setitem__(self, key, value): - self._stored_secrets.append(key) - keyring.set_password("cura", key, value) + try: + keyring.set_password("cura", key, value) + self._stored_secrets.add(key) + self._preferences.setValue(f"general/{key}", None) + self._preferences.setValue("general/keyring", ";".join(self._stored_secrets)) + except: + Logger.logException("w", f"Could not store {key} in keyring.") + self._preferences.setValue(f"general/{key}", value) def __getitem__(self, key): + secret = self._preferences.getValue(f"general/{key}") if key in self._stored_secrets: - return keyring.get_password("cura", key) - return None + try: + secret = keyring.get_password("cura", key) + except: + if secret: + Logger.logException("w", + f"{key} obtained from preferences, consider giving Cura access to the keyring") + if secret is None or secret == 'null': + Logger.logException("w", f"Could not load {key}") + return secret From 2796b9bef30986cc0c5192224d1010631734f5ef Mon Sep 17 00:00:00 2001 From: Jelle Spijker Date: Mon, 15 Mar 2021 13:06:42 +0100 Subject: [PATCH 05/22] Store new keys to preference CURA-7180 keyring storage --- cura/OAuth2/SecretStorage.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/cura/OAuth2/SecretStorage.py b/cura/OAuth2/SecretStorage.py index c083aa8a0c..c662f4a53b 100644 --- a/cura/OAuth2/SecretStorage.py +++ b/cura/OAuth2/SecretStorage.py @@ -7,12 +7,14 @@ from UM.Logger import Logger class SecretStorage: def __init__(self, preferences: Optional["Preferences"] = None): - self._stored_secrets = {} + self._stored_secrets = set() if preferences: self._preferences = preferences keys = self._preferences.getValue("general/keyring") if keys is not None and keys != '': self._stored_secrets = set(keys.split(";")) + else: + self._preferences.addPreference("general/keyring", "{}") def __delitem__(self, key): if key in self._stored_secrets: @@ -28,10 +30,13 @@ class SecretStorage: keyring.set_password("cura", key, value) self._stored_secrets.add(key) self._preferences.setValue(f"general/{key}", None) - self._preferences.setValue("general/keyring", ";".join(self._stored_secrets)) except: Logger.logException("w", f"Could not store {key} in keyring.") - self._preferences.setValue(f"general/{key}", value) + if key in self._stored_secrets: + self._stored_secrets.remove(key) + self._preferences.addPreference("general/{key}".format(key=key), "{}") + self._preferences.setValue("general/{key}".format(key=key), value) + self._preferences.setValue("general/keyring", ";".join(self._stored_secrets)) def __getitem__(self, key): secret = self._preferences.getValue(f"general/{key}") From fcf698f00bb8ad60f3cd40b87cb7f5deea501b89 Mon Sep 17 00:00:00 2001 From: Jelle Spijker Date: Mon, 15 Mar 2021 14:16:27 +0100 Subject: [PATCH 06/22] Added documentation and refactored CURA-7180 keyring storage --- cura/OAuth2/SecretStorage.py | 33 ++++++++++++++++++++------------- 1 file changed, 20 insertions(+), 13 deletions(-) diff --git a/cura/OAuth2/SecretStorage.py b/cura/OAuth2/SecretStorage.py index c662f4a53b..f0b04786dc 100644 --- a/cura/OAuth2/SecretStorage.py +++ b/cura/OAuth2/SecretStorage.py @@ -6,6 +6,11 @@ from UM.Logger import Logger class SecretStorage: + """ + Secret storage vault. It will by default store a secret in the system keyring. If that fails, is not available or + not allowed it will store in the Cura preferences. This is the unsafe "old" behaviour + """ + def __init__(self, preferences: Optional["Preferences"] = None): self._stored_secrets = set() if preferences: @@ -16,7 +21,7 @@ class SecretStorage: else: self._preferences.addPreference("general/keyring", "{}") - def __delitem__(self, key): + def __delitem__(self, key: str): if key in self._stored_secrets: self._stored_secrets.remove(key) self._preferences.setValue("general/keyring", ";".join(self._stored_secrets)) @@ -25,28 +30,30 @@ class SecretStorage: # TODO: handle removal of secret from preferences pass - def __setitem__(self, key, value): + def __setitem__(self, key: str, value: str): try: keyring.set_password("cura", key, value) self._stored_secrets.add(key) - self._preferences.setValue(f"general/{key}", None) + self._preferences.setValue("general/{key}".format(key = key), None) except: - Logger.logException("w", f"Could not store {key} in keyring.") + Logger.logException("w", "Could not store {key} in keyring.".format(key = key)) if key in self._stored_secrets: self._stored_secrets.remove(key) - self._preferences.addPreference("general/{key}".format(key=key), "{}") - self._preferences.setValue("general/{key}".format(key=key), value) + self._preferences.addPreference("general/{key}".format(key = key), "{}") + self._preferences.setValue("general/{key}".format(key = key), value) self._preferences.setValue("general/keyring", ";".join(self._stored_secrets)) - def __getitem__(self, key): - secret = self._preferences.getValue(f"general/{key}") + def __getitem__(self, key: str) -> Optional[str]: + secret = None if key in self._stored_secrets: try: secret = keyring.get_password("cura", key) except: - if secret: - Logger.logException("w", - f"{key} obtained from preferences, consider giving Cura access to the keyring") - if secret is None or secret == 'null': - Logger.logException("w", f"Could not load {key}") + secret = self._preferences.getValue("general/{key}".format(key = key)) + Logger.logException("w", "{key} obtained from preferences, consider giving Cura access to the keyring".format(key = key)) + else: + secret = self._preferences.getValue(f"general/{key}") + Logger.logException("w", "{key} obtained from preferences, consider giving Cura access to the keyring".format(key = key)) + if secret is None or secret == '': + Logger.logException("w", "Could not load {key}".format(key = key)) return secret From 96e39810f2fe3fb409dd67d9bdfa87f1b3478a41 Mon Sep 17 00:00:00 2001 From: Jelle Spijker Date: Mon, 15 Mar 2021 14:35:00 +0100 Subject: [PATCH 07/22] Sync after log in works again Needed to restore the access token after the auth data was written to the preference file. CURA-7180 keyring storage --- cura/OAuth2/AuthorizationService.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/cura/OAuth2/AuthorizationService.py b/cura/OAuth2/AuthorizationService.py index dd63cf384e..35ccffed4b 100644 --- a/cura/OAuth2/AuthorizationService.py +++ b/cura/OAuth2/AuthorizationService.py @@ -267,17 +267,18 @@ class AuthorizationService: self._user_profile = self.getUserProfile() # Store all the sensitive stuff in the keyring - # Don't store the access_token, as it's very long and that (or tried workarounds) causes issues on Windows. self._secret_storage["refresh_token"] = auth_data.refresh_token + + # The access_token will still be stored in the preference file on windows, due to a 255 length limitation self._secret_storage["access_token"] = auth_data.access_token - - # And remove that data again so it isn't stored in the preferences. - # Keep the access_token, as it's very long and that (or tried workarounds) causes issues on Windows. + # Store the data in the preference, setting both tokens to None so they won't be written auth_data.refresh_token = None auth_data.access_token = None - self._preferences.setValue(self._settings.AUTH_DATA_PREFERENCE_KEY, json.dumps(vars(auth_data))) + + # restore access token so that syncing for the current session doesn't fail + auth_data.access_token = self._secret_storage["access_token"] else: self._user_profile = None self._preferences.resetPreference(self._settings.AUTH_DATA_PREFERENCE_KEY) From 392ee68557c7f595fd62c46a38ff256e21326ba6 Mon Sep 17 00:00:00 2001 From: Jelle Spijker Date: Mon, 15 Mar 2021 16:18:16 +0100 Subject: [PATCH 08/22] Updated about dialog with new dependencies CURA-7180 keyring storage --- resources/qml/Dialogs/AboutDialog.qml | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/resources/qml/Dialogs/AboutDialog.qml b/resources/qml/Dialogs/AboutDialog.qml index f8018d3d34..bffdbd75ca 100644 --- a/resources/qml/Dialogs/AboutDialog.qml +++ b/resources/qml/Dialogs/AboutDialog.qml @@ -1,4 +1,4 @@ -// Copyright (c) 2020 Ultimaker B.V. +// Copyright (c) 2021 Ultimaker B.V. // Cura is released under the terms of the LGPLv3 or higher. import QtQuick 2.2 @@ -15,7 +15,7 @@ UM.Dialog title: catalog.i18nc("@title:window The argument is the application name.", "About %1").arg(CuraApplication.applicationDisplayName) minimumWidth: 500 * screenScaleFactor - minimumHeight: 650 * screenScaleFactor + minimumHeight: 700 * screenScaleFactor width: minimumWidth height: minimumHeight @@ -158,7 +158,9 @@ UM.Dialog projectsModel.append({ name: "Sentry", description: catalog.i18nc("@Label", "Python Error tracking library"), license: "BSD 2-Clause 'Simplified'", url: "https://sentry.io/for/python/" }); projectsModel.append({ name: "libnest2d", description: catalog.i18nc("@label", "Polygon packing library, developed by Prusa Research"), license: "LGPL", url: "https://github.com/tamasmeszaros/libnest2d" }); projectsModel.append({ name: "pynest2d", description: catalog.i18nc("@label", "Python bindings for libnest2d"), license: "LGPL", url: "https://github.com/Ultimaker/pynest2d" }); - + projectsModel.append({ name: "keyring", description: catalog.i18nc("@label", "Support library for system keyring access"), license: "MIT", url: "https://github.com/jaraco/keyring" }); + projectsModel.append({ name: "dbus-python", description: catalog.i18nc("@label", "Support library for dbus communication"), license: "MIT", url: "http://dbus.freedesktop.org/doc/dbus-python/" }); + projectsModel.append({ name: "SecretStorage", description: catalog.i18nc("@label", "Support library for storing secrets"), license: "BSD", url: "https://github.com/mitya57/secretstorage" }); projectsModel.append({ name: "Noto Sans", description: catalog.i18nc("@label", "Font"), license: "Apache 2.0", url: "https://www.google.com/get/noto/" }); projectsModel.append({ name: "Font-Awesome-SVG-PNG", description: catalog.i18nc("@label", "SVG icons"), license: "SIL OFL 1.1", url: "https://github.com/encharm/Font-Awesome-SVG-PNG" }); projectsModel.append({ name: "AppImageKit", description: catalog.i18nc("@label", "Linux cross-distribution application deployment"), license: "MIT", url: "https://github.com/AppImage/AppImageKit" }); From 0070866afc9283099b1a673f942464a7c3383f0c Mon Sep 17 00:00:00 2001 From: Jelle Spijker Date: Tue, 16 Mar 2021 11:47:00 +0100 Subject: [PATCH 09/22] Removed unused dependencies from about dialog CURA-7180 keyring storage --- resources/qml/Dialogs/AboutDialog.qml | 2 -- 1 file changed, 2 deletions(-) diff --git a/resources/qml/Dialogs/AboutDialog.qml b/resources/qml/Dialogs/AboutDialog.qml index bffdbd75ca..a0d8b6c4e6 100644 --- a/resources/qml/Dialogs/AboutDialog.qml +++ b/resources/qml/Dialogs/AboutDialog.qml @@ -159,8 +159,6 @@ UM.Dialog projectsModel.append({ name: "libnest2d", description: catalog.i18nc("@label", "Polygon packing library, developed by Prusa Research"), license: "LGPL", url: "https://github.com/tamasmeszaros/libnest2d" }); projectsModel.append({ name: "pynest2d", description: catalog.i18nc("@label", "Python bindings for libnest2d"), license: "LGPL", url: "https://github.com/Ultimaker/pynest2d" }); projectsModel.append({ name: "keyring", description: catalog.i18nc("@label", "Support library for system keyring access"), license: "MIT", url: "https://github.com/jaraco/keyring" }); - projectsModel.append({ name: "dbus-python", description: catalog.i18nc("@label", "Support library for dbus communication"), license: "MIT", url: "http://dbus.freedesktop.org/doc/dbus-python/" }); - projectsModel.append({ name: "SecretStorage", description: catalog.i18nc("@label", "Support library for storing secrets"), license: "BSD", url: "https://github.com/mitya57/secretstorage" }); projectsModel.append({ name: "Noto Sans", description: catalog.i18nc("@label", "Font"), license: "Apache 2.0", url: "https://www.google.com/get/noto/" }); projectsModel.append({ name: "Font-Awesome-SVG-PNG", description: catalog.i18nc("@label", "SVG icons"), license: "SIL OFL 1.1", url: "https://github.com/encharm/Font-Awesome-SVG-PNG" }); projectsModel.append({ name: "AppImageKit", description: catalog.i18nc("@label", "Linux cross-distribution application deployment"), license: "MIT", url: "https://github.com/AppImage/AppImageKit" }); From 6372fbed54d7c1acd5dcb266d10b99df1f02299b Mon Sep 17 00:00:00 2001 From: Jelle Spijker Date: Tue, 16 Mar 2021 11:57:58 +0100 Subject: [PATCH 10/22] Added copyright notice CURA-7180 keyring storage --- cura/OAuth2/SecretStorage.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/cura/OAuth2/SecretStorage.py b/cura/OAuth2/SecretStorage.py index f0b04786dc..509c60d4f8 100644 --- a/cura/OAuth2/SecretStorage.py +++ b/cura/OAuth2/SecretStorage.py @@ -1,3 +1,5 @@ +# Copyright (c) 2021 Ultimaker B.V. +# Cura is released under the terms of the LGPLv3 or higher. from typing import Optional import keyring # TODO: Add to about as dependency From d06a25595a9b451cf7ed01df322b0b7a3d255727 Mon Sep 17 00:00:00 2001 From: Jelle Spijker Date: Tue, 16 Mar 2021 19:28:56 +0100 Subject: [PATCH 11/22] Use a descriptor to optionally store to Keyring CURA-7180 keyring storage --- cura/OAuth2/AuthorizationService.py | 23 +---------- cura/OAuth2/Models.py | 25 ++++++++++-- cura/OAuth2/SecretStorage.py | 61 ----------------------------- 3 files changed, 23 insertions(+), 86 deletions(-) delete mode 100644 cura/OAuth2/SecretStorage.py diff --git a/cura/OAuth2/AuthorizationService.py b/cura/OAuth2/AuthorizationService.py index 35ccffed4b..68c0db78b6 100644 --- a/cura/OAuth2/AuthorizationService.py +++ b/cura/OAuth2/AuthorizationService.py @@ -17,7 +17,6 @@ from UM.i18n import i18nCatalog from cura.OAuth2.AuthorizationHelpers import AuthorizationHelpers, TOKEN_TIMESTAMP_FORMAT from cura.OAuth2.LocalAuthorizationServer import LocalAuthorizationServer from cura.OAuth2.Models import AuthenticationResponse -from cura.OAuth2.SecretStorage import SecretStorage i18n_catalog = i18nCatalog("cura") @@ -53,7 +52,6 @@ class AuthorizationService: self.onAuthStateChanged.connect(self._authChanged) - self._secret_storage = None # type: Optional[SecretStorage] def _authChanged(self, logged_in): if logged_in and self._unable_to_get_data_message is not None: @@ -62,7 +60,6 @@ class AuthorizationService: def initialize(self, preferences: Optional["Preferences"] = None) -> None: if preferences is not None: self._preferences = preferences - self._secret_storage = SecretStorage(preferences) if self._preferences: self._preferences.addPreference(self._settings.AUTH_DATA_PREFERENCE_KEY, "{}") @@ -234,11 +231,6 @@ class AuthorizationService: try: preferences_data = json.loads(self._preferences.getValue(self._settings.AUTH_DATA_PREFERENCE_KEY)) - # Since we stored all the sensitive stuff in the keyring, restore that now. - # Don't store the access_token, as it's very long and that (or tried workarounds) causes issues on Windows. - preferences_data["refresh_token"] = self._secret_storage["refresh_token"] - preferences_data["access_token"] = self._secret_storage["access_token"] - if preferences_data: self._auth_data = AuthenticationResponse(**preferences_data) # Also check if we can actually get the user profile information. @@ -265,20 +257,7 @@ class AuthorizationService: self._auth_data = auth_data if auth_data: self._user_profile = self.getUserProfile() - - # Store all the sensitive stuff in the keyring - self._secret_storage["refresh_token"] = auth_data.refresh_token - - # The access_token will still be stored in the preference file on windows, due to a 255 length limitation - self._secret_storage["access_token"] = auth_data.access_token - - # Store the data in the preference, setting both tokens to None so they won't be written - auth_data.refresh_token = None - auth_data.access_token = None - self._preferences.setValue(self._settings.AUTH_DATA_PREFERENCE_KEY, json.dumps(vars(auth_data))) - - # restore access token so that syncing for the current session doesn't fail - auth_data.access_token = self._secret_storage["access_token"] + self._preferences.setValue(self._settings.AUTH_DATA_PREFERENCE_KEY, json.dumps(auth_data.dump())) else: self._user_profile = None self._preferences.resetPreference(self._settings.AUTH_DATA_PREFERENCE_KEY) diff --git a/cura/OAuth2/Models.py b/cura/OAuth2/Models.py index f49fdc1421..2c077fa548 100644 --- a/cura/OAuth2/Models.py +++ b/cura/OAuth2/Models.py @@ -1,6 +1,8 @@ # Copyright (c) 2020 Ultimaker B.V. # Cura is released under the terms of the LGPLv3 or higher. -from typing import Optional, Dict, Any, List +from typing import Optional, Dict, Any, List, Union +from copy import deepcopy +from cura.OAuth2.KeyringAttribute import KeyringAttribute class BaseModel: @@ -37,12 +39,29 @@ class AuthenticationResponse(BaseModel): # Data comes from the token response with success flag and error message added. success = True # type: bool token_type = None # type: Optional[str] - access_token = None # type: Optional[str] - refresh_token = None # type: Optional[str] expires_in = None # type: Optional[str] scope = None # type: Optional[str] err_message = None # type: Optional[str] received_at = None # type: Optional[str] + access_token = KeyringAttribute() + refresh_token = KeyringAttribute() + + def __init__(self, **kwargs: Any) -> None: + self.access_token = kwargs.pop("access_token", None) + self.refresh_token = kwargs.pop("refresh_token", None) + super(AuthenticationResponse, self).__init__(**kwargs) + + def dump(self) -> dict[Union[bool, Optional[str]]]: + """ + Dumps the dictionary of Authentication attributes. KeyringAttributes are transformed to public attributes + If the keyring was used, these will have a None value, otherwise they will have the secret value + + :return: Dictionary of Authentication attributes + """ + dumped = deepcopy(vars(self)) + dumped["access_token"] = dumped.pop("_access_token") + dumped["refresh_token"] = dumped.pop("_refresh_token") + return dumped class ResponseStatus(BaseModel): diff --git a/cura/OAuth2/SecretStorage.py b/cura/OAuth2/SecretStorage.py deleted file mode 100644 index 509c60d4f8..0000000000 --- a/cura/OAuth2/SecretStorage.py +++ /dev/null @@ -1,61 +0,0 @@ -# Copyright (c) 2021 Ultimaker B.V. -# Cura is released under the terms of the LGPLv3 or higher. -from typing import Optional - -import keyring # TODO: Add to about as dependency - -from UM.Logger import Logger - - -class SecretStorage: - """ - Secret storage vault. It will by default store a secret in the system keyring. If that fails, is not available or - not allowed it will store in the Cura preferences. This is the unsafe "old" behaviour - """ - - def __init__(self, preferences: Optional["Preferences"] = None): - self._stored_secrets = set() - if preferences: - self._preferences = preferences - keys = self._preferences.getValue("general/keyring") - if keys is not None and keys != '': - self._stored_secrets = set(keys.split(";")) - else: - self._preferences.addPreference("general/keyring", "{}") - - def __delitem__(self, key: str): - if key in self._stored_secrets: - self._stored_secrets.remove(key) - self._preferences.setValue("general/keyring", ";".join(self._stored_secrets)) - keyring.delete_password("cura", key) - else: - # TODO: handle removal of secret from preferences - pass - - def __setitem__(self, key: str, value: str): - try: - keyring.set_password("cura", key, value) - self._stored_secrets.add(key) - self._preferences.setValue("general/{key}".format(key = key), None) - except: - Logger.logException("w", "Could not store {key} in keyring.".format(key = key)) - if key in self._stored_secrets: - self._stored_secrets.remove(key) - self._preferences.addPreference("general/{key}".format(key = key), "{}") - self._preferences.setValue("general/{key}".format(key = key), value) - self._preferences.setValue("general/keyring", ";".join(self._stored_secrets)) - - def __getitem__(self, key: str) -> Optional[str]: - secret = None - if key in self._stored_secrets: - try: - secret = keyring.get_password("cura", key) - except: - secret = self._preferences.getValue("general/{key}".format(key = key)) - Logger.logException("w", "{key} obtained from preferences, consider giving Cura access to the keyring".format(key = key)) - else: - secret = self._preferences.getValue(f"general/{key}") - Logger.logException("w", "{key} obtained from preferences, consider giving Cura access to the keyring".format(key = key)) - if secret is None or secret == '': - Logger.logException("w", "Could not load {key}".format(key = key)) - return secret From f51c466155acff7a97735c8fa5a00c0094603e74 Mon Sep 17 00:00:00 2001 From: Jelle Spijker Date: Tue, 16 Mar 2021 19:28:56 +0100 Subject: [PATCH 12/22] Use a descriptor to optionally store to Keyring CURA-7180 keyring storage --- cura/OAuth2/KeyringAttribute.py | 40 +++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) create mode 100644 cura/OAuth2/KeyringAttribute.py diff --git a/cura/OAuth2/KeyringAttribute.py b/cura/OAuth2/KeyringAttribute.py new file mode 100644 index 0000000000..50600c5b9b --- /dev/null +++ b/cura/OAuth2/KeyringAttribute.py @@ -0,0 +1,40 @@ +# Copyright (c) 2021 Ultimaker B.V. +# Cura is released under the terms of the LGPLv3 or higher. +from typing import Optional, Type + +import keyring + +from UM.Logger import Logger + + +class KeyringAttribute: + """ + Descriptor for attributes that need to be stored in the keyring. With Fallback behaviour to the preference cfg file + """ + def __get__(self, instance: Type["BaseModel"], owner: type) -> str: + if self._store_secure: + return keyring.get_password("cura", self._keyring_name) + else: + return getattr(instance, self._name) + + def __set__(self, instance: Type["BaseModel"], value: str): + if self._store_secure: + setattr(instance, self._name, None) + try: + keyring.set_password("cura", self._keyring_name, value) + except keyring.errors.PasswordSetError: + self._store_secure = False + setattr(instance, self._name, value) + Logger.logException("w", "Keyring access denied") + else: + setattr(instance, self._name, value) + + def __set_name__(self, owner: type, name: str): + self._name = "_{}".format(name) + self._keyring_name = name + self._store_secure = False + try: + self._store_secure = keyring.backend.KeyringBackend.viable + except keyring.errors.KeyringError: + Logger.logException("w", "Could not use keyring") + setattr(owner, self._name, None) From b6b9dd18647bc8871042898607a59b949b45d371 Mon Sep 17 00:00:00 2001 From: Jelle Spijker Date: Wed, 17 Mar 2021 07:34:28 +0100 Subject: [PATCH 13/22] Fixed wrong typing CURA-7180 keyring storage --- cura/OAuth2/Models.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cura/OAuth2/Models.py b/cura/OAuth2/Models.py index 2c077fa548..9538c95af4 100644 --- a/cura/OAuth2/Models.py +++ b/cura/OAuth2/Models.py @@ -51,7 +51,7 @@ class AuthenticationResponse(BaseModel): self.refresh_token = kwargs.pop("refresh_token", None) super(AuthenticationResponse, self).__init__(**kwargs) - def dump(self) -> dict[Union[bool, Optional[str]]]: + def dump(self) -> Dict[str, Union[bool, Optional[str]]]: """ Dumps the dictionary of Authentication attributes. KeyringAttributes are transformed to public attributes If the keyring was used, these will have a None value, otherwise they will have the secret value From 3fd3fd7c7de58f946b8fb508c09bd4b61c59c9fa Mon Sep 17 00:00:00 2001 From: Jelle Spijker Date: Wed, 17 Mar 2021 08:54:16 +0100 Subject: [PATCH 14/22] Obfuscate sensitive preference data from the back-up CURA-7180 keyring storage --- cura/Backups/Backup.py | 40 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/cura/Backups/Backup.py b/cura/Backups/Backup.py index 011eb97310..fa135f951b 100644 --- a/cura/Backups/Backup.py +++ b/cura/Backups/Backup.py @@ -5,6 +5,7 @@ import io import os import re import shutil +from copy import deepcopy from zipfile import ZipFile, ZIP_DEFLATED, BadZipfile from typing import Dict, Optional, TYPE_CHECKING @@ -27,6 +28,9 @@ class Backup: IGNORED_FILES = [r"cura\.log", r"plugins\.json", r"cache", r"__pycache__", r"\.qmlc", r"\.pyc"] """These files should be ignored when making a backup.""" + SECRETS_SETTINGS = ["general/ultimaker_auth_data"] + """Secret preferences that need to obfuscated when making a backup of Cura""" + catalog = i18nCatalog("cura") """Re-use translation catalog""" @@ -43,6 +47,9 @@ class Backup: Logger.log("d", "Creating backup for Cura %s, using folder %s", cura_release, version_data_dir) + # obfuscate sensitive secrets + secrets = self._obfuscate() + # Ensure all current settings are saved. self._application.saveSettings() @@ -78,6 +85,8 @@ class Backup: "profile_count": str(profile_count), "plugin_count": str(plugin_count) } + # Restore the obfuscated settings + self._illuminate(**secrets) def _makeArchive(self, buffer: "io.BytesIO", root_path: str) -> Optional[ZipFile]: """Make a full archive from the given root path with the given name. @@ -134,6 +143,9 @@ class Backup: "Tried to restore a Cura backup that is higher than the current version.")) return False + # Get the current secrets and store since the back-up doesn't contain those + secrets = self._obfuscate() + version_data_dir = Resources.getDataStoragePath() archive = ZipFile(io.BytesIO(self.zip_file), "r") extracted = self._extractArchive(archive, version_data_dir) @@ -146,6 +158,9 @@ class Backup: Logger.log("d", "Moving preferences file from %s to %s", backup_preferences_file, preferences_file) shutil.move(backup_preferences_file, preferences_file) + # Restore the obfuscated settings + self._illuminate(**secrets) + return extracted @staticmethod @@ -173,3 +188,28 @@ class Backup: Logger.logException("e", "Unable to extract the backup due to permission or file system errors.") return False return True + + def _obfuscate(self) -> Dict[str, str]: + """ + Obfuscate and remove the secret preferences that are specified in SECRETS_SETTINGS + + :return: a dictionary of the removed secrets. Note: the '/' is replaced by '__' + """ + preferences = self._application.getPreferences() + secrets = {} + for secret in self.SECRETS_SETTINGS: + secrets[secret.replace("/", "__")] = deepcopy(preferences.getValue(secret)) + preferences.setValue(secret, None) + self._application.savePreferences() + return secrets + + def _illuminate(self, **kwargs) -> None: + """ + Restore the obfuscated settings + + :param kwargs: a dict of obscured preferences. Note: the '__' of the keys will be replaced by '/' + """ + preferences = self._application.getPreferences() + for key, value in kwargs.items(): + preferences.setValue(key.replace("__", "/"), value) + self._application.savePreferences() From c462b62edcd14d57117dfe620da505497543ad9e Mon Sep 17 00:00:00 2001 From: Jelle Spijker Date: Wed, 17 Mar 2021 09:22:24 +0100 Subject: [PATCH 15/22] Handle raised error when there is no keyring backend present CURA-7180 keyring storage --- cura/OAuth2/KeyringAttribute.py | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/cura/OAuth2/KeyringAttribute.py b/cura/OAuth2/KeyringAttribute.py index 50600c5b9b..c64ea86975 100644 --- a/cura/OAuth2/KeyringAttribute.py +++ b/cura/OAuth2/KeyringAttribute.py @@ -13,7 +13,12 @@ class KeyringAttribute: """ def __get__(self, instance: Type["BaseModel"], owner: type) -> str: if self._store_secure: - return keyring.get_password("cura", self._keyring_name) + try: + return keyring.get_password("cura", self._keyring_name) + except keyring.errors.NoKeyringError: + self._store_secure = False + Logger.logException("w", "No keyring backend present") + return getattr(instance, self._name) else: return getattr(instance, self._name) @@ -26,6 +31,10 @@ class KeyringAttribute: self._store_secure = False setattr(instance, self._name, value) Logger.logException("w", "Keyring access denied") + except keyring.errors.NoKeyringError: + self._store_secure = False + setattr(instance, self._name, value) + Logger.logException("w", "No keyring backend present") else: setattr(instance, self._name, value) @@ -35,6 +44,6 @@ class KeyringAttribute: self._store_secure = False try: self._store_secure = keyring.backend.KeyringBackend.viable - except keyring.errors.KeyringError: + except keyring.errors.NoKeyringError: Logger.logException("w", "Could not use keyring") setattr(owner, self._name, None) From 387fc36dc6b77c516aa33d2ee6c94df42dbc5043 Mon Sep 17 00:00:00 2001 From: Jelle Spijker Date: Wed, 17 Mar 2021 09:30:31 +0100 Subject: [PATCH 16/22] Small aesthetic code changes CURA-7180 keyring storage --- cura/OAuth2/AuthorizationService.py | 2 -- cura/OAuth2/Models.py | 2 +- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/cura/OAuth2/AuthorizationService.py b/cura/OAuth2/AuthorizationService.py index 68c0db78b6..da654b52bb 100644 --- a/cura/OAuth2/AuthorizationService.py +++ b/cura/OAuth2/AuthorizationService.py @@ -52,7 +52,6 @@ class AuthorizationService: self.onAuthStateChanged.connect(self._authChanged) - def _authChanged(self, logged_in): if logged_in and self._unable_to_get_data_message is not None: self._unable_to_get_data_message.hide() @@ -230,7 +229,6 @@ class AuthorizationService: return try: preferences_data = json.loads(self._preferences.getValue(self._settings.AUTH_DATA_PREFERENCE_KEY)) - if preferences_data: self._auth_data = AuthenticationResponse(**preferences_data) # Also check if we can actually get the user profile information. diff --git a/cura/OAuth2/Models.py b/cura/OAuth2/Models.py index 9538c95af4..4c84872a09 100644 --- a/cura/OAuth2/Models.py +++ b/cura/OAuth2/Models.py @@ -1,4 +1,4 @@ -# Copyright (c) 2020 Ultimaker B.V. +# Copyright (c) 2021 Ultimaker B.V. # Cura is released under the terms of the LGPLv3 or higher. from typing import Optional, Dict, Any, List, Union from copy import deepcopy From dadda742ecca248151bc0d2bdda2561e338c1370 Mon Sep 17 00:00:00 2001 From: Kostas Karmas Date: Mon, 29 Mar 2021 16:31:31 +0200 Subject: [PATCH 17/22] Add pywin32 to the requirements Allows the keyring library to use the Windows Credential Manager as a backend. CURA-7180 --- requirements.txt | 2 ++ resources/qml/Dialogs/AboutDialog.qml | 1 + 2 files changed, 3 insertions(+) diff --git a/requirements.txt b/requirements.txt index b13e160868..2233fb81e7 100644 --- a/requirements.txt +++ b/requirements.txt @@ -32,3 +32,5 @@ urllib3==1.25.6 PyYAML==5.1.2 zeroconf==0.24.1 comtypes==1.1.7 +pywin32==300 +keyring==23.0.1 diff --git a/resources/qml/Dialogs/AboutDialog.qml b/resources/qml/Dialogs/AboutDialog.qml index a0d8b6c4e6..1ee0b31040 100644 --- a/resources/qml/Dialogs/AboutDialog.qml +++ b/resources/qml/Dialogs/AboutDialog.qml @@ -159,6 +159,7 @@ UM.Dialog projectsModel.append({ name: "libnest2d", description: catalog.i18nc("@label", "Polygon packing library, developed by Prusa Research"), license: "LGPL", url: "https://github.com/tamasmeszaros/libnest2d" }); projectsModel.append({ name: "pynest2d", description: catalog.i18nc("@label", "Python bindings for libnest2d"), license: "LGPL", url: "https://github.com/Ultimaker/pynest2d" }); projectsModel.append({ name: "keyring", description: catalog.i18nc("@label", "Support library for system keyring access"), license: "MIT", url: "https://github.com/jaraco/keyring" }); + projectsModel.append({ name: "pywin32", description: catalog.i18nc("@label", "Python extensions for Microsoft Windows"), license: "PSF", url: "https://github.com/mhammond/pywin32" }); projectsModel.append({ name: "Noto Sans", description: catalog.i18nc("@label", "Font"), license: "Apache 2.0", url: "https://www.google.com/get/noto/" }); projectsModel.append({ name: "Font-Awesome-SVG-PNG", description: catalog.i18nc("@label", "SVG icons"), license: "SIL OFL 1.1", url: "https://github.com/encharm/Font-Awesome-SVG-PNG" }); projectsModel.append({ name: "AppImageKit", description: catalog.i18nc("@label", "Linux cross-distribution application deployment"), license: "MIT", url: "https://github.com/AppImage/AppImageKit" }); From dbb15b7c7190e28a6946d98499f494cdc1f1f8bd Mon Sep 17 00:00:00 2001 From: Kostas Karmas Date: Mon, 29 Mar 2021 16:33:59 +0200 Subject: [PATCH 18/22] Account for exception occured when storing long tokens on Windows The Windows Credential Manager allows for up to 256bits passwords. If a password longer than that is attempted to be written, it will throw a BaseException. CURA-7180 --- cura/OAuth2/KeyringAttribute.py | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/cura/OAuth2/KeyringAttribute.py b/cura/OAuth2/KeyringAttribute.py index c64ea86975..6281f9ae92 100644 --- a/cura/OAuth2/KeyringAttribute.py +++ b/cura/OAuth2/KeyringAttribute.py @@ -1,11 +1,16 @@ # Copyright (c) 2021 Ultimaker B.V. # Cura is released under the terms of the LGPLv3 or higher. -from typing import Optional, Type +from typing import Type, TYPE_CHECKING import keyring +from keyring.backend import KeyringBackend +from keyring.errors import NoKeyringError, PasswordSetError from UM.Logger import Logger +if TYPE_CHECKING: + from cura.OAuth2.Models import BaseModel + class KeyringAttribute: """ @@ -15,7 +20,7 @@ class KeyringAttribute: if self._store_secure: try: return keyring.get_password("cura", self._keyring_name) - except keyring.errors.NoKeyringError: + except NoKeyringError: self._store_secure = False Logger.logException("w", "No keyring backend present") return getattr(instance, self._name) @@ -27,14 +32,20 @@ class KeyringAttribute: setattr(instance, self._name, None) try: keyring.set_password("cura", self._keyring_name, value) - except keyring.errors.PasswordSetError: + except PasswordSetError: self._store_secure = False setattr(instance, self._name, value) Logger.logException("w", "Keyring access denied") - except keyring.errors.NoKeyringError: + except NoKeyringError: self._store_secure = False setattr(instance, self._name, value) Logger.logException("w", "No keyring backend present") + except BaseException as e: + # A BaseException can occur in Windows when the keyring attempts to write a token longer than 256 + # characters in the Windows Credentials Manager. + self._store_secure = False + setattr(instance, self._name, value) + Logger.log("w", "Keyring failed: {}".format(e)) else: setattr(instance, self._name, value) @@ -43,7 +54,7 @@ class KeyringAttribute: self._keyring_name = name self._store_secure = False try: - self._store_secure = keyring.backend.KeyringBackend.viable - except keyring.errors.NoKeyringError: + self._store_secure = KeyringBackend.viable + except NoKeyringError: Logger.logException("w", "Could not use keyring") setattr(owner, self._name, None) From fb5d59db32bbbfb48acc96a33602790cde955987 Mon Sep 17 00:00:00 2001 From: Kostas Karmas Date: Mon, 29 Mar 2021 17:03:09 +0200 Subject: [PATCH 19/22] Fix comment The Windows Credential Manager actually fails if the password is more than 1024 bits long. CURA-7180 --- cura/OAuth2/KeyringAttribute.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cura/OAuth2/KeyringAttribute.py b/cura/OAuth2/KeyringAttribute.py index 6281f9ae92..64e813d242 100644 --- a/cura/OAuth2/KeyringAttribute.py +++ b/cura/OAuth2/KeyringAttribute.py @@ -41,7 +41,7 @@ class KeyringAttribute: setattr(instance, self._name, value) Logger.logException("w", "No keyring backend present") except BaseException as e: - # A BaseException can occur in Windows when the keyring attempts to write a token longer than 256 + # A BaseException can occur in Windows when the keyring attempts to write a token longer than 1024 # characters in the Windows Credentials Manager. self._store_secure = False setattr(instance, self._name, value) From bde88d7875644d8da74de904402157b094bd7f93 Mon Sep 17 00:00:00 2001 From: Remco Burema Date: Tue, 30 Mar 2021 17:24:25 +0200 Subject: [PATCH 20/22] Don't store cerain keys ever. CURA-7180 --- cura/OAuth2/KeyringAttribute.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/cura/OAuth2/KeyringAttribute.py b/cura/OAuth2/KeyringAttribute.py index 64e813d242..f717f83bef 100644 --- a/cura/OAuth2/KeyringAttribute.py +++ b/cura/OAuth2/KeyringAttribute.py @@ -11,6 +11,7 @@ from UM.Logger import Logger if TYPE_CHECKING: from cura.OAuth2.Models import BaseModel +DONT_EVER_STORE = ["refresh_token"] class KeyringAttribute: """ @@ -34,17 +35,20 @@ class KeyringAttribute: keyring.set_password("cura", self._keyring_name, value) except PasswordSetError: self._store_secure = False - setattr(instance, self._name, value) + if self._name not in DONT_EVER_STORE: + setattr(instance, self._name, value) Logger.logException("w", "Keyring access denied") except NoKeyringError: self._store_secure = False - setattr(instance, self._name, value) + if self._name not in DONT_EVER_STORE: + setattr(instance, self._name, value) Logger.logException("w", "No keyring backend present") except BaseException as e: # A BaseException can occur in Windows when the keyring attempts to write a token longer than 1024 # characters in the Windows Credentials Manager. self._store_secure = False - setattr(instance, self._name, value) + if self._name not in DONT_EVER_STORE: + setattr(instance, self._name, value) Logger.log("w", "Keyring failed: {}".format(e)) else: setattr(instance, self._name, value) From 72248d47e18a1d5163e5bfe6df6a49743c0b2ee7 Mon Sep 17 00:00:00 2001 From: Remco Burema Date: Tue, 30 Mar 2021 20:40:33 +0200 Subject: [PATCH 21/22] Windows workaround for keyring issue. So there is an issue with keyring w.r.t. frozen installs (maybe also local). If you have pywin32 installed, it works fine locally. Take a note here, that a variant of this package, pywin32-ctypes, a rudimentary version of that package that works wholly within python, is already installed as its a dependency for keyring on windows. Due to an unknown reason, when running it fails to detect this, so some workaround is needed, _or_ the 'normal' pywin32 package should be installed. However, problems occurred when attempts where made to install pywin32 via cx_freeze. Then the actual workaround was encountered (https://github.com/jaraco/keyring/issues/468), which _should_ hopefully let use use the keyring on windows without needing the 'full' version of pywin32. CURA-7180 --- cura/OAuth2/KeyringAttribute.py | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/cura/OAuth2/KeyringAttribute.py b/cura/OAuth2/KeyringAttribute.py index f717f83bef..1cc1424e8f 100644 --- a/cura/OAuth2/KeyringAttribute.py +++ b/cura/OAuth2/KeyringAttribute.py @@ -11,7 +11,15 @@ from UM.Logger import Logger if TYPE_CHECKING: from cura.OAuth2.Models import BaseModel -DONT_EVER_STORE = ["refresh_token"] +# Need to do some extra workarounds on windows: +import sys +from UM.Platform import Platform +if Platform.isWindows() and hasattr(sys, "frozen"): + from keyring.backends.Windows import WinVaultKeyring + keyring.set_keyring(WinVaultKeyring()) + +# Even if errors happen, we don't want this stored locally: +DONT_EVER_STORE_LOCALLY = ["refresh_token"] class KeyringAttribute: """ @@ -35,19 +43,19 @@ class KeyringAttribute: keyring.set_password("cura", self._keyring_name, value) except PasswordSetError: self._store_secure = False - if self._name not in DONT_EVER_STORE: + if self._name not in DONT_EVER_STORE_LOCALLY: setattr(instance, self._name, value) Logger.logException("w", "Keyring access denied") except NoKeyringError: self._store_secure = False - if self._name not in DONT_EVER_STORE: + if self._name not in DONT_EVER_STORE_LOCALLY: setattr(instance, self._name, value) Logger.logException("w", "No keyring backend present") except BaseException as e: # A BaseException can occur in Windows when the keyring attempts to write a token longer than 1024 # characters in the Windows Credentials Manager. self._store_secure = False - if self._name not in DONT_EVER_STORE: + if self._name not in DONT_EVER_STORE_LOCALLY: setattr(instance, self._name, value) Logger.log("w", "Keyring failed: {}".format(e)) else: From e1490a68dfd1d84bf9b0e85b0cd33c2ffcdae324 Mon Sep 17 00:00:00 2001 From: Remco Burema Date: Tue, 30 Mar 2021 21:46:19 +0200 Subject: [PATCH 22/22] Also need to import this. CURA-7180 --- cura/OAuth2/KeyringAttribute.py | 1 + 1 file changed, 1 insertion(+) diff --git a/cura/OAuth2/KeyringAttribute.py b/cura/OAuth2/KeyringAttribute.py index 1cc1424e8f..76385d6a0b 100644 --- a/cura/OAuth2/KeyringAttribute.py +++ b/cura/OAuth2/KeyringAttribute.py @@ -15,6 +15,7 @@ if TYPE_CHECKING: import sys from UM.Platform import Platform if Platform.isWindows() and hasattr(sys, "frozen"): + import win32timezone from keyring.backends.Windows import WinVaultKeyring keyring.set_keyring(WinVaultKeyring())