bcgov / dts-vc-issuer-service

Digital Trust Verifiable Credential Issuer Service
Apache License 2.0
0 stars 11 forks source link

Verifier application/Application consomatrice #67

Closed torjc01 closed 3 years ago

torjc01 commented 3 years ago

Signed-off-by: torjc01 julio_cesar.torres@sct.gouv.qc.ca

swcurran commented 3 years ago

Sorry it took so long to get back to this. Thanks for dealing with the license issue -- that makes it easier.

I tried running with normal ./manage start but I don't see the verifier container(s) starting. Does that still need to be added to the script? I didn't look for other instructions for starting the verifier...

torjc01 commented 3 years ago

Hi Stephen, thanks for your answer. Normally ./manage start is enough to do its magic... Well, the container is already added to the script and it should start, in my point of view. I am going to check it out on a fresh directory, so I can follow all the steps to mount the application and make sure it starts as it is supposed to.

swcurran commented 3 years ago

Ah...I think I know the issue. I run docker on one machine and port forward the ports I need to my desktop. I have to add port 10000 to the forwarding. Sorry about that -- I'll try again.

torjc01 commented 3 years ago

Hi @swcurran, I would like you to pull the sources again. I noticed some headers missing to avoid CORS errors. They've just been added. I am working on the page's look and feel, to make it more like the issuer's page. The text informing users what is going on will be added along with this interface change. Is there anything you think it is important saying?

Tks

swcurran commented 3 years ago

Nice! I got through it all and like the overall flow. A couple of tweaks in using this:

  1. Suggest that we use the "connectionless" proof request vs. the establishing a connection with the verifier.
  2. Suggest that the proof request used be one with selective disclosure.
  3. I think the whole process should be wrapped in directions for the user, with instructions about what is happening. Don't know exactly what to say or how to wrap it, but I think that would be useful to do that.
  4. Should be a button that says (more or less) "Return to issuer", that closes the window or gives instructions how to return.
  5. The English/Français toggle works, but the text of the button doesn't change.
torjc01 commented 3 years ago

Hi @swcurran! We are going to address every point suggested, but I would like to ask you some questions to completely understand your requests:

  1. Martin is already working on it, we're going to integrate it at the end of this iteration. However, we believe there's a bunch of code that we must add to the application so it can work connectionlessly, and that it will be the last piece of code to be pulled into this PR;
  2. When you suggest selective disclosure, you mean letting the user decide which attributes to reveal on his proof, right? Looking at the Vaccine Credential Dataset Proposed​, the only fields that are non-mandatory are Vaccine brand, Vaccine manufacturer, Disease, Administration Centre and Due date of next dose. So, I suppose these are the attributes I would let the selective disclosure on the user's discretion, and the other attributes are required. Do I get this right? We also had a discussion here about ZKP, but we don't think this is relevant for this use case, as our attributes are mostly textual, so not the right data type for ZKP;
  3. We are looking into the information to add to the flow, so that it is informative and clear enough for the intended audience. We search for inspiration in a couple of places, such as the LSBC application which has some guidance on their site;
  4. No problem, the button will be added;
  5. Also no problem, we're gonna fix this bug.

Well, I think this covers pretty much the pending questions. Please let me know it you see anything else.

Tks!

Julio

swcurran commented 3 years ago

Thanks @torjc01 -- sounds great. I'd like to merge this sooner than later.

The only concern I have with merging it now is that the "Test Your credential" link on the main screen all the time, and quite large. It would be nice if it was only there after the user has a credential, and it was visually separate from the main flow. I should have included it on my earlier comments.

Regards selective disclosure. The Proof Request from the verifier should only ask for limited information vs. the full set of data in the credential. Does that make sense? It would show that the airline can allow the person to proceed with only limited data displayed.

torjc01 commented 3 years ago

Hi @swcurran, that's great. I am going to move the "Test your credential" to the bottom of the screen. Also, I am going to make it a bit smaller, so it doesn't take all this space. About it appearing, I test if immunizationRecords.length > 0 and it returns true when one or more credentials are created. Maybe this is not the good test, perhaps I should use patientCredentials. I am going to try it.

The selective disclosure is simpler than what I thought. Do you think asking for recipient_fullName, recipient_birthDate and vaccine_dateOfVaccination is good enough?

Tks,

swcurran commented 3 years ago

Sounds good! Thanks. I think your proposed selective disclosure list is good enough for now, and easy to change.

torjc01 commented 3 years ago

Hi @swcurran! I have committed some code to address your issues, here's a brief debriefing (no pun intended):

  1. Selective disclosure: as we discussed with the attributes recipient_fullName, recipient_birthDate and vaccine_dateOfVaccination. If you need something else, it will be trivial to add to the flow.
  2. Linking applications: the link to Test your credential has been moved do the bottom of the page, and now it is unmistakably recognized. I called it Verifying your vaccination credential.
  3. Directions to the user: I added directions on the issuer's bottom page, on the verifier's landing page, and through the other pages in the flow, telling the user what's going on. I am not sure if these are up to your expectations, but again, they're easy to complement.
  4. Return to the issuer: there is a button now, on the verifier's page header that takes back to the issuer page.
  5. Buggy toggle: debugged, now it works as intended.

The connectionless proof, on the other hand, is still to come. Martin St-Pierre has it implemented in another branch and he is giving me a hand to integrate it into this PR. However, it isn't ready yet.

Could you please take a look at these changes and tell me if they are as you expected, please?

Tks!

swcurran commented 3 years ago

You bet! I've been watching the commits, and will try to give a try today.