fission-codes / fission

Fission CLI & server
https://runfission.com/docs
119 stars 14 forks source link

CLI: Expose UCAN generation #631

Closed expede closed 1 year ago

expede commented 1 year ago

Changelog:

Overall I tried to keep the change dead simple, especially since this is rewriting in Rust. It's not horrible, but I'm just kind of tossing convenient typeclass instances wherever makes sense etc.

bgins commented 1 year ago

Uses existing keys directly — no need to pass that around via the console

* @bgins this seemed like the easiest/safest thing, but if you want it as an option lemme know

Yeah! This seems like the best approach. 💯 No need to add it as an option.

Overall I tried to keep the change dead simple, especially since this is rewriting in Rust. It's not horrible, but I'm just kind of tossing convenient typeclass instances wherever makes sense etc.

It all reads pretty reasonable to me. And agreed, no need to make it get fancy with it.

A question about the facts option. Would the proper syntax for generating a UCAN with facts be:

fission ucan generate -f '[{"challenge": "abc"}]' -r "*" -p "SUPER_USER" -a "did:key:z6MkgYGF3thn8k1Fv4p4dWXKtsXCnLH7q9yw4QgNPULDmDKB" 

This produces a option -f: Unable to parse facts: "Error in $[0]: Unable to parse UCAN.Fact" error for me, but maybe the facts are formatted incorrectly?

Another question. Are there other changes coming in this PR? It's marked as a draft, but feature-wise it seems ready to go. ✨

expede commented 1 year ago

Another question. Are there other changes coming in this PR? It's marked as a draft, but feature-wise it seems ready to go. ✨

🙌

@bgins it's set as draft because of the commented out lines, but I don't want to comment them back in until I know that you have what you need. I guess I could flip it over to RFR!

Lemme look at the facts stuff 👀

expede commented 1 year ago

@bgins, I fixed the fact object issue

$ fission ucan generate \
    --facts     '[{"foo":"abc"}]' \
    --resource "*" \
    --potency  "SUPER_USER" \
    --audience "did:key:z6MkgYGF3thn8k1Fv4p4dWXKtsXCnLH7q9yw4QgNPULDmDKB"

eyJ1YXYiOiIxLjAuMCIsImFsZyI6IkVkRFNBIiwiY3R5IjpudWxsLCJ0eXAiOiJKV1QifQ.eyJuYmYiOjE2NjY5NjI4NjEsImlzcyI6ImRpZDprZXk6ejZNa3JUWE1zVGV1UjlpVW55TXAzbjVvRDVVZlY0YThyZkJmOTFWOHJNWXN1aWNWIiwicHJmIjpudWxsLCJhdWQiOiJkaWQ6a2V5Ono2TWtnWUdGM3RobjhrMUZ2NHA0ZFdYS3RzWENuTEg3cTl5dzRRZ05QVUxEbURLQiIsImZjdCI6WyJmcm9tTGlzdCBbKFwiZm9vXCIsU3RyaW5nIFwiYWJjXCIpXSJdLCJwdGMiOiJTVVBFUl9VU0VSIiwicnNjIjoiKiIsImV4cCI6MTY2Njk2MjkyMX0.JNb8ZoIynfWpqoEbb6MnsHvQlu3nqcIxzFIZc9u_l5dSOLLydpewVHZzrdeRi4zfY5IW_nOQbtvJ791puRyYCg
bgins commented 1 year ago

@bgins, I fixed the fact object issue

$ fission ucan generate \
    --facts     '[{"foo":"abc"}]' \
    --resource "*" \
    --potency  "SUPER_USER" \
    --audience "did:key:z6MkgYGF3thn8k1Fv4p4dWXKtsXCnLH7q9yw4QgNPULDmDKB"

eyJ1YXYiOiIxLjAuMCIsImFsZyI6IkVkRFNBIiwiY3R5IjpudWxsLCJ0eXAiOiJKV1QifQ.eyJuYmYiOjE2NjY5NjI4NjEsImlzcyI6ImRpZDprZXk6ejZNa3JUWE1zVGV1UjlpVW55TXAzbjVvRDVVZlY0YThyZkJmOTFWOHJNWXN1aWNWIiwicHJmIjpudWxsLCJhdWQiOiJkaWQ6a2V5Ono2TWtnWUdGM3RobjhrMUZ2NHA0ZFdYS3RzWENuTEg3cTl5dzRRZ05QVUxEbURLQiIsImZjdCI6WyJmcm9tTGlzdCBbKFwiZm9vXCIsU3RyaW5nIFwiYWJjXCIpXSJdLCJwdGMiOiJTVVBFUl9VU0VSIiwicnNjIjoiKiIsImV4cCI6MTY2Njk2MjkyMX0.JNb8ZoIynfWpqoEbb6MnsHvQlu3nqcIxzFIZc9u_l5dSOLLydpewVHZzrdeRi4zfY5IW_nOQbtvJ791puRyYCg

@expede the facts look like they aren't serializing to JSON correctly. The decoded payload from that token looks like this:

{
  "nbf": 1666962861,
  "iss": "did:key:z6MkrTXMsTeuR9iUnyMp3n5oD5UfV4a8rfBf91V8rMYsuicV",
  "prf": null,
  "aud": "did:key:z6MkgYGF3thn8k1Fv4p4dWXKtsXCnLH7q9yw4QgNPULDmDKB",
  "fct": [
    "fromList [(\"foo\",String \"abc\")]"
  ],
  "ptc": "SUPER_USER",
  "rsc": "*",
  "exp": 1666962921
}

We are converting the facts to JSON here as Unknown I think:

https://github.com/fission-codes/fission/blob/c5c49a4c9a94196a7617c684187d99d4d5cc3615/fission-core/library/Fission/Web/Auth/Token/UCAN/Fact/Types.hs#L29-L32

No changes there, so perhaps this wasn't working before and we didn't notice it? 🤔

expede commented 1 year ago

No changes there, so perhaps this wasn't working before and we didn't notice it? 🤔

@bgins it's the parseObj on line 46, which is me trying to not add a new case 😅 Sorry I really should have deserialized the output to check; that's very much on me!

expede commented 1 year ago

@bgins updated! Here's the decoded output now:

Screen Shot 2022-10-28 at 19 53 41
bgins commented 1 year ago

@bgins updated! Here's the decoded output now:

Screen Shot 2022-10-28 at 19 53 41

Yeah, that looks good! Tried it out with a few different cases over here and the facts are working great now. 🙌