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

Allow usage with write-only tokens #37

Open
mjbnz opened this issue Sep 24, 2021 · 3 comments
Open

Allow usage with write-only tokens #37

mjbnz opened this issue Sep 24, 2021 · 3 comments
Labels
tracked Issue is tracked in 1Password's internal ticketing system as well.
Milestone

Comments

@mjbnz
Copy link

mjbnz commented Sep 24, 2021

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.

@verkaufer
Copy link
Member

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?

@mjbnz
Copy link
Author

mjbnz commented Sep 27, 2021

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.

@verkaufer
Copy link
Member

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 verkaufer added this to the v2.2 milestone Sep 30, 2021
@verkaufer verkaufer modified the milestones: v2.2, 2.3 Dec 17, 2021
@edif2008 edif2008 changed the title Allow usage with write-only tokens [12] Allow usage with write-only tokens Feb 8, 2022
@edif2008 edif2008 added the tracked Issue is tracked in 1Password's internal ticketing system as well. label May 30, 2022
@edif2008 edif2008 changed the title [12] Allow usage with write-only tokens Allow usage with write-only tokens May 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tracked Issue is tracked in 1Password's internal ticketing system as well.
Projects
None yet
Development

No branches or pull requests

3 participants