Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Invalidation of sessions to due frequency of calls #23

Open
jakubgs opened this issue Feb 10, 2022 · 5 comments
Open

Invalidation of sessions to due frequency of calls #23

jakubgs opened this issue Feb 10, 2022 · 5 comments

Comments

@jakubgs
Copy link

jakubgs commented Feb 10, 2022

I've been using this plugin for a while and I appreciate it being freely available. I've been seeing issues with sessions invalidating.

Specifically it happens when I run multiple playbooks on big numbers of hosts, which causes the API to hit some kind of rate llimit:

An unhandled exception occurred while templating '{{lookup("bitwarden", "example", field="username")}}'.
Error was a <class 'ansible.errors.AnsibleError'>,
original message: An unhandled exception occurred while running the lookup plugin 'bitwarden'.
Error was a <class 'ansible.errors.AnsibleError'>,
original message: Error accessing Bitwarden vault. Run 'bw login' to login.

This becomes quite an issue when trying to make big changes rapidly.

@jakubgs
Copy link
Author

jakubgs commented Feb 10, 2022

One issue appears to be the fact that the plugin calls both bw --version and bw status for each credential required:

image

Which is pretty absurd if you ask me.

@jakubgs
Copy link
Author

jakubgs commented Feb 10, 2022

The first issue can be easily fixed by using shutil.which to check if the tool is available:

diff --git a/ansible/lookup_plugins/bitwarden.py b/ansible/lookup_plugins/bitwarden.py
index 48c3c1f..e78aa63 100755
--- a/ansible/lookup_plugins/bitwarden.py
+++ b/ansible/lookup_plugins/bitwarden.py
@@ -19,6 +19,7 @@ import json
 import os
 import sys
 
+from shutil import which
 from subprocess import Popen, PIPE, STDOUT, check_output
 
 from ansible.errors import AnsibleError
@@ -70,9 +71,7 @@ class Bitwarden(object):
     def __init__(self, path):
         self._cli_path = path
         self._bw_session = ""
-        try:
-            check_output([self._cli_path, "--version"])
-        except OSError:
+        if which("bw") is None:
             raise AnsibleError("Command not found: {0}".format(self._cli_path))
 
     @property

Which gets rid of the first batch of pointless mass bw --version calls.

@jakubgs
Copy link
Author

jakubgs commented Feb 10, 2022

If we look at BitWarden CLI source we can see how checking status works:

  private async status(): Promise<string> {
    const authed = await this.stateService.getIsAuthenticated();
    if (!authed) {
      return "unauthenticated";
    }

    const isLocked = await this.vaultTimeoutService.isLocked();
    return isLocked ? "locked" : "unlocked";
  }

https://github.com/bitwarden/cli/blob/2ae2fdfd/src/commands/status.command.ts#L44-L52

Which calls their jslib:

  async getIsAuthenticated(options?: StorageOptions): Promise<boolean> {
    return (await this.getAccessToken(options)) != null && (await this.getUserId(options)) != null;
  }

https://github.com/bitwarden/jslib/blob/2ae2fdfd/common/src/services/state.service.ts#L1519-L1521

So it tries to get an access token from the API and check user ID to verify state of session.

@jakubgs
Copy link
Author

jakubgs commented Feb 10, 2022

The thing is, I don't see a reason to check bw status if calls like bw get item fail with the same unauthenticated error anyway.

What's the point of the bw status call? Am I missing something? It seems to bring no value and just adds unnecessary calls.

@jakubgs
Copy link
Author

jakubgs commented Feb 10, 2022

I've made my own fork, and applied the two fixes recommended here(among others):

  • 0dbd2ef5 - drop use of bw --version call to check availability
  • c1a973d5 - drop logged_in check and calls to bw status

It works well for me. Maybe it is useful to someone else.

jakubgs added a commit to status-im/infra-nimbus that referenced this issue Feb 10, 2022
jakubgs added a commit to status-im/infra-nimbus that referenced this issue Feb 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant