profunktor / http4s-jwt-auth

:lock: Opinionated JWT authentication library for Http4s
https://http4s-jwt-auth.profunktor.dev/
121 stars 15 forks source link

JwtSecretKey takes a String #449

Closed hunterpayne closed 2 months ago

hunterpayne commented 3 months ago

Strings in the JVM can be internalized. For this reason, it has long been the case that storing cryptographic material in a Java String is verboten (forbidden). Please change the signature of JwtSecretKey from taking a String to something more appropriate for cryptographic material.

java.security.PrivateKey would probably be the best choice but others like Array[Byte] or Array[Char] are probably good choices too.

froth commented 3 months ago

Thanks for opening this issue, I agree. Would you be interested in opening a pull request?

@gvolpe this is a breaking change, we would have to release 2.0.0. Is there something else we should change if a breaking release happens anyways?

hunterpayne commented 3 months ago

Which type should we choose or should I do something that allows all the referenced types?

gvolpe commented 3 months ago

should I do something that allows all the referenced types?

This should be ideal if possible 👍🏽

this is a breaking change, we would have to release 2.0.0.

Indeed. I think the best way to go about it would be to deprecate the existing method that takes a String noticing it's unsafe and make the 2.0.0 release. It can then be removed in the following major release.

hunterpayne commented 3 months ago

Which branch should I make the changes on? master or 1.x?

froth commented 3 months ago

Master is pretty outdated I think. I would say base it on 1.0 for now and we will sort it out and maybe create a series/2.0 branch or so.

hunterpayne commented 3 months ago

https://github.com/profunktor/http4s-jwt-auth/pull/452

froth commented 2 months ago

fixed in #452 thanks!