vdk-control-api-auth: vdk credentials cache refactoring (#2606)

This is refactoring the credentials cache storage system to make it more
reusable . Particulary I want to reuse it to get corrected authenticated
user in telemetry later.

- We are providing explcit interface for credentials cache intead of
using Union of classes
- Implement SingletonInMemoryCredentials cache to enable different parts
of the program to easily access and reuse the same credentials.
- Added method read_cached_access_token_only and use it in
get_authenticated_username() to make it easier to record user using vdk
even if his token has expired.
- Default credentials_cache to LocalFolderCredentialsCache which is more
commonly used.
- Deprecate cache_locally in favour of explicitly passing the credential
cache instance, this is better design pattern.

projects/vdk-plugins/vdk-control-api-auth/src/vdk/plugin/control_api_auth/auth_config.py

@@ -1,8 +1,10 @@
 # Copyright 2021-2023 VMware, Inc.
 # SPDX-License-Identifier: Apache-2.0
+import abc
 import logging
 import os
 import pathlib
+from abc import abstractmethod
 from pathlib import Path
 from typing import Optional
 
@@ -49,9 +51,30 @@ def local_config_folder(self) -> str:
         return os.getenv("VDK_BASE_CONFIG_FOLDER", str(Path.home()))
 
 
-class InMemAuthConfiguration:
+class CredentialsCache(abc.ABC):
+    @abstractmethod
+    def save_credentials(self, content: str) -> None:
+        """
+        Save the credentials data securely
+        :param content: the content of the credentials. It's a text based format.
+        :return:
+        """
+
+    def delete_credentials(self) -> None:
+        """
+        Delete any credentials data stored.
+        """
+
+    def read_credentials(self) -> Optional[str]:
+        """
+        Read the stored credentials data
+        :return: the data
+        """
+
+
+class InMemoryCredentialsCache(CredentialsCache):
     """
-    A class providing the equivalent of an in-memory AuthConfigFolder.
+    A class providing the equivalent of an in-memory LocalFolderCredentialsCache.
     This configuration is available as long as the python process is running,
     and is lost when the process is terminated.
     """
@@ -62,16 +85,31 @@ def __init__(self):
         self._config = dict()
 
     def save_credentials(self, content: str) -> None:
-        self._config[InMemAuthConfiguration.CREDS_KEY] = content
+        self._config[InMemoryCredentialsCache.CREDS_KEY] = content
 
     def delete_credentials(self) -> None:
-        self._config[InMemAuthConfiguration.CREDS_KEY] = ""
+        self._config[InMemoryCredentialsCache.CREDS_KEY] = ""
 
     def read_credentials(self) -> Optional[str]:
-        return self._config.get(InMemAuthConfiguration.CREDS_KEY)
+        return self._config.get(InMemoryCredentialsCache.CREDS_KEY)
 
 
-class AuthConfigFolder:
+class SingletonInMemoryCredentialsCache:
+    """
+    Singleton class for InMemAuthConfiguration,
+    ensuring that there is only one instance of
+    InMemAuthConfiguration with shared state across the application.
+    """
+
+    _instance = None
+
+    def __new__(cls):
+        if cls._instance is None:
+            cls._instance = InMemoryCredentialsCache()
+        return cls._instance
+
+
+class LocalFolderCredentialsCache(CredentialsCache):
     """
     A class responsible for managing the configuration files in the vdk configuration
     folder
@@ -84,7 +122,7 @@ class AuthConfigFolder:
 
     def __init__(self, base_dir=AuthConfig().local_config_folder):
         self.vdk_config_folder = os.path.join(
-            base_dir, AuthConfigFolder.CONFIG_FOLDER_NAME
+            base_dir, LocalFolderCredentialsCache.CONFIG_FOLDER_NAME
         )
         if os.path.isfile(self.vdk_config_folder):
             raise VDKAuthOSError(
@@ -154,4 +192,10 @@ def read_credentials(self):
             )
 
     def __get_cred_file(self):
-        return os.path.join(self.vdk_config_folder, AuthConfigFolder.CREDENTIALS_FILE)
+        return os.path.join(
+            self.vdk_config_folder, LocalFolderCredentialsCache.CREDENTIALS_FILE
+        )
+
+
+# Keep for backwards compatability the old name until needed
+AuthConfigFolder = LocalFolderCredentialsCache

projects/vdk-plugins/vdk-control-api-auth/src/vdk/plugin/control_api_auth/authentication.py

@@ -1,10 +1,12 @@
 # Copyright 2021-2023 VMware, Inc.
 # SPDX-License-Identifier: Apache-2.0
 import logging
-from typing import List
+import warnings
 from typing import Optional
 
-from vdk.plugin.control_api_auth.auth_config import InMemAuthConfiguration
+from vdk.plugin.control_api_auth.auth_config import CredentialsCache
+from vdk.plugin.control_api_auth.auth_config import LocalFolderCredentialsCache
+from vdk.plugin.control_api_auth.auth_config import SingletonInMemoryCredentialsCache
 from vdk.plugin.control_api_auth.auth_exception import VDKInvalidAuthParamError
 from vdk.plugin.control_api_auth.autorization_code_auth import RedirectAuthentication
 from vdk.plugin.control_api_auth.base_auth import BaseAuth
@@ -26,8 +28,9 @@ def __init__(
         authorization_url: str = None,
         auth_discovery_url: str = None,
         auth_type: str = None,
-        cache_locally: bool = False,
+        cache_locally: bool = None,
         possible_jwt_user_keys=None,
+        credentials_cache: CredentialsCache = LocalFolderCredentialsCache(),
     ):
         """
         :param username: A user's username in case basic authentication is used.
@@ -52,9 +55,12 @@ def __init__(
         :param cache_locally:
             A flag, indicating if credentials should be cached locally (in a
             file).
+            Deprecated: use credentials_cache instead to pass how credentials are stored explicitly.
         :param possible_jwt_user_keys:
             Used by get_authenticated_username to try to discover correct username if OAuth2 and JWT token is used. It is a list of keys where the first existing key in a JTW token is returned.
             Defaults to some common user keys.
+        :param credentials_cache:
+            The configuration store used to actually store the credentials
         """
         self._username = username
         self._password = password
@@ -64,11 +70,20 @@ def __init__(
         self._auth_url = authorization_url
         self._auth_discovery_url = auth_discovery_url
         self._auth_type = auth_type
-        # Check if credentials should be cached on the local filesystem
-        if cache_locally:
-            self._auth = BaseAuth()
-        else:
-            self._auth = BaseAuth(conf=InMemAuthConfiguration())
+
+        if cache_locally is not None:
+            warnings.warn(
+                "Authentication constructor cache_locally argument is deprecated. "
+                "Use credentials_cache to pass explicitly the credential cache store."
+            )
+            credentials_cache = (
+                LocalFolderCredentialsCache()
+                if cache_locally
+                else SingletonInMemoryCredentialsCache()
+            )
+
+        self._auth = BaseAuth(credentials_cache)
+
         if possible_jwt_user_keys:
             self._possible_jwt_user_keys = possible_jwt_user_keys
         else:  # sensible defaults
@@ -123,6 +138,8 @@ def get_authenticated_username(self) -> Optional[str]:
                 self._token = self.read_access_token()
             except Exception:
                 log.debug(f"Could not to read access token.", exc_info=True)
+        if not self._token:
+            self._token = self._auth.read_cached_access_token_only()
         if not self._token:
             return None
 

projects/vdk-plugins/vdk-control-api-auth/src/vdk/plugin/control_api_auth/base_auth.py

@@ -4,14 +4,13 @@
 import logging
 import time
 from typing import Optional
-from typing import Union
 
 from requests import post
 from requests.auth import HTTPBasicAuth
 from requests_oauthlib import OAuth2Session
 from vdk.plugin.control_api_auth.auth_config import AuthConfig
-from vdk.plugin.control_api_auth.auth_config import AuthConfigFolder
-from vdk.plugin.control_api_auth.auth_config import InMemAuthConfiguration
+from vdk.plugin.control_api_auth.auth_config import CredentialsCache
+from vdk.plugin.control_api_auth.auth_config import LocalFolderCredentialsCache
 from vdk.plugin.control_api_auth.auth_exception import VDKAuthException
 from vdk.plugin.control_api_auth.auth_request_values import AuthRequestValues
 from vdk.plugin.control_api_auth.login_types import LoginTypes
@@ -96,7 +95,7 @@ class BaseAuth:
 
     def __init__(
         self,
-        conf: Union[AuthConfigFolder, InMemAuthConfiguration] = AuthConfigFolder(),
+        conf: CredentialsCache = LocalFolderCredentialsCache(),
         vdk_config=AuthConfig(),
     ):
         self._conf = conf
@@ -172,8 +171,8 @@ def __exchange_refresh_for_access_token(self):
 
     def read_access_token(self) -> Optional[str]:
         """
-        Read access token from _cache or fetch it from Authorization server.
-        If not available in _cache it will get it using provided configuration during VDK CLI login to fetch it.
+        Read access token from cache or fetch it from Authorization server.
+        If not available in cache it will get it using provided configuration during VDK CLI login to fetch it.
         If it detects that token is about to expire it will try to refresh it.
         :return: the access token or None if it cannot detect any credentials.
         """
@@ -185,6 +184,14 @@ def read_access_token(self) -> Optional[str]:
             self.acquire_and_cache_access_token()
         return self._cache.access_token
 
+    def read_cached_access_token_only(self) -> Optional[str]:
+        """
+        Read access token from credentials cache and will return it if available.
+        Will not check if it is valid (for example not expired) but return it directly.
+        Will not try to fetch it form Authorization provider if it is not available. Instead, will simply return None
+        """
+        return self._cache.access_token
+
     def acquire_and_cache_access_token(self):
         """
         Acquires and caches access token

projects/vdk-plugins/vdk-control-api-auth/tests/test_api_token_auth.py

@@ -4,6 +4,7 @@
 from pytest_httpserver.pytest_plugin import PluginHTTPServer
 from test_core_auth import allow_oauthlib_insecure_transport
 from test_core_auth import get_json_response_mock
+from vdk.plugin.control_api_auth.auth_config import InMemoryCredentialsCache
 from vdk.plugin.control_api_auth.auth_exception import VDKInvalidAuthParamError
 from vdk.plugin.control_api_auth.authentication import Authentication
 
@@ -16,14 +17,19 @@ def test_api_token_success_authentication(httpserver: PluginHTTPServer):
         token="apitoken",
         authorization_url=httpserver.url_for("/foo"),
         auth_type="api-token",
+        credentials_cache=InMemoryCredentialsCache(),
     )
     auth.authenticate()
 
     assert auth.read_access_token() == "axczfe12casASDCz"
 
 
 def test_api_token_no_auth_url():
-    auth = Authentication(token="apitoken", auth_type="api-token")
+    auth = Authentication(
+        token="apitoken",
+        auth_type="api-token",
+        credentials_cache=InMemoryCredentialsCache(),
+    )
 
     with pytest.raises(VDKInvalidAuthParamError) as exc_info:
         auth.authenticate()
@@ -35,7 +41,9 @@ def test_api_token_no_auth_url():
 def test_api_token_no_auth_type_specified(httpserver: PluginHTTPServer):
     httpserver.expect_request("/foo").respond_with_json(get_json_response_mock())
     auth = Authentication(
-        token="apitoken", authorization_url=httpserver.url_for("/foo")
+        token="apitoken",
+        authorization_url=httpserver.url_for("/foo"),
+        credentials_cache=InMemoryCredentialsCache(),
     )
 
     with pytest.raises(VDKInvalidAuthParamError) as exc_info:

projects/vdk-plugins/vdk-control-api-auth/tests/test_authorization_code_auth.py

@@ -4,18 +4,18 @@
 from pytest_httpserver.pytest_plugin import PluginHTTPServer
 from test_core_auth import allow_oauthlib_insecure_transport
 from test_core_auth import get_json_response_mock
+from vdk.plugin.control_api_auth.auth_config import InMemoryCredentialsCache
 from vdk.plugin.control_api_auth.auth_exception import VDKInvalidAuthParamError
 from vdk.plugin.control_api_auth.auth_exception import VDKLoginFailedError
 from vdk.plugin.control_api_auth.authentication import Authentication
 from vdk.plugin.control_api_auth.autorization_code_auth import LoginHandler
 from vdk.plugin.control_api_auth.autorization_code_auth import RedirectAuthentication
 from vdk.plugin.control_api_auth.base_auth import BaseAuth
-from vdk.plugin.control_api_auth.base_auth import InMemAuthConfiguration
 
 
 def test_verify_redirect_url():
     allow_oauthlib_insecure_transport()
-    in_mem_conf = InMemAuthConfiguration()
+    in_mem_conf = InMemoryCredentialsCache()
     auth = BaseAuth(in_mem_conf)
 
     auth = RedirectAuthentication(
@@ -38,7 +38,7 @@ def test_verify_redirect_url():
 def test_login_handler_exceptions(httpserver: PluginHTTPServer):
     allow_oauthlib_insecure_transport()
     httpserver.expect_request("/foo").respond_with_json(get_json_response_mock())
-    in_mem_conf = InMemAuthConfiguration()
+    in_mem_conf = InMemoryCredentialsCache()
     auth = BaseAuth(in_mem_conf)
 
     handler = LoginHandler(
@@ -75,6 +75,7 @@ def test_authorization_code_no_secret(httpserver: PluginHTTPServer):
         token="apitoken",
         authorization_url=httpserver.url_for("/foo"),
         auth_type="credentials",
+        credentials_cache=InMemoryCredentialsCache(),
     )
     with pytest.raises(VDKInvalidAuthParamError) as exc_info:
         auth.authenticate()

projects/vdk-plugins/vdk-control-api-auth/tests/test_core_auth.py

@@ -10,7 +10,7 @@
 from unittest.mock import patch
 
 from pytest_httpserver.pytest_plugin import PluginHTTPServer
-from vdk.plugin.control_api_auth.auth_config import InMemAuthConfiguration
+from vdk.plugin.control_api_auth.auth_config import InMemoryCredentialsCache
 from vdk.plugin.control_api_auth.base_auth import AuthenticationCache
 from vdk.plugin.control_api_auth.base_auth import AuthenticationCacheSerDe
 from vdk.plugin.control_api_auth.base_auth import BaseAuth
@@ -41,10 +41,12 @@ def test_auth_cache_backwards_compatibility_missing_keys():
     assert cache.access_token is None
 
 
-# The Following tests use the AuthConfigFolder to test configuration storage
+# The Following tests use the LocalFolderCredentialsCache to test configuration storage
 # ==========================================================================
 def test_auth_updates():
-    with patch("vdk.plugin.control_api_auth.auth_config.AuthConfigFolder") as mock_conf:
+    with patch(
+        "vdk.plugin.control_api_auth.auth_config.LocalFolderCredentialsCache"
+    ) as mock_conf:
         # after login cache is populated with auth uri and refresh token
         test_cache = AuthenticationCache()
         mock_conf.read_credentials.return_value = AuthenticationCacheSerDe.serialize(
@@ -65,7 +67,9 @@ def test_auth_updates():
 
 def test_auth_acquire_token(httpserver: PluginHTTPServer):
     allow_oauthlib_insecure_transport()
-    with patch("vdk.plugin.control_api_auth.auth_config.AuthConfigFolder") as mock_conf:
+    with patch(
+        "vdk.plugin.control_api_auth.auth_config.LocalFolderCredentialsCache"
+    ) as mock_conf:
         httpserver.expect_request("/foo").respond_with_json(get_json_response_mock())
 
         test_cache = AuthenticationCache(
@@ -89,7 +93,9 @@ def test_auth_acquire_token(httpserver: PluginHTTPServer):
 
 def test_auth_read_access_token_expired(httpserver: PluginHTTPServer):
     allow_oauthlib_insecure_transport()
-    with patch("vdk.plugin.control_api_auth.auth_config.AuthConfigFolder") as mock_conf:
+    with patch(
+        "vdk.plugin.control_api_auth.auth_config.LocalFolderCredentialsCache"
+    ) as mock_conf:
         httpserver.expect_request("/foo").respond_with_json(get_json_response_mock())
 
         test_cache = AuthenticationCache(
@@ -115,7 +121,9 @@ def test_auth_read_access_token_expired(httpserver: PluginHTTPServer):
 
 
 def test_auth_read_access_token():
-    with patch("vdk.plugin.control_api_auth.auth_config.AuthConfigFolder") as mock_conf:
+    with patch(
+        "vdk.plugin.control_api_auth.auth_config.LocalFolderCredentialsCache"
+    ) as mock_conf:
         test_cache = AuthenticationCache(
             "http://foo", "refresh", "access-token", time() + 200
         )
@@ -137,7 +145,7 @@ def test_auth_read_access_token():
 # ==========================================================================
 def test_auth_updates_in_memory():
     # after login cache is populated with auth uri and refresh token
-    in_mem_conf = InMemAuthConfiguration()
+    in_mem_conf = InMemoryCredentialsCache()
     auth = BaseAuth(in_mem_conf)
     auth.update_refresh_token("refresh")
     auth.update_access_token("access")
@@ -151,7 +159,7 @@ def test_auth_updates_in_memory():
 def test_auth_acquire_token_in_memory(httpserver: PluginHTTPServer):
     allow_oauthlib_insecure_transport()
     httpserver.expect_request("/foo").respond_with_json(get_json_response_mock())
-    in_mem_conf = InMemAuthConfiguration()
+    in_mem_conf = InMemoryCredentialsCache()
 
     auth = BaseAuth(conf=in_mem_conf)
 
@@ -176,7 +184,7 @@ def test_auth_read_access_token_expired_in_memory(httpserver: PluginHTTPServer):
         access_token="expired-token",
         access_token_expiration_time=time(),
     )
-    in_mem_conf = InMemAuthConfiguration()
+    in_mem_conf = InMemoryCredentialsCache()
     in_mem_conf.save_credentials(AuthenticationCacheSerDe.serialize(test_cache))
 
     auth = BaseAuth(in_mem_conf)
@@ -192,7 +200,7 @@ def test_auth_read_access_token_in_memory():
     test_cache = AuthenticationCache(
         "http://foo", "refresh", "access-token", time() + 200
     )
-    in_mem_conf = InMemAuthConfiguration()
+    in_mem_conf = InMemoryCredentialsCache()
     in_mem_conf.save_credentials(AuthenticationCacheSerDe.serialize(test_cache))
 
     auth = BaseAuth(in_mem_conf)