Re: Review Request 66621: Add support for alg RS256 to JWT library.

2018-05-15 Thread Mesos Reviewbot 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66621/#review203199
---



Patch looks great!

Reviews applied: [66621]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose --disable-libtool-wrappers' ENVIRONMENT='GLOG_v=1 
MESOS_VERBOSE=1'; ./support/docker-build.sh

- Mesos Reviewbot


On May 15, 2018, 9:05 p.m., Clement Michaud wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66621/
> ---
> 
> (Updated May 15, 2018, 9:05 p.m.)
> 
> 
> Review request for mesos, Alexander Rojas, Greg Mann, and Till Toenshoff.
> 
> 
> Bugs: MESOS-8788
> https://issues.apache.org/jira/browse/MESOS-8788
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add support for alg RS256 to JWT library.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/jwt.hpp 
> 768cbf6fa91537ff9f45f236f4033097c5cea959 
>   3rdparty/libprocess/include/process/ssl/utilities.hpp 
> b7cc31c33fd35c93754407f8b350eeb993177f1d 
>   3rdparty/libprocess/src/jwt.cpp 921031e6fe3ced5a6be6bc96190fae6d8282ae26 
>   3rdparty/libprocess/src/ssl/utilities.cpp 
> 4d3727daf53ec62a19255da5a9804d342e770ec2 
>   3rdparty/libprocess/src/tests/jwt_keys.hpp PRE-CREATION 
>   3rdparty/libprocess/src/tests/jwt_tests.cpp 
> eb36a9aed3b11208c7cdc6f20b5347f46821a207 
> 
> 
> Diff: https://reviews.apache.org/r/66621/diff/6/
> 
> 
> Testing
> ---
> 
> make check
> 
> I added the same tests than the ones for HS256 (i.e., validation in following 
> cases: bad header, bad payload, unknown alg, unsupported alg, valid token 
> etc.. and creation of a valid token). I also added a test to verify that the 
> validation of a RS256 token fails when using the wrong public key.
> 
> 
> Thanks,
> 
> Clement Michaud
> 
>

Re: Review Request 66621: Add support for alg RS256 to JWT library.

2018-05-15 Thread Till Toenshoff 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66621/#review203192
---


Ship it!




Ship It!

- Till Toenshoff


On May 15, 2018, 9:05 p.m., Clement Michaud wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66621/
> ---
> 
> (Updated May 15, 2018, 9:05 p.m.)
> 
> 
> Review request for mesos, Alexander Rojas, Greg Mann, and Till Toenshoff.
> 
> 
> Bugs: MESOS-8788
> https://issues.apache.org/jira/browse/MESOS-8788
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add support for alg RS256 to JWT library.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/jwt.hpp 
> 768cbf6fa91537ff9f45f236f4033097c5cea959 
>   3rdparty/libprocess/include/process/ssl/utilities.hpp 
> b7cc31c33fd35c93754407f8b350eeb993177f1d 
>   3rdparty/libprocess/src/jwt.cpp 921031e6fe3ced5a6be6bc96190fae6d8282ae26 
>   3rdparty/libprocess/src/ssl/utilities.cpp 
> 4d3727daf53ec62a19255da5a9804d342e770ec2 
>   3rdparty/libprocess/src/tests/jwt_keys.hpp PRE-CREATION 
>   3rdparty/libprocess/src/tests/jwt_tests.cpp 
> eb36a9aed3b11208c7cdc6f20b5347f46821a207 
> 
> 
> Diff: https://reviews.apache.org/r/66621/diff/6/
> 
> 
> Testing
> ---
> 
> make check
> 
> I added the same tests than the ones for HS256 (i.e., validation in following 
> cases: bad header, bad payload, unknown alg, unsupported alg, valid token 
> etc.. and creation of a valid token). I also added a test to verify that the 
> validation of a RS256 token fails when using the wrong public key.
> 
> 
> Thanks,
> 
> Clement Michaud
> 
>

Re: Review Request 66621: Add support for alg RS256 to JWT library.

2018-05-15 Thread Mesos Reviewbot Windows 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66621/#review203172
---



PASS: Mesos patch 66621 was successfully built and tested.

Reviews applied: `['66621']`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66621

- Mesos Reviewbot Windows


On May 15, 2018, 9:05 p.m., Clement Michaud wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66621/
> ---
> 
> (Updated May 15, 2018, 9:05 p.m.)
> 
> 
> Review request for mesos, Alexander Rojas and Till Toenshoff.
> 
> 
> Bugs: MESOS-8788
> https://issues.apache.org/jira/browse/MESOS-8788
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add support for alg RS256 to JWT library.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/jwt.hpp 
> 768cbf6fa91537ff9f45f236f4033097c5cea959 
>   3rdparty/libprocess/include/process/ssl/utilities.hpp 
> b7cc31c33fd35c93754407f8b350eeb993177f1d 
>   3rdparty/libprocess/src/jwt.cpp 921031e6fe3ced5a6be6bc96190fae6d8282ae26 
>   3rdparty/libprocess/src/ssl/utilities.cpp 
> 4d3727daf53ec62a19255da5a9804d342e770ec2 
>   3rdparty/libprocess/src/tests/jwt_keys.hpp PRE-CREATION 
>   3rdparty/libprocess/src/tests/jwt_tests.cpp 
> eb36a9aed3b11208c7cdc6f20b5347f46821a207 
> 
> 
> Diff: https://reviews.apache.org/r/66621/diff/6/
> 
> 
> Testing
> ---
> 
> make check
> 
> I added the same tests than the ones for HS256 (i.e., validation in following 
> cases: bad header, bad payload, unknown alg, unsupported alg, valid token 
> etc.. and creation of a valid token). I also added a test to verify that the 
> validation of a RS256 token fails when using the wrong public key.
> 
> 
> Thanks,
> 
> Clement Michaud
> 
>

Re: Review Request 66621: Add support for alg RS256 to JWT library.

2018-05-15 Thread Clement Michaud 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66621/
---

(Updated mai 15, 2018, 9:05 après-midi)


Review request for mesos, Alexander Rojas and Till Toenshoff.


Changes
---

Remove useless empty line


Bugs: MESOS-8788
https://issues.apache.org/jira/browse/MESOS-8788


Repository: mesos


Description
---

Add support for alg RS256 to JWT library.


Diffs (updated)
-

  3rdparty/libprocess/include/process/jwt.hpp 
768cbf6fa91537ff9f45f236f4033097c5cea959 
  3rdparty/libprocess/include/process/ssl/utilities.hpp 
b7cc31c33fd35c93754407f8b350eeb993177f1d 
  3rdparty/libprocess/src/jwt.cpp 921031e6fe3ced5a6be6bc96190fae6d8282ae26 
  3rdparty/libprocess/src/ssl/utilities.cpp 
4d3727daf53ec62a19255da5a9804d342e770ec2 
  3rdparty/libprocess/src/tests/jwt_keys.hpp PRE-CREATION 
  3rdparty/libprocess/src/tests/jwt_tests.cpp 
eb36a9aed3b11208c7cdc6f20b5347f46821a207 


Diff: https://reviews.apache.org/r/66621/diff/6/

Changes: https://reviews.apache.org/r/66621/diff/5-6/


Testing
---

make check

I added the same tests than the ones for HS256 (i.e., validation in following 
cases: bad header, bad payload, unknown alg, unsupported alg, valid token etc.. 
and creation of a valid token). I also added a test to verify that the 
validation of a RS256 token fails when using the wrong public key.


Thanks,

Clement Michaud

Re: Review Request 66621: Add support for alg RS256 to JWT library.

2018-05-15 Thread Clement Michaud 


> On mai 15, 2018, 2:30 après-midi, Till Toenshoff wrote:
> > 3rdparty/libprocess/src/ssl/utilities.cpp
> > Lines 433 (patched)
> > 
> >
> > Would it make sense to use `ERR_reason_error_string(ERR_get_error())` 
> > instead?
> > 
> > Also note that this would print an error like this for reasons that are 
> > unknown (returning a nullptr):
> > "Failed to sign the message: " -- that looks broken. Can we please have 
> > it show no colon at all for such case? The ternary operator is what we 
> > prefer for those purposes.
> 
> Clement Michaud wrote:
> ERR_error_string gives more info about the error compared to 
> ERR_reason_error_string which is a subset, so I propose to keep 
> ERR_error_string. Moreover both options already exist in the code.
> 
> I handled the null pointer with ternary as suggested though.

Eventually I used ERR_reason_error_string because there was an example using 
the ternary in the same file so I just did the exact same thing.


- Clement


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66621/#review202608
---


On mai 15, 2018, 8:49 après-midi, Clement Michaud wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66621/
> ---
> 
> (Updated mai 15, 2018, 8:49 après-midi)
> 
> 
> Review request for mesos, Alexander Rojas and Till Toenshoff.
> 
> 
> Bugs: MESOS-8788
> https://issues.apache.org/jira/browse/MESOS-8788
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add support for alg RS256 to JWT library.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/jwt.hpp 
> 768cbf6fa91537ff9f45f236f4033097c5cea959 
>   3rdparty/libprocess/include/process/ssl/utilities.hpp 
> b7cc31c33fd35c93754407f8b350eeb993177f1d 
>   3rdparty/libprocess/src/jwt.cpp 921031e6fe3ced5a6be6bc96190fae6d8282ae26 
>   3rdparty/libprocess/src/ssl/utilities.cpp 
> 4d3727daf53ec62a19255da5a9804d342e770ec2 
>   3rdparty/libprocess/src/tests/jwt_keys.hpp PRE-CREATION 
>   3rdparty/libprocess/src/tests/jwt_tests.cpp 
> eb36a9aed3b11208c7cdc6f20b5347f46821a207 
> 
> 
> Diff: https://reviews.apache.org/r/66621/diff/5/
> 
> 
> Testing
> ---
> 
> make check
> 
> I added the same tests than the ones for HS256 (i.e., validation in following 
> cases: bad header, bad payload, unknown alg, unsupported alg, valid token 
> etc.. and creation of a valid token). I also added a test to verify that the 
> validation of a RS256 token fails when using the wrong public key.
> 
> 
> Thanks,
> 
> Clement Michaud
> 
>

Re: Review Request 66621: Add support for alg RS256 to JWT library.

2018-05-15 Thread Clement Michaud 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66621/
---

(Updated mai 15, 2018, 8:49 après-midi)


Review request for mesos, Alexander Rojas and Till Toenshoff.


Changes
---

Add period to comments, sort includes, fixes error message in openssl utilities


Bugs: MESOS-8788
https://issues.apache.org/jira/browse/MESOS-8788


Repository: mesos


Description
---

Add support for alg RS256 to JWT library.


Diffs (updated)
-

  3rdparty/libprocess/include/process/jwt.hpp 
768cbf6fa91537ff9f45f236f4033097c5cea959 
  3rdparty/libprocess/include/process/ssl/utilities.hpp 
b7cc31c33fd35c93754407f8b350eeb993177f1d 
  3rdparty/libprocess/src/jwt.cpp 921031e6fe3ced5a6be6bc96190fae6d8282ae26 
  3rdparty/libprocess/src/ssl/utilities.cpp 
4d3727daf53ec62a19255da5a9804d342e770ec2 
  3rdparty/libprocess/src/tests/jwt_keys.hpp PRE-CREATION 
  3rdparty/libprocess/src/tests/jwt_tests.cpp 
eb36a9aed3b11208c7cdc6f20b5347f46821a207 


Diff: https://reviews.apache.org/r/66621/diff/5/

Changes: https://reviews.apache.org/r/66621/diff/4-5/


Testing
---

make check

I added the same tests than the ones for HS256 (i.e., validation in following 
cases: bad header, bad payload, unknown alg, unsupported alg, valid token etc.. 
and creation of a valid token). I also added a test to verify that the 
validation of a RS256 token fails when using the wrong public key.


Thanks,

Clement Michaud

Re: Review Request 66621: Add support for alg RS256 to JWT library.

2018-05-15 Thread Clement Michaud 


> On mai 15, 2018, 2:30 après-midi, Till Toenshoff wrote:
> > 3rdparty/libprocess/src/ssl/utilities.cpp
> > Lines 433 (patched)
> > 
> >
> > Would it make sense to use `ERR_reason_error_string(ERR_get_error())` 
> > instead?
> > 
> > Also note that this would print an error like this for reasons that are 
> > unknown (returning a nullptr):
> > "Failed to sign the message: " -- that looks broken. Can we please have 
> > it show no colon at all for such case? The ternary operator is what we 
> > prefer for those purposes.

ERR_error_string gives more info about the error compared to 
ERR_reason_error_string which is a subset, so I propose to keep 
ERR_error_string. Moreover both options already exist in the code.

I handled the null pointer with ternary as suggested though.


- Clement


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66621/#review202608
---


On avr. 21, 2018, 11:22 matin, Clement Michaud wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66621/
> ---
> 
> (Updated avr. 21, 2018, 11:22 matin)
> 
> 
> Review request for mesos, Alexander Rojas and Till Toenshoff.
> 
> 
> Bugs: MESOS-8788
> https://issues.apache.org/jira/browse/MESOS-8788
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add support for alg RS256 to JWT library.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/jwt.hpp 
> 768cbf6fa91537ff9f45f236f4033097c5cea959 
>   3rdparty/libprocess/include/process/ssl/utilities.hpp 
> b7cc31c33fd35c93754407f8b350eeb993177f1d 
>   3rdparty/libprocess/src/jwt.cpp 921031e6fe3ced5a6be6bc96190fae6d8282ae26 
>   3rdparty/libprocess/src/ssl/utilities.cpp 
> 4d3727daf53ec62a19255da5a9804d342e770ec2 
>   3rdparty/libprocess/src/tests/jwt_keys.hpp PRE-CREATION 
>   3rdparty/libprocess/src/tests/jwt_tests.cpp 
> eb36a9aed3b11208c7cdc6f20b5347f46821a207 
> 
> 
> Diff: https://reviews.apache.org/r/66621/diff/4/
> 
> 
> Testing
> ---
> 
> make check
> 
> I added the same tests than the ones for HS256 (i.e., validation in following 
> cases: bad header, bad payload, unknown alg, unsupported alg, valid token 
> etc.. and creation of a valid token). I also added a test to verify that the 
> validation of a RS256 token fails when using the wrong public key.
> 
> 
> Thanks,
> 
> Clement Michaud
> 
>

Re: Review Request 66621: Add support for alg RS256 to JWT library.

2018-05-15 Thread Till Toenshoff 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66621/#review202608
---



Thanks for your contribution Clement. I appreciate how you put large efforts in 
matching the style already -- some tiny nits left.


3rdparty/libprocess/include/process/jwt.hpp
Lines 23 (patched)


Why do we need this? There is a `typedef struct rsa_st RSA;` in 
"openssl/ossl_typ.h" - commonly dragged in by "openssl/rsa.h", no?



3rdparty/libprocess/include/process/jwt.hpp
Line 108 (original), 124 (patched)


Thanks for this one! :)



3rdparty/libprocess/src/jwt.cpp
Line 18 (original)


Please leave this blank as it was.



3rdparty/libprocess/src/jwt.cpp
Lines 23 (patched)


This needs to go below the other system header, "memory".

See 
https://github.com/apache/mesos/blob/e6298aef83039dacc80b8e2a8778efacbaa63efc/docs/c%2B%2B-style-guide.md#order-of-includes



3rdparty/libprocess/src/jwt.cpp
Lines 304 (patched)


Comments should generally terminate with punctuation to make it easy for us 
readers to parse them in whole.

Add the missing trailing periods here and everywhere else in your new 
comments please.



3rdparty/libprocess/src/jwt.cpp
Lines 356-359 (patched)


Let's lighten this up for the reader by having a temporary that holds the 
concatenated message;

```
const string message = base64::encode_url_safe(stringify(header), false) + 
"." +
   base64::encode_url_safe(stringify(payload), false);

onst Try signature = sign_rsa_sha256(message, privateKey);
```



3rdparty/libprocess/src/ssl/utilities.cpp
Line 22 (original), 24 (patched)


Not yours, but could you please move this include up, below "memory"?



3rdparty/libprocess/src/ssl/utilities.cpp
Lines 30 (patched)


Please move this up below "string" (which is below "memory" then) :).



3rdparty/libprocess/src/ssl/utilities.cpp
Lines 40 (patched)


So far we did not use `using std::string;` but referenced `std::string` a 
bunch of times (see e.g. line 103). Lets make sure things remain consistent by 
adapting towards one of those directions, but entirely.



3rdparty/libprocess/src/ssl/utilities.cpp
Lines 385 (patched)


Let's compare towards a `nullptr` here please.



3rdparty/libprocess/src/ssl/utilities.cpp
Lines 390 (patched)


`if (rsa == nullptr)` would be more common within the Mesos codebase.



3rdparty/libprocess/src/ssl/utilities.cpp
Lines 433 (patched)


Would it make sense to use `ERR_reason_error_string(ERR_get_error())` 
instead?

Also note that this would print an error like this for reasons that are 
unknown (returning a nullptr):
"Failed to sign the message: " -- that looks broken. Can we please have it 
show no colon at all for such case? The ternary operator is what we prefer for 
those purposes.



3rdparty/libprocess/src/ssl/utilities.cpp
Lines 436-437 (patched)


Reformat please:

```
return string(
reinterpret_cast(signatureData.data()),
signatureLength);
```

Argument continuations have a single argument per line.



3rdparty/libprocess/src/tests/jwt_keys.hpp
Lines 17 (patched)


Lets rephrase a little here please;

```
Private and public keys used for JWT tests.
```



3rdparty/libprocess/src/tests/jwt_keys.hpp
Lines 74 (patched)


We commonly trail such `#endif` by a comment referencing the opening of the 
condition, making navigation for the reader a bit less painful, hopefully :)

```
#endif // __JWT_KEYS_HPP__
```



3rdparty/libprocess/src/tests/jwt_tests.cpp
Line 31 (original), 36-37 (patched)


Switch these two to alphabetize.



3rdparty/libprocess/src/tests/jwt_tests.cpp
Lines 324 (patched)


Period.



3rdparty/libprocess/src/tests/jwt_tests.cpp
Line 179 (original), 338 (patched)


Terminate the comment with punctuation, please - add a period here.



3rdpart

Re: Review Request 66621: Add support for alg RS256 to JWT library.

2018-04-22 Thread Mesos Reviewbot 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66621/#review201716
---



Patch looks great!

Reviews applied: [66621]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose --disable-libtool-wrappers' ENVIRONMENT='GLOG_v=1 
MESOS_VERBOSE=1'; ./support/docker-build.sh

- Mesos Reviewbot


On April 21, 2018, 11:22 a.m., Clement Michaud wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66621/
> ---
> 
> (Updated April 21, 2018, 11:22 a.m.)
> 
> 
> Review request for mesos, Alexander Rojas and Till Toenshoff.
> 
> 
> Bugs: MESOS-8788
> https://issues.apache.org/jira/browse/MESOS-8788
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add support for alg RS256 to JWT library.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/jwt.hpp 
> 768cbf6fa91537ff9f45f236f4033097c5cea959 
>   3rdparty/libprocess/include/process/ssl/utilities.hpp 
> b7cc31c33fd35c93754407f8b350eeb993177f1d 
>   3rdparty/libprocess/src/jwt.cpp 921031e6fe3ced5a6be6bc96190fae6d8282ae26 
>   3rdparty/libprocess/src/ssl/utilities.cpp 
> 4d3727daf53ec62a19255da5a9804d342e770ec2 
>   3rdparty/libprocess/src/tests/jwt_keys.hpp PRE-CREATION 
>   3rdparty/libprocess/src/tests/jwt_tests.cpp 
> eb36a9aed3b11208c7cdc6f20b5347f46821a207 
> 
> 
> Diff: https://reviews.apache.org/r/66621/diff/4/
> 
> 
> Testing
> ---
> 
> make check
> 
> I added the same tests than the ones for HS256 (i.e., validation in following 
> cases: bad header, bad payload, unknown alg, unsupported alg, valid token 
> etc.. and creation of a valid token). I also added a test to verify that the 
> validation of a RS256 token fails when using the wrong public key.
> 
> 
> Thanks,
> 
> Clement Michaud
> 
>

Re: Review Request 66621: Add support for alg RS256 to JWT library.

2018-04-21 Thread Mesos Reviewbot Windows 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66621/#review201690
---



PASS: Mesos patch 66621 was successfully built and tested.

Reviews applied: `['66621']`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66621

- Mesos Reviewbot Windows


On April 21, 2018, 11:22 a.m., Clement Michaud wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66621/
> ---
> 
> (Updated April 21, 2018, 11:22 a.m.)
> 
> 
> Review request for mesos, Alexander Rojas and Till Toenshoff.
> 
> 
> Bugs: MESOS-8788
> https://issues.apache.org/jira/browse/MESOS-8788
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add support for alg RS256 to JWT library.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/jwt.hpp 
> 768cbf6fa91537ff9f45f236f4033097c5cea959 
>   3rdparty/libprocess/include/process/ssl/utilities.hpp 
> b7cc31c33fd35c93754407f8b350eeb993177f1d 
>   3rdparty/libprocess/src/jwt.cpp 921031e6fe3ced5a6be6bc96190fae6d8282ae26 
>   3rdparty/libprocess/src/ssl/utilities.cpp 
> 4d3727daf53ec62a19255da5a9804d342e770ec2 
>   3rdparty/libprocess/src/tests/jwt_keys.hpp PRE-CREATION 
>   3rdparty/libprocess/src/tests/jwt_tests.cpp 
> eb36a9aed3b11208c7cdc6f20b5347f46821a207 
> 
> 
> Diff: https://reviews.apache.org/r/66621/diff/4/
> 
> 
> Testing
> ---
> 
> make check
> 
> I added the same tests than the ones for HS256 (i.e., validation in following 
> cases: bad header, bad payload, unknown alg, unsupported alg, valid token 
> etc.. and creation of a valid token). I also added a test to verify that the 
> validation of a RS256 token fails when using the wrong public key.
> 
> 
> Thanks,
> 
> Clement Michaud
> 
>

Re: Review Request 66621: Add support for alg RS256 to JWT library.

2018-04-21 Thread Clement Michaud 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66621/
---

(Updated avr. 21, 2018, 11:22 matin)


Review request for mesos, Alexander Rojas and Till Toenshoff.


Bugs: MESOS-8788
https://issues.apache.org/jira/browse/MESOS-8788


Repository: mesos


Description
---

Add support for alg RS256 to JWT library.


Diffs (updated)
-

  3rdparty/libprocess/include/process/jwt.hpp 
768cbf6fa91537ff9f45f236f4033097c5cea959 
  3rdparty/libprocess/include/process/ssl/utilities.hpp 
b7cc31c33fd35c93754407f8b350eeb993177f1d 
  3rdparty/libprocess/src/jwt.cpp 921031e6fe3ced5a6be6bc96190fae6d8282ae26 
  3rdparty/libprocess/src/ssl/utilities.cpp 
4d3727daf53ec62a19255da5a9804d342e770ec2 
  3rdparty/libprocess/src/tests/jwt_keys.hpp PRE-CREATION 
  3rdparty/libprocess/src/tests/jwt_tests.cpp 
eb36a9aed3b11208c7cdc6f20b5347f46821a207 


Diff: https://reviews.apache.org/r/66621/diff/4/

Changes: https://reviews.apache.org/r/66621/diff/3-4/


Testing
---

make check

I added the same tests than the ones for HS256 (i.e., validation in following 
cases: bad header, bad payload, unknown alg, unsupported alg, valid token etc.. 
and creation of a valid token). I also added a test to verify that the 
validation of a RS256 token fails when using the wrong public key.


Thanks,

Clement Michaud

Re: Review Request 66621: Add support for alg RS256 to JWT library.

2018-04-20 Thread Mesos Reviewbot 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66621/#review201613
---



Patch looks great!

Reviews applied: [66621]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose --disable-libtool-wrappers' ENVIRONMENT='GLOG_v=1 
MESOS_VERBOSE=1'; ./support/docker-build.sh

- Mesos Reviewbot


On April 19, 2018, 11:10 p.m., Clément Michaud wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66621/
> ---
> 
> (Updated April 19, 2018, 11:10 p.m.)
> 
> 
> Review request for mesos, Alexander Rojas and Till Toenshoff.
> 
> 
> Bugs: MESOS-8788
> https://issues.apache.org/jira/browse/MESOS-8788
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add support for alg RS256 to JWT library.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/jwt.hpp 
> 768cbf6fa91537ff9f45f236f4033097c5cea959 
>   3rdparty/libprocess/include/process/ssl/utilities.hpp 
> b7cc31c33fd35c93754407f8b350eeb993177f1d 
>   3rdparty/libprocess/src/jwt.cpp 921031e6fe3ced5a6be6bc96190fae6d8282ae26 
>   3rdparty/libprocess/src/ssl/utilities.cpp 
> 4d3727daf53ec62a19255da5a9804d342e770ec2 
>   3rdparty/libprocess/src/tests/jwt_keys.hpp PRE-CREATION 
>   3rdparty/libprocess/src/tests/jwt_tests.cpp 
> eb36a9aed3b11208c7cdc6f20b5347f46821a207 
> 
> 
> Diff: https://reviews.apache.org/r/66621/diff/3/
> 
> 
> Testing
> ---
> 
> make check
> 
> I added the same tests than the ones for HS256 (i.e., validation in following 
> cases: bad header, bad payload, unknown alg, unsupported alg, valid token 
> etc.. and creation of a valid token). I also added a test to verify that the 
> validation of a RS256 token fails when using the wrong public key.
> 
> 
> Thanks,
> 
> Clément Michaud
> 
>

Re: Review Request 66621: Add support for alg RS256 to JWT library.

2018-04-19 Thread Mesos Reviewbot Windows 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66621/#review201591
---



FAIL: Failed to apply the current review.

Failed command: `python.exe .\support\apply-reviews.py -n -r 66621`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66621

Relevant logs:

- 
[apply-review-66621-stderr.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66621/logs/apply-review-66621-stderr.log):

```
Traceback (most recent call last):
  File ".\support\apply-reviews.py", line 447, in 
main()
  File ".\support\apply-reviews.py", line 442, in main
reviewboard(options)
  File ".\support\apply-reviews.py", line 432, in reviewboard
apply_review(options)
  File ".\support\apply-reviews.py", line 163, in apply_review
commit_patch(options)
  File ".\support\apply-reviews.py", line 292, in commit_patch
shell(cmd, options['dry_run'])
  File ".\support\apply-reviews.py", line 147, in shell
error_code = subprocess.call(command, stderr=subprocess.STDOUT, shell=True)
  File "C:\Program Files\Python27\lib\subprocess.py", line 168, in call
return Popen(*popenargs, **kwargs).wait()
  File "C:\Program Files\Python27\lib\subprocess.py", line 390, in __init__
errread, errwrite)
  File "C:\Program Files\Python27\lib\subprocess.py", line 610, in 
_execute_child
args = '{} /c "{}"'.format (comspec, args)
UnicodeEncodeError: 'ascii' codec can't encode character u'\xe9' in position 
23: ordinal not in range(128)
```

- Mesos Reviewbot Windows


On April 19, 2018, 4:10 p.m., Clément Michaud wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66621/
> ---
> 
> (Updated April 19, 2018, 4:10 p.m.)
> 
> 
> Review request for mesos, Alexander Rojas and Till Toenshoff.
> 
> 
> Bugs: MESOS-8788
> https://issues.apache.org/jira/browse/MESOS-8788
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add support for alg RS256 to JWT library.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/jwt.hpp 
> 768cbf6fa91537ff9f45f236f4033097c5cea959 
>   3rdparty/libprocess/include/process/ssl/utilities.hpp 
> b7cc31c33fd35c93754407f8b350eeb993177f1d 
>   3rdparty/libprocess/src/jwt.cpp 921031e6fe3ced5a6be6bc96190fae6d8282ae26 
>   3rdparty/libprocess/src/ssl/utilities.cpp 
> 4d3727daf53ec62a19255da5a9804d342e770ec2 
>   3rdparty/libprocess/src/tests/jwt_keys.hpp PRE-CREATION 
>   3rdparty/libprocess/src/tests/jwt_tests.cpp 
> eb36a9aed3b11208c7cdc6f20b5347f46821a207 
> 
> 
> Diff: https://reviews.apache.org/r/66621/diff/3/
> 
> 
> Testing
> ---
> 
> make check
> 
> I added the same tests than the ones for HS256 (i.e., validation in following 
> cases: bad header, bad payload, unknown alg, unsupported alg, valid token 
> etc.. and creation of a valid token). I also added a test to verify that the 
> validation of a RS256 token fails when using the wrong public key.
> 
> 
> Thanks,
> 
> Clément Michaud
> 
>

Re: Review Request 66621: Add support for alg RS256 to JWT library.

2018-04-19 Thread Clément Michaud 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66621/
---

(Updated avr. 19, 2018, 11:10 après-midi)


Review request for mesos, Alexander Rojas and Till Toenshoff.


Changes
---

Fixed all issues reported by Alexander


Bugs: MESOS-8788
https://issues.apache.org/jira/browse/MESOS-8788


Repository: mesos


Description
---

Add support for alg RS256 to JWT library.


Diffs (updated)
-

  3rdparty/libprocess/include/process/jwt.hpp 
768cbf6fa91537ff9f45f236f4033097c5cea959 
  3rdparty/libprocess/include/process/ssl/utilities.hpp 
b7cc31c33fd35c93754407f8b350eeb993177f1d 
  3rdparty/libprocess/src/jwt.cpp 921031e6fe3ced5a6be6bc96190fae6d8282ae26 
  3rdparty/libprocess/src/ssl/utilities.cpp 
4d3727daf53ec62a19255da5a9804d342e770ec2 
  3rdparty/libprocess/src/tests/jwt_keys.hpp PRE-CREATION 
  3rdparty/libprocess/src/tests/jwt_tests.cpp 
eb36a9aed3b11208c7cdc6f20b5347f46821a207 


Diff: https://reviews.apache.org/r/66621/diff/3/

Changes: https://reviews.apache.org/r/66621/diff/2-3/


Testing
---

make check

I added the same tests than the ones for HS256 (i.e., validation in following 
cases: bad header, bad payload, unknown alg, unsupported alg, valid token etc.. 
and creation of a valid token). I also added a test to verify that the 
validation of a RS256 token fails when using the wrong public key.


Thanks,

Clément Michaud

Re: Review Request 66621: Add support for alg RS256 to JWT library.

2018-04-19 Thread Mesos Reviewbot 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66621/#review201536
---



Patch looks great!

Reviews applied: [66621]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose --disable-libtool-wrappers' ENVIRONMENT='GLOG_v=1 
MESOS_VERBOSE=1'; ./support/docker-build.sh

- Mesos Reviewbot


On April 16, 2018, 9 p.m., Clément Michaud wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66621/
> ---
> 
> (Updated April 16, 2018, 9 p.m.)
> 
> 
> Review request for mesos, Alexander Rojas and Till Toenshoff.
> 
> 
> Bugs: MESOS-8788
> https://issues.apache.org/jira/browse/MESOS-8788
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add support for alg RS256 to JWT library.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/jwt.hpp 
> 768cbf6fa91537ff9f45f236f4033097c5cea959 
>   3rdparty/libprocess/include/process/ssl/utilities.hpp 
> b7cc31c33fd35c93754407f8b350eeb993177f1d 
>   3rdparty/libprocess/src/jwt.cpp 921031e6fe3ced5a6be6bc96190fae6d8282ae26 
>   3rdparty/libprocess/src/ssl/utilities.cpp 
> 4d3727daf53ec62a19255da5a9804d342e770ec2 
>   3rdparty/libprocess/src/tests/jwt_keys.hpp PRE-CREATION 
>   3rdparty/libprocess/src/tests/jwt_tests.cpp 
> eb36a9aed3b11208c7cdc6f20b5347f46821a207 
> 
> 
> Diff: https://reviews.apache.org/r/66621/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> I added the same tests than the ones for HS256 (i.e., validation in following 
> cases: bad header, bad payload, unknown alg, unsupported alg, valid token 
> etc.. and creation of a valid token). I also added a test to verify that the 
> validation of a RS256 token fails when using the wrong public key.
> 
> 
> Thanks,
> 
> Clément Michaud
> 
>

Re: Review Request 66621: Add support for alg RS256 to JWT library.

2018-04-19 Thread Alexander Rojas 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66621/#review201514
---




3rdparty/libprocess/include/process/jwt.hpp
Lines 19 (patched)


I really would prefer, if possible, to forward declare `RSA_shared_ptr` 
here and leave in include in the cpp.



3rdparty/libprocess/include/process/ssl/utilities.hpp
Lines 140 (patched)


I'm not so sure we gain too much by declaring this typedef. Moreover, we 
use typedefs rather sparecely in the code base, so I would get away with it.



3rdparty/libprocess/include/process/ssl/utilities.hpp
Lines 189-190 (patched)


This comment needs an update.



3rdparty/libprocess/src/jwt.cpp
Lines 303 (patched)


We don't use `auto` unless the type is obvious from the lhs (like a call to 
`new Type` or `make_shared`) but in this case it is not obvious.



3rdparty/libprocess/src/jwt.cpp
Lines 364-365 (patched)


This could be better formatted:

```c++
  return JWT(
  header,
  payload,
  base64::encode_url_safe(signarure.ger(), false));
```



3rdparty/libprocess/src/ssl/utilities.cpp
Lines 374 (patched)


**Here and below**

This files apparently uses snake case everywhere instead of snake case. 
Could you rectify that here in your function names and the variable names 
withing these functions?



3rdparty/libprocess/src/ssl/utilities.cpp
Lines 382 (patched)


missed an space between `if` and `(`.



3rdparty/libprocess/src/ssl/utilities.cpp
Lines 401-406 (patched)


Note that we tend to avoid using `std::string` or `std::vector` in cpp 
files. Instead, at the top of the file add:

```c++
using std::string;
using std::vector;
```



3rdparty/libprocess/src/ssl/utilities.cpp
Lines 405 (patched)


missed `#include `



3rdparty/libprocess/src/ssl/utilities.cpp
Lines 415 (patched)


let's avoid one letter variable names.



3rdparty/libprocess/src/ssl/utilities.cpp
Lines 423 (patched)


missed an space between `if` and `(`.



3rdparty/libprocess/src/ssl/utilities.cpp
Lines 427 (patched)


I'd add an empty line here.


- Alexander Rojas


On April 16, 2018, 11 p.m., Clément Michaud wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66621/
> ---
> 
> (Updated April 16, 2018, 11 p.m.)
> 
> 
> Review request for mesos, Alexander Rojas and Till Toenshoff.
> 
> 
> Bugs: MESOS-8788
> https://issues.apache.org/jira/browse/MESOS-8788
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add support for alg RS256 to JWT library.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/jwt.hpp 
> 768cbf6fa91537ff9f45f236f4033097c5cea959 
>   3rdparty/libprocess/include/process/ssl/utilities.hpp 
> b7cc31c33fd35c93754407f8b350eeb993177f1d 
>   3rdparty/libprocess/src/jwt.cpp 921031e6fe3ced5a6be6bc96190fae6d8282ae26 
>   3rdparty/libprocess/src/ssl/utilities.cpp 
> 4d3727daf53ec62a19255da5a9804d342e770ec2 
>   3rdparty/libprocess/src/tests/jwt_keys.hpp PRE-CREATION 
>   3rdparty/libprocess/src/tests/jwt_tests.cpp 
> eb36a9aed3b11208c7cdc6f20b5347f46821a207 
> 
> 
> Diff: https://reviews.apache.org/r/66621/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> I added the same tests than the ones for HS256 (i.e., validation in following 
> cases: bad header, bad payload, unknown alg, unsupported alg, valid token 
> etc.. and creation of a valid token). I also added a test to verify that the 
> validation of a RS256 token fails when using the wrong public key.
> 
> 
> Thanks,
> 
> Clément Michaud
> 
>

Re: Review Request 66621: Add support for alg RS256 to JWT library.

2018-04-16 Thread Clément Michaud 


> On avr. 16, 2018, 9:52 après-midi, Mesos Reviewbot Windows wrote:
> > FAIL: Failed to apply the current review.
> > 
> > Failed command: `python.exe .\support\apply-reviews.py -n -r 66621`
> > 
> > All the build artifacts available at: 
> > http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66621
> > 
> > Relevant logs:
> > 
> > - 
> > [apply-review-66621-stderr.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66621/logs/apply-review-66621-stderr.log):
> > 
> > ```
> > Traceback (most recent call last):
> >   File ".\support\apply-reviews.py", line 447, in 
> > main()
> >   File ".\support\apply-reviews.py", line 442, in main
> > reviewboard(options)
> >   File ".\support\apply-reviews.py", line 432, in reviewboard
> > apply_review(options)
> >   File ".\support\apply-reviews.py", line 163, in apply_review
> > commit_patch(options)
> >   File ".\support\apply-reviews.py", line 292, in commit_patch
> > shell(cmd, options['dry_run'])
> >   File ".\support\apply-reviews.py", line 147, in shell
> > error_code = subprocess.call(command, stderr=subprocess.STDOUT, 
> > shell=True)
> >   File "C:\Program Files\Python27\lib\subprocess.py", line 168, in call
> > return Popen(*popenargs, **kwargs).wait()
> >   File "C:\Program Files\Python27\lib\subprocess.py", line 390, in __init__
> > errread, errwrite)
> >   File "C:\Program Files\Python27\lib\subprocess.py", line 610, in 
> > _execute_child
> > args = '{} /c "{}"'.format (comspec, args)
> > UnicodeEncodeError: 'ascii' codec can't encode character u'\xe9' in 
> > position 23: ordinal not in range(128)
> > ```
> 
> Clément Michaud wrote:
> Can anyone help me find what is wrong with the bot?The script is running 
> well on my laptop.

Can it be the accent in my first name?


- Clément


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66621/#review201264
---


On avr. 16, 2018, 9 après-midi, Clément Michaud wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66621/
> ---
> 
> (Updated avr. 16, 2018, 9 après-midi)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: https://issues.apache.org/jira/browse/MESOS-8788
> 
> https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/MESOS-8788
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add support for alg RS256 to JWT library.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/jwt.hpp 
> 768cbf6fa91537ff9f45f236f4033097c5cea959 
>   3rdparty/libprocess/include/process/ssl/utilities.hpp 
> b7cc31c33fd35c93754407f8b350eeb993177f1d 
>   3rdparty/libprocess/src/jwt.cpp 921031e6fe3ced5a6be6bc96190fae6d8282ae26 
>   3rdparty/libprocess/src/ssl/utilities.cpp 
> 4d3727daf53ec62a19255da5a9804d342e770ec2 
>   3rdparty/libprocess/src/tests/jwt_keys.hpp PRE-CREATION 
>   3rdparty/libprocess/src/tests/jwt_tests.cpp 
> eb36a9aed3b11208c7cdc6f20b5347f46821a207 
> 
> 
> Diff: https://reviews.apache.org/r/66621/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> I've added the same tests than the ones for HS256 (i.e., validation in 
> following cases: bad header, bad payload, unknown alg, unsupported alg, valid 
> token etc.. and creation of a valid token).
> 
> 
> Thanks,
> 
> Clément Michaud
> 
>

Re: Review Request 66621: Add support for alg RS256 to JWT library.

2018-04-16 Thread Clément Michaud 


> On avr. 16, 2018, 9:52 après-midi, Mesos Reviewbot Windows wrote:
> > FAIL: Failed to apply the current review.
> > 
> > Failed command: `python.exe .\support\apply-reviews.py -n -r 66621`
> > 
> > All the build artifacts available at: 
> > http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66621
> > 
> > Relevant logs:
> > 
> > - 
> > [apply-review-66621-stderr.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66621/logs/apply-review-66621-stderr.log):
> > 
> > ```
> > Traceback (most recent call last):
> >   File ".\support\apply-reviews.py", line 447, in 
> > main()
> >   File ".\support\apply-reviews.py", line 442, in main
> > reviewboard(options)
> >   File ".\support\apply-reviews.py", line 432, in reviewboard
> > apply_review(options)
> >   File ".\support\apply-reviews.py", line 163, in apply_review
> > commit_patch(options)
> >   File ".\support\apply-reviews.py", line 292, in commit_patch
> > shell(cmd, options['dry_run'])
> >   File ".\support\apply-reviews.py", line 147, in shell
> > error_code = subprocess.call(command, stderr=subprocess.STDOUT, 
> > shell=True)
> >   File "C:\Program Files\Python27\lib\subprocess.py", line 168, in call
> > return Popen(*popenargs, **kwargs).wait()
> >   File "C:\Program Files\Python27\lib\subprocess.py", line 390, in __init__
> > errread, errwrite)
> >   File "C:\Program Files\Python27\lib\subprocess.py", line 610, in 
> > _execute_child
> > args = '{} /c "{}"'.format (comspec, args)
> > UnicodeEncodeError: 'ascii' codec can't encode character u'\xe9' in 
> > position 23: ordinal not in range(128)
> > ```

Can anyone help me find what is wrong with the bot?The script is running well 
on my laptop.


- Clément


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66621/#review201264
---


On avr. 16, 2018, 9 après-midi, Clément Michaud wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66621/
> ---
> 
> (Updated avr. 16, 2018, 9 après-midi)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: https://issues.apache.org/jira/browse/MESOS-8788
> 
> https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/MESOS-8788
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add support for alg RS256 to JWT library.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/jwt.hpp 
> 768cbf6fa91537ff9f45f236f4033097c5cea959 
>   3rdparty/libprocess/include/process/ssl/utilities.hpp 
> b7cc31c33fd35c93754407f8b350eeb993177f1d 
>   3rdparty/libprocess/src/jwt.cpp 921031e6fe3ced5a6be6bc96190fae6d8282ae26 
>   3rdparty/libprocess/src/ssl/utilities.cpp 
> 4d3727daf53ec62a19255da5a9804d342e770ec2 
>   3rdparty/libprocess/src/tests/jwt_keys.hpp PRE-CREATION 
>   3rdparty/libprocess/src/tests/jwt_tests.cpp 
> eb36a9aed3b11208c7cdc6f20b5347f46821a207 
> 
> 
> Diff: https://reviews.apache.org/r/66621/diff/2/
> 
> 
> Testing
> ---
> 
> I've added the same tests than the ones for HS256 (i.e., validation in 
> following cases: bad header, bad payload, unknown alg, unsupported alg, valid 
> token etc.. and creation of a valid token).
> 
> 
> Thanks,
> 
> Clément Michaud
> 
>

Re: Review Request 66621: Add support for alg RS256 to JWT library.

2018-04-16 Thread Mesos Reviewbot Windows 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66621/#review201264
---



FAIL: Failed to apply the current review.

Failed command: `python.exe .\support\apply-reviews.py -n -r 66621`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66621

Relevant logs:

- 
[apply-review-66621-stderr.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66621/logs/apply-review-66621-stderr.log):

```
Traceback (most recent call last):
  File ".\support\apply-reviews.py", line 447, in 
main()
  File ".\support\apply-reviews.py", line 442, in main
reviewboard(options)
  File ".\support\apply-reviews.py", line 432, in reviewboard
apply_review(options)
  File ".\support\apply-reviews.py", line 163, in apply_review
commit_patch(options)
  File ".\support\apply-reviews.py", line 292, in commit_patch
shell(cmd, options['dry_run'])
  File ".\support\apply-reviews.py", line 147, in shell
error_code = subprocess.call(command, stderr=subprocess.STDOUT, shell=True)
  File "C:\Program Files\Python27\lib\subprocess.py", line 168, in call
return Popen(*popenargs, **kwargs).wait()
  File "C:\Program Files\Python27\lib\subprocess.py", line 390, in __init__
errread, errwrite)
  File "C:\Program Files\Python27\lib\subprocess.py", line 610, in 
_execute_child
args = '{} /c "{}"'.format (comspec, args)
UnicodeEncodeError: 'ascii' codec can't encode character u'\xe9' in position 
23: ordinal not in range(128)
```

- Mesos Reviewbot Windows


On April 16, 2018, 9 p.m., Clément Michaud wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66621/
> ---
> 
> (Updated April 16, 2018, 9 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: https://issues.apache.org/jira/browse/MESOS-8788
> 
> https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/MESOS-8788
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add support for alg RS256 to JWT library.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/jwt.hpp 
> 768cbf6fa91537ff9f45f236f4033097c5cea959 
>   3rdparty/libprocess/include/process/ssl/utilities.hpp 
> b7cc31c33fd35c93754407f8b350eeb993177f1d 
>   3rdparty/libprocess/src/jwt.cpp 921031e6fe3ced5a6be6bc96190fae6d8282ae26 
>   3rdparty/libprocess/src/ssl/utilities.cpp 
> 4d3727daf53ec62a19255da5a9804d342e770ec2 
>   3rdparty/libprocess/src/tests/jwt_keys.hpp PRE-CREATION 
>   3rdparty/libprocess/src/tests/jwt_tests.cpp 
> eb36a9aed3b11208c7cdc6f20b5347f46821a207 
> 
> 
> Diff: https://reviews.apache.org/r/66621/diff/2/
> 
> 
> Testing
> ---
> 
> I've added the same tests than the ones for HS256 (i.e., validation in 
> following cases: bad header, bad payload, unknown alg, unsupported alg, valid 
> token etc.. and creation of a valid token).
> 
> 
> Thanks,
> 
> Clément Michaud
> 
>

Re: Review Request 66621: Add support for alg RS256 to JWT library.

2018-04-16 Thread Clément Michaud 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66621/
---

(Updated avr. 16, 2018, 9 après-midi)


Review request for mesos.


Changes
---

Fix style issues and add one test validating RS256 token with wrong public key


Summary (updated)
-

Add support for alg RS256 to JWT library.


Bugs: https://issues.apache.org/jira/browse/MESOS-8788

https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/MESOS-8788


Repository: mesos


Description (updated)
---

Add support for alg RS256 to JWT library.


Diffs (updated)
-

  3rdparty/libprocess/include/process/jwt.hpp 
768cbf6fa91537ff9f45f236f4033097c5cea959 
  3rdparty/libprocess/include/process/ssl/utilities.hpp 
b7cc31c33fd35c93754407f8b350eeb993177f1d 
  3rdparty/libprocess/src/jwt.cpp 921031e6fe3ced5a6be6bc96190fae6d8282ae26 
  3rdparty/libprocess/src/ssl/utilities.cpp 
4d3727daf53ec62a19255da5a9804d342e770ec2 
  3rdparty/libprocess/src/tests/jwt_keys.hpp PRE-CREATION 
  3rdparty/libprocess/src/tests/jwt_tests.cpp 
eb36a9aed3b11208c7cdc6f20b5347f46821a207 


Diff: https://reviews.apache.org/r/66621/diff/2/

Changes: https://reviews.apache.org/r/66621/diff/1-2/


Testing
---

I've added the same tests than the ones for HS256 (i.e., validation in 
following cases: bad header, bad payload, unknown alg, unsupported alg, valid 
token etc.. and creation of a valid token).


Thanks,

Clément Michaud