Merge pull request #18 from conda-incubator/bugfix-better-keyring-err…

…or-message

Adds better error handling when keyring is not supported

conda_auth/constants.py

@@ -3,5 +3,3 @@
 """
 
 PLUGIN_NAME = "conda-auth"
-
-LOGOUT_ERROR_MESSAGE = "Unable to logout."

conda_auth/handlers/base.py

@@ -4,10 +4,11 @@
 from collections.abc import Mapping
 
 import conda.base.context
-import keyring
 from conda.models.channel import Channel
 from conda.base.context import context as global_context
 
+from ..storage import storage
+
 
 class AuthManager(ABC):
     """
@@ -57,11 +58,8 @@ def store(self, channel: Channel, settings: Mapping[str, str]) -> str:
     def save_credentials(self, channel: Channel, username: str, secret: str) -> None:
         """
         Saves the provided credentials to our credential store.
-
-        TODO: Method may be expanded in the future to allow the use of other storage
-              mechanisms.
         """
-        keyring.set_password(self.get_keyring_id(channel), username, secret)
+        storage.set_password(self.get_keyring_id(channel), username, secret)
 
     def fetch_secret(
         self, channel: Channel, settings: Mapping[str, str | None]

conda_auth/handlers/basic_auth.py

@@ -5,15 +5,13 @@
 
 from collections.abc import Mapping
 
-import keyring
-from keyring.errors import PasswordDeleteError
 from requests.auth import _basic_auth_str  # type: ignore
-from conda.exceptions import CondaError
 from conda.models.channel import Channel
 from conda.plugins.types import ChannelAuthBase
 
-from ..constants import LOGOUT_ERROR_MESSAGE, PLUGIN_NAME
+from ..constants import PLUGIN_NAME
 from ..exceptions import CondaAuthError
+from ..storage import storage
 from .base import AuthManager
 
 USERNAME_PARAM_NAME: str = "username"
@@ -54,10 +52,7 @@ def remove_secret(
         keyring_id = self.get_keyring_id(channel)
         username = self.get_username(settings)
 
-        try:
-            keyring.delete_password(keyring_id, username)
-        except PasswordDeleteError as exc:
-            raise CondaAuthError(f"{LOGOUT_ERROR_MESSAGE} {exc}")
+        storage.delete_password(keyring_id, username)
 
     def get_auth_type(self) -> str:
         return HTTP_BASIC_AUTH_NAME
@@ -88,7 +83,7 @@ def get_password(
         # Now try retrieving it from the password manager
         if password is None:
             keyring_id = self.get_keyring_id(channel)
-            password = keyring.get_password(keyring_id, username)
+            password = storage.get_password(keyring_id, username)
 
             if password is None:
                 raise CondaAuthError("Password not found")
@@ -115,7 +110,7 @@ def __init__(self, channel_name: str):
         self.username, self.password = manager.get_secret(channel_name)
 
         if self.username is None and self.password is None:
-            raise CondaError(
+            raise CondaAuthError(
                 f"Unable to find user credentials for requests with channel {channel_name}"
             )
 

conda_auth/handlers/token.py

@@ -5,14 +5,12 @@
 
 from collections.abc import Mapping
 
-import keyring
-from keyring.errors import PasswordDeleteError
-from conda.exceptions import CondaError
 from conda.models.channel import Channel
 from conda.plugins.types import ChannelAuthBase
 
-from ..constants import LOGOUT_ERROR_MESSAGE, PLUGIN_NAME
+from ..constants import PLUGIN_NAME
 from ..exceptions import CondaAuthError
+from ..storage import storage
 from .base import AuthManager
 
 TOKEN_PARAM_NAME: str = "token"
@@ -48,7 +46,7 @@ def _fetch_secret(
         if token is None:
             # Try password manager if there was nothing there
             keyring_id = self.get_keyring_id(channel)
-            token = keyring.get_password(keyring_id, USERNAME)
+            token = storage.get_password(keyring_id, USERNAME)
 
             if token is None:
                 raise CondaAuthError("Token not found")
@@ -60,10 +58,7 @@ def remove_secret(
     ) -> None:
         keyring_id = self.get_keyring_id(channel)
 
-        try:
-            keyring.delete_password(keyring_id, USERNAME)
-        except PasswordDeleteError as exc:
-            raise CondaAuthError(f"{LOGOUT_ERROR_MESSAGE} {exc}")
+        storage.delete_password(keyring_id, USERNAME)
 
     def get_auth_type(self) -> str:
         return TOKEN_NAME
@@ -107,7 +102,7 @@ def __init__(self, channel_name: str):
         self.is_anaconda_dot_org = is_anaconda_dot_org(channel_name)
 
         if self.token is None:
-            raise CondaError(
+            raise CondaAuthError(
                 f"Unable to find authorization token for requests with channel {channel_name}"
             )
 

conda_auth/storage/__init__.py

@@ -0,0 +1,31 @@
+# flake8: noqa: F401
+from keyring import get_keyring
+from keyring.errors import NoKeyringError
+
+from .base import Storage
+from .keyring import KeyringStorage
+from ..exceptions import CondaAuthError
+
+
+def get_storage_backend() -> Storage:
+    """
+    Determine the correct storage backend to use, raise CondaAuthError if none found.
+
+    TODO: Add future support for another storage backend when keyring cannot be used.
+    """
+    try:
+        keyring_tester = get_keyring()
+        # Retrieve a dummy password to try to trigger NoKeyringError
+        keyring_tester.get_password("conda_auth", "test")
+    except NoKeyringError:
+        raise CondaAuthError(
+            "Unable to find a credential storage backend, which means this operating system"
+            " is likely unsupported. One way to overcome this is by installing a third party"
+            " keyring storage backend. You can find more information about that here: "
+            "https://pypi.org/project/keyring"
+        )
+
+    return KeyringStorage()
+
+
+storage = get_storage_backend()

conda_auth/storage/base.py

@@ -0,0 +1,25 @@
+from __future__ import annotations
+
+from abc import ABC, abstractmethod
+
+
+class Storage(ABC):
+    """ABC class for all credential storage backends"""
+
+    @abstractmethod
+    def set_password(self, key_id: str, username: str, password: str) -> None:
+        """
+        Sets the password for a specific ``key_id``
+        """
+
+    @abstractmethod
+    def get_password(self, key_id: str, username: str) -> str | None:
+        """
+        Gets the password for a specific ``key_id``
+        """
+
+    @abstractmethod
+    def delete_password(self, key_id: str, username: str) -> None:
+        """
+        Deletes the password for a specific ``key_id``
+        """

conda_auth/storage/keyring.py

@@ -0,0 +1,25 @@
+from __future__ import annotations
+
+import keyring
+from keyring.errors import PasswordDeleteError
+
+from .base import Storage
+from ..exceptions import CondaAuthError
+
+
+class KeyringStorage(Storage):
+    """
+    Storage implementation for keyring library
+    """
+
+    def get_password(self, key_id: str, username: str) -> str | None:
+        return keyring.get_password(key_id, username)
+
+    def set_password(self, key_id: str, username: str, password: str) -> None:
+        return keyring.set_password(key_id, username, password)
+
+    def delete_password(self, key_id: str, username: str) -> None:
+        try:
+            return keyring.delete_password(key_id, username)
+        except PasswordDeleteError as exc:
+            raise CondaAuthError(f"Unable to remove secret: {exc}")

docs/user/index.md

@@ -66,6 +66,25 @@ You can do this by running the following command:
 conda auth logout <channel_name>
 ```
 
+### Storage backend unavailable?
+
+Conda auth relies on the [keyring](https://github.com/jaraco/keyring) package to store its passwords and secrets.
+Because of this, it only supports a limited number of operating systems, mostly desktop operating systems like
+Windows, OSX and several Linux variants.
+
+If you want to use conda-auth, but are not using a supported operating system, you can install the
+[keyring-alt](https://github.com/jaraco/keyrings.alt) package:
+
+```
+conda install -c conda-forge keyrings.alt
+```
+
+```{caution}
+This method stores passwords and secrets in a plain text file on the filesystem and may not be acceptable for
+production usage. Please read the [project's README](https://github.com/jaraco/keyrings.alt) for more
+information.
+```
+
 ## Reporting bugs
 
 Have you found a bug you want to let us know about? Please create an issue at our

requirements.dev.txt

@@ -1,5 +1,6 @@
 conda-forge::darker
 conda-forge::flake8
+conda-forge::keyrings.alt
 conda-forge::mypy
 conda-forge::pip
 conda-forge::pyupgrade

tests/cli/test_logout.py

@@ -14,7 +14,7 @@ def test_logout_of_active_session(mocker, runner, keyring):
 
     # setup mocks
     mock_context = mocker.patch("conda_auth.cli.context")
-    keyring_mocks = keyring(secret)
+    keyring_mock, _ = keyring(secret)
     mock_context.channel_settings = [
         {"channel": channel_name, "auth": HTTP_BASIC_AUTH_NAME, "username": username}
     ]
@@ -26,7 +26,7 @@ def test_logout_of_active_session(mocker, runner, keyring):
     assert result.exit_code == 0
 
     # Make sure the delete password call was invoked correctly
-    assert keyring_mocks.basic.delete_password.mock_calls == [
+    assert keyring_mock.delete_password.mock_calls == [
         mocker.call(f"{PLUGIN_NAME}::{HTTP_BASIC_AUTH_NAME}::{channel_name}", username)
     ]
 

tests/conftest.py

@@ -1,31 +1,19 @@
-from unittest.mock import MagicMock
-from typing import NamedTuple
-
 import pytest
 from click.testing import CliRunner
 
 
-class KeyringMocks(NamedTuple):
-    basic: MagicMock
-    token: MagicMock
-    base: MagicMock
-
-
 @pytest.fixture
 def keyring(mocker):
     """
     Used to mock keyring for the duration of our tests
     """
 
     def _keyring(secret):
-        token = mocker.patch("conda_auth.handlers.token.keyring")
-        basic = mocker.patch("conda_auth.handlers.basic_auth.keyring")
-        base = mocker.patch("conda_auth.handlers.base.keyring")
-
-        basic.get_password.return_value = secret
-        token.get_password.return_value = secret
+        get_keyring = mocker.patch("conda_auth.storage.get_keyring")
+        keyring_storage = mocker.patch("conda_auth.storage.keyring.keyring")
+        keyring_storage.get_password.return_value = secret
 
-        return KeyringMocks(basic, token, base)
+        return keyring_storage, get_keyring
 
     return _keyring
 

tests/handlers/test_basic_auth.py

@@ -13,7 +13,6 @@
     BasicAuthManager,
 )
 from conda_auth.exceptions import CondaAuthError
-from conda_auth.constants import LOGOUT_ERROR_MESSAGE
 
 
 @pytest.fixture(autouse=True)
@@ -91,14 +90,14 @@ def test_basic_auth_manager_cache_exists(keyring):
     manager._cache = {channel.canonical_name: (username, secret)}
 
     # setup mocks
-    keyring_mock = keyring(secret)
+    keyring_mock, _ = keyring(secret)
 
     # run code under test
     manager.store(channel, settings)
 
     # make assertions
     assert manager._cache == {channel.canonical_name: (username, secret)}
-    keyring_mock.basic.get_password.assert_not_called()
+    keyring_mock.get_password.assert_not_called()
 
 
 def test_basic_auth_manager_remove_existing_secret(keyring):
@@ -112,13 +111,13 @@ def test_basic_auth_manager_remove_existing_secret(keyring):
     channel = Channel("tester")
 
     # setup mocks
-    keyring_mocks = keyring(secret)
+    keyring_mock, _ = keyring(secret)
 
     # run code under test
     manager.remove_secret(channel, settings)
 
     # make assertions
-    keyring_mocks.basic.delete_password.assert_called_once()
+    keyring_mock.delete_password.assert_called_once()
 
 
 def test_basic_auth_manager_remove_existing_secret_no_username(keyring):
@@ -149,14 +148,12 @@ def test_basic_auth_manager_remove_non_existing_secret(keyring):
     channel = Channel("tester")
 
     # setup mocks
-    keyring_mocks = keyring(secret)
+    keyring_mock, _ = keyring(secret)
     message = "Secret not found."
-    keyring_mocks.basic.delete_password.side_effect = PasswordDeleteError(message)
-
-    # run code under test
+    keyring_mock.delete_password.side_effect = PasswordDeleteError(message)
 
     # make assertions
-    with pytest.raises(CondaAuthError, match=f"{LOGOUT_ERROR_MESSAGE} {message}"):
+    with pytest.raises(CondaAuthError, match=f"Unable to remove secret: {message}"):
         manager.remove_secret(channel, settings)
 
 

tests/handlers/test_token.py

@@ -5,7 +5,6 @@
 from conda.models.channel import Channel
 from keyring.errors import PasswordDeleteError
 
-from conda_auth.constants import LOGOUT_ERROR_MESSAGE
 from conda_auth.exceptions import CondaAuthError
 from conda_auth.handlers.token import (
     is_anaconda_dot_org,
@@ -81,13 +80,13 @@ def test_basic_auth_manager_remove_existing_secret(keyring):
     channel = Channel("tester")
 
     # setup mocks
-    keyring_mocks = keyring(secret)
+    keyring_mock, _ = keyring(secret)
 
     # run code under test
     manager.remove_secret(channel, settings)
 
     # make assertions
-    keyring_mocks.token.delete_password.assert_called_once()
+    keyring_mock.delete_password.assert_called_once()
 
 
 def test_basic_auth_manager_remove_non_existing_secret(keyring):
@@ -102,12 +101,12 @@ def test_basic_auth_manager_remove_non_existing_secret(keyring):
     channel = Channel("tester")
 
     # setup mocks
-    keyring_mocks = keyring(secret)
+    keyring_mock, _ = keyring(secret)
     message = "Secret not found."
-    keyring_mocks.token.delete_password.side_effect = PasswordDeleteError(message)
+    keyring_mock.delete_password.side_effect = PasswordDeleteError(message)
 
     # make assertions
-    with pytest.raises(CondaAuthError, match=f"{LOGOUT_ERROR_MESSAGE} {message}"):
+    with pytest.raises(CondaAuthError, match=f"Unable to remove secret: {message}"):
         manager.remove_secret(channel, settings)