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

Clarify semantics between jwt claims and vc properties #226

Merged
merged 2 commits into from
Feb 12, 2024
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion index.html
Original file line number Diff line number Diff line change
Expand Up @@ -413,13 +413,24 @@ <h2>JOSE Header Parameters and JWT Claims</h2>
defined in <a data-cite="VC-DATA-MODEL-2.0#validity-period">Validity Period</a>,
which represent the validity of the data that is being secured.
</p>
<p>
The claims and security provided by this specification are independent of the data
secured and semantics provided by the [[VC-DATA-MODEL-2.0]]. This means that while the security
features of this specification ensure data integrity and authenticity, they do
not dictate the interpretation of claim data.
</p>
<p>
Implementers SHOULD avoid setting JWT claims to values that conflict with
Copy link
Contributor

Choose a reason for hiding this comment

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

SHOULD -> MUST
otherwise if an implementation does set conflicting values what does this mean?

Copy link
Member

Choose a reason for hiding this comment

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

It means that implementer (of Issuer and/or Holder/Presenter software) SHOULD understand that they're introducing inconsistencies to their data, and that downstream implementers (of Holder/Receiver and/or Verifier software) MAY reject such JWTs and their enclosed VCs.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@David-Chadwick responding here. I agree with @TallTed's interpretation.

If @selfissued also believes this language is better as a MUST I will make a revision PR.

verifiable credential properties, especially with pairs such as
Copy link
Contributor

Choose a reason for hiding this comment

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

verifiable credential properties when the claims and properties refer to the same concept, for example,

`iss` and `issuer`, `jti` and `id`, and `sub` and `credentialSubject.id`.
Comment on lines +423 to +425
Copy link
Member

Choose a reason for hiding this comment

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

@David-Chadwick — Answering your comment --

@decentralgabe This PR was merged without addressing my requested editorial change.

I think you're referring to this comment as your "requested editorial change".

Please note that your intent was not obvious because you did not submit it as a change request using the GitHub tooling, which would have shown the existing PR content in contrast to your suggestion, as you will see below, in my suggested massage of your suggestion:

Suggested change
Implementers SHOULD avoid setting JWT claims to values that conflict with
verifiable credential properties, especially with pairs such as
`iss` and `issuer`, `jti` and `id`, and `sub` and `credentialSubject.id`.
Implementers SHOULD avoid setting JWT claims to values that conflict with
the values of verifiable credential properties when a claim and property
pair refer to the same conceptual entity, especially with pairs such as
`iss` and `issuer`, `jti` and `id`, and `sub` and `credentialSubject.id`.
For example, JWK claim `iss` should not be set to a value which conflicts
with the value of verifiable credential property `issuer`.

I think a new PR (possibly starting with a new issue) will be needed to enact this (to my eyes, editorial) change, which I believe should only touch this paragraph.

Copy link
Contributor

Choose a reason for hiding this comment

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

Correct. It is only this paragraph that needs changing. But note that the GitHub tooling did not work (again) otherwise I would have used it.

Copy link
Member

Choose a reason for hiding this comment

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

note that the GitHub tooling did not work (again) otherwise I would have used it

Maybe try manually using human language and Markdown formatting around your suggested changes, something like --

I suggest line 424 above --

verifiable credential properties, especially with pairs such as

-- be changed to the following --

verifiable credential properties when the claims and properties refer to the same concept, for example,

I've created issue #235 to track this going forward.

Copy link
Contributor

Choose a reason for hiding this comment

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

Many thanks

</p>
<p>
The JWT Claim Names <code>vc</code> and <code>vp</code>
MUST NOT be present.
</p>
<p>
Additional members may be present as header parameters and claims.
If they are not understood, they MUST be ignored.
If they are not understood, they MUST be ignored.
</p>
</section>
</section>
Expand Down
Loading