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

followup to #226, further clarifying semantics between jwt claims and vc properties #235

Closed
TallTed opened this issue Feb 15, 2024 · 1 comment · Fixed by #236
Closed
Assignees

Comments

@TallTed
Copy link
Member

TallTed commented Feb 15, 2024

Originally posted by @TallTed in #226 (comment)

Edited somewhat for posting as a new issue.

Answering @David-Chadwick's #226 (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 --

        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`.

-- in contrast to your suggestion --

        Implementers SHOULD avoid setting JWT claims to values that conflict with
        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`.

I suggest the following --

        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 this change should only touch this paragraph, is editorial, and hopefully is not controversial.

@David-Chadwick
Copy link
Contributor

@TallTed I like your improved text, thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants