From f491bb92502714968286899c72b954f2cfa01288 Mon Sep 17 00:00:00 2001
From: Guy Afik <53861351+GuyAfik@users.noreply.github.com>
Date: Mon, 27 Feb 2023 11:44:02 +0200
Subject: [PATCH] make authentication with api-key to be chosen before
 username/password (#109)

* use api key if both username and apikey exist

* add unit-tests

* add docstrings

* remove func parameters

* changelog

* add unit-test
---
 .gitignore                 |  3 ++
 CHANGELOG.md               |  3 ++
 demisto_client/__init__.py |  2 +-
 tests/mocks_test.py        | 86 +++++++++++++++++++++++++++++++++++++-
 4 files changed, 92 insertions(+), 2 deletions(-)

diff --git a/.gitignore b/.gitignore
index 63fb12a9..168468a4 100644
--- a/.gitignore
+++ b/.gitignore
@@ -94,3 +94,6 @@ git_push.sh
 .pytest_cache
 py27
 py37
+
+# PyCharm configurtion files
+.idea/
\ No newline at end of file
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 325a8105..d212c3a2 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -3,6 +3,9 @@
 [PyPI History][1]
 [1]: https://pypi.org/project/demisto-py/#history
 
+# 3.2.8
+* Fixed an issue where demisto-py used username/password authentication even when api-key was provided.
+
 ## 3.2.7
 * Added an ability to change `DEMISTO_BASE_URL` environment variable to api url, when using XSIAM.
 
diff --git a/demisto_client/__init__.py b/demisto_client/__init__.py
index b86ac76e..88981c26 100644
--- a/demisto_client/__init__.py
+++ b/demisto_client/__init__.py
@@ -109,7 +109,7 @@ def configure(base_url=None, api_key=None, verify_ssl=None, proxy=None, username
     if connection_pool_maxsize:
         configuration.connection_pool_maxsize = connection_pool_maxsize
 
-    if username is None or auth_id:
+    if api_key or auth_id:
         api_client = ApiClient(configuration)
         api_client.user_agent = 'demisto-py/' + __version__
         api_instance = demisto_api.DefaultApi(api_client)
diff --git a/tests/mocks_test.py b/tests/mocks_test.py
index 0de52a1d..2746ec0f 100644
--- a/tests/mocks_test.py
+++ b/tests/mocks_test.py
@@ -1,5 +1,6 @@
+import os
 import unittest
-
+import pytest
 from urllib3_mock import Responses
 import demisto_client
 from datetime import datetime
@@ -410,3 +411,86 @@ def test_import_incidentfields(mocker):
     else:
         assert hasattr(res, 'error')
         assert res.error == 'Partial Error Description'
+
+
+CONFIGURE_TEST_PARAMS = [
+        (
+            'api_key', 'username', False
+        ),
+        (
+            None, 'username', True
+        ),
+        (
+            'api_key', None, False
+        ),
+        (
+             None, None, False
+        ),
+    ]
+
+
+@pytest.mark.parametrize(
+    "_api_key, username, should_login_called",
+    CONFIGURE_TEST_PARAMS
+)
+def test_configure_client_no_env_vars(mocker, _api_key, username, should_login_called):
+    """
+    Given:
+        Case A: both api key and username were provided
+        Case B: only username was provided
+        Case C: only api key was provided
+
+    When:
+        configuring the client
+
+    Then:
+        Case A: make sure login was not called and authentication is via api key
+        Case B: make sure login was called and authentication is via username/password
+        Case C: make sure login was not called and authentication is via api key
+    """
+    login_mocker = mocker.patch.object(demisto_client, 'login')
+    if not _api_key and not username:
+        with pytest.raises(ValueError):
+            demisto_client.configure(base_url='test.com', api_key=_api_key, username=username)
+    else:
+        demisto_client.configure(base_url='test.com', api_key=_api_key, username=username)
+
+    assert login_mocker.called == should_login_called
+
+
+@pytest.mark.parametrize(
+    "_api_key, username, should_login_called",
+    CONFIGURE_TEST_PARAMS
+)
+def test_configure_client_env_vars(mocker, _api_key, username, should_login_called):
+    """
+    Given:
+        Case A: both api key and username were provided through environment variables
+        Case B: only username was provided through environment variable
+        Case C: only api key was provided through environment variable
+
+    When:
+        configuring the client
+
+    Then:
+        Case A: make sure login was not called and authentication is via api key
+        Case B: make sure login was called and authentication is via username/password
+        Case C: make sure login was not called and authentication is via api key
+    """
+    def getenv_side_effect(parameter):
+        if parameter == 'DEMISTO_API_KEY':
+            return _api_key
+        elif parameter == 'DEMISTO_USERNAME':
+            return username
+        return None
+
+    mocker.patch.object(os, 'getenv', side_effect=getenv_side_effect)
+
+    login_mocker = mocker.patch.object(demisto_client, 'login')
+    if not _api_key and not username:
+        with pytest.raises(ValueError):
+            demisto_client.configure(base_url='test.com')
+    else:
+        demisto_client.configure(base_url='test.com')
+
+    assert login_mocker.called == should_login_called