ansible-onepasswordconnect-collection icon indicating copy to clipboard operation
ansible-onepasswordconnect-collection copied to clipboard

Allow usage with write-only tokens

Open mjbnz opened this issue 3 years ago • 3 comments

Summary

Using the collection to store generated passwords for system root accounts, a read access token is required.

Use cases

When an automation tool is storing passwords for generated resources for administrators to use in the case of system failure (root passwords for bare metal systems for example), it would be nice to have a write only token so that those items are not at risk of compromise if the automation host is compromised.

Proposed solution

Perhaps just adding a module parameter that tells generic_item to not try to read, just blindly create.

Another potential solution is to create an added permission level that allows the searching of items ids, but not reading fields. That's a larger task for the 1Password team.

edit: and another is allowing the passing of an item_id saved from a previous creation as a parameter, and have the module use that to do a blind update.

Is there a workaround to accomplish this today?

I'm changing line 329 of generic_item.py to:

        item = None

which works fine, but does have the side effect of creating new items, not changing existing ones.

mjbnz avatar Sep 24 '21 04:09 mjbnz

I agree this would be useful! Unfortunately the Ansible module needs information about the item before determining what to do next. For example, if you didn't provide an item_id and set state: present, Ansible searches the vault -- which requires read permissions -- to find an item with a matching name before it can know if a create, update, or no-op action is needed.

That said, I think checking for whether item_id is set would be the best way forward.

If we change the behavior to do a blind update when you provide an item_id, would it make sense to perform a blind update if state: present or would you expect a different state value in that case?

verkaufer avatar Sep 27 '21 18:09 verkaufer

I agree this would be useful! Unfortunately the Ansible module needs information about the item before determining what to do next. For example, if you didn't provide an item_id and set state: present, Ansible searches the vault -- which requires read permissions -- to find an item with a matching name before it can know if a create, update, or no-op action is needed.

I think retaining existing behaviour when not providing item_id makes sense - the specific use case where you can't read from the vault means that you'd need to help out the module where possible, such as providing the item id.

That said, I think checking for whether item_id is set would be the best way forward.

If we change the behavior to do a blind update when you provide an item_id, would it make sense to perform a blind update if state: present or would you expect a different state value in that case?

I think in the case where item_id is provided, it would be safe to just interpret state: present as requesting a blind update, state: absent would be requesting a blind delete. For creation, where no item_id is known, the lack of item_id specified would cause the module to search first - perhaps a "special" item_id of 0 could indicate a blind create rather than a blind update?

That means that you could use the presence of an item_id parameter to indicate a write-only token, and blind operations should be carried out.

The only other criteria to check in this instance is state: absent with item_id: 0, in which case you could just skip execution.

Error handling when an update or deletion of an item_id that does not exist might need thought. probably returning as no change? throwing an error might be ok too, as the user could utilise ignore_errors if required. I am unsure on the best choice here.

mjbnz avatar Sep 27 '21 22:09 mjbnz

in the case where item_id is provided, it would be safe to just interpret state: present as requesting a blind update, state: absent would be requesting a blind delete.

Makes sense. 😄 Fortunately Connect supports PUT semantics, so we wouldn't need to have a "special" item ID (we let clients generate client IDs, but there are validation rules for those IDs).

We can ignore errors when state:absent returns a 404 because the system still accomplished the user's intent: to delete the item.

Error handling when an update or deletion of an item_id that does not exist might need thought As I mentioned above, we can use upsert semantics for update + item_id.

I'll work on a PR to address these ideas. I can't give you a date for when it will be released, but you'll be notified when it's ready :)

verkaufer avatar Sep 29 '21 16:09 verkaufer