From dc8e6ae450536911007eded2bf0368faaed4dee2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=F0=9F=92=A5Hedi=20Ghediri?= <h.ghediri@criteo.com>
Date: Tue, 16 Jul 2024 13:55:47 +0200
Subject: [PATCH 1/2] =?UTF-8?q?=F0=9F=A7=91=E2=80=8D=F0=9F=92=BB=20login?=
 =?UTF-8?q?=20command=20-=20print=20selected=20username=20to=20avoid=20amb?=
 =?UTF-8?q?iguity?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This will help preventing an unexpected username saved in the store.

The behaviour is similar to most cli apps:

```
$ cl login
Please enter your user name [default_user_name]: user_input
```
If the user enters empty string, then `default_user_name` is used instead
of `user_input`.
---
 cmd/login.go                                  | 26 ++++----
 test/integration/test-consent.sh              | 10 +--
 test/integration/test-login.sh                | 66 +++++++++++++++++++
 test/integration/test-system-cmd.sh           |  6 +-
 test/packages-src/login/bonjour.bat           |  6 --
 test/packages-src/login/bonjour.sh            |  6 --
 test/packages-src/login/manifest.mf           | 20 ++++--
 test/packages-src/login/print-credentials.bat |  5 ++
 test/packages-src/login/print-credentials.sh  |  5 ++
 9 files changed, 114 insertions(+), 36 deletions(-)
 create mode 100755 test/integration/test-login.sh
 delete mode 100755 test/packages-src/login/bonjour.bat
 delete mode 100755 test/packages-src/login/bonjour.sh
 create mode 100755 test/packages-src/login/print-credentials.bat
 create mode 100755 test/packages-src/login/print-credentials.sh

diff --git a/cmd/login.go b/cmd/login.go
index 873521c..b270079 100644
--- a/cmd/login.go
+++ b/cmd/login.go
@@ -1,9 +1,11 @@
 package cmd
 
 import (
+	"bufio"
 	"encoding/json"
 	"fmt"
 	"os"
+	"strings"
 	"syscall"
 
 	"github.com/criteo/command-launcher/internal/command"
@@ -50,17 +52,17 @@ The credential will be stored in your system vault.`, appCtx.PasswordEnvVar()),
 			appCtx, _ := context.AppContext()
 			username := loginFlags.username
 			if username == "" {
-				username = os.Getenv(appCtx.UsernameEnvVar())
-				if username == "" {
-					fmt.Printf("Please enter your user name: ")
-					nb, err := fmt.Scan(&username)
-					if err != nil {
-						return err
-					}
-
-					if nb != 1 {
-						return fmt.Errorf("invalid entries (expected only one argument)")
-					}
+				reader := bufio.NewReader(os.Stdin)
+				defaultUser := defaultUsername()
+				fmt.Printf("Please enter your user name [%s]: ", defaultUser)
+				input, err := reader.ReadString('\n')
+				if err != nil {
+					return err
+				}
+				if input = strings.TrimSpace(input); input != "" {
+					username = input
+				} else {
+					username = defaultUser
 				}
 			}
 
@@ -101,7 +103,7 @@ The credential will be stored in your system vault.`, appCtx.PasswordEnvVar()),
 			return nil
 		},
 	}
-	loginCmd.Flags().StringVarP(&loginFlags.username, "user", "u", defaultUsername(), "User name")
+	loginCmd.Flags().StringVarP(&loginFlags.username, "user", "u", "", "User name")
 	loginCmd.Flags().StringVarP(&loginFlags.password, "password", "p", "", "User password")
 
 	rootCmd.AddCommand(loginCmd)
diff --git a/test/integration/test-consent.sh b/test/integration/test-consent.sh
index 2efa781..085d4de 100755
--- a/test/integration/test-consent.sh
+++ b/test/integration/test-consent.sh
@@ -20,7 +20,7 @@ $CL_PATH login -u test-user -p test-password
 
 echo "> test consent disabled"
 $CL_PATH config enable_user_consent false
-RESULT=$($CL_PATH bonjour-consent)
+RESULT=$($CL_PATH print-credentials-with-consent)
 echo "$RESULT"
 echo "$RESULT" | grep -q "test-user"
 if [ $? -eq 0 ]; then
@@ -40,7 +40,7 @@ fi
 
 echo "> test consent enabled - user refused"
 $CL_PATH config enable_user_consent true
-RESULT=$(echo 'n' | $CL_PATH bonjour-consent)
+RESULT=$(echo 'n' | $CL_PATH print-credentials-with-consent)
 echo "$RESULT"
 echo "$RESULT" | grep -q "authorize the access?"
 if [ $? -eq 0 ]; then
@@ -68,7 +68,7 @@ fi
 
 echo "> test consent enabled - user authorized"
 $CL_PATH config enable_user_consent true
-RESULT=$(echo 'y' | $CL_PATH bonjour-consent)
+RESULT=$(echo 'y' | $CL_PATH print-credentials-with-consent)
 echo "$RESULT"
 echo "$RESULT" | grep -q "authorize the access?"
 if [ $? -eq 0 ]; then
@@ -96,7 +96,7 @@ fi
 
 echo "> test consent authorized - should not request authorization again"
 $CL_PATH config enable_user_consent true
-RESULT=$(echo 'y' | $CL_PATH bonjour-consent)
+RESULT=$(echo 'y' | $CL_PATH print-credentials-with-consent)
 echo "$RESULT"
 echo "$RESULT" | grep -q "authorize the access?"
 if [ $? -eq 0 ]; then
@@ -109,7 +109,7 @@ fi
 echo "> test consent authorized - should request authorization once expired"
 sleep 5
 $CL_PATH config enable_user_consent true
-RESULT=$(echo 'y' | $CL_PATH bonjour-consent)
+RESULT=$(echo 'y' | $CL_PATH print-credentials-with-consent)
 echo "$RESULT"
 echo "$RESULT" | grep -q "authorize the access?"
 if [ $? -eq 0 ]; then
diff --git a/test/integration/test-login.sh b/test/integration/test-login.sh
new file mode 100755
index 0000000..7879494
--- /dev/null
+++ b/test/integration/test-login.sh
@@ -0,0 +1,66 @@
+#!/bin/bash
+
+# required environment varibale
+# CL_PATH
+# CL_HOME
+# OUTPUT_DIR
+
+SCRIPT_DIR=${1:-$( cd -- "$( dirname -- "${BASH_SOURCE[0]}" )" &> /dev/null && pwd )}
+
+# First copy the dropin packages for test
+rm -rf $CL_HOME/dropins
+mkdir -p $CL_HOME/dropins
+cp -R $SCRIPT_DIR/../packages-src/login $CL_HOME/dropins
+
+
+###
+# login without -u
+###
+echo "Checking login without -u flag"
+RESULT=$(echo | $CL_PATH login -p test-password) # The echo simulates an empty input from the user
+echo -e "Actual:\n$RESULT"
+EXPECTED_USERNAME_PROMPT="Please enter your user name [$(whoami)]: "
+TEST_DESCRIPTION="should prompt '$EXPECTED_USERNAME_PROMPT'"
+if ! grep -qF "$EXPECTED_USERNAME_PROMPT" <(echo "$RESULT"); then
+    echo "KO - $TEST_DESCRIPTION"
+    exit 1
+else
+    echo "OK - $TEST_DESCRIPTION"
+fi
+
+RESULT=$($CL_PATH print-credentials)
+echo -e "Actual:\n$RESULT"
+EXPECTED_USERNAME="CL_USERNAME: $(whoami)"
+TEST_DESCRIPTION="should have username $(whoami)"
+if ! grep -qF "$EXPECTED_USERNAME" <(echo "$RESULT"); then
+    echo "KO - $TEST_DESCRIPTION"
+    exit 1
+else
+    echo "OK - $TEST_DESCRIPTION"
+fi
+
+
+###
+# login with -u
+###
+echo "Checking login with -u flag"
+RESULT=$($CL_PATH login -u test-user -p test-password)
+echo -e "Actual:\n$RESULT"
+TEST_DESCRIPTION=" No output is expected"
+if [[ "$RESULT" =~ ^[[:space:]]*$ ]]; then
+    echo "OK - $TEST_DESCRIPTION"
+else
+    echo "KO - $TEST_DESCRIPTION"
+    exit 1
+fi
+
+RESULT=$($CL_PATH print-credentials)
+echo -e "Actual:\n$RESULT"
+EXPECTED_USERNAME="CL_USERNAME: test-user"
+TEST_DESCRIPTION="should have username test-user"
+if ! grep -qF "$EXPECTED_USERNAME" <(echo "$RESULT"); then
+    echo "KO - $TEST_DESCRIPTION"
+    exit 1
+else
+    echo "OK - $TEST_DESCRIPTION"
+fi
diff --git a/test/integration/test-system-cmd.sh b/test/integration/test-system-cmd.sh
index 76d07ff..4f8cb59 100755
--- a/test/integration/test-system-cmd.sh
+++ b/test/integration/test-system-cmd.sh
@@ -73,7 +73,7 @@ fi
 echo "> test login extension without setup system package"
 echo "* should NOT use username returned from extension"
 RESULT=$($CL_PATH login -u test-user -p test-password)
-RESULT=$($CL_PATH bonjour-consent)
+RESULT=$($CL_PATH print-credentials-with-consent)
 echo "$RESULT" | grep -q "SECRET_1"
 if [ $? -ne 0 ]; then
   # ok
@@ -105,7 +105,7 @@ echo "> test login extension enabled by system package config"
 # set system package name
 RESULT=$($CL_PATH config system_package system-pkg-demo)
 $CL_PATH login -u test-user -p test-password
-RESULT=$($CL_PATH bonjour-consent)
+RESULT=$($CL_PATH print-credentials-with-consent)
 
 echo "* should use username returned from extension"
 echo "$RESULT" | grep -q "SECRET_1"
@@ -157,7 +157,7 @@ else
   exit 1
 fi
 
-echo "$RESULT" | grep 'dropin bonjour default bonjour-consent'
+echo "$RESULT" | grep 'dropin print-credentials default print-credentials-with-consent'
 if [ $? -eq 0 ]; then
   echo "OK"
 else
diff --git a/test/packages-src/login/bonjour.bat b/test/packages-src/login/bonjour.bat
deleted file mode 100755
index 2100295..0000000
--- a/test/packages-src/login/bonjour.bat
+++ /dev/null
@@ -1,6 +0,0 @@
-ECHO off
-
-ECHO bonjour!
-ECHO %CL_USERNAME%
-ECHO %CL_PASSWORD%
-ECHO %CL_AUTH_TOKEN%
diff --git a/test/packages-src/login/bonjour.sh b/test/packages-src/login/bonjour.sh
deleted file mode 100755
index 29eda19..0000000
--- a/test/packages-src/login/bonjour.sh
+++ /dev/null
@@ -1,6 +0,0 @@
-#!/bin/sh
-
-echo "bonjour!"
-echo $CL_USERNAME
-echo $CL_PASSWORD
-echo $CL_AUTH_TOKEN
diff --git a/test/packages-src/login/manifest.mf b/test/packages-src/login/manifest.mf
index 60dcb5f..9c8aea9 100644
--- a/test/packages-src/login/manifest.mf
+++ b/test/packages-src/login/manifest.mf
@@ -1,12 +1,12 @@
 {
-  "pkgName": "bonjour",
+  "pkgName": "print-credentials",
   "version": "1.0.0",
   "cmds": [
     {
-      "name": "bonjour-consent",
+      "name": "print-credentials-with-consent",
       "type": "executable",
-      "short": "print bonjour from command launcher",
-      "executable": "{{.PackageDir}}/bonjour.{{if eq .Os \"windows\"}}bat{{else}}sh{{end}}",
+      "short": "print credentials from command launcher",
+      "executable": "{{.PackageDir}}/print-credentials.{{if eq .Os \"windows\"}}bat{{else}}sh{{end}}",
       "args": [],
       "requiredFlags": [
         "name\t n\t greeting name",
@@ -14,6 +14,18 @@
       ],
       "checkFlags": true,
       "requestedResources": [ "USERNAME", "PASSWORD", "AUTH_TOKEN" ]
+    },
+    {
+      "name": "print-credentials",
+      "type": "executable",
+      "short": "print credentials from command launcher",
+      "executable": "{{.PackageDir}}/print-credentials.{{if eq .Os \"windows\"}}bat{{else}}sh{{end}}",
+      "args": [],
+      "requiredFlags": [
+        "name\t n\t greeting name",
+        "language\t l\tgreeting language"
+      ],
+      "checkFlags": true
     }
   ]
 }
diff --git a/test/packages-src/login/print-credentials.bat b/test/packages-src/login/print-credentials.bat
new file mode 100755
index 0000000..99affc9
--- /dev/null
+++ b/test/packages-src/login/print-credentials.bat
@@ -0,0 +1,5 @@
+@ECHO off
+
+ECHO CL_USERNAME: %CL_USERNAME%
+ECHO CL_PASSWORD: %CL_PASSWORD%
+ECHO CL_AUTH_TOKEN: %CL_AUTH_TOKEN%
diff --git a/test/packages-src/login/print-credentials.sh b/test/packages-src/login/print-credentials.sh
new file mode 100755
index 0000000..8192506
--- /dev/null
+++ b/test/packages-src/login/print-credentials.sh
@@ -0,0 +1,5 @@
+#!/bin/sh
+
+echo "CL_USERNAME: $CL_USERNAME"
+echo "CL_PASSWORD: $CL_PASSWORD"
+echo "CL_AUTH_TOKEN: $CL_AUTH_TOKEN"

From 01e9887f3afac999fedf11daa11726feb60dc4e2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=F0=9F=92=A5Hedi=20Ghediri?= <h.ghediri@criteo.com>
Date: Wed, 7 Aug 2024 09:32:50 +0200
Subject: [PATCH 2/2] Make CL_USERNAME the default login username

---
 cmd/login.go | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/cmd/login.go b/cmd/login.go
index b270079..0162fb6 100644
--- a/cmd/login.go
+++ b/cmd/login.go
@@ -26,13 +26,18 @@ var (
 	loginFlags = LoginFlags{}
 )
 
-func defaultUsername() string {
-	user, present := os.LookupEnv("USER")
-	if !present {
-		user, _ = os.LookupEnv("USERNAME")
+func defaultUsername(appCtx context.LauncherContext) string {
+	user, present := os.LookupEnv(appCtx.UsernameEnvVar())
+	if present {
+		return user
 	}
 
-	return user
+	user, present = os.LookupEnv("USER")
+	if present {
+		return user
+	}
+
+	return os.Getenv("USERNAME")
 }
 
 func AddLoginCmd(rootCmd *cobra.Command, appCtx context.LauncherContext, loginHook command.Command) {
@@ -53,7 +58,7 @@ The credential will be stored in your system vault.`, appCtx.PasswordEnvVar()),
 			username := loginFlags.username
 			if username == "" {
 				reader := bufio.NewReader(os.Stdin)
-				defaultUser := defaultUsername()
+				defaultUser := defaultUsername(appCtx)
 				fmt.Printf("Please enter your user name [%s]: ", defaultUser)
 				input, err := reader.ReadString('\n')
 				if err != nil {