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

Onepassword lookup add service accounts #6660

Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
9070a5a
add service account token and bypass required fields when service acc…
Domi-cc Jun 8, 2023
eb6352e
add token to base class
Domi-cc Jun 8, 2023
07ba8cf
add Info
jansagurna Jun 8, 2023
e831a25
add service_account_token
jansagurna Jun 8, 2023
095562a
add service_account_token
jansagurna Jun 8, 2023
f7cb8ed
add documentation
Domi-cc Jun 8, 2023
f007397
add service_account_token
Domi-cc Jun 9, 2023
375d6d5
fix E111: indentation is not a multiple of 4
Domi-cc Jun 9, 2023
90abfb7
fix lint problems
Domi-cc Jun 9, 2023
ae94bbf
Update plugins/lookup/onepassword_raw.py
jansagurna Jun 12, 2023
4ab33a6
Update plugins/modules/onepassword_info.py
jansagurna Jun 12, 2023
d12b22a
Update plugins/lookup/onepassword.py
jansagurna Jun 12, 2023
9c15c70
add changelog fragment
jansagurna Jun 12, 2023
a67bfea
change type service_account_token to align to domain option
jansagurna Jun 12, 2023
7163369
add fragment value
jansagurna Jun 12, 2023
a793b59
Update changelogs/fragments/6660-onepassword-lookup-service-account.yaml
jansagurna Jun 13, 2023
90461c7
Update plugins/lookup/onepassword.py
jansagurna Jun 13, 2023
9904b2d
remove service_account_token from onepassword_info.py
jansagurna Jun 13, 2023
ccaf67d
adjust V1 to raise error if service_account_token is set
jansagurna Jun 13, 2023
1434ecd
adjust V1 to raise error if service_account_token is set
jansagurna Jun 13, 2023
f61fbe1
adjust V1 to raise error if service_account_token is set
jansagurna Jun 13, 2023
9090651
adjust if assert_logged_in
jansagurna Jun 13, 2023
d34a9c7
Update plugins/lookup/onepassword.py
jansagurna Jun 13, 2023
fc0d07d
Update plugins/lookup/onepassword.py
jansagurna Jun 13, 2023
f98620f
remove double return
jansagurna Jun 13, 2023
7b4b61c
remove new line
jansagurna Jun 13, 2023
e53fb70
remove new line
jansagurna Jun 13, 2023
a2870a8
remove new line
jansagurna Jun 13, 2023
c233708
remove spaces
jansagurna Jun 13, 2023
b6290c5
remove new line
jansagurna Jun 13, 2023
5e7f8f7
remove spaces
jansagurna Jun 13, 2023
a0bfb65
Update plugins/lookup/onepassword_raw.py
jansagurna Jun 13, 2023
ce5c38f
add _check_required_params
jansagurna Jun 13, 2023
990328f
Update plugins/lookup/onepassword.py
jansagurna Jun 14, 2023
7cc6c5f
Update plugins/lookup/onepassword.py
jansagurna Jun 14, 2023
df8a5a9
remove _check_required_params
jansagurna Jun 14, 2023
b1fae64
remove spaces
jansagurna Jun 14, 2023
48b36b8
Update plugins/lookup/onepassword.py
jansagurna Jun 14, 2023
4346b29
remove code
jansagurna Jun 15, 2023
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
121 changes: 74 additions & 47 deletions plugins/lookup/onepassword.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@
description: The username used to sign in.
secret_key:
description: The secret key used when performing an initial sign in.
service_account_token:
description: The access key for a service account.
jansagurna marked this conversation as resolved.
Show resolved Hide resolved
jansagurna marked this conversation as resolved.
Show resolved Hide resolved
vault:
description: Vault containing the item to retrieve (case-insensitive). If absent will search all vaults.
notes:
Expand Down Expand Up @@ -113,28 +115,30 @@
class OnePassCLIBase(with_metaclass(abc.ABCMeta, object)):
bin = "op"

def __init__(self, subdomain=None, domain="1password.com", username=None, secret_key=None, master_password=None):
def __init__(self, subdomain=None, domain="1password.com", username=None, secret_key=None, master_password=None, service_account_token=None):
self.subdomain = subdomain
self.domain = domain
self.username = username
self.master_password = master_password
self.secret_key = secret_key
self.service_account_token = service_account_token

self._path = None
self._version = None

def _check_required_params(self, required_params):
non_empty_attrs = dict((param, getattr(self, param, None)) for param in required_params if getattr(self, param, None))
missing = set(required_params).difference(non_empty_attrs)
if missing:
prefix = "Unable to sign in to 1Password. Missing required parameter"
plural = ""
suffix = ": {params}.".format(params=", ".join(missing))
if len(missing) > 1:
plural = "s"

msg = "{prefix}{plural}{suffix}".format(prefix=prefix, plural=plural, suffix=suffix)
raise AnsibleLookupError(msg)
if not self.service_account_token:
Copy link
Contributor

Choose a reason for hiding this comment

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

This will skip parameter validation for OnePassCLIv1. Even if this won't be called by methods in a OnePassCLIv1 object in the current plugin code, this should do the right thing when called in isolation.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this method should change. Instead, have the caller decide whether calling this method is even necessary. That should be two places is the OnePassCLIv2 class.

non_empty_attrs = dict((param, getattr(self, param, None)) for param in required_params if getattr(self, param, None))
missing = set(required_params).difference(non_empty_attrs)
if missing:
prefix = "Unable to sign in to 1Password. Missing required parameter"
plural = ""
suffix = ": {params}. Or use a service_account_token.".format(params=", ".join(missing))
if len(missing) > 1:
plural = "s"

msg = "{prefix}{plural}{suffix}".format(prefix=prefix, plural=plural, suffix=suffix)
raise AnsibleLookupError(msg)

@abc.abstractmethod
def _parse_field(self, data_json, field_name, section_title):
Expand Down Expand Up @@ -472,55 +476,77 @@ def _parse_field(self, data_json, field_name, section_title=None):
return ""

def assert_logged_in(self):
args = ["account", "list"]
if self.subdomain:
account = "{subdomain}.{domain}".format(subdomain=self.subdomain, domain=self.domain)
args.extend(["--account", account])
if self.service_account_token:
# maybe not necessary
args = ["account", "get"]
environment_update = {"OP_SERVICE_ACCOUNT_TOKEN": self.service_account_token}
rc, out, err = self._run(args, ignore_errors=False, environment_update=environment_update)

rc, out, err = self._run(args)
return not bool(rc)

if out:
# Running 'op account get' if there are no accounts configured on the system drops into
# an interactive prompt. Only run 'op account get' after first listing accounts to see
# if there are any previously configured accounts.
args = ["account", "get"]
else:

Copy link
Contributor

Choose a reason for hiding this comment

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

This is still probably necessary as it checks that the token is valid.

The else can be removed.

Copy link
Contributor

@samdoran samdoran Jun 13, 2023

Choose a reason for hiding this comment

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

Should this command me op user get --me as the docs suggest? op whoami is also sufficient.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this should ignore errors. If the token is invalid, this should raise an AnsibleLookupError.

args = ["account", "list"]
if self.subdomain:
account = "{subdomain}.{domain}".format(subdomain=self.subdomain, domain=self.domain)
args.extend(["--account", account])

rc, out, err = self._run(args, ignore_errors=True)
rc, out, err = self._run(args)

return not bool(rc)
if out:
# Running 'op account get' if there are no accounts configured on the system drops into
# an interactive prompt. Only run 'op account get' after first listing accounts to see
# if there are any previously configured accounts.
args = ["account", "get"]
if self.subdomain:
account = "{subdomain}.{domain}".format(subdomain=self.subdomain, domain=self.domain)
args.extend(["--account", account])

rc, out, err = self._run(args, ignore_errors=True)

return False
return not bool(rc)

return False

def full_signin(self):
required_params = [
"subdomain",
"username",
"secret_key",
"master_password",
]
self._check_required_params(required_params)
if self.service_account_token:
environment_update = {"OP_SERVICE_ACCOUNT_TOKEN": self.service_account_token}
args = [
"whoami",
]

args = [
"account", "add", "--raw",
"--address", "{0}.{1}".format(self.subdomain, self.domain),
"--email", to_bytes(self.username),
"--signin",
]
return self._run(args, environment_update=environment_update)
else:
Copy link
Contributor

Choose a reason for hiding this comment

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

These changes aren't needed since full_signin will be skipped if there is service token. If there is a problem with the service token, the lookup will exit before getting here.

Copy link
Contributor

Choose a reason for hiding this comment

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

Removed these changes.

required_params = [
"subdomain",
"username",
"secret_key",
"master_password",
]
self._check_required_params(required_params)

environment_update = {"OP_SECRET_KEY": self.secret_key}
return self._run(args, command_input=to_bytes(self.master_password), environment_update=environment_update)
args = [
"account", "add", "--raw",
"--address", "{0}.{1}".format(self.subdomain, self.domain),
"--email", to_bytes(self.username),
"--signin",
]

environment_update = {"OP_SECRET_KEY": self.secret_key}

return self._run(args, command_input=to_bytes(self.master_password), environment_update=environment_update)

def get_raw(self, item_id, vault=None, token=None):
args = ["item", "get", item_id, "--format", "json"]
if vault is not None:
args += ["--vault={0}".format(vault)]
if token is not None:
if not self.service_account_token and token is not None:
args += [to_bytes("--session=") + token]

return self._run(args)
if self.service_account_token:
environment_update = {"OP_SERVICE_ACCOUNT_TOKEN": self.service_account_token}
return self._run(args, environment_update=environment_update)
else:
jansagurna marked this conversation as resolved.
Show resolved Hide resolved
return self._run(args)
Domi-cc marked this conversation as resolved.
Show resolved Hide resolved

def signin(self):
self._check_required_params(['master_password'])
Expand All @@ -533,12 +559,13 @@ def signin(self):


class OnePass(object):
def __init__(self, subdomain=None, domain="1password.com", username=None, secret_key=None, master_password=None):
def __init__(self, subdomain=None, domain="1password.com", username=None, secret_key=None, master_password=None, service_account_token=None):
self.subdomain = subdomain
self.domain = domain
self.username = username
self.secret_key = secret_key
self.master_password = master_password
self.service_account_token = service_account_token

self.logged_in = False
self.token = None
Expand All @@ -551,7 +578,7 @@ def _get_cli_class(self):
for cls in OnePassCLIBase.__subclasses__():
if cls.supports_version == version.split(".")[0]:
try:
return cls(self.subdomain, self.domain, self.username, self.secret_key, self.master_password)
return cls(self.subdomain, self.domain, self.username, self.secret_key, self.master_password, self.service_account_token)
felixfontein marked this conversation as resolved.
Show resolved Hide resolved
except TypeError as e:
raise AnsibleLookupError(e)

Expand Down Expand Up @@ -614,12 +641,12 @@ def run(self, terms, variables=None, **kwargs):
username = self.get_option("username")
secret_key = self.get_option("secret_key")
master_password = self.get_option("master_password")
service_account_token = self.get_option("service_account_token")

op = OnePass(subdomain, domain, username, secret_key, master_password)
op = OnePass(subdomain, domain, username, secret_key, master_password, service_account_token)
op.assert_logged_in()

values = []
for term in terms:
values.append(op.get_field(term, field, section, vault))

Copy link
Contributor

Choose a reason for hiding this comment

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

This space can stay.

return values
5 changes: 4 additions & 1 deletion plugins/lookup/onepassword_raw.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@
description: The username used to sign in.
secret_key:
description: The secret key used when performing an initial sign in.
service_account_token:
description: The access key for a service account.
jansagurna marked this conversation as resolved.
Show resolved Hide resolved
jansagurna marked this conversation as resolved.
Show resolved Hide resolved
vault:
description: Vault containing the item to retrieve (case-insensitive). If absent will search all vaults.
notes:
Expand Down Expand Up @@ -89,8 +91,9 @@ def run(self, terms, variables=None, **kwargs):
username = self.get_option("username")
secret_key = self.get_option("secret_key")
master_password = self.get_option("master_password")
service_account_token = self.get_option("service_account_token")

op = OnePass(subdomain, domain, username, secret_key, master_password)
op = OnePass(subdomain, domain, username, secret_key, master_password, service_account_token)
op.assert_logged_in()

values = []
Expand Down
6 changes: 6 additions & 0 deletions plugins/modules/onepassword_info.py
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,11 @@
description:
- 1Password username.
- Only required for initial sign in.
service_account_token:
type: str
description:
- 1Password service account token.
- Only required for initial sign in.
jansagurna marked this conversation as resolved.
Show resolved Hide resolved
master_password:
type: str
description:
Expand Down Expand Up @@ -375,6 +380,7 @@ def main():
username=dict(type='str'),
master_password=dict(required=True, type='str', no_log=True),
secret_key=dict(type='str', no_log=True),
service_account_token=dict(type='str', no_log=True),
felixfontein marked this conversation as resolved.
Show resolved Hide resolved
), default=None),
search_terms=dict(required=True, type='list', elements='dict'),
),
Expand Down