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

vc-jose-cose cannot restrict use of specific JWT Claim Names "in any JWT Claim Set" #305

Closed
TallTed opened this issue Sep 27, 2024 · 6 comments · Fixed by #307
Closed

vc-jose-cose cannot restrict use of specific JWT Claim Names "in any JWT Claim Set" #305

TallTed opened this issue Sep 27, 2024 · 6 comments · Fixed by #307
Assignees
Labels
CR1 has-pr question Further information is requested

Comments

@TallTed
Copy link
Member

TallTed commented Sep 27, 2024

Originally posted by @TallTed in #304 (comment)

Is this accurate? We're restricting use of JWT Claim Names vc and vp in any JWT Claim Set, even a JWT Claim Set that does not encode a Verifiable Credential or Verifiable Presentation?

@decentralgabe
Copy link
Collaborator

@decentralgabe decentralgabe added the question Further information is requested label Sep 27, 2024
@TallTed
Copy link
Member Author

TallTed commented Sep 27, 2024

The text that was problematic:

                  The JWT Claim Names <code>vc</code> and <code>vp</code>
                  MUST NOT be present in any JWT Claim Set.

— has been changed to this —

                  The JWT Claim Names <code>vc</code> and <code>vp</code>
                  MUST NOT be present.

My immediate issue has been mostly resolved (better text might be more explicit about when/where these JWT Claim Names are forbidden), but I am VERY concerned that there's no commit showing the change in the commit history!

@TallTed TallTed changed the title use of specific JWT Claim Names cannot be restricted in *any* JWT Claim Set, only those that are going into SD-JWT vc-jose-cose cannot restrict use of specific JWT Claim Names "in any JWT Claim Set" Sep 27, 2024
@decentralgabe
Copy link
Collaborator

@TallTed
Copy link
Member Author

TallTed commented Sep 27, 2024

That commit changed —

            The JWT Claim Names <code>vc</code> and <code>vp</code>
            MUST NOT be present in any JWT Claims Set that comprises a
            Verifiable Credential or a Verifiable Presentation.

— to —

                  The JWT Claim Names <code>vc</code> and <code>vp</code>
                  MUST NOT be present in any JWT Claim Set.

The missing commit changed present in any JWT Claim Set. to present. as is currently in the main branch.

Wormholes and alternate realities are bad.

@decentralgabe
Copy link
Collaborator

decentralgabe commented Sep 27, 2024

@TallTed I believe I found the commit here 5e36a96#diff-0eb547304658805aad788d320f10bf1f292797b5e6d745a3bf617584da017051R410

I agree this was tough to find because of squash merging, which I've disabled.

@TallTed
Copy link
Member Author

TallTed commented Sep 27, 2024

As noted on #307, that fixes line ~318.

There were two other similarly-phrased paragraphs, at line ~607 and ~1179, which I found by viewing this file on the main branch, and searching for MUST NOT be present.

(Something else changed while I was doing the above, as ~1179 is no longer present, but ~607 still remains.)

I remain concerned about other changes that may have been made, and then un-made by other squash merges, without being noticed because they're not on spots that have been brought to light by specific review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CR1 has-pr question Further information is requested
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants