Improve sub claim description in transaction tokens
#279
Conversation
Enhanced the description of the `sub` claim to clarify its uniqueness and association with the `aud` Trust Domain. This addresses issue oauth-wg#225.
|
|
||
| `sub`: | ||
| : REQUIRED A unique identifier for the subject within the context of the `aud` Trust Domain. | ||
| : REQUIRED This claim represents the principal of the transaction as defined by Section 4.1.2 of {{RFC7519}}. The value MUST be unique within the context of the `aud` Trust Domain. Unlike OpenID Connect, the `sub` claim is NOT associated with the `iss` claim. |
There was a problem hiding this comment.
Unlike OpenID Connect, the
subclaim is NOT associated with theissclaim.
We got comments in WGLC about the appropriateness fo describing wht something is not. Is there a specific reason to call this out here (I have a recollection of removing a similar sentence earlier)?
There was a problem hiding this comment.
That clause was added after the long discussion about how the aud claim defines the trust domain and the context in which the sub identifier is unique. I think it's helpful because the default from an OpenID perspective is that sub is only guaranteed to be unique within the context of the iss claim. If developers bring this perspective to the implementation it may cause issues. I suppose that statement could be moved to a "security considerations" section instead of inline. However, that line is not new to the definition. It is currently in draft-04 and to my knowledge there wasn't a comment to remove it.
There was a problem hiding this comment.
Hmmm.... somehow I have a memory of it being objected to, yet have no proof....
OK, perhaps something like this
| : REQUIRED This claim represents the principal of the transaction as defined by Section 4.1.2 of {{RFC7519}}. The value MUST be unique within the context of the `aud` Trust Domain. Unlike OpenID Connect, the `sub` claim is NOT associated with the `iss` claim. | |
| Note: Unlike OpenID Connect, the `sub` claim is NOT associated with the `iss` claim. |
There was a problem hiding this comment.
The GitHub app on my iPad makes that suggestion look like it removes the REQUIRED and other description of what the sub claim is and just leaves the note. I'm fine labeling the comment as a note, but I think the rest of the description of the sub claim should remain. My apologies if that is what you are suggesting as well... it's just hard for me to tell:)
There was a problem hiding this comment.
I support adding the "Note:" prefix to the sentence that starts with "Unlike OpenID Connect..."
There was a problem hiding this comment.
@gffletch - yeah, I did not mean to remove the REQUIRED section. Perhaps better for you to just make the change in the PR rather than chancing it with the committing the suggestion.
There was a problem hiding this comment.
I've updated the PR (I think) with your suggested change @PieterKas.
Added `Note:` to the clause about OpenID and the `sub` claim.
3 participants
Enhanced the description of the
subclaim to clarify its uniqueness and association with theaudTrust Domain. This addresses issue #225.