Merge pull request #598 from reubenmiller/fix-load-session-with-empty-token

fix(sessions): fallback to username/password auth (if present) if a token is not set

Author: reubenmiller

pkg/cmd/devices/enroll/enroll.manual.go

@@ -135,7 +135,7 @@ func (n *DeviceEnrollCmd) RunE(cmd *cobra.Command, args []string) error {
 	if err != nil {
 		return err
 	}
-	_ = llog
+	c8y.Logger = llog
 
 	c8yclient, err := n.factory.Client()
 	if err != nil {

pkg/cmd/factory/c8yclient.go

@@ -266,6 +266,14 @@ func CreateCumulocityClient(f *cmdutil.Factory, sessionFile, username, password
 	}
 }
 
+func parseBasicAuthCredentials(client *c8y.Client, tenant, username, password string) {
+	if t, u, found := strings.Cut(username, "/"); found {
+		tenant = t
+		username = u
+	}
+	client.SetTenantUsernamePassword(tenant, username, password)
+}
+
 func loadAuthentication(conf *config.Config, client *c8y.Client) error {
 	loginType := conf.GetLoginTypeRaw()
 	if loginType == "" {
@@ -280,7 +288,7 @@ func loadAuthentication(conf *config.Config, client *c8y.Client) error {
 
 		// password
 		if p, err := conf.GetPassword(); err == nil && p != "" {
-			client.SetTenantUsernamePassword(conf.GetTenant(), conf.GetUsername(), p)
+			parseBasicAuthCredentials(client, conf.GetTenant(), conf.GetUsername(), p)
 			return nil
 		}
 
@@ -308,7 +316,7 @@ func loadAuthentication(conf *config.Config, client *c8y.Client) error {
 		if err != nil {
 			return err
 		}
-		client.SetTenantUsernamePassword(conf.GetTenant(), conf.GetUsername(), password)
+		parseBasicAuthCredentials(client, conf.GetTenant(), conf.GetUsername(), password)
 		return nil
 	}
 

pkg/cmd/root/root.go

@@ -820,12 +820,6 @@ func (c *CmdRoot) Configure(disableEncryptionCheck, forceVerbose, forceDebug boo
 			return c.client, nil
 		}
 		client, err := factory.CreateCumulocityClient(c.Factory, c.SessionFile, c.SessionUsername, c.SessionPassword, disableEncryptionCheck)()
-		if client != nil {
-			if c.SessionUsername != "" || c.SessionPassword != "" {
-				client.SetUsernamePassword(c.SessionUsername, c.SessionPassword)
-				c.log.Debug("Forcing basic authentication as user provided username/password")
-			}
-		}
 
 		if c.log != nil {
 			c8y.Logger = c.log
@@ -872,11 +866,15 @@ func (c *CmdRoot) checkSessionExists(cmd *cobra.Command, args []string) error {
 	// print log information
 	sessionFile := cfg.GetSessionFile()
 	if sessionFile != "" {
-		log.Infof("Loaded session: %s", cfg.HideSensitiveInformationIfActive(client, sessionFile))
+		log.Infof("Loading session from file: %s", cfg.HideSensitiveInformationIfActive(client, sessionFile))
 		if _, err := os.Stat(sessionFile); err != nil {
 			if c8ysession.IsSessionFilePath(sessionFile) {
 				log.Warnf("Failed to verify session file. %s", err)
+			} else {
+				log.Warnf("Given file is not a session file. %s", err)
 			}
+		} else {
+			log.Infof("Loaded session: %s", cfg.HideSensitiveInformationIfActive(client, sessionFile))
 		}
 	}
 

pkg/cmd/sessions/create/create.manual.go

@@ -174,6 +174,7 @@ func (n *CmdCreate) promptArgs(cmd *cobra.Command, args []string) error {
 	if err != nil {
 		return err
 	}
+	c8y.Logger = log
 	prompter := prompt.NewPrompt(log)
 
 	if !cmd.Flags().Changed("host") {

pkg/cmd/sessions/login/login.manual.go

@@ -401,6 +401,7 @@ func (n *CmdLogin) RunE(cmd *cobra.Command, args []string) error {
 	if err != nil {
 		return err
 	}
+	c8y.Logger = log
 
 	canChangeActiveSession := true
 	// Warn users if they try to use this command directly

pkg/config/cliConfiguration.go

@@ -701,6 +701,10 @@ func (c *Config) BindAuthorization() error {
 
 // GetUsername returns the Cumulocity username for the session
 func (c *Config) GetUsername() string {
+	if v := c.GetSessionUsername(); v != "" {
+		c.Logger.Infof("Using session username override")
+		return v
+	}
 	v := c.viper.GetString("username")
 
 	if v != "" {
@@ -961,6 +965,11 @@ func (c *Config) WritePersistentConfig() error {
 
 // GetPassword returns the decrypted password of the current session
 func (c *Config) GetPassword() (string, error) {
+	if v := c.GetSessionPassword(); v != "" {
+		c.Logger.Infof("Using session password override")
+		return v, nil
+	}
+
 	value := c.GetPasswordRaw()
 
 	if value == "" {
@@ -1720,16 +1729,32 @@ func ParseLoginTypeWithDefault(v string) string {
 
 // GetLoginTypeWithDefault get the preferred login type
 func (c *Config) GetLoginTypeWithDefault() string {
-	v := c.viper.GetString(SettingsLoginType)
+	v := c.GetLoginTypeRaw()
 	return ParseLoginTypeWithDefault(v)
 }
 
 // GetLoginTypeRaw get the raw value, where it could also be an empty value
 func (c *Config) GetLoginTypeRaw() string {
+	if c.HasSessionUsernameOrPassword() {
+		// Force BASIC AUTH
+		return c8y.LoginTypeBasic
+	}
 	v := c.viper.GetString(SettingsLoginType)
 	return strings.ToUpper(v)
 }
 
+func (c *Config) HasSessionUsernameOrPassword() bool {
+	return c.GetSessionUsername() != "" || c.GetSessionPassword() != ""
+}
+
+func (c *Config) GetSessionUsername() string {
+	return c.viper.GetString("settings.defaults.sessionUsername")
+}
+
+func (c *Config) GetSessionPassword() string {
+	return c.viper.GetString("settings.defaults.sessionPassword")
+}
+
 // SetLoginType sets the authorization method, e.g. BASIC, OAUTH2_INTERNAL, NONE
 func (c *Config) SetLoginType(v string) {
 	value, err := c8y.ParseLoginType(v)

tests/manual/sessions/create/create-session.sh

@@ -0,0 +1,57 @@
+#!/usr/bin/env bash
+set -e
+if [ -n "$CI" ]; then
+    set -x
+fi
+
+# store the current session variables, before clearing the session (to simulate creating a new session)
+BACKUP_C8Y_HOST="$C8Y_HOST"
+BACKUP_C8Y_TENANT="$C8Y_TENANT"
+BACKUP_C8Y_USER="$C8Y_USER"
+BACKUP_C8Y_PASSWORD="$C8Y_PASSWORD"
+
+if [ -z "$C8Y_PASSWORD" ]; then
+    echo "This test requires the 'C8Y_PASSWORD' env variable to be set!"
+    exit 1
+fi
+
+echo "Clearing existing session" >&2
+eval "$(c8y sessions clear --shell bash)" ||:
+
+TMPDIR=$(mktemp -d)
+cleanup () {
+    rm -rf "$TMPDIR"
+}
+trap cleanup EXIT
+export C8Y_SESSION_HOME="$TMPDIR"
+
+#
+# Test case 1: Create a session and reference it via the --session global flag
+#
+c8y sessions create \
+    --mode dev \
+    --username "$BACKUP_C8Y_USER" \
+    --host "$BACKUP_C8Y_HOST" \
+    --tenant "$BACKUP_C8Y_TENANT" \
+    --password "$BACKUP_C8Y_PASSWORD" \
+    --name subtenant
+c8y devices list -p 1 -n --session "$TMPDIR/subtenant.json"
+
+#
+# Test case 2: Create a session without storing the password and use a combination
+# of --session and --sessionPassword
+#
+c8y sessions create \
+    --mode dev \
+    --username "$BACKUP_C8Y_USER" \
+    --host "$BACKUP_C8Y_HOST" \
+    --tenant "$BACKUP_C8Y_TENANT" \
+    --password "$BACKUP_C8Y_PASSWORD" \
+    --name subtenant \
+    --noStorage
+
+echo "Checking session credentials using custom session password" >&2
+c8y devices list -p 1 -n --session "$TMPDIR/subtenant.json" --sessionPassword "$BACKUP_C8Y_PASSWORD"
+
+echo "Checking resolution of session using just the name" >&2
+c8y devices list -p 1 -n --session "subtenant.json" --sessionPassword "$BACKUP_C8Y_PASSWORD"

tests/manual/sessions/create/session_create.yaml

@@ -64,3 +64,8 @@ tests:
           "password": "test",
           "username": "dummy@me.com"
         }
+
+  It supports creating a session and referering it using the session global flag:
+    command: |
+      ./manual/sessions/create/create-session.sh
+    exit-code: 0