Support loading jwk from json #221
Conversation
|
This is still just a draft, there is much to do regarding error handling (currently I just lazily throw |
There was a problem hiding this comment.
I added a bunch of small nitpicks and one thing I really think we should change.
One of jwt-cpp's strengths has been that its trivial to add custom algorithms and this has indeed been used in academics to research new ones.
The jwks part doesn't take that into account at all. While I get that the set of algorithms currently allowed in jwks is fixed, it might be useful to provide a way to add custom ones for research or if someone needs support of future algorithms before we add it.
Apart from that I have no major complains. Obviously documentation is non existant yet, but thats fine for a draft, just something we should keep an eye on before finishing it.
EDIT: Macos and no base seems to fail, so we need to take the user provided base64 function into account.
|
|
||
| return pkey; | ||
| } | ||
|
|
There was a problem hiding this comment.
Key should definitly have some additional introspection methods, is it an async key ? if so rsa ? or ecdsa, what size ?
I think you get the point.
include/jwt-cpp/jwt.h
Outdated
| @@ -905,6 +943,9 @@ namespace jwt { | |||
| } else | |||
| throw rsa_exception(error::rsa_error::no_key_provided); | |||
| } | |||
|
|
|||
| rsa(std::shared_ptr<EVP_PKEY> pkey, const EVP_MD* (*md)(), std::string name) | |||
There was a problem hiding this comment.
I think rather than the EVP_PKEY algorithms should take the key structure above, in order to reduce typing for the user and providing a nicer interface.
include/jwt-cpp/jwt.h
Outdated
| using alg_name = std::string; | ||
| using alg_list = std::vector<alg_name>; | ||
| using algorithms = std::unordered_map<std::string, alg_list>; | ||
| static const algorithms supported_alg = {{"RSA", {"RS256", "RS384", "RS512", "PS256", "PS384", "PS512"}}, |
There was a problem hiding this comment.
This is a problem. The entire point of the algorithm support we used to have is to easily allow user defined algorithms, either for researching new ones or adding support for custom ones. A global list of algorithms doesn't really work out for that.
| return nullptr; | ||
| } | ||
|
|
||
| if (alg_name == "RS256") { |
There was a problem hiding this comment.
Same issue as above with supported_alg.
include/jwt-cpp/jwt.h
Outdated
| ec = error::token_verification_error::wrong_algorithm; | ||
| return; | ||
| } | ||
| algs.at(algo)->verify(data, sig, ec); |
There was a problem hiding this comment.
I know this is my own code, but since we are already at changing it, this would be better:
auto alg = algs.find(algo);
if(alg == algs.end()) {
ec = error::token_verification_error::wrong_algorithm;
return;
}
alg->second->verify(data, sig, ec);Prevents the duplicate lookup.
include/jwt-cpp/jwt.h
Outdated
| std::string keyid = ""; | ||
| if (key.has_key_id()) { | ||
| keyid = key.get_key_id(); | ||
| typename keysets::const_iterator it = keys.find(keyid); |
I looked into how to make it possible to extend JWKs by custom algorithm and I ended up exposing |
kleinmrk
force-pushed
the
support-loading-jwk-from-json
branch
from
6e36d68
to
abbd94f
Compare
| new algo<jwt::algorithm::rs256>(jwt::algorithm::rs256(key.get_pkey()))); | ||
| } else if (alg_name == "RS384") { | ||
| return std::unique_ptr<algo<jwt::algorithm::rs384>>( | ||
| new algo<jwt::algorithm::rs384>(jwt::algorithm::rs384(key.get_pkey()))); |
There was a problem hiding this comment.
Please use make_unique
include/jwt-cpp/jwt.h
Outdated
| * \param jwt Token to check | ||
| * \param ec error_code filled with details on error | ||
| */ | ||
| void verify(const decoded_jwt<json_traits>& jwt, std::error_code& ec) const { |
There was a problem hiding this comment.
A lot of code moved so the diff is very confusing. It hard to tell what is new 🙊
kleinmrk
force-pushed
the
support-loading-jwk-from-json
branch
from
abbd94f
to
55b5b14
Compare
This MR enables loading keys from json to
jwkobjects. It also adds an interface to the verifier class which accepts such keys and uses them as appropriate, based on thekidandalgclaims, during token verification.So far only RSA and oct keys can be loaded from json to
jwk