Merge pull request #173 from cyberark/more-ut

Add more unit tests

conjurapi/authn/auth_token.go

@@ -31,7 +31,7 @@ func hasField(fields map[string]string, name string) (hasField bool) {
 func NewToken(data []byte) (token *AuthnToken, err error) {
 	fields := make(map[string]string)
 	if err = json.Unmarshal(data, &fields); err != nil {
-		err = fmt.Errorf("Unable to unmarshal token : %s", err)
+		err = fmt.Errorf("Unable to unmarshal token: %s", err)
 		return
 	}
 
@@ -53,7 +53,7 @@ func (t *AuthnToken) FromJSON(data []byte) (err error) {
 
 	err = json.Unmarshal(data, &t)
 	if err != nil {
-		err = fmt.Errorf("Unable to unmarshal access token %s", err)
+		err = fmt.Errorf("Unable to unmarshal access token: %s", err)
 		return
 	}
 
@@ -67,7 +67,7 @@ func (t *AuthnToken) FromJSON(data []byte) (err error) {
 	}
 	err = json.Unmarshal(payloadJSON, &payloadFields)
 	if err != nil {
-		err = fmt.Errorf("Unable to unmarshal access token field 'payload' : %s", err)
+		err = fmt.Errorf("Unable to unmarshal access token field 'payload': %s", err)
 		return
 	}
 

conjurapi/authn/auth_token_test.go

@@ -52,12 +52,12 @@ func TestToken_Parse(t *testing.T) {
 
 	t.Run("Malformed JSON in token is reported", func(t *testing.T) {
 		_, err := NewToken([]byte(token_mangled_2_s))
-		assert.Equal(t, "Unable to unmarshal access token field 'payload' : invalid character 'o' in literal false (expecting 'a')", err.Error())
+		assert.Equal(t, "Unable to unmarshal access token field 'payload': invalid character 'o' in literal false (expecting 'a')", err.Error())
 	})
 
 	t.Run("Invalid JSON in token is reported", func(t *testing.T) {
 		token, err := NewToken([]byte("invalid json"))
-		assert.EqualError(t, err, "Unable to unmarshal token : invalid character 'i' looking for beginning of value")
+		assert.EqualError(t, err, "Unable to unmarshal token: invalid character 'i' looking for beginning of value")
 		assert.Nil(t, token)
 	})
 
@@ -78,4 +78,10 @@ func TestToken_Parse(t *testing.T) {
 		_, err := NewToken([]byte(token_exp_before_issued))
 		assert.EqualError(t, err, "access token expired before it was issued")
 	})
+
+	t.Run("FromJSON returns error when provided invalid JSON", func(t *testing.T) {
+		token := &AuthnToken{}
+		err := token.FromJSON([]byte("invalid json"))
+		assert.EqualError(t, err, "Unable to unmarshal access token: invalid character 'i' looking for beginning of value")
+	})
 }

conjurapi/authn/token_file_authenticator_test.go

@@ -2,7 +2,6 @@ package authn
 
 import (
 	"fmt"
-	"io/ioutil"
 	"os"
 	"path"
 	"testing"
@@ -27,7 +26,7 @@ func ensureWriteFile(filepath, filecontents string) {
 		prevModTime = time.Now().Add(-time.Second)
 	}
 
-	err = ioutil.WriteFile(filepath, []byte(filecontents), 0600)
+	err = os.WriteFile(filepath, []byte(filecontents), 0600)
 	if err != nil {
 		panic(err)
 	}
@@ -55,7 +54,7 @@ func ensureWriteFile(filepath, filecontents string) {
 
 func TestTokenFileAuthenticator_RefreshToken(t *testing.T) {
 	t.Run("Retrieve existent token file", func(t *testing.T) {
-		token_file, _ := ioutil.TempFile("", "existent-token-file")
+		token_file, _ := os.CreateTemp("", "existent-token-file")
 		token_file_name := token_file.Name()
 		defer os.Remove(token_file_name)
 
@@ -74,15 +73,13 @@ func TestTokenFileAuthenticator_RefreshToken(t *testing.T) {
 	})
 
 	t.Run("Retrieve eventually existent token file", func(t *testing.T) {
-		token_dir, _ := ioutil.TempDir("", "existent-token-file")
+		token_dir := t.TempDir()
 		token_file_name := path.Join(token_dir, "token")
-		defer os.RemoveAll(token_dir)
 
 		token_file_contents := "token-from-file-contents"
 		go func() {
-			ioutil.WriteFile(token_file_name, []byte(token_file_contents), 0600)
+			os.WriteFile(token_file_name, []byte(token_file_contents), 0600)
 		}()
-		defer os.Remove(token_file_name)
 
 		authenticator := TokenFileAuthenticator{
 			TokenFile:   token_file_name,
@@ -109,11 +106,33 @@ func TestTokenFileAuthenticator_RefreshToken(t *testing.T) {
 		assert.Error(t, err)
 		assert.Equal(t, "Operation waitForTextFile timed out.", err.Error())
 	})
+
+	t.Run("Doesn't time out if MaxWaitTime is -1", func(t *testing.T) {
+		tempDir := t.TempDir()
+		token_file_name := path.Join(tempDir, "token")
+
+		go func() {
+			// Wait some time before writing the file
+			time.Sleep(500 * time.Millisecond)
+			token_file_contents := "token-from-file-contents"
+			os.WriteFile(token_file_name, []byte(token_file_contents), 0600)
+		}()
+
+		authenticator := TokenFileAuthenticator{
+			TokenFile:   token_file_name,
+			MaxWaitTime: -1, // Disable timeout
+		}
+
+		token, err := authenticator.RefreshToken()
+
+		assert.NoError(t, err)
+		assert.Equal(t, "token-from-file-contents", string(token))
+	})
 }
 
 func TestTokenFileAuthenticator_NeedsTokenRefresh(t *testing.T) {
 	t.Run("Token refresh needed after updates", func(t *testing.T) {
-		token_file, _ := ioutil.TempFile("", "existent-token-file")
+		token_file, _ := os.CreateTemp("", "existent-token-file")
 		token_file_name := token_file.Name()
 		defer os.Remove(token_file_name)
 

conjurapi/authn/wait_for_file.go

@@ -2,7 +2,6 @@ package authn
 
 import (
 	"fmt"
-	"io/ioutil"
 	"os"
 	"time"
 )
@@ -23,7 +22,7 @@ waiting_loop:
 			if _, err := os.Stat(fileName); os.IsNotExist(err) {
 				time.Sleep(100 * time.Millisecond)
 			} else {
-				fileBytes, err = ioutil.ReadFile(fileName)
+				fileBytes, err = os.ReadFile(fileName)
 				break waiting_loop
 			}
 		}

conjurapi/authn/wait_for_file_test.go

@@ -1,7 +1,6 @@
 package authn
 
 import (
-	"io/ioutil"
 	"os"
 	"testing"
 	"time"
@@ -18,12 +17,12 @@ func Test_waitForTextFile(t *testing.T) {
 	})
 
 	t.Run("Returns bytes for eventually existent filename", func(t *testing.T) {
-		file_to_exist, _ := ioutil.TempFile("", "existent-file")
+		file_to_exist, _ := os.CreateTemp("", "existent-file")
 		file_to_exist_name := file_to_exist.Name()
 
 		os.Remove(file_to_exist_name)
 		go func() {
-			ioutil.WriteFile(file_to_exist_name, []byte("some random stuff"), 0600)
+			os.WriteFile(file_to_exist_name, []byte("some random stuff"), 0600)
 		}()
 		defer os.Remove(file_to_exist_name)
 

conjurapi/authn_test.go

@@ -150,6 +150,16 @@ func TestClient_RotateHostAPIKey(t *testing.T) {
 				assert.Contains(t, err.Error(), "must represent a host")
 			},
 		},
+		{
+			name:   "Rotate the API key of a foreign host: Malformed ID",
+			hostID: "id:with:too:many:colons",
+			login:  "host/bob",
+			assertions: func(t *testing.T, tc rotateHostAPIKeyTestCase, conjur *Client) {
+				_, err := conjur.RotateUserAPIKey(tc.hostID)
+				assert.Error(t, err)
+				assert.Contains(t, err.Error(), "Malformed ID")
+			},
+		},
 	}
 
 	for _, tc := range testCases {
@@ -214,6 +224,16 @@ func TestClient_RotateUserAPIKey(t *testing.T) {
 				assert.Contains(t, err.Error(), "must represent a user")
 			},
 		},
+		{
+			name:   "Rotate the API key of a user: Malformed ID",
+			userID: "id:with:too:many:colons",
+			login:  "alice",
+			assertions: func(t *testing.T, tc rotateUserAPIKeyTestCase, conjur *Client) {
+				_, err := conjur.RotateUserAPIKey(tc.userID)
+				assert.Error(t, err)
+				assert.Contains(t, err.Error(), "Malformed ID")
+			},
+		},
 	}
 
 	for _, tc := range testCases {

conjurapi/client.go

@@ -5,7 +5,6 @@ import (
 	"crypto/x509"
 	"fmt"
 	"io"
-	"io/ioutil"
 	"net/http"
 	"net/url"
 	"os"
@@ -70,7 +69,7 @@ func NewClientFromOidcCode(config Config, code, nonce, code_verifier string) (*C
 // ReadResponseBody fully reads a response and closes it.
 func ReadResponseBody(response io.ReadCloser) ([]byte, error) {
 	defer response.Close()
-	return ioutil.ReadAll(response)
+	return io.ReadAll(response)
 }
 
 func NewClientFromToken(config Config, token string) (*Client, error) {
@@ -125,15 +124,7 @@ func NewClientFromEnvironment(config Config) (*Client, error) {
 		return NewClientFromKey(config, *loginPair)
 	}
 
-	client, err := newClientFromStoredCredentials(config)
-	if err != nil {
-		return nil, err
-	}
-
-	if client != nil {
-		return client, nil
-	}
-	return nil, fmt.Errorf("Environment variables and machine identity files satisfying at least one authentication strategy must be present!")
+	return newClientFromStoredCredentials(config)
 }
 
 func NewClientFromJwt(config Config, authnJwtServiceID string) (*Client, error) {
@@ -146,7 +137,7 @@ func NewClientFromJwt(config Config, authnJwtServiceID string) (*Client, error)
 			jwtTokenPath = "/var/run/secrets/kubernetes.io/serviceaccount/token"
 		}
 
-		jwtToken, err := ioutil.ReadFile(jwtTokenPath)
+		jwtToken, err := os.ReadFile(jwtTokenPath)
 		if err != nil {
 			return nil, err
 		}
@@ -273,10 +264,10 @@ func (c *Client) LoginRequest(login string, password string) (*http.Request, err
 	authenticateURL := makeRouterURL(c.authnURL(), "login").String()
 
 	req, err := http.NewRequest("GET", authenticateURL, nil)
-	req.SetBasicAuth(login, password)
 	if err != nil {
 		return nil, err
 	}
+	req.SetBasicAuth(login, password)
 	req.Header.Set("Content-Type", "text/plain")
 
 	return req, nil
@@ -531,7 +522,7 @@ func (c *Client) LoadPolicyRequest(mode PolicyMode, policyID string, policy io.R
 	case PolicyModePut:
 		method = "PUT"
 	default:
-		return nil, fmt.Errorf("Invalid PolicyMode : %d", mode)
+		return nil, fmt.Errorf("Invalid PolicyMode: %d", mode)
 	}
 
 	return http.NewRequest(