CyberArk(identity): add tests for LoginUsernamePassword with mock and real APIs

- Introduced `identity_test.go` to validate `LoginUsernamePassword` behavior.
- Added mock server support for `failureUser` to simulate bad user scenarios.
- Included new test data for `failureUser` in `start_authentication_bad_user_session_id.json`.
- Implemented `SkipIfNoEnv` utility to conditionally skip tests based on environment variables.

Signed-off-by: Richard Wall <richard.wall@cyberark.com>

Author: wallrj-cyberark

pkg/internal/cyberark/identity/identity_test.go

@@ -0,0 +1,130 @@
+package identity
+
+// This file contains tests for the LoginUsernamePassword function in the
+// identity package. The tests cover both a mock API server and the real API,
+// depending on the environment variables set. The tests are intended to
+// demonstrate that the mock API behaves the same as the real API
+
+import (
+	"net/http"
+	"os"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"k8s.io/klog/v2"
+	"k8s.io/klog/v2/ktesting"
+
+	"github.com/jetstack/preflight/pkg/internal/cyberark/servicediscovery"
+	arktesting "github.com/jetstack/preflight/pkg/internal/cyberark/testing"
+
+	_ "k8s.io/klog/v2/ktesting/init"
+)
+
+// inputs holds the various input values for the tests.
+type inputs struct {
+	httpClient *http.Client
+	baseURL    string
+	subdomain  string
+	username   string
+	password   string
+}
+
+// TestLoginUsernamePassword_MockAPI tests the LoginUsernamePassword function
+// against a mock API server. The mock server is configured to return different
+// responses based on the username and password used in the request.
+func TestLoginUsernamePassword_MockAPI(t *testing.T) {
+	loginUsernamePasswordTests(t, func(t testing.TB) inputs {
+		baseURL, httpClient := MockIdentityServer(t)
+		return inputs{
+			httpClient: httpClient,
+			baseURL:    baseURL,
+			subdomain:  "subdomain-ignored-by-mock",
+			username:   successUser,
+			password:   successPassword,
+		}
+	})
+}
+
+// TestLoginUsernamePassword_RealAPI tests the LoginUsernamePassword function
+// against the real API. The environment variables are used to configure the
+// client.
+func TestLoginUsernamePassword_RealAPI(t *testing.T) {
+	arktesting.SkipIfNoEnv(t)
+	subdomain := os.Getenv("ARK_SUBDOMAIN")
+	httpClient := http.DefaultClient
+	services, err := servicediscovery.New(httpClient).DiscoverServices(t.Context(), subdomain)
+	require.NoError(t, err)
+
+	loginUsernamePasswordTests(t, func(t testing.TB) inputs {
+		return inputs{
+			httpClient: httpClient,
+			baseURL:    services.Identity.API,
+			subdomain:  subdomain,
+			username:   os.Getenv("ARK_USERNAME"),
+			password:   os.Getenv("ARK_SECRET"),
+		}
+	})
+}
+
+// loginUsernamePasswordTests runs tests which are expected to pass regardless of
+// whether the mock or real API is used.
+func loginUsernamePasswordTests(t *testing.T, inputsGenerator func(t testing.TB) inputs) {
+	type testCase struct {
+		name          string
+		modifier      func(in *inputs)
+		expectedError string
+	}
+	tests := []testCase{
+		{
+			name: "success",
+		},
+		{
+			name: "bad-username",
+			modifier: func(in *inputs) {
+				in.username = failureUser
+			},
+			expectedError: `^got a failure response from request to advance authentication: ` +
+				`message="Authentication \(login or challenge\) has failed\. ` +
+				`Please try again or contact your system administrator\."`,
+		},
+		{
+			name: "empty-username",
+			modifier: func(in *inputs) {
+				in.username = ""
+			},
+			expectedError: `^got a failure response from request to start authentication: ` +
+				`message="Authentication \(login or challenge\) has failed\. ` +
+				`Please try again or contact your system administrator\."`,
+		},
+		{
+			name: "bad-password",
+			modifier: func(in *inputs) {
+				in.password = "bad-password"
+			},
+			expectedError: `^got a failure response from request to advance authentication: ` +
+				`message="Authentication \(login or challenge\) has failed\. ` +
+				`Please try again or contact your system administrator\."`,
+		},
+	}
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			logger := ktesting.NewLogger(t, ktesting.DefaultConfig)
+			ctx := klog.NewContext(t.Context(), logger)
+
+			in := inputsGenerator(t)
+			if test.modifier != nil {
+				test.modifier(&in)
+			}
+			cl := New(in.httpClient, in.baseURL, in.subdomain)
+			err := cl.LoginUsernamePassword(ctx, in.username, []byte(in.password))
+			if test.expectedError != "" {
+				if assert.Error(t, err) {
+					assert.Regexp(t, test.expectedError, err.Error())
+				}
+				return
+			}
+			require.NoError(t, err)
+		})
+	}
+}

pkg/internal/cyberark/identity/mock.go

@@ -17,6 +17,7 @@ import (
 
 const (
 	successUser                   = "test@example.com"
+	failureUser                   = "test-fail@example.com"
 	successUserMultipleChallenges = "test-multiple-challenges@example.com"
 	successUserMultipleMechanisms = "test-multiple-mechanisms@example.com"
 	noUPMechanism                 = "noup@example.com"
@@ -35,6 +36,9 @@ var (
 	//go:embed testdata/start_authentication_success.json
 	startAuthenticationSuccessResponse string
 
+	//go:embed testdata/start_authentication_bad_user_session_id.json
+	startAuthenticationBadUserResponse string
+
 	//go:embed testdata/start_authentication_success_multiple_challenges.json
 	startAuthenticationSuccessMultipleChallengesResponse string
 
@@ -133,11 +137,6 @@ func (mis *mockIdentityServer) handleStartAuthentication(w http.ResponseWriter,
 		return
 	}
 
-	// Important: Experimentally, the Identity server we generated the testdata from seems to accept
-	// any user and provide a success response for StartAuthentication, so filtering on the user here
-	// doesn't match the server's behavior; however, it's helpful to do so since it lets us test different
-	// error handling capabilities in the client.
-
 	switch reqBody.User {
 	case successUser:
 		w.WriteHeader(http.StatusOK)
@@ -160,9 +159,16 @@ func (mis *mockIdentityServer) handleStartAuthentication(w http.ResponseWriter,
 		w.WriteHeader(http.StatusOK)
 		_, _ = w.Write([]byte(startAuthenticationFailureResponse))
 
+	case failureUser:
+		// Experimentally, the real API produces a 200 response and what looks
+		// like a success response body. but the login is rejected later by the
+		// AdvanceAuthentication stage, perhaps by virtue of the sessionID which
+		// is returned here and supplied to AdvanceAuthentication.
+		w.WriteHeader(http.StatusOK)
+		_, _ = w.Write([]byte(startAuthenticationBadUserResponse))
+
 	default:
-		w.WriteHeader(http.StatusForbidden)
-		_, _ = w.Write([]byte(`{"message":"MOCK SERVER: invalid user"}`))
+		panic("programmer error: should not be reached")
 	}
 }
 

pkg/internal/cyberark/identity/testdata/start_authentication_bad_user_session_id.json

@@ -0,0 +1,40 @@
+{
+  "success": true,
+  "Result": {
+    "ClientHints": {
+      "PersistDefault": false,
+      "AllowPersist": true,
+      "AllowForgotPassword": true,
+      "EndpointAuthenticationEnabled": false
+    },
+    "Version": "1.0",
+    "SessionId": "bad-user-session-id",
+    "EventDescription": null,
+    "RetryWaitingTime": 0,
+    "SecurityImageName": null,
+    "AllowLoginMfaCache": false,
+    "Challenges": [
+      {
+        "Mechanisms": [
+          {
+            "AnswerType": "Text",
+            "Name": "UP",
+            "PromptMechChosen": "Enter Password",
+            "PromptSelectMech": "Password",
+            "MechanismId": "aaaaaaa_AAAAAAAAAAAAAAAAAAAAAAAAAAAA-1111111",
+            "Enrolled": true
+          }
+        ]
+      }
+    ],
+    "Summary": "NewPackage",
+    "TenantId": "TENANTID"
+  },
+  "Message": null,
+  "MessageID": null,
+  "Exception": null,
+  "ErrorID": null,
+  "ErrorCode": null,
+  "IsSoftError": false,
+  "InnerExceptions": null
+}

pkg/internal/cyberark/testing/testing.go

@@ -0,0 +1,18 @@
+package testing
+
+import (
+	"os"
+	"testing"
+)
+
+// SkipIfNoEnv skips the test if the required CyberArk environment variables are not set.
+func SkipIfNoEnv(t testing.TB) {
+	t.Helper()
+
+	if os.Getenv("ARK_SUBDOMAIN") == "" ||
+		os.Getenv("ARK_USERNAME") == "" ||
+		os.Getenv("ARK_SECRET") == "" {
+		t.Skip("Skipping test because one of ARK_SUBDOMAIN, ARK_USERNAME or ARK_SECRET isn't set")
+	}
+
+}