Skip to content

Commit

Permalink
feat: improve messages for easier i18n (#3457)
Browse files Browse the repository at this point in the history
  • Loading branch information
zepatrik authored Aug 28, 2023
1 parent dda19e8 commit 37f1657
Show file tree
Hide file tree
Showing 25 changed files with 305 additions and 137 deletions.
44 changes: 25 additions & 19 deletions cmd/clidoc/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,11 @@ var inAMinute = time.Date(2020, 1, 1, 1, 0, 0, 0, time.UTC).Add(time.Minute)
var messages map[string]*text.Message

func init() {
text.Now = func() time.Time {
return inAMinute
}
text.Until = func(t time.Time) time.Duration {
return time.Second
return time.Minute
}
text.Since = func(time.Time) time.Duration {
return time.Minute
}

messages = map[string]*text.Message{
Expand All @@ -59,8 +59,8 @@ func init() {
"NewInfoSelfServiceSettingsRegenerateLookup": text.NewInfoSelfServiceSettingsRegenerateLookup(),
"NewInfoSelfServiceSettingsDisableLookup": text.NewInfoSelfServiceSettingsDisableLookup(),
"NewInfoSelfServiceSettingsLookupConfirm": text.NewInfoSelfServiceSettingsLookupConfirm(),
"NewInfoSelfServiceSettingsLookupSecretList": text.NewInfoSelfServiceSettingsLookupSecretList([]string{"{code-1}", "{code-2}"}, []interface{}{
text.NewInfoSelfServiceSettingsLookupSecret("{code}"),
"NewInfoSelfServiceSettingsLookupSecretList": text.NewInfoSelfServiceSettingsLookupSecretList([]string{"{secrets_list}"}, []interface{}{
text.NewInfoSelfServiceSettingsLookupSecret("{secret}"),
text.NewInfoSelfServiceSettingsLookupSecretUsed(aSecondAgo),
}),
"NewInfoSelfServiceSettingsLookupSecret": text.NewInfoSelfServiceSettingsLookupSecret("{secret}"),
Expand All @@ -82,19 +82,25 @@ func init() {
"NewErrorSystemGeneric": text.NewErrorSystemGeneric("{reason}"),
"NewValidationErrorGeneric": text.NewValidationErrorGeneric("{reason}"),
"NewValidationErrorRequired": text.NewValidationErrorRequired("{field}"),
"NewErrorValidationMinLength": text.NewErrorValidationMinLength("length must be >= 5, but got 3"),
"NewErrorValidationMaxLength": text.NewErrorValidationMaxLength("length must be <= 5, but got 6"),
"NewErrorValidationInvalidFormat": text.NewErrorValidationInvalidFormat("does not match pattern \"^[a-z]*$\""),
"NewErrorValidationMinimum": text.NewErrorValidationMinimum("must be >= 5 but found 3"),
"NewErrorValidationExclusiveMinimum": text.NewErrorValidationExclusiveMinimum("must be > 5 but found 5"),
"NewErrorValidationMaximum": text.NewErrorValidationMaximum("must be <= 5 but found 6"),
"NewErrorValidationExclusiveMaximum": text.NewErrorValidationExclusiveMaximum("must be < 5 but found 5"),
"NewErrorValidationMultipleOf": text.NewErrorValidationMultipleOf("7 not multipleOf 3"),
"NewErrorValidationMaxItems": text.NewErrorValidationMaxItems("maximum 3 items allowed, but found 4 items"),
"NewErrorValidationMinItems": text.NewErrorValidationMinItems("minimum 3 items allowed, but found 2 items"),
"NewErrorValidationUniqueItems": text.NewErrorValidationUniqueItems("items at index 0 and 2 are equal"),
"NewErrorValidationWrongType": text.NewErrorValidationWrongType("expected number, but got string"),
"NewErrorValidationPasswordPolicyViolation": text.NewErrorValidationPasswordPolicyViolation("{reason}"),
"NewErrorValidationMinLength": text.NewErrorValidationMinLength(5, 3),
"NewErrorValidationMaxLength": text.NewErrorValidationMaxLength(5, 6),
"NewErrorValidationInvalidFormat": text.NewErrorValidationInvalidFormat("{pattern}"),
"NewErrorValidationMinimum": text.NewErrorValidationMinimum(5, 3),
"NewErrorValidationExclusiveMinimum": text.NewErrorValidationExclusiveMinimum(5, 5),
"NewErrorValidationMaximum": text.NewErrorValidationMaximum(5, 6),
"NewErrorValidationExclusiveMaximum": text.NewErrorValidationExclusiveMaximum(5, 5),
"NewErrorValidationMultipleOf": text.NewErrorValidationMultipleOf(7, 3),
"NewErrorValidationMaxItems": text.NewErrorValidationMaxItems(3, 4),
"NewErrorValidationMinItems": text.NewErrorValidationMinItems(3, 2),
"NewErrorValidationUniqueItems": text.NewErrorValidationUniqueItems(0, 2),
"NewErrorValidationWrongType": text.NewErrorValidationWrongType([]string{"{allowed_types_list}"}, "{actual_type}"),
"NewErrorValidationConst": text.NewErrorValidationConst("{expected}"),
"NewErrorValidationConstGeneric": text.NewErrorValidationConstGeneric(),
"NewErrorValidationPasswordPolicyViolationGeneric": text.NewErrorValidationPasswordPolicyViolationGeneric("{reason}"),
"NewErrorValidationPasswordIdentifierTooSimilar": text.NewErrorValidationPasswordIdentifierTooSimilar(),
"NewErrorValidationPasswordMinLength": text.NewErrorValidationPasswordMinLength(6, 5),
"NewErrorValidationPasswordMaxLength": text.NewErrorValidationPasswordMaxLength(72, 80),
"NewErrorValidationPasswordTooManyBreaches": text.NewErrorValidationPasswordTooManyBreaches(101),
"NewErrorValidationInvalidCredentials": text.NewErrorValidationInvalidCredentials(),
"NewErrorValidationDuplicateCredentials": text.NewErrorValidationDuplicateCredentials(),
"NewErrorValidationDuplicateCredentialsWithHints": text.NewErrorValidationDuplicateCredentialsWithHints("{reason}", nil, nil, ""),
Expand Down
4 changes: 3 additions & 1 deletion hash/hasher_bcrypt.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ import (
"context"
"fmt"

"github.com/ory/kratos/text"

"go.opentelemetry.io/otel"
"go.opentelemetry.io/otel/attribute"
"go.opentelemetry.io/otel/trace"
Expand Down Expand Up @@ -58,7 +60,7 @@ func validateBcryptPasswordLength(password []byte) error {
if len(password) > 72 {
return schema.NewPasswordPolicyViolationError(
"#/password",
"passwords are limited to a maximum length of 72 characters",
text.NewErrorValidationPasswordMaxLength(72, len(password)),
)
}
return nil
Expand Down
8 changes: 4 additions & 4 deletions schema/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,16 +87,16 @@ func (r *ValidationErrorContextPasswordPolicyViolation) AddContext(_, _ string)

func (r *ValidationErrorContextPasswordPolicyViolation) FinishInstanceContext() {}

func NewPasswordPolicyViolationError(instancePtr string, reason string) error {
func NewPasswordPolicyViolationError(instancePtr string, message *text.Message) error {
return errors.WithStack(&ValidationError{
ValidationError: &jsonschema.ValidationError{
Message: fmt.Sprintf("the password does not fulfill the password policy because: %s", reason),
Message: fmt.Sprintf("the password does not fulfill the password policy because: %s", message.Text),
InstancePtr: instancePtr,
Context: &ValidationErrorContextPasswordPolicyViolation{
Reason: reason,
Reason: message.Text,
},
},
Messages: new(text.Messages).Add(text.NewErrorValidationPasswordPolicyViolation(reason)),
Messages: new(text.Messages).Add(message),
})
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,8 @@
{
"id": 4000001,
"text": "The active recovery strategy code is not enabled. Please enable it in the configuration.",
"type": "error"
"type": "error",
"context": {}
}
]
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,8 @@
{
"id": 4000001,
"text": "The active recovery strategy code is not enabled. Please enable it in the configuration.",
"type": "error"
"type": "error",
"context": {}
}
]
},
Expand Down
5 changes: 4 additions & 1 deletion selfservice/strategy/password/registration.go
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,10 @@ func (s *Strategy) validateCredentials(ctx context.Context, i *identity.Identity
if _, ok := errorsx.Cause(err).(*herodot.DefaultError); ok {
return err
}
return schema.NewPasswordPolicyViolationError("#/password", err.Error())
if message := new(text.Message); errors.As(err, &message) {
return schema.NewPasswordPolicyViolationError("#/password", message)
}
return schema.NewPasswordPolicyViolationError("#/password", text.NewErrorValidationPasswordPolicyViolationGeneric(err.Error()))
}
}

Expand Down
12 changes: 6 additions & 6 deletions selfservice/strategy/password/settings_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -133,10 +133,10 @@ func TestSettings(t *testing.T) {
}

t.Run("description=should fail if password violates policy", func(t *testing.T) {
var check = func(t *testing.T, actual string) {
var check = func(t *testing.T, reason, actual string) {
assert.Empty(t, gjson.Get(actual, "ui.nodes.#(attributes.name==password).attributes.value").String(), "%s", actual)
assert.NotEmpty(t, gjson.Get(actual, "ui.nodes.#(attributes.name==csrf_token).attributes.value").String(), "%s", actual)
assert.Contains(t, gjson.Get(actual, "ui.nodes.#(attributes.name==password).messages.0.text").String(), "password can not be used because", "%s", actual)
assert.Equal(t, reason, gjson.Get(actual, "ui.nodes.#(attributes.name==password).messages.0.text").String(), "%s", actual)
}

t.Run("session=with privileged session", func(t *testing.T) {
Expand All @@ -148,15 +148,15 @@ func TestSettings(t *testing.T) {
}

t.Run("type=api", func(t *testing.T) {
check(t, expectValidationError(t, true, false, apiUser1, payload))
check(t, "The password must be at least 8 characters long, but got 6.", expectValidationError(t, true, false, apiUser1, payload))
})

t.Run("spa=spa", func(t *testing.T) {
check(t, expectValidationError(t, false, true, browserUser1, payload))
check(t, "The password must be at least 8 characters long, but got 6.", expectValidationError(t, false, true, browserUser1, payload))
})

t.Run("type=browser", func(t *testing.T) {
check(t, expectValidationError(t, false, false, browserUser1, payload))
check(t, "The password must be at least 8 characters long, but got 6.", expectValidationError(t, false, false, browserUser1, payload))
})
})

Expand Down Expand Up @@ -190,7 +190,7 @@ func TestSettings(t *testing.T) {

t.Run("type=browser", func(t *testing.T) {
_ = testhelpers.NewSettingsLoginAcceptAPIServer(t, testhelpers.NewSDKCustomClient(publicTS, browserUser1), conf)
check(t, expectValidationError(t, false, false, browserUser1, payload))
check(t, "The password must be at least 8 characters long, but got 6.", expectValidationError(t, false, false, browserUser1, payload))
})
})
})
Expand Down
9 changes: 5 additions & 4 deletions selfservice/strategy/password/validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ import (
"strings"
"time"

"github.com/ory/kratos/text"

"github.com/arbovm/levenshtein"
"github.com/dgraph-io/ristretto"
"github.com/hashicorp/go-retryablehttp"
Expand Down Expand Up @@ -45,7 +47,6 @@ var (
_ Validator = new(DefaultPasswordValidator)
ErrNetworkFailure = stderrs.New("unable to check if password has been leaked because an unexpected network error occurred")
ErrUnexpectedStatusCode = stderrs.New("unexpected status code")
ErrTooManyBreaches = stderrs.New("the password has been found in data breaches and must no longer be used")
)

// DefaultPasswordValidator implements Validator. It is based on best
Expand Down Expand Up @@ -179,15 +180,15 @@ func (s *DefaultPasswordValidator) validate(ctx context.Context, identifier, pas
passwordPolicyConfig := s.reg.Config().PasswordPolicyConfig(ctx)

if len(password) < int(passwordPolicyConfig.MinPasswordLength) {
return errors.Errorf("password length must be at least %d characters but only got %d", passwordPolicyConfig.MinPasswordLength, len(password))
return text.NewErrorValidationPasswordMinLength(int(passwordPolicyConfig.MinPasswordLength), len(password))
}

if passwordPolicyConfig.IdentifierSimilarityCheckEnabled && len(identifier) > 0 {
compIdentifier, compPassword := strings.ToLower(identifier), strings.ToLower(password)
dist := levenshtein.Distance(compIdentifier, compPassword)
lcs := float32(lcsLength(compIdentifier, compPassword)) / float32(len(compPassword))
if dist < s.minIdentifierPasswordDist || lcs > s.maxIdentifierPasswordSubstrThreshold {
return errors.Errorf("the password is too similar to the user identifier")
return text.NewErrorValidationPasswordIdentifierTooSimilar()
}
}

Expand Down Expand Up @@ -215,7 +216,7 @@ func (s *DefaultPasswordValidator) validate(ctx context.Context, identifier, pas

v, ok := c.(int64)
if ok && v > int64(s.reg.Config().PasswordPolicyConfig(ctx).MaxBreaches) {
return errors.WithStack(ErrTooManyBreaches)
return text.NewErrorValidationPasswordTooManyBreaches(v)
}

return nil
Expand Down
4 changes: 3 additions & 1 deletion selfservice/strategy/password/validator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ import (
"testing"
"time"

"github.com/ory/kratos/text"

"github.com/stretchr/testify/assert"

"github.com/ory/herodot"
Expand Down Expand Up @@ -224,7 +226,7 @@ func TestDefaultPasswordValidationStrategy(t *testing.T) {
res: func(t *testing.T, hash string) string {
return fmt.Sprintf("%s:%d", hash, conf.PasswordPolicyConfig(ctx).MaxBreaches+1)
},
expectErr: password.ErrTooManyBreaches,
expectErr: text.NewErrorValidationPasswordTooManyBreaches(int64(conf.PasswordPolicyConfig(ctx).MaxBreaches) + 1),
},
} {
t.Run(fmt.Sprintf("case=%s/expected err=%s", tc.name, tc.expectErr), func(t *testing.T) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@
{
"id": 4000001,
"text": "unable to remove this security key because it would lock you out of your account",
"type": "error"
"type": "error",
"context": {}
}
],
"meta": {}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@
{
"id": 4000001,
"text": "unable to remove this security key because it would lock you out of your account",
"type": "error"
"type": "error",
"context": {}
}
],
"meta": {}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ describe("Registration failures with email profile", () => {
.should("have.value", "12345678")

cy.submitPasswordForm()
cy.get('[data-testid="ui/message/4000005"]').should(
cy.get('[data-testid="ui/message/4000034"]').should(
"contain.text",
"data breaches",
)
Expand All @@ -79,7 +79,7 @@ describe("Registration failures with email profile", () => {
cy.get('input[name="password"]').type(identity)

cy.submitPasswordForm()
cy.get('[data-testid="ui/message/4000005"]').should(
cy.get('[data-testid="ui/message/4000031"]').should(
"contain.text",
"too similar",
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ context("Settings failures with email profile", () => {
it("fails if password policy is violated", () => {
cy.get('input[name="password"]').clear().type("12345678")
cy.get('button[value="password"]').click()
cy.get('[data-testid="ui/message/4000005"]').should(
cy.get('[data-testid="ui/message/4000034"]').should(
"contain.text",
"data breaches",
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,14 +61,15 @@ context("Settings success with email profile", () => {
cy.get('input[name="password"]').clear().type("123")
cy.get('button[value="password"]').click()
cy.get('[data-testid="ui/message/1050001"]').should("not.exist")
cy.get('[data-testid="ui/message/4000005"]').should("exist")
cy.get('[data-testid="ui/message/4000032"]').should("exist")
cy.get('input[name="password"]').should("be.empty")

password = up(password)
cy.get('input[name="password"]').clear().type(password)
cy.get('button[value="password"]').click()
cy.expectSettingsSaved()
cy.get('[data-testid="ui/message/4000005"]').should("not.exist")
cy.get('[data-testid="ui/message/4000032"]').should("not.exist")
cy.get('[data-testid="ui/message/1050001"]').should("exist")
cy.get('input[name="password"]').should("be.empty")
})

Expand Down
6 changes: 3 additions & 3 deletions test/e2e/cypress/support/commands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -811,17 +811,17 @@ Cypress.Commands.add(
failOnStatusCode: false,
})
})
.then(({ status }) => {
.then(({ status, body }) => {
console.log("Login sequence completed: ", {
email,
password,
expectSession,
})
if (expectSession) {
expect(status).to.eq(200)
expect(status).to.eq(200, body)
return cy.getSession()
} else {
expect(status).to.not.eq(200)
expect(status).to.not.eq(200, body)
return cy.noSession()
}
})
Expand Down
8 changes: 7 additions & 1 deletion text/id.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ const (
ErrorValidationRequired
ErrorValidationMinLength
ErrorValidationInvalidFormat
ErrorValidationPasswordPolicyViolation
ErrorValidationPasswordPolicyViolationGeneric
ErrorValidationInvalidCredentials
ErrorValidationDuplicateCredentials
ErrorValidationTOTPVerifierWrong
Expand All @@ -122,6 +122,12 @@ const (
ErrorValidationWrongType
ErrorValidationDuplicateCredentialsOnOIDCLink
ErrorValidationDuplicateCredentialsWithHints
ErrorValidationConst
ErrorValidationConstGeneric
ErrorValidationPasswordIdentifierTooSimilar
ErrorValidationPasswordMinLength
ErrorValidationPasswordMaxLength
ErrorValidationPasswordTooManyBreaches
)

const (
Expand Down
12 changes: 12 additions & 0 deletions text/message.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,3 +72,15 @@ func (m *Message) Scan(value interface{}) error {
func (m Message) Value() (driver.Value, error) {
return sqlxx.JSONValue(&m)
}

func (m *Message) Error() string {
return m.Text
}

func (m *Message) Is(err error) bool {
em, ok := err.(*Message)
if !ok {
return false
}
return m.ID == em.ID
}
2 changes: 1 addition & 1 deletion text/message_login.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ func NewInfoLoginWith(provider string) *Message {
func NewErrorValidationLoginFlowExpired(expiredAt time.Time) *Message {
return &Message{
ID: ErrorValidationLoginFlowExpired,
Text: fmt.Sprintf("The login flow expired %.2f minutes ago, please try again.", Now().Sub(expiredAt).Minutes()),
Text: fmt.Sprintf("The login flow expired %.2f minutes ago, please try again.", Since(expiredAt).Minutes()),
Type: Error,
Context: context(map[string]interface{}{
"expired_at": expiredAt,
Expand Down
2 changes: 1 addition & 1 deletion text/message_recovery.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import (
func NewErrorValidationRecoveryFlowExpired(expiredAt time.Time) *Message {
return &Message{
ID: ErrorValidationRecoveryFlowExpired,
Text: fmt.Sprintf("The recovery flow expired %.2f minutes ago, please try again.", (-Until(expiredAt)).Minutes()),
Text: fmt.Sprintf("The recovery flow expired %.2f minutes ago, please try again.", Since(expiredAt).Minutes()),
Type: Error,
Context: context(map[string]interface{}{
"expired_at": expiredAt,
Expand Down
2 changes: 1 addition & 1 deletion text/message_registration.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ func NewInfoRegistrationContinue() *Message {
func NewErrorValidationRegistrationFlowExpired(expiredAt time.Time) *Message {
return &Message{
ID: ErrorValidationRegistrationFlowExpired,
Text: fmt.Sprintf("The registration flow expired %.2f minutes ago, please try again.", (-Until(expiredAt)).Minutes()),
Text: fmt.Sprintf("The registration flow expired %.2f minutes ago, please try again.", Since(expiredAt).Minutes()),
Type: Error,
Context: context(map[string]interface{}{
"expired_at": expiredAt,
Expand Down
2 changes: 1 addition & 1 deletion text/message_settings.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import (
func NewErrorValidationSettingsFlowExpired(expiredAt time.Time) *Message {
return &Message{
ID: ErrorValidationSettingsFlowExpired,
Text: fmt.Sprintf("The settings flow expired %.2f minutes ago, please try again.", (-Until(expiredAt)).Minutes()),
Text: fmt.Sprintf("The settings flow expired %.2f minutes ago, please try again.", Since(expiredAt).Minutes()),
Type: Error,
Context: context(map[string]interface{}{
"expired_at": expiredAt,
Expand Down
Loading

0 comments on commit 37f1657

Please sign in to comment.