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

feat!: update session API #227

Merged
merged 12 commits into from
Feb 28, 2023
Merged

feat!: update session API #227

merged 12 commits into from
Feb 28, 2023

Conversation

Gozala
Copy link
Collaborator

@Gozala Gozala commented Feb 14, 2023

Fixes #226

Overview

Changes in this PR update our session logic which previously just assigned a did:key to a did:mailto principal in the validation context.

Instead now we have ucan/attest capability that can be used to attest to the fact that specific delegation (that has non did:key issuer) has been authorized by the user. In other words when we perform interactive authorization flow like with access/authorize we can delegate ucan/attest to the agent been authorized that proofs that we performed an out-of-bound authorization flow in which user has authorized delegation we're attesting to.

Now instead of assigning a key we set authorization CID, that corresponds to the UCAN Delegation without proofs and signature. With this changes when validator encounters delegation issued by principal other than did:key e.g did:mailto it will attempt to find corresponding ./update capability proof that sets permit to a CID derived from delegation. If such (valid) delegation is found, validator will not check signature and continue with UCAN verification process. If (session) delegation is found, validator falls back to doing key resolution instead.

All in all this gives us a way to authorize agent with a specific set of capabilities as opposed to giving it a sudo capabilities as it used to be the case previously.

P.S: I had to update Link schema so that validator could look for ./update with a very specific CID in it as opposed to finding a mismatching one, which could be problem if there were multiple ./update proofs, because validator stops as soon as it finds success path.

Link to the updated spec

@Gozala Gozala changed the title wip: update session API feat!: update session API Feb 16, 2023
@Gozala Gozala marked this pull request as ready for review February 16, 2023 07:39
@heyjay44 heyjay44 added this to the w3up phase 3 milestone Feb 16, 2023
@heyjay44 heyjay44 mentioned this pull request Feb 16, 2023
83 tasks
@Gozala Gozala requested a review from gobengo February 17, 2023 18:34
Copy link
Contributor

@vasco-santos vasco-santos left a comment

Choose a reason for hiding this comment

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

LGTM! Great work

packages/core/src/delegation.js Outdated Show resolved Hide resolved
@Gozala Gozala requested a review from gobengo February 28, 2023 02:02
* @template {UCAN.DID} ID
* @implements {UCAN.Signer<ID, Signature.NON_STANDARD>}
*/
class Account {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: if possible I'd avoid using Account at all here (it's too easy to misinterpret) and call this something like NonStandardSigner. just a thought

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Renamed it to Absentee which I think reflect the better than either misleading Account or NonStandardSigner which isn't specific enough. I'll assume it's ok, if not we can rename in the followup change.

packages/principal/test/account.spec.js Outdated Show resolved Hide resolved
packages/validator/src/lib.js Show resolved Hide resolved
packages/validator/src/schema/link.js Show resolved Hide resolved
packages/validator/test/session.spec.js Outdated Show resolved Hide resolved
packages/validator/src/lib.js Outdated Show resolved Hide resolved
Copy link
Member

@alanshaw alanshaw left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@github-actions github-actions bot mentioned this pull request Feb 28, 2023
This was referenced Apr 11, 2023
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.

Update ./update capability to support extended fields
5 participants