fix: error handling on identity import (#3520)

When importing identities without any traits, or with malformed traits, 500s are returned. This improves the error handling and messaging.

Author: zepatrik

go.mod

@@ -146,7 +146,6 @@ require (
 	github.com/felixge/httpsnoop v1.0.3 // indirect
 	github.com/fsnotify/fsnotify v1.6.0 // indirect
 	github.com/fxamacker/cbor/v2 v2.4.0 // indirect
-	github.com/go-bindata/go-bindata v3.1.2+incompatible // indirect
 	github.com/go-crypt/x v0.2.1 // indirect
 	github.com/go-jose/go-jose/v3 v3.0.0 // indirect
 	github.com/go-logr/logr v1.2.3 // indirect

go.sum

@@ -212,8 +212,6 @@ github.com/fxamacker/cbor/v2 v2.4.0 h1:ri0ArlOR+5XunOP8CRUowT0pSJOwhW098ZCUyskZD
 github.com/fxamacker/cbor/v2 v2.4.0/go.mod h1:TA1xS00nchWmaBnEIxPSE5oHLuJBAVvqrtAnWBwBCVo=
 github.com/ghodss/yaml v1.0.0 h1:wQHKEahhL6wmXdzwWG11gIVCkOv05bNOh+Rxn0yngAk=
 github.com/ghodss/yaml v1.0.0/go.mod h1:4dBDuWmgqj2HViK6kFavaiC9ZROes6MMH2rRYeMEF04=
-github.com/go-bindata/go-bindata v3.1.2+incompatible h1:5vjJMVhowQdPzjE1LdxyFF7YFTXg5IgGVW4gBr5IbvE=
-github.com/go-bindata/go-bindata v3.1.2+incompatible/go.mod h1:xK8Dsgwmeed+BBsSy2XTopBn/8uK2HWuGSnA11C3Joo=
 github.com/go-crypt/crypt v0.2.9 h1:5gWWTId2Qyqs9ROIsegt5pnqo9wUSRLbhpkR6JSftjg=
 github.com/go-crypt/crypt v0.2.9/go.mod h1:JjzdTYE2mArb6nBoIvvpF7o46/rK/1pfmlArCRMTFUk=
 github.com/go-crypt/x v0.2.1 h1:OGw78Bswme9lffCOX6tyuC280ouU5391glsvThMtM5U=
@@ -849,8 +847,6 @@ github.com/ory/nosurf v1.2.7 h1:YrHrbSensQyU6r6HT/V5+HPdVEgrOTMJiLoJABSBOp4=
 github.com/ory/nosurf v1.2.7/go.mod h1:d4L3ZBa7Amv55bqxCBtCs63wSlyaiCkWVl4vKf3OUxA=
 github.com/ory/sessions v1.2.2-0.20220110165800-b09c17334dc2 h1:zm6sDvHy/U9XrGpixwHiuAwpp0Ock6khSVHkrv6lQQU=
 github.com/ory/sessions v1.2.2-0.20220110165800-b09c17334dc2/go.mod h1:dk2InVEVJ0sfLlnXv9EAgkf6ecYs/i80K/zI+bUmuGM=
-github.com/ory/x v0.0.588 h1:3qoC1d7qTKnMLwS3Os7KVbDLeSOUHXON8hUFuGUoScQ=
-github.com/ory/x v0.0.588/go.mod h1:ksLBEd6iW6czGpE6eNA0gCIxO1FFeqIxCZgsgwNrzMM=
 github.com/ory/x v0.0.589 h1:ZNQ+nBzTCm3jI2ZZY/1kGWSE4jEtyvDYWu0ScfLgzac=
 github.com/ory/x v0.0.589/go.mod h1:ksLBEd6iW6czGpE6eNA0gCIxO1FFeqIxCZgsgwNrzMM=
 github.com/pascaldekloe/goe v0.0.0-20180627143212-57f6aae5913c/go.mod h1:lzWF7FIEvWOWxwDKqyGYQf6ZUaNfKdP144TG7ZOy1lc=

identity/.snapshots/TestHandler-case=should_be_able_to_import_users-without_traits.json

@@ -0,0 +1,7 @@
+{
+  "schema_id": "default",
+  "state": "active",
+  "traits": {},
+  "metadata_public": null,
+  "metadata_admin": null
+}

identity/handler.go

@@ -409,7 +409,7 @@ type AdminCreateIdentityImportCredentialsOidcProvider struct {
 func (h *Handler) create(w http.ResponseWriter, r *http.Request, _ httprouter.Params) {
 	var cr CreateIdentityBody
 	if err := jsonx.NewStrictDecoder(r.Body).Decode(&cr); err != nil {
-		h.r.Writer().WriteErrorCode(w, r, http.StatusBadRequest, errors.WithStack(err))
+		h.r.Writer().WriteError(w, r, errors.WithStack(herodot.ErrBadRequest.WithError(err.Error())))
 		return
 	}
 

identity/handler_test.go

@@ -89,9 +89,15 @@ func TestHandler(t *testing.T) {
 	send := func(t *testing.T, base *httptest.Server, method, href string, expectCode int, send interface{}) gjson.Result {
 		t.Helper()
 		var b bytes.Buffer
-		if send != nil {
-			require.NoError(t, json.NewEncoder(&b).Encode(send))
+		switch raw := send.(type) {
+		case json.RawMessage:
+			b = *bytes.NewBuffer(raw)
+		default:
+			if send != nil {
+				require.NoError(t, json.NewEncoder(&b).Encode(send))
+			}
 		}
+
 		req, err := http.NewRequest(method, base.URL+href, &b)
 		require.NoError(t, err)
 		req.Header.Set("Content-Type", "application/json")
@@ -190,7 +196,19 @@ func TestHandler(t *testing.T) {
 			actual, err := reg.PrivilegedIdentityPool().GetIdentityConfidential(ctx, uuid.FromStringOrNil(res.Get("id").String()))
 			require.NoError(t, err)
 
-			snapshotx.SnapshotTExceptMatchingKeys(t, identity.WithCredentialsAndAdminMetadataInJSON(*actual), ignoreDefault)
+			snapshotx.SnapshotT(t, identity.WithCredentialsAndAdminMetadataInJSON(*actual), snapshotx.ExceptNestedKeys(ignoreDefault...))
+		})
+
+		t.Run("without traits", func(t *testing.T) {
+			res := send(t, adminTS, "POST", "/identities", http.StatusCreated, json.RawMessage("{}"))
+			actual, err := reg.PrivilegedIdentityPool().GetIdentityConfidential(ctx, uuid.FromStringOrNil(res.Get("id").String()))
+			require.NoError(t, err)
+
+			snapshotx.SnapshotT(t, identity.WithCredentialsAndAdminMetadataInJSON(*actual), snapshotx.ExceptNestedKeys(ignoreDefault...))
+		})
+
+		t.Run("with malformed traits", func(t *testing.T) {
+			send(t, adminTS, "POST", "/identities", http.StatusBadRequest, json.RawMessage(`{"traits": not valid JSON}`))
 		})
 
 		t.Run("with cleartext password and oidc credentials", func(t *testing.T) {
@@ -276,7 +294,7 @@ func TestHandler(t *testing.T) {
 					actual, err := reg.PrivilegedIdentityPool().GetIdentityConfidential(ctx, uuid.FromStringOrNil(res.Get("id").String()))
 					require.NoError(t, err)
 
-					snapshotx.SnapshotTExceptMatchingKeys(t, identity.WithCredentialsAndAdminMetadataInJSON(*actual), append(ignoreDefault, "hashed_password"))
+					snapshotx.SnapshotT(t, identity.WithCredentialsAndAdminMetadataInJSON(*actual), snapshotx.ExceptNestedKeys(ignoreDefault...), snapshotx.ExceptNestedKeys("hashed_password"))
 
 					require.NoError(t, hash.Compare(ctx, []byte(tt.pass), []byte(gjson.GetBytes(actual.Credentials[identity.CredentialsTypePassword].Config, "hashed_password").String())))
 				})

identity/validator.go

@@ -6,6 +6,10 @@ package identity
 import (
 	"context"
 
+	"github.com/pkg/errors"
+
+	"github.com/ory/herodot"
+
 	"github.com/tidwall/sjson"
 
 	"github.com/ory/kratos/driver/config"
@@ -47,9 +51,13 @@ func (v *Validator) ValidateWithRunner(ctx context.Context, i *Identity, runners
 		return err
 	}
 
+	if len(i.Traits) == 0 {
+		i.Traits = []byte(`{}`)
+	}
+
 	traits, err := sjson.SetRawBytes([]byte(`{}`), "traits", i.Traits)
 	if err != nil {
-		return err
+		return errors.WithStack(herodot.ErrBadRequest.WithError(err.Error()))
 	}
 
 	return v.v.Validate(ctx, s.URL.String(), traits, schema.WithExtensionRunner(runner))

schema/validator.go

@@ -68,7 +68,12 @@ func (v *Validator) Validate(
 		return errors.WithStack(herodot.ErrInternalServerError.WithReasonf("Unable to parse validate JSON object against JSON schema.").WithDebugf("%s", err))
 	}
 
-	if err := schema.Validate(bytes.NewBuffer(document)); err != nil {
+	// we decode explicitly here, so we can handle the error, and it is not lost in the schema validation
+	dec, err := jsonschema.DecodeJSON(bytes.NewBuffer(document))
+	if err != nil {
+		return errors.WithStack(herodot.ErrBadRequest.WithReasonf("Unable to parse validate JSON object against JSON schema.").WithDebugf("%s", err))
+	}
+	if err := schema.ValidateInterface(dec); err != nil {
 		return errors.WithStack(err)
 	}