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 verifiable credentials flow in frontend #2097

Merged
merged 6 commits into from
Dec 4, 2023
Merged

Add verifiable credentials flow in frontend #2097

merged 6 commits into from
Dec 4, 2023

Conversation

nmattia
Copy link
Collaborator

@nmattia nmattia commented Dec 4, 2023

This adds a "verifiable credentials" flow to the frontend. In four commits:

  1. The canister code is updated to accept 2 index.js files; without this, the browser will refuse to fetch the new index file for the second flow due to bad CSP.
  2. Adds the actual VC postMessage interface, flow & screens to the II webapp.
  3. Updates the test-app by introducing a few react component to send & receive VC related requests to an IDP (II).
  4. A couple of tests are added to go through a full VC flow.

This is more of a "clean proof-of-concept" than an actual feature, as many things are still missing:

  • The VC flow does not support PIN identities
  • The SRI hash fix is a bit ugly and should probably be fixed more generally (for 2+ index files)
  • The issuer types are duplicated between the issuer app and II; moreover in both cases they include both the actual Issuer API (specced) and the helper API like add_employee. This should be cleaned up somehow.
  • The tests merely checks that the flow is successful; they should also check the validity of the presentation and be extended to test for non-happy cases.
  • There is no support for changing the consent message language
  • The test-app requests are not typechecked (which might cause frustration if the types ever change)
  • The consent message is not shown as rendered markdown

All of these will come in other PRs.


🟢 Some screens were added
🟡 Some screens were changed

@nmattia nmattia marked this pull request as ready for review December 4, 2023 09:37
Copy link
Member

@peterpeterparker peterpeterparker left a comment

Choose a reason for hiding this comment

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

Only had a look to the configuration and Rust code. Looks good, just few questions.

Rubber stamp for the JS/HTML, did not had a look.

src/internet_identity/src/assets.rs Show resolved Hide resolved
package.json Show resolved Hide resolved
src/internet_identity/src/http.rs Show resolved Hide resolved
Copy link
Contributor

@frederikrothenberger frederikrothenberger left a comment

Choose a reason for hiding this comment

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

Thanks, LGTM!

@nmattia nmattia enabled auto-merge December 4, 2023 14:57
@nmattia nmattia added this pull request to the merge queue Dec 4, 2023
Merged via the queue into main with commit 90e5580 Dec 4, 2023
52 of 53 checks passed
@nmattia nmattia deleted the nm-vc-flow branch December 4, 2023 15:11
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.

3 participants