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.

Sign up for GitHub

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

Define a "jwt base class" for exceptions #252

Draft
wants to merge 10 commits into
base: master
Choose a base branch
from
Draft

Define a "jwt base class" for exceptions #252

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
67c26e3
define a base exception class
prince-chrismc Oct 1, 2022
c10724d
fix
prince-chrismc Oct 1, 2022
c437f82
add missing namespace
prince-chrismc Oct 1, 2022
bb6da77
fix ambiguity
prince-chrismc Oct 1, 2022
01b7e20
fix copy pasting
prince-chrismc Oct 1, 2022
180055f
missing namespace
prince-chrismc Oct 1, 2022
9a67f42
linter
prince-chrismc Oct 1, 2022
ef502ee
traits
prince-chrismc Oct 1, 2022
647449e
messing with exceptions some more
Oct 4, 2022
c465e4d
add missing error code
Oct 4, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 34 additions & 29 deletions include/jwt-cpp/jwt.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,19 +90,26 @@ namespace jwt {
* \brief Everything related to error codes issued by the library
*/
namespace error {
struct signature_verification_exception : public std::system_error {
/**
* \brief Generic base class for all JWT specific exceptions
*/
struct exception : public std::exception {
using std::exception::exception;
};

struct signature_verification_exception : exception, public std::system_error {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feels really weird cause now signature_verification_exception inherits from std::exception twice.
Could we get away with inheriting exception from std::system_error (which inherits std::runtime_error, which in turn inherits std::exception) ?

using system_error::system_error;
};
struct signature_generation_exception : public std::system_error {
struct signature_generation_exception : exception, public std::system_error {
using system_error::system_error;
};
struct rsa_exception : public std::system_error {
struct rsa_exception : exception, public std::system_error {
using system_error::system_error;
};
struct ecdsa_exception : public std::system_error {
struct ecdsa_exception : exception, public std::system_error {
using system_error::system_error;
};
struct token_verification_exception : public std::system_error {
struct token_verification_exception : exception, public std::system_error {
using system_error::system_error;
};
/**
Expand Down Expand Up @@ -362,15 +369,8 @@ namespace jwt {
}
}
} // namespace error

// FIXME: Remove
// Keep backward compat at least for a couple of revisions
using error::ecdsa_exception;
using error::rsa_exception;
using error::signature_generation_exception;
using error::signature_verification_exception;
using error::token_verification_exception;
} // namespace jwt

namespace std {
template<>
struct is_error_code_enum<jwt::error::rsa_error> : true_type {};
Expand All @@ -383,6 +383,7 @@ namespace std {
template<>
struct is_error_code_enum<jwt::error::token_verification_error> : true_type {};
} // namespace std

namespace jwt {
/**
* \brief A collection for working with certificates
Expand Down Expand Up @@ -974,7 +975,7 @@ namespace jwt {
} else if (!public_key.empty()) {
pkey = helper::load_public_key_from_string(public_key, public_key_password);
} else
throw rsa_exception(error::rsa_error::no_key_provided);
throw error::rsa_exception(error::rsa_error::no_key_provided);
}
/**
* Sign jwt data
Expand Down Expand Up @@ -1084,13 +1085,13 @@ namespace jwt {
pkey = helper::load_public_ec_key_from_string(public_key, public_key_password);
check_public_key(pkey.get());
} else {
throw ecdsa_exception(error::ecdsa_error::no_key_provided);
throw error::ecdsa_exception(error::ecdsa_error::no_key_provided);
}
if (!pkey) throw ecdsa_exception(error::ecdsa_error::invalid_key);
if (!pkey) throw error::ecdsa_exception(error::ecdsa_error::invalid_key);

size_t keysize = EVP_PKEY_bits(pkey.get());
if (keysize != signature_length * 4 && (signature_length != 132 || keysize != 521))
throw ecdsa_exception(error::ecdsa_error::invalid_key_size);
throw error::ecdsa_exception(error::ecdsa_error::invalid_key_size);
}

/**
Expand Down Expand Up @@ -1190,25 +1191,29 @@ namespace jwt {
#ifdef JWT_OPENSSL_3_0
std::unique_ptr<EVP_PKEY_CTX, decltype(&EVP_PKEY_CTX_free)> ctx(
EVP_PKEY_CTX_new_from_pkey(nullptr, pkey, nullptr), EVP_PKEY_CTX_free);
if (!ctx) { throw ecdsa_exception(error::ecdsa_error::create_context_failed); }
if (EVP_PKEY_public_check(ctx.get()) != 1) { throw ecdsa_exception(error::ecdsa_error::invalid_key); }
if (!ctx) { throw error::ecdsa_exception(error::ecdsa_error::create_context_failed); }
if (EVP_PKEY_public_check(ctx.get()) != 1) {
throw error::ecdsa_exception(error::ecdsa_error::invalid_key);
}
#else
std::unique_ptr<EC_KEY, decltype(&EC_KEY_free)> eckey(EVP_PKEY_get1_EC_KEY(pkey), EC_KEY_free);
if (!eckey) { throw ecdsa_exception(error::ecdsa_error::invalid_key); }
if (EC_KEY_check_key(eckey.get()) == 0) throw ecdsa_exception(error::ecdsa_error::invalid_key);
if (!eckey) { throw error::ecdsa_exception(error::ecdsa_error::invalid_key); }
if (EC_KEY_check_key(eckey.get()) == 0) throw error::ecdsa_exception(error::ecdsa_error::invalid_key);
#endif
}

static void check_private_key(EVP_PKEY* pkey) {
#ifdef JWT_OPENSSL_3_0
std::unique_ptr<EVP_PKEY_CTX, decltype(&EVP_PKEY_CTX_free)> ctx(
EVP_PKEY_CTX_new_from_pkey(nullptr, pkey, nullptr), EVP_PKEY_CTX_free);
if (!ctx) { throw ecdsa_exception(error::ecdsa_error::create_context_failed); }
if (EVP_PKEY_private_check(ctx.get()) != 1) { throw ecdsa_exception(error::ecdsa_error::invalid_key); }
if (!ctx) { throw error::ecdsa_exception(error::ecdsa_error::create_context_failed); }
if (EVP_PKEY_private_check(ctx.get()) != 1) {
throw error::ecdsa_exception(error::ecdsa_error::invalid_key);
}
#else
std::unique_ptr<EC_KEY, decltype(&EC_KEY_free)> eckey(EVP_PKEY_get1_EC_KEY(pkey), EC_KEY_free);
if (!eckey) { throw ecdsa_exception(error::ecdsa_error::invalid_key); }
if (EC_KEY_check_key(eckey.get()) == 0) throw ecdsa_exception(error::ecdsa_error::invalid_key);
if (!eckey) { throw error::ecdsa_exception(error::ecdsa_error::invalid_key); }
if (EC_KEY_check_key(eckey.get()) == 0) throw error::ecdsa_exception(error::ecdsa_error::invalid_key);
#endif
}

Expand Down Expand Up @@ -1314,7 +1319,7 @@ namespace jwt {
} else if (!public_key.empty()) {
pkey = helper::load_public_key_from_string(public_key, public_key_password);
} else
throw ecdsa_exception(error::ecdsa_error::load_key_bio_read);
throw error::ecdsa_exception(error::ecdsa_error::load_key_bio_read);
}
/**
* Sign jwt data
Expand Down Expand Up @@ -1449,7 +1454,7 @@ namespace jwt {
} else if (!public_key.empty()) {
pkey = helper::load_public_key_from_string(public_key, public_key_password);
} else
throw rsa_exception(error::rsa_error::no_key_provided);
throw error::rsa_exception(error::rsa_error::no_key_provided);
}

/**
Expand Down Expand Up @@ -2316,13 +2321,13 @@ namespace jwt {
/**
* Attempt to parse JSON was unsuccessful
*/
struct invalid_json_exception : public std::runtime_error {
struct invalid_json_exception : exception, public std::runtime_error {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See above, given that we dont actually set a dynamic error message, we should probably add a "invalid_json" error code and stick that in.

invalid_json_exception() : runtime_error("invalid json") {}
};
/**
* Attempt to access claim was unsuccessful
*/
struct claim_not_present_exception : public std::out_of_range {
struct claim_not_present_exception : exception, public std::out_of_range {
claim_not_present_exception() : out_of_range("claim not found") {}
};
} // namespace error
Expand Down
2 changes: 2 additions & 0 deletions tests/ClaimTest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ TEST(ClaimTest, PicoJSONTraitsAsDouble) {

TEST(ClaimTest, MapOfClaim) {
using map = jwt::details::map_of_claims<jwt::traits::kazuho_picojson>;
ASSERT_THROW(map::parse_claims(R"##(__ not json __)##"), jwt::error::exception);
ASSERT_THROW(map::parse_claims(R"##(__ not json __)##"), jwt::error::invalid_json_exception);
const map claims{
map::parse_claims(R"##({ "array": [1], "string" : "hello world", "number": 9.9, "bool": true})##")};
Expand All @@ -137,5 +138,6 @@ TEST(ClaimTest, MapOfClaim) {
ASSERT_EQ(claims.get_claim("string").as_string(), "hello world");
ASSERT_EQ(claims.get_claim("number").as_number(), 9.9);
ASSERT_EQ(claims.get_claim("bool").as_bool(), true);
ASSERT_THROW(claims.get_claim("__missing__"), jwt::error::exception);
ASSERT_THROW(claims.get_claim("__missing__"), jwt::error::claim_not_present_exception);
}
1 change: 1 addition & 0 deletions tests/JwksTest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ TEST(JwksTest, OneKeyParse) {
"kid": "123456789",
"x5t": "NjVBRjY5MDlCMUIwNzU4RTA2QzZFMDQ4QzQ2MDAyQjVDNjk1RTM2Qg"
})";
ASSERT_THROW(jwt::parse_jwk("__not_json__"), jwt::error::exception);
ASSERT_THROW(jwt::parse_jwk("__not_json__"), jwt::error::invalid_json_exception);
ASSERT_THROW(jwt::parse_jwk(R"##(["not","an","object"])##"), std::bad_cast);

Expand Down
10 changes: 5 additions & 5 deletions tests/OpenSSLErrorTest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -490,7 +490,7 @@ TEST(OpenSSLErrorTest, ExtractPubkeyFromCert) {
try {
jwt::helper::extract_pubkey_from_cert(sample_cert, "");
FAIL(); // Should never reach this
} catch (const jwt::rsa_exception& e) { ec = e.code(); }
} catch (const jwt::error::rsa_exception& e) { ec = e.code(); }
});
}

Expand All @@ -516,7 +516,7 @@ TEST(OpenSSLErrorTest, ConvertCertBase64DerToPem) {
try {
jwt::helper::convert_base64_der_to_pem(sample_cert_base64_der);
FAIL(); // Should never reach this
} catch (const jwt::rsa_exception& e) { ec = e.code(); }
} catch (const jwt::error::rsa_exception& e) { ec = e.code(); }
});
}

Expand Down Expand Up @@ -545,7 +545,7 @@ TEST(OpenSSLErrorTest, LoadPublicKeyFromString) {
try {
jwt::helper::load_public_key_from_string(rsa_pub_key, "");
FAIL(); // Should never reach this
} catch (const jwt::rsa_exception& e) { ec = e.code(); }
} catch (const jwt::error::rsa_exception& e) { ec = e.code(); }
});
}

Expand Down Expand Up @@ -582,7 +582,7 @@ TEST(OpenSSLErrorTest, LoadPublicKeyCertFromString) {
try {
jwt::helper::load_public_key_from_string(sample_cert, "");
FAIL(); // Should never reach this
} catch (const jwt::rsa_exception& e) { ec = e.code(); }
} catch (const jwt::error::rsa_exception& e) { ec = e.code(); }
});
}

Expand Down Expand Up @@ -619,7 +619,7 @@ TEST(OpenSSLErrorTest, LoadPrivateKeyFromString) {
try {
jwt::helper::load_private_key_from_string(rsa_priv_key, "");
FAIL(); // Should never reach this
} catch (const jwt::rsa_exception& e) { ec = e.code(); }
} catch (const jwt::error::rsa_exception& e) { ec = e.code(); }
});
}

Expand Down
Loading