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.

Sign up for GitHub

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

add --reveal to cli for v2.30.0 and later #65

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

add --reveal to cli for v2.30.0 and later #65

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 8 additions & 5 deletions onepassword/client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,8 @@ class OnePassword:
:param account: 1Password account name (Optional, default=None)
:param password: password of 1Password account (Optional, default=None)
"""
cli_version = tuple(map(int, read_bash_return("op --version").split(".")))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We actually try not to do this and just fix the version in the install.py it gave us a lot more headaches trying to be fully backwards compatible and so we try to maintain and bump when we can. I know we suggest you can have any version over 2.0.0 in the README but if this new flag is a break change, let's update that suggestion :D


def __init__(self, signin_method: str = "app", account: str | None = None, password: str | None = None) -> None:
# pragma: no cover
if SERVICE_ACCOUNT_TOKEN in os.environ.keys():
Expand Down Expand Up @@ -421,8 +423,8 @@ def list_items(vault: str = "Private") -> dict:
items = json.loads(read_bash_return("op items list --vault='{}' --format=json".format(vault), single=False))
return items

@staticmethod
def get_item(uuid: str | bytes, fields: str | bytes | list | None = None):
@classmethod
def get_item(cls, uuid: str | bytes, fields: str | bytes | list | None = None):
"""
Helper function to get a certain field, you can find the UUID you need using list_items

Expand All @@ -431,9 +433,10 @@ def get_item(uuid: str | bytes, fields: str | bytes | list | None = None):
(Optional, default=None which means all fields returned)
:return: Dictionary of the item with requested fields
"""
reveal = "--reveal" if cls.cli_version >= (2, 30, 0) else ""
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we go with my suggestions you won't need this and can just add --reveal to every op call that requires it

if isinstance(fields, list):
item_list = json.loads(read_bash_return(
"op item get {} --format=json --fields label={}".format(uuid, ",label=".join(fields)),
"op item get {} {} --format=json --fields label={}".format(uuid, reveal, ",label=".join(fields)),
single=False))
item = {}
if isinstance(item_list, dict):
Expand All @@ -444,10 +447,10 @@ def get_item(uuid: str | bytes, fields: str | bytes | list | None = None):
elif isinstance(fields, str):
item = {
fields: read_bash_return(
"op item get {} --fields label={}".format(uuid, fields), single=False).rstrip('\n')
"op item get {} {} --fields label={}".format(uuid, reveal, fields), single=False).rstrip('\n')
}
else:
item = json.loads(read_bash_return("op item get {} --format=json".format(uuid), single=False))
item = json.loads(read_bash_return("op item get {} {} --format=json".format(uuid, reveal), single=False))
return item

@staticmethod
Expand Down
77 changes: 77 additions & 0 deletions test/test_cli_version.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
import json
from unittest import TestCase
from unittest.mock import patch

from onepassword import client


class TestCliVersion(TestCase):
@patch.object(client, "read_bash_return")
@patch.object(client.OnePassword, "cli_version", (2, 29, 0))
def test_get_item_no_reveal(self, mock_read_bash_return):
op = client.OnePassword()

mock_read_bash_return.return_value = json.dumps([])
op.get_item("uuid")
mock_read_bash_return.assert_called_once_with(
"op item get uuid --format=json",
single=False,
)
mock_read_bash_return.reset_mock()

mock_read_bash_return.return_value = json.dumps(
[{"id": "field", "value": "value"}]
)
op.get_item("uuid", "field")
mock_read_bash_return.assert_called_once_with(
"op item get uuid --fields label=field", single=False
)
mock_read_bash_return.reset_mock()

mock_read_bash_return.return_value = json.dumps(
[
{"id": "list", "value": "value"},
{"id": "of", "value": "value"},
{"id": "fields", "value": "value"},
]
)
op.get_item("uuid", ["list", "of", "fields"])
mock_read_bash_return.assert_called_once_with(
"op item get uuid --format=json --fields label=list,label=of,label=fields",
single=False,
)

@patch.object(client, "read_bash_return")
@patch.object(client.OnePassword, "cli_version", (2, 30, 0))
def test_get_item_reveal(self, mock_read_bash_return):
op = client.OnePassword()

mock_read_bash_return.return_value = json.dumps([])
op.get_item("uuid")
mock_read_bash_return.assert_called_once_with(
"op item get uuid --reveal --format=json",
single=False,
)
mock_read_bash_return.reset_mock()

mock_read_bash_return.return_value = json.dumps(
[{"id": "field", "value": "value"}]
)
op.get_item("uuid", "field")
mock_read_bash_return.assert_called_once_with(
"op item get uuid --reveal --fields label=field", single=False
)
mock_read_bash_return.reset_mock()

mock_read_bash_return.return_value = json.dumps(
[
{"id": "list", "value": "value"},
{"id": "of", "value": "value"},
{"id": "fields", "value": "value"},
]
)
op.get_item("uuid", ["list", "of", "fields"])
mock_read_bash_return.assert_called_once_with(
"op item get uuid --reveal --format=json --fields label=list,label=of,label=fields",
single=False,
)