Expose DWN PermissionsGrant Message #252
Conversation
Review or Edit in CodeSandboxOpen the branch in Web Editor • VS Code • Insiders |
Codecov Report
@@ Coverage Diff @@
## main #252 +/- ##
==========================================
+ Coverage 90.94% 90.99% +0.04%
==========================================
Files 69 69
Lines 13314 13399 +85
Branches 1340 1346 +6
==========================================
+ Hits 12109 12192 +83
- Misses 1180 1182 +2
Partials 25 25
|
diehuxx
force-pushed
the
diehuxx/permissions
branch
2 times, most recently
from
44d1b3c to
eafd2d6
Compare
diehuxx
force-pushed
the
diehuxx/permissions
branch
from
eafd2d6 to
a7e015b
Compare
diehuxx
requested review from
frankhinek,
csuwildcat and
mistermoe
as code owners
packages/api/src/dwn-api.ts
Outdated
| @@ -91,6 +106,11 @@ export type RecordsWriteRequest = { | |||
| store?: boolean; | |||
| } | |||
|
|
|||
| export type PermissionGrantRequest = { | |||
There was a problem hiding this comment.
@diehuxx This type appears to be a duplicate. It isn't used anywhere as best I can tell. Should it be removed?
packages/api/src/dwn-api.ts
Outdated
| import { PermissionsGrant } from '@tbd54566975/dwn-sdk-js'; | ||
| import { PermissionsGrantMessage } from '@tbd54566975/dwn-sdk-js'; | ||
| import { PermissionsGrantOptions } from '@tbd54566975/dwn-sdk-js'; |
There was a problem hiding this comment.
Can these be consolidated into the existing import statement a few lines up?
| @@ -184,8 +188,11 @@ export class DwnManager { | |||
| }); | |||
| dwnRpcRequest.message = message; | |||
| messageData = data; | |||
|
|
|||
| } else if ('message' in request) { | |||
There was a problem hiding this comment.
@diehuxx The approach to sending a PermissionsGrant is different than how both Record and ProtocolsConfigure messages are sent to a remote DWN. What was the motivation for introducing a new message property to the send request object?
There was a problem hiding this comment.
I've been wanting to make that addition separate of PermissionsGrants. The existing code forces an existing message to be turned into options then re built and signed. Why not just send the existing message?
| @@ -569,4 +571,57 @@ describe('DwnApi', () => { | |||
| }); | |||
| }); | |||
| }); | |||
|
|
|||
| describe('permissions.grant()', () => { | |||
There was a problem hiding this comment.
Could a test for sending a PermissionsGrant to a remote DWN be added in this PR?
There was a problem hiding this comment.
If I understand agents correctly, setting dwn.permissions.grant(target : bobDid.did, ...) will send to a remote. We do that in the test in describe('target: did'
Minimum permissions interface to expose to unblock delegated permissions grants.
dwn.permissions.send()will be extended in a future PR to to send otherPermissionsinterface messages.