Another proposal. Fixes the bug and logs a warning

src/caosdb/configuration.py

@@ -24,20 +24,20 @@
 
 """Created on 20.09.2016."""
 
-from sys import hexversion
-if hexversion < 0x03000000:
-    from ConfigParser import ConfigParser  # @UnusedImport @UnresolvedImport
-else:
-    from configparser import ConfigParser  # @UnresolvedImport @Reimport
+try:
+    # python2
+    from ConfigParser import ConfigParser
+except ImportError:
+    # python3
+    from configparser import ConfigParser
 
 _DEFAULTS = {"Connection":
              {"url": None,
-                 "timeout": "200",
-                 "username": None,
-                 "password": None,
-                 "password_method": "plain",
-                 "debug": "0",
-                 "cacert": None},
+              "timeout": "200",
+              "username": None,
+              "password_method": "plain",
+              "debug": "0",
+              "cacert": None},
              "Container":
              {"debug": "0"}}
 

src/caosdb/connection/authentication/external_credentials_provider.py

+# -*- coding: utf-8 -*-
+#
+# ** header v3.0
+# This file is a part of the CaosDB Project.
+#
+# Copyright (C) 2018 Research Group Biomedical Physics,
+# Max-Planck-Institute for Dynamics and Self-Organization Göttingen
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU Affero General Public License as
+# published by the Free Software Foundation, either version 3 of the
+# License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU Affero General Public License for more details.
+#
+# You should have received a copy of the GNU Affero General Public License
+# along with this program. If not, see <https://www.gnu.org/licenses/>.
+#
+# ** end header
+#
+"""external_credentials_provider.
+"""
+from __future__ import absolute_import, unicode_literals
+from abc import ABCMeta
+import logging
+from .plain import PlainTextCredentialsProvider
+
+# meta class compatible with Python 2 *and* 3:
+ABC = ABCMeta(str('ABC'), (object, ), {str('__slots__'): ()})
+
+class ExternalCredentialsProvider(PlainTextCredentialsProvider, ABC):
+    """ExternalCredentialsProvider.
+
+    Abstract subclass of PlainTextCredentialsProvider which should be used to
+    implement external credentials provider (e.g. pass, keyring, or any other call
+    to an external program, which presents the plain text password, which is to be
+    used for the authentication.
+
+    Parameters
+    ----------
+    callback: Function
+        A function which has **kwargs argument. This funktion will be called
+        each time a password is needed with the current connection
+        configuration as parameters.
+    """
+
+    def __init__(self, callback):
+        super(ExternalCredentialsProvider, self).__init__()
+        self._callback = callback
+        self._config = None
+
+    def configure(self, **config):
+        """configure.
+
+        Parameters
+        ----------
+        **config
+            Keyword arguments containing the necessary arguments for the
+            concrete implementation of this class.
+
+        Attributes
+        ----------
+        password : str
+            The password. This password is not stored in this class. A callback
+            is called to provide the password each time this property is
+            called.
+
+        Returns
+        -------
+        None
+        """
+        if "password" in config:
+            if "password_method" in config:
+                authm = "`{}`".format(config["password_method"])
+            else:
+                authm = "an external credentials provider"
+            self.logger.log(logging.WARNING,
+                            ("`password` defined. You configured caosdb to "
+                             "use %s as authentication method and yet "
+                             "provided a password yourself. This indicates "
+                             "a misconfiguration (e.g. in your "
+                             "pycaosdb.ini) and should be avoided."),
+                            authm)
+        self._config = dict(config)
+
+    @property
+    def password(self):
+        return self._callback(**self._config)

src/caosdb/connection/authentication/interface.py

@@ -25,10 +25,6 @@
 caosdb server."""
 from abc import ABCMeta, abstractmethod, abstractproperty
 import logging
-try:
-    from urllib.parse import quote
-except ImportError:
-    from urllib import quote
 from caosdb.connection.utils import urlencode
 from caosdb.connection.interface import CaosDBServerConnection
 from caosdb.connection.utils import parse_session_token
@@ -46,17 +42,24 @@ class AbstractAuthenticator(ABC):
     Interface for different authentication mechanisms. e.g. username/password
     authentication or SSH key authentication.
 
+    Attributes
+    ----------
+    logger : Logger
+        A logger which should be used for all logging which has to do with
+        authentication.
+
     Methods
     -------
-    login
-    logout
-    configure
+    login (abstract)
+    logout (abstract)
+    configure (abstract)
     on_request
     on_response
     """
 
     def __init__(self):
         self._session_token = None
+        self.logger = _LOGGER
 
     @abstractmethod
     def login(self):
@@ -141,6 +144,7 @@ class AbstractAuthenticator(ABC):
             headers['Cookie'] = self._session_token
 
 
+
 class CredentialsAuthenticator(AbstractAuthenticator):
     """CredentialsAuthenticator
 
@@ -220,10 +224,21 @@ class CredentialsProvider(ABC):
 
     Attributes
     ----------
-    password
-    username
+    password (abstract)
+    username (abstract)
+    logger : Logger
+        A logger which should be used for all logging which has to do with the
+        provision of credentials. This is usually just the "authentication"
+        logger.
+
+    Methods
+    -------
+    configure (abstract)
     """
 
+    def __init__(self):
+        self.logger = _LOGGER
+
     @abstractmethod
     def configure(self, **config):
         """configure.

src/caosdb/connection/authentication/keyring.py

@@ -30,7 +30,8 @@ retrieve the password.
 import sys
 import imp
 from getpass import getpass
-from .plain import PlainTextCredentialsProvider
+from caosdb.exceptions import ConfigurationException
+from .external_credentials_provider import ExternalCredentialsProvider
 from .interface import CredentialsAuthenticator
 
 
@@ -46,7 +47,7 @@ def get_authentication_provider():
     CredentialsAuthenticator
         with a 'KeyringCaller' as back-end.
     """
-    return CredentialsAuthenticator(KeyringCaller())
+    return CredentialsAuthenticator(KeyringCaller(callback=_call_keyring))
 
 
 def _get_external_keyring():
@@ -64,7 +65,16 @@ def _get_external_keyring():
         fil.close()
 
 
-def _call_keyring(app, username):
+def _call_keyring(**config):
+    if "username" not in config:
+        raise ConfigurationException("Your configuration did not provide a "
+                                     "`username` which is needed by the "
+                                     "`KeyringCaller` to retrieve the "
+                                     "password in question.")
+    url = config.get("url")
+    username = config.get("username")
+    app = "PyCaosDB — {}".format(url)
+    password = _call_keyring(app=app, username=username)
     external_keyring = _get_external_keyring()
     password = external_keyring.get_password(app, username)
     if password is None:
@@ -76,7 +86,7 @@ def _call_keyring(app, username):
     return password
 
 
-class KeyringCaller(PlainTextCredentialsProvider):
+class KeyringCaller(ExternalCredentialsProvider):
     """KeyringCaller.
 
     A class for retrieving the password from the external 'gnome keyring' and
@@ -91,22 +101,3 @@ class KeyringCaller(PlainTextCredentialsProvider):
     password
     username
     """
-
-    def configure(self, **config):
-        """configure.
-
-        Parameters
-        ----------
-        **config
-            Keyword arguments which contain at least the keywords "username" and
-            "url".
-
-        Returns
-        -------
-        None
-        """
-        url = config.get("url")
-        username = config.get("username")
-        app = "PyCaosDB — {}".format(url)
-        password = _call_keyring(app=app, username=username)
-        super(KeyringCaller, self).configure(password=password, **config)

src/caosdb/connection/authentication/pass.py

@@ -28,8 +28,9 @@ password.
 """
 
 from subprocess import check_output, CalledProcessError
+from caosdb.exceptions import ConfigurationException
 from .interface import CredentialsAuthenticator
-from .plain import PlainTextCredentialsProvider
+from .external_credentials_provider import ExternalCredentialsProvider
 
 
 def get_authentication_provider():
@@ -44,13 +45,19 @@ def get_authentication_provider():
     CredentialsAuthenticator
         with a 'PassCaller' as back-end.
     """
-    return CredentialsAuthenticator(PassCaller())
+    return CredentialsAuthenticator(PassCaller(callback=_call_pass))
 
 
-def _call_pass(password_identifier):
+def _call_pass(**config):
+    if "password_identifier" not in config:
+        raise ConfigurationException("Your configuration did not provide a "
+                                     "`password_identifier` which is needed "
+                                     "by the `PassCaller` to retrieve the "
+                                     "password in question.")
+
     try:
         return check_output(
-            "pass " + password_identifier,
+            "pass " + config["password_identifier"],
             shell=True).splitlines()[0].decode("UTF-8")
     except CalledProcessError as exc:
         raise RuntimeError(
@@ -59,7 +66,7 @@ def _call_pass(password_identifier):
             "incorrect or missing.".format(exc.returncode))
 
 
-class PassCaller(PlainTextCredentialsProvider):
+class PassCaller(ExternalCredentialsProvider):
     """PassCaller.
 
     A class for retrieving the password from the external program 'pass' and
@@ -74,21 +81,5 @@ class PassCaller(PlainTextCredentialsProvider):
     password
     username
     """
-
-    def configure(self, **config):
-        """configure.
-
-        Parameters
-        ----------
-        **config
-            Keyword arguments which contain at least the keywords "username" and
-            "password_identifier".
-
-        Returns
-        -------
-        None
-        """
-        if "password_identifier" in config:
-            password = _call_pass(config["password_identifier"])
-            config["password"] = password
-        super(PassCaller, self).configure(**config)
+    # all the work is done in _call_pass and the super class
+    pass

src/caosdb/connection/authentication/plain.py

@@ -59,6 +59,7 @@ class PlainTextCredentialsProvider(CredentialsProvider):
     """
 
     def __init__(self):
+        super(PlainTextCredentialsProvider, self).__init__()
         self._password = None
         self._username = None
 

src/caosdb/exceptions.py

@@ -32,6 +32,29 @@ class CaosDBException(Exception):
         Exception.__init__(self, msg)
         self.msg = msg
 
+class ConfigurationException(CaosDBException):
+    """ConfigurationException.
+
+    Indicates a misconfiguration.
+
+    Parameters
+    ----------
+    msg : str
+        A descriptin of the misconfiguration. The constructor adds
+        a few lines with explainingg where to find the configuration.
+
+    Attributes
+    ----------
+    msg : str
+        A description of the misconfiguration.
+    """
+    def __init__(self, msg):
+        super(ConfigurationException, self).__init__(msg +
+                                                     ConfigurationException._INFO)
+
+    _INFO = ("\n\nPlease check your ~/.pycaosdb.ini and your $PWD/"
+             ".pycaosdb.ini. Do at least one of them exist and are they correct?")
+
 
 class ClientErrorException(CaosDBException):
     def __init__(self, msg, status, body):

tox.ini

@@ -2,5 +2,6 @@
 envlist= py27, py34, py35, py36
 skip_missing_interpreters = true
 [testenv]
-deps=nose
-commands= nosetests {posargs}
+deps=pytest
+     nose
+commands=py.test {posargs}

unittests/test_authentication_external.py

+"""test_authentication_external.
+
+Tests for the external_credentials_provider modul.
+"""
+
+from __future__ import unicode_literals
+import logging
+from pytest import raises
+from caosdb.connection.authentication import (
+    external_credentials_provider as ecp
+)
+
+class _TestCaller(ecp.ExternalCredentialsProvider):
+    pass
+
+def test_callback():
+    def _callback(**config):
+        assert "opt" in config
+        return "secret"
+    t = _TestCaller(callback=_callback)
+    t.configure(opt=None)
+    assert t.password == "secret"
+
+def test_log_password_incident():
+    class _mylogger(logging.Logger):
+        def log(self, level, msg, *args, **kwargs):
+            assert level == logging.WARNING
+            assert "`password` defined." in msg
+            raise Exception("log")
+
+    t = _TestCaller(callback=None)
+    t.logger = _mylogger("mylogger")
+    with raises(Exception) as exc_info:
+        t.configure(password="password")
+    assert exc_info.value.args[0] == "log"

unittests/test_authentication_keyring.py

+# -*- coding: utf-8 -*-
+#
+# ** header v3.0
+# This file is a part of the CaosDB Project.
+#
+# Copyright (C) 2018 Research Group Biomedical Physics,
+# Max-Planck-Institute for Dynamics and Self-Organization Göttingen
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU Affero General Public License as
+# published by the Free Software Foundation, either version 3 of the
+# License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU Affero General Public License for more details.
+#
+# You should have received a copy of the GNU Affero General Public License
+# along with this program. If not, see <https://www.gnu.org/licenses/>.
+#
+# ** end header
+#
+"""test_authentication_keyring
+
+Tests for the caosdb.connection.authentication.keyring module.
+"""
+import sys
+from pytest import raises
+from caosdb.connection.authentication.keyring import KeyringCaller
+
+def test_initialization():
+    def _callback(**config):
+        assert not config
+        raise Exception("_callback")
+    k = KeyringCaller(callback=_callback)
+    k.configure()
+    with raises(Exception) as exc_info:
+        assert k.password is None
+    assert exc_info.value.args[0] == "_callback"

unittests/test_authentication_pass.py

+# -*- coding: utf-8 -*-
+#
+# ** header v3.0
+# This file is a part of the CaosDB Project.
+#
+# Copyright (C) 2018 Research Group Biomedical Physics,
+# Max-Planck-Institute for Dynamics and Self-Organization Göttingen
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU Affero General Public License as
+# published by the Free Software Foundation, either version 3 of the
+# License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU Affero General Public License for more details.
+#
+# You should have received a copy of the GNU Affero General Public License
+# along with this program. If not, see <https://www.gnu.org/licenses/>.
+#
+# ** end header
+#
+"""test_authentication_pass
+
+Tests for the caosdb.connection.authentication.pass module.
+"""
+import sys
+from pytest import raises
+_PASSCALLER="caosdb.connection.authentication.pass"
+__import__(_PASSCALLER)
+PassCaller = sys.modules[_PASSCALLER].PassCaller
+
+def test_initialization():
+    def _callback(**config):
+        assert not config
+        raise Exception("_callback")
+    p = PassCaller(callback=_callback)
+    p.configure()
+    with raises(Exception) as exc_info:
+        p.password
+    assert exc_info.value.args[0] == "_callback"

unittests/test_authentication_plain.py

@@ -52,3 +52,8 @@ def test_subclass_configure():
         instance.configure(password="OH NO!")
     assert exc_info.value.args[0] == ("configure() got multiple values for "
                                       "keyword argument 'password'")
+
+def test_plain_has_logger():
+    p = PlainTextCredentialsProvider()
+    assert hasattr(p, "logger")
+    assert p.logger.name == "authentication"