From 29a6b3121e10a38c917973cbd45f2906689efdc7 Mon Sep 17 00:00:00 2001 From: RoyK Date: Fri, 20 Aug 2021 14:50:34 +0200 Subject: [PATCH 1/6] feat[bw]: add cache to Bitwarden class Adds cache decorator using Python dictionary. Now each value gathered from bw via the CLI is store in the cache. If the same exact field and key is requested, then initiall the data from the cache is returned instead of executing BW again, which is a lot slower. --- lookup_plugins/bitwarden.py | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/lookup_plugins/bitwarden.py b/lookup_plugins/bitwarden.py index 03a7828..ba8f270 100755 --- a/lookup_plugins/bitwarden.py +++ b/lookup_plugins/bitwarden.py @@ -68,6 +68,7 @@ class Bitwarden(object): def __init__(self, path): self._cli_path = path self._bw_session = "" + self.cache = dict() try: check_output([self._cli_path, "--version"]) except OSError: @@ -93,6 +94,19 @@ def logged_in(self): else: return False + def cache(func): + def inner(*args, **kwargs): + self = args[0] + cache_key = '_'.join(args[1:]) + + if cache_key not in self.cache: + value = func(*args, **kwargs) + self.cache[cache_key] = value + + return self.cache[cache_key] + + return inner + def _run(self, args): my_env = os.environ.copy() if self.session != "": @@ -132,13 +146,16 @@ def status(self): raise AnsibleError("Error decoding Bitwarden status: %s" % e) return data['status'] + @cache def get_entry(self, key, field): return self._run(["get", field, key]) + @cache def get_notes(self, key): data = json.loads(self.get_entry(key, 'item')) return data['notes'] + @cache def get_custom_field(self, key, field): data = json.loads(self.get_entry(key, 'item')) return next(x for x in data['fields'] if x['name'] == field)['value'] From e64fc3fefa13714c5945500c55fd9276fdedfede Mon Sep 17 00:00:00 2001 From: RoyK Date: Fri, 20 Aug 2021 14:52:38 +0200 Subject: [PATCH 2/6] feat[Lookup]: make Bitwarden object persist For the caching functionality to work properly, the Bitwarden object should live as long as the Lookup module exists, instead of creating a new object, and thus a new cache, for each lookup. Therefore the Bitwarden object is persisted in the initial lookup and reused after that. --- lookup_plugins/bitwarden.py | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/lookup_plugins/bitwarden.py b/lookup_plugins/bitwarden.py index ba8f270..683275e 100755 --- a/lookup_plugins/bitwarden.py +++ b/lookup_plugins/bitwarden.py @@ -168,10 +168,17 @@ def get_attachments(self, key, itemid, output): class LookupModule(LookupBase): + def __init__(self, *args, **kwargs): + self.bw = None + + # Init super + super().__init__(*args, **kwargs) + def run(self, terms, variables=None, **kwargs): - bw = Bitwarden(path=kwargs.get('path', 'bw')) + if not self.bw: + self.bw = Bitwarden(path=kwargs.get('path', 'bw')) - if not bw.logged_in: + if not self.bw.logged_in: raise AnsibleError("Not logged into Bitwarden: please run " "'bw login', or 'bw unlock' and set the " "BW_SESSION environment variable first") @@ -180,26 +187,26 @@ def run(self, terms, variables=None, **kwargs): values = [] if kwargs.get('sync'): - bw.sync() + self.bw.sync() if kwargs.get('session'): - bw.session = kwargs.get('session') + self.bw.session = kwargs.get('session') for term in terms: if kwargs.get('custom_field'): - values.append(bw.get_custom_field(term, field)) + values.append(self.bw.get_custom_field(term, field)) elif field == 'notes': - values.append(bw.get_notes(term)) + values.append(self.bw.get_notes(term)) elif kwargs.get('attachments'): if kwargs.get('itemid'): itemid = kwargs.get('itemid') output = kwargs.get('output', term) - values.append(bw.get_attachments(term, itemid, output)) + values.append(self.bw.get_attachments(term, itemid, output)) else: raise AnsibleError("Missing value for - itemid - " "Please set parameters as example: - " "itemid='f12345-d343-4bd0-abbf-4532222' ") else: - values.append(bw.get_entry(term, field)) + values.append(self.bw.get_entry(term, field)) return values From befbcc22c0b96a825341e4a5af86ded4bb38c2d1 Mon Sep 17 00:00:00 2001 From: RoyK Date: Fri, 20 Aug 2021 15:20:19 +0200 Subject: [PATCH 3/6] chore[bw]: move cache to global variable The `LookupModule` is initialised during every lookup. This results in a new Bitwarden object for every lookup removing the benefit of the cache. However, importing the bitwarden Lookup module is only done at specific stages during a playbook and not run for every lookup itself. So changing the cache to a global variable makes it persistent for as long as possible and thus making the cache actually benefical for improving the executing time. Fixes #11 --- lookup_plugins/bitwarden.py | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/lookup_plugins/bitwarden.py b/lookup_plugins/bitwarden.py index 683275e..723ccb7 100755 --- a/lookup_plugins/bitwarden.py +++ b/lookup_plugins/bitwarden.py @@ -63,12 +63,15 @@ - Items from Bitwarden vault """ +# Global variables +CACHE = dict() + class Bitwarden(object): def __init__(self, path): self._cli_path = path self._bw_session = "" - self.cache = dict() + try: check_output([self._cli_path, "--version"]) except OSError: @@ -97,13 +100,13 @@ def logged_in(self): def cache(func): def inner(*args, **kwargs): self = args[0] - cache_key = '_'.join(args[1:]) + key = '_'.join(args[1:]) - if cache_key not in self.cache: + if key not in CACHE: value = func(*args, **kwargs) - self.cache[cache_key] = value + CACHE[key] = value - return self.cache[cache_key] + return CACHE[key] return inner @@ -137,6 +140,9 @@ def _run(self, args): return out.strip() def sync(self): + global CACHE + + CACHE = dict() # Clear cache to prevent using old values in cache self._run(['sync']) def status(self): From 2d441b2a3b9f0c2338097c6cfb3289dcf95b192f Mon Sep 17 00:00:00 2001 From: RoyK Date: Fri, 20 Aug 2021 15:23:38 +0200 Subject: [PATCH 4/6] feat[bw]: make logged_in rely on global variable During every lookup the Bitwarden object verifies whether it is logged in. This executes CLI and HTTP calls which take time, which is unnecessary for most of the time. Therefore I've implemented a global variable which is undefined initially, thus ensuring that `logged_in` function is executed and otherwise use the previous versions. Manual verification shows that this decreases overall execution time of lookup plugin --- lookup_plugins/bitwarden.py | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/lookup_plugins/bitwarden.py b/lookup_plugins/bitwarden.py index 723ccb7..67d27f9 100755 --- a/lookup_plugins/bitwarden.py +++ b/lookup_plugins/bitwarden.py @@ -65,6 +65,7 @@ # Global variables CACHE = dict() +LOGGED_IN = None class Bitwarden(object): @@ -91,11 +92,13 @@ def cli_path(self): @property def logged_in(self): - # Parse Bitwarden status to check if logged in - if self.status() == 'unlocked': - return True - else: - return False + global LOGGED_IN + + if LOGGED_IN is None: + # Parse Bitwarden status to check if logged in + LOGGED_IN = (self.status() == 'unlocked') + + return LOGGED_IN def cache(func): def inner(*args, **kwargs): From 6eaefd8be765c555c54d8892c36a281605c2b66c Mon Sep 17 00:00:00 2001 From: RoyK Date: Fri, 20 Aug 2021 15:31:22 +0200 Subject: [PATCH 5/6] chore[bw]: remove unnecessary code --- lookup_plugins/bitwarden.py | 1 - 1 file changed, 1 deletion(-) diff --git a/lookup_plugins/bitwarden.py b/lookup_plugins/bitwarden.py index 67d27f9..46593ee 100755 --- a/lookup_plugins/bitwarden.py +++ b/lookup_plugins/bitwarden.py @@ -102,7 +102,6 @@ def logged_in(self): def cache(func): def inner(*args, **kwargs): - self = args[0] key = '_'.join(args[1:]) if key not in CACHE: From 2709e008fd4a3c5d5e429205bca12b344d5cc0b7 Mon Sep 17 00:00:00 2001 From: RoyK Date: Sun, 22 Aug 2021 15:41:35 +0200 Subject: [PATCH 6/6] refactor[bw]: reduce global variable to single bw instance By setting the BW object as a global instance, everything can be implemented in the Bitwarden class itself, thus reducing the number of global variables and making it more object oriented. --- lookup_plugins/bitwarden.py | 50 ++++++++++++++++--------------------- 1 file changed, 22 insertions(+), 28 deletions(-) diff --git a/lookup_plugins/bitwarden.py b/lookup_plugins/bitwarden.py index 46593ee..1d70c94 100755 --- a/lookup_plugins/bitwarden.py +++ b/lookup_plugins/bitwarden.py @@ -64,14 +64,15 @@ """ # Global variables -CACHE = dict() -LOGGED_IN = None +bw = None class Bitwarden(object): def __init__(self, path): self._cli_path = path self._bw_session = "" + self._cache = dict() + self._logged_in = None try: check_output([self._cli_path, "--version"]) @@ -92,23 +93,22 @@ def cli_path(self): @property def logged_in(self): - global LOGGED_IN - - if LOGGED_IN is None: + if self._logged_in is None: # Parse Bitwarden status to check if logged in - LOGGED_IN = (self.status() == 'unlocked') + self._logged_in = (self.status() == 'unlocked') - return LOGGED_IN + return self._logged_in def cache(func): def inner(*args, **kwargs): + self = args[0] key = '_'.join(args[1:]) - if key not in CACHE: + if key not in self._cache: value = func(*args, **kwargs) - CACHE[key] = value + self._cache[key] = value - return CACHE[key] + return self._cache[key] return inner @@ -142,9 +142,7 @@ def _run(self, args): return out.strip() def sync(self): - global CACHE - - CACHE = dict() # Clear cache to prevent using old values in cache + self._cache = dict() # Clear cache to prevent using old values in cache self._run(['sync']) def status(self): @@ -176,17 +174,13 @@ def get_attachments(self, key, itemid, output): class LookupModule(LookupBase): - def __init__(self, *args, **kwargs): - self.bw = None - - # Init super - super().__init__(*args, **kwargs) - def run(self, terms, variables=None, **kwargs): - if not self.bw: - self.bw = Bitwarden(path=kwargs.get('path', 'bw')) + global bw + + if not bw: + bw = Bitwarden(path=kwargs.get('path', 'bw')) - if not self.bw.logged_in: + if not bw.logged_in: raise AnsibleError("Not logged into Bitwarden: please run " "'bw login', or 'bw unlock' and set the " "BW_SESSION environment variable first") @@ -195,26 +189,26 @@ def run(self, terms, variables=None, **kwargs): values = [] if kwargs.get('sync'): - self.bw.sync() + bw.sync() if kwargs.get('session'): - self.bw.session = kwargs.get('session') + bw.session = kwargs.get('session') for term in terms: if kwargs.get('custom_field'): - values.append(self.bw.get_custom_field(term, field)) + values.append(bw.get_custom_field(term, field)) elif field == 'notes': - values.append(self.bw.get_notes(term)) + values.append(bw.get_notes(term)) elif kwargs.get('attachments'): if kwargs.get('itemid'): itemid = kwargs.get('itemid') output = kwargs.get('output', term) - values.append(self.bw.get_attachments(term, itemid, output)) + values.append(bw.get_attachments(term, itemid, output)) else: raise AnsibleError("Missing value for - itemid - " "Please set parameters as example: - " "itemid='f12345-d343-4bd0-abbf-4532222' ") else: - values.append(self.bw.get_entry(term, field)) + values.append(bw.get_entry(term, field)) return values