From d360c7a32106570b47c2d84e5928485410718ee3 Mon Sep 17 00:00:00 2001 From: Feny Mehta Date: Thu, 4 Jun 2026 14:53:59 +0530 Subject: [PATCH 01/10] feat: add Twilio Lookup v2 API Signed-off-by: Feny Mehta Co-authored-by: Cursor --- .../e2e-tests/toolchainconfig.yaml | 1 + go.mod | 6 +- go.sum | 8 +- test/e2e/parallel/phone_lookup_test.go | 132 ++++++++++++++++++ testsupport/wait/host.go | 42 ++++++ 5 files changed, 184 insertions(+), 5 deletions(-) create mode 100644 test/e2e/parallel/phone_lookup_test.go diff --git a/deploy/host-operator/e2e-tests/toolchainconfig.yaml b/deploy/host-operator/e2e-tests/toolchainconfig.yaml index 127b39888..0cb3f8cee 100644 --- a/deploy/host-operator/e2e-tests/toolchainconfig.yaml +++ b/deploy/host-operator/e2e-tests/toolchainconfig.yaml @@ -20,6 +20,7 @@ spec: workatoWebHookURL: https://webhooks.testwebhook verification: enabled: true + phoneLookupMode: 'disabled' excludedEmailDomains: 'redhat.com,acme.com' secret: ref: 'host-operator-secret' diff --git a/go.mod b/go.mod index 282c5d1ec..e6601b7d6 100644 --- a/go.mod +++ b/go.mod @@ -1,7 +1,7 @@ module github.com/codeready-toolchain/toolchain-e2e require ( - github.com/codeready-toolchain/api v0.0.0-20260305144020-4ff0e6b6e174 + github.com/codeready-toolchain/api v0.0.0-20260603082246-cfa3dd9db9cc github.com/codeready-toolchain/toolchain-common v0.0.0-20260305144813-52d9242e8c74 github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc github.com/fatih/color v1.18.0 @@ -142,6 +142,10 @@ require ( sigs.k8s.io/yaml v1.6.0 // indirect ) +replace github.com/codeready-toolchain/api => github.com/fbm3307/toolchainapi v0.0.0-20260604080936-f3460ab84acc + +replace github.com/codeready-toolchain/toolchain-common => github.com/fbm3307/toolchain-common v0.0.0-20260604083249-f7d10b0cf7f8 + go 1.24.4 toolchain go1.24.13 diff --git a/go.sum b/go.sum index cade9004e..9d78a0882 100644 --- a/go.sum +++ b/go.sum @@ -26,10 +26,6 @@ github.com/chzyer/test v0.0.0-20180213035817-a1ea475d72b1/go.mod h1:Q3SI9o4m/ZMn github.com/cloudflare/circl v1.1.0/go.mod h1:prBCrKB9DV4poKZY1l9zBXg2QJY7mvgRvtMxxK7fi4I= github.com/cloudflare/circl v1.6.3 h1:9GPOhQGF9MCYUeXyMYlqTR6a5gTrgR/fBLXvUgtVcg8= github.com/cloudflare/circl v1.6.3/go.mod h1:2eXP6Qfat4O/Yhh8BznvKnJ+uzEoTQ6jVKJRn81BiS4= -github.com/codeready-toolchain/api v0.0.0-20260305144020-4ff0e6b6e174 h1:hed3ZyardxswS6yMB0ME9l3vEkO+pFouyk4dvIiAQOo= -github.com/codeready-toolchain/api v0.0.0-20260305144020-4ff0e6b6e174/go.mod h1:PMg6kNHuCGNlu3MOdrCisqGkBpvzB0qS1+E6nrXxPAc= -github.com/codeready-toolchain/toolchain-common v0.0.0-20260305144813-52d9242e8c74 h1:yxKX0m6Kk1AIWwaGpf25flTfTrYrEJ9TyLW5NEQSq0Y= -github.com/codeready-toolchain/toolchain-common v0.0.0-20260305144813-52d9242e8c74/go.mod h1:NEnnq2R5GYoWdN0b0iaSdX7L1ndrzxl3ML0m7XLsSqk= github.com/cpuguy83/go-md2man/v2 v2.0.6/go.mod h1:oOW0eioCTA6cOiMLiUPZOpcVxMig6NIQQ7OS05n1F4g= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= @@ -50,6 +46,10 @@ github.com/exponent-io/jsonpath v0.0.0-20210407135951-1de76d718b3f h1:Wl78ApPPB2 github.com/exponent-io/jsonpath v0.0.0-20210407135951-1de76d718b3f/go.mod h1:OSYXu++VVOHnXeitef/D8n/6y4QV8uLHSFXX4NeXMGc= github.com/fatih/color v1.18.0 h1:S8gINlzdQ840/4pfAwic/ZE0djQEH3wM94VfqLTZcOM= github.com/fatih/color v1.18.0/go.mod h1:4FelSpRwEGDpQ12mAdzqdOukCy4u8WUtOY6lkT/6HfU= +github.com/fbm3307/toolchain-common v0.0.0-20260604083249-f7d10b0cf7f8 h1:sObsn7DQTpOV3xFUjz3CukBbV0yBbocoy7oY44lmohI= +github.com/fbm3307/toolchain-common v0.0.0-20260604083249-f7d10b0cf7f8/go.mod h1:pcEdk1XDpioLNGX3/p10q6TG+JiM1fpNGksjUGN4j+8= +github.com/fbm3307/toolchainapi v0.0.0-20260604080936-f3460ab84acc h1:uqVQWx9dBBUQ63BuyinZX8UYPDPiPzUmOrinw+Luw/M= +github.com/fbm3307/toolchainapi v0.0.0-20260604080936-f3460ab84acc/go.mod h1:PMg6kNHuCGNlu3MOdrCisqGkBpvzB0qS1+E6nrXxPAc= github.com/frankban/quicktest v1.14.6 h1:7Xjx+VpznH+oBnejlPUj8oUpdxnVs4f8XU8WnHkI4W8= github.com/frankban/quicktest v1.14.6/go.mod h1:4ptaffx2x8+WTWXmUCuVU6aPUX1/Mz7zb5vbUoiM6w0= github.com/fsnotify/fsnotify v1.8.0 h1:dAwr6QBTBZIkG8roQaJjGof0pp0EeF+tNV7YBP3F/8M= diff --git a/test/e2e/parallel/phone_lookup_test.go b/test/e2e/parallel/phone_lookup_test.go new file mode 100644 index 000000000..4dc6631e6 --- /dev/null +++ b/test/e2e/parallel/phone_lookup_test.go @@ -0,0 +1,132 @@ +package parallel + +import ( + "fmt" + "net/http" + "strings" + "testing" + + toolchainv1alpha1 "github.com/codeready-toolchain/api/api/v1alpha1" + testconfig "github.com/codeready-toolchain/toolchain-common/pkg/test/config" + . "github.com/codeready-toolchain/toolchain-e2e/testsupport" + "github.com/codeready-toolchain/toolchain-e2e/testsupport/wait" + "github.com/gofrs/uuid" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestPhoneLookupMode(t *testing.T) { + await := WaitForDeployments(t) + hostAwait := await.Host() + route := hostAwait.RegistrationServiceURL + + t.Run("phoneLookupMode can be set on ToolchainConfig", func(t *testing.T) { + hostAwait.UpdateToolchainConfig(t, testconfig.RegistrationService().Verification().PhoneLookupMode("log")) + VerifyToolchainConfig(t, hostAwait, wait.UntilToolchainConfigHasPhoneLookupMode("log")) + + config := hostAwait.GetToolchainConfig(t) + require.NotNil(t, config.Spec.Host.RegistrationService.Verification.PhoneLookupMode) + assert.Equal(t, "log", *config.Spec.Host.RegistrationService.Verification.PhoneLookupMode) + }) + + t.Run("disabled mode skips phone lookup annotations", func(t *testing.T) { + hostAwait.UpdateToolchainConfig(t, testconfig.RegistrationService().Verification().PhoneLookupMode("disabled")) + + userSignup, token := signup(t, hostAwait) + phoneNumber := uniqueUKPhoneNumber() + + NewHTTPRequest(t). + InvokeEndpoint("PUT", route+"/api/v1/signup/verification", token, + fmt.Sprintf(`{ "country_code":"+44", "phone_number":"%s" }`, phoneNumber), http.StatusNoContent) + + userSignup, err := hostAwait.WaitForUserSignup(t, userSignup.Name, + wait.UntilUserSignupHasAnnotationNotEmpty(toolchainv1alpha1.UserSignupVerificationCodeAnnotationKey)) + require.NoError(t, err) + + assert.Empty(t, userSignup.Annotations[toolchainv1alpha1.UserSignupPhoneLookupResultAnnotationKey]) + assert.Empty(t, userSignup.Annotations[toolchainv1alpha1.UserSignupPhoneLookupPhoneHashAnnotationKey]) + }) + + t.Run("log mode stores phone lookup annotations when lookup succeeds", func(t *testing.T) { + hostAwait.UpdateToolchainConfig(t, testconfig.RegistrationService().Verification().PhoneLookupMode("log")) + + userSignup, token := signup(t, hostAwait) + phoneNumber := uniqueUKPhoneNumber() + + NewHTTPRequest(t). + InvokeEndpoint("PUT", route+"/api/v1/signup/verification", token, + fmt.Sprintf(`{ "country_code":"+44", "phone_number":"%s" }`, phoneNumber), http.StatusNoContent) + + userSignup, err := hostAwait.WaitForUserSignup(t, userSignup.Name, + wait.UntilUserSignupHasAnnotationNotEmpty(toolchainv1alpha1.UserSignupVerificationCodeAnnotationKey)) + require.NoError(t, err) + + lookupResult := userSignup.Annotations[toolchainv1alpha1.UserSignupPhoneLookupResultAnnotationKey] + if lookupResult == "" { + t.Log("Twilio Lookup did not return a result (fail-open); skipping lookup annotation assertions") + return + } + + assert.Contains(t, []string{"allowed", "rejected"}, lookupResult) + assert.NotEmpty(t, userSignup.Annotations[toolchainv1alpha1.UserSignupPhoneLookupPhoneHashAnnotationKey]) + assert.NotEmpty(t, userSignup.Annotations[toolchainv1alpha1.UserSignupPhoneLookupCarrierRiskAnnotationKey]) + assert.NotEmpty(t, userSignup.Annotations[toolchainv1alpha1.UserSignupPhoneLookupNumberBlockedAnnotationKey]) + assert.NotEmpty(t, userSignup.Annotations[toolchainv1alpha1.UserSignupPhoneLookupRiskScoreAnnotationKey]) + // verification must proceed in log mode even when lookup result is rejected + assert.NotEmpty(t, userSignup.Annotations[toolchainv1alpha1.UserSignupVerificationCodeAnnotationKey]) + }) + + t.Run("enabled mode blocks verification for previously rejected signup", func(t *testing.T) { + hostAwait.UpdateToolchainConfig(t, testconfig.RegistrationService().Verification().PhoneLookupMode("enabled")) + + userSignup, token := signup(t, hostAwait) + phoneNumber := uniqueUKPhoneNumber() + + _, err := wait.For(t, hostAwait.Awaitility, &toolchainv1alpha1.UserSignup{}). + Update(userSignup.Name, hostAwait.Namespace, + func(us *toolchainv1alpha1.UserSignup) { + if us.Annotations == nil { + us.Annotations = map[string]string{} + } + us.Annotations[toolchainv1alpha1.UserSignupPhoneLookupResultAnnotationKey] = "rejected" + }) + require.NoError(t, err) + + responseMap := NewHTTPRequest(t). + InvokeEndpoint("PUT", route+"/api/v1/signup/verification", token, + fmt.Sprintf(`{ "country_code":"+44", "phone_number":"%s" }`, phoneNumber), http.StatusForbidden). + UnmarshalMap() + + require.NotEmpty(t, responseMap) + assert.Equal(t, "Forbidden", responseMap["status"]) + + userSignup, err = hostAwait.WaitForUserSignup(t, userSignup.Name) + require.NoError(t, err) + assert.Equal(t, "rejected", userSignup.Annotations[toolchainv1alpha1.UserSignupPhoneLookupResultAnnotationKey]) + assert.Empty(t, userSignup.Annotations[toolchainv1alpha1.UserSignupVerificationCodeAnnotationKey]) + }) + + t.Run("enabled mode skips lookup for US numbers", func(t *testing.T) { + hostAwait.UpdateToolchainConfig(t, testconfig.RegistrationService().Verification().PhoneLookupMode("enabled")) + + userSignup, token := signup(t, hostAwait) + phoneNumber := strings.ReplaceAll(uuid.Must(uuid.NewV4()).String(), "-", "")[:10] + + NewHTTPRequest(t). + InvokeEndpoint("PUT", route+"/api/v1/signup/verification", token, + fmt.Sprintf(`{ "country_code":"+1", "phone_number":"%s" }`, phoneNumber), http.StatusNoContent) + + userSignup, err := hostAwait.WaitForUserSignup(t, userSignup.Name, + wait.UntilUserSignupHasAnnotationNotEmpty(toolchainv1alpha1.UserSignupVerificationCodeAnnotationKey)) + require.NoError(t, err) + + assert.Empty(t, userSignup.Annotations[toolchainv1alpha1.UserSignupPhoneLookupResultAnnotationKey]) + assert.NotEmpty(t, userSignup.Annotations[toolchainv1alpha1.UserSignupVerificationCodeAnnotationKey]) + }) +} + +func uniqueUKPhoneNumber() string { + // UK mobile numbers are 10 digits after country code; use a unique suffix to avoid phone-in-use conflicts + suffix := strings.ReplaceAll(uuid.Must(uuid.NewV4()).String(), "-", "")[:10] + return "77009" + suffix[len(suffix)-5:] +} diff --git a/testsupport/wait/host.go b/testsupport/wait/host.go index b3bece872..569f0c682 100644 --- a/testsupport/wait/host.go +++ b/testsupport/wait/host.go @@ -12,6 +12,7 @@ import ( "time" toolchainv1alpha1 "github.com/codeready-toolchain/api/api/v1alpha1" + "github.com/codeready-toolchain/toolchain-common/pkg/configuration" "github.com/codeready-toolchain/toolchain-common/pkg/condition" "github.com/codeready-toolchain/toolchain-common/pkg/spacebinding" "github.com/codeready-toolchain/toolchain-common/pkg/test" @@ -484,6 +485,34 @@ func ContainsCondition(expected toolchainv1alpha1.Condition) UserSignupWaitCrite } } +// UntilUserSignupHasAnnotation returns a `UserSignupWaitCriterion` which checks that the given +// UserSignup has an annotation with the expected key and value +func UntilUserSignupHasAnnotation(key, value string) UserSignupWaitCriterion { + return UserSignupWaitCriterion{ + Match: func(actual *toolchainv1alpha1.UserSignup) bool { + actualValue, exist := actual.Annotations[key] + return exist && actualValue == value + }, + Diff: func(actual *toolchainv1alpha1.UserSignup) string { + return fmt.Sprintf("expected UserSignup annotation '%s' to be '%s'\nbut it was '%s'", key, value, actual.Annotations[key]) + }, + } +} + +// UntilUserSignupHasAnnotationNotEmpty returns a `UserSignupWaitCriterion` which checks that the given +// UserSignup has a non-empty annotation for the expected key +func UntilUserSignupHasAnnotationNotEmpty(key string) UserSignupWaitCriterion { + return UserSignupWaitCriterion{ + Match: func(actual *toolchainv1alpha1.UserSignup) bool { + actualValue, exist := actual.Annotations[key] + return exist && actualValue != "" + }, + Diff: func(actual *toolchainv1alpha1.UserSignup) string { + return fmt.Sprintf("expected UserSignup annotation '%s' to be non-empty\nbut it was '%s'", key, actual.Annotations[key]) + }, + } +} + // UntilUserSignupHasLabel returns a `UserSignupWaitCriterion` which checks that the given // UserSignup has a `key` equal to the given `value` func UntilUserSignupHasLabel(key, value string) UserSignupWaitCriterion { @@ -1697,6 +1726,19 @@ func UntilToolchainConfigHasVerificationEnabled(expected bool) ToolchainConfigWa } } +func UntilToolchainConfigHasPhoneLookupMode(expected string) ToolchainConfigWaitCriterion { + return ToolchainConfigWaitCriterion{ + Match: func(actual *toolchainv1alpha1.ToolchainConfig) bool { + mode := configuration.GetString(actual.Spec.Host.RegistrationService.Verification.PhoneLookupMode, "log") + return mode == expected + }, + Diff: func(actual *toolchainv1alpha1.ToolchainConfig) string { + mode := configuration.GetString(actual.Spec.Host.RegistrationService.Verification.PhoneLookupMode, "log") + return fmt.Sprintf("expected phoneLookupMode to be '%s'.\n\tactual: '%s'", expected, mode) + }, + } +} + // WaitForToolchainConfig waits until the ToolchainConfig is available with the provided criteria, if any func (a *HostAwaitility) WaitForToolchainConfig(t *testing.T, criteria ...ToolchainConfigWaitCriterion) (*toolchainv1alpha1.ToolchainConfig, error) { // there should only be one ToolchainConfig with the name "config" From 65981b7b3b2a41b890adbef112fe83cc2921142d Mon Sep 17 00:00:00 2001 From: Feny Mehta Date: Thu, 4 Jun 2026 15:13:58 +0530 Subject: [PATCH 02/10] fix: golangci-lint gofmt error Signed-off-by: Feny Mehta Co-authored-by: Cursor --- testsupport/wait/host.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/testsupport/wait/host.go b/testsupport/wait/host.go index 569f0c682..9f6b19dbd 100644 --- a/testsupport/wait/host.go +++ b/testsupport/wait/host.go @@ -12,8 +12,8 @@ import ( "time" toolchainv1alpha1 "github.com/codeready-toolchain/api/api/v1alpha1" - "github.com/codeready-toolchain/toolchain-common/pkg/configuration" "github.com/codeready-toolchain/toolchain-common/pkg/condition" + "github.com/codeready-toolchain/toolchain-common/pkg/configuration" "github.com/codeready-toolchain/toolchain-common/pkg/spacebinding" "github.com/codeready-toolchain/toolchain-common/pkg/test" testconfig "github.com/codeready-toolchain/toolchain-common/pkg/test/config" From cc76fd42c046d378a8e35fce25cf7c26406b8398 Mon Sep 17 00:00:00 2001 From: Feny Mehta Date: Thu, 4 Jun 2026 15:24:01 +0530 Subject: [PATCH 03/10] fix: use typed PhoneLookupMode enum in e2e tests Signed-off-by: Feny Mehta Co-authored-by: Cursor --- go.mod | 4 ++-- go.sum | 8 ++++---- test/e2e/parallel/phone_lookup_test.go | 14 +++++++------- testsupport/wait/host.go | 13 +++++++++---- 4 files changed, 22 insertions(+), 17 deletions(-) diff --git a/go.mod b/go.mod index e6601b7d6..93b393288 100644 --- a/go.mod +++ b/go.mod @@ -142,9 +142,9 @@ require ( sigs.k8s.io/yaml v1.6.0 // indirect ) -replace github.com/codeready-toolchain/api => github.com/fbm3307/toolchainapi v0.0.0-20260604080936-f3460ab84acc +replace github.com/codeready-toolchain/api => github.com/fbm3307/toolchainapi v0.0.0-20260604095004-7254759589cb -replace github.com/codeready-toolchain/toolchain-common => github.com/fbm3307/toolchain-common v0.0.0-20260604083249-f7d10b0cf7f8 +replace github.com/codeready-toolchain/toolchain-common => github.com/fbm3307/toolchain-common v0.0.0-20260604095108-e1b8464eda32 go 1.24.4 diff --git a/go.sum b/go.sum index 9d78a0882..cb5bfab1d 100644 --- a/go.sum +++ b/go.sum @@ -46,10 +46,10 @@ github.com/exponent-io/jsonpath v0.0.0-20210407135951-1de76d718b3f h1:Wl78ApPPB2 github.com/exponent-io/jsonpath v0.0.0-20210407135951-1de76d718b3f/go.mod h1:OSYXu++VVOHnXeitef/D8n/6y4QV8uLHSFXX4NeXMGc= github.com/fatih/color v1.18.0 h1:S8gINlzdQ840/4pfAwic/ZE0djQEH3wM94VfqLTZcOM= github.com/fatih/color v1.18.0/go.mod h1:4FelSpRwEGDpQ12mAdzqdOukCy4u8WUtOY6lkT/6HfU= -github.com/fbm3307/toolchain-common v0.0.0-20260604083249-f7d10b0cf7f8 h1:sObsn7DQTpOV3xFUjz3CukBbV0yBbocoy7oY44lmohI= -github.com/fbm3307/toolchain-common v0.0.0-20260604083249-f7d10b0cf7f8/go.mod h1:pcEdk1XDpioLNGX3/p10q6TG+JiM1fpNGksjUGN4j+8= -github.com/fbm3307/toolchainapi v0.0.0-20260604080936-f3460ab84acc h1:uqVQWx9dBBUQ63BuyinZX8UYPDPiPzUmOrinw+Luw/M= -github.com/fbm3307/toolchainapi v0.0.0-20260604080936-f3460ab84acc/go.mod h1:PMg6kNHuCGNlu3MOdrCisqGkBpvzB0qS1+E6nrXxPAc= +github.com/fbm3307/toolchain-common v0.0.0-20260604095108-e1b8464eda32 h1:0hDjq6yEF3IfohFQayMKvBLHCRXfDrrfuqmA3qdkL/s= +github.com/fbm3307/toolchain-common v0.0.0-20260604095108-e1b8464eda32/go.mod h1:oTa3Nrm18nLZoY288yT0iNK9BWYtyZiEC6kpBbQuSk0= +github.com/fbm3307/toolchainapi v0.0.0-20260604095004-7254759589cb h1:bXPET+ypBRQzmSUdUvL963GLYFNc4eZmh7a5LyxbLBc= +github.com/fbm3307/toolchainapi v0.0.0-20260604095004-7254759589cb/go.mod h1:PMg6kNHuCGNlu3MOdrCisqGkBpvzB0qS1+E6nrXxPAc= github.com/frankban/quicktest v1.14.6 h1:7Xjx+VpznH+oBnejlPUj8oUpdxnVs4f8XU8WnHkI4W8= github.com/frankban/quicktest v1.14.6/go.mod h1:4ptaffx2x8+WTWXmUCuVU6aPUX1/Mz7zb5vbUoiM6w0= github.com/fsnotify/fsnotify v1.8.0 h1:dAwr6QBTBZIkG8roQaJjGof0pp0EeF+tNV7YBP3F/8M= diff --git a/test/e2e/parallel/phone_lookup_test.go b/test/e2e/parallel/phone_lookup_test.go index 4dc6631e6..d6f53ed40 100644 --- a/test/e2e/parallel/phone_lookup_test.go +++ b/test/e2e/parallel/phone_lookup_test.go @@ -21,16 +21,16 @@ func TestPhoneLookupMode(t *testing.T) { route := hostAwait.RegistrationServiceURL t.Run("phoneLookupMode can be set on ToolchainConfig", func(t *testing.T) { - hostAwait.UpdateToolchainConfig(t, testconfig.RegistrationService().Verification().PhoneLookupMode("log")) - VerifyToolchainConfig(t, hostAwait, wait.UntilToolchainConfigHasPhoneLookupMode("log")) + hostAwait.UpdateToolchainConfig(t, testconfig.RegistrationService().Verification().PhoneLookupMode(toolchainv1alpha1.PhoneLookupModeLog)) + VerifyToolchainConfig(t, hostAwait, wait.UntilToolchainConfigHasPhoneLookupMode(toolchainv1alpha1.PhoneLookupModeLog)) config := hostAwait.GetToolchainConfig(t) require.NotNil(t, config.Spec.Host.RegistrationService.Verification.PhoneLookupMode) - assert.Equal(t, "log", *config.Spec.Host.RegistrationService.Verification.PhoneLookupMode) + assert.Equal(t, toolchainv1alpha1.PhoneLookupModeLog, *config.Spec.Host.RegistrationService.Verification.PhoneLookupMode) }) t.Run("disabled mode skips phone lookup annotations", func(t *testing.T) { - hostAwait.UpdateToolchainConfig(t, testconfig.RegistrationService().Verification().PhoneLookupMode("disabled")) + hostAwait.UpdateToolchainConfig(t, testconfig.RegistrationService().Verification().PhoneLookupMode(toolchainv1alpha1.PhoneLookupModeDisabled)) userSignup, token := signup(t, hostAwait) phoneNumber := uniqueUKPhoneNumber() @@ -48,7 +48,7 @@ func TestPhoneLookupMode(t *testing.T) { }) t.Run("log mode stores phone lookup annotations when lookup succeeds", func(t *testing.T) { - hostAwait.UpdateToolchainConfig(t, testconfig.RegistrationService().Verification().PhoneLookupMode("log")) + hostAwait.UpdateToolchainConfig(t, testconfig.RegistrationService().Verification().PhoneLookupMode(toolchainv1alpha1.PhoneLookupModeLog)) userSignup, token := signup(t, hostAwait) phoneNumber := uniqueUKPhoneNumber() @@ -77,7 +77,7 @@ func TestPhoneLookupMode(t *testing.T) { }) t.Run("enabled mode blocks verification for previously rejected signup", func(t *testing.T) { - hostAwait.UpdateToolchainConfig(t, testconfig.RegistrationService().Verification().PhoneLookupMode("enabled")) + hostAwait.UpdateToolchainConfig(t, testconfig.RegistrationService().Verification().PhoneLookupMode(toolchainv1alpha1.PhoneLookupModeEnabled)) userSignup, token := signup(t, hostAwait) phoneNumber := uniqueUKPhoneNumber() @@ -107,7 +107,7 @@ func TestPhoneLookupMode(t *testing.T) { }) t.Run("enabled mode skips lookup for US numbers", func(t *testing.T) { - hostAwait.UpdateToolchainConfig(t, testconfig.RegistrationService().Verification().PhoneLookupMode("enabled")) + hostAwait.UpdateToolchainConfig(t, testconfig.RegistrationService().Verification().PhoneLookupMode(toolchainv1alpha1.PhoneLookupModeEnabled)) userSignup, token := signup(t, hostAwait) phoneNumber := strings.ReplaceAll(uuid.Must(uuid.NewV4()).String(), "-", "")[:10] diff --git a/testsupport/wait/host.go b/testsupport/wait/host.go index 9f6b19dbd..3e64838c6 100644 --- a/testsupport/wait/host.go +++ b/testsupport/wait/host.go @@ -13,7 +13,6 @@ import ( toolchainv1alpha1 "github.com/codeready-toolchain/api/api/v1alpha1" "github.com/codeready-toolchain/toolchain-common/pkg/condition" - "github.com/codeready-toolchain/toolchain-common/pkg/configuration" "github.com/codeready-toolchain/toolchain-common/pkg/spacebinding" "github.com/codeready-toolchain/toolchain-common/pkg/test" testconfig "github.com/codeready-toolchain/toolchain-common/pkg/test/config" @@ -1726,14 +1725,20 @@ func UntilToolchainConfigHasVerificationEnabled(expected bool) ToolchainConfigWa } } -func UntilToolchainConfigHasPhoneLookupMode(expected string) ToolchainConfigWaitCriterion { +func UntilToolchainConfigHasPhoneLookupMode(expected toolchainv1alpha1.PhoneLookupMode) ToolchainConfigWaitCriterion { return ToolchainConfigWaitCriterion{ Match: func(actual *toolchainv1alpha1.ToolchainConfig) bool { - mode := configuration.GetString(actual.Spec.Host.RegistrationService.Verification.PhoneLookupMode, "log") + mode := toolchainv1alpha1.PhoneLookupModeLog + if actual.Spec.Host.RegistrationService.Verification.PhoneLookupMode != nil { + mode = *actual.Spec.Host.RegistrationService.Verification.PhoneLookupMode + } return mode == expected }, Diff: func(actual *toolchainv1alpha1.ToolchainConfig) string { - mode := configuration.GetString(actual.Spec.Host.RegistrationService.Verification.PhoneLookupMode, "log") + mode := toolchainv1alpha1.PhoneLookupModeLog + if actual.Spec.Host.RegistrationService.Verification.PhoneLookupMode != nil { + mode = *actual.Spec.Host.RegistrationService.Verification.PhoneLookupMode + } return fmt.Sprintf("expected phoneLookupMode to be '%s'.\n\tactual: '%s'", expected, mode) }, } From 6a7cfd8c918e622c90f13d2b1a4665e38cd9f86e Mon Sep 17 00:00:00 2001 From: Feny Mehta Date: Thu, 4 Jun 2026 17:46:15 +0530 Subject: [PATCH 04/10] fix: use non-default PhoneLookupMode in config e2e test Signed-off-by: Feny Mehta Co-authored-by: Cursor --- test/e2e/parallel/phone_lookup_test.go | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/test/e2e/parallel/phone_lookup_test.go b/test/e2e/parallel/phone_lookup_test.go index d6f53ed40..667804395 100644 --- a/test/e2e/parallel/phone_lookup_test.go +++ b/test/e2e/parallel/phone_lookup_test.go @@ -21,12 +21,9 @@ func TestPhoneLookupMode(t *testing.T) { route := hostAwait.RegistrationServiceURL t.Run("phoneLookupMode can be set on ToolchainConfig", func(t *testing.T) { - hostAwait.UpdateToolchainConfig(t, testconfig.RegistrationService().Verification().PhoneLookupMode(toolchainv1alpha1.PhoneLookupModeLog)) - VerifyToolchainConfig(t, hostAwait, wait.UntilToolchainConfigHasPhoneLookupMode(toolchainv1alpha1.PhoneLookupModeLog)) - - config := hostAwait.GetToolchainConfig(t) - require.NotNil(t, config.Spec.Host.RegistrationService.Verification.PhoneLookupMode) - assert.Equal(t, toolchainv1alpha1.PhoneLookupModeLog, *config.Spec.Host.RegistrationService.Verification.PhoneLookupMode) + // Non-default value: CRD default "log" is omitted in spec when set explicitly, so use "enabled" to verify persistence. + hostAwait.UpdateToolchainConfig(t, testconfig.RegistrationService().Verification().PhoneLookupMode(toolchainv1alpha1.PhoneLookupModeEnabled)) + VerifyToolchainConfig(t, hostAwait, wait.UntilToolchainConfigHasPhoneLookupMode(toolchainv1alpha1.PhoneLookupModeEnabled)) }) t.Run("disabled mode skips phone lookup annotations", func(t *testing.T) { From 91504da4600d847e9a2dacc00b33230291390e2e Mon Sep 17 00:00:00 2001 From: Feny Mehta Date: Fri, 5 Jun 2026 12:08:43 +0530 Subject: [PATCH 05/10] fix: use Twilio magic numbers for deterministic e2e lookup tests Signed-off-by: Feny Mehta Co-authored-by: Cursor --- test/e2e/parallel/phone_lookup_test.go | 46 ++++++++++++-------------- 1 file changed, 21 insertions(+), 25 deletions(-) diff --git a/test/e2e/parallel/phone_lookup_test.go b/test/e2e/parallel/phone_lookup_test.go index 667804395..0e3acccc3 100644 --- a/test/e2e/parallel/phone_lookup_test.go +++ b/test/e2e/parallel/phone_lookup_test.go @@ -15,6 +15,20 @@ import ( "github.com/stretchr/testify/require" ) +// Twilio test credentials with magic lookup numbers return deterministic SMS Pumping Risk +// responses at no cost. The API takes country_code and phone_number separately. +// See: https://www.twilio.com/docs/lookup/magic-numbers-for-lookup/testing-sms-pumping-risk-with-magic-numbers +// +// Magic numbers used: +// +// +441234567890 → high risk, not blocked, score 2 +// +441234567891 → high risk, blocked, score 34 +// +911234567890 → low risk, not blocked, score 2 +const ( + twilioMagicPhoneHighRisk = "1234567890" // +44 prefix → high risk, not blocked + twilioMagicPhoneHighRiskBlocked = "1234567891" // +44 prefix → high risk, blocked +) + func TestPhoneLookupMode(t *testing.T) { await := WaitForDeployments(t) hostAwait := await.Host() @@ -30,11 +44,9 @@ func TestPhoneLookupMode(t *testing.T) { hostAwait.UpdateToolchainConfig(t, testconfig.RegistrationService().Verification().PhoneLookupMode(toolchainv1alpha1.PhoneLookupModeDisabled)) userSignup, token := signup(t, hostAwait) - phoneNumber := uniqueUKPhoneNumber() - NewHTTPRequest(t). InvokeEndpoint("PUT", route+"/api/v1/signup/verification", token, - fmt.Sprintf(`{ "country_code":"+44", "phone_number":"%s" }`, phoneNumber), http.StatusNoContent) + fmt.Sprintf(`{ "country_code":"+44", "phone_number":"%s" }`, twilioMagicPhoneHighRisk), http.StatusNoContent) userSignup, err := hostAwait.WaitForUserSignup(t, userSignup.Name, wait.UntilUserSignupHasAnnotationNotEmpty(toolchainv1alpha1.UserSignupVerificationCodeAnnotationKey)) @@ -48,27 +60,19 @@ func TestPhoneLookupMode(t *testing.T) { hostAwait.UpdateToolchainConfig(t, testconfig.RegistrationService().Verification().PhoneLookupMode(toolchainv1alpha1.PhoneLookupModeLog)) userSignup, token := signup(t, hostAwait) - phoneNumber := uniqueUKPhoneNumber() - NewHTTPRequest(t). InvokeEndpoint("PUT", route+"/api/v1/signup/verification", token, - fmt.Sprintf(`{ "country_code":"+44", "phone_number":"%s" }`, phoneNumber), http.StatusNoContent) + fmt.Sprintf(`{ "country_code":"+44", "phone_number":"%s" }`, twilioMagicPhoneHighRiskBlocked), http.StatusNoContent) userSignup, err := hostAwait.WaitForUserSignup(t, userSignup.Name, wait.UntilUserSignupHasAnnotationNotEmpty(toolchainv1alpha1.UserSignupVerificationCodeAnnotationKey)) require.NoError(t, err) - lookupResult := userSignup.Annotations[toolchainv1alpha1.UserSignupPhoneLookupResultAnnotationKey] - if lookupResult == "" { - t.Log("Twilio Lookup did not return a result (fail-open); skipping lookup annotation assertions") - return - } - - assert.Contains(t, []string{"allowed", "rejected"}, lookupResult) + assert.Equal(t, "rejected", userSignup.Annotations[toolchainv1alpha1.UserSignupPhoneLookupResultAnnotationKey]) assert.NotEmpty(t, userSignup.Annotations[toolchainv1alpha1.UserSignupPhoneLookupPhoneHashAnnotationKey]) - assert.NotEmpty(t, userSignup.Annotations[toolchainv1alpha1.UserSignupPhoneLookupCarrierRiskAnnotationKey]) - assert.NotEmpty(t, userSignup.Annotations[toolchainv1alpha1.UserSignupPhoneLookupNumberBlockedAnnotationKey]) - assert.NotEmpty(t, userSignup.Annotations[toolchainv1alpha1.UserSignupPhoneLookupRiskScoreAnnotationKey]) + assert.Equal(t, "high", userSignup.Annotations[toolchainv1alpha1.UserSignupPhoneLookupCarrierRiskAnnotationKey]) + assert.Equal(t, "true", userSignup.Annotations[toolchainv1alpha1.UserSignupPhoneLookupNumberBlockedAnnotationKey]) + assert.Equal(t, "34", userSignup.Annotations[toolchainv1alpha1.UserSignupPhoneLookupRiskScoreAnnotationKey]) // verification must proceed in log mode even when lookup result is rejected assert.NotEmpty(t, userSignup.Annotations[toolchainv1alpha1.UserSignupVerificationCodeAnnotationKey]) }) @@ -77,8 +81,6 @@ func TestPhoneLookupMode(t *testing.T) { hostAwait.UpdateToolchainConfig(t, testconfig.RegistrationService().Verification().PhoneLookupMode(toolchainv1alpha1.PhoneLookupModeEnabled)) userSignup, token := signup(t, hostAwait) - phoneNumber := uniqueUKPhoneNumber() - _, err := wait.For(t, hostAwait.Awaitility, &toolchainv1alpha1.UserSignup{}). Update(userSignup.Name, hostAwait.Namespace, func(us *toolchainv1alpha1.UserSignup) { @@ -91,7 +93,7 @@ func TestPhoneLookupMode(t *testing.T) { responseMap := NewHTTPRequest(t). InvokeEndpoint("PUT", route+"/api/v1/signup/verification", token, - fmt.Sprintf(`{ "country_code":"+44", "phone_number":"%s" }`, phoneNumber), http.StatusForbidden). + fmt.Sprintf(`{ "country_code":"+91", "phone_number":"%s" }`, twilioMagicPhoneHighRisk), http.StatusForbidden). UnmarshalMap() require.NotEmpty(t, responseMap) @@ -121,9 +123,3 @@ func TestPhoneLookupMode(t *testing.T) { assert.NotEmpty(t, userSignup.Annotations[toolchainv1alpha1.UserSignupVerificationCodeAnnotationKey]) }) } - -func uniqueUKPhoneNumber() string { - // UK mobile numbers are 10 digits after country code; use a unique suffix to avoid phone-in-use conflicts - suffix := strings.ReplaceAll(uuid.Must(uuid.NewV4()).String(), "-", "")[:10] - return "77009" + suffix[len(suffix)-5:] -} From 45398b5bc9fa1ffc5e4f996aa6104916a34d053a Mon Sep 17 00:00:00 2001 From: Feny Mehta Date: Mon, 8 Jun 2026 16:48:30 +0530 Subject: [PATCH 06/10] fix: remove phone_hash from details JSON, bump api+common replaces Co-authored-by: Cursor Signed-off-by: Feny Mehta --- go.mod | 4 ++-- go.sum | 8 ++++---- test/e2e/parallel/phone_lookup_test.go | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/go.mod b/go.mod index bdd3e8594..923be8fdc 100644 --- a/go.mod +++ b/go.mod @@ -142,9 +142,9 @@ require ( sigs.k8s.io/yaml v1.6.0 // indirect ) -replace github.com/codeready-toolchain/api => github.com/fbm3307/toolchainapi v0.0.0-20260608073802-89362d94dca4 +replace github.com/codeready-toolchain/api => github.com/fbm3307/toolchainapi v0.0.0-20260608110443-4e4fa9fe0621 -replace github.com/codeready-toolchain/toolchain-common => github.com/fbm3307/toolchain-common v0.0.0-20260608074508-590711573ffe +replace github.com/codeready-toolchain/toolchain-common => github.com/fbm3307/toolchain-common v0.0.0-20260608111724-c30cc73c6b91 go 1.24.4 diff --git a/go.sum b/go.sum index 02f40d4ba..32503f415 100644 --- a/go.sum +++ b/go.sum @@ -46,10 +46,10 @@ github.com/exponent-io/jsonpath v0.0.0-20210407135951-1de76d718b3f h1:Wl78ApPPB2 github.com/exponent-io/jsonpath v0.0.0-20210407135951-1de76d718b3f/go.mod h1:OSYXu++VVOHnXeitef/D8n/6y4QV8uLHSFXX4NeXMGc= github.com/fatih/color v1.18.0 h1:S8gINlzdQ840/4pfAwic/ZE0djQEH3wM94VfqLTZcOM= github.com/fatih/color v1.18.0/go.mod h1:4FelSpRwEGDpQ12mAdzqdOukCy4u8WUtOY6lkT/6HfU= -github.com/fbm3307/toolchain-common v0.0.0-20260608074508-590711573ffe h1:ZcRGUQ4PQ5jdklG+sjbNiRcWDWZRJTnAw7sMzhUFMcE= -github.com/fbm3307/toolchain-common v0.0.0-20260608074508-590711573ffe/go.mod h1:qM3O9UX4HOWPYW4QT7G3cZxO91+XKQBUZaxlOIN73Uo= -github.com/fbm3307/toolchainapi v0.0.0-20260608073802-89362d94dca4 h1:iMRDRRzSq9Ie33g3WqfzDvvnLzesgTRKEnI3AhTTIfs= -github.com/fbm3307/toolchainapi v0.0.0-20260608073802-89362d94dca4/go.mod h1:PMg6kNHuCGNlu3MOdrCisqGkBpvzB0qS1+E6nrXxPAc= +github.com/fbm3307/toolchain-common v0.0.0-20260608111724-c30cc73c6b91 h1:ZRjnFxN8UIcmJo+BjNTiaCpyYVCj7TclarGbRRko3mE= +github.com/fbm3307/toolchain-common v0.0.0-20260608111724-c30cc73c6b91/go.mod h1:xcm86+meY59A4Rs62c8wVAznTuPk2QQW/yijPWZxHLw= +github.com/fbm3307/toolchainapi v0.0.0-20260608110443-4e4fa9fe0621 h1:4Ga0w0uuWUw8Q3KgJsgMncH+1F2S048y0eCAzSSlYbA= +github.com/fbm3307/toolchainapi v0.0.0-20260608110443-4e4fa9fe0621/go.mod h1:PMg6kNHuCGNlu3MOdrCisqGkBpvzB0qS1+E6nrXxPAc= github.com/frankban/quicktest v1.14.6 h1:7Xjx+VpznH+oBnejlPUj8oUpdxnVs4f8XU8WnHkI4W8= github.com/frankban/quicktest v1.14.6/go.mod h1:4ptaffx2x8+WTWXmUCuVU6aPUX1/Mz7zb5vbUoiM6w0= github.com/fsnotify/fsnotify v1.8.0 h1:dAwr6QBTBZIkG8roQaJjGof0pp0EeF+tNV7YBP3F/8M= diff --git a/test/e2e/parallel/phone_lookup_test.go b/test/e2e/parallel/phone_lookup_test.go index 32b7bf18b..8ade23914 100644 --- a/test/e2e/parallel/phone_lookup_test.go +++ b/test/e2e/parallel/phone_lookup_test.go @@ -95,7 +95,7 @@ func TestPhoneLookupMode(t *testing.T) { if us.Annotations == nil { us.Annotations = map[string]string{} } - us.Annotations[toolchainv1alpha1.UserSignupPhoneLookupDetailsAnnotationKey] = `{"result":"rejected","phone_hash":"abc123"}` + us.Annotations[toolchainv1alpha1.UserSignupPhoneLookupDetailsAnnotationKey] = `{"result":"rejected"}` }) require.NoError(t, err) From fb34892045054629af045388a2513d27e2c4b524 Mon Sep 17 00:00:00 2001 From: Feny Mehta Date: Tue, 9 Jun 2026 13:14:33 +0530 Subject: [PATCH 07/10] fix: remove fork replace directives, use upstream api and toolchain-common master Signed-off-by: Feny Mehta Co-authored-by: Cursor --- go.mod | 8 ++------ go.sum | 8 ++++---- 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/go.mod b/go.mod index 923be8fdc..365a2a07d 100644 --- a/go.mod +++ b/go.mod @@ -1,8 +1,8 @@ module github.com/codeready-toolchain/toolchain-e2e require ( - github.com/codeready-toolchain/api v0.0.0-20260603082246-cfa3dd9db9cc - github.com/codeready-toolchain/toolchain-common v0.0.0-20260603091009-6db0c02f4506 + github.com/codeready-toolchain/api v0.0.0-20260609071155-c8f486b1a581 + github.com/codeready-toolchain/toolchain-common v0.0.0-20260609073430-82d1748db579 github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc github.com/fatih/color v1.18.0 github.com/ghodss/yaml v1.0.0 @@ -142,10 +142,6 @@ require ( sigs.k8s.io/yaml v1.6.0 // indirect ) -replace github.com/codeready-toolchain/api => github.com/fbm3307/toolchainapi v0.0.0-20260608110443-4e4fa9fe0621 - -replace github.com/codeready-toolchain/toolchain-common => github.com/fbm3307/toolchain-common v0.0.0-20260608111724-c30cc73c6b91 - go 1.24.4 toolchain go1.24.13 diff --git a/go.sum b/go.sum index 32503f415..43354f72d 100644 --- a/go.sum +++ b/go.sum @@ -26,6 +26,10 @@ github.com/chzyer/test v0.0.0-20180213035817-a1ea475d72b1/go.mod h1:Q3SI9o4m/ZMn github.com/cloudflare/circl v1.1.0/go.mod h1:prBCrKB9DV4poKZY1l9zBXg2QJY7mvgRvtMxxK7fi4I= github.com/cloudflare/circl v1.6.3 h1:9GPOhQGF9MCYUeXyMYlqTR6a5gTrgR/fBLXvUgtVcg8= github.com/cloudflare/circl v1.6.3/go.mod h1:2eXP6Qfat4O/Yhh8BznvKnJ+uzEoTQ6jVKJRn81BiS4= +github.com/codeready-toolchain/api v0.0.0-20260609071155-c8f486b1a581 h1:KE14RYWzMatSrwGa2wOB4SoVkbvpTm8hfnUH/nrpnfw= +github.com/codeready-toolchain/api v0.0.0-20260609071155-c8f486b1a581/go.mod h1:PMg6kNHuCGNlu3MOdrCisqGkBpvzB0qS1+E6nrXxPAc= +github.com/codeready-toolchain/toolchain-common v0.0.0-20260609073430-82d1748db579 h1:1qfOdNV6gRQSE0xOmJggqQxAiEjOpV7nZ6Xph75Mb1I= +github.com/codeready-toolchain/toolchain-common v0.0.0-20260609073430-82d1748db579/go.mod h1:aYvTzEtTuw3O+kjWMMkH/1YgV4pgUPC3v3X2Li3ixlM= github.com/cpuguy83/go-md2man/v2 v2.0.6/go.mod h1:oOW0eioCTA6cOiMLiUPZOpcVxMig6NIQQ7OS05n1F4g= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= @@ -46,10 +50,6 @@ github.com/exponent-io/jsonpath v0.0.0-20210407135951-1de76d718b3f h1:Wl78ApPPB2 github.com/exponent-io/jsonpath v0.0.0-20210407135951-1de76d718b3f/go.mod h1:OSYXu++VVOHnXeitef/D8n/6y4QV8uLHSFXX4NeXMGc= github.com/fatih/color v1.18.0 h1:S8gINlzdQ840/4pfAwic/ZE0djQEH3wM94VfqLTZcOM= github.com/fatih/color v1.18.0/go.mod h1:4FelSpRwEGDpQ12mAdzqdOukCy4u8WUtOY6lkT/6HfU= -github.com/fbm3307/toolchain-common v0.0.0-20260608111724-c30cc73c6b91 h1:ZRjnFxN8UIcmJo+BjNTiaCpyYVCj7TclarGbRRko3mE= -github.com/fbm3307/toolchain-common v0.0.0-20260608111724-c30cc73c6b91/go.mod h1:xcm86+meY59A4Rs62c8wVAznTuPk2QQW/yijPWZxHLw= -github.com/fbm3307/toolchainapi v0.0.0-20260608110443-4e4fa9fe0621 h1:4Ga0w0uuWUw8Q3KgJsgMncH+1F2S048y0eCAzSSlYbA= -github.com/fbm3307/toolchainapi v0.0.0-20260608110443-4e4fa9fe0621/go.mod h1:PMg6kNHuCGNlu3MOdrCisqGkBpvzB0qS1+E6nrXxPAc= github.com/frankban/quicktest v1.14.6 h1:7Xjx+VpznH+oBnejlPUj8oUpdxnVs4f8XU8WnHkI4W8= github.com/frankban/quicktest v1.14.6/go.mod h1:4ptaffx2x8+WTWXmUCuVU6aPUX1/Mz7zb5vbUoiM6w0= github.com/fsnotify/fsnotify v1.8.0 h1:dAwr6QBTBZIkG8roQaJjGof0pp0EeF+tNV7YBP3F/8M= From cae70d894c4b5a075050508d9d0fb9ceaf3cf35d Mon Sep 17 00:00:00 2001 From: Feny Mehta Date: Tue, 9 Jun 2026 15:44:42 +0530 Subject: [PATCH 08/10] =?UTF-8?q?fix:=20address=20review=20comments=20?= =?UTF-8?q?=E2=80=94=20use=20states.Rejected,=20fix=20phone=20number=20bug?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Remove config persistence and disabled mode tests (duplicative) - Use states.SetRejected/Rejected instead of JSON annotation - Add // given // when // then comments to all subtests - Verify UserSignup is NOT rejected in log mode - Fix US number test: use magic number instead of UUID hex digits - Remove unused phoneLookupDetailsResult helper Signed-off-by: Feny Mehta Co-authored-by: Cursor --- test/e2e/parallel/phone_lookup_test.go | 65 +++++++------------------- 1 file changed, 17 insertions(+), 48 deletions(-) diff --git a/test/e2e/parallel/phone_lookup_test.go b/test/e2e/parallel/phone_lookup_test.go index 8ade23914..2b8a5a6d1 100644 --- a/test/e2e/parallel/phone_lookup_test.go +++ b/test/e2e/parallel/phone_lookup_test.go @@ -1,17 +1,15 @@ package parallel import ( - "encoding/json" "fmt" "net/http" - "strings" "testing" toolchainv1alpha1 "github.com/codeready-toolchain/api/api/v1alpha1" + "github.com/codeready-toolchain/toolchain-common/pkg/states" testconfig "github.com/codeready-toolchain/toolchain-common/pkg/test/config" . "github.com/codeready-toolchain/toolchain-e2e/testsupport" "github.com/codeready-toolchain/toolchain-e2e/testsupport/wait" - "github.com/gofrs/uuid" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -30,101 +28,72 @@ const ( twilioMagicPhoneHighRiskBlocked = "1234567891" // +44 prefix → high risk, blocked ) -func phoneLookupDetailsResult(t *testing.T, signup *toolchainv1alpha1.UserSignup) string { - t.Helper() - raw := signup.Annotations[toolchainv1alpha1.UserSignupPhoneLookupDetailsAnnotationKey] - if raw == "" { - return "" - } - var details map[string]string - require.NoError(t, json.Unmarshal([]byte(raw), &details)) - return details["result"] -} - func TestPhoneLookupMode(t *testing.T) { await := WaitForDeployments(t) hostAwait := await.Host() route := hostAwait.RegistrationServiceURL - t.Run("phoneLookupMode can be set on ToolchainConfig", func(t *testing.T) { - // Non-default value: CRD default "log" is omitted in spec when set explicitly, so use "enabled" to verify persistence. - hostAwait.UpdateToolchainConfig(t, testconfig.RegistrationService().Verification().PhoneLookupMode(toolchainv1alpha1.PhoneLookupModeEnabled)) - VerifyToolchainConfig(t, hostAwait, wait.UntilToolchainConfigHasPhoneLookupMode(toolchainv1alpha1.PhoneLookupModeEnabled)) - }) - - t.Run("disabled mode skips phone lookup annotations", func(t *testing.T) { - hostAwait.UpdateToolchainConfig(t, testconfig.RegistrationService().Verification().PhoneLookupMode(toolchainv1alpha1.PhoneLookupModeDisabled)) - - userSignup, token := signup(t, hostAwait) - NewHTTPRequest(t). - InvokeEndpoint("PUT", route+"/api/v1/signup/verification", token, - fmt.Sprintf(`{ "country_code":"+44", "phone_number":"%s" }`, twilioMagicPhoneHighRisk), http.StatusNoContent) - - userSignup, err := hostAwait.WaitForUserSignup(t, userSignup.Name, - wait.UntilUserSignupHasAnnotationNotEmpty(toolchainv1alpha1.UserSignupVerificationCodeAnnotationKey)) - require.NoError(t, err) - - assert.Empty(t, userSignup.Annotations[toolchainv1alpha1.UserSignupPhoneLookupDetailsAnnotationKey]) - }) - t.Run("log mode stores phone lookup annotations when lookup succeeds", func(t *testing.T) { + // given hostAwait.UpdateToolchainConfig(t, testconfig.RegistrationService().Verification().PhoneLookupMode(toolchainv1alpha1.PhoneLookupModeLog)) - userSignup, token := signup(t, hostAwait) + + // when NewHTTPRequest(t). InvokeEndpoint("PUT", route+"/api/v1/signup/verification", token, fmt.Sprintf(`{ "country_code":"+44", "phone_number":"%s" }`, twilioMagicPhoneHighRiskBlocked), http.StatusNoContent) + // then userSignup, err := hostAwait.WaitForUserSignup(t, userSignup.Name, wait.UntilUserSignupHasAnnotationNotEmpty(toolchainv1alpha1.UserSignupVerificationCodeAnnotationKey)) require.NoError(t, err) - assert.Equal(t, "rejected", phoneLookupDetailsResult(t, userSignup)) - assert.Equal(t, "high", userSignup.Annotations[toolchainv1alpha1.UserSignupPhoneLookupCarrierRiskAnnotationKey]) - assert.Equal(t, "true", userSignup.Annotations[toolchainv1alpha1.UserSignupPhoneLookupNumberBlockedAnnotationKey]) + assert.NotEmpty(t, userSignup.Annotations[toolchainv1alpha1.UserSignupPhoneLookupDetailsAnnotationKey]) assert.NotEmpty(t, userSignup.Annotations[toolchainv1alpha1.UserSignupVerificationCodeAnnotationKey]) + assert.False(t, states.Rejected(userSignup), "UserSignup should NOT be rejected in log mode") }) t.Run("enabled mode blocks verification for previously rejected signup", func(t *testing.T) { + // given hostAwait.UpdateToolchainConfig(t, testconfig.RegistrationService().Verification().PhoneLookupMode(toolchainv1alpha1.PhoneLookupModeEnabled)) - userSignup, token := signup(t, hostAwait) + _, err := wait.For(t, hostAwait.Awaitility, &toolchainv1alpha1.UserSignup{}). Update(userSignup.Name, hostAwait.Namespace, func(us *toolchainv1alpha1.UserSignup) { - if us.Annotations == nil { - us.Annotations = map[string]string{} - } - us.Annotations[toolchainv1alpha1.UserSignupPhoneLookupDetailsAnnotationKey] = `{"result":"rejected"}` + states.SetRejected(us, true) }) require.NoError(t, err) + // when responseMap := NewHTTPRequest(t). InvokeEndpoint("PUT", route+"/api/v1/signup/verification", token, fmt.Sprintf(`{ "country_code":"+91", "phone_number":"%s" }`, twilioMagicPhoneHighRisk), http.StatusForbidden). UnmarshalMap() + // then require.NotEmpty(t, responseMap) assert.Equal(t, "Forbidden", responseMap["status"]) userSignup, err = hostAwait.WaitForUserSignup(t, userSignup.Name) require.NoError(t, err) - assert.Equal(t, "rejected", phoneLookupDetailsResult(t, userSignup)) + assert.True(t, states.Rejected(userSignup), "UserSignup should remain rejected") assert.Empty(t, userSignup.Annotations[toolchainv1alpha1.UserSignupVerificationCodeAnnotationKey]) }) t.Run("enabled mode skips lookup for US numbers", func(t *testing.T) { + // given hostAwait.UpdateToolchainConfig(t, testconfig.RegistrationService(). Verification().PhoneLookupMode(toolchainv1alpha1.PhoneLookupModeEnabled). Verification().PhoneLookupExcludedCountries([]string{"US", "CA"})) - userSignup, token := signup(t, hostAwait) - phoneNumber := strings.ReplaceAll(uuid.Must(uuid.NewV4()).String(), "-", "")[:10] + // when — use the magic number; US is excluded so lookup is never called NewHTTPRequest(t). InvokeEndpoint("PUT", route+"/api/v1/signup/verification", token, - fmt.Sprintf(`{ "country_code":"+1", "phone_number":"%s" }`, phoneNumber), http.StatusNoContent) + fmt.Sprintf(`{ "country_code":"+1", "phone_number":"%s" }`, twilioMagicPhoneHighRisk), http.StatusNoContent) + // then userSignup, err := hostAwait.WaitForUserSignup(t, userSignup.Name, wait.UntilUserSignupHasAnnotationNotEmpty(toolchainv1alpha1.UserSignupVerificationCodeAnnotationKey)) require.NoError(t, err) From c4da9fe05b5c67d5319ccb1a0812146a8b9275a8 Mon Sep 17 00:00:00 2001 From: Feny Mehta Date: Tue, 9 Jun 2026 19:53:38 +0530 Subject: [PATCH 09/10] fix: use fictional NANPA 555-01XX number for US phone lookup test The previous UUID-derived number had hex chars, then the magic number +11234567890 had invalid area code 123. Use +12025550123 which is in the NANPA 555-0100..0199 range reserved for fictional use, safe with Twilio test credentials. Signed-off-by: Feny Mehta Co-authored-by: Cursor --- test/e2e/parallel/phone_lookup_test.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/test/e2e/parallel/phone_lookup_test.go b/test/e2e/parallel/phone_lookup_test.go index 2b8a5a6d1..b8fda8a15 100644 --- a/test/e2e/parallel/phone_lookup_test.go +++ b/test/e2e/parallel/phone_lookup_test.go @@ -26,6 +26,7 @@ import ( const ( twilioMagicPhoneHighRisk = "1234567890" // +44 prefix → high risk, not blocked twilioMagicPhoneHighRiskBlocked = "1234567891" // +44 prefix → high risk, blocked + usTestPhone = "2025550123" // +1 prefix → fictional NANPA 555-01XX range, safe with test credentials ) func TestPhoneLookupMode(t *testing.T) { @@ -88,10 +89,10 @@ func TestPhoneLookupMode(t *testing.T) { Verification().PhoneLookupExcludedCountries([]string{"US", "CA"})) userSignup, token := signup(t, hostAwait) - // when — use the magic number; US is excluded so lookup is never called + // when — US is excluded so lookup is never called NewHTTPRequest(t). InvokeEndpoint("PUT", route+"/api/v1/signup/verification", token, - fmt.Sprintf(`{ "country_code":"+1", "phone_number":"%s" }`, twilioMagicPhoneHighRisk), http.StatusNoContent) + fmt.Sprintf(`{ "country_code":"+1", "phone_number":"%s" }`, usTestPhone), http.StatusNoContent) // then userSignup, err := hostAwait.WaitForUserSignup(t, userSignup.Name, From 2f8016d87ec87e68e7bc7812d0e6eed9e26c627d Mon Sep 17 00:00:00 2001 From: Feny Mehta Date: Wed, 10 Jun 2026 16:05:28 +0530 Subject: [PATCH 10/10] =?UTF-8?q?fix:=20address=20review=20=E2=80=94=20add?= =?UTF-8?q?=20t.Parallel,=20high-risk=20test,=20verify=20provisioning?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add t.Parallel() for parallel test execution - Add "enabled mode rejects high-risk phone number" test case - Complete verification flow and call VerifyResourcesProvisionedForSignup in log mode and US-skip tests to confirm full provisioning - Verify states.Rejected assertions in all applicable tests Signed-off-by: Feny Mehta Co-authored-by: Cursor --- test/e2e/parallel/phone_lookup_test.go | 61 +++++++++++++++++++++++--- 1 file changed, 56 insertions(+), 5 deletions(-) diff --git a/test/e2e/parallel/phone_lookup_test.go b/test/e2e/parallel/phone_lookup_test.go index b8fda8a15..9e79a8a50 100644 --- a/test/e2e/parallel/phone_lookup_test.go +++ b/test/e2e/parallel/phone_lookup_test.go @@ -30,11 +30,12 @@ const ( ) func TestPhoneLookupMode(t *testing.T) { - await := WaitForDeployments(t) - hostAwait := await.Host() + t.Parallel() + awaitilities := WaitForDeployments(t) + hostAwait := awaitilities.Host() route := hostAwait.RegistrationServiceURL - t.Run("log mode stores phone lookup annotations when lookup succeeds", func(t *testing.T) { + t.Run("log mode stores annotations and user is provisioned", func(t *testing.T) { // given hostAwait.UpdateToolchainConfig(t, testconfig.RegistrationService().Verification().PhoneLookupMode(toolchainv1alpha1.PhoneLookupModeLog)) userSignup, token := signup(t, hostAwait) @@ -44,7 +45,7 @@ func TestPhoneLookupMode(t *testing.T) { InvokeEndpoint("PUT", route+"/api/v1/signup/verification", token, fmt.Sprintf(`{ "country_code":"+44", "phone_number":"%s" }`, twilioMagicPhoneHighRiskBlocked), http.StatusNoContent) - // then + // then — verification code is set and lookup details are recorded userSignup, err := hostAwait.WaitForUserSignup(t, userSignup.Name, wait.UntilUserSignupHasAnnotationNotEmpty(toolchainv1alpha1.UserSignupVerificationCodeAnnotationKey)) require.NoError(t, err) @@ -52,6 +53,42 @@ func TestPhoneLookupMode(t *testing.T) { assert.NotEmpty(t, userSignup.Annotations[toolchainv1alpha1.UserSignupPhoneLookupDetailsAnnotationKey]) assert.NotEmpty(t, userSignup.Annotations[toolchainv1alpha1.UserSignupVerificationCodeAnnotationKey]) assert.False(t, states.Rejected(userSignup), "UserSignup should NOT be rejected in log mode") + + // complete verification and confirm user is provisioned + NewHTTPRequest(t).InvokeEndpoint("GET", route+fmt.Sprintf("/api/v1/signup/verification/%s", + userSignup.Annotations[toolchainv1alpha1.UserSignupVerificationCodeAnnotationKey]), token, "", http.StatusOK) + + userSignup, err = wait.For(t, hostAwait.Awaitility, &toolchainv1alpha1.UserSignup{}). + Update(userSignup.Name, hostAwait.Namespace, + func(instance *toolchainv1alpha1.UserSignup) { + states.SetApprovedManually(instance, true) + }) + require.NoError(t, err) + + VerifyResourcesProvisionedForSignup(t, awaitilities, userSignup) + }) + + t.Run("enabled mode rejects high-risk phone number", func(t *testing.T) { + // given + hostAwait.UpdateToolchainConfig(t, testconfig.RegistrationService(). + Verification().PhoneLookupMode(toolchainv1alpha1.PhoneLookupModeEnabled). + Verification().PhoneLookupExcludedCountries([]string{"US", "CA"})) + userSignup, token := signup(t, hostAwait) + + // when + responseMap := NewHTTPRequest(t). + InvokeEndpoint("PUT", route+"/api/v1/signup/verification", token, + fmt.Sprintf(`{ "country_code":"+44", "phone_number":"%s" }`, twilioMagicPhoneHighRiskBlocked), http.StatusForbidden). + UnmarshalMap() + + // then + require.NotEmpty(t, responseMap) + assert.Equal(t, "Forbidden", responseMap["status"]) + + userSignup, err := hostAwait.WaitForUserSignup(t, userSignup.Name) + require.NoError(t, err) + assert.True(t, states.Rejected(userSignup), "UserSignup should be rejected in enabled mode with high-risk phone") + assert.Empty(t, userSignup.Annotations[toolchainv1alpha1.UserSignupVerificationCodeAnnotationKey]) }) t.Run("enabled mode blocks verification for previously rejected signup", func(t *testing.T) { @@ -82,7 +119,7 @@ func TestPhoneLookupMode(t *testing.T) { assert.Empty(t, userSignup.Annotations[toolchainv1alpha1.UserSignupVerificationCodeAnnotationKey]) }) - t.Run("enabled mode skips lookup for US numbers", func(t *testing.T) { + t.Run("enabled mode skips lookup for US numbers and user is provisioned", func(t *testing.T) { // given hostAwait.UpdateToolchainConfig(t, testconfig.RegistrationService(). Verification().PhoneLookupMode(toolchainv1alpha1.PhoneLookupModeEnabled). @@ -101,5 +138,19 @@ func TestPhoneLookupMode(t *testing.T) { assert.Empty(t, userSignup.Annotations[toolchainv1alpha1.UserSignupPhoneLookupDetailsAnnotationKey]) assert.NotEmpty(t, userSignup.Annotations[toolchainv1alpha1.UserSignupVerificationCodeAnnotationKey]) + assert.False(t, states.Rejected(userSignup), "UserSignup should NOT be rejected for excluded country") + + // complete verification and confirm user is provisioned + NewHTTPRequest(t).InvokeEndpoint("GET", route+fmt.Sprintf("/api/v1/signup/verification/%s", + userSignup.Annotations[toolchainv1alpha1.UserSignupVerificationCodeAnnotationKey]), token, "", http.StatusOK) + + userSignup, err = wait.For(t, hostAwait.Awaitility, &toolchainv1alpha1.UserSignup{}). + Update(userSignup.Name, hostAwait.Namespace, + func(instance *toolchainv1alpha1.UserSignup) { + states.SetApprovedManually(instance, true) + }) + require.NoError(t, err) + + VerifyResourcesProvisionedForSignup(t, awaitilities, userSignup) }) }