From ce3d1fa849a7b4cab1e35649faf7107164ca9952 Mon Sep 17 00:00:00 2001 From: Bruno Castro-Karney Date: Wed, 15 Nov 2023 16:37:13 -0800 Subject: [PATCH 1/4] replace lastpass cli with 1password cli --- .python-version | 2 +- db_facts/lpass.py | 20 ++++++-------------- 2 files changed, 7 insertions(+), 15 deletions(-) diff --git a/.python-version b/.python-version index ccb3fd7..a804d27 100644 --- a/.python-version +++ b/.python-version @@ -1 +1 @@ -db_facts-3.8.4 +db_facts-5.0.0 diff --git a/db_facts/lpass.py b/db_facts/lpass.py index 00a11f9..8286561 100644 --- a/db_facts/lpass.py +++ b/db_facts/lpass.py @@ -19,20 +19,12 @@ def pull_lastpass_aws_iam(lastpass_entry_name: str) -> LastPassAWSIAM: def lpass_field(name: str, field: str) -> str: - if field == 'notes': - field_arg = '--notes' - elif field == 'username': - field_arg = '--username' - elif field == 'password': - field_arg = '--password' - elif field == 'url': - field_arg = '--url' - else: - field_arg = '--field=' + field - raw_output = check_output(['lpass', - 'show', - field_arg, - name]) + # This used to use the lastpass-cli to pull credentials. But we've moved + # from lastpass to 1password. This command retrieves the fields in the + # same format from 1password instead. + raw_output = check_output( + ['op', 'item', 'get', name, '--field', f'label={field}']) + return raw_output.decode('utf-8').rstrip('\n') From 0eaae5c33d172f3ac8123f20c00104252df61304 Mon Sep 17 00:00:00 2001 From: Bruno Castro-Karney Date: Wed, 15 Nov 2023 16:49:20 -0800 Subject: [PATCH 2/4] unit tests expect op cli call --- db_facts/lpass.py | 4 ++++ tests/test_lpass.py | 48 ++++++++++++++++----------------------------- 2 files changed, 21 insertions(+), 31 deletions(-) diff --git a/db_facts/lpass.py b/db_facts/lpass.py index 8286561..baf7834 100644 --- a/db_facts/lpass.py +++ b/db_facts/lpass.py @@ -22,6 +22,10 @@ def lpass_field(name: str, field: str) -> str: # This used to use the lastpass-cli to pull credentials. But we've moved # from lastpass to 1password. This command retrieves the fields in the # same format from 1password instead. + + # Note this won't work for the way 1password stores notes and URLs, which + # is different from lpass. But as of now db-facts doesn't ever rely on + # these fields. raw_output = check_output( ['op', 'item', 'get', name, '--field', f'label={field}']) diff --git a/tests/test_lpass.py b/tests/test_lpass.py index 5dd3a45..89334e8 100644 --- a/tests/test_lpass.py +++ b/tests/test_lpass.py @@ -4,61 +4,47 @@ class TestLPass(unittest.TestCase): - @patch('db_facts.lpass.check_output') - def test_lpass_field_notes(self, mock_check_output): - mock_check_output.return_value = "fakenotes\n".encode("utf-8") - out = lpass.lpass_field('my_name', 'notes') - mock_check_output.\ - assert_called_with(['lpass', 'show', '--notes', 'my_name']) - assert out == "fakenotes" - @patch('db_facts.lpass.check_output') def test_lpass_field_username(self, mock_check_output): mock_check_output.return_value = "fakeuser\n".encode("utf-8") out = lpass.lpass_field('my_name', 'username') - mock_check_output.\ - assert_called_with(['lpass', 'show', '--username', 'my_name']) + mock_check_output.assert_called_with( + ['op', 'item', 'get', 'my_name', '--field', 'label=username']) assert out == "fakeuser" @patch('db_facts.lpass.check_output') def test_lpass_field_password(self, mock_check_output): mock_check_output.return_value = "fakepassword\n".encode("utf-8") out = lpass.lpass_field('my_name', 'password') - mock_check_output.\ - assert_called_with(['lpass', 'show', '--password', 'my_name']) + mock_check_output.assert_called_with( + ['op', 'item', 'get', 'my_name', '--field', 'label=password']) assert out == "fakepassword" - @patch('db_facts.lpass.check_output') - def test_lpass_field_url(self, mock_check_output): - mock_check_output.return_value = "fakeurl\n".encode("utf-8") - out = lpass.lpass_field('my_name', 'url') - mock_check_output.\ - assert_called_with(['lpass', 'show', '--url', 'my_name']) - assert out == "fakeurl" - @patch('db_facts.lpass.check_output') def test_lpass_field_field1(self, mock_check_output): mock_check_output.return_value = "fakefield1\n".encode("utf-8") out = lpass.lpass_field('my_name', 'field1') - mock_check_output.\ - assert_called_with(['lpass', 'show', '--field=field1', 'my_name']) + mock_check_output.assert_called_with( + ['op', 'item', 'get', 'my_name', '--field', 'label=field1']) assert out == "fakefield1" @patch('db_facts.lpass.check_output') def test_db_info_from_lpass(self, mock_check_output): def fake_check_output(args): - assert args[0] == 'lpass' - assert args[1] == 'show' + assert args[0] == 'op' + assert args[1] == 'item' + assert args[2] == 'get' assert args[3] == 'my_lpass_name' + assert args[4] == '--field' ret = { - "--username": 'fakeuser', - "--password": 'fakepassword', - "--field=Hostname": 'fakehost', - "--field=Port": '123', - "--field=Type": 'faketype', - "--field=Database": 'fakedatabase', + "label=username": 'fakeuser', + "label=password": 'fakepassword', + "label=Hostname": 'fakehost', + "label=Port": '123', + "label=Type": 'faketype', + "label=Database": 'fakedatabase', } - return (ret[args[2]] + "\n").encode('utf-8') + return (ret[args[5]] + "\n").encode('utf-8') mock_check_output.side_effect = fake_check_output db_info = lpass.db_info_from_lpass('my_lpass_name') expected_db_info = { From 55e9d1db4a8e802c7333cdea541496e488b31fde Mon Sep 17 00:00:00 2001 From: Bruno Castro-Karney Date: Fri, 17 Nov 2023 11:38:08 -0800 Subject: [PATCH 3/4] more visible comments, raise error on lpass_field notes|url --- .python-version | 2 +- db_facts/lpass.py | 13 +++++++++++++ tests/test_lpass.py | 15 +++++++++++++++ 3 files changed, 29 insertions(+), 1 deletion(-) diff --git a/.python-version b/.python-version index a804d27..ccb3fd7 100644 --- a/.python-version +++ b/.python-version @@ -1 +1 @@ -db_facts-5.0.0 +db_facts-3.8.4 diff --git a/db_facts/lpass.py b/db_facts/lpass.py index baf7834..a74f237 100644 --- a/db_facts/lpass.py +++ b/db_facts/lpass.py @@ -1,3 +1,10 @@ +# ************************************************ +# *** ATTENTION *** THIS DOES NOT USE LASTPASS *** +# ************************************************ +# BlueLabs is currently transitioning off lastpass and onto 1password. +# Even though this file says "lpass", all underlying calls to the +# lpass CLI have been replaced with calls to the 1password CLI. + from subprocess import check_output from .db_facts_types import LastPassUsernamePassword, LastPassAWSIAM from .db_type import canonicalize_db_type, db_protocol @@ -19,6 +26,8 @@ def pull_lastpass_aws_iam(lastpass_entry_name: str) -> LastPassAWSIAM: def lpass_field(name: str, field: str) -> str: + # *** ATTENTION *** THIS DOES NOT USE LASTPASS *** + # This used to use the lastpass-cli to pull credentials. But we've moved # from lastpass to 1password. This command retrieves the fields in the # same format from 1password instead. @@ -26,6 +35,10 @@ def lpass_field(name: str, field: str) -> str: # Note this won't work for the way 1password stores notes and URLs, which # is different from lpass. But as of now db-facts doesn't ever rely on # these fields. + if field == 'notes' or field == 'url': + raise NotImplementedError( + 'Cannot retrieve notes or URL fields from 1password') + raw_output = check_output( ['op', 'item', 'get', name, '--field', f'label={field}']) diff --git a/tests/test_lpass.py b/tests/test_lpass.py index 89334e8..7ecb2f5 100644 --- a/tests/test_lpass.py +++ b/tests/test_lpass.py @@ -1,9 +1,24 @@ +# ************************************************ +# *** ATTENTION *** THIS DOES NOT USE LASTPASS *** +# ************************************************ +# BlueLabs is currently transitioning off lastpass and onto 1password. +# Even though this file says "lpass", all underlying calls to the +# lpass CLI have been replaced with calls to the 1password CLI. + import unittest from unittest.mock import patch from db_facts import lpass class TestLPass(unittest.TestCase): + def test_lpass_field_url_raises(self): + with self.assertRaises(NotImplementedError): + out = lpass.lpass_field('my_name', 'url') + + def test_lpass_field_notes_raises(self): + with self.assertRaises(NotImplementedError): + out = lpass.lpass_field('my_name', 'notes') + @patch('db_facts.lpass.check_output') def test_lpass_field_username(self, mock_check_output): mock_check_output.return_value = "fakeuser\n".encode("utf-8") From 1992ed527789b247191e7cb746e1df589ee512bf Mon Sep 17 00:00:00 2001 From: Bruno Castro-Karney Date: Fri, 17 Nov 2023 11:49:48 -0800 Subject: [PATCH 4/4] remove unused var --- tests/test_lpass.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_lpass.py b/tests/test_lpass.py index 7ecb2f5..564ba39 100644 --- a/tests/test_lpass.py +++ b/tests/test_lpass.py @@ -13,11 +13,11 @@ class TestLPass(unittest.TestCase): def test_lpass_field_url_raises(self): with self.assertRaises(NotImplementedError): - out = lpass.lpass_field('my_name', 'url') + lpass.lpass_field('my_name', 'url') def test_lpass_field_notes_raises(self): with self.assertRaises(NotImplementedError): - out = lpass.lpass_field('my_name', 'notes') + lpass.lpass_field('my_name', 'notes') @patch('db_facts.lpass.check_output') def test_lpass_field_username(self, mock_check_output):