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

Add query to list of supported actions for authors/recipients #285

Open
thehenrytsai opened this issue Mar 29, 2023 · 12 comments
Open

Add query to list of supported actions for authors/recipients #285

thehenrytsai opened this issue Mar 29, 2023 · 12 comments
Labels
feature New feature or request

Comments

@thehenrytsai
Copy link
Contributor

thehenrytsai commented Mar 29, 2023

Currently only write is supported.

@thehenrytsai thehenrytsai added the feature New feature or request label Mar 29, 2023
@csuwildcat
Copy link
Member

Should add read to this as well.

@thehenrytsai thehenrytsai moved this to High Priority Backlog in DWN Mar 29, 2023
@thehenrytsai thehenrytsai added this to DWN Mar 29, 2023
@thehenrytsai thehenrytsai changed the title Add query to list of supported actions for protocol-based message authorization Add query and read to list of supported actions for protocol-based message authorization Mar 30, 2023
@thehenrytsai thehenrytsai moved this from High Priority Backlog to In Progress in DWN Apr 24, 2023
@frankhinek frankhinek moved this to In Progress in Web5 Roadmap May 1, 2023
@frankhinek
Copy link
Contributor

Needs to be complete by EOD May 14th at the latest

@csuwildcat
Copy link
Member

I will note that 'completed' here = in a released version of the DWN SDK and Web5 JS, so probably needs to land as a PR well before that.

@diehuxx
Copy link

diehuxx commented May 3, 2023

I've thought about protocol query a bit more. I think I've figured out what the "difficult" part is and I have an idea of how to approach it.

Difficult Part

The current ProtocolAuthorization#authorize code is meant to look at the protocol definition for a single rule set based on the given actor and protocol path. For query, we potentially need to look down multiple protocolPaths to get multiple rule sets. So, calling authorize once at the top of RecordsQuery#handler won't be sufficient. We need to do some authorization for each record returned from the message store query.

Approach

If protocol appears in the RecordsQuery filters: First, do a messageStore#query using all the filters given in the RecordsQuery descriptor. This will give us a list of RecordsWrites. Then, we iterate over each RecordsWrite to get the associated rule set for recordsWrite.descriptor.protocolPath and the actor of the RecordsQuery.

Open Questions

  • What do we return when the RecordsQuery filter does not specify protocol? Do we omit all protocol records? Or include protocol records, checking each protocol definition to see if the record is auth'd for query? If we include all that are allowed, that involves looking through potentially all protocol definitions on the DWN. Not sure if we want that.

@csuwildcat
Copy link
Member

I would require protocol to be set if the protocolPath is set, and not search through protocols at all if protocol isn't set.

@thehenrytsai
Copy link
Contributor Author

Agree that enforcing protocol as a filter will help scope the search of a protocol-authorized query.

Then, we iterate over each RecordsWrite to get the associated rule set for recordsWrite.descriptor.protocolPath and the actor of the RecordsQuery.

This seems super heavy on processing. Is it possible to perhaps "flip" the logic? As in, go through the entire protocol definition hierarchy to find the authorized protocol paths matching the query, then perform a scoped query for each matched protocol path?

Also maybe I misinterpreted it, I vaguely recall @csuwildcat told us a couple days ago that he was okay for a protocol-authorized query to return records of a specific protocol path only? If that's true, then we can just make protocolPath also a required property for protocol-authorized query and it will make our lives much easier.

@csuwildcat
Copy link
Member

I have no issue making protocolPath required for protocol-related queries

@diehuxx
Copy link

diehuxx commented May 4, 2023

As in, go through the entire protocol definition hierarchy to find the authorized protocol paths matching the query, then perform a scoped query for each matched protocol path?

I like this. But you're right that requiring protocolPath would make it a moot point.

I have no issue making protocolPath required for protocol-related queries

Nice. This makes things way easier.

@thehenrytsai
Copy link
Contributor Author

@csuwildcat and @diehuxx, should/can we add an extra requirement that contextId needs to exist in the query also?

@diehuxx
Copy link

diehuxx commented Aug 1, 2023

Adding a note for my own benefit:

Someone at DWN office hours on discord flagged that they have a use case for query and would like this implemented. If someone is the recipient of a doctor record, they should be able to query for patientFiles records, which the doctor is obviously not the recipient of.

I'll update this week or next with a timeline for implementing query.

@csuwildcat csuwildcat changed the title Add query and read to list of supported actions for protocol-based message authorization Add query to list of supported actions for protocol-based message authorization Sep 19, 2023
@csuwildcat
Copy link
Member

csuwildcat commented Sep 21, 2023

I understand the problem here, and at first blush it seems like on write of a record we should index the DIDs that would be able to access it, if we didn't want to do that dynamically.

@diehuxx
Copy link

diehuxx commented Sep 21, 2023

Summary of a discussion just now:

There a few cases to break protocol queries into.

Summary

Add a new index protocolLineage to RecordsWrites that contains all the recordIds of the ancestor tree for a protocol message, with the root recordId at the beginning.

For $globalRole-auth'd queries, we require protocolPath to appear in the RecordsQuery filter. For $contextRole and who: 'author' | 'recipient', we require both protocolPath and contextId to appear in the query.

$globalRole

protocolPath are required in the RecordsQuery filter. if those are not present, return empty list.

  1. Get the protocol rule set for the protocolPath being queried.
  2. Check if there is a role $action rule that matches the $globalRole path.
  3. Check if the queryer has a $globalRole.
  4. If yes to both checks, query the messageStore and return all matching messages. Otherwise, return empty list.

$contextRole

protocolPath and contextId are required in the RecordsQuery filter. if those are not present, return empty list.

  1. Get the protocol rule set for the protocol path being queried.
  2. Check if there is a role $action rule that matches the $contextRole path.
  3. Check if there is a $contextRole in that contextId
  4. If yes to both checks, query the messageStore and return all matching messages. Otherwise, return empty list.

who: 'anyone' | 'recipient' | 'anyone'

This is the most complex and may require further discussion. For now, we aim to implement role-auth'd queries first.
protocolPath is required in the RecordsQuery filter for who: 'anyone' to work. Both protocolPath and contextId are required for who: 'author' | 'recipient' to work.

  1. Get the protocol rule set for the protocol path being queried.
  2. If there is a who: 'anyone' rule, query the message store and return all matches.
  3. If contextId is not present, return empty list. Else, proceed to the next step.
  4. For each who: 'author' | 'recipient' rule
    a. Query message store, filtering by protocolPath: protocolActionRule.of and either author or recipient. Call this matchingAncestorRecords.
    b. Query message store, filtering for messages with matchingAncestorRecords.map((r) => r.recordId) in their ancestry list. Return all matching messages.

@csuwildcat csuwildcat changed the title Add query to list of supported actions for protocol-based message authorization Add query to list of supported actions for authors/recipients Nov 14, 2023
@csuwildcat csuwildcat removed this from Web5 Roadmap Apr 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants