Skip to content

Commit

Permalink
fix: report correct errors for json schema validation (#3085)
Browse files Browse the repository at this point in the history
- Implemented the translation of `jsonschema.ValidationError` to errors codes documented [here](https://www.ory.sh/docs/kratos/concepts/ui-user-interface#machine-readable-format)
- Added missing error codes for relevant schema errors
  | Validation         | Name                            | ID      |
  | ------------------ | ------------------------------- | ------- |
  | `maxLength`        | ErrorValidationMaxLength        | 4000017 |
  | `minimum`          | ErrorValidationMinimum.         | 4000018 |
  | `exclusiveMinimum` | ErrorValidationExclusiveMinimum | 4000019 |
  | `maximum`          | ErrorValidationMaximum          | 4000020 |
  | `exclusiveMaximum` | ErrorValidationExclusiveMaximum | 4000021 |
  | `multipleOf`       | ErrorValidationMultipleOf       | 4000022 |
  | `maxItems`         | ErrorValidationMaxItems         | 4000023 |
  | `minItems`         | ErrorValidationMinItems         | 4000024 |
  | `uniqueItems`      | ErrorValidationUniqueItems      | 4000025 |
  | `type`             | ErrorValidationWrongType        | 4000026 |
- Updated e2e tests to check these IDs explicitly
  • Loading branch information
CaptainStandby committed Feb 10, 2023
1 parent 74ae852 commit 9477ea4
Show file tree
Hide file tree
Showing 10 changed files with 279 additions and 55 deletions.
14 changes: 12 additions & 2 deletions cmd/clidoc/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,18 @@ func init() {
"NewErrorSystemGeneric": text.NewErrorSystemGeneric("{reason}"),
"NewValidationErrorGeneric": text.NewValidationErrorGeneric("{reason}"),
"NewValidationErrorRequired": text.NewValidationErrorRequired("{field}"),
"NewErrorValidationMinLength": text.NewErrorValidationMinLength(1, 2),
"NewErrorValidationInvalidFormat": text.NewErrorValidationInvalidFormat("{format}", "{value}"),
"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}"),
"NewErrorValidationInvalidCredentials": text.NewErrorValidationInvalidCredentials(),
"NewErrorValidationDuplicateCredentials": text.NewErrorValidationDuplicateCredentials(),
Expand Down
20 changes: 0 additions & 20 deletions schema/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,16 +18,6 @@ type ValidationError struct {
Messages text.Messages
}

func NewMinLengthError(instancePtr string, expected, actual int) error {
return errors.WithStack(&ValidationError{
ValidationError: &jsonschema.ValidationError{
Message: fmt.Sprintf("length must be >= %d, but got %d", expected, actual),
InstancePtr: instancePtr,
},
Messages: new(text.Messages).Add(text.NewErrorValidationMinLength(expected, actual)),
})
}

func NewRequiredError(missingPtr, missingFieldName string) error {
return errors.WithStack(&ValidationError{
ValidationError: &jsonschema.ValidationError{
Expand All @@ -41,16 +31,6 @@ func NewRequiredError(missingPtr, missingFieldName string) error {
})
}

func NewInvalidFormatError(instancePtr, format, value string) error {
return errors.WithStack(&ValidationError{
ValidationError: &jsonschema.ValidationError{
Message: fmt.Sprintf("%q is not valid %q", value, format),
InstancePtr: instancePtr,
},
Messages: new(text.Messages).Add(text.NewErrorValidationInvalidFormat(value, format)),
})
}

func NewTOTPVerifierWrongError(instancePtr string) error {
t := text.NewErrorValidationTOTPVerifierWrong()
return errors.WithStack(&ValidationError{
Expand Down
107 changes: 107 additions & 0 deletions selfservice/strategy/password/registration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,113 @@ func TestRegistration(t *testing.T) {
})
})

t.Run("case=should return correct error ids from validation failures", func(t *testing.T) {
test := func(t *testing.T, constraint string, setValues func(url.Values), expectedId text.ID, expectedMesage string) {
template := `{
"$id": "https://example.com/person.schema.json",
"$schema": "http://json-schema.org/draft-07/schema#",
"title": "Person",
"type": "object",
"properties": {
"traits": {
"type": "object",
"properties": {
"foobar": {
%s
},
"username": {
"type": "string",
"ory.sh/kratos": {
"credentials": {
"password": {
"identifier": true
}
}
}
}
},
"required": [
"foobar",
"username"
]
}
},
"additionalProperties": false
}`

testhelpers.SetDefaultIdentitySchemaFromRaw(conf, []byte(fmt.Sprintf(template, constraint)))

browserClient := testhelpers.NewClientWithCookies(t)
f := testhelpers.InitializeRegistrationFlowViaBrowser(t, browserClient, publicTS, false, false, false)
c := f.Ui

values := testhelpers.SDKFormFieldsToURLValues(c.Nodes)
setValues(values)
values.Set("traits.username", "registration-identifier-9")
values.Set("password", x.NewUUID().String())
actual, _ := testhelpers.RegistrationMakeRequest(t, false, false, f, browserClient, values.Encode())

assert.NotEmpty(t, gjson.Get(actual, "id").String(), "%s", actual)
assert.Contains(t, gjson.Get(actual, "ui.action").String(), publicTS.URL+registration.RouteSubmitFlow, "%s", actual)
registrationhelpers.CheckFormContent(t, []byte(actual), "password", "csrf_token", "traits.username")
assert.EqualValues(t, "registration-identifier-9", gjson.Get(actual, "ui.nodes.#(attributes.name==traits.username).attributes.value").String(), "%s", actual)
assert.Empty(t, gjson.Get(actual, "ui.nodes.messages").Array())
assert.Len(t, gjson.Get(actual, "ui.nodes.#(attributes.name==traits.username).messages").Array(), 0)
assert.Equal(t, int64(expectedId), gjson.Get(actual, "ui.nodes.#(attributes.name==traits.foobar).messages.0.id").Int())
assert.Equal(t, expectedMesage, gjson.Get(actual, "ui.nodes.#(attributes.name==traits.foobar).messages.0.text").String())
assert.Equal(t, "error", gjson.Get(actual, "ui.nodes.#(attributes.name==traits.foobar).messages.0.type").String())
}

const key = "traits.foobar"
t.Run("case=string violating minLength", func(t *testing.T) {
test(t, `"type": "string", "minLength": 5`, func(v url.Values) { v.Set(key, "bar") }, text.ErrorValidationMinLength, "length must be >= 5, but got 3")
})

t.Run("case=string violating maxLength", func(t *testing.T) {
test(t, `"type": "string", "maxLength": 5`, func(v url.Values) { v.Set(key, "qwerty") }, text.ErrorValidationMaxLength, "length must be <= 5, but got 6")
})

t.Run("case=string violating pattern", func(t *testing.T) {
test(t, `"type": "string", "pattern": "^[a-z]*$"`, func(v url.Values) { v.Set(key, "FUBAR") }, text.ErrorValidationInvalidFormat, "does not match pattern \"^[a-z]*$\"")
})

t.Run("case=number violating minimum", func(t *testing.T) {
test(t, `"type": "number", "minimum": 5`, func(v url.Values) { v.Set(key, "3") }, text.ErrorValidationMinimum, "must be >= 5 but found 3")
})

t.Run("case=number violating exclusiveMinimum", func(t *testing.T) {
test(t, `"type": "number", "exclusiveMinimum": 5`, func(v url.Values) { v.Set(key, "5") }, text.ErrorValidationExclusiveMinimum, "must be > 5 but found 5")
})

t.Run("case=number violating maximum", func(t *testing.T) {
test(t, `"type": "number", "maximum": 5`, func(v url.Values) { v.Set(key, "6") }, text.ErrorValidationMaximum, "must be <= 5 but found 6")
})

t.Run("case=number violating exclusiveMaximum", func(t *testing.T) {
test(t, `"type": "number", "exclusiveMaximum": 5`, func(v url.Values) { v.Set(key, "5") }, text.ErrorValidationExclusiveMaximum, "must be < 5 but found 5")
})

t.Run("case=number violating multipleOf", func(t *testing.T) {
test(t, `"type": "number", "multipleOf": 3`, func(v url.Values) { v.Set(key, "7") }, text.ErrorValidationMultipleOf, "7 not multipleOf 3")
})

t.Run("case=array violating maxItems", func(t *testing.T) {
test(t, `"type": "array", "items": { "type": "string" }, "maxItems": 3`, func(v url.Values) { v.Add(key, "a"); v.Add(key, "b"); v.Add(key, "c"); v.Add(key, "d") }, text.ErrorValidationMaxItems, "maximum 3 items allowed, but found 4 items")
})

t.Run("case=array violating minItems", func(t *testing.T) {
test(t, `"type": "array", "items": { "type": "string" }, "minItems": 3`, func(v url.Values) { v.Add(key, "a"); v.Add(key, "b") }, text.ErrorValidationMinItems, "minimum 3 items allowed, but found 2 items")
})

t.Run("case=array violating uniqueItems", func(t *testing.T) {
test(t, `"type": "array", "items": { "type": "string" }, "uniqueItems": true`, func(v url.Values) { v.Add(key, "abc"); v.Add(key, "XYZ"); v.Add(key, "abc") }, text.ErrorValidationUniqueItems, "items at index 0 and 2 are equal")
})

t.Run("case=wrong type", func(t *testing.T) {
test(t, `"type": "number"`, func(v url.Values) { v.Set(key, "blabla") }, text.ErrorValidationWrongType, "expected number, but got string")
})
})

t.Run("case=should return an error because not passing validation and reset previous errors and values", func(t *testing.T) {
testhelpers.SetDefaultIdentitySchema(conf, "file://./stub/registration.schema.json")

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ describe("Registration failures with email profile", () => {
.should("have.value", "12345678")

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

cy.submitPasswordForm()
cy.get('*[data-testid^="ui/message"]').should(
cy.get('[data-testid="ui/message/4000005"]').should(
"contain.text",
"too similar",
)
Expand Down Expand Up @@ -113,7 +113,8 @@ describe("Registration failures with email profile", () => {
.invoke("text")
.then((text) => {
expect(text.trim()).to.be.oneOf([
'"" is not valid "email"length must be >= 3, but got 0',
'"" is not valid "email"',
"length must be >= 3, but got 0",
"Property email is missing.",
])
})
Expand Down Expand Up @@ -176,7 +177,7 @@ describe("Registration failures with email profile", () => {
cy.get('input[name="traits.website"]').type("http://s")

cy.submitPasswordForm()
cy.get('*[data-testid^="ui/message"]').should(
cy.get('[data-testid="ui/message/4000003"]').should(
"contain.text",
"length must be >= 10",
)
Expand All @@ -190,15 +191,15 @@ describe("Registration failures with email profile", () => {
cy.get('input[name="password"]').then(($el) => $el.remove())

cy.submitPasswordForm()
cy.get('*[data-testid^="ui/message"]').should(
cy.get('[data-testid="ui/message/4000002"]').should(
"contain.text",
"Property website is missing.",
)
cy.get('*[data-testid^="ui/message"]').should(
cy.get('[data-testid="ui/message/4000002"]').should(
"contain.text",
"Property email is missing.",
)
cy.get('*[data-testid^="ui/message"]').should(
cy.get('[data-testid="ui/message/4000002"]').should(
"contain.text",
"Property password is missing.",
)
Expand All @@ -219,7 +220,7 @@ describe("Registration failures with email profile", () => {
cy.get('input[name="traits.age"]').type("600")

cy.submitPasswordForm()
cy.get('*[data-testid^="ui/message"]').should(
cy.get('[data-testid="ui/message/4000020"]').should(
"contain.text",
"must be <= 300 but found 600",
)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Copyright © 2023 Ory Corp
// SPDX-License-Identifier: Apache-2.0

import { APP_URL, appPrefix, gen } from "../../../../helpers"
import { appPrefix, APP_URL, gen } from "../../../../helpers"
import { routes as express } from "../../../../helpers/express"
import { routes as react } from "../../../../helpers/react"

Expand Down Expand Up @@ -130,7 +130,7 @@ context("Registration success with email profile", () => {
cy.longRegisterLifespan()
cy.submitPasswordForm()

cy.get('*[data-testid^="ui/message/"]').should(
cy.get('[data-testid="ui/message/4040001"]').should(
"contain.text",
"The registration flow expired",
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
// SPDX-License-Identifier: Apache-2.0

import { appPrefix, gen, website } from "../../../../helpers"
import { routes as react } from "../../../../helpers/react"
import { routes as express } from "../../../../helpers/express"
import { routes as react } from "../../../../helpers/react"

context("Settings failures with email profile", () => {
;[
Expand Down Expand Up @@ -62,7 +62,7 @@ context("Settings failures with email profile", () => {
it("fails with validation errors", () => {
cy.get('input[name="traits.website"]').clear().type("http://s")
cy.get('[name="method"][value="profile"]').click()
cy.get('[data-testid^="ui/message"]').should(
cy.get('[data-testid="ui/message/4000003"]').should(
"contain.text",
"length must be >= 10",
)
Expand Down 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"]').should(
cy.get('[data-testid="ui/message/4000005"]').should(
"contain.text",
"data breaches",
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
// SPDX-License-Identifier: Apache-2.0

import { appPrefix, gen, website } from "../../../../helpers"
import { routes as react } from "../../../../helpers/react"
import { routes as express } from "../../../../helpers/express"
import { routes as react } from "../../../../helpers/react"

context("Social Sign Up Successes", () => {
;[
Expand Down Expand Up @@ -78,7 +78,7 @@ context("Social Sign Up Successes", () => {
cy.get("#registration-password").should("not.exist")
cy.get('[name="traits.email"]').should("have.value", email)
cy.get('[name="traits.website"]').should("have.value", "http://s")
cy.get('[data-testid="ui/message/4000001"]').should(
cy.get('[data-testid="ui/message/4000003"]').should(
"contain.text",
"length must be >= 10",
)
Expand Down Expand Up @@ -149,7 +149,7 @@ context("Social Sign Up Successes", () => {

cy.triggerOidc(app)

cy.get('[data-testid="ui/message/4000001"]').should(
cy.get('[data-testid="ui/message/4000003"]').should(
"contain.text",
"length must be >= 10",
)
Expand Down
10 changes: 10 additions & 0 deletions text/id.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,16 @@ const (
ErrorValidationNoLookup
ErrorValidationSuchNoWebAuthnUser
ErrorValidationLookupInvalid
ErrorValidationMaxLength
ErrorValidationMinimum
ErrorValidationExclusiveMinimum
ErrorValidationMaximum
ErrorValidationExclusiveMaximum
ErrorValidationMultipleOf
ErrorValidationMaxItems
ErrorValidationMinItems
ErrorValidationUniqueItems
ErrorValidationWrongType
)

const (
Expand Down
Loading

0 comments on commit 9477ea4

Please sign in to comment.