add revocation to js issuer and disclose functions #33

AshHalliwell · 2021-04-14T22:53:05Z

No description provided.

ehsan-fj · 2021-04-15T20:58:47Z

src/assets/js/irma-functions.js

@@ -26,7 +28,7 @@ function irmaDiscloseOrSign(attributes, header = 'Disclosing attribute with', la
    });
 }

-function irmaIssueCredential(credential, attributes, header = 'Issuing credential with', disclosePayload = null) {
+function irmaIssueCredential(credential, attributes, header = 'Issuing credential with', disclosePayload = null, revocationKey = None) {


revocationKey = null?

Hum, I think would be better to have a specific method called irmaIssueRevocableCredential instead, as this could be more meaningful.

Cool, I've separated the irmaIssueRevocableCredential and irmaIssueCredential functions, but IrmaDiscloseOrSign is still handling both the original disclosure and disclosure with revocation. Does that sound sensible?

Also what part of the front end needs to be changed to use this? Or should we hold off making those changes until we know revocation is working?

I agree that disclosure or sign seems to fit better into a single function.

rafaelbernard · 2021-04-18T21:29:03Z

src/assets/js/irma-functions.js

+  let validity = {};
+  if (attributes.validity) {
+    validity = {validity: attributes.validity};
+    delete attributes.validity;
+  }
+  const request = {
+    '@context': 'https://irma.app/ld/request/issuance/v2',
+    'credentials': [{
+      credential: credential,
+      attributes: attributes,
+      revocationKey: revocationKey,
+      ...validity,
+    }],
+    'disclose': disclosePayload,
+  };


This is a part we can extract from both functions into a third function to create the request based on the arguments.

Could we also fold it back into a single function, giving it the same treatment as validity? I think that might actually be tidier than two functions, if revocationKey can be passed in attributes

Yeah, we can do that.

rafaelbernard · 2021-04-18T21:35:56Z

Also what part of the front end needs to be changed to use this? Or should we hold off making those changes until we know revocation is working?

No need to apply right now. Let´s wait on Ehsan´s research to revocation being fully working before applying to the UI, but we need to be sure that is backwards compatible with the current usage of the UI. So, please, just go through the flow to be sure everything remains working and we will be fine.

… into revocation

rafaelbernard · 2021-05-10T01:12:35Z

src/id-card/index.html

@@ -223,6 +223,7 @@
                    },
                    saveIdCard: function () {
                        console.log('saveIdCard');
+                        this.idCardToIssue['revocationKey'] = `${luxon.DateTime.now().ts}`;


Would not be better to use as a key something like version1 or pocSS or something that is really a key that is repeatable? I think we do a better use of the key if we identify it as some "group" or "cluster" of the given credential issued by this key.

That use above is more suited to the issued attribute.

I don't think so! The key should be something unique, otherwise, you won't be able to revoke it! as current irma cli subcommand doesn't accept issued! So, for example, if issue all your credential with the same key, when you want to revoke one of them, you would revoke all of them!
BTW, issued is generated by irma server!

We may need to do more tests about that, but I am not sure that it must always have a totally unique value as we have alternatives. Document says:

For example, the following RevocationRequest instructs the server to revoke the irma-demo.MijnOverheid.root instance to which it previously assigned bsn-12345 as revocationKey during issuance, and which was issued at Unix nano timestamp 1583765731750425000. If issued is not specified, all previously issued credentials with matching revocationKey are revoked.

I can see that is expected to have revocationKey to revoke a set of credentials with the same key and a more particular credential using the key and issued properties. If we use the key as a group, we can, for instance, make demos showing that we can revoke the IDNZ card from 2021 if we create a key name v2021. And, at the same time, we can show that we can revoke a specific IDNZ card from a member that decided to cancel his card using the key and the issued property that we have stored on database.

Please read my comment again!!

as current irma cli subcommand doesn't accept issued!

So if issue credential with non-unique keys, we won't be able to revoke them individually, at least for now! Does it make sense?

But we would rely on irma-front-end, right? We have an endpoint that accepts both revocationKey and issued.

That endpoint doesn't work either! I've already mentioned that here:https://internetnz.atlassian.net/wiki/spaces/RCENG/pages/1011581103/IRMA+-+Feature+-+Revocation#revoking-credential
The error is known to IRMA and they are working on that!

Well, so we are here considering not a full implementation of revocation, but a very limited revocation structure aiming to work with the only parts that are working now. If that is the case, that is fine, because we can work of what we have, despite its best usage. I would add a comment about the usage we "would like to but cannot use" and, maybe, just use this.idCardToIssue.revocationKey = this.idCardToIssue.number as this is already unique enough.

I believe that's been the plan since we knew what parts of it are working and what parts are not! But I would say the overall feature is not production-ready yet.

Yeah, I see you. No problem. As the title says add revocation and we did not necessarily mention the limitations on the implementation, I wanted to be sure about what exactly we are implementing (and what we are not).

Symptoms of working with something at the edge. :-D

add revocation to js issuer and disclose functions

eaf4f13

AshHalliwell requested review from ehsan-fj and rafaelbernard April 14, 2021 22:53

ehsan-fj reviewed Apr 15, 2021

View reviewed changes

seperated revocable credential issuer

99bdc77

AshHalliwell marked this pull request as ready for review April 16, 2021 03:42

rafaelbernard reviewed Apr 18, 2021

View reviewed changes

AshHalliwell and others added 9 commits April 19, 2021 12:29

receive revocationKey in attributes

4b300d7

merge master

262d2ac

set revocation key and request for proof

790f235

request for nonrevoked proof when disclosing

9fc7a6f

styling

af45592

convert revocation key to string

4e56135

not asking nonrevok proof for now

7945cac

not asking nonrevok proof for now

e5108e0

Merge branch 'revocation' of https://github.com/InternetNZ/irma-demo-ui…

ea1ff23

… into revocation

rafaelbernard reviewed May 10, 2021

View reviewed changes

use id card number as revocationKey

b24aa32

ehsan-fj requested a review from rafaelbernard May 10, 2021 03:33

rafaelbernard approved these changes May 10, 2021

View reviewed changes

ehsan-fj merged commit 4eaf258 into master May 10, 2021

ehsan-fj deleted the revocation branch May 10, 2021 03:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add revocation to js issuer and disclose functions #33

add revocation to js issuer and disclose functions #33

AshHalliwell commented Apr 14, 2021

ehsan-fj Apr 15, 2021

rafaelbernard Apr 15, 2021

AshHalliwell Apr 15, 2021

AshHalliwell Apr 15, 2021

rafaelbernard Apr 18, 2021

rafaelbernard Apr 18, 2021

AshHalliwell Apr 19, 2021

rafaelbernard Apr 19, 2021

rafaelbernard commented Apr 18, 2021

rafaelbernard May 10, 2021

ehsan-fj May 10, 2021

rafaelbernard May 10, 2021

ehsan-fj May 10, 2021

rafaelbernard May 10, 2021

ehsan-fj May 10, 2021

rafaelbernard May 10, 2021

ehsan-fj May 10, 2021

rafaelbernard May 10, 2021

add revocation to js issuer and disclose functions #33

add revocation to js issuer and disclose functions #33

Conversation

AshHalliwell commented Apr 14, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rafaelbernard commented Apr 18, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment