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

[WIP] SyncEngine for specific protocols + delegate sync. #836

Merged
merged 17 commits into from
Aug 23, 2024

Conversation

LiranCohen
Copy link
Member

@LiranCohen LiranCohen commented Aug 13, 2024

Resolves #826

This PR adds the ability to Sync a subset of data by providing specific protocols a user would like to sync. Additionally sync now supports using a grant and delegateDid to perform sync.

Introduced a CachedPermissions class to help cache the lookup of permission grants records from the DWN. This is used both in DwnApi and SyncEngineLevel.

Copy link

changeset-bot bot commented Aug 13, 2024

🦋 Changeset detected

Latest commit: 96944b7

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 5 packages
Name Type
@web5/agent Minor
@web5/identity-agent Minor
@web5/proxy-agent Minor
@web5/user-agent Minor
@web5/api Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@LiranCohen LiranCohen force-pushed the lirancohen/grants-api branch 2 times, most recently from 090dd73 to 2b0eda3 Compare August 14, 2024 02:46
Base automatically changed from lirancohen/grants-api to lirancohen/grants-web5 August 14, 2024 03:03
@LiranCohen LiranCohen force-pushed the lirancohen/grants-sync branch 2 times, most recently from ed8a9e4 to 85ec6a5 Compare August 14, 2024 14:50
Copy link

codecov bot commented Aug 19, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 93.35%. Comparing base (34590b2) to head (96944b7).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #836      +/-   ##
==========================================
+ Coverage   93.27%   93.35%   +0.07%     
==========================================
  Files         115      116       +1     
  Lines       32447    32691     +244     
  Branches     2547     2598      +51     
==========================================
+ Hits        30266    30518     +252     
+ Misses       2143     2134       -9     
- Partials       38       39       +1     
Components Coverage Δ
agent 87.20% <100.00%> (+0.31%) ⬆️
api 99.53% <100.00%> (+<0.01%) ⬆️
common 98.68% <ø> (ø)
credentials 94.95% <ø> (ø)
crypto 93.79% <ø> (ø)
dids 97.77% <ø> (ø)
identity-agent 96.42% <ø> (ø)
crypto-aws-kms 100.00% <ø> (ø)
proxy-agent 96.42% <ø> (ø)
user-agent 96.42% <ø> (ø)

Base automatically changed from lirancohen/grants-web5 to main August 19, 2024 16:51
Copy link
Contributor

github-actions bot commented Aug 19, 2024

TBDocs Report

🛑 Errors: 0
⚠️ Warnings: 5

@web5/api

  • Project entry file: packages/api/src/index.ts

@web5/crypto

  • Project entry file: packages/crypto/src/index.ts
📄 File: ./packages/crypto/src/utils.ts
⚠️ extractor:typedoc:missing-docs: CryptoUtils (Variable) does not have any documentation.
⚠️ extractor:typedoc:missing-docs: CryptoUtils.__type.randomPin (Property) does not have any documentation.
⚠️ extractor:typedoc:missing-docs: CryptoUtils.__type.randomUuid (Property) does not have any documentation.
⚠️ extractor:typedoc:missing-docs: CryptoUtils.__type.randomBytes (Property) does not have any documentation.
⚠️ extractor:typedoc:missing-docs: CryptoUtils.__type.getJoseSignatureAlgorithmFromPublicKey (Property) does not have any documentation.

@web5/crypto-aws-kms

  • Project entry file: packages/crypto-aws-kms/src/index.ts

@web5/dids

  • Project entry file: packages/dids/src/index.ts

@web5/credentials

  • Project entry file: packages/credentials/src/index.ts

TBDocs Report Updated at 2024-08-23T18:02:49Z 96944b7

@LiranCohen LiranCohen force-pushed the lirancohen/grants-sync branch 3 times, most recently from 5c9bfb1 to 01b5606 Compare August 22, 2024 00:27
@LiranCohen LiranCohen marked this pull request as ready for review August 22, 2024 01:03
This refactors a lot of what's in #824 with regards to creating/fetching grants.

Satisfies: #827

Introduces a `PermissionsApi` interface and an `AgentPermissionsApi` concrete implementation.

The interface implements the following methods `fetchGrants`, `fetchRequests`, `isGrantRevoked`, `createGrant`, `createRequest`, `createRevocation` as convenience methods for dealing with the built-in permission protocol records.

The `AgentPermissionsApi` implements an additional static method `matchGrantFromArray` which was moved from a `PermissionsUtil` class, which is used to find the appropriate grant to use when authoring a message.

A Private API usedin a connected state to find and cache the correct grants to use for the request.

A Permissions API which implements `request`, `grant`, `queryRequests`, and `queryGrants` that a user can utilize

The `Web5` permissions api introduces 3 helper classes to represent permissions:
 Class to represent a permission request record. It implements convenience methods similar to the `Record` class where you can `store()`, `import()` or `send()` the underlying request record. Additionally a `grant()` method will create a `PermissionGrant` object.

 Class to represent a grant record. It implements convenience methods similar to the `Record` class where you can `store()`, `import()` or `send()` the underlying grant record. Additionally a `revoke()` method will create a `GrantRevocation` object, and `isRevoked()` will check if the underlying grant has been revoked.

 Class to represent a permission grant revocation record. It implements convenience methods similar to the `Record` class where you can `store()`  or `send()` the underlying revocation record.
@@ -74,17 +93,19 @@ export class SyncEngineLevel implements SyncEngine {

set agent(agent: Web5PlatformAgent) {
this._agent = agent;
this._cachedPermissionsApi = new CachedPermissions({ agent: agent as Web5Agent, cachedDefault: true });
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if agent is set to Web5PlatformAgent in CachedPermissions would it fix needing to cast this?

}
});

let reply: MessagesReadReply;

try {
reply = await this.agent.rpc.sendDwnRequest({
dwnUrl,
targetDid : did,
dwnUrl, targetDid : did,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

formatting mistake


constructor(options: { agent: Web5Agent, connectedDid: string, delegateDid?: string }) {
this.agent = options.agent;
this.connectedDid = options.connectedDid;
this.delegateDid = options.delegateDid;
this.permissionsApi = new AgentPermissionsApi({ agent: this.agent });
this.cachedPermissionsApi = new CachedPermissions({ agent: this.agent, cachedDefault: true });
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should it be named AgentCachedPermissionsApi for consistency? it seems like all the rest of the Agent...Api its a class that is coupled to agents and requires passing the Agent singleton into it

/**
* An instance of the `AgentPermissionsApi` that is used to interact with permissions grants used during sync
*/
private _cachedPermissionsApi: CachedPermissions;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

per the description this is kind of confusing. should there be some sort of inheritance?

*
* This will store the grants as the DWN owner to be used later when impersonating the connected DID.
*/
static async processConnectedGrants({ grants, agent, delegateDid }: {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this doesn't seem like the right place for this method? it doesn't rely on any internal state from the Web5 class (from what I can tell) and seems to be pretty highly related to the connect work in Agent

Copy link
Contributor

@shamilovtim shamilovtim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rubber stamp

@LiranCohen LiranCohen merged commit 3d1f825 into main Aug 23, 2024
35 checks passed
@LiranCohen LiranCohen deleted the lirancohen/grants-sync branch August 23, 2024 20:53
@github-actions github-actions bot mentioned this pull request Aug 23, 2024
This was referenced Sep 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement the ability to use grants from the connect flow during sync
2 participants