phabalicious icon indicating copy to clipboard operation
phabalicious copied to clipboard

feat: Add support for consuming secrets from 1Password reference links

Open d34dman opened this issue 1 year ago • 4 comments
trafficstars

@stmh am toying around the idea of using 1Password reference for populating secrets when 1password cli is available in the environment.

d34dman avatar Jul 25 '24 12:07 d34dman

hi @d34dman thanks for the MR.

Instead of passing the 1p reference I'd try to extract the vault id and item id from the url and use the already existing ways to get the secret. This will have the benefit that it will also work with 1p connect. So onePasswordReference is an alternative to onePasswordId and onePasswordVaultId

stmh avatar Jul 25 '24 12:07 stmh

@stmh , Thanks for the review :)

Instead of passing the 1p reference I'd try to extract the vault id and item id from the url and use the already existing ways to get the secret. This will have the benefit that it will also work with 1p connect. So onePasswordReference is an alternative to onePasswordId and onePasswordVaultId

I tried to re-use as much as possible. But I must confess I don't have a good understanding of the underlying foundation. The reasons I didn't use the existing method is because I ran into the same limitation as the existing method. The limitation being, references allows you to target a specific field inside an item.

Example, op://Developer/PHAB.test_reference/text targets a "text" field; inside vault - Developer; item PHAB.test_reference.

Seems like the limitation is not coming from 1P cli, but from phabalicious. Which extracts a specific field from json and uses that as secret value. So this would either create a breaking change or force us to use references with all the limitation that current method brings and causes a lot of confusion for end users.

Reference: https://github.com/factorial-io/phabalicious/blob/main/src/Utilities/PasswordManager.php#L393-L444

Should i still proceed to refactor to use extractSecretFrom1PasswordPayload method, or did you had something else in mind?

d34dman avatar Jul 31 '24 08:07 d34dman

@stmh maybe i can create another method to get item from vault using onePasswordId and onePasswordVaultId and then extract text and pass it back. This way, it doesn't really use op read.

This make me think, we should actually be adding another key like "onePasswordSubKey" to get what we want. If the subKey is not present, then it would as it does now.

d34dman avatar Aug 01 '24 07:08 d34dman

I missed the fact that the link was targeting a different field. So what is missing is a way to target a specific field in an optional section. We can add them as properties to the secret data and if they are set change the current implementation that iterates over all sections and return items with purpose password or name password.

then we might also introduce the url notation for getting vault, item, name and section.

In the end, it needs to be compatible with 1password connect, as this is used in the ci.

stmh avatar Aug 02 '24 13:08 stmh

@d34dman there is now a similar way part of phab to consume sth different than a password. See https://github.com/factorial-io/phabalicious/pull/338, closing this

stmh avatar Nov 05 '24 14:11 stmh