Conversation
|
👋 Thanks for assigning @tnull as a reviewer! |
|
Fixes #92 |
tankyleo
force-pushed
the
2026-02-drop-jsonwebtoken
branch
from
7ff7d0a to
bdd21e1
Compare
Note you'll have to put that in the OP otherwise it doesn't link the issue. |
rust/auth-impls/Cargo.toml
Outdated
| base64 = { version = "0.22.1", optional = true, default-features = false, features = ["std"] } | ||
| bitcoin_hashes = { version = "0.19", optional = true, default-features = false } | ||
| hex-conservative = { version = "1.0", optional = true, default-features = false } | ||
| rsa = { version = "0.9.10", optional = true, default-features = false, features = ["sha2"] } |
There was a problem hiding this comment.
We already have openssl in our tree - I do wonder if we could use openssl::rsa (https://docs.rs/openssl/latest/openssl/rsa/index.html) rather than this additional dependency?
There was a problem hiding this comment.
Thank you yes great idea
tnull
left a comment
tnull
left a comment
There was a problem hiding this comment.
Thanks for the update, some minor comments, otherwise looks good.
However, I think vss-server in general gets into the realm where it could use some some fuzzing or proptest coverage to ensure that our implementations behave as expected, are safe, never panic, etc.
|
|
||
| const BEARER_PREFIX: &str = "Bearer "; | ||
|
|
||
| fn parse_public_key_pem(pem: &str) -> Result<PKey<Public>, String> { |
There was a problem hiding this comment.
Should we just return () or, if you prefer, a proper enum error type here? Using a string introduces some unnecessary allocations, and they can't easily be handled. Same goes for new then ofc.
| serde_json = { version = "1.0.149", optional = true, default-features = false, features = ["std"] } | ||
|
|
||
| [dev-dependencies] | ||
| jsonwebtoken = { version = "9.3.0", default-features = false, features = ["use_pem"] } |
There was a problem hiding this comment.
If we already keep it as a dev dependency, can we add some parity/backwards compat tests that ensure our new custom implementation behaves the same way as this did?
3 participants
Fixes #92.