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

Begin production rule definition #11

Merged
merged 6 commits into from Jan 17, 2023
Merged

Conversation

OR13
Copy link
Contributor

@OR13 OR13 commented Aug 31, 2022

The intention of the Pull Request is to simply define the existing production rules in greater detail, this PR is supposed to be editorial, I am not trying to change any of the existing behavior.


Preview | Diff

index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
Co-authored-by: Kristina <52878547+Sakurann@users.noreply.github.com>
index.html Show resolved Hide resolved
index.html Show resolved Hide resolved
Copy link
Collaborator

@selfissued selfissued left a comment

Choose a reason for hiding this comment

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

Several suggestions - some normative - some editorial.

index.html Outdated Show resolved Hide resolved
index.html Show resolved Hide resolved
index.html Show resolved Hide resolved
index.html Show resolved Hide resolved
index.html Show resolved Hide resolved
index.html Show resolved Hide resolved
index.html Show resolved Hide resolved
index.html Show resolved Hide resolved
index.html Show resolved Hide resolved
@iherman
Copy link
Member

iherman commented Oct 19, 2022

The issue was discussed in a meeting on 2022-10-19

  • no resolutions were taken
View the transcript

2.6. Begin production rule definition (pr vc-jwt#11)

See github pull request vc-jwt#11.

Orie Steele: there are changes requested on this.
… not sure when FPWD will be ready.
… please submit all your comments ASAP.

index.html Show resolved Hide resolved
@OR13 OR13 requested a review from selfissued October 21, 2022 13:25
@OR13
Copy link
Contributor Author

OR13 commented Oct 21, 2022

@selfissued I have addressed your feedback, let me know if you have additional concerns.

index.html Outdated Show resolved Hide resolved
Co-authored-by: Ted Thibodeau Jr <tthibodeau@openlinksw.com>
@OR13
Copy link
Contributor Author

OR13 commented Oct 26, 2022

I will need to update this based on the resolutions for validFrom and validUntil.

@iherman
Copy link
Member

iherman commented Oct 27, 2022

The issue was discussed in a meeting on 2022-10-26

  • no resolutions were taken
View the transcript

1.5. Begin production rule definition (pr vc-jwt#11)

See github pull request vc-jwt#11.

Orie Steele: That had comments / changes requested by Mike Jones, I believe I addressed them..
… I did request a re-review. TallTed had some editorial comments that I accepted, I am just requesting Mike Jones to re-review..
… That's it for VC-JWT PRs..

Michael Jones: Do you want to talk about the issues?.

Orie Steele: I was just doing PRs..

Michael Jones: Thank you, that's fine, that's to TallTed for the recent approval..

Copy link
Collaborator

@selfissued selfissued left a comment

Choose a reason for hiding this comment

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

We may need to beef up the language about inclusion of alg as a required header parameter, but we could do that in a separate PR.

@OR13
Copy link
Contributor Author

OR13 commented Nov 2, 2022

I believe I have addressed all comments, I'd like to merge soon. @selfissued would you be willing to press the button?

@iherman
Copy link
Member

iherman commented Nov 2, 2022

The issue was discussed in a meeting on 2022-11-02

  • no resolutions were taken
View the transcript

1.5. Begin production rule definition (pr vc-jwt#11)

See github pull request vc-jwt#11.

Orie Steele: VC-JWT still has PR 11 open.
… mike jones request changes and i have responded.
… but there are still changes requested but i believe that those have been addressed.
… but i would like to accelerate the pace with improvements to that draft.
… additional reviews would help.
… that is it for VC-JWT.

Michael Jones: given that poll taken on previous call, there seems to be validFrom and validUntil.
… language needs to be updated in the PR.
… when to be merged in the vc data model.

Michael Prorock: +1 wait for upstream merge.

Orie Steele: intention was not make any changes, only document what was already required.

Michael Prorock: then correct / update.

Orie Steele: i don't thing we should coming with those changes in the PR.

Michael Jones: ok, then i think i can approve.

@OR13
Copy link
Contributor Author

OR13 commented Nov 30, 2022

Open since august... only 1 approval... perhaps we should pause this work item?

@OR13
Copy link
Contributor Author

OR13 commented Nov 30, 2022

I'm withdrawing this PR, until the WG decides they want to focus on this work item.

@OR13 OR13 closed this Nov 30, 2022
@iherman
Copy link
Member

iherman commented Nov 30, 2022

The issue was discussed in a meeting on 2022-11-30

  • no resolutions were taken
View the transcript

2.4. Begin production rule definition (pr vc-jwt#11)

See github pull request vc-jwt#11.

Orie Steele: has only one approval -- could indicate general lack of interest and focus on other issues - perhaps pause?.

Manu Sporny: +1 for pausing VC-JWT work item..

Manu Sporny: (given the recent discussion around how we're going to process Work Items).

Dmitri Zagidulin: I am somewhat surprised at the lack of engagement on that item..

Kerri Lemoie: what is ivan's comment in reference to?.

Manu Sporny: and seeing how VC-JWT duplicates work done in JsonWebSignature2020.

Orie Steele: There has not been an FPWD for vc-jwt, and I don't recommend we seek one at this time..

Dave Longley: it is true that VC-JWT is duplicative of JsonWebSignature2020.

Manu Sporny: I would expect multiple -1s for VC-JWT based on engagement and how we're planning to adopt work items now..

David Chadwick: lots of interest in JWT- like at JFF plugfest. Lack of time but also implementors are getting it to work ok without so many errors as thought..

Manu Sporny: There were 17 different organizations implementing the DI approaches... vs. 6 for VC-JWT -- useful information on each cryptosuite..

David Chadwick: what is the difference between a credential and a verifiable credential- add topic to future agenda please?.

Kristina Yasuda: chairs will discuss how to resolve that.

Joe Andrieu: +1 to discuss David's question. It's a good one for the group to clear up..

Michael Jones: agree with DavidC that there's a lot of interest in vc-jwt but doesn't indicate that there aren't any problems.

Orie Steele: issues that remains: 1) mappings to the core data model, 2) concrete documentation on how to define how to obtain a public key to verify a credential..

David Chadwick: Orie: did:jwk solves the public key problem.

Orie Steele: should tackle core-data model issues first.
… gain wg consensus about core-data model first..

Sten Reijers: +1 to Orie comment that VC-JWT is in a bad shape.

David Chadwick: orie: the issue that Joe and I are discussing about credential vs verifiable credential is pertinent to jwt-vc.

Orie Steele: its not that JWS/JWT or JOSE is in bad shape... its just W3C "versions" of it..

Manu Sporny: agree in pursuing consensus on JWT but new process could negatively affect this work. Implementors could indicate what they support..
… agree to hit pause until the wg can focus on fixing the core data model concerns then comeback to vc-jwt.

Orie Steele: This is an "unofficial" draft for a vc-jws... which is massively simpler than vc-jwt: https://transmute-industries.github.io/vc-jws/.

Sten Reijers: Is there a link to VC-JOSE issue?.

Orie Steele: Here is the original draft pre IETF 114: https://or13.github.io/draft-osteele-vc-jose/.

Orie Steele: Here is the issue regarding vc-jose proposal: w3c/vc-data-model#971.

Kristina Yasuda: This work can happen within the process. Thank you for raising this topic. Chairs will discuss..

@OR13 OR13 reopened this Dec 15, 2022
@David-Chadwick
Copy link
Contributor

I think the whole section

For backward compatibility with JWT processors, the following registered JWT claim names MUST be used, instead of or in addition to, their respective standard verifiable credential counterparts:

exp MUST represent the expirationDate property , encoded as a UNIX timestamp ( NumericDate ).
iss MUST represent the issuer property of a verifiable credential or the holder property of a verifiable presentation.
nbf MUST represent issuanceDate , encoded as a UNIX timestamp ( NumericDate ).
jti MUST represent the id property of the verifiable credential or verifiable presentation .
sub MUST represent the id property contained in the credentialSubject .

should be removed and the new JWT-VC spec should create the JWT independently of the contents of the VC or VP. In other words the signer of the JWT does not need to be the issuer of the VC but is whoever the signing entity is. Similarly the expiry date of the JWT is independent of the expirationDate of the credential. I think the new rules should be as follows

exp MUST represent the expiration date of the JWT (and is independent of the expirationDate of the VC/VP)
iss MUST represent the issuer of the JWT (and is independent of the issuer of the VC/VP)
nbf MUST represent the time the JWT is issued (i.e. now)
jti MUST represent the id of the JWT (and is independent of the id property of the VC/VP)

sub is the most difficult one to decide. First VCs do not need to contain a subject.id property, so the current text cannot always apply. So we could say something like "sub MUST represent the subject of the JWT. Normally this will be the subject identifier of the VC or the holder identifier of the VP, but it could be a local identifier used by the issuer to identify the subject". Comments?

@OR13
Copy link
Contributor Author

OR13 commented Jan 16, 2023

I think the whole section

For backward compatibility with JWT processors, the following registered JWT claim names MUST be used, instead of or in addition to, their respective standard verifiable credential counterparts:

exp MUST represent the expirationDate property , encoded as a UNIX timestamp ( NumericDate ).
iss MUST represent the issuer property of a verifiable credential or the holder property of a verifiable presentation.
nbf MUST represent issuanceDate , encoded as a UNIX timestamp ( NumericDate ).
jti MUST represent the id property of the verifiable credential or verifiable presentation .
sub MUST represent the id property contained in the credentialSubject .

should be removed and the new JWT-VC spec should create the JWT independently of the contents of the VC or VP. In other words the signer of the JWT does not need to be the issuer of the VC but is whoever the signing entity is. Similarly the expiry date of the JWT is independent of the expirationDate of the credential. I think the new rules should be as follows

exp MUST represent the expiration date of the JWT (and is independent of the expirationDate of the VC/VP) iss MUST represent the issuer of the JWT (and is independent of the issuer of the VC/VP) nbf MUST represent the time the JWT is issued (i.e. now) jti MUST represent the id of the JWT (and is independent of the id property of the VC/VP)

sub is the most difficult one to decide. First VCs do not need to contain a subject.id property, so the current text cannot always apply. So we could say something like "sub MUST represent the subject of the JWT. Normally this will be the subject identifier of the VC or the holder identifier of the VP, but it could be a local identifier used by the issuer to identify the subject". Comments?

A topic for an issue probably... The goal of this PR was to clarify the existing behavior, not introduce new behavior.

@David-Chadwick
Copy link
Contributor

V2 is allowed to make breaking changes and this is what I am proposing for the JWT spec. I think we all agree that the current v1.1 spec is problematical (or even broken) so whilst I can see the point in clarifying how v1.1 should work (as Implementation Guidelines to v1.1) I think v2 should fix things based on the v2 DM (which will also have breaking changes with v1.1)

@OR13
Copy link
Contributor Author

OR13 commented Jan 17, 2023

@David-Chadwick indeed, v2 can make breaking changes... thats not the intention of this PR.

This PR attempts to show exactly how broken v1.1 is, so that we can decide where to break from it for V2.

@OR13
Copy link
Contributor Author

OR13 commented Jan 17, 2023

@brentzundel @Sakurann can we please merge this PR.

@David-Chadwick
Copy link
Contributor

This PR attempts to show exactly how broken v1.1 is, so that we can decide where to break from it for V2.

Now you have confused me. Surely this document is for the v2 DM only? If so then why are we creating v1.1 PRs in it? If you want clarifying text for v1.1 then you should do a PR on that document shouldn't you?
I think this PR should be dropped and we should only have production rules for V2 in it. (But we might want an Appendix detailing the differences between V2 and V1.1)

Copy link
Collaborator

@selfissued selfissued left a comment

Choose a reason for hiding this comment

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

I agree that we should merge this PR at this point. I just re-reviewed it and it is purely editorial.

@OR13 OR13 merged commit 36e99ce into main Jan 17, 2023
@OR13 OR13 deleted the feat/begin-define-production-rules branch June 7, 2023 17:45
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.

None yet

6 participants