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

replace lastpass cli with 1password cli #39

Merged
merged 4 commits into from
Nov 20, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
37 changes: 23 additions & 14 deletions db_facts/lpass.py
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -19,20 +26,22 @@ 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])
# *** 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.

Comment on lines +31 to +34

Choose a reason for hiding this comment

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

Can you make this comment even more explicit? I'd like it to make clear that ALL functions referencing LastPass/lpass are being redirected to pull data from 1Password instead. The names have not been changed in the interest of time. This needs to be an unmissable comment for anyone working on this in the future, so you can go wild with all caps.

# 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}'])

return raw_output.decode('utf-8').rstrip('\n')


Expand Down
61 changes: 31 additions & 30 deletions tests/test_lpass.py
Original file line number Diff line number Diff line change
@@ -1,64 +1,65 @@
# ************************************************
# *** 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


Choose a reason for hiding this comment

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

Please add an impossible-to-miss comment here with the same disclaimer from lpass.py

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"
monikered marked this conversation as resolved.
Show resolved Hide resolved
def test_lpass_field_url_raises(self):
with self.assertRaises(NotImplementedError):
out = lpass.lpass_field('my_name', 'url')
Brunope marked this conversation as resolved.
Show resolved Hide resolved

def test_lpass_field_notes_raises(self):
with self.assertRaises(NotImplementedError):
out = lpass.lpass_field('my_name', 'notes')
Brunope marked this conversation as resolved.
Show resolved Hide resolved

@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"
Comment on lines -31 to -37

Choose a reason for hiding this comment

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

same deal as with the Notes field test function -- I'd prefer for this to throw an error rather than be deleted entirely


@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 = {
Expand Down