From aa59c1ad43ae405f684a8e2fb784b79e7d25d87f Mon Sep 17 00:00:00 2001
From: Luke Murphy <lukewm@riseup.net>
Date: Fri, 1 Jan 2021 18:52:32 +0100
Subject: [PATCH] Make secret generation more robust

Closes https://git.autonomic.zone/coop-cloud/abra/issues/68.
---
 CHANGELOG.md |  3 ++-
 abra         | 32 ++++++++++----------------------
 2 files changed, 12 insertions(+), 23 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index b59f382..03dceae 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,6 +1,6 @@
 # abra x.x.x (UNRELEASED)
 
-- `secret auto` merged into `secret generate` and `app new --auto ` is now `app new --secrets` ([#64](https://git.autonomic.zone/coop-cloud/abra/pulls/64))
+- `secret auto` merged into `secret generate` and `app new --auto` is now `app new --secrets` ([#64](https://git.autonomic.zone/coop-cloud/abra/pulls/64))
 - Avoid outputting length during secret generation when not in use ([#67](https://git.autonomic.zone/coop-cloud/abra/issues/67))
 - Support graceful failure when missing secret generation commands ([44d3ac3a1cb86edc9b9e91eea1a00e70eae14965](https://git.autonomic.zone/coop-cloud/abra/commit/44d3ac3a1cb86edc9b9e91eea1a00e70eae14965))
 - Fix secret detection when using new `.env` file format in apps ([55324524ca77141666ffe6cc41b62cc71cf89ace](https://git.autonomic.zone/coop-cloud/abra/commit/55324524ca77141666ffe6cc41b62cc71cf89ace))
@@ -16,6 +16,7 @@
 - Use Docker-in-Docker (dind), and `dind-bats-kcov` Docker image, for `make test` ([1600b6277fbbffc4c6de1e4ba799c7bbe72ec6a0](https://git.autonomic.zone/coop-cloud/abra/commit/1600b6277fbbffc4c6de1e4ba799c7bbe72ec6a0))
 - Add built-in documentation using `abra help <subcommand>...`, see [#50](https://git.autonomic.zone/coop-cloud/abra/issues/50)
 - `version` subcommand [e6b24fe](https://git.autonomic.zone/coop-cloud/abra/commit/e6b24fe)
+- Use `# length=x` comments to generate passwords with `pwgen` and drop `KEY`/`PASSWORD` logic ([#68](https://git.autonomic.zone/coop-cloud/abra/issues/68))
 
 # abra 0.4.1 (2020-12-24)
 
diff --git a/abra b/abra
index 498a7be..abeb478 100755
--- a/abra
+++ b/abra
@@ -385,18 +385,8 @@ get_servers() {
 }
 
 get_app_secrets() {
-  get_app_passwords
-  get_app_keys
-}
-
-get_app_passwords() {
   # FIXME 3wc: requires bash 4, use for loop instead
-  mapfile -t PASSWORDS < <(grep "SECRET.*PASSWORD.*VERSION.*" "$ENV_FILE")
-}
-
-get_app_keys() {
-  # FIXME 3wc: requires bash 4, use for loop instead
-  mapfile -t KEYS < <(grep "SECRET.*KEY.*VERSION.*" "$ENV_FILE")
+  mapfile -t PASSWORDS < <(grep "SECRET.*VERSION.*" "$ENV_FILE")
 }
 
 load_instance() {
@@ -457,7 +447,9 @@ parse_secret() {
     # shellcheck disable=SC2001
     abra__length_="$(echo "$SECRET" | sed -e 's/.*[^0-9]\([0-9]\+\)[^0-9]*$/\1/')"
   else
-    abra__length_=32
+    # Note(decentral1se): unset this so that a length value from another secret
+    # definition does not get passed on to another secret generation flow
+    unset abra__length_
   fi
 
   abra__secret_="${SECRET%_VERSION=*}"  # strip _VERSION=v1
@@ -466,7 +458,7 @@ parse_secret() {
 
   abra__version_="$(echo "$SECRET" | sed -n 's/.*\(v[0-9]\).*/\1/p')"
 
-  if [[ "$SECRET" == *"length"* ]]; then
+  if [[ -n "$abra__length_" ]]; then
     echo "Generating $abra__secret_, version: $abra__version_, length: $abra__length_"
   else
     echo "Generating $abra__secret_, version: $abra__version_"
@@ -497,10 +489,6 @@ auto_gen_secrets (){
   for PASSWORD in "${PASSWORDS[@]}"; do
     parse_secret "$PASSWORD"
   done
-
-  for KEY in "${KEYS[@]}"; do
-    parse_secret "$KEY"
-  done
 }
 
 #######################################
@@ -671,7 +659,7 @@ sub_app_new (){
 
   get_app_secrets
 
-  if [ "${#PASSWORDS[@]}" -gt 0 ] || [ "${#KEYS[@]}" -gt 0 ] && [ "$abra___secrets" == "true" ]; then
+  if [ "${#PASSWORDS[@]}" -gt 0 ] && [ "$abra___secrets" == "true" ]; then
     auto_gen_secrets
   fi
 
@@ -980,12 +968,12 @@ sub_app_secret_generate(){
     auto_gen_secrets
   fi
 
-  if [[ "$SECRET" == *"password"* ]]; then
-    require_pwqgen
-    PWGEN="${abra__cmd_:-pwqgen}"
-  else
+  if [[ -n "$LENGTH" ]]; then
     require_pwgen
     PWGEN=${abra__cmd_:-pwgen -s "$LENGTH" 1}
+  else
+    require_pwqgen
+    PWGEN="${abra__cmd_:-pwqgen}"
   fi
 
   if [ -z "$SECRET" ] || [ -z "$VERSION" ] && [ "$abra___all" == "false" ]; then