Merge pull request #20 from cyberark/increase-code-coverage

Increase code coverage to 100%

Jenkinsfile

@@ -23,7 +23,7 @@ pipeline {
       post {
         always {
           junit 'output/**/*.xml'
-          cobertura autoUpdateHealth: true, autoUpdateStability: true, coberturaReportFile: 'coverage.xml', conditionalCoverageTargets: '100, 0, 0', failUnhealthy: true, failUnstable: false, lineCoverageTargets: '99, 0, 0', maxNumberOfBuilds: 0, methodCoverageTargets: '100, 0, 0', onlyStable: false, sourceEncoding: 'ASCII', zoomCoverageChart: false
+          cobertura autoUpdateHealth: true, autoUpdateStability: true, coberturaReportFile: 'coverage.xml', conditionalCoverageTargets: '100, 0, 0', failUnhealthy: true, failUnstable: false, lineCoverageTargets: '100, 0, 0', maxNumberOfBuilds: 0, methodCoverageTargets: '100, 0, 0', onlyStable: false, sourceEncoding: 'ASCII', zoomCoverageChart: false
           ccCoverage("coverage.py")
         }
       }

conjur/client.py

@@ -40,8 +40,6 @@ class Client():
     #pylint: disable=too-many-arguments,too-many-locals
     def __init__(self,
                  account='default',
-                 api_class=Api,
-                 api_config_class=ApiConfig,
                  api_key=None,
                  ca_bundle=None,
                  debug=False,
@@ -67,7 +65,7 @@ def __init__(self,
             logging.info("Not all expected variables were provided. " \
                 "Using conjurrc as credential store...")
             try:
-                on_disk_config = dict(api_config_class())
+                on_disk_config = dict(ApiConfig())
 
                 # We want to retain any overrides that the user provided from params
                 # but only if those values are valid
@@ -81,22 +79,22 @@ def __init__(self,
 
         if api_key:
             logging.info("Using API key from parameters...")
-            self._api = api_class(api_key=api_key,
-                                  http_debug=http_debug,
-                                  login_id=login_id,
-                                  ssl_verify=ssl_verify,
-                                  **config)
+            self._api = Api(api_key=api_key,
+                            http_debug=http_debug,
+                            login_id=login_id,
+                            ssl_verify=ssl_verify,
+                            **config)
         elif password:
             logging.info("Creating API key with login ID/password combo...")
-            self._api = api_class(http_debug=http_debug,
-                                  ssl_verify=ssl_verify,
-                                  **config)
+            self._api = Api(http_debug=http_debug,
+                            ssl_verify=ssl_verify,
+                            **config)
             self._api.login(login_id, password)
         else:
             logging.info("Using API key with netrc credentials...")
-            self._api = api_class(http_debug=http_debug,
-                                  ssl_verify=ssl_verify,
-                                  **config)
+            self._api = Api(http_debug=http_debug,
+                            ssl_verify=ssl_verify,
+                            **config)
 
         logging.info("Client initialized")
 

test/test_api.py

@@ -3,6 +3,8 @@
 from datetime import datetime
 from unittest.mock import call, patch, MagicMock
 
+import urllib3
+
 from conjur.http import HttpVerb
 from conjur.endpoints import ConjurEndpoint
 
@@ -68,6 +70,27 @@ def verify_http_call(self, http_client, method, endpoint, *args,
                                             **extra_args,
                                             ssl_verify=ssl_verify)
 
+    def test_new_client_throws_error_when_no_url(self):
+        with self.assertRaises(Exception):
+            Api(login_id='mylogin', api_key='apikey', ssl_verify=False)
+
+    @patch('conjur.api.invoke_endpoint', return_value=MockClientResponse())
+    def test_new_client_delegates_ssl_verify_flag(self, mock_http_client):
+        Api(url='http://localhost', ssl_verify=True).login('myuser', 'mypass')
+        self.verify_http_call(mock_http_client, HttpVerb.GET, ConjurEndpoint.LOGIN,
+                              auth=('myuser', 'mypass'),
+                              api_token=False,
+                              ssl_verify=True)
+
+    @patch('conjur.api.invoke_endpoint', return_value=MockClientResponse())
+    def test_new_client_overrides_ssl_verify_flag_with_ca_bundle_if_provided(self, mock_http_client):
+        Api(url='http://localhost', ssl_verify=True,
+                ca_bundle='cabundle').login('myuser', 'mypass')
+        self.verify_http_call(mock_http_client, HttpVerb.GET, ConjurEndpoint.LOGIN,
+                              auth=('myuser', 'mypass'),
+                              api_token=False,
+                              ssl_verify='cabundle')
+
 
     @patch('conjur.api.invoke_endpoint', return_value=MockClientResponse())
     def test_login_invokes_http_client_correctly(self, mock_http_client):
@@ -77,6 +100,14 @@ def test_login_invokes_http_client_correctly(self, mock_http_client):
                               api_token=False,
                               ssl_verify=True)
 
+    def test_login_throws_error_when_username_not_provided(self):
+        with self.assertRaises(RuntimeError):
+            Api(url='http://localhost').login(None, 'mypass')
+
+    def test_login_throws_error_when_password_not_provided(self):
+        with self.assertRaises(RuntimeError):
+            Api(url='http://localhost').login('myuser', None)
+
     @patch('conjur.api.invoke_endpoint', return_value=MockClientResponse())
     def test_login_saves_login_id(self, _):
         api = Api(url='http://localhost')
@@ -85,6 +116,30 @@ def test_login_saves_login_id(self, _):
 
         self.assertEquals(api.login_id, 'myuser')
 
+    @patch('logging.warning')
+    @patch('conjur.api.invoke_endpoint', return_value=MockClientResponse())
+    def test_new_client_shows_warning_when_ssl_verify_is_false(self, mock_http_client,
+            logging_warn_func):
+        Api(url='http://localhost', login_id='mylogin', api_key='apikey',
+                ssl_verify=False)
+
+        calls = [
+            call("************************************************************"),
+            call("'ssl_verify' is False - YOU ARE VULNERABLE TO MITM ATTACKS!"),
+            call("************************************************************"),
+        ]
+        logging_warn_func.assert_has_calls(calls)
+
+    @patch('urllib3.disable_warnings')
+    @patch('logging.warning')
+    @patch('conjur.api.invoke_endpoint', return_value=MockClientResponse())
+    def test_new_client_disables_insecure_warnings_in_urllib_when_sslverify_is_false(self,
+            mock_http_client, logging_warn_func, disable_warning_func):
+        Api(url='http://localhost', login_id='mylogin', api_key='apikey',
+                ssl_verify=False)
+
+        disable_warning_func.assert_called_once_with(urllib3.exceptions.InsecureRequestWarning)
+
     @patch('conjur.api.invoke_endpoint', return_value=MockClientResponse())
     def test_if_api_token_is_missing_fetch_a_new_one(self, mock_http_client):
         api = Api(url='http://localhost')
@@ -133,6 +188,14 @@ def test_authenticate_invokes_http_client_correctly(self, mock_http_client):
                               api_token=False,
                               ssl_verify=True)
 
+    def test_authenticate_throws_error_without_login_id_specified(self):
+        with self.assertRaises(RuntimeError):
+            Api(url='http://localhost', api_key='apikey').authenticate()
+
+    def test_authenticate_throws_error_without_api_key_specified(self):
+        with self.assertRaises(RuntimeError):
+            Api(url='http://localhost', login_id='mylogin').authenticate()
+
     @patch('conjur.api.invoke_endpoint', return_value=MockClientResponse())
     def test_account_info_is_passed_down_to_http_call(self, mock_http_client):
         Api(url='http://localhost',

test/test_cli.py

@@ -1,9 +1,10 @@
 import unittest
+from unittest.mock import patch
 
 from .util.cli_helpers import cli_arg_test, cli_test
 
 from conjur.version import __version__
-
+from conjur.cli import Cli
 
 RESOURCE_LIST = [
     'some_id1',
@@ -16,6 +17,12 @@ class CliTest(unittest.TestCase):
     def test_cli_without_args_shows_help(self, cli_invocation, output, client):
         self.assertIn("usage: cli", output)
 
+    @patch('conjur.cli.Cli')
+    def test_cli_is_run_when_launch_is_invoked(self, cli_instance):
+        Cli.launch()
+
+        cli_instance.return_value.run.assert_called_once_with()
+
     @cli_test(["-h"])
     def test_cli_shows_help_with_short_help_flag(self, cli_invocation, output, client):
         self.assertIn("usage: cli", output)
@@ -63,6 +70,9 @@ def test_cli_passes_debug_short_flag_to_client(self): pass
     @cli_arg_test(["--debug"], debug=True)
     def test_cli_passes_debug_long_flag_to_client(self): pass
 
+    @cli_arg_test(["--verbose"], debug=True)
+    def test_cli_passes_verbose_as_the_debug_long_flag_to_client(self): pass
+
     # User
     @cli_arg_test(["-u", "myuser"], login_id='myuser')
     def test_cli_passes_login_id_short_flag_to_client(self): pass