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

Added support for custom content_type and signature_type (#111) #112

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

jjbarr
Copy link

@jjbarr jjbarr commented Mar 5, 2025

This is my attempt at implementing the changes discussed in #111. Please let me know if I need to do any additional fixes.

@jesselawson
Copy link

This looks great! Would you mind if I propose some readme organization changes now that we have these additions?

@jjbarr
Copy link
Author

jjbarr commented Mar 6, 2025

Fine by me!

@jedisct1
Copy link
Owner

jedisct1 commented Mar 6, 2025

Friendship among coworkers is great.

@jedisct1
Copy link
Owner

jedisct1 commented Mar 6, 2025

I'm not sure that tieing a content_type to a key is the best way to proceed, as a key can be used to sign different content types.

What would be alternatives? Store this in Claims, maybe?

@jjbarr
Copy link
Author

jjbarr commented Mar 8, 2025

It would get pretty awkward to store it in claims, since everything else in there just directly serializes to/from the claims section of the JWT.

We could have for_content_type/for_signature_type wrap the key in a struct that has its own sign /authenticate (as is appropriate for the type) method that attaches the header? That feels like a slightly cleaner approach to me.

@jjbarr
Copy link
Author

jjbarr commented Mar 12, 2025

So to sketch this out, each <algorithm>Like trait would get a sign_with_header_options()/authenticate_with_header_options() method. The methods for_content_type and for_signature_type would be on HeaderOptions, existing sign/authenticate methods would then become, for example:

fn sign<CustomClaims: Serialize + DeserializeOwned>(
    &self,
    claims: JWTClaims<CustomClaims>,
) -> Result<String, Error> {
    self.sign_with_header_options(claims, default::Default())
}

@jjbarr
Copy link
Author

jjbarr commented Mar 20, 2025

I finally had some time available, so I took a crack at implementing an API similar to what I described above. I pushed it to this branch, although maybe it should be its own pull request?

In any case, this is a bit of an experiment, so I'm entirely fine reverting it if you decide this isn't the way you want this done.

@alterstep
Copy link

sign_with_custom_enevelope?
-> more generic

@alterstep
Copy link

or only sign_with_options?

@jjbarr
Copy link
Author

jjbarr commented Mar 23, 2025

sign_with_options seems cleaner, I can change that.

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

Successfully merging this pull request may close these issues.

4 participants