Merged in bug/eula-email (pull request #212)

fix eula issue when email not present in jwt * bug fix
2026-02-26 21:46:00 +00:00
parent c668485e6f
commit 62b5de5722
7 changed files with 648 additions and 312 deletions
@@ -522,6 +522,172 @@ func TestListEulaAgreements(t *testing.T) {
 	})
 }

+// TestListEulaAgreements_NonEmailIdentifier verifies that GET /admin/eula/agreements
+// returns 200 (not 500) when agreements contain non-email user identifiers.
+// Reproduces the bug where username/subject ID fallback values in userEmail caused
+// JSON serialization failure via openapi_types.Email strict validation.
+func TestListEulaAgreements_NonEmailIdentifier(t *testing.T) {
+	cfg := &ControllerConfig{}
+	test.CreateDB(t, cfg)
+	initializeTestConfig(t, cfg)
+
+	svc := createControllerServices(cfg)
+	cons := queryapi.NewControllers(svc, cfg)
+
+	versionID := createTestEulaVersion(t, svc.Eula, "nonemail-agree-"+uuid.New().String()[:8], "Non-Email Test", true)
+
+	// Record agreements with non-email identifiers (simulating username/subject fallback)
+	nonEmailIdentifiers := []string{"accessuser", "bare-subject-id-12345", "admin_username"}
+	for _, identifier := range nonEmailIdentifiers {
+		_, err := svc.Eula.RecordAgreement(t.Context(), &eula.RecordAgreementInput{
+			CognitoSubjectID: "user-" + uuid.New().String()[:8],
+			UserEmail:        identifier,
+			EulaVersionID:    *versionID,
+			IPAddress:        "192.168.1.1",
+		})
+		require.NoError(t, err)
+	}
+
+	t.Run("returns 200 with non-email userEmail values", func(t *testing.T) {
+		ctx, rec := setupEulaTestContext(http.MethodGet, "/admin/eula/agreements", nil)
+
+		page := int32(1)
+		pageSize := int32(50)
+		params := queryapi.ListEulaAgreementsParams{
+			Page:     &page,
+			PageSize: &pageSize,
+		}
+
+		err := cons.ListEulaAgreements(ctx, params)
+		require.NoError(t, err)
+		assert.Equal(t, http.StatusOK, rec.Code, "expected 200 OK, got %d: %s", rec.Code, rec.Body.String())
+
+		var response queryapi.EulaAgreementList
+		err = json.Unmarshal(rec.Body.Bytes(), &response)
+		require.NoError(t, err)
+
+		// Verify non-email identifiers are present in the response
+		foundIdentifiers := make(map[string]bool)
+		for _, a := range response.Agreements {
+			foundIdentifiers[string(a.UserEmail)] = true
+		}
+		for _, id := range nonEmailIdentifiers {
+			assert.True(t, foundIdentifiers[id], "expected to find non-email identifier %q in response", id)
+		}
+	})
+}
+
+// TestListEulaVersions_NonEmailCreatedBy verifies that GET /admin/eula
+// returns 200 (not 500) when versions have non-email createdBy/activatedBy values.
+func TestListEulaVersions_NonEmailCreatedBy(t *testing.T) {
+	cfg := &ControllerConfig{}
+	test.CreateDB(t, cfg)
+	initializeTestConfig(t, cfg)
+
+	svc := createControllerServices(cfg)
+	cons := queryapi.NewControllers(svc, cfg)
+
+	// Create a version with a non-email createdBy (simulating username fallback)
+	versionStr := "nonemail-ver-" + uuid.New().String()[:8]
+	_, err := svc.Eula.CreateVersion(t.Context(), &eula.CreateVersionInput{
+		Version:   versionStr,
+		Title:     "Non-Email CreatedBy Test",
+		Content:   "# Test EULA\n\nContent.",
+		CreatedBy: "adminuser", // non-email identifier
+	})
+	require.NoError(t, err)
+
+	t.Run("returns 200 with non-email createdBy", func(t *testing.T) {
+		ctx, rec := setupEulaTestContext(http.MethodGet, "/admin/eula", nil)
+
+		page := int32(1)
+		pageSize := int32(50)
+		params := queryapi.ListEulaVersionsParams{
+			Page:     &page,
+			PageSize: &pageSize,
+		}
+
+		err := cons.ListEulaVersions(ctx, params)
+		require.NoError(t, err)
+		assert.Equal(t, http.StatusOK, rec.Code, "expected 200 OK, got %d: %s", rec.Code, rec.Body.String())
+
+		var response queryapi.EulaVersionList
+		err = json.Unmarshal(rec.Body.Bytes(), &response)
+		require.NoError(t, err)
+
+		// Find our version and verify the non-email createdBy is present
+		found := false
+		for _, v := range response.Versions {
+			if v.Version == versionStr {
+				assert.Equal(t, "adminuser", string(v.CreatedBy))
+				found = true
+				break
+			}
+		}
+		assert.True(t, found, "expected to find version %q in response", versionStr)
+	})
+}
+
+// TestActivateEulaVersion_NonEmailActivatedBy verifies that activating a version
+// with a non-email admin identifier doesn't cause serialization errors when
+// the version is later retrieved.
+func TestActivateEulaVersion_NonEmailActivatedBy(t *testing.T) {
+	cfg := &ControllerConfig{}
+	test.CreateDB(t, cfg)
+	initializeTestConfig(t, cfg)
+
+	svc := createControllerServices(cfg)
+	cons := queryapi.NewControllers(svc, cfg)
+
+	// Create version with non-email createdBy
+	versionStr := "nonemail-act-" + uuid.New().String()[:8]
+	created, err := svc.Eula.CreateVersion(t.Context(), &eula.CreateVersionInput{
+		Version:   versionStr,
+		Title:     "Non-Email ActivatedBy Test",
+		Content:   "# Test EULA\n\nContent.",
+		CreatedBy: "sysadmin",
+	})
+	require.NoError(t, err)
+
+	// Activate with non-email identifier
+	_, err = svc.Eula.ActivateVersion(t.Context(), created.ID, "opsadmin")
+	require.NoError(t, err)
+
+	t.Run("returns 200 listing version with non-email activatedBy", func(t *testing.T) {
+		ctx, rec := setupEulaTestContext(http.MethodGet, "/admin/eula", nil)
+
+		page := int32(1)
+		pageSize := int32(50)
+		params := queryapi.ListEulaVersionsParams{
+			Page:     &page,
+			PageSize: &pageSize,
+		}
+
+		err := cons.ListEulaVersions(ctx, params)
+		require.NoError(t, err)
+		assert.Equal(t, http.StatusOK, rec.Code, "expected 200 OK, got %d: %s", rec.Code, rec.Body.String())
+
+		var response queryapi.EulaVersionList
+		err = json.Unmarshal(rec.Body.Bytes(), &response)
+		require.NoError(t, err)
+
+		// Find our version and verify the non-email activatedBy is present
+		found := false
+		for _, v := range response.Versions {
+			if v.Version == versionStr {
+				assert.Equal(t, "sysadmin", string(v.CreatedBy))
+				assert.True(t, v.ActivatedBy.IsSpecified(), "activatedBy should be set")
+				val, err := v.ActivatedBy.Get()
+				require.NoError(t, err)
+				assert.Equal(t, "opsadmin", string(val))
+				found = true
+				break
+			}
+		}
+		assert.True(t, found, "expected to find version %q in response", versionStr)
+	})
+}
+
 func TestGetEulaCompliance(t *testing.T) {
 	// This test requires real Cognito access - opt-in via env var
 	if os.Getenv("ENABLE_COGNITO_INTEGRATION_TESTS") != "true" {