1Password / connect-sdk-python

Python SDK for 1Password Connect
https://developer.1password.com/docs/connect
MIT License
200 stars 31 forks source link

Failure to update items with files #57

Closed HaddadJoe closed 1 year ago

HaddadJoe commented 1 year ago

Your environment

SDK Version: 1.3.0 Connect Server Version: 1.5.7 OS: MacOS Montery 12.6 Python Version: 3.7.5

What happened?

Updating an item that contains a file fails with error

onepasswordconnectsdk.client.FailedToRetrieveItemException: Unable to post item. Received 400                    for /v1/vaults/********/items/***** with message: Validation: (validateVaultItem failed to Validate), Couldn't validate the item: "[ItemValidator] has found 1 errors, 0 warnings: \nErrors:{1. Field of unexpected type \"\" at \"item.details.documentAttributes\". Requires one of [\"object\"]}"

What did you expect to happen?

After downloading the item be able to update it from the sdk

Steps to reproduce

  1. Create json document secret using the json below. I tried with different combinations all jsons break it for us.
  2. run this code (silly code, update without any modification)
    client_from_token: Client = new_client()
    item_title = "your_item"
    vault_title = "your_vault"
    item = client_from_token.get_item(item_title, vault_title)
    client_from_token.update_item(item.id, item.vault.id, item)

Notes & Logs

had to zip, github does not allow json files uploads. bla.json.zip

HaddadJoe commented 1 year ago

Any feedback here @jpcoenen ? I'm happy to open PRs if you approve the issue

jpcoenen commented 1 year ago

Hi @HaddadJoe,

The error returned here is not super helpful, but it comes from the fact that Connect does not yet support modifying or uploading files stored on 1Password (we want to add support for that in the future). That makes it difficult to update a Document item because that item basically represents a file. Nevertheless, the error returned is unhelpful; I will file an internal issue for that.

Could you share what it is that you want to change for the item: the file that is stored in the document or any of the other fields? I think the latter is easier to fix and might already be possible without a change to Connect.

HaddadJoe commented 1 year ago

I'm just trying to change some of the other fields like tags and notes. I understand that files might be harder to handle but let's just return an exception when files are changed or just ignore that change(even though that might be tricky as the user would expect a change). The rest of the fields should be able to be editable.

volodymyrZotov commented 1 year ago

Hi @HaddadJoe. I was trying to reproduce the issue with the steps you provided, but could not.

Used: SDK Version: 1.3.0 Connect Server Version: 1.5.7 Python Version: 3.7.5

I was able successfully to update an item with the file. Here is the code example

# example with updating title
item = client.get_item("Login1", "dev")
item.title = 'Login'
client.update_item(item.id, item.vault.id, item)

# example without modifying item
item = client.get_item("Login", "dev")
client.update_item(item.id, item.vault.id, item)

# verify there is an item with file
file = client.get_files(item.id, item.vault.id)
print(file)

# it prints
[{'content_path': '/v1/vaults/<vauldId>/items/<itemId>/files/<fileId>/content',
 'id': 'fileId',
 'name': 'bla.json',
 'size': 32}]

Maybe there is something specific in your item, that prevents it from updating?

volodymyrZotov commented 1 year ago

Hi @HaddadJoe. Is it still the case for you?

HaddadJoe commented 1 year ago

We upgraded our op-connect server to the latest 1password/connect-sync:1.5.7 and ran the same code above, still getting the error on the update

from legalist.legalist.credentials.one_pass import client

item_title = "bla"
vault_title = "Engineering-Shared"
item = client.get_item(item_title, vault_title)
print(item.id)
client.update_item(item.id, item.vault.id, item)

here's the error, there was no update to the python sdk since then. I see a lot of commits but the team didn't cut a release yet

Traceback (most recent call last):
  File "/Users/joehaddad/Library/Caches/pypoetry/virtualenvs/legalist-4Blx9NQP-py3.11/lib/python3.11/site-packages/onepasswordconnectsdk/client.py", line 286, in update_item
    response.raise_for_status()
  File "/Users/joehaddad/Library/Caches/pypoetry/virtualenvs/legalist-4Blx9NQP-py3.11/lib/python3.11/site-packages/requests/models.py", line 1021, in raise_for_status
    raise HTTPError(http_error_msg, response=self)
requests.exceptions.HTTPError: 400 Client Error: Bad Request for url: https://staging.1pass.legalist.com/v1/vaults/5owhekkdsih65suiisvfd37niq/items/4zajgalyrblts4ljdx72t7nxkm

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/Users/joehaddad/Desktop/legalist/legalist/local_testing.py", line 7, in <module>
    client.update_item(item.id, item.vault.id, item)
  File "/Users/joehaddad/Library/Caches/pypoetry/virtualenvs/legalist-4Blx9NQP-py3.11/lib/python3.11/site-packages/onepasswordconnectsdk/client.py", line 288, in update_item
    raise FailedToRetrieveItemException(
onepasswordconnectsdk.client.FailedToRetrieveItemException: Unable to post item. Received 400                    for /v1/vaults/5owhekkdsih65suiisvfd37niq/items/4zajgalyrblts4ljdx72t7nxkm with message: Validation: (validateVaultItem failed to Validate), Couldn't validate the item: "[ItemValidator] has found 1 errors, 0 warnings: \nErrors:{1. Field of unexpected type \"\" at \"item.details.documentAttributes\". Requires one of [\"object\"]}"

Process finished with exit code 1

Did you manage to replicate that @volodymyrZotov ?

HaddadJoe commented 1 year ago

@volodymyrZotov any updates here?

volodymyrZotov commented 1 year ago

Hi @HaddadJoe. Unfortunately, I didn't have a chance to try to reproduce it once again. But this issue is on the our team's radar and we'll back on that soon.

volodymyrZotov commented 1 year ago

✅ Fixed in Connect v.1.7.2