1Password/connect-sdk-python

Example for onepasswordconnectsdk.load_dict is incorrect

kwilsonmg opened this issue · 3 comments

Your environment

SDK Version: N/A

Connect Server Version: N/A

OS: N/A

Python Version: N/A

What happened?

I am admittedly unsure what category to file this under, but I have discovered an error in USAGE.md. The load_dict example is inaccurate and will result in errors unless each item has opvault defined. If run without the fix (as below, though with values filled in) you get the following Traceback (or similar). This traceback also explicitly states the problem, which my pull request resolves.

# example dict configuration for onepasswordconnectsdk.load_dict(connect_client, CONFIG)
CONFIG = {
    "server": {
        "opitem": "My database item",
        "opfield": "specific_section.hostname",
        "opvault": "some_vault_id",
    },
    "database": {
        "opitem": "My database item",
        "opfield": ".database",
    },
    "username": {
        "opitem": "My database item",
        "opfield": ".username",
    },
    "password": {
        "opitem": "My database item",
        "opfield": ".password",
    },
}

values_dict = onepasswordconnectsdk.load_dict(connect_client, CONFIG)
Traceback (most recent call last):
  File "/Users/kylewilson/Documents/1PasswordTest/passwordtest.py", line 96, in <module>
    values_dict = onepasswordconnectsdk.load_dict(connect_client, CONFIG)
                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/kylewilson/Documents/1PasswordTest/.venv/lib/python3.11/site-packages/onepasswordconnectsdk/config.py", line 65, in load_dict
    item_vault = _vault_uuid_for_field(field=field, vault_tag=vault_tag)
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/kylewilson/Documents/1PasswordTest/.venv/lib/python3.11/site-packages/onepasswordconnectsdk/config.py", line 158, in _vault_uuid_for_field
    raise NoVaultSetForFieldException(
onepasswordconnectsdk.config.NoVaultSetForFieldException: There         is no vault for database field

What did you expect to happen?

It should be as follows. This actually functions when tested:

# example dict configuration for onepasswordconnectsdk.load_dict(connect_client, CONFIG)
CONFIG = {
    "server": {
        "opitem": "My database item",
        "opfield": "specific_section.hostname",
        "opvault": "some_vault_id",
    },
    "database": {
        "opitem": "My database item",
        "opfield": ".database",
        "opvault": "some_vault_id",
    },
    "username": {
        "opitem": "My database item",
        "opfield": ".username",
        "opvault": "some_vault_id",
    },
    "password": {
        "opitem": "My database item",
        "opfield": ".password",
        "opvault": "some_vault_id",
    },
}

values_dict = onepasswordconnectsdk.load_dict(connect_client, CONFIG)

Steps to reproduce

  1. Run the example as-is, though obviously filling in the opitem and opvault fields where already present. Do not add any other fields.

Notes & Logs

Hi @kwilsonmg . Good catch! Thanks for raising this!

Looking closer into that, I see that the error is thrown as the vaultID is not provided. However if go down the flow it uses item: Item = client.get_item(parsed_item.item_title, parsed_item.vault_uuid), which can find an item by title/ID only, so the vault_uuid is optional here.

That makes me think that the fix for that might be slightly different than changing the USAGE.md file. Simply saying, not throw an error if the opvault value is not provided.

So are you saying that _vault_uuid_for_field should be redefined to not throw an error or that we should try/except the error when that function is called in load_dict? @volodymyrZotov

I'd say we just don't need to throw an error from _vault_uuid_for_field as we should be able to find an item just by item_uuid.