credential-handler / chapi.io

https://chapi.io/
Other
7 stars 4 forks source link

Add VC Presentation for Native Wallet docs. #41

Closed BigBlueHat closed 12 months ago

BigBlueHat commented 1 year ago

@brianorwhatever and I are working toward launching this before Wednesday of next week.

We also started some Mermaid.js (#30) for this section:

sequenceDiagram
  # User visits Relying Party website and clicks something. Then this happens...

  participant site as OpenBadge Verifier Site
  participant app as Native Wallet App
  participant chapi as User's Browser / CHAPI
  participant exchanger as VC-API Exchanger

  site ->> exchanger: 1. creates an exchange within a workflow
  exchanger ->> site: 2. exchange URL/ID returned (in some JSON)
  site ->> chapi: 3. triggers CHAPI `get()` request w/blank VP + `protocols.vcapi` containing URL
  chapi ->> app: 4. triggers app URL to open app w/`?request=`
  app ->> exchanger: 5. uses VC-API  URL from `?request=` to send VPR
  exchanger ->> app: 6. returns VPR to wallet
  app ->> exchanger: 7. responds with VP
  exchanger ->> app: 9. sends success response to wallet
  # Webhooks here maybe?
  loop poll exchange status
    site ->> exchanger: exchange complete?
    exchanger ->> site: ...wait for it...
  end
dlongley commented 1 year ago

Still reviewing this, but it struck me that (maybe?) the diagram would look better with the position of Native Wallet App and User's Browser / CHAPI positionally reversed.

dlongley commented 1 year ago

The VP doesn't have to be blank (and it's a blank VPR really, it just is the value of the VerifiablePresentation property for arcane reasons). So maybe we leave that bit out -- it's not relevant to native wallets anyway.

dlongley commented 1 year ago
  1. uses VC-API URL from ?request= to send VPR

I think we're calling this URL too many different things. This is the "exchange URL" from the beginning; it will be found under protocols.vcapi ... but maybe that's not relevant?

Also, the VPR isn't being sent an empty {} JSON is POSTed to the exchange URL. Then, as step 6 says, the VPR is returned as a response.

BigBlueHat commented 1 year ago

Still reviewing this, but it struck me that (maybe?) the diagram would look better with the position of Native Wallet App and User's Browser / CHAPI positionally reversed.

This look better?

sequenceDiagram
  # User visits Relying Party website and clicks something. Then this happens...

  participant site as OpenBadge Verifier Site
  participant chapi as User's Browser / CHAPI
  participant app as Native Wallet App
  participant exchanger as VC-API Exchanger

  site ->> exchanger: 1. creates an exchange within a workflow
  exchanger ->> site: 2. exchange URL/ID returned (in some JSON)
  site ->> chapi: 3. triggers CHAPI `get()` request w/blank VP + `protocols.vcapi` containing URL
  chapi ->> app: 4. triggers app URL to open app w/`?request=`
  app ->> exchanger: 5. uses VC-API  URL from `?request=` to send VPR
  exchanger ->> app: 6. returns VPR to wallet
  app ->> exchanger: 7. responds with VP
  exchanger ->> app: 9. sends success response to wallet
  # Webhooks here maybe?
  loop poll exchange status
    site ->> exchanger: exchange complete?
    exchanger ->> site: ...wait for it...
  end

Sadly, Mermaid (still...) lacks the ability to color code nodes in a Sequence diagram...so we can't call out stuff the user will "touch" vs. the "invisible glue"/developer bits.

BigBlueHat commented 1 year ago

The VP doesn't have to be blank (and it's a blank VPR really, it just is the value of the VerifiablePresentation property for arcane reasons). So maybe we leave that bit out -- it's not relevant to native wallets anyway.

Are you referring to the sending of the empty JSON object? Meaning, it'd be valid to send a completely empty POST to the exchanges endpoint?

BigBlueHat commented 1 year ago

The VP doesn't have to be blank (and it's a blank VPR really, it just is the value of the VerifiablePresentation property for arcane reasons). So maybe we leave that bit out -- it's not relevant to native wallets anyway.

Are you referring to the sending of the empty JSON object? Meaning, it'd be valid to send a completely empty POST to the exchanges endpoint?

Never mind. I realized you're referring to the Mermaid chart... I'll get that in as it's own commit, so it can be reviewed directly.

dlongley commented 1 year ago

@BigBlueHat,

This look better?

Yeah, I think so -- seems more legible to me. It seems easy to read the individual back and forth interactions and fewer of the vertical lines are overlapping the text. Not sure if it's optimal yet, but better, I think.

cloudflare-pages[bot] commented 1 year ago

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: 1175639
Status: ✅  Deploy successful!
Preview URL: https://641134a9.chapi-io.pages.dev
Branch Preview URL: https://vc-api-for-presentation.chapi-io.pages.dev

View logs

BigBlueHat commented 1 year ago

@dlongley I made a few more changes to the chart and also improved the viewing experience for mermaid charts across the site. Here's a demo: https://vc-api-for-presentation.chapi-io.pages.dev/developers/wallets/native/#verifiable-credential-storage

BigBlueHat commented 1 year ago

That link is to localhost ... which either means you want me to clone and run to see or the demo is elsewhere and the link needs updating. Either way, I'm inclined to approve this PR to get this material in. Thanks!

Sorry. Fixed. The whole site on this PR is visible at https://vc-api-for-presentation.chapi-io.pages.dev/ or the page with the mermaid chart at https://vc-api-for-presentation.chapi-io.pages.dev/developers/wallets/native/#verifiable-credential-storage

BigBlueHat commented 1 year ago

@brianorwhatever I'd love your input (however brief) as much of this was done in collaboration with you (as ever). 😄

dlongley commented 1 year ago

JFYI, the mermaid diagram looks like this for me (dark mode):

mermaid-diagram

It's not top priority to fix this, but would be nice if we could somehow.

BigBlueHat commented 1 year ago

@dlongley good catch. It's definitely non-trivial to fix this...sadly. Mermaid has themes (and one that's dark mode friendly), but it's gonna take a bit to do that dynamically, etc. I'll file and issue and try to get to it soon.

brianorwhatever commented 12 months ago

Let's address this small change request and get this merged in @BigBlueHat

BigBlueHat commented 12 months ago

Let's address this small change request and get this merged in @BigBlueHat

Cleaned that unfinished stuff up, and made a couple other smaller improvements. I think it's ready.

BigBlueHat commented 12 months ago

@brianorwhatever can you re-review or change the "requested changes" status at least? I think this is ready and would like to move on to fixing it in further PRs if/when we find more to change. Thanks!