go-fed / activity

ActivityStreams & ActivityPub in golang, oh my!
BSD 3-Clause "New" or "Revised" License
702 stars 111 forks source link

Separate w3id.org/security/v1 from activitystreams vocab #118

Closed zauberstuhl closed 4 years ago

zauberstuhl commented 4 years ago

I just saw that you added publickey fields for actors with 9b139af4089daa99e62b8214d4907b2cb7291ff7

I think this is a mistake because its not a part of the official implementation but an extension (https://w3id.org/security/v1).

I know that e.g. friendica will not use the publickey fields until you add the right context. Therefore I propose not to mix activitystreams with other extension.

Adding a seperate spec file would be cleaner in my opinion.

The current PR would render following:

{
  "@context": [
    "https://www.w3.org/ns/activitystreams",
    {
      "widsv": "https://w3id.org/security/v1"
    }
  ],
  "followers": "http://localhost:8080/api/v1/ap/actor/lukas/followers",
  "following": "http://localhost:8080/api/v1/ap/actor/lukas/following",
  "id": "http://localhost:8080/api/v1/ap/actor/lukas",
  "inbox": "http://localhost:8080/api/v1/ap/actor/lukas/inbox",
  "outbox": "http://localhost:8080/api/v1/ap/actor/lukas/outbox",
  "preferredUsername": "lukas",
  "publicKey": {
    "id": "http://localhost:8080/api/v1/ap/actor/lukas#publicKey",
    "owner": "http://localhost:8080/api/v1/ap/actor/lukas",
    "publicKeyPem": "-----BEGIN PUBLIC KEY-----\nMIIBCgKCAQEAvrXuxELmzDqqqeLSzxQPPxi8QX2IGAvkY9SY0HeZ0s3/D49CERyd\n5IbCDre10d2Ilko1SIM1ySXD+VH4cK2MuH46lOdgoyGAq//1E+KzfGYHWDuFiMUU\nocnkQwLbmp/HRvEvXbL0VkNvxmN9Dar2ynEO+OvPLOwtALeOXOiZNuvfMmkx7YG0\naYsKz27qw1t2O9bhoOnNK0qMOyN/705cMrr3er1qGAjcf1DadXHzoxUabIA1ATRb\n4+0X89ed0n5YQRw9P7TeUeN4kGjGGJxvJwl+ajzFlx1R6uTwfuPjbqf+EriE1BLv\nGhp89lS6kWva7uVioakRkIOIzorBXnrcmwIDAQAB\n-----END PUBLIC KEY-----\n",
    "type": "PublicKey"
  },
  "type": "Person"
}

The optimal context would look like this:

  "@context": [
    "https://www.w3.org/ns/activitystreams",
    "https://w3id.org/security/v1"
  ],

Not sure how I can get rid of widsv, yet.

Thoughts? Objections?

cjslep commented 4 years ago

I think this is a mistake because its not a part of the official implementation but an extension (https://w3id.org/security/v1).

You're right! thanks for doing this PR!

Not sure how I can get rid of widsv, yet.

Thinking back to when I first coded the astool, I think I default to using aliased extensions, and defaulting ActivityStreams to non-namespaced. This is because of the type/property name collision problem. As far as I am aware, no other ActivityPub software has thought about this though?

It's the right thing to do (not be ambiguous) but if other software has already decided to risk name collisions... we can do something similar, I guess.

zauberstuhl commented 4 years ago

It's the right thing to do (not be ambiguous) but if other software has already decided to risk name collisions... we can do something similar, I guess.

I would say leave it be for the moment. Lets see what other projects think about it: https://talk.feneas.org/t/what-is-the-correct-way-of-setting-the-ap-context/152

cjslep commented 4 years ago

I'm going to merge this and fix the astool to properly propagate the prefix to the properties.

puckipedia commented 4 years ago

This is not actually valid AS2, and does not work on anything that actually processes JSON-LD, like Kroeg.

Links to the JSON-LD Playground:

cjslep commented 4 years ago

Seems like it's just missing the trailing fragment literal #? Is that required in the context? Otherwise should be fine.

puckipedia commented 4 years ago

Not only. The JSON-LD spec indicates that putting a string in the @context list is the same as including the entire object itself; this is not the same as just saying that the same URI is the prefix itself.

cjslep commented 4 years ago

If I understand it right, you're saying each instance of the widsv is like re-including the entire spec (repeatedly, if the alias appears repeatedly)?

puckipedia commented 4 years ago

No, the inverse. Look where the context URI points, and you can see how the terms are built up. That doesn't happen if you put the context as a term alias.

cjslep commented 4 years ago

I don't think I need more aliasing in the context for each individual property. I don't think that's needed based on the compactIRI section and Example 23:

https://json-ld.org/spec/latest/json-ld/#compact-iris https://json-ld.org/spec/latest/json-ld/#example-23-prefix-expansion

cjslep commented 4 years ago

But I am open to being corrected (an example would really help me understand).

puckipedia commented 4 years ago

When expanding using your context, and compacting with a "proper" set of contexts, it gets misinterpreted, see here

puckipedia commented 4 years ago

(oh, and turns out PublicKey isn't a type, oops)

Using just the proper term prefix will work in some cases but can lead to misinterpretations in some other cases (note how it says sec:publicKey instead of just publicKey)

cjslep commented 4 years ago

I've finally sat down at the computer (instead of my phone) and have been playing around with it. I see exactly what you mean, now.

The rest of this is a rant, not to you, but to the Elder Gods. No one should read this cathartic stream of sad and ugly.

Why the golly jeepers can it not recursively apply across different @context definitions? Why the frick does it get to pre-define sec as the alias and now importers of that context are stuck with it and any potential collisions? I get it, I get why, but lord this means importing a bunch of @context spec files is even more dangerous at causing collisions than I thought. How the twiddlesticks is importing publicKey as sec:publicKey plus from "sec": "https://w3id.org/security/v1" and publicKey as other:publicKey plus "other" : "https://example.com/some-other-spec" resolved? I don't think I want to know. That's too much meta code-generating logic to bring to Go, that call of cthulhu I already hear is annoying as heck on its own.

What is the gosh darn solution here

cjslep commented 4 years ago

Also don't get me started on PublicKey, I don't know why it gets to be a typeless dict which goes against everything else I've encountered in the typed AS2 ecosystem. That sucker needs to have a type.

I am this ->||<- close to just saying native static typing plus JSON-LD/AS2 is a no-go cowpatty stain.

puckipedia commented 4 years ago

JSON-LD uses a more sophisticated namespace system than XML, which is kinda nice but also (as you have experienced) kinda annoying. What you can do (and what I do) is to split off everything non-AS2 that you use into your own context URI, which you add to your outgoing documents, see https://puckipedia.com/-/context.json (the .json is to avoid my frontend from taking the request)

And yeah, actually translating JSON-LD objects to static types I have avoided as well, but what I use is a statically-ish typed variant of the node map which is used inside the flattening algorithm; this works surprisingly well, and is super resilient (Kroeg, which is written in Rust, has never had trouble parsing new extensions, the core doesn't even know what AS2 is!)

puckipedia commented 4 years ago

And the last included context wins, for the record. (I implemented the JSON-LD API in both C# and Rust, because I have a severe case of NIHing everything I touch)

cjslep commented 4 years ago

Glad that's worked out for you. Unfortunately for me, I and this library have a severe aversion to actually doing anything JSON-LD related. Clearly one of us took the better approach.

Splitting off a document doesn't change the fundamental problem that a context-user cannot properly alias a vocabulary; that can only be done by the vocabulary owner. What the turd.

puckipedia commented 4 years ago

You can define your own context with the terms you use, obviously. The entire idea is that you can use whatever context, as long as it gets you the same predicate IRIs, but that takes more effort to get right, obviously. And I'm not sure I'd call this the "better" approach, more the "I have too much free time and bad ideas" one, lol

cjslep commented 4 years ago

Great scott I wonder if I can just dig in and redefine the alias term and avoid the context:

{
  "@context": {
    "widsv": "https://w3id.org/security#"
  },
  "widsv:publicKey": {
    "@id": "https://contoso.com/#key",
    "@type": "widsv:PublicKey",
    "widsv:publicKeyPem": "hewwo"
  }
}

...but I'm pretty sure this is really toodling incompatible, frickity lick a duck.

puckipedia commented 4 years ago

Yeah, this is valid in ~most cases you'll probably end up outputting, as long as you don't output raw ID strings (e.g. `"widsv:publicKey": "https://contoso.com/#key"), but that's why I shy from directly saying "you can just do this", people'll probably do it wrong.

cjslep commented 4 years ago

OK, in 00d19306e473675b12c35e366fb5f41eed55c7e0 everything should be parsed nice and lovely now. Had to insert a massive hack to make "PublicKey" a "type" but not really:

{
  "@context": [
    "https://www.w3.org/ns/activitystreams",
    "https://w3id.org/security/v1"
  ],
  "publicKey": {
    "publicKeyPem": "test PEM value"
  },
  "type": "Person"
}