Skip to content

Commit

Permalink
feat: Improved error messages/warnings
Browse files Browse the repository at this point in the history
New: If no secret/publicKey is provided by the user, no signature check
is performed. Rather then silently skipping the signature check, this
commit introduces a warning in this case.

New: If the `exp` claim is not set and the token validation check is
skipped, a warning is shown.

New: After a successful signature validation a success message is shown.

Changed: The `InvalidAlgorithm` error message now shows the selected
algorithm and the algorithm specified in the JWT header.

New: If no signature validation is performed, the return code is now
`2`.

All tests have been updated accordingly.
  • Loading branch information
codedust committed Jul 11, 2021
1 parent 0de1342 commit cb37030
Show file tree
Hide file tree
Showing 2 changed files with 100 additions and 60 deletions.
93 changes: 65 additions & 28 deletions src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -465,7 +465,7 @@ fn encode_token(matches: &ArgMatches) -> JWTResult<String> {
fn decode_token(
matches: &ArgMatches,
) -> (
JWTResult<TokenData<Payload>>,
Option<JWTResult<TokenData<Payload>>>,
JWTResult<TokenData<Payload>>,
OutputFormat,
) {
Expand Down Expand Up @@ -506,8 +506,12 @@ fn decode_token(

(
match secret {
Some(secret_key) => decode::<Payload>(&jwt, &secret_key.unwrap(), &secret_validator),
None => dangerous_insecure_decode::<Payload>(&jwt),
Some(secret_key) => Some(decode::<Payload>(
&jwt,
&secret_key.unwrap(),
&secret_validator,
)),
None => None, // unable to safely decode token => validated_token is set to None
},
token_data,
if matches.is_present("json") {
Expand Down Expand Up @@ -540,8 +544,9 @@ fn print_encoded_token(token: JWTResult<String>) {
}

fn print_decoded_token(
validated_token: JWTResult<TokenData<Payload>>,
validated_token: Option<JWTResult<TokenData<Payload>>>,
token_data: JWTResult<TokenData<Payload>>,
options_algorithm: Algorithm,
format: OutputFormat,
) {
let should_validate_exp = if let Ok(token) = &token_data {
Expand All @@ -550,62 +555,89 @@ fn print_decoded_token(
false
};

if let Err(err) = &validated_token {
match err.kind() {
match validated_token {
Some(Err(ref err)) => match err.kind() {
ErrorKind::InvalidToken => {
println!("{}", Red.bold().paint("The JWT provided is invalid"))
println!(
"{}",
Red.bold().paint("Error: The JWT provided is invalid.")
)
}
ErrorKind::InvalidSignature => eprintln!(
"{}",
Red.bold()
.paint("The JWT provided has an invalid signature",)
.paint("Error: The JWT provided has an invalid signature",)
),
ErrorKind::InvalidRsaKey => eprintln!(
"{}",
Red.bold()
.paint("The secret provided isn't a valid RSA key",)
.paint("Error: The secret provided isn't a valid RSA key.",)
),
ErrorKind::InvalidEcdsaKey => eprintln!(
"{}",
Red.bold()
.paint("The secret provided isn't a valid ECDSA key",)
.paint("Error: The secret provided isn't a valid ECDSA key.",)
),
ErrorKind::ExpiredSignature => {
if should_validate_exp {
println!("{}", Red.bold().paint("The token has expired"))
println!("{}", Red.bold().paint("Error: The token has expired."))
} else {
println!(
"{}",
Red.bold().paint(
"Warning: The `exp` claim is not set. Skipping token expiration check."
)
)
}
}
ErrorKind::InvalidIssuer => {
println!("{}", Red.bold().paint("The token issuer is invalid"))
println!(
"{}",
Red.bold().paint("Error: The token issuer is invalid.")
)
}
ErrorKind::InvalidAudience => eprintln!(
"{}",
Red.bold()
.paint("The token audience doesn't match the subject",)
.paint("Error: The token audience doesn't match the subject.",)
),
ErrorKind::InvalidSubject => eprintln!(
"{}",
Red.bold()
.paint("The token subject doesn't match the audience",)
.paint("Error: The token subject doesn't match the audience.",)
),
ErrorKind::ImmatureSignature => eprintln!(
"{}",
Red.bold()
.paint("The `nbf` claim is in the future which isn't allowed",)
.paint("Error: The `nbf` claim is in the future which isn't allowed.",)
),
ErrorKind::InvalidAlgorithm => eprintln!(
"{}",
Red.bold().paint(
"The JWT provided has a different signing algorithm than the one you \
provided",
ErrorKind::InvalidAlgorithm => {
let jwt_algorithm = match token_data {
Ok(ref token) => token.header.alg,
Err(_) => panic!("Error: Invalid token data."),
};
eprintln!(
"{}",
Red.bold().paint(format!(
"Error: Invalid signature! The JWT provided has a different signing \
algorithm ({:?}) than the one selected for validation ({:?}).",
jwt_algorithm, options_algorithm
))
)
),
}
_ => eprintln!(
"{} {:?}",
Red.bold().paint("The JWT provided is invalid because"),
Red.bold()
.paint("Error: The JWT provided is invalid because"),
err
),
};
},
Some(Ok(_)) => eprintln!("{}", Green.bold().paint("Success! JWT signature is valid!")),
None => eprintln!(
"{}",
Red.bold()
.paint("Warning! JWT signature has not been validated!")
),
}

match (format, token_data) {
Expand All @@ -622,11 +654,12 @@ fn print_decoded_token(
}

exit(match validated_token {
Err(err) => match (err.kind(), should_validate_exp) {
(ErrorKind::ExpiredSignature, false) => 0,
_ => 1,
Some(Err(err)) => match (err.kind(), should_validate_exp) {
(ErrorKind::ExpiredSignature, false) => 0, // signature expired, but expiration time should be ignored
_ => 1, // token validation error
},
Ok(_) => 0,
Some(Ok(_)) => 0, // successful signature check
None => 2, // no signature check performed
})
}

Expand All @@ -644,7 +677,11 @@ fn main() {
("decode", Some(decode_matches)) => {
let (validated_token, token_data, format) = decode_token(&decode_matches);

print_decoded_token(validated_token, token_data, format);
let options_algorithm = translate_algorithm(SupportedAlgorithms::from_string(
decode_matches.value_of("algorithm").unwrap(),
));

print_decoded_token(validated_token, token_data, options_algorithm, format);
}
_ => (),
}
Expand Down
67 changes: 35 additions & 32 deletions tests/jwt-cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -228,9 +228,9 @@ mod tests {
let decode_matches = decode_matcher.subcommand_matches("decode").unwrap();
let (decoded_token, _, _) = decode_token(&decode_matches);

assert!(decoded_token.is_ok());
assert!(decoded_token.as_ref().unwrap().is_ok());

let TokenData { claims, header } = decoded_token.unwrap();
let TokenData { claims, header } = decoded_token.unwrap().unwrap();

assert_eq!(header.alg, Algorithm::HS256);
assert_eq!(header.kid, Some("1234".to_string()));
Expand Down Expand Up @@ -262,9 +262,9 @@ mod tests {
let decode_matches = decode_matcher.subcommand_matches("decode").unwrap();
let (decoded_token, _, _) = decode_token(&decode_matches);

assert!(decoded_token.is_ok());
assert!(decoded_token.as_ref().unwrap().is_ok());

let TokenData { claims, header: _ } = decoded_token.unwrap();
let TokenData { claims, header: _ } = decoded_token.unwrap().unwrap();
let iat = from_value::<i64>(claims.0["iat"].clone());

assert!(iat.is_ok());
Expand All @@ -284,7 +284,7 @@ mod tests {
let decode_matches = decode_matcher.subcommand_matches("decode").unwrap();
let (decoded_token, token_data, _) = decode_token(&decode_matches);

assert!(decoded_token.is_err());
assert!(decoded_token.as_ref().unwrap().is_err());

let TokenData { claims, header: _ } = token_data.unwrap();

Expand All @@ -304,9 +304,9 @@ mod tests {
let decode_matches = decode_matcher.subcommand_matches("decode").unwrap();
let (decoded_token, _, _) = decode_token(&decode_matches);

assert!(decoded_token.is_ok());
assert!(decoded_token.as_ref().unwrap().is_ok());

let TokenData { claims, header: _ } = decoded_token.unwrap();
let TokenData { claims, header: _ } = decoded_token.unwrap().unwrap();
let exp = from_value::<i64>(claims.0["exp"].clone());

assert!(exp.is_ok());
Expand All @@ -333,9 +333,9 @@ mod tests {
let decode_matches = decode_matcher.subcommand_matches("decode").unwrap();
let (decoded_token, _, _) = decode_token(&decode_matches);

assert!(decoded_token.is_ok());
assert!(decoded_token.as_ref().unwrap().is_ok());

let TokenData { claims, header: _ } = decoded_token.unwrap();
let TokenData { claims, header: _ } = decoded_token.unwrap().unwrap();

assert!(claims.0.get("iat").is_none());
}
Expand All @@ -361,9 +361,9 @@ mod tests {
let decode_matches = decode_matcher.subcommand_matches("decode").unwrap();
let (decoded_token, _, _) = decode_token(&decode_matches);

assert!(decoded_token.is_ok());
assert!(decoded_token.as_ref().unwrap().is_ok());

let TokenData { claims, header: _ } = decoded_token.unwrap();
let TokenData { claims, header: _ } = decoded_token.unwrap().unwrap();
let exp_claim = from_value::<i64>(claims.0["exp"].clone());

assert!(exp_claim.is_ok());
Expand All @@ -390,9 +390,9 @@ mod tests {
let decode_matches = decode_matcher.subcommand_matches("decode").unwrap();
let (decoded_token, _, _) = decode_token(&decode_matches);

assert!(decoded_token.is_ok());
assert!(decoded_token.as_ref().unwrap().is_ok());

let TokenData { claims, header: _ } = decoded_token.unwrap();
let TokenData { claims, header: _ } = decoded_token.unwrap().unwrap();
let exp_claim = from_value::<i64>(claims.0["exp"].clone());
let iat_claim = from_value::<i64>(claims.0["iat"].clone());

Expand Down Expand Up @@ -426,9 +426,9 @@ mod tests {
let decode_matches = decode_matcher.subcommand_matches("decode").unwrap();
let (decoded_token, _, _) = decode_token(&decode_matches);

assert!(decoded_token.is_ok());
assert!(decoded_token.as_ref().unwrap().is_ok());

let TokenData { claims, header: _ } = decoded_token.unwrap();
let TokenData { claims, header: _ } = decoded_token.unwrap().unwrap();
let nbf_claim = from_value::<i64>(claims.0["nbf"].clone());
let iat_claim = from_value::<i64>(claims.0["iat"].clone());

Expand Down Expand Up @@ -457,7 +457,7 @@ mod tests {
let decode_matches = matches.subcommand_matches("decode").unwrap();
let (result, _, _) = decode_token(&decode_matches);

assert!(result.is_ok());
assert!(result.unwrap().is_ok());
}

#[test]
Expand All @@ -471,9 +471,10 @@ mod tests {
])
.unwrap();
let decode_matches = matches.subcommand_matches("decode").unwrap();
let (result, _, format) = decode_token(&decode_matches);
let (validated_token, token_data, format) = decode_token(&decode_matches);

assert!(result.is_ok());
assert!(validated_token.is_none()); // no signature validation
assert!(token_data.is_ok());
assert!(format == OutputFormat::Json);
}

Expand All @@ -493,7 +494,7 @@ mod tests {
let decode_matches = matches.subcommand_matches("decode").unwrap();
let (result, _, _) = decode_token(&decode_matches);

assert!(result.is_err());
assert!(result.unwrap().is_err());
}

#[test]
Expand All @@ -508,9 +509,10 @@ mod tests {
])
.unwrap();
let decode_matches = matches.subcommand_matches("decode").unwrap();
let (result, _, _) = decode_token(&decode_matches);
let (validated_token, token_data, _) = decode_token(&decode_matches);

assert!(result.is_ok());
assert!(validated_token.is_none()); // no signature validation
assert!(token_data.is_ok());
}

#[test]
Expand All @@ -523,9 +525,10 @@ mod tests {
])
.unwrap();
let decode_matches = matches.subcommand_matches("decode").unwrap();
let (result, _, _) = decode_token(&decode_matches);
let (validated_token, token_data, _) = decode_token(&decode_matches);

assert!(result.is_ok());
assert!(validated_token.is_none()); // no signature validation
assert!(token_data.is_ok());
}

#[test]
Expand All @@ -538,9 +541,10 @@ mod tests {
])
.unwrap();
let decode_matches = matches.subcommand_matches("decode").unwrap();
let (result, _, _) = decode_token(&decode_matches);
let (validated_token, token_data, _) = decode_token(&decode_matches);

assert!(result.is_ok());
assert!(validated_token.is_none()); // no signature validation
assert!(token_data.is_ok());
}

#[test]
Expand All @@ -553,9 +557,10 @@ mod tests {
])
.unwrap();
let decode_matches = matches.subcommand_matches("decode").unwrap();
let (result, _, _) = decode_token(&decode_matches);
let (validated_token, token_data, _) = decode_token(&decode_matches);

assert!(result.is_ok());
assert!(validated_token.is_none()); // no signature validation
assert!(token_data.is_ok());
}

#[test]
Expand Down Expand Up @@ -589,7 +594,7 @@ mod tests {
let decode_matches = decode_matcher.subcommand_matches("decode").unwrap();
let (result, _, _) = decode_token(&decode_matches);

assert!(result.is_ok());
assert!(result.unwrap().is_ok());
}

#[test]
Expand Down Expand Up @@ -623,9 +628,7 @@ mod tests {
let decode_matches = decode_matcher.subcommand_matches("decode").unwrap();
let (result, _, _) = decode_token(&decode_matches);

dbg!(&result);

assert!(result.is_ok());
assert!(result.unwrap().is_ok());
}

#[test]
Expand Down Expand Up @@ -659,7 +662,7 @@ mod tests {
let decode_matches = decode_matcher.subcommand_matches("decode").unwrap();
let (decoded_token, token_data, _) = decode_token(&decode_matches);

assert!(decoded_token.is_ok());
assert!(decoded_token.as_ref().unwrap().is_ok());

let TokenData { claims, header: _ } = token_data.unwrap();

Expand Down

0 comments on commit cb37030

Please sign in to comment.