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

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

Merged
merged 3 commits into from Sep 25, 2019

Conversation

@Zauberstuhl
Copy link

Zauberstuhl commented Sep 16, 2019

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

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?

Lukas Matt added 3 commits Sep 15, 2019
* update go-mod httpsig
./astool/astool -spec astool/activitystreams.jsonld \
  -spec astool/security-v1.jsonld ./streams
@Zauberstuhl Zauberstuhl changed the title Seperate w3id.org/security/v1 from activitystreams vocab Separate w3id.org/security/v1 from activitystreams vocab Sep 16, 2019
@cjslep

This comment has been minimized.

Copy link
Member

cjslep commented Sep 18, 2019

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

This comment has been minimized.

Copy link
Author

Zauberstuhl commented Sep 18, 2019

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

This comment has been minimized.

Copy link
Member

cjslep commented Sep 25, 2019

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

@cjslep cjslep merged commit bf19674 into go-fed:master Sep 25, 2019
@puckipedia

This comment has been minimized.

Copy link

puckipedia commented Sep 27, 2019

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

This comment has been minimized.

Copy link
Member

cjslep commented Sep 27, 2019

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

@puckipedia

This comment has been minimized.

Copy link

puckipedia commented Sep 27, 2019

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

This comment has been minimized.

Copy link
Member

cjslep commented Sep 27, 2019

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

This comment has been minimized.

Copy link

puckipedia commented Sep 27, 2019

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

This comment has been minimized.

Copy link
Member

cjslep commented Sep 27, 2019

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

This comment has been minimized.

Copy link
Member

cjslep commented Sep 27, 2019

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

@puckipedia

This comment has been minimized.

Copy link

puckipedia commented Sep 27, 2019

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

@puckipedia

This comment has been minimized.

Copy link

puckipedia commented Sep 27, 2019

(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

This comment has been minimized.

Copy link
Member

cjslep commented Sep 27, 2019

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

This comment has been minimized.

Copy link
Member

cjslep commented Sep 27, 2019

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

This comment has been minimized.

Copy link

puckipedia commented Sep 27, 2019

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

This comment has been minimized.

Copy link

puckipedia commented Sep 27, 2019

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

This comment has been minimized.

Copy link
Member

cjslep commented Sep 27, 2019

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

This comment has been minimized.

Copy link

puckipedia commented Sep 27, 2019

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

This comment has been minimized.

Copy link
Member

cjslep commented Sep 27, 2019

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

This comment has been minimized.

Copy link

puckipedia commented Sep 29, 2019

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

This comment has been minimized.

Copy link
Member

cjslep commented Oct 9, 2019

OK, in 00d1930 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"
}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.