update api to allow credential-wrapped requests #489
Conversation
dpetran
force-pushed
the
feature/credentials-in-api
branch
from
2b442f2
to
4e901b0
Compare
- stage - query - multiquery - history In order to handle string requests, we need to understand string when discerning whether we're dealing with an insert or a retract, so there are a couple updates to handle that (fql.parse/update? and update/insert? and update/retract?). In each case we unwrap the credential subject before parsing. We only use a limited subset of the credential spec internally, but the wide world of credentials has a very large schema and I didn't want to include it in our internal query/transaction schema.
We were just pulling the "issuer" field off of the verified credential, but that part of the credential is not signed. Now we just use the did key from the proof, which is part of the cryptographic signature. Also refactored the `generate` function and stubbed out an assertion that we can use in the future once our incoming credential subjects are proper json-ld.
If no role is specified, but a did is, look up the roles associated with the identity and enforce them on the operation.
When staging with a policy-activating opt (:did or :role), you need to be able to subsequently transact with different policies for different users. This commit makes that possible by resetting the db to a root db after checking policies. If you want to interact with the db with a specific policy wrapping, you need to specify the identity/role with which you are doing so every time.
dpetran
force-pushed
the
feature/credentials-in-api
branch
from
c644193
to
32a3349
Compare
|
This is ready for a review, I've gone ahead and tied the credential identity into the policy enforcement by utilizing the Some notes: I changed how we handle policy-wrapped dbs. Now, if you provide an option that will activate a policy while transacting (a |
src/fluree/db/json_ld/policy.cljc
Outdated
| (go-try | ||
| (try | ||
| (->> (<? (fql/query db {"select" "?role" | ||
| "where" [[did "https://ns.flur.ee/ledger#role" "?role"]]})) |
There was a problem hiding this comment.
Since this is internal code, I think we should drop down to the lower level index-range to avoid all of the extra parsing and validation machinery in the query system. I think fql/query should be limited to external use.
We'd have to look up the subject id for the did with the dbproto/-subid method, and we'd look up the corresponding sub-id for the role iri which should be in a constants map somewhere (if it's not in a constants map, then we can use dbproto/-subid to look it up too), and then we would do an index-range call against the spot index and that should return a collection of all the roles for that did. That index range call would look something like:
(<? (query-range/index-range db :spot = [did-sid role-iri-sid]))
src/fluree/db/json_ld/policy.cljc
Outdated
| "where" [[did "https://ns.flur.ee/ledger#role" "?role"]]})) | ||
| (not-empty)) | ||
| ;; in case no "f:role" exists | ||
| (catch Exception _)))) |
There was a problem hiding this comment.
I don't think we should be catching and ignoring the most general Exception type here as there could be another unanticipated issue that would be swallowed here. Your comment above suggests this catch block is to handle the case of there not being an f:role property. If that's the case, I think we should catch only that specific exception type and, instead of just ignoring it, we should log it out.
Secondly, this is a cljc file, so we have to also ensure we're handling the javascript case (which try* / catch* does).
Since we know the subject ids for the predicates we care about, we don't need to invoke the whole query pipeline in order to get the results we care about. Cleaned up a redundant ns require for fql. Added some test cases to verify that no errors are thrown when there are no roles or identities.
src/fluree/db/json_ld/policy.cljc
Outdated
| [db sid] | ||
| (let [in-ch (async/chan 2 cat) | ||
| out-ch (async/chan 2 (comp cat (map flake/o))) | ||
| did-roles-ch (query-range/index-range db :spot = [sid const/$_role])] |
There was a problem hiding this comment.
I didn't realize during my last review that roles were operating on iris and not sids. I don't know if I would have made the suggestion had I realized that because of the extra hassle of looking up all those iris, but using index-range is still much faster than running an entire query, so I'll leave it to you to decide which method you go with.
If you do go the index-range route, then I have more suggestions. There is a (sadly undocumented) option to index-range that allows you to pass a transducer to transform all the flakes directly, so you don't have to create an extra channel. You can pass an options map using this arity of the index-range function, and you can specify a transducer to transform all the flakes with the :flake-xf key.
Also, index-range is different from tree-chan in that it is eager and will always return the entire collection of flakes, and it could possibly return an exception if there was any error at any point in its processing, so this is one of those functions that return channels that act like promises and should be accessed with <?. pipeline-async won't work without explicit error handling with an error channel, which will introduce even more complexity.
Instead of introducing error channels and all that, you could do something like this:
(defn roles-for-sid
[db sid]
(go-try
(let [did-role-sids (<? (query-range/index-range db :spot = [sid const/$_role]
{:flake-xf (map flake/o)}))]
(loop [[did-role-sid & r] did-role-sids
did-role-iris []]
(if did-role-sid
(let [did-role-iri (<? (query-range/index-range db :spot = [did-role-sid const/$xsd:anyURI]))]
(recur r (conj did-role-iris did-role-iri)))
did-role-iris)))))There was a problem hiding this comment.
It was bothering me that we are looking up role sids, then looking up role iris, and then in policy-map translating those iris back into sids, so I'm refactoring a bit so that we only pass around sids - only one index-range call is necessary. And with that handy :flake-xf key I think it can be even simpler than what I've got working right now.
I'll take a little longer and see if I can get this bit working more efficiently, but I did notice that we look up all polices with a fql/query call, so maybe in the future we'll have to revisit that as well.
We were looking up an identity's roles, then looking up those roles iris, only to translate them back into iris. This allows us to skip the iri lookups and just keep the sids.
src/fluree/db/json_ld/policy.cljc
Outdated
| @@ -528,10 +514,10 @@ | |||
| {:status 400 | |||
| :error :db/policy-exception})) | |||
|
|
|||
| (not roles) | |||
| (not (not-empty role-sids)) | |||
There was a problem hiding this comment.
I think you can replace (not (not-empty role-sids)) with (empty? role-sids)
| (let [canonicalized #?(:clj (jld-processor/canonize credential-subject) | ||
| :cljs (<p! (jld-processor/canonize credential-subject))) | ||
|
|
||
| ;; TODO: assert this once our credential subjects are proper json-ld |
There was a problem hiding this comment.
Is there a ticket for json-ld credential subjects? If not, could you write one (and put it in the backlog)?
There was a problem hiding this comment.
| {"@context" "https://www.w3.org/2018/credentials/v1" | ||
| "id" "" | ||
| "type" ["VerifiableCredential" "CommitProof"] | ||
| "issuer" (:id did) | ||
| "issuanceDate" (util/current-time-iso) | ||
| "credentialSubject" credential-subject | ||
| "proof" proof})))) | ||
| "proof" proof})))) | ||
|
|
||
| (defn verify | ||
| "Takes a credential and returns the credential subject and issuer id if it verifies. If |
There was a problem hiding this comment.
I know this is unrelated to this pull request, but it looks like this docstring is out of date. Could you update it to say that this function returns nil if there's no signature?
Also, there's a function for (not (not-empty ...)).
In order to handle string requests, we need to understand string when discerning whether we're dealing with an insert or a retract, so there are a couple updates to handle that (fql.parse/update? and update/insert? and update/retract?).
In each case we unwrap the credential subject before parsing. We only use a limited subset of the credential spec internally, but the wide world of credentials has a very large schema and I didn't want to include it in our internal query/transaction schema.
https://github.com/fluree/core/issues/2