oauth-wg / oauth-selective-disclosure-jwt

https://datatracker.ietf.org/doc/draft-ietf-oauth-selective-disclosure-jwt/
Other
55 stars 27 forks source link

Provide ABNF #393

Closed bifurcation closed 1 month ago

bifurcation commented 7 months ago

It seems like it could be helpful to implementors, allowing them to quickly validate whether what they have is syntactically an SD-JWT or an SD-JWT with key binding. Something like:

base64url ::= ALPHA / DIGIT / "-" / "_"
JWT ::= base64url "." base64url "." base64url
SD-JWT ::= JWT "~" [base64url "~"]*
Fnord ::= SD-JWT JWT
bc-pi commented 5 months ago

It seems like it could be helpful to implementors, allowing them to quickly validate whether what they have is syntactically an SD-JWT or an SD-JWT with key binding. Something like:

base64url ::= ALPHA / DIGIT / "-" / "_"
JWT ::= base64url "." base64url "." base64url
SD-JWT ::= JWT "~" [base64url "~"]*
Fnord ::= SD-JWT JWT

The actual helpfulness of ABNF IMHO really depends on the readers familiarity with ABNF. I don't know that such familiarity is particularity prevalent. But, as long as it's correct, it doesn't hurt to include either. And while I'm not overly familiar with ABNF myself, I know enough to know that that isn't valid ABNF and doesn't quite correctly convey the SD-JWT constructs. I've endeavored* to fix it up but am not 100% sure this is correct either:

ALPHA = %x41-5A / %x61-7A ; A-Z / a-z
DIGIT = %x30-39 ; 0-9
base64url = 1*(ALPHA / DIGIT / "-" / "_")
JWT = base64url "." base64url "." base64url
SD-JWT = JWT "~" *[base64url "~"]
SD-JWT-KB = SD-JWT JWT

* with a bit of help from https://author-tools.ietf.org/abnf

danielfett commented 5 months ago

Looks good to me, thank you!

Maybe a small improvement would be to introduce a name for disclosure?

ALPHA = %x41-5A / %x61-7A ; A-Z / a-z
DIGIT = %x30-39 ; 0-9
base64url = 1*(ALPHA / DIGIT / "-" / "_")
JWT = base64url "." base64url "." base64url
DISCLOSURE = base64url
SD-JWT = JWT "~" *[DISCLOSURE "~"]
SD-JWT-KB = SD-JWT JWT
bc-pi commented 5 months ago

That's a good improvement, thanks!

bifurcation commented 5 months ago

Looks good to me, and appears valid according to the IETF ABNF parser.

Sakurann commented 5 months ago

(SD-JWT-KB part of the ABNF depends on another PR)

Sakurann commented 4 months ago

need to wait after #394 is resolved to do a PR.

bc-pi commented 1 month ago

maybe add a KB-JWT line to be even more better? a la:

ALPHA = %x41-5A / %x61-7A ; A-Z / a-z
DIGIT = %x30-39 ; 0-9
base64url = 1*(ALPHA / DIGIT / "-" / "_")
JWT = base64url "." base64url "." base64url
DISCLOSURE = base64url
SD-JWT = JWT "~" *[DISCLOSURE "~"]
KB-JWT = JWT
SD-JWT-KB = SD-JWT KB-JWT