decentralized-identity / presentation-exchange

Specification that codifies an inter-related pair of data formats for defining proof presentations (Presentation Definition) and subsequent proof submissions (Presentation Submission)
https://identity.foundation/presentation-exchange
Apache License 2.0
85 stars 37 forks source link

erroneous in presentation_definition JSON Schema #280

Closed tlodderstedt closed 3 months ago

tlodderstedt commented 2 years ago

Hi, references

"filter": { "$ref": "http://json-schema.org/schema#" }

in lines 190, 203 and 222 of test/presentation-definition/schema.json cannot be resolved by a JSON schema parser. What is the meaning? I'm asking previous revisions of the schema referred to a filter element in the same schema defined as

"filter": {
      "type": "object",
      "properties": {
        "type": { "type": "string" },
        "format": { "type": "string" },
        "pattern": { "type": "string" },
        "minimum": { "type": ["number", "string"] },
        "minLength": { "type": "integer" },
        "maxLength": { "type": "integer" },
        "exclusiveMinimum": { "type": ["number", "string"] },
        "exclusiveMaximum": { "type": ["number", "string"] },
        "maximum": { "type": ["number", "string"] },
        "const": { "type": ["number", "string"] },
        "enum": {
          "type": "array",
          "items": { "type": ["number", "string"] }
        },
        "not": {
          "type": "object",
          "minProperties": 1
        }
      },
      "required": ["type"],
      "additionalProperties": false
    }
JaceHensley commented 2 years ago

so the "filter" attribute is a JSON Schema object, so you need a json schema for json schema itself. I ran into an issue with that old filter schema because it was missing a few things, and the more I looked into the more I saw was missing. That link resolves to this document:

{
    "$schema": "https://json-schema.org/draft/2020-12/schema",
    "$id": "https://json-schema.org/draft/2020-12/schema",
    "$vocabulary": {
        "https://json-schema.org/draft/2020-12/vocab/core": true,
        "https://json-schema.org/draft/2020-12/vocab/applicator": true,
        "https://json-schema.org/draft/2020-12/vocab/unevaluated": true,
        "https://json-schema.org/draft/2020-12/vocab/validation": true,
        "https://json-schema.org/draft/2020-12/vocab/meta-data": true,
        "https://json-schema.org/draft/2020-12/vocab/format-annotation": true,
        "https://json-schema.org/draft/2020-12/vocab/content": true
    },
    "$dynamicAnchor": "meta",

    "title": "Core and Validation specifications meta-schema",
    "allOf": [
        {"$ref": "meta/core"},
        {"$ref": "meta/applicator"},
        {"$ref": "meta/unevaluated"},
        {"$ref": "meta/validation"},
        {"$ref": "meta/meta-data"},
        {"$ref": "meta/format-annotation"},
        {"$ref": "meta/content"}
    ],
    "type": ["object", "boolean"],
    "$comment": "This meta-schema also defines keywords that have appeared in previous drafts in order to prevent incompatible extensions as they remain in common use.",
    "properties": {
        "definitions": {
            "$comment": "\"definitions\" has been replaced by \"$defs\".",
            "type": "object",
            "additionalProperties": { "$dynamicRef": "#meta" },
            "deprecated": true,
            "default": {}
        },
        "dependencies": {
            "$comment": "\"dependencies\" has been split and replaced by \"dependentSchemas\" and \"dependentRequired\" in order to serve their differing semantics.",
            "type": "object",
            "additionalProperties": {
                "anyOf": [
                    { "$dynamicRef": "#meta" },
                    { "$ref": "meta/validation#/$defs/stringArray" }
                ]
            },
            "deprecated": true,
            "default": {}
        },
        "$recursiveAnchor": {
            "$comment": "\"$recursiveAnchor\" has been replaced by \"$dynamicAnchor\".",
            "$ref": "meta/core#/$defs/anchorString",
            "deprecated": true
        },
        "$recursiveRef": {
            "$comment": "\"$recursiveRef\" has been replaced by \"$dynamicRef\".",
            "$ref": "meta/core#/$defs/uriReferenceString",
            "deprecated": true
        }
    }
}

which itself has references to other files. According to AJV you just need to provide a loadSchema function to resolve external resources. But if there's a better way to do this let's get it figured out :D

tlodderstedt commented 2 years ago

Have you be able to run this construct with any other parser than AJV? We use https://python-jsonschema.readthedocs.io/en/stable/ and I also tried to use the schema with https://www.jsonschemavalidator.net/. It did not work so I had to revert to the previous schema definition in my local copy.

tlodderstedt commented 2 years ago

Is there any appetite to fix this problem?

JaceHensley commented 2 years ago

I think we should definitely fix this. But I do think that having external/hosted schemas is gonna be necessary at some point, thinking about how CM contains a PD for example.

tlodderstedt commented 2 years ago

Externally hosted schemas work if the "consuming" schema refers to a certain element in that schema, like "http://example.org/schema#someelement" (see https://json-schema.org/understanding-json-schema/structuring.html#ref). The way you refer to the external schema is different. I have never seen that construct before.

JaceHensley commented 2 years ago

but what if you don't want to reference some element of hosted schema but the schema itself?

tlodderstedt commented 2 years ago

What would be the semantics of such a reference? The "ref" elements is part of the definition of a schema element, i.e. it defines that element using kind of an alias.

JaceHensley commented 2 years ago

So AJV actually handles this by default, without needing to supply a loadSchema function. But this does seem like it's allowed for a $ref to be a url that doesn't refer to a particular element

tlodderstedt commented 2 years ago

And how does AJV interpret that construct? What is the allowed value range for the filter element?

tlodderstedt commented 2 years ago

@JaceHensley Have you seen my last comment?

JaceHensley commented 2 years ago

With that AJV allows filter be to a a valid JSON schema object ({type: 'object'}, ({oneOf: [...]}, ({type: 'string'}, etc)

bumblefudge commented 2 years ago

DIscussed on today's call - there is appetite to fix, but we're having trouble understanding and replicating! David mentioned that matching against some strings is working, and Jace mentioned that more complex filtering using oneOf don't seem to be allowed. @tlodderstedt , Is there any chance we could get a test/example repo set up for working through examples of failing filters and non-failing filters on a call to debug more systematically?

decentralgabe commented 3 months ago

this should be addressed by specifying Draft 7 here https://github.com/decentralized-identity/presentation-exchange/pull/481

closing this now, please re-open if this solution is insufficient