From 40146b60a8e62e31d3ea67b40670d0f06a1ce987 Mon Sep 17 00:00:00 2001
From: Josh Burton <josh@arklabs.nz>
Date: Thu, 14 Dec 2023 17:00:03 +1300
Subject: [PATCH 1/2] fix: fix op cli always generating passwords

When invoking the op cli, if the onepassword item had no password recipe, generate-password would always be appended to the cli args and any password value would be overwritten by a generated password
---
 onepassword/cli/op.go         |  9 +++----
 onepassword/cli/utils.go      | 10 +++++++
 onepassword/cli/utils_test.go | 49 +++++++++++++++++++++++++++++++++++
 3 files changed, 62 insertions(+), 6 deletions(-)

diff --git a/onepassword/cli/op.go b/onepassword/cli/op.go
index 5983d67c..4ea50815 100644
--- a/onepassword/cli/op.go
+++ b/onepassword/cli/op.go
@@ -100,12 +100,9 @@ func (op *OP) create(ctx context.Context, item *onepassword.Item, vaultUuid stri
 	args := []opArg{p("item"), p("create"), p("-")}
 	// 'op item create' command doesn't support generating passwords when using templates
 	// therefore need to use --generate-password flag to set it
-	if pf := passwordField(item); pf != nil {
-		recipeStr := "letters,digits,32"
-		if pf.Recipe != nil {
-			recipeStr = passwordRecipeToString(pf.Recipe)
-		}
-		args = append(args, f("generate-password", recipeStr))
+	recipe := passwordRecipe(item)
+	if recipe != "" {
+		args = append(args, f("generate-password", recipe))
 	}
 
 	err = op.execJson(ctx, &res, payload, args...)
diff --git a/onepassword/cli/utils.go b/onepassword/cli/utils.go
index 46aee594..9bfd4079 100644
--- a/onepassword/cli/utils.go
+++ b/onepassword/cli/utils.go
@@ -60,6 +60,16 @@ func passwordField(item *onepassword.Item) *onepassword.ItemField {
 	return nil
 }
 
+func passwordRecipe(item *onepassword.Item) string {
+	str := ""
+	if pf := passwordField(item); pf != nil {
+		if pf.Recipe != nil {
+			return passwordRecipeToString(pf.Recipe)
+		}
+	}
+	return str
+}
+
 func passwordRecipeToString(recipe *onepassword.GeneratorRecipe) string {
 	str := ""
 	if recipe != nil {
diff --git a/onepassword/cli/utils_test.go b/onepassword/cli/utils_test.go
index 7a1c18b8..ac47f544 100644
--- a/onepassword/cli/utils_test.go
+++ b/onepassword/cli/utils_test.go
@@ -50,6 +50,55 @@ func TestPasswordField(t *testing.T) {
 	}
 }
 
+func TestPasswordRecipeExtraction(t *testing.T) {
+	tests := map[string]struct {
+		item           *onepassword.Item
+		expectedString string
+	}{
+		"should return empty string if item has no fields": {
+			item:           &onepassword.Item{},
+			expectedString: "",
+		},
+		"should return empty string if no password field": {
+			item: &onepassword.Item{
+				Fields: []*onepassword.ItemField{
+					{Purpose: onepassword.FieldPurposeNotes},
+				},
+			},
+			expectedString: "",
+		},
+		"should return empty string if no password recipe": {
+			item: &onepassword.Item{
+				Fields: []*onepassword.ItemField{
+					{ID: "username", Purpose: onepassword.FieldPurposeUsername},
+					{ID: "password", Purpose: onepassword.FieldPurposePassword},
+				},
+			},
+			expectedString: "",
+		},
+		"should return recipe string": {
+			item: &onepassword.Item{
+				Fields: []*onepassword.ItemField{
+					{ID: "username", Purpose: onepassword.FieldPurposeUsername},
+					{ID: "password", Purpose: onepassword.FieldPurposePassword, Recipe: &onepassword.GeneratorRecipe{
+						Length: 30,
+					}},
+				},
+			},
+			expectedString: "30",
+		},
+	}
+
+	for description, test := range tests {
+		t.Run(description, func(t *testing.T) {
+			actualString := passwordRecipe(test.item)
+			if actualString != test.expectedString {
+				t.Errorf("Unexpected password recipe string. Expected \"%s\", but got \"%s\"", test.expectedString, actualString)
+			}
+		})
+	}
+}
+
 func TestPasswordRecipeToString(t *testing.T) {
 	tests := map[string]struct {
 		recipe         *onepassword.GeneratorRecipe

From 738709c30e65c3b1d2998c2fc220e139f65d7322 Mon Sep 17 00:00:00 2001
From: Josh Burton <josh@arklabs.nz>
Date: Sat, 23 Dec 2023 10:50:23 +1300
Subject: [PATCH 2/2] simplifies implementation of passwordRecipe function

---
 onepassword/cli/utils.go | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/onepassword/cli/utils.go b/onepassword/cli/utils.go
index 9bfd4079..9c4afb55 100644
--- a/onepassword/cli/utils.go
+++ b/onepassword/cli/utils.go
@@ -61,13 +61,10 @@ func passwordField(item *onepassword.Item) *onepassword.ItemField {
 }
 
 func passwordRecipe(item *onepassword.Item) string {
-	str := ""
 	if pf := passwordField(item); pf != nil {
-		if pf.Recipe != nil {
-			return passwordRecipeToString(pf.Recipe)
-		}
+		return passwordRecipeToString(pf.Recipe)
 	}
-	return str
+	return ""
 }
 
 func passwordRecipeToString(recipe *onepassword.GeneratorRecipe) string {