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: report correct errors for json schema validation #3085

Merged
merged 4 commits into from
Feb 10, 2023
Merged
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
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"),
jonas-jonas marked this conversation as resolved.
Show resolved Hide resolved
"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