Skip to content

remove securing json #88

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

Merged
merged 10 commits into from
Aug 11, 2023
Merged

remove securing json #88

merged 10 commits into from
Aug 11, 2023

Conversation

mprorock
Copy link
Contributor

@mprorock mprorock commented May 22, 2023

Remove securing json section - focus on securing core data model


Preview | Diff

Remove securing json section - focus on securing core data model
@mprorock mprorock requested a review from OR13 May 22, 2023 20:52
@mprorock
Copy link
Contributor Author

This will let us focus very clearly on securing the core data model section and have something very usable. This is obviously not to say we don't want to secure JSON with registered claims, but that we can do that elsewhere or revisit later

Copy link
Contributor

@Sakurann Sakurann left a comment

Choose a reason for hiding this comment

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

It should be the other way around. There is sufficient market interest in having a vanilla JSON payload in VC-JWT.

@iherman
Copy link
Member

iherman commented May 25, 2023

The issue was discussed in a meeting on 2023-05-24

  • no resolutions were taken
View the transcript

1.4. remove securing json (pr vc-jwt#88)

See github pull request vc-jwt#88.

Michael Prorock: We have a valid change request in -- for PR #88 -- but it will need discussion.
… This one does something potentially controversial. It removes the securing JSON section. It makes VC-JWT focus only on securing the core data model with no transformations or mappings, nothing else.
… Kristina has mentioned market interest in vanilla JSON claims in VC-JWT.
… So I've raised that to get comments from the community.

Kristina Yasuda: Any comments on that?

Orie Steele: Regarding removing VC-JWT media type and securing plain JSON -- I'm in favor of this PR. I'm in favor of this based on the day 3 F2F resolution and the work load for this group.
… We have several technical recommendations to work on and it's very unlikely to get consensus on the day 3 resolution in the core data model and without that consensus we will not be able to elevate this item to the point where it will go into CR.
… Without adding normative statements that will be contentious and it won't be able to get merged and I foresee this not going anywhere as long as this section stays in the document. I'm making that assessment based on the rate of engagement and so on.

Dave Longley: +1 to remove the section.

Phillip Long: +1 to remove this section.

Kristina Yasuda: Any other PRs?

@OR13 OR13 mentioned this pull request Jun 1, 2023
@paulfdietrich
Copy link

This change might also necessitate some removal in A.1 and A.3, as well as A4.1. I think these sections would be confusing otherwise.

@OR13
Copy link
Contributor

OR13 commented Jun 3, 2023

@mprorock can you apply @paulfdietrich comment? And remove those appendix sections?

@mprorock
Copy link
Contributor Author

mprorock commented Jun 5, 2023

@mprorock can you apply @paulfdietrich comment? And remove those appendix sections?

done, thanks for the careful read @paulfdietrich

@OR13
Copy link
Contributor

OR13 commented Jun 6, 2023

It should be the other way around. There is sufficient market interest in having a vanilla JSON payload in VC-JWT.

@Sakurann we are not chartered to secure arbitrary JSON, we are chartered to secure "vc+ld+json".

@iherman please confirm the charter details, if I am incorrect, we have wasted a tremendous amount of time, over objections that are unfounded.

@OR13 OR13 added the tag-needs-resolution Issue the Technical Architecture Group has raised and looks for a response on. label Jun 6, 2023
@brentzundel
Copy link
Member

@OR13 our scope is sufficiently broad enough to support specifying how to secure arbitrary JSON using VC-JWT (in addition to how to secure vc+ld+json), but that isn't the same as having consensus to do so.

@OR13
Copy link
Contributor

OR13 commented Jun 6, 2023

From my perspective there is no consensus to define a normative mapping, or to retain optional mappings with no normative teeth... this is the same as admitting there is no consensus to secure JSON, or any other content types in this working group.

@brentzundel @Sakurann please establish consensus on this topic, editors can't advance the document to horizontal review without it.

@OR13
Copy link
Contributor

OR13 commented Jun 7, 2023

Conflict free version is here: https://github.com/w3c/vc-jwt/pull/102/files

Copy link
Contributor

@OR13 OR13 left a comment

Choose a reason for hiding this comment

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

conflicts exist

Copy link
Contributor

@OR13 OR13 left a comment

Choose a reason for hiding this comment

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

seems github lag caused this to not be updated for a while, conflcts are 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.

I was wrong to previously approve this. This PR removes the normative language for VCs using JWT Claims Sets and the example mapping. We should keep both.

This PR should be closed without being merged.

@iherman
Copy link
Member

iherman commented Jun 8, 2023

It should be the other way around. There is sufficient market interest in having a vanilla JSON payload in VC-JWT.

@Sakurann we are not chartered to secure arbitrary JSON, we are chartered to secure "vc+ld+json".

@iherman please confirm the charter details, if I am incorrect, we have wasted a tremendous amount of time, over objections that are unfounded.

I do not see any charter issues. The charter does not mention media types, only VC-s in general.

@OR13 OR13 added blocked-by-w3c-administrative-process Awaiting action from W3C Staff and removed tag-needs-resolution Issue the Technical Architecture Group has raised and looks for a response on. pending-wg-review labels Jul 19, 2023
@OR13
Copy link
Contributor

OR13 commented Jul 19, 2023

I removed the labels related to external review... I am hopeful we can clean this up eventually.

@w3cbot w3cbot added the tag-needs-resolution Issue the Technical Architecture Group has raised and looks for a response on. label Jul 19, 2023
@OR13 OR13 removed the blocked-by-w3c-administrative-process Awaiting action from W3C Staff label Jul 21, 2023
@OR13
Copy link
Contributor

OR13 commented Aug 1, 2023

Conflicts exist.

I suggest we merge this PR before merging any others, its making it very difficult to maintain the spec, and the sections in question are not needed.

@TallTed
Copy link
Member

TallTed commented Aug 2, 2023

@mprorock

It appears that we need you to resolve the conflicts on this PR, and then maybe @selfissued (who still has an open change request, according to the reviewers list) should re-review, so it can be merged...

@mprorock
Copy link
Contributor Author

mprorock commented Aug 2, 2023

@OR13 please double check that i resolved conflicts correctly - as you noted this PR is a blocker and I really can't do any work until this is in.

Copy link
Contributor

@OR13 OR13 left a comment

Choose a reason for hiding this comment

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

This PR removes normative sections of the spec related to Key Discovery and Registered Claims... those sections should be preserved.

Copy link
Contributor

@OR13 OR13 left a comment

Choose a reason for hiding this comment

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

Seems correct.

@mprorock
Copy link
Contributor Author

mprorock commented Aug 2, 2023

Chairs - @brentzundel @Sakurann can we get consensus on this so we can merge - otherwise we are blocked from further work on prepping this item for CR

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.

https://datatracker.ietf.org/doc/draft-terbu-oauth-sd-jwt-vc/ has been adopted by the OAuth working group. Quoting from its introduction:

This specification defines Verifiable Credentials based on the SD-JWT format with a JWT Claim Set. It can be used when there are no selective disclosable claims, too.

Doing JWT VCs in the OAuth working group - the home of both JWTs and SD-JWTs - makes sense. Since that work will happen there, I'm now OK with this PR being merged here.

@selfissued selfissued merged commit 1e19172 into main Aug 11, 2023
@decentralgabe decentralgabe deleted the feature/remove-sec-json branch February 26, 2024 20:06
@plehegar plehegar added tag-tracker Group bringing to attention of the TAG, or tracked by the TAG but not needing response. and removed tag-needs-resolution Issue the Technical Architecture Group has raised and looks for a response on. labels Apr 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tag-tracker Group bringing to attention of the TAG, or tracked by the TAG but not needing response.
Projects
None yet
Development

Successfully merging this pull request may close these issues.