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

Define the JSON representation as JSON (without excluding directives) #396

Closed
wants to merge 1 commit into from

Conversation

OR13
Copy link
Contributor

@OR13 OR13 commented Sep 16, 2020

Producer

  1. Unknown/not understood properties MUST be preserved (ignored / serialized).
  2. No specific language should be added regarding directives to application/did+json

Consumer

  1. Unknown/not understood properties MUST be preserved (ignored / serialized).
  2. No specific language should be added regarding directives to application/did+json

The JSON section should not reference the JSON-LD representation at all.

https://github.com/microsoft/api-guidelines/blob/vNext/Guidelines.md#61-ignore-rule

For loosely coupled clients where the exact shape of the data is not known before the call, if the server returns something the client wasn't expecting, the client MUST safely ignore it.

Some services MAY add fields to responses without changing versions numbers. Services that do so MUST make this clear in their documentation and clients MUST ignore unknown fields.

https://developers.google.com/knowledge-graph/reference/rest/v1?apix_params=%7B%22query%22%3A%22chuck%20norris%22%7D#http_request

Google Knowledge Graph return JSON-LD with content-type: application/json; charset=UTF-8

https://developers.google.com/knowledge-graph/reference/rest/v1

Azure Digital Twin API returns JSON-LD with content-type application/json

https://docs.microsoft.com/en-us/azure/digital-twins/how-to-manage-routes-apis-cli

JSON Schema has this to say: https://json-schema.org/draft-06/json-schema-core.html

A JSON document is an information resource (series of octets) described by the application/json media type.

In JSON Schema, the terms "JSON document", "JSON text", and "JSON value" are interchangeable because of the data model it defines.

JSON Schema is only defined over JSON documents. However, any document or memory structure that can be parsed into or processed according to the JSON Schema data model can be interpreted against a JSON Schema, including media types like CBOR [RFC7049].

http://json-schema.org/understanding-json-schema/about.html

You may have noticed that the JSON Schema itself is written in JSON. It is data itself, not a computer program. It’s just a declarative format for “describing the structure of other data”. This is both its strength and its weakness (which it shares with other similar schema languages).

https://developer.mozilla.org/en-US/docs/Learn/JavaScript/Objects/JSON

A JSON object can be stored in its own file, which is basically just a text file with an extension of .json, and a MIME type of application/json.

https://tools.ietf.org/html/rfc4627

JSON's design goals were for it to be minimal, portable, textual, and a subset of JavaScript.


Preview | Diff

@OR13 OR13 requested review from msporny, jricher and rhiaro and removed request for jricher September 16, 2020 14:19
Copy link
Member

@msporny msporny left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR is far less complicated than the alternative proposed in #394.

@OR13 OR13 requested review from jricher and removed request for rhiaro September 16, 2020 15:25
Copy link
Contributor

@jricher jricher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not the purpose of the JSON section as written. If a new representation is desired, it should be defined in its own section with its own mime type.

@OR13
Copy link
Contributor Author

OR13 commented Sep 17, 2020

This proposal is compatible with the proposals outlined in: #398

@OR13
Copy link
Contributor Author

OR13 commented Sep 17, 2020

This is not the purpose of the JSON section as written. If a new representation is desired, it should be defined in its own section with its own mime type.

@jricher I am open to doing this, would you oppose application/did+insecure+json ?

@msporny
Copy link
Member

msporny commented Sep 17, 2020

This is not the purpose of the JSON section as written. If a new representation is desired, it should be defined in its own section with its own mime type.

It may come as a surprise to the WG that the intent of the JSON section was to remove processing directives that are valid JSON that leads to loss of data when converting between formats. It may also come as a surprise to the WG that representations must now list "processing directives" that are to be eliminated when converting to the abstract data model. It will certainly come as a surprise to the JSON-LD implementers that they now have to look at the registry and construct the @context array in a way that was not the intent of the original producer... order matters in JSON-LD and this is not a simple processing rule.

If the intent of the section you wrote on JSON was the above, I expect there to be vigorous debate on the two sentences that this PR modifies. Primarily because there are implementers that are going to support both JSON and JSON-LD representations, and some of those implementers do not agree with the two statements this PR is modifying.

My suggestion is that if a DID Method would like to remove known properties such as @context, it can do so in the DID Method specification. This enables those that want to do removal/extra processing the liberty to do that while not affecting other DID Methods that don't want to do that sort of processing. We can probably get to consensus on that proposal.

@jricher
Copy link
Contributor

jricher commented Sep 18, 2020

@msporny your argument for preserving the context data speaks to it not being a property of the did document but of the resolution metadata that processing the representation into the abstract document would produce.

If the context data has to be preserved, then "context" needs to be defined as a property on the DID document itself. As I recall, there was significant pushback against that.

@OR13
Copy link
Contributor Author

OR13 commented Sep 18, 2020

DID Core defines what DID JSON is....if it differs widely from how Google, Microsoft and 99% of developers use JSON, we failed.... there is nothing stopping DID Method Authors from using JSON Schema / whatever they want to massively restrict what can be presented in a did document... in fact... any DID method that does not do this, is creating security issues for everyone who uses it.... If a DID Method author wants to return a DID Document with $schema.... don't make them create a new mime type.... just ignore the property, like you would foo, or bar.

@jricher
Copy link
Contributor

jricher commented Sep 18, 2020

If you ignore properties that add security, that creates security problems, no? These arguments of "it's just JSON" and "this special property needs to be preserved" contradict each other.

@OR13
Copy link
Contributor Author

OR13 commented Sep 18, 2020

If you ignore properties that add security, that creates security problems, no? These arguments of "it's just JSON" and "this special property needs to be preserved" contradict each other.

which property "adds security" ?

The argument is "JSON does not delete properties that people don't understand.... it ignores them".

@jricher
Copy link
Contributor

jricher commented Sep 18, 2020

I'm not saying delete the property, I'm saying that you should never produce the property so it's an error to consume it.

@OR13
Copy link
Contributor Author

OR13 commented Sep 18, 2020

I'm not saying delete the property, I'm saying that you should never produce the property so it's an error to consume it.

if the property is bar or __proto__ is it also an error to consume it?

if we are headed towards restricting JSON representation to "only known good values", I support that, but I expect @selfissued will object.

@msporny
Copy link
Member

msporny commented Sep 18, 2020

if we are headed towards restricting JSON representation to "only known good values", I support that, but I expect @selfissued will object.

I expect many in the group to object to that, myself included -- it eliminates any possibility of local extensibility and forces all extensibility to happen through a centralized registry.

@msporny
Copy link
Member

msporny commented Sep 18, 2020

These arguments of "it's just JSON" and "this special property needs to be preserved" contradict each other.

That's not the argument. The argument is "it's just JSON, and therefore all properties are preserved." Eliminating arbitrary properties is something that JSON does not do. It will come as a surprise to developers.

There is a lot of talking past each other on this topic. I'm going to try and summarize some assertions I've heard on the calls/discussion threads to see if it changes anyone's understanding of the issues at hand:

  1. Multiple DID Method implementers are preserving @context in application/json today -- that is unlikely to change as that has been deployed behavior both inside and outside of the DID Ecosystem for years now (see schema.org).

  2. Focusing on @context is neither here nor there, it's a symptom of the problem. The real problem is stating that there is a fixed set of properties that must be removed when converting to/from the abstract data model (this is the point of contention). For example, JSON-LD does not forbid using JSON Schema at the top level... so, does the JSON-LD representation now need to specify that $schema at the top level is allowed and if it does that, does the JSON representation state that it is not allowed? A simpler rule would be to "Always preserve when going to/from the abstract data model if possible."

  3. One of the problems that has been identified is DID Core asserting that it will remove properties when transforming to/from a particular representation... whether or not to remove properties is ultimately a DID Method decision (that's the only place where you can concretely make the security decision, if there is one to be made). So, if you want to remove properties, specify in your DID Method that they should be removed and when they should be removed. This would allow everyone to continue doing what they want to do -- if you want to remove @context, or $schema, or "all unknown properties", you can do so in your DID Method specification.

  4. Another problem that has been highlighted is that some conversions may not be possible -- for example if the UNKNOWN input property name is a CBOR number and the property value is a CBOR tagged bytes and the output representation is JSON. Since we use Infra, we can preserve the CBOR property name and tagged bytes in the abstract data model (Infra supports ints as keys and bytes as values), but the serialization to JSON will fail because JSON can't represent those types of things if we don't make it clear how to convert that into something JSON can understand. Not being able to convert an unknown property into a target representation from the abstract data model is a valid time to throw an error. We should state this clearly in the specification and provide warnings just like the JSON specification provides for Numbers and what sorts of ranges are likely to be interoperable.

@peacekeeper
Copy link
Contributor

peacekeeper commented Sep 18, 2020

I'm not saying delete the property, I'm saying that you should never produce the property so it's an error to consume it.

This resonates with me. There is a lot of talk about "dropping" or "removing" or "preserving" @context or $schema or whatever. But those are not properties of the DID subject (see w3c/did-extensions#131). There is no @context "property" in the abstract data model. It only shows up in the application/did+ld+json representation because of the production rules of that specific representation. But it shouldn't show up in any other representations.

@OR13
Copy link
Contributor Author

OR13 commented Sep 19, 2020

I'm not saying delete the property, I'm saying that you should never produce the property so it's an error to consume it.

This resonates with me. There is a lot of talk about "dropping" or "removing" or "preserving" @context or $schema or whatever. But those are not properties of the DID subject (see w3c/did-spec-registries#131). There is no @context "property" in the abstract data model. It only shows up in the application/did+ld+json representation because of the production rules of that specific representation. But it shouldn't show up in any other representations.

My point is that telling JSON developers what they can or can't produce, in their JSON... is the path to rampant conformance test failure... and remains a terrible idea.

I agree, a JSON developer who intends to not support any other representation SHOULD NOT produce an @context in a did document.

It's also the case that if they do, they still have a JSON document.... we can decide to throw an error, and treat DID JSON differently than vanilla JSON.... or we can not throw an error.

I am in favor of not throwing extra errors, especially when they are completely not expected...

I am in favor of all these representations being valid: https://github.com/transmute-industries/did-core/tree/master/packages/did-representation-examples/json

AND

all these representations being valid: https://github.com/transmute-industries/did-core/tree/master/packages/did-representation-examples/jsonld

Note this one in particular: https://github.com/transmute-industries/did-core/blob/master/packages/did-representation-examples/jsonld/didDocument.xml

@context is actually not allows in XML.... so parsers throw an error.

@context is allowed in JSON, which is why you can use .json or .jsonld for a file that contains it, and parsers don't throw errors.

https://sucuri.net/guides/owasp-top-10-security-vulnerabilities-2020/

How to Prevent Insecure Deserializations

The best way to protect your web application from this type of risk is not to accept serialized objects from untrusted sources.

If you can’t do this, OWASP security provides more technical recommendations that you (or your developers) can try to implement:

  • Implementing integrity checks such as digital signatures on any serialized objects to prevent hostile object creation or data tampering.
  • Enforcing strict type constraints during deserialization before object creation as the code typically expects a definable set of classes. Bypasses to this technique have been demonstrated, so reliance solely on this is not advisable.
  • Isolating and running code that deserializes in low privilege environments when possible.
  • Logging deserialization exceptions and failures, such as where the incoming type is not the expected type, or the deserialization throws exceptions.
  • Restricting or monitoring incoming and outgoing network connectivity from containers or servers that deserialize.
  • Monitoring deserialization, alerting if a user deserializes constantly.

The point I am making is that NO EXCEPTION is thrown when parsing JSON with property members that are allowed in JSON.

If we decide that we are going to throw errors for JSON membership we are going to enter a state where some developers do the extra unnecessary work that did core suggests and most don't.... thats called attack surface... and we are creating it by not understanding the default behavior of JSON, which is to not throw errors when "unknown" properties are present.... to a JSON developer who has never read the did core spec or JSON-LD... @context is just another "unknown" property, like "foo"... to developers that have read both did core and json-ld, they know what it is and they can decide if they care about it... regardless of if its present in a response with content-type application/json or application/ld+json.... in fact, many people prefer to use application/json for JSON-LD... including Google and Microsoft.

consider https://github.com/decentralized-identity/universal-resolver/blob/7d7fd7c7696a8a68f88e0f88660e1a6e0c1bdb79/resolver/java/README.md#local-resolver

DIDDocument didDocument1 = uniResolver.resolve("did:sov:WRfXPg8dantKVubE3HX8pw").getDidDocument();
System.out.println(didDocument1.toJson());

@peacekeeper will you be adding a toJsonLd() call here?... will everyone else in the world also be adding a separate serialization call for JSON-LD?

Consider:

GET https://dev.uniresolver.io/1.0/identifiers/did%3Asov%3AWRfXPg8dantKVubE3HX8pw

With response:

content-type: application/ld+json;profile="https://w3c-ccg.github.io/did-resolution/";charset=utf-8

Any yet, no @context in the outer object... and the inner one contains a broken link: https://www.w3.org/2019/did/v1

{
  "didDocument": {
    "@context": "https://www.w3.org/2019/did/v1",
    "id": "did:sov:WRfXPg8dantKVubE3HX8pw",
    "service": [
      {
        "type": "agent",
        "serviceEndpoint": "https://agents.danubeclouds.com/agent/WRfXPg8dantKVubE3HX8pw"
      },
      {
        "type": "xdi",
        "serviceEndpoint": "https://xdi03-at.danubeclouds.com/cl/+!:did:sov:WRfXPg8dantKVubE3HX8pw"
      }
    ],
    "authentication": [
      {
        "type": "Ed25519SignatureAuthentication2018",
        "publicKey": ["did:sov:WRfXPg8dantKVubE3HX8pw#key-1"]
      }
    ],
    "publicKey": [
      {
        "id": "did:sov:WRfXPg8dantKVubE3HX8pw#key-1",
        "type": "Ed25519VerificationKey2018",
        "publicKeyBase58": "H3C2AVvLMv6gmMNam3uVAjZpfkcJCwDwnZn6z3wXmqPV"
      }
    ]
  },
  "content": null,
  "contentType": null,
  "resolverMetadata": {
    "duration": 404,
    "identifier": "did:sov:WRfXPg8dantKVubE3HX8pw",
    "driverId": "driver-universalresolver/driver-did-sov",
    "didUrl": {
      "didUrlString": "did:sov:WRfXPg8dantKVubE3HX8pw",
      "did": {
        "didString": "did:sov:WRfXPg8dantKVubE3HX8pw",
        "method": "sov",
        "methodSpecificId": "WRfXPg8dantKVubE3HX8pw",
        "parseTree": null,
        "parseRuleCount": null
      },
      "parameters": null,
      "parametersMap": {},
      "path": "",
      "query": null,
      "fragment": null,
      "parseTree": null,
      "parseRuleCount": null
    }
  },
  "methodMetadata": {
    "network": "_",
    "poolVersion": 2,
    "nymResponse": {
      "result": {
        "reqId": 1.6005277792004823e18,
        "txnTime": 1.541709495e9,
        "state_proof": {
          "proof_nodes": "+QUt+FGAgICAgICAgKDvFfrMCOLpuL9giztlBG11bvJ0QZwp8RL6jWVZ0eaZY6B7KxPxoJBwPU9pj4Z1cY56ncndZBdj4M025Lv6uN1+r4CAgICAgID5AbGgDfZhlmsSLPpH8h+4QX/FJ0LhDPpa6gkgtGXJE+/GYy2gnJJhtvUXvaFlBL7C4NQvzxFDtdjJBoaiUPe/7s04tAiAoBPtvrC1K0vNz+comkShJEPjkI9D2teqtMocYTHOgGVkoIt9bbwvIPhgyMCIf5OGLOGKzpolAszUYt3yvW0Kdh6DoOPB7mcQpCz988/duQmQQuOP43tSAcRtNfZZsZWY+qf2oLm+GDuZiKKsofr4WP1wXz7t4ms9nJrpNCYO7i39x0mfoO1rPqrXvuZ7KEYzVjb31Cqc2oJ3xb2lCmiDD+GtfMEkoKv/IHK4BZKua956Aty4njJPZ8yMzj8LPp32nC06xpBzoGg7ewjsw2putdbSL47OXRZx0C0oNoZt0MgP2PAXEwWXoCKSp9l/cTkiAKfJ2qJrNBVnI32id+s1h/ucj8Z87o2IgKCAQnQwmv77fE25HGqib0hcwNlxdYUbtfP1tacVe12O36DudbhfP/7QWc22C4CW1LsR0KFUPVUDxG3rNUe7IyTa4aCVygnlNLjmErhhbpqap9IPm2A+oIb4oYJH2fkiv2SgnYCA+J2fIEUXWXmiOzUjXAl+0g1WTcital73zH8ZtT6Dg4YBLbh7+Hm4d3siaWRlbnRpZmllciI6IkJyWURBNU51YmVqRFZIa0NZQmJwWTUiLCJyb2xlIjpudWxsLCJzZXFObyI6MTA0LCJ0eG5UaW1lIjoxNTQxNzA5NDk1LCJ2ZXJrZXkiOiJ+UDdGM0JOczVWbVE2ZVZwd2tOS0o1RCJ9+HGAgICgQDj+RMw3oDQR033N2xB1faB3Lt7tmCG8NxKXJ2k8yxyAgICAgICgTVGRVdLjRSHPrDOfuBUcTzb6TNl/RmE9Nvfh6urMYJCAgICAoIV9l2oQ++sRf52AZfu2vTUsv34IPmvoVIVOaoZsv7bdgPkCEaC41Lk6ufV+6e7Ix/mSJJeR2/EGvgJM15+vCCJmMQcfVKDutuyiL2waqRwyyvSGiJUWZHdz269vQq8ffPwjQevRJ6Cbxqm5SIcbJRPub4s7HI9/eHQG/ZHOpkO7zerH0W5LF6C/F1jNxuyPqhlG0Y2KUOC9Zqm8I0RLjC74k6MjrooCB6AlSH/yMgGTOI9uSifFSxaHnWFaeBIxYkPrGWTrBNEJd6CwXFjwMUKPR80gnQ9A4IwoSTXAe0lNQZ55r6e8JTStlaBd+WLVCZmydA/TpsM3idMe+AyRT0b6vHmK+ejUvHaYaqBz4MPdRW7RimjZQCKox0pptP5662qlOOv69WoYwbLECqAshdWvX3iLxV30APLP5zw59ytFiGBCQ3eV945ZpYI0WaDYLFZ+ULpb8Yqy7pK4Jb73mgDA40Eq+ONHle+ofyLSMaCYkcMSRUf5KD6AGQ1SfqaMwf9Y3Y1hOqeUTtQ8Y2gsuqDAbGKjImGhk1qLdhOF4U1DLNsZZI8944K86K4RWBG7bqACqCGhaFjQsb3hZxm5t8kcGJYpL80UyAnzmuKM4QROIaBZfm0CS4U715XaHDhfjY6QVs/icU7cM8mgPAX6iV2aUqBj1lMErO95R0tBGewmjzYQemgmnu4qUqQu3dLVHzOPIaBkJoFzPkKjlTOTunHWL5qRBcwTs2NLdgYHkXd5WKYkwoA=",
          "multi_signature": {
            "participants": [
              "atbsovrin",
              "sovrin.sicpa.com",
              "royal_sovrin",
              "esatus_AG",
              "BIGAWSUSEAST1-001",
              "Stuard",
              "VeridiumIDC",
              "pcValidator01",
              "findentity",
              "icenode",
              "OASFCU",
              "danube",
              "ServerVS"
            ],
            "value": {
              "txn_root_hash": "J9sz4AWRXMcxrgsd34EnJuWEWirxhF6ff6hoi7qnrJwF",
              "ledger_id": 1.0,
              "state_root_hash": "Anp3F94brTRsEXeBzuc6UT3dEbCNRmtWqWjfzZSGq2b2",
              "timestamp": 1.600527564e9,
              "pool_state_root_hash": "4Asf2cSvxUguXoSsxHxbKSmgXHiaNgWWYR5uLj35BiuA"
            },
            "signature": "QqicX8XBy1muxoEmEG8FGUtt9HJsJjKhwFogwmaYJuf9sMjY28cYQF5ZxFGiWfH1B3oTmazmPPdjsGocKRd2V3D7nf7Vi8YZ5BLQTzgqSvR5ZdPZM6sL75hUdQEycdMknM2dRbKpdg1JMm2Z4KsqRTabQ31R81a6RPjwJFBdPsPdMR"
          },
          "root_hash": "Anp3F94brTRsEXeBzuc6UT3dEbCNRmtWqWjfzZSGq2b2"
        },
        "seqNo": 104.0,
        "type": "105",
        "dest": "WRfXPg8dantKVubE3HX8pw",
        "identifier": "CLxfRVymK6dP4vZTXpV9Nx",
        "data": "{\"dest\":\"WRfXPg8dantKVubE3HX8pw\",\"identifier\":\"BrYDA5NubejDVHkCYBbpY5\",\"role\":null,\"seqNo\":104,\"txnTime\":1541709495,\"verkey\":\"~P7F3BNs5VmQ6eVpwkNKJ5D\"}"
      },
      "op": "REPLY"
    },
    "attrResponse": {
      "result": {
        "raw": "endpoint",
        "reqId": 1.60052777951781837e18,
        "txnTime": 1.504719117e9,
        "state_proof": {
          "proof_nodes": "+QVU+QGxoDRPgPflKd7QIK5Fo8EN3M6KqQxys3VaCRHr9z28QilNoEbRzz6Wr1R1qFyOQV6y2ytzjrUseBFVOg55n0+O8pogoO1nmmsef0kbAFGiDExjMotT5l5uVgCYPlaabjuH0uuVoL1bXkghDc6QgJfv0/llNge5P6RVfawF69+SrEMlwuAdoDbwXd3nt+tG0kBJczaxyMM28fYvJJlK0qF4C4JQlVNhoADwmtsGbpyFi96Ty2f96C1A9//35/YO/mVcQz0MR2BroCvNLAroB75vbB7mht16k6Y1xbDuIPw4lDuVYCqesdzToGaukDt9Enwhjew6xEP1GP4mc1/3M2EDq9spHX7aO1RzoIWeQZEF0w+/c7mLP524wEbbIWoJLEOFLsLkA9DQkoAfoMhz+9gXpLrC+N/kYXvhOTLBWwUewdjrYABXF6f3qdZJgKC3INkSRTc1C6tWotIkuGJniB1QpW/5yFSjiZsZ/PJpfYCgnYYIbdQasQ1+tBO4RkCNrytynSgCl7P9EtcL/Q65aDiAoD0IhExdIKfNUBiMlvRqekKIgX18IncVRXyaJSZVa+QWgPjEuFggZlhQZzhkYW50S1Z1YkUzSFg4cHc6MTpiNmJmN2JjOGQ5NmYzZWE5ZDEzMmM4M2IzZGE4ZTc3NjBlNDIwMTM4NDg1NjU3MzcyZGI0ZDZhOTgxZDNmZDlluGj4ZrhkeyJsc24iOjIxLCJsdXQiOjE1MDQ3MTkxMTcsInZhbCI6ImI3NjY5YmJmNTZjZjAwNGQ3NTBjMmMyYTJmMmFmYzg3NWM3N2RmNmQ4MGY0YTIzNDk5ZDMyNDVhZmEwM2JkM2IiffhRgICgOtaMWXJKXlq2j6JXA2Sk1M2C5mkKV985NJiz8S1lJZ2AgICAgICAoOdb6wLBOYg6WUaC6OZUiMSW9cQZycoYy7kUzyVKQV4vgICAgICA+HGAgICAoAWdZXUi7Ay4ihzsukSfWwBLHN+Mnf2ez1gmIM8quPgnoO20JJ60U3LEtKUpi4xdM8B5rI6eAyR1uwNUVQdjFyKXgKAcPVvH3KJM15XUG84TDsEXXsYUdBOdicjAEQfQKlFhN4CAgICAgICAgPkCEaC41Lk6ufV+6e7Ix/mSJJeR2/EGvgJM15+vCCJmMQcfVKDutuyiL2waqRwyyvSGiJUWZHdz269vQq8ffPwjQevRJ6Cbxqm5SIcbJRPub4s7HI9/eHQG/ZHOpkO7zerH0W5LF6C/F1jNxuyPqhlG0Y2KUOC9Zqm8I0RLjC74k6MjrooCB6AlSH/yMgGTOI9uSifFSxaHnWFaeBIxYkPrGWTrBNEJd6CwXFjwMUKPR80gnQ9A4IwoSTXAe0lNQZ55r6e8JTStlaBd+WLVCZmydA/TpsM3idMe+AyRT0b6vHmK+ejUvHaYaqBz4MPdRW7RimjZQCKox0pptP5662qlOOv69WoYwbLECqAshdWvX3iLxV30APLP5zw59ytFiGBCQ3eV945ZpYI0WaDYLFZ+ULpb8Yqy7pK4Jb73mgDA40Eq+ONHle+ofyLSMaCYkcMSRUf5KD6AGQ1SfqaMwf9Y3Y1hOqeUTtQ8Y2gsuqDAbGKjImGhk1qLdhOF4U1DLNsZZI8944K86K4RWBG7bqACqCGhaFjQsb3hZxm5t8kcGJYpL80UyAnzmuKM4QROIaBZfm0CS4U715XaHDhfjY6QVs/icU7cM8mgPAX6iV2aUqBj1lMErO95R0tBGewmjzYQemgmnu4qUqQu3dLVHzOPIaBkJoFzPkKjlTOTunHWL5qRBcwTs2NLdgYHkXd5WKYkwoA=",
          "multi_signature": {
            "participants": [
              "atbsovrin",
              "sovrin.sicpa.com",
              "royal_sovrin",
              "esatus_AG",
              "BIGAWSUSEAST1-001",
              "Stuard",
              "VeridiumIDC",
              "pcValidator01",
              "findentity",
              "icenode",
              "OASFCU",
              "danube",
              "ServerVS"
            ],
            "value": {
              "txn_root_hash": "J9sz4AWRXMcxrgsd34EnJuWEWirxhF6ff6hoi7qnrJwF",
              "ledger_id": 1.0,
              "state_root_hash": "Anp3F94brTRsEXeBzuc6UT3dEbCNRmtWqWjfzZSGq2b2",
              "timestamp": 1.600527564e9,
              "pool_state_root_hash": "4Asf2cSvxUguXoSsxHxbKSmgXHiaNgWWYR5uLj35BiuA"
            },
            "signature": "QqicX8XBy1muxoEmEG8FGUtt9HJsJjKhwFogwmaYJuf9sMjY28cYQF5ZxFGiWfH1B3oTmazmPPdjsGocKRd2V3D7nf7Vi8YZ5BLQTzgqSvR5ZdPZM6sL75hUdQEycdMknM2dRbKpdg1JMm2Z4KsqRTabQ31R81a6RPjwJFBdPsPdMR"
          },
          "root_hash": "Anp3F94brTRsEXeBzuc6UT3dEbCNRmtWqWjfzZSGq2b2"
        },
        "seqNo": 21.0,
        "type": "104",
        "dest": "WRfXPg8dantKVubE3HX8pw",
        "identifier": "CLxfRVymK6dP4vZTXpV9Nx",
        "data": "{\"endpoint\":{\"agent\":\"https://agents.danubeclouds.com/agent/WRfXPg8dantKVubE3HX8pw\",\"xdi\":\"https://xdi03-at.danubeclouds.com/cl/+!:did:sov:WRfXPg8dantKVubE3HX8pw\"}}"
      },
      "op": "REPLY"
    }
  }
}

Why not just return application/json, wouldn't that be better than claiming to support JSON-LD, but returning stuff that will explode when you actually try and use it? I even asked for vanilla JSON.... accept: application/json, text/plain, */*

Asking for JSON should yield JSON.... asking for JSON-LD should yield JSON-LD....

JSON DID Documents SHOULD NOT have an @context... and IMO, there are a lot of other properties they SHOULD NOT have... like __proto__.... the choice of what JSON goes in a DID Document is up to the DID Method Author... as is clearly demonstrated with example above.... We cannot force DID Method Authors to produce valid JSON-LD... and similarly, We cannot force DID Method Authors to produce JSON which complies with a deny-list this group cooks up in a misguided attempt to redefine what JSON is.

My recommendation for any DID Method authors reading this in the future trying to support JSON is review OWASP, use something like JSON Schema, don't allow arbitrary unknown JSON members (this can lead to arbitrary code execution), and most of all, don't use a deny-list... use an allow-list and consider anything else denied by default.

@peacekeeper
Copy link
Contributor

The JSON section should not reference the JSON-LD representation at all.

I think I agree with this and also agree with merging the PR, but for different reasons than @OR13 .

If I understand correctly, @OR13 wants to be able to add @context to plain JSON representations and even to YAML and other non-JSON representations, which I think is a bad idea. Only the production rules for JSON-LD should add @context.

Therefore, since the production rules for JSON do not add an @context anyway, it feels right to me to not even mention @context in this section.

@peacekeeper
Copy link
Contributor

@peacekeeper will you be adding a toJsonLd() call here?... will everyone else in the world also be adding a separate serialization call for JSON-LD?

@OR13 we should discuss Universal Resolver and other concrete implementations elsewhere, but yes the plan is of course to support multiple representations (JSON-LD, JSON, ...), and yes you're right that there's a missing @context in the outer resolution result object (that's an implementation bug).

@peacekeeper peacekeeper mentioned this pull request Oct 1, 2020
@msporny
Copy link
Member

msporny commented Oct 1, 2020

This PR is still under debate... we had a special topic call to try to resolve some things:

https://www.w3.org/2020/09/29-did-topic-minutes.html

@msporny msporny added the do not merge Do not merge - waiting on resolution to issue label Oct 12, 2020
@msporny msporny added the needs special call Needs a special topic call to make progress label Oct 26, 2020
@rhiaro
Copy link
Member

rhiaro commented Nov 16, 2020

I believe this is now superceded by #454 and #455

@rhiaro rhiaro added the pending close Issue will be closed shortly if no objections label Nov 16, 2020
@peacekeeper
Copy link
Contributor

+1 to @rhiaro this is superceded by #454, which includes the exact same changes as this PR and some more.

@msporny
Copy link
Member

msporny commented Dec 1, 2020

With PR #469 and #455 merged, and with #454 on track to being merged, those three PRs address what this PR was attempting to define. There is no way this PR is going to go into the spec at this point and it has been marked pending close for 15 days. Closing this PR given that it has been overtaken by events.

@msporny msporny closed this Dec 1, 2020
@msporny msporny deleted the feat/let-json-be-json-with-directives branch December 6, 2020 21:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do not merge Do not merge - waiting on resolution to issue needs special call Needs a special topic call to make progress pending close Issue will be closed shortly if no objections
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants