Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: return a specific error message for email & phone validation errors #4166

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
fix: return a specific error message for email & phone validation errors
jonas-jonas committed Oct 22, 2024

Verified

This commit was signed with the committer’s verified signature.
sverweij Sander Verweij
commit 0efa61f8ff78f7bef08155418611f88c20ddba3b
2 changes: 2 additions & 0 deletions cmd/clidoc/main.go
Original file line number Diff line number Diff line change
@@ -181,6 +181,8 @@ func init() {
"NewInfoSelfServiceLoginAAL2CodeAddress": text.NewInfoSelfServiceLoginAAL2CodeAddress("{channel}", "{address}"),
"NewErrorCaptchaFailed": text.NewErrorCaptchaFailed(),
"NewCaptchaContainerMessage": text.NewCaptchaContainerMessage(),
"NewErrorValidationEmail": text.NewErrorValidationEmail("{value}"),
"NewErrorValidationPhone": text.NewErrorValidationPhone("{value}"),
}
}

2 changes: 1 addition & 1 deletion selfservice/strategy/link/strategy_verification_test.go
Original file line number Diff line number Diff line change
@@ -143,7 +143,7 @@ func TestVerification(t *testing.T) {
t.Run("description=should require a valid email to be sent", func(t *testing.T) {
check := func(t *testing.T, actual string, value string) {
assert.EqualValues(t, string(node.LinkGroup), gjson.Get(actual, "active").String(), "%s", actual)
assert.EqualValues(t, fmt.Sprintf("%q is not valid \"email\"", value),
assert.EqualValues(t, fmt.Sprintf("%s is not a valid email address", value),
gjson.Get(actual, "ui.nodes.#(attributes.name==email).messages.0.text").String(),
"%s", actual)
}
2 changes: 2 additions & 0 deletions text/id.go
Original file line number Diff line number Diff line change
@@ -153,6 +153,8 @@ const (
ErrorValidationTraitsMismatch
ErrorValidationAccountNotFound
ErrorValidationCaptchaError
ErrorValidationEmail
ErrorValidationPhone
)

const (
3 changes: 3 additions & 0 deletions text/id_test.go
Original file line number Diff line number Diff line change
@@ -74,4 +74,7 @@ func TestIDs(t *testing.T) {

assert.Equal(t, 1070015, int(InfoNodeLabelCaptcha))
assert.Equal(t, 4000038, int(ErrorValidationCaptchaError))

assert.Equal(t, 4000039, int(ErrorValidationEmail))
assert.Equal(t, 4000040, int(ErrorValidationPhone))
}
22 changes: 22 additions & 0 deletions text/message_validation.go
Original file line number Diff line number Diff line change
@@ -195,6 +195,28 @@ func NewErrorValidationConstGeneric() *Message {
}
}

func NewErrorValidationEmail(value string) *Message {
return &Message{
ID: ErrorValidationEmail,
Text: fmt.Sprintf("%s is not a valid email address", value),
Type: Error,
Context: context(map[string]any{
"value": value,
}),
}
}

func NewErrorValidationPhone(value string) *Message {
return &Message{
ID: ErrorValidationEmail,
Text: fmt.Sprintf("%s is not a valid phone number", value),
Type: Error,
Context: context(map[string]any{
"value": value,
}),
}
}

func NewErrorValidationPasswordPolicyViolationGeneric(reason string) *Message {
return &Message{
ID: ErrorValidationPasswordPolicyViolationGeneric,
24 changes: 24 additions & 0 deletions ui/container/container.go
Original file line number Diff line number Diff line change
@@ -271,6 +271,30 @@ func translateValidationError(err *jsonschema.ValidationError) *text.Message {
return text.NewErrorValidationConst(expectedValue)
}
return text.NewErrorValidationConstGeneric()
case "format":
value, format := "", ""
_, _ = fmt.Sscanf(err.Message, "%s is not valid %s", &value, &format)
if len(value) < 3 {
// This should not happen, because the library returns the message: "value" is not a valid "format"
// But if it does, we can return a generic error instead of panicking later due to index out of range
return text.NewValidationErrorGeneric(err.Message)
}

// Format is extracted with quotes around the actual string, so we remove all quotes from the string to make it easier to match against
format = strings.ReplaceAll(format, "\"", "")
// The value is user supplied, so it could contain quotes. Replacing all quotes would also remove the user supplied ones.
// While strings with quotes in them are most likely never valid values here, we should still echo the full user entered string to the user to make it easier to understand the error.
// E.g. if we replaced all quotes, the user could see the error [email protected] is not a valid email address, if they entered it as valid"[email protected] (note the quote)
value = value[1 : len(value)-1]

switch format {
case "email":
return text.NewErrorValidationEmail(value)
case "tel":
return text.NewErrorValidationPhone(value)
default:
return text.NewValidationErrorGeneric(err.Message)
}
default:
return text.NewValidationErrorGeneric(err.Message)
}

Unchanged files with check annotations Beta

// Copyright © 2024 Ory Corp

Check failure on line 1 in test/e2e/playwright/tests/desktop/identifier_first/oidc.login.spec.ts

GitHub Actions / Run Playwright end-to-end tests (mysql)

[chromium] › desktop/identifier_first/oidc.login.spec.ts:78:9 › account enumeration protection on › login

1) [chromium] › desktop/identifier_first/oidc.login.spec.ts:78:9 › account enumeration protection on › login Test timeout of 30000ms exceeded.

Check failure on line 1 in test/e2e/playwright/tests/desktop/identifier_first/oidc.login.spec.ts

GitHub Actions / Run Playwright end-to-end tests (postgres)

[chromium] › desktop/identifier_first/oidc.login.spec.ts:78:9 › account enumeration protection on › login

1) [chromium] › desktop/identifier_first/oidc.login.spec.ts:78:9 › account enumeration protection on › login Test timeout of 30000ms exceeded.

Check failure on line 1 in test/e2e/playwright/tests/desktop/identifier_first/oidc.login.spec.ts

GitHub Actions / Run Playwright end-to-end tests (cockroach)

[chromium] › desktop/identifier_first/oidc.login.spec.ts:78:9 › account enumeration protection on › login

1) [chromium] › desktop/identifier_first/oidc.login.spec.ts:78:9 › account enumeration protection on › login Test timeout of 30000ms exceeded.

Check failure on line 1 in test/e2e/playwright/tests/desktop/identifier_first/oidc.login.spec.ts

GitHub Actions / Run Playwright end-to-end tests (sqlite)

[chromium] › desktop/identifier_first/oidc.login.spec.ts:78:9 › account enumeration protection on › login

1) [chromium] › desktop/identifier_first/oidc.login.spec.ts:78:9 › account enumeration protection on › login Test timeout of 30000ms exceeded.
// SPDX-License-Identifier: Apache-2.0
import { faker } from "@faker-js/faker"
await loginHydra(page)
await page.waitForURL(

Check failure on line 86 in test/e2e/playwright/tests/desktop/identifier_first/oidc.login.spec.ts

GitHub Actions / Run Playwright end-to-end tests (mysql)

[chromium] › desktop/identifier_first/oidc.login.spec.ts:78:9 › account enumeration protection on › login

1) [chromium] › desktop/identifier_first/oidc.login.spec.ts:78:9 › account enumeration protection on › login Error: page.waitForURL: Test timeout of 30000ms exceeded. =========================== logs =========================== waiting for navigation until "load" ============================================================ 84 | await loginHydra(page) 85 | > 86 | await page.waitForURL( | ^ 87 | new RegExp(config.selfservice.default_browser_return_url), 88 | ) 89 | at /home/runner/work/kratos/kratos/test/e2e/playwright/tests/desktop/identifier_first/oidc.login.spec.ts:86:18

Check failure on line 86 in test/e2e/playwright/tests/desktop/identifier_first/oidc.login.spec.ts

GitHub Actions / Run Playwright end-to-end tests (postgres)

[chromium] › desktop/identifier_first/oidc.login.spec.ts:78:9 › account enumeration protection on › login

1) [chromium] › desktop/identifier_first/oidc.login.spec.ts:78:9 › account enumeration protection on › login Error: page.waitForURL: Test timeout of 30000ms exceeded. =========================== logs =========================== waiting for navigation until "load" ============================================================ 84 | await loginHydra(page) 85 | > 86 | await page.waitForURL( | ^ 87 | new RegExp(config.selfservice.default_browser_return_url), 88 | ) 89 | at /home/runner/work/kratos/kratos/test/e2e/playwright/tests/desktop/identifier_first/oidc.login.spec.ts:86:18

Check failure on line 86 in test/e2e/playwright/tests/desktop/identifier_first/oidc.login.spec.ts

GitHub Actions / Run Playwright end-to-end tests (cockroach)

[chromium] › desktop/identifier_first/oidc.login.spec.ts:78:9 › account enumeration protection on › login

1) [chromium] › desktop/identifier_first/oidc.login.spec.ts:78:9 › account enumeration protection on › login Error: page.waitForURL: Test timeout of 30000ms exceeded. =========================== logs =========================== waiting for navigation until "load" ============================================================ 84 | await loginHydra(page) 85 | > 86 | await page.waitForURL( | ^ 87 | new RegExp(config.selfservice.default_browser_return_url), 88 | ) 89 | at /home/runner/work/kratos/kratos/test/e2e/playwright/tests/desktop/identifier_first/oidc.login.spec.ts:86:18

Check failure on line 86 in test/e2e/playwright/tests/desktop/identifier_first/oidc.login.spec.ts

GitHub Actions / Run Playwright end-to-end tests (sqlite)

[chromium] › desktop/identifier_first/oidc.login.spec.ts:78:9 › account enumeration protection on › login

1) [chromium] › desktop/identifier_first/oidc.login.spec.ts:78:9 › account enumeration protection on › login Error: page.waitForURL: Test timeout of 30000ms exceeded. =========================== logs =========================== waiting for navigation until "load" ============================================================ 84 | await loginHydra(page) 85 | > 86 | await page.waitForURL( | ^ 87 | new RegExp(config.selfservice.default_browser_return_url), 88 | ) 89 | at /home/runner/work/kratos/kratos/test/e2e/playwright/tests/desktop/identifier_first/oidc.login.spec.ts:86:18
new RegExp(config.selfservice.default_browser_return_url),
)