From 8233fa3861c4688e705a9dbfce88c732587697b7 Mon Sep 17 00:00:00 2001
From: Timm Fitschen <mail@timmfitschen.de>
Date: Fri, 12 Oct 2018 13:46:10 +0200
Subject: [PATCH] Another proposal. Fixes the bug and logs a warning
---
src/caosdb/configuration.py | 22 ++---
.../external_credentials_provider.py | 91 +++++++++++++++++++
.../connection/authentication/interface.py | 33 +++++--
.../connection/authentication/keyring.py | 37 +++-----
src/caosdb/connection/authentication/pass.py | 37 +++-----
src/caosdb/connection/authentication/plain.py | 1 +
src/caosdb/exceptions.py | 23 +++++
tox.ini | 5 +-
unittests/test_authentication_external.py | 35 +++++++
unittests/test_authentication_keyring.py | 40 ++++++++
unittests/test_authentication_pass.py | 42 +++++++++
unittests/test_authentication_plain.py | 5 +
12 files changed, 303 insertions(+), 68 deletions(-)
create mode 100644 src/caosdb/connection/authentication/external_credentials_provider.py
create mode 100644 unittests/test_authentication_external.py
create mode 100644 unittests/test_authentication_keyring.py
create mode 100644 unittests/test_authentication_pass.py
diff --git a/src/caosdb/configuration.py b/src/caosdb/configuration.py
index 8b4af56e..9f6b558a 100644
--- a/src/caosdb/configuration.py
+++ b/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"}}
diff --git a/src/caosdb/connection/authentication/external_credentials_provider.py b/src/caosdb/connection/authentication/external_credentials_provider.py
new file mode 100644
index 00000000..8164d49b
--- /dev/null
+++ b/src/caosdb/connection/authentication/external_credentials_provider.py
@@ -0,0 +1,91 @@
+# -*- 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)
diff --git a/src/caosdb/connection/authentication/interface.py b/src/caosdb/connection/authentication/interface.py
index d3512174..fe9cb917 100644
--- a/src/caosdb/connection/authentication/interface.py
+++ b/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.
diff --git a/src/caosdb/connection/authentication/keyring.py b/src/caosdb/connection/authentication/keyring.py
index da9e7d41..abdbf112 100644
--- a/src/caosdb/connection/authentication/keyring.py
+++ b/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)
diff --git a/src/caosdb/connection/authentication/pass.py b/src/caosdb/connection/authentication/pass.py
index ed8c1991..9399fc4f 100644
--- a/src/caosdb/connection/authentication/pass.py
+++ b/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
diff --git a/src/caosdb/connection/authentication/plain.py b/src/caosdb/connection/authentication/plain.py
index 6cb7b1b8..83dd5929 100644
--- a/src/caosdb/connection/authentication/plain.py
+++ b/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
diff --git a/src/caosdb/exceptions.py b/src/caosdb/exceptions.py
index 581acc2c..485ad908 100644
--- a/src/caosdb/exceptions.py
+++ b/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):
diff --git a/tox.ini b/tox.ini
index 5a7cee25..11866fb9 100644
--- a/tox.ini
+++ b/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}
diff --git a/unittests/test_authentication_external.py b/unittests/test_authentication_external.py
new file mode 100644
index 00000000..c595d38f
--- /dev/null
+++ b/unittests/test_authentication_external.py
@@ -0,0 +1,35 @@
+"""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"
diff --git a/unittests/test_authentication_keyring.py b/unittests/test_authentication_keyring.py
new file mode 100644
index 00000000..721adefd
--- /dev/null
+++ b/unittests/test_authentication_keyring.py
@@ -0,0 +1,40 @@
+# -*- 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"
diff --git a/unittests/test_authentication_pass.py b/unittests/test_authentication_pass.py
new file mode 100644
index 00000000..968fdfd9
--- /dev/null
+++ b/unittests/test_authentication_pass.py
@@ -0,0 +1,42 @@
+# -*- 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"
diff --git a/unittests/test_authentication_plain.py b/unittests/test_authentication_plain.py
index 5e9c3c3d..6b59a231 100644
--- a/unittests/test_authentication_plain.py
+++ b/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"
--
GitLab