address comments

Author: jayakasadev

benches/jwt.rs

@@ -23,7 +23,7 @@ fn bench_encode_custom_extra_headers(c: &mut Criterion) {
     let key = EncodingKey::from_secret("secret".as_ref());
     let mut extras = HashMap::with_capacity(1);
     extras.insert("custom".to_string(), "header".to_string());
-    let header = &Header { extras: Some(extras), ..Default::default() };
+    let header = &Header { extras, ..Default::default() };
 
     c.bench_function("bench_encode", |b| {
         b.iter(|| encode(black_box(header), black_box(&claim), black_box(&key)))

examples/custom_header.rs

@@ -22,7 +22,7 @@ fn main() {
     let header = Header {
         kid: Some("signing_key".to_owned()),
         alg: Algorithm::HS512,
-        extras: Some(extras),
+        extras,
         ..Default::default()
     };
 

src/crypto/mod.rs

@@ -1,3 +1,4 @@
+use ring::constant_time::verify_slices_are_equal;
 use ring::{hmac, signature};
 
 use crate::algorithms::Algorithm;
@@ -10,16 +11,6 @@ pub(crate) mod ecdsa;
 pub(crate) mod eddsa;
 pub(crate) mod rsa;
 
-/// Only used internally when signing/verifying with HMAC, to map from our enum to the Ring HMAC structs.
-fn alg_to_hmac(alg: Algorithm) -> hmac::Algorithm {
-    match alg {
-        Algorithm::HS256 => hmac::HMAC_SHA256,
-        Algorithm::HS384 => hmac::HMAC_SHA384,
-        Algorithm::HS512 => hmac::HMAC_SHA512,
-        _ => unreachable!("Tried to get HMAC alg for a non-HMAC algorithm"),
-    }
-}
-
 /// The actual HS signing + encoding
 /// Could be in its own file to match RSA/EC but it's 2 lines...
 pub(crate) fn sign_hmac(alg: hmac::Algorithm, key: &[u8], message: &[u8]) -> String {
@@ -33,9 +24,9 @@ pub(crate) fn sign_hmac(alg: hmac::Algorithm, key: &[u8], message: &[u8]) -> Str
 /// If you just want to encode a JWT, use `encode` instead.
 pub fn sign(message: &[u8], key: &EncodingKey, algorithm: Algorithm) -> Result<String> {
     match algorithm {
-        Algorithm::HS256 | Algorithm::HS384 | Algorithm::HS512 => {
-            Ok(sign_hmac(alg_to_hmac(algorithm), key.inner(), message))
-        }
+        Algorithm::HS256 => Ok(sign_hmac(hmac::HMAC_SHA256, key.inner(), message)),
+        Algorithm::HS384 => Ok(sign_hmac(hmac::HMAC_SHA384, key.inner(), message)),
+        Algorithm::HS512 => Ok(sign_hmac(hmac::HMAC_SHA512, key.inner(), message)),
 
         Algorithm::ES256 | Algorithm::ES384 => {
             ecdsa::sign(ecdsa::alg_to_ec_signing(algorithm), key.inner(), message)
@@ -83,10 +74,8 @@ pub fn verify(
     match algorithm {
         Algorithm::HS256 | Algorithm::HS384 | Algorithm::HS512 => {
             // we just re-sign the message with the key and compare if they are equal
-            let encoding_key = &EncodingKey::from_secret(key.as_bytes());
-            let key = &hmac::Key::new(alg_to_hmac(algorithm), encoding_key.inner());
-            let digest = hmac::sign(key, message);
-            Ok(hmac::verify(key, message, digest.as_ref()).is_ok())
+            let signed = sign(message, &EncodingKey::from_secret(key.as_bytes()), algorithm)?;
+            Ok(verify_slices_are_equal(signature.as_ref(), signed.as_ref()).is_ok())
         }
         Algorithm::ES256 | Algorithm::ES384 => verify_ring(
             ecdsa::alg_to_ec_verification(algorithm),

src/header.rs

@@ -69,9 +69,8 @@ pub struct Header {
     /// Any additional non-standard headers not defined in [RFC7515#4.1](https://datatracker.ietf.org/doc/html/rfc7515#section-4.1).
     /// Once serialized, all keys will be converted to fields at the root level of the header payload
     /// Ex: Dict("custom" -> "header") will be converted to "{"typ": "JWT", ..., "custom": "header"}"
-    #[serde(skip_serializing_if = "Option::is_none")]
     #[serde(flatten)]
-    pub extras: Option<HashMap<String, String>>,
+    pub extras: HashMap<String, String>,
 }
 
 impl Header {
@@ -88,7 +87,7 @@ impl Header {
             x5c: None,
             x5t: None,
             x5t_s256: None,
-            extras: None,
+            extras: Default::default(),
         }
     }
 

tests/hmac.rs

@@ -52,6 +52,7 @@ fn encode_with_custom_header() {
     .unwrap();
     assert_eq!(my_claims, token_data.claims);
     assert_eq!("kid", token_data.header.kid.unwrap());
+    assert!(token_data.header.extras.is_empty());
 }
 
 #[test]
@@ -62,9 +63,9 @@ fn encode_with_extra_custom_header() {
         company: "ACME".to_string(),
         exp: OffsetDateTime::now_utc().unix_timestamp() + 10000,
     };
-    let mut extra = HashMap::with_capacity(1);
-    extra.insert("custom".to_string(), "header".to_string());
-    let header = Header { kid: Some("kid".to_string()), extras: Some(extra), ..Default::default() };
+    let mut extras = HashMap::with_capacity(1);
+    extras.insert("custom".to_string(), "header".to_string());
+    let header = Header { kid: Some("kid".to_string()), extras, ..Default::default() };
     let token = encode(&header, &my_claims, &EncodingKey::from_secret(b"secret")).unwrap();
     let token_data = decode::<Claims>(
         &token,
@@ -74,7 +75,7 @@ fn encode_with_extra_custom_header() {
     .unwrap();
     assert_eq!(my_claims, token_data.claims);
     assert_eq!("kid", token_data.header.kid.unwrap());
-    assert_eq!("header", token_data.header.extras.unwrap().get("custom").unwrap().as_str());
+    assert_eq!("header", token_data.header.extras.get("custom").unwrap().as_str());
 }
 
 #[test]
@@ -85,10 +86,10 @@ fn encode_with_multiple_extra_custom_headers() {
         company: "ACME".to_string(),
         exp: OffsetDateTime::now_utc().unix_timestamp() + 10000,
     };
-    let mut extra = HashMap::with_capacity(1);
-    extra.insert("custom1".to_string(), "header1".to_string());
-    extra.insert("custom2".to_string(), "header2".to_string());
-    let header = Header { kid: Some("kid".to_string()), extras: Some(extra), ..Default::default() };
+    let mut extras = HashMap::with_capacity(2);
+    extras.insert("custom1".to_string(), "header1".to_string());
+    extras.insert("custom2".to_string(), "header2".to_string());
+    let header = Header { kid: Some("kid".to_string()), extras, ..Default::default() };
     let token = encode(&header, &my_claims, &EncodingKey::from_secret(b"secret")).unwrap();
     let token_data = decode::<Claims>(
         &token,
@@ -98,7 +99,7 @@ fn encode_with_multiple_extra_custom_headers() {
     .unwrap();
     assert_eq!(my_claims, token_data.claims);
     assert_eq!("kid", token_data.header.kid.unwrap());
-    let extras = token_data.header.extras.unwrap();
+    let extras = token_data.header.extras;
     assert_eq!("header1", extras.get("custom1").unwrap().as_str());
     assert_eq!("header2", extras.get("custom2").unwrap().as_str());
 }
@@ -151,7 +152,7 @@ fn decode_token_missing_parts() {
 
 #[test]
 #[wasm_bindgen_test]
-#[should_panic(expected = "missing field `exp`")]
+#[should_panic(expected = "InvalidSignature")]
 fn decode_token_invalid_signature() {
     let token =
         "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiJiQGIuY29tIiwiY29tcGFueSI6IkFDTUUifQ.wrong";