----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56667/#review168040 -----------------------------------------------------------
3rdparty/libprocess/include/process/jwt.hpp Lines 13-14 (patched) <https://reviews.apache.org/r/56667/#comment240063> s/JWT_HPP/PROCESS_JWT_HPP/ 3rdparty/libprocess/include/process/jwt.hpp Lines 26-27 (patched) <https://reviews.apache.org/r/56667/#comment240067> Could you provide a comment here explaining the motivation for adding the new error type? 3rdparty/libprocess/include/process/jwt.hpp Lines 38-40 (patched) <https://reviews.apache.org/r/56667/#comment240066> Two newlines here? 3rdparty/libprocess/include/process/jwt.hpp Lines 42 (patched) <https://reviews.apache.org/r/56667/#comment240077> Should we also provide a link to RFC-7515 here? 3rdparty/libprocess/include/process/jwt.hpp Lines 124-125 (patched) <https://reviews.apache.org/r/56667/#comment240095> Fits on one line 3rdparty/libprocess/include/process/jwt.hpp Lines 134 (patched) <https://reviews.apache.org/r/56667/#comment240064> s/JWT_HPP/PROCESS_JWT_HPP/ 3rdparty/libprocess/src/jwt.cpp Lines 16 (patched) <https://reviews.apache.org/r/56667/#comment240085> I think this include should occur first? 3rdparty/libprocess/src/jwt.cpp Lines 42-43 (patched) <https://reviews.apache.org/r/56667/#comment240089> To make this text more consistent, perhaps should do "Expected 3 components in token"? 3rdparty/libprocess/src/jwt.cpp Lines 169 (patched) <https://reviews.apache.org/r/56667/#comment240116> Do you think `parse_component` would be a better name for this? 3rdparty/libprocess/src/jwt.cpp Lines 201-203 (patched) <https://reviews.apache.org/r/56667/#comment240124> It looks like this will silently ignore 'typ' headers which are not strings - should we return an error in that case? 3rdparty/libprocess/src/jwt.cpp Lines 221 (patched) <https://reviews.apache.org/r/56667/#comment240125> To be consistent with the naming above, you could use the variable name `alg_string` here. 3rdparty/libprocess/src/jwt.cpp Lines 288 (patched) <https://reviews.apache.org/r/56667/#comment240133> Should have a `break;` after this line. 3rdparty/libprocess/src/jwt.cpp Lines 303 (patched) <https://reviews.apache.org/r/56667/#comment240137> Shouldn't we use `header.typ` here? 3rdparty/libprocess/src/tests/jwt_tests.cpp Lines 38 (patched) <https://reviews.apache.org/r/56667/#comment240144> It might be more obvious what's going on, and more durable, if we use the `generate_hmac_sha256` helper here and below to make the key: ``` const string header = "NOT A VALID HEADER"; const string payload = base64::encode_url_safe("{\"exp\":9999999999,\"sub\":\"foo\"}"); const string signature = base64::encode_url_safe(generate_hmac_sha256(strings::join(".", header, payload))); const string token = strings::join(".", header, payload, signature); ``` 3rdparty/libprocess/src/tests/jwt_tests.cpp Lines 249 (patched) <https://reviews.apache.org/r/56667/#comment240150> Use `generate_hmac_hs256` here? - Greg Mann On March 6, 2017, 2:53 p.m., Jan Schlicht wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/56667/ > ----------------------------------------------------------- > > (Updated March 6, 2017, 2:53 p.m.) > > > Review request for mesos, Alexander Rojas and Greg Mann. > > > Bugs: MESOS-7001 > https://issues.apache.org/jira/browse/MESOS-7001 > > > Repository: mesos > > > Description > ------- > > JSON Web Tokens can be used to create claim-based access tokens and is > typically used for HTTP authentication. > This implementation is intended for internal use, e.g. Mesos is supposed > to only parse tokens that it also created. It doesn't fully comply with > RFC 7519. Currently the only supported cryptographic algorithm is HMAC > with SHA-256. > > > Diffs > ----- > > 3rdparty/libprocess/Makefile.am 75386184108214e67a58c328258ec204099d638c > 3rdparty/libprocess/include/process/jwt.hpp PRE-CREATION > 3rdparty/libprocess/src/jwt.cpp PRE-CREATION > 3rdparty/libprocess/src/tests/jwt_tests.cpp PRE-CREATION > > > Diff: https://reviews.apache.org/r/56667/diff/6/ > > > Testing > ------- > > make check > > > Thanks, > > Jan Schlicht > >
