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

Scrambled credentials in /me route #371

Open
maiertech opened this issue Sep 24, 2024 · 2 comments
Open

Scrambled credentials in /me route #371

maiertech opened this issue Sep 24, 2024 · 2 comments
Labels
Blocked An issue that is blocked by another issue or another blocker. Bug Something isn't working

Comments

@maiertech
Copy link
Member

maiertech commented Sep 24, 2024

Description

When clicking on credentials accepted from the NGDIL demo in the /me route, scrambled credentials are displayed. Scrambled means that the data prop, which contains credentialSubject does not match with the logo and display_name.

It seems that when the backend hands over the state to the frontend, it is already scrambled and the frontend just correctly displays scrambled credentials.

In some cases, the credentials route is blank and the console suggest that this is related to #351. But this might also be a separate bug.

I also observe inconsistencies with rendered credentials and blank routes in UniMe 0.6.12.

Hardware Specification

n/a

Steps to Reproduce the Bug

  1. On dev, load the credentials from the first journey of the NGDIL demo.
  2. Accept the credentials. You will be redirected to /me.
  3. Tap on different NGDIL issued credentials and compare the credential logo with the text. They do not always match.
  4. In some cases you will see a blank credential page.

Actual Behaviour

Animated gif posted to Slack due to size restriction here.

Expected Behaviour

It should render the credentials exactly as NGDIL issued them.

Errors

If a credential route is rendered as a blank page, this error shows up in the console:

Unhandled Promise Rejection: TypeError: ctx[0].data.credentialSubject[/*field*/ ctx[3]].startsWith is not a function. (In 'ctx[0].data.credentialSubject[/*field*/ ctx[3]].startsWith('data:image/')', 'ctx[0].data.credentialSubject...

This may be a separate bug.

Findings

All of the following findings are frontend only.

Credential offers looks fine:

offers

The /me list also looks fine, i.e. logos match display names:

list

But the application state read by the CredentialsList already contains scrambled data. Here, e.g., for the National ID:

appState

The credentials route just renders the scrambled credential it found in the state:

national-id

This begs the question: do we get scrambled data from the backend? Yes we do. This screenshot shows scrambled data from the payload from the backend that is stored as application store:

handover

@maiertech maiertech added the Bug Something isn't working label Sep 24, 2024
@nanderstabel nanderstabel added the Blocked An issue that is blocked by another issue or another blocker. label Sep 25, 2024
@nanderstabel
Copy link
Contributor

nanderstabel commented Sep 25, 2024

Indeed the scrambled credentials bug is different from the blank page bug.

Apparently the credentials get 'scrambled' because the back end assumes that when it does a Batch Credentials Request to the NGDIL issuer that the credentials in the Response are returned in the same order as they have been requested.

The OID4VCI spec indeed states:

Every entry of the array corresponds to the Credential Request object at the same array index in the credential_requests parameter of the Batch Credential Request.

When we receive the credentials, we 'bind' some credential display data to the credentials that we fetch from: https://api.demo.ngdil.com/.well-known/openid-credential-issuer

However, we do this assuming the original order has been respected by the issuer. In this case, that order has not been respected so we incorrectly bound the wrong display data to the wrong credentials.

This means we have a quite crucial reliance on the Issuer in this regard since we need to trust that they will respect original order of the credential IDs in the Batch Credential Request (according to OID4VCI draft 13).

The only way of properly preventing this problem is by completely deprecating Batch Credentials, and instead Send multiple 'single' Credential Requests so we have complete control over the order of Credentials that will be issued so that we can always bind the right display data to the right credential. We can safely do this since Batch Credentials are already deprecated in OID4VCI draft 14.

If we do choose for this option we first need to check whether the NGDIL demo supports a flow with multiple single Credential Requests

@M-Adam-Hus
Copy link

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Blocked An issue that is blocked by another issue or another blocker. Bug Something isn't working
Projects
Status: Backlog
Development

No branches or pull requests

3 participants