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
Show file tree
Hide file tree
Changes from all commits
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
2 changes: 2 additions & 0 deletions cmd/clidoc/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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}"),
}
}

Expand Down
3 changes: 2 additions & 1 deletion selfservice/strategy/link/strategy_verification_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"net/http"
"net/http/httptest"
"net/url"
"strings"
"sync"
"testing"
"time"
Expand Down Expand Up @@ -143,7 +144,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, strings.ReplaceAll(fmt.Sprintf("%q is not a valid email address", value), "\"", ""),
gjson.Get(actual, "ui.nodes.#(attributes.name==email).messages.0.text").String(),
"%s", actual)
}
Expand Down
2 changes: 2 additions & 0 deletions text/id.go
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,8 @@ const (
ErrorValidationTraitsMismatch
ErrorValidationAccountNotFound
ErrorValidationCaptchaError
ErrorValidationEmail
ErrorValidationPhone
)

const (
Expand Down
3 changes: 3 additions & 0 deletions text/id_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Up @@ -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,
Expand Down
24 changes: 24 additions & 0 deletions ui/container/container.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
Loading