From bff3ba577b8db2eb417d0c5501efddfc00b928f9 Mon Sep 17 00:00:00 2001
From: Jaime van Kessel <nallath@gmail.com>
Date: Mon, 21 Dec 2020 14:02:45 +0100
Subject: [PATCH 01/16] 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 <r.burema@ultimaker.com>
Date: Thu, 24 Dec 2020 14:39:22 +0100
Subject: [PATCH 02/16] 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 <spijker.jelle@gmail.com>
Date: Thu, 11 Mar 2021 14:21:51 +0100
Subject: [PATCH 03/16] 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 <spijker.jelle@gmail.com>
Date: Mon, 15 Mar 2021 11:48:42 +0100
Subject: [PATCH 04/16] 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 <spijker.jelle@gmail.com>
Date: Mon, 15 Mar 2021 13:06:42 +0100
Subject: [PATCH 05/16] 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 <spijker.jelle@gmail.com>
Date: Mon, 15 Mar 2021 14:16:27 +0100
Subject: [PATCH 06/16] 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 <spijker.jelle@gmail.com>
Date: Mon, 15 Mar 2021 14:35:00 +0100
Subject: [PATCH 07/16] 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 <spijker.jelle@gmail.com>
Date: Mon, 15 Mar 2021 16:18:16 +0100
Subject: [PATCH 08/16] 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 <spijker.jelle@gmail.com>
Date: Tue, 16 Mar 2021 11:47:00 +0100
Subject: [PATCH 09/16] 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 <spijker.jelle@gmail.com>
Date: Tue, 16 Mar 2021 11:57:58 +0100
Subject: [PATCH 10/16] 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 <spijker.jelle@gmail.com>
Date: Tue, 16 Mar 2021 19:28:56 +0100
Subject: [PATCH 11/16] 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 <spijker.jelle@gmail.com>
Date: Tue, 16 Mar 2021 19:28:56 +0100
Subject: [PATCH 12/16] 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 <spijker.jelle@gmail.com>
Date: Wed, 17 Mar 2021 07:34:28 +0100
Subject: [PATCH 13/16] 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 <spijker.jelle@gmail.com>
Date: Wed, 17 Mar 2021 08:54:16 +0100
Subject: [PATCH 14/16] 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 <spijker.jelle@gmail.com>
Date: Wed, 17 Mar 2021 09:22:24 +0100
Subject: [PATCH 15/16] 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 <spijker.jelle@gmail.com>
Date: Wed, 17 Mar 2021 09:30:31 +0100
Subject: [PATCH 16/16] 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