Re: Review Request 37999: Implemented http::AuthenticatorManager

[Alexander Rojas]

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

(Updated Nov. 18, 2015, 10:15 a.m.)


Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
Toenshoff.


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


Repository: mesos


Description
---

Introduces the authenticator manager, which is a class which handles the actual 
authentication procedure during the execution of `ProcessManager::handle()` and 
it also takes care of the life cycle of instances of http::Authenticator.

No tests are added at this point since no public API is generated, the goal of 
this patch is to implement the manager and verify nothing breaks afterwards. 
Authenticator manager tests proper come in a latter patch.


Diffs (updated)
-

  3rdparty/libprocess/Makefile.am cdefa37528ea69422978a9772f955042e882dde4 
  3rdparty/libprocess/include/Makefile.am 
e6be2c4db121585bbe7f3c0627de048adc7cfb2c 
  3rdparty/libprocess/include/process/authenticator.hpp PRE-CREATION 
  3rdparty/libprocess/include/process/event.hpp 
28ce1928877084f0e1a73fdad789224c86e53f46 
  3rdparty/libprocess/include/process/http.hpp 
90c9be122ee0c402b806d70fc818e3c03b15101a 
  3rdparty/libprocess/src/CMakeLists.txt 
fb9bd04832779ac43151f2feb3dfbf58cb996434 
  3rdparty/libprocess/src/authentication_router.hpp PRE-CREATION 
  3rdparty/libprocess/src/authentication_router.cpp PRE-CREATION 
  3rdparty/libprocess/src/process.cpp 7abdf21a5784920251c3627f9820c12fdc356c6e 
  3rdparty/libprocess/src/process_reference.hpp 
e6110bba2a54948be68e58ab9de988565b7d95a8 

Diff: https://reviews.apache.org/r/37999/diff/


Testing
---

make check


Thanks,

Alexander Rojas

Re: Review Request 37999: Implemented http::AuthenticatorManager

[Alexander Rojas]

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

(Updated Nov. 17, 2015, 2:35 p.m.)


Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
Toenshoff.


Changes
---

Third round of reviews.


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


Repository: mesos


Description
---

Introduces the authenticator manager, which is a class which handles the actual 
authentication procedure during the execution of `ProcessManager::handle()` and 
it also takes care of the life cycle of instances of http::Authenticator.

No tests are added at this point since no public API is generated, the goal of 
this patch is to implement the manager and verify nothing breaks afterwards. 
Authenticator manager tests proper come in a latter patch.


Diffs (updated)
-

  3rdparty/libprocess/Makefile.am cdefa37528ea69422978a9772f955042e882dde4 
  3rdparty/libprocess/include/Makefile.am 
e6be2c4db121585bbe7f3c0627de048adc7cfb2c 
  3rdparty/libprocess/include/process/authenticator.hpp PRE-CREATION 
  3rdparty/libprocess/include/process/event.hpp 
28ce1928877084f0e1a73fdad789224c86e53f46 
  3rdparty/libprocess/include/process/http.hpp 
90c9be122ee0c402b806d70fc818e3c03b15101a 
  3rdparty/libprocess/src/CMakeLists.txt 
fb9bd04832779ac43151f2feb3dfbf58cb996434 
  3rdparty/libprocess/src/process.cpp 7abdf21a5784920251c3627f9820c12fdc356c6e 
  3rdparty/libprocess/src/realm_manager.hpp PRE-CREATION 
  3rdparty/libprocess/src/realm_manager.cpp PRE-CREATION 

Diff: https://reviews.apache.org/r/37999/diff/


Testing
---

make check


Thanks,

Alexander Rojas

Re: Review Request 37999: Implemented http::AuthenticatorManager

[Alexander Rojas]

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

(Updated Nov. 16, 2015, 5:23 p.m.)


Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
Toenshoff.


Changes
---

Second round of reviews.


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


Repository: mesos


Description
---

Introduces the authenticator manager, which is a class which handles the actual 
authentication procedure during the execution of `ProcessManager::handle()` and 
it also takes care of the life cycle of instances of http::Authenticator.

No tests are added at this point since no public API is generated, the goal of 
this patch is to implement the manager and verify nothing breaks afterwards. 
Authenticator manager tests proper come in a latter patch.


Diffs (updated)
-

  3rdparty/libprocess/Makefile.am cdefa37528ea69422978a9772f955042e882dde4 
  3rdparty/libprocess/include/Makefile.am 
e6be2c4db121585bbe7f3c0627de048adc7cfb2c 
  3rdparty/libprocess/include/process/authenticator.hpp PRE-CREATION 
  3rdparty/libprocess/include/process/event.hpp 
28ce1928877084f0e1a73fdad789224c86e53f46 
  3rdparty/libprocess/include/process/http.hpp 
90c9be122ee0c402b806d70fc818e3c03b15101a 
  3rdparty/libprocess/src/CMakeLists.txt 
fb9bd04832779ac43151f2feb3dfbf58cb996434 
  3rdparty/libprocess/src/process.cpp 7abdf21a5784920251c3627f9820c12fdc356c6e 
  3rdparty/libprocess/src/realm_manager.hpp PRE-CREATION 
  3rdparty/libprocess/src/realm_manager.cpp PRE-CREATION 

Diff: https://reviews.apache.org/r/37999/diff/


Testing
---

make check


Thanks,

Alexander Rojas

Re: Review Request 37999: Implemented http::AuthenticatorManager

[Alexander Rojas]

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

(Updated Nov. 16, 2015, 10:51 a.m.)


Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
Toenshoff.


Changes
---

Review changes.


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


Repository: mesos


Description
---

Introduces the authenticator manager, which is a class which handles the actual 
authentication procedure during the execution of `ProcessManager::handle()` and 
it also takes care of the life cycle of instances of http::Authenticator.

No tests are added at this point since no public API is generated, the goal of 
this patch is to implement the manager and verify nothing breaks afterwards. 
Authenticator manager tests proper come in a latter patch.


Diffs (updated)
-

  3rdparty/libprocess/Makefile.am cdefa37528ea69422978a9772f955042e882dde4 
  3rdparty/libprocess/include/Makefile.am 
e6be2c4db121585bbe7f3c0627de048adc7cfb2c 
  3rdparty/libprocess/include/process/authenticator.hpp PRE-CREATION 
  3rdparty/libprocess/include/process/event.hpp 
28ce1928877084f0e1a73fdad789224c86e53f46 
  3rdparty/libprocess/include/process/http.hpp 
90c9be122ee0c402b806d70fc818e3c03b15101a 
  3rdparty/libprocess/src/CMakeLists.txt 
fb9bd04832779ac43151f2feb3dfbf58cb996434 
  3rdparty/libprocess/src/authentication_manager.hpp PRE-CREATION 
  3rdparty/libprocess/src/authentication_manager.cpp PRE-CREATION 
  3rdparty/libprocess/src/process.cpp b45b5b14cc7868d73a6fe02c02b9baf8b44b891f 

Diff: https://reviews.apache.org/r/37999/diff/


Testing
---

make check


Thanks,

Alexander Rojas

Re: Review Request 37999: Implemented http::AuthenticatorManager

[Alexander Rojas]

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

(Updated Nov. 10, 2015, 4:59 a.m.)


Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
Toenshoff.


Changes
---

Rebase


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


Repository: mesos


Description
---

Introduces the authenticator manager, which is a class which handles the actual 
authentication procedure during the execution of `ProcessManager::handle()` and 
it also takes care of the life cycle of instances of http::Authenticator.

No tests are added at this point since no public API is generated, the goal of 
this patch is to implement the manager and verify nothing breaks afterwards. 
Authenticator manager tests proper come in a latter patch.


Diffs (updated)
-

  3rdparty/libprocess/include/Makefile.am 
e6be2c4db121585bbe7f3c0627de048adc7cfb2c 
  3rdparty/libprocess/include/process/authenticator.hpp PRE-CREATION 
  3rdparty/libprocess/include/process/event.hpp 
16ddbd77afa6efdf6bad201aa497ee102aa863ae 
  3rdparty/libprocess/include/process/http.hpp 
90c9be122ee0c402b806d70fc818e3c03b15101a 
  3rdparty/libprocess/src/process.cpp a94712b9ac3b60fb047b3a5a4d84a56fa4d02313 

Diff: https://reviews.apache.org/r/37999/diff/


Testing
---

make check


Thanks,

Alexander Rojas

Re: Review Request 37999: Implemented http::AuthenticatorManager

[Alexander Rojas]

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

(Updated Nov. 9, 2015, 11:29 p.m.)


Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
Toenshoff.


Changes
---

Changes after Bernd's review.


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


Repository: mesos


Description
---

Introduces the authenticator manager, which is a class which handles the actual 
authentication procedure during the execution of `ProcessManager::handle()` and 
it also takes care of the life cycle of instances of http::Authenticator.

No tests are added at this point since no public API is generated, the goal of 
this patch is to implement the manager and verify nothing breaks afterwards. 
Authenticator manager tests proper come in a latter patch.


Diffs (updated)
-

  3rdparty/libprocess/include/Makefile.am 
e6be2c4db121585bbe7f3c0627de048adc7cfb2c 
  3rdparty/libprocess/include/process/authenticator.hpp PRE-CREATION 
  3rdparty/libprocess/include/process/event.hpp 
16ddbd77afa6efdf6bad201aa497ee102aa863ae 
  3rdparty/libprocess/include/process/http.hpp 
90c9be122ee0c402b806d70fc818e3c03b15101a 
  3rdparty/libprocess/src/process.cpp a94712b9ac3b60fb047b3a5a4d84a56fa4d02313 

Diff: https://reviews.apache.org/r/37999/diff/


Testing
---

make check


Thanks,

Alexander Rojas

Re: Review Request 37999: Implemented http::AuthenticatorManager

[Bernd Mathiske]

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



3rdparty/libprocess/src/process.cpp (line 533)


Since we are in a cpp file, we can use "using std::unique_ptr" above and 
skip the qualifier "std::" here.



3rdparty/libprocess/src/process.cpp (line 696)


This seems to be a trick. Are we testing whether we are at "/" here? Ok, so 
you copied this code from somewhere else in the same file, but this is too 
obscure IMHO when written without any comment.

Please use something more obvious or comment on what this is doing.



3rdparty/libprocess/src/process.cpp (line 750)


Shouldn't we push the realm as challenge here already?

If we do this below only if all challenges are empty, we are missing the 
case where challenges are present some are and some are not.



3rdparty/libprocess/src/process.cpp (line 755)


s/on/one



3rdparty/libprocess/src/process.cpp (line 756)


s/was already/has already been

relevance to the present => use present perfect, not imperfect



3rdparty/libprocess/src/process.cpp (line 757)


s/executed/reached


- Bernd Mathiske


On Nov. 5, 2015, 9:07 a.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37999/
> ---
> 
> (Updated Nov. 5, 2015, 9:07 a.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-3231
> https://issues.apache.org/jira/browse/MESOS-3231
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Introduces the authenticator manager, which is a class which handles the 
> actual authentication procedure during the execution of 
> `ProcessManager::handle()` and it also takes care of the life cycle of 
> instances of http::Authenticator.
> 
> No tests are added at this point since no public API is generated, the goal 
> of this patch is to implement the manager and verify nothing breaks 
> afterwards. Authenticator manager tests proper come in a latter patch.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/Makefile.am 
> fcc62e99b92b9d2e7ab344e561a06dd6de1fef7e 
>   3rdparty/libprocess/include/process/authenticator.hpp PRE-CREATION 
>   3rdparty/libprocess/include/process/event.hpp 
> 16ddbd77afa6efdf6bad201aa497ee102aa863ae 
>   3rdparty/libprocess/include/process/http.hpp 
> 90c9be122ee0c402b806d70fc818e3c03b15101a 
>   3rdparty/libprocess/src/process.cpp 
> a94712b9ac3b60fb047b3a5a4d84a56fa4d02313 
> 
> Diff: https://reviews.apache.org/r/37999/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 37999: Implemented http::AuthenticatorManager

[Bernd Mathiske]

> On Nov. 6, 2015, 7:22 a.m., Bernd Mathiske wrote:
> > 3rdparty/libprocess/src/process.cpp, line 750
> > 
> >
> > Shouldn't we push the realm as challenge here already?
> > 
> > If we do this below only if all challenges are empty, we are missing 
> > the case where challenges are present some are and some are not.
> 
> Alexander Rojas wrote:
> Well, as it is mentioned in the comment below either:
> 1. Authentication succeed, no need to add challenges, forward to the 
> handler.
> 2. It is a multi-step authentication, the case is also handled in this 
> block. If there is at least one multi-step, we only send the headers 
> corresponding to them (and they come in the challenges part) in which case, 
> challenges won't be null in the block below.
> 3. Authentication failed, we send all the challenges.
> 
> So no, we don't push challenges here because we are still looking for at 
> least one multi-step mechanism.

Just to clarify: so we are not expecting the user to switch to another still 
available scheme when there is a multi-step one? Why not advertise more choice 
to the user then?


- Bernd


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


On Nov. 5, 2015, 9:07 a.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37999/
> ---
> 
> (Updated Nov. 5, 2015, 9:07 a.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-3231
> https://issues.apache.org/jira/browse/MESOS-3231
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Introduces the authenticator manager, which is a class which handles the 
> actual authentication procedure during the execution of 
> `ProcessManager::handle()` and it also takes care of the life cycle of 
> instances of http::Authenticator.
> 
> No tests are added at this point since no public API is generated, the goal 
> of this patch is to implement the manager and verify nothing breaks 
> afterwards. Authenticator manager tests proper come in a latter patch.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/Makefile.am 
> fcc62e99b92b9d2e7ab344e561a06dd6de1fef7e 
>   3rdparty/libprocess/include/process/authenticator.hpp PRE-CREATION 
>   3rdparty/libprocess/include/process/event.hpp 
> 16ddbd77afa6efdf6bad201aa497ee102aa863ae 
>   3rdparty/libprocess/include/process/http.hpp 
> 90c9be122ee0c402b806d70fc818e3c03b15101a 
>   3rdparty/libprocess/src/process.cpp 
> a94712b9ac3b60fb047b3a5a4d84a56fa4d02313 
> 
> Diff: https://reviews.apache.org/r/37999/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 37999: Implemented http::AuthenticatorManager

[Alexander Rojas]

> On Nov. 6, 2015, 4:22 p.m., Bernd Mathiske wrote:
> > 3rdparty/libprocess/src/process.cpp, line 750
> > 
> >
> > Shouldn't we push the realm as challenge here already?
> > 
> > If we do this below only if all challenges are empty, we are missing 
> > the case where challenges are present some are and some are not.
> 
> Alexander Rojas wrote:
> Well, as it is mentioned in the comment below either:
> 1. Authentication succeed, no need to add challenges, forward to the 
> handler.
> 2. It is a multi-step authentication, the case is also handled in this 
> block. If there is at least one multi-step, we only send the headers 
> corresponding to them (and they come in the challenges part) in which case, 
> challenges won't be null in the block below.
> 3. Authentication failed, we send all the challenges.
> 
> So no, we don't push challenges here because we are still looking for at 
> least one multi-step mechanism.
> 
> Bernd Mathiske wrote:
> Just to clarify: so we are not expecting the user to switch to another 
> still available scheme when there is a multi-step one? Why not advertise more 
> choice to the user then?

First time he connects to the endpoint he gets all the options, but if he's in 
the middle of a multi-step it means that the user already started 
authentication and he already chose one. He will either fail (and get all the 
options) or succeed, in which case we don't need a new set.

I guess a user can change the authentication mechanism when he already started 
one… but I wouldn't trust such user anyway.


- Alexander


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


On Nov. 5, 2015, 6:07 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37999/
> ---
> 
> (Updated Nov. 5, 2015, 6:07 p.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-3231
> https://issues.apache.org/jira/browse/MESOS-3231
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Introduces the authenticator manager, which is a class which handles the 
> actual authentication procedure during the execution of 
> `ProcessManager::handle()` and it also takes care of the life cycle of 
> instances of http::Authenticator.
> 
> No tests are added at this point since no public API is generated, the goal 
> of this patch is to implement the manager and verify nothing breaks 
> afterwards. Authenticator manager tests proper come in a latter patch.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/Makefile.am 
> fcc62e99b92b9d2e7ab344e561a06dd6de1fef7e 
>   3rdparty/libprocess/include/process/authenticator.hpp PRE-CREATION 
>   3rdparty/libprocess/include/process/event.hpp 
> 16ddbd77afa6efdf6bad201aa497ee102aa863ae 
>   3rdparty/libprocess/include/process/http.hpp 
> 90c9be122ee0c402b806d70fc818e3c03b15101a 
>   3rdparty/libprocess/src/process.cpp 
> a94712b9ac3b60fb047b3a5a4d84a56fa4d02313 
> 
> Diff: https://reviews.apache.org/r/37999/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 37999: Implemented http::AuthenticatorManager

[Alexander Rojas]

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

(Updated Nov. 6, 2015, 6:07 p.m.)


Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
Toenshoff.


Changes
---

Addresses issues in Bernd's review.


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


Repository: mesos


Description
---

Introduces the authenticator manager, which is a class which handles the actual 
authentication procedure during the execution of `ProcessManager::handle()` and 
it also takes care of the life cycle of instances of http::Authenticator.

No tests are added at this point since no public API is generated, the goal of 
this patch is to implement the manager and verify nothing breaks afterwards. 
Authenticator manager tests proper come in a latter patch.


Diffs (updated)
-

  3rdparty/libprocess/include/Makefile.am 
fcc62e99b92b9d2e7ab344e561a06dd6de1fef7e 
  3rdparty/libprocess/include/process/authenticator.hpp PRE-CREATION 
  3rdparty/libprocess/include/process/event.hpp 
16ddbd77afa6efdf6bad201aa497ee102aa863ae 
  3rdparty/libprocess/include/process/http.hpp 
90c9be122ee0c402b806d70fc818e3c03b15101a 
  3rdparty/libprocess/src/process.cpp a94712b9ac3b60fb047b3a5a4d84a56fa4d02313 

Diff: https://reviews.apache.org/r/37999/diff/


Testing
---

make check


Thanks,

Alexander Rojas

Re: Review Request 37999: Implemented http::AuthenticatorManager

[Bernd Mathiske]

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

Ship it!



3rdparty/libprocess/src/process.cpp (line 750)


s/advertise/advertised


- Bernd Mathiske


On Nov. 6, 2015, 9:07 a.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37999/
> ---
> 
> (Updated Nov. 6, 2015, 9:07 a.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-3231
> https://issues.apache.org/jira/browse/MESOS-3231
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Introduces the authenticator manager, which is a class which handles the 
> actual authentication procedure during the execution of 
> `ProcessManager::handle()` and it also takes care of the life cycle of 
> instances of http::Authenticator.
> 
> No tests are added at this point since no public API is generated, the goal 
> of this patch is to implement the manager and verify nothing breaks 
> afterwards. Authenticator manager tests proper come in a latter patch.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/Makefile.am 
> fcc62e99b92b9d2e7ab344e561a06dd6de1fef7e 
>   3rdparty/libprocess/include/process/authenticator.hpp PRE-CREATION 
>   3rdparty/libprocess/include/process/event.hpp 
> 16ddbd77afa6efdf6bad201aa497ee102aa863ae 
>   3rdparty/libprocess/include/process/http.hpp 
> 90c9be122ee0c402b806d70fc818e3c03b15101a 
>   3rdparty/libprocess/src/process.cpp 
> a94712b9ac3b60fb047b3a5a4d84a56fa4d02313 
> 
> Diff: https://reviews.apache.org/r/37999/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 37999: Implemented http::AuthenticatorManager

[Alexander Rojas]

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

(Updated Nov. 5, 2015, 6:07 p.m.)


Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
Toenshoff.


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


Repository: mesos


Description
---

Introduces the authenticator manager, which is a class which handles the actual 
authentication procedure during the execution of `ProcessManager::handle()` and 
it also takes care of the life cycle of instances of http::Authenticator.

No tests are added at this point since no public API is generated, the goal of 
this patch is to implement the manager and verify nothing breaks afterwards. 
Authenticator manager tests proper come in a latter patch.


Diffs (updated)
-

  3rdparty/libprocess/include/Makefile.am 
fcc62e99b92b9d2e7ab344e561a06dd6de1fef7e 
  3rdparty/libprocess/include/process/authenticator.hpp PRE-CREATION 
  3rdparty/libprocess/include/process/event.hpp 
16ddbd77afa6efdf6bad201aa497ee102aa863ae 
  3rdparty/libprocess/include/process/http.hpp 
90c9be122ee0c402b806d70fc818e3c03b15101a 
  3rdparty/libprocess/src/process.cpp a94712b9ac3b60fb047b3a5a4d84a56fa4d02313 

Diff: https://reviews.apache.org/r/37999/diff/


Testing
---

make check


Thanks,

Alexander Rojas

Re: Review Request 37999: Implemented http::AuthenticatorManager

[Alexander Rojas]

> On Oct. 21, 2015, 1:46 p.m., Bernd Mathiske wrote:
> > 3rdparty/libprocess/src/process.cpp, line 885
> > 
> >
> > Why are we doing this? Why isn't it a failure if there is no success 
> > and also no challenge? Please comment in the source code on this.
> > 
> > My take is that if these authenticators would have liked to pose a 
> > challenge, they would already have done so.

Change the comment to:

```c++
// At this point on of the following is true:
// 1. Authentication succeeded, in which case the principal was
//already returned and this code is not executed.
// 2. It is a multi-step authentication protocol and challenges is
//not empty, because they are filled with steps.
// 3. Authentication failed for any reason, e.g. credentials were not
//provided in which case the authentication is reset to the first
//step.
```

Which makes clear why is this done here.


> On Oct. 21, 2015, 1:46 p.m., Bernd Mathiske wrote:
> > 3rdparty/libprocess/src/process.cpp, line 886
> > 
> >
> > s/with those/those with

Comment changed completely.


- Alexander


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


On Nov. 4, 2015, 12:25 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37999/
> ---
> 
> (Updated Nov. 4, 2015, 12:25 p.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-3231
> https://issues.apache.org/jira/browse/MESOS-3231
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Introduces the authenticator manager, which is a class which handles the 
> actual authentication procedure during the execution of 
> `ProcessManager::handle()` and it also takes care of the life cycle of 
> instances of http::Authenticator.
> 
> No tests are added at this point since no public API is generated, the goal 
> of this patch is to implement the manager and verify nothing breaks 
> afterwards. Authenticator manager tests proper come in a latter patch.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/Makefile.am 
> fcc62e99b92b9d2e7ab344e561a06dd6de1fef7e 
>   3rdparty/libprocess/include/process/authenticator.hpp PRE-CREATION 
>   3rdparty/libprocess/include/process/event.hpp 
> 16ddbd77afa6efdf6bad201aa497ee102aa863ae 
>   3rdparty/libprocess/include/process/http.hpp 
> 90c9be122ee0c402b806d70fc818e3c03b15101a 
>   3rdparty/libprocess/src/process.cpp 
> a94712b9ac3b60fb047b3a5a4d84a56fa4d02313 
> 
> Diff: https://reviews.apache.org/r/37999/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 37999: Implemented http::AuthenticatorManager

[Alexander Rojas]

> On Oct. 21, 2015, 1:03 p.m., Bernd Mathiske wrote:
> > 3rdparty/libprocess/include/process/authenticator.hpp, line 52
> > 
> >
> > Duplicate text.

I used a separate line because Doxygen uses the first line as a brief summary 
(It is also in our [style 
guide](https://github.com/apache/mesos/blob/c3940cd4da29eb6539096c6ec6dcab1a70993c42/docs/doxygen-style-guide.md#source-code-documentation-syntax)).
 However, given that I haven't seen the rendered result. I will remove this 
line.


> On Oct. 21, 2015, 1:03 p.m., Bernd Mathiske wrote:
> > 3rdparty/libprocess/include/process/authenticator.hpp, line 107
> > 
> >
> > s/are/is (contents is singular)
> > 
> > s/a HTTP/an HTTP 
> > 
> > (Please check all other places. If "a" is followed by "H" in an 
> > abbreviation, which is therefore pronounced "aytsh", this constitutes an 
> > "a" followed by a vowel sound, so it becomes "an)

Sorry, _contents_ is 
[plural](http://english.stackexchange.com/questions/13556/content-or-contents). 
Though there are different meanings of the word _content_.


> On Oct. 21, 2015, 1:03 p.m., Bernd Mathiske wrote:
> > 3rdparty/libprocess/src/process.cpp, line 793
> > 
> >
> > Suggestion: break this up into two calls.
> > 
> > removeAllAuthenticators()
> > removeAuthenticator()

I will let you decide this one with till, because he suggested the exact 
opposite (which has also been suggested by other reviewers), in his Sept. 10, 
2015, 8:06 a.m. review (see above).


- Alexander


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


On Nov. 4, 2015, 12:25 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37999/
> ---
> 
> (Updated Nov. 4, 2015, 12:25 p.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-3231
> https://issues.apache.org/jira/browse/MESOS-3231
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Introduces the authenticator manager, which is a class which handles the 
> actual authentication procedure during the execution of 
> `ProcessManager::handle()` and it also takes care of the life cycle of 
> instances of http::Authenticator.
> 
> No tests are added at this point since no public API is generated, the goal 
> of this patch is to implement the manager and verify nothing breaks 
> afterwards. Authenticator manager tests proper come in a latter patch.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/Makefile.am 
> fcc62e99b92b9d2e7ab344e561a06dd6de1fef7e 
>   3rdparty/libprocess/include/process/authenticator.hpp PRE-CREATION 
>   3rdparty/libprocess/include/process/event.hpp 
> 16ddbd77afa6efdf6bad201aa497ee102aa863ae 
>   3rdparty/libprocess/include/process/http.hpp 
> 90c9be122ee0c402b806d70fc818e3c03b15101a 
>   3rdparty/libprocess/src/process.cpp 
> a94712b9ac3b60fb047b3a5a4d84a56fa4d02313 
> 
> Diff: https://reviews.apache.org/r/37999/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 37999: Implemented http::AuthenticatorManager

[Alexander Rojas]

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

(Updated Nov. 4, 2015, 12:25 p.m.)


Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
Toenshoff.


Changes
---

Addresses Bernd's review issues.


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


Repository: mesos


Description
---

Introduces the authenticator manager, which is a class which handles the actual 
authentication procedure during the execution of `ProcessManager::handle()` and 
it also takes care of the life cycle of instances of http::Authenticator.

No tests are added at this point since no public API is generated, the goal of 
this patch is to implement the manager and verify nothing breaks afterwards. 
Authenticator manager tests proper come in a latter patch.


Diffs (updated)
-

  3rdparty/libprocess/include/Makefile.am 
fcc62e99b92b9d2e7ab344e561a06dd6de1fef7e 
  3rdparty/libprocess/include/process/authenticator.hpp PRE-CREATION 
  3rdparty/libprocess/include/process/event.hpp 
16ddbd77afa6efdf6bad201aa497ee102aa863ae 
  3rdparty/libprocess/include/process/http.hpp 
90c9be122ee0c402b806d70fc818e3c03b15101a 
  3rdparty/libprocess/src/process.cpp a94712b9ac3b60fb047b3a5a4d84a56fa4d02313 

Diff: https://reviews.apache.org/r/37999/diff/


Testing
---

make check


Thanks,

Alexander Rojas

Re: Review Request 37999: Implemented http::AuthenticatorManager

[Alexander Rojas]

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

(Updated Oct. 22, 2015, 2:57 p.m.)


Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
Toenshoff.


Changes
---

Rebase to account for changes in previous review.


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


Repository: mesos


Description
---

Introduces the authenticator manager, which is a class which handles the actual 
authentication procedure during the execution of `ProcessManager::handle()` and 
it also takes care of the life cycle of instances of http::Authenticator.

No tests are added at this point since no public API is generated, the goal of 
this patch is to implement the manager and verify nothing breaks afterwards. 
Authenticator manager tests proper come in a latter patch.


Diffs (updated)
-

  3rdparty/libprocess/include/Makefile.am 
fcc62e99b92b9d2e7ab344e561a06dd6de1fef7e 
  3rdparty/libprocess/include/process/authenticator.hpp PRE-CREATION 
  3rdparty/libprocess/include/process/event.hpp 
16ddbd77afa6efdf6bad201aa497ee102aa863ae 
  3rdparty/libprocess/include/process/http.hpp 
591c1a959057155e1bf0f5bd73352e78d1c15cb3 
  3rdparty/libprocess/src/process.cpp 954d31424bc8f8ecfa78b80513c480f2514ec271 

Diff: https://reviews.apache.org/r/37999/diff/


Testing
---

make check


Thanks,

Alexander Rojas

Re: Review Request 37999: Implemented http::AuthenticatorManager

[Alexander Rojas]

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

(Updated Oct. 21, 2015, 12:01 p.m.)


Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
Toenshoff.


Changes
---

Addresses issues from Till's review.


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


Repository: mesos


Description
---

Introduces the authenticator manager, which is a class which handles the actual 
authentication procedure during the execution of `ProcessManager::handle()` and 
it also takes care of the life cycle of instances of http::Authenticator.

No tests are added at this point since no public API is generated, the goal of 
this patch is to implement the manager and verify nothing breaks afterwards. 
Authenticator manager tests proper come in a latter patch.


Diffs (updated)
-

  3rdparty/libprocess/include/Makefile.am 
fcc62e99b92b9d2e7ab344e561a06dd6de1fef7e 
  3rdparty/libprocess/include/process/authenticator.hpp PRE-CREATION 
  3rdparty/libprocess/include/process/event.hpp 
16ddbd77afa6efdf6bad201aa497ee102aa863ae 
  3rdparty/libprocess/include/process/http.hpp 
591c1a959057155e1bf0f5bd73352e78d1c15cb3 
  3rdparty/libprocess/src/process.cpp 954d31424bc8f8ecfa78b80513c480f2514ec271 

Diff: https://reviews.apache.org/r/37999/diff/


Testing
---

make check


Thanks,

Alexander Rojas

Re: Review Request 37999: Implemented http::AuthenticatorManager

[Alexander Rojas]

> On Oct. 21, 2015, 5:25 a.m., Till Toenshoff wrote:
> > 3rdparty/libprocess/src/process.cpp, lines 841-842
> > 
> >
> > If an `authenticate`-request failed, the error message is never shown 
> > anywhere.
> > 
> > I think we should add something that helps us debugging the 
> > authentication via logs.
> > 
> > Something like this would possibly do:
> > ```
> >   } else {
> >   LOG(WARNING) << "Authentication failed: " << 
> > result.error();
> >   }
> > ```

Using `VLOG` since we are in libprocess here.


- Alexander


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


On Oct. 20, 2015, noon, Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37999/
> ---
> 
> (Updated Oct. 20, 2015, noon)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-3231
> https://issues.apache.org/jira/browse/MESOS-3231
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Introduces the authenticator manager, which is a class which handles the 
> actual authentication procedure during the execution of 
> `ProcessManager::handle()` and it also takes care of the life cycle of 
> instances of http::Authenticator.
> 
> No tests are added at this point since no public API is generated, the goal 
> of this patch is to implement the manager and verify nothing breaks 
> afterwards. Authenticator manager tests proper come in a latter patch.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/Makefile.am 
> fcc62e99b92b9d2e7ab344e561a06dd6de1fef7e 
>   3rdparty/libprocess/include/process/authenticator.hpp PRE-CREATION 
>   3rdparty/libprocess/include/process/event.hpp 
> 16ddbd77afa6efdf6bad201aa497ee102aa863ae 
>   3rdparty/libprocess/include/process/http.hpp 
> 591c1a959057155e1bf0f5bd73352e78d1c15cb3 
>   3rdparty/libprocess/src/process.cpp 
> 954d31424bc8f8ecfa78b80513c480f2514ec271 
> 
> Diff: https://reviews.apache.org/r/37999/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 37999: Implemented http::AuthenticatorManager

[Bernd Mathiske]

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



3rdparty/libprocess/include/process/authenticator.hpp (line 52)


Duplicate text.



3rdparty/libprocess/include/process/authenticator.hpp (line 53)


Suggested rephrasing: "These parameters come in the form of a map. Their interpretation depends on the specific authenticator instance 
that receives them. For example, a basic authenticator could expect mappings 
from user names to encrypted passwords."



3rdparty/libprocess/include/process/authenticator.hpp (line 69)


s/It/Its

better, simpler, replace these two sentences with:
"Possible outcomes:"



3rdparty/libprocess/include/process/authenticator.hpp (line 74)


s/possible/



3rdparty/libprocess/include/process/authenticator.hpp (line 77)


s/finish/finished



3rdparty/libprocess/include/process/authenticator.hpp (line 79)


s/instance of an



3rdparty/libprocess/include/process/authenticator.hpp (line 80)


s/The contents must be ready to be used/This will be used



3rdparty/libprocess/include/process/authenticator.hpp (line 86)


used? in what way? by whom?

Suggestion: "A standard HTTP authenticator will typically only read the 
'WWW-Authenticate' header of the request."



3rdparty/libprocess/include/process/authenticator.hpp (line 98)


s/use/be used
s/a HTTP/an HTTP



3rdparty/libprocess/include/process/authenticator.hpp (line 107)


s/are/is (contents is singular)

s/a HTTP/an HTTP 

(Please check all other places. If "a" is followed by "H" in an 
abbreviation, which is therefore pronounced "aytsh", this constitutes an "a" 
followed by a vowel sound, so it becomes "an)



3rdparty/libprocess/include/process/http.hpp (line 494)


s/for/with/



3rdparty/libprocess/src/process.cpp (line 662)


It may not be 100% clear to everybody what the parameters mean and what he 
method is supposed to accomplish. Probably better to explain here with a quick 
comment. (Instead of letting the implementation below become the specification 
of what might be intended.)

(No need to repeat such comments in the AuthenticatorProcess declaration.)



3rdparty/libprocess/src/process.cpp (line 665)


It is not immediately clear what this parameter does. Please write a 
comment for this method.



3rdparty/libprocess/src/process.cpp (line 686)


Please move this to line 667.



3rdparty/libprocess/src/process.cpp (line 691)


Please also document what the Future return type does.



3rdparty/libprocess/src/process.cpp (line 696)


s/provides a has/provide a hash



3rdparty/libprocess/src/process.cpp (line 698)


Insert a blank line, please!



3rdparty/libprocess/src/process.cpp (line 793)


Suggestion: break this up into two calls.

removeAllAuthenticators()
removeAuthenticator()


- Bernd Mathiske


On Oct. 21, 2015, 3:01 a.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37999/
> ---
> 
> (Updated Oct. 21, 2015, 3:01 a.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-3231
> https://issues.apache.org/jira/browse/MESOS-3231
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Introduces the authenticator manager, which is a class which handles the 
> actual authentication procedure during the execution of 
> `ProcessManager::handle()` and it also takes care of the life cycle of 
> instances of http::Authenticator.
> 
> No tests are added at this point since no public API is generated, the goal 
> of this patch is to implement the manager and verify nothing breaks 
> afterwards. Authenticator manager tests proper 

Re: Review Request 37999: Implemented http::AuthenticatorManager

[Bernd Mathiske]

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



3rdparty/libprocess/src/process.cpp (line 833)


s/Check the realm/Obtain the realms



3rdparty/libprocess/src/process.cpp (line 843)


s/ream's/realms'



3rdparty/libprocess/src/process.cpp (line 844)


Please add an s to this var name. list => plural. This will read better in 
the subsequent code. Give it a try!



3rdparty/libprocess/src/process.cpp (line 848)


Aligning the arguments of the foreach makes it easier to associated them 
with it instead of mistaking the second one for something new:

foreach (const std::unique_ptr& authenticator,
 authenticators_[realm]) {



3rdparty/libprocess/src/process.cpp (line 869)


Insert blank line.



3rdparty/libprocess/src/process.cpp (line 885)


Why are we doing this? Why isn't it a failure if there is no success and 
also no challenge? Please comment in the source code on this.

My take is that if these authenticators would have liked to pose a 
challenge, they would already have done so.



3rdparty/libprocess/src/process.cpp (line 886)


s/with those/those with



3rdparty/libprocess/src/process.cpp (line 2839)


Rather than a conditional expression and a complicated comment, let's break 
this up into clear if-then-else blocks with dedicated local comments.


- Bernd Mathiske


On Oct. 21, 2015, 3:01 a.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37999/
> ---
> 
> (Updated Oct. 21, 2015, 3:01 a.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-3231
> https://issues.apache.org/jira/browse/MESOS-3231
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Introduces the authenticator manager, which is a class which handles the 
> actual authentication procedure during the execution of 
> `ProcessManager::handle()` and it also takes care of the life cycle of 
> instances of http::Authenticator.
> 
> No tests are added at this point since no public API is generated, the goal 
> of this patch is to implement the manager and verify nothing breaks 
> afterwards. Authenticator manager tests proper come in a latter patch.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/Makefile.am 
> fcc62e99b92b9d2e7ab344e561a06dd6de1fef7e 
>   3rdparty/libprocess/include/process/authenticator.hpp PRE-CREATION 
>   3rdparty/libprocess/include/process/event.hpp 
> 16ddbd77afa6efdf6bad201aa497ee102aa863ae 
>   3rdparty/libprocess/include/process/http.hpp 
> 591c1a959057155e1bf0f5bd73352e78d1c15cb3 
>   3rdparty/libprocess/src/process.cpp 
> 954d31424bc8f8ecfa78b80513c480f2514ec271 
> 
> Diff: https://reviews.apache.org/r/37999/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 37999: Implemented http::AuthenticatorManager

[Till Toenshoff]

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



3rdparty/libprocess/src/process.cpp (lines 841 - 842)


If an `authenticate`-request failed, the error message is never shown 
anywhere.

I think we should add something that helps us debugging the authentication 
via logs.

Something like this would possibly do:
```
  } else {
  LOG(WARNING) << "Authentication failed: " << result.error();
  }
```


- Till Toenshoff


On Oct. 20, 2015, 10 a.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37999/
> ---
> 
> (Updated Oct. 20, 2015, 10 a.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-3231
> https://issues.apache.org/jira/browse/MESOS-3231
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Introduces the authenticator manager, which is a class which handles the 
> actual authentication procedure during the execution of 
> `ProcessManager::handle()` and it also takes care of the life cycle of 
> instances of http::Authenticator.
> 
> No tests are added at this point since no public API is generated, the goal 
> of this patch is to implement the manager and verify nothing breaks 
> afterwards. Authenticator manager tests proper come in a latter patch.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/Makefile.am 
> fcc62e99b92b9d2e7ab344e561a06dd6de1fef7e 
>   3rdparty/libprocess/include/process/authenticator.hpp PRE-CREATION 
>   3rdparty/libprocess/include/process/event.hpp 
> 16ddbd77afa6efdf6bad201aa497ee102aa863ae 
>   3rdparty/libprocess/include/process/http.hpp 
> 591c1a959057155e1bf0f5bd73352e78d1c15cb3 
>   3rdparty/libprocess/src/process.cpp 
> 954d31424bc8f8ecfa78b80513c480f2514ec271 
> 
> Diff: https://reviews.apache.org/r/37999/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 37999: Implemented http::AuthenticatorManager

[Alexander Rojas]

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

(Updated Oct. 9, 2015, 12:53 p.m.)


Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
Toenshoff.


Changes
---

Fixes big bug of dlangling reference. Addresses till's review issues.


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


Repository: mesos


Description
---

Introduces the authenticator manager, which is a class which handles the actual 
authentication procedure during the execution of `ProcessManager::handle()` and 
it also takes care of the life cycle of instances of http::Authenticator.

No tests are added at this point since no public API is generated, the goal of 
this patch is to implement the manager and verify nothing breaks afterwards. 
Authenticator manager tests proper come in a latter patch.


Diffs (updated)
-

  3rdparty/libprocess/include/Makefile.am 
fcc62e99b92b9d2e7ab344e561a06dd6de1fef7e 
  3rdparty/libprocess/include/process/authenticator.hpp PRE-CREATION 
  3rdparty/libprocess/include/process/event.hpp 
16ddbd77afa6efdf6bad201aa497ee102aa863ae 
  3rdparty/libprocess/include/process/http.hpp 
591c1a959057155e1bf0f5bd73352e78d1c15cb3 
  3rdparty/libprocess/src/process.cpp d1c81f1d244f02bf42cab97198587ce1b8c7c407 

Diff: https://reviews.apache.org/r/37999/diff/


Testing
---

make check


Thanks,

Alexander Rojas

Re: Review Request 37999: Implemented http::AuthenticatorManager

[Till Toenshoff]

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


Partial, entirely incomplete but maybe helpful review :)


3rdparty/libprocess/include/process/http.hpp (line 493)


THe RFCs always use `WWW-Authenticate`, shouldn't we as well here and 
everywhere else?



3rdparty/libprocess/src/process.cpp (line 2611)


s/Tecnically/technically/
s/principa/principal/



3rdparty/libprocess/src/process.cpp (lines 2643 - 2647)


This still makes me think we should fix the flaky fetcher tests before 
anything else. Lets put some combined efforts into that please.


- Till Toenshoff


On Sept. 30, 2015, 8:20 a.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37999/
> ---
> 
> (Updated Sept. 30, 2015, 8:20 a.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-3231
> https://issues.apache.org/jira/browse/MESOS-3231
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Introduces the authenticator manager, which is a class which handles the 
> actual authentication procedure during the execution of 
> `ProcessManager::handle()` and it also takes care of the life cycle of 
> instances of http::Authenticator.
> 
> No tests are added at this point since no public API is generated, the goal 
> of this patch is to implement the manager and verify nothing breaks 
> afterwards. Authenticator manager tests proper come in a latter patch.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/Makefile.am 
> fcc62e99b92b9d2e7ab344e561a06dd6de1fef7e 
>   3rdparty/libprocess/include/process/authenticator.hpp PRE-CREATION 
>   3rdparty/libprocess/include/process/event.hpp 
> 16ddbd77afa6efdf6bad201aa497ee102aa863ae 
>   3rdparty/libprocess/include/process/http.hpp 
> ba3f0bc7df33795e332c374fbad04106b9d56416 
>   3rdparty/libprocess/src/process.cpp 
> d1c81f1d244f02bf42cab97198587ce1b8c7c407 
> 
> Diff: https://reviews.apache.org/r/37999/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 37999: Implemented http::AuthenticatorManager

[Alexander Rojas]

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

(Updated Sept. 30, 2015, 10:20 a.m.)


Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
Toenshoff.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Introduces the authenticator manager, which is a class which handles the actual 
authentication procedure during the execution of `ProcessManager::handle()` and 
it also takes care of the life cycle of instances of http::Authenticator.

No tests are added at this point since no public API is generated, the goal of 
this patch is to implement the manager and verify nothing breaks afterwards. 
Authenticator manager tests proper come in a latter patch.


Diffs (updated)
-

  3rdparty/libprocess/include/Makefile.am 
fcc62e99b92b9d2e7ab344e561a06dd6de1fef7e 
  3rdparty/libprocess/include/process/authenticator.hpp PRE-CREATION 
  3rdparty/libprocess/include/process/event.hpp 
16ddbd77afa6efdf6bad201aa497ee102aa863ae 
  3rdparty/libprocess/include/process/http.hpp 
ba3f0bc7df33795e332c374fbad04106b9d56416 
  3rdparty/libprocess/src/process.cpp d1c81f1d244f02bf42cab97198587ce1b8c7c407 

Diff: https://reviews.apache.org/r/37999/diff/


Testing
---

make check


Thanks,

Alexander Rojas

Re: Review Request 37999: Implemented http::AuthenticatorManager

[Alexander Rojas]

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

(Updated Sept. 28, 2015, 11:42 a.m.)


Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
Toenshoff.


Changes
---

Updated includes.


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


Repository: mesos


Description
---

Introduces the authenticator manager, which is a class which handles the actual 
authentication procedure during the execution of `ProcessManager::handle()` and 
it also takes care of the life cycle of instances of http::Authenticator.

No tests are added at this point since no public API is generated, the goal of 
this patch is to implement the manager and verify nothing breaks afterwards. 
Authenticator manager tests proper come in a latter patch.


Diffs (updated)
-

  3rdparty/libprocess/include/Makefile.am 
fcc62e99b92b9d2e7ab344e561a06dd6de1fef7e 
  3rdparty/libprocess/include/process/authenticator.hpp PRE-CREATION 
  3rdparty/libprocess/include/process/event.hpp 
16ddbd77afa6efdf6bad201aa497ee102aa863ae 
  3rdparty/libprocess/include/process/http.hpp 
fbd6cf7967173495875a8ea90ed28ade88b982a2 
  3rdparty/libprocess/src/process.cpp c03fba47435505484704e6c8a61e56123859781f 

Diff: https://reviews.apache.org/r/37999/diff/


Testing
---

make check


Thanks,

Alexander Rojas

Re: Review Request 37999: Implemented http::AuthenticatorManager

[Alex Clemmer]

> On Sept. 23, 2015, 5:39 p.m., Alex Clemmer wrote:
> > Can we please add this file to the CMakeLists.txt file in 
> > 3rdparty/libprocess/src as well?
> 
> Alexander Rojas wrote:
> The included file is a header and so far cmake file doesn't seem to care 
> about the include directory (can you correct me if I'm wrong). Still, 
> shouln't the headers be there too? otherwise cmake wouldn't do proper 
> dependency analysis.
> 
> Alex Clemmer wrote:
> Oh, yes, my bad, those changes aren't committed yet. (I have too many 
> reviews up and lose track of them all, sorry.)

To follow up, actually... we won't have to commit the header file lists after 
all! After investigation, it turns out that all CMake needs is to have the 
directory linked with `include_directory` and it just works. (I apologize for 
the lack of clarity here... I had never used CMake before I started this, and 
the truth is that when you write hundreds of lines of code in an unfamiliar 
language, you sometimes forget what is true and what is false. :) )


- Alex


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


On Sept. 23, 2015, 8:57 a.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37999/
> ---
> 
> (Updated Sept. 23, 2015, 8:57 a.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-3231
> https://issues.apache.org/jira/browse/MESOS-3231
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Introduces the authenticator manager, which is a class which handles the 
> actual authentication procedure during the execution of 
> `ProcessManager::handle()` and it also takes care of the life cycle of 
> instances of http::Authenticator.
> 
> No tests are added at this point since no public API is generated, the goal 
> of this patch is to implement the manager and verify nothing breaks 
> afterwards. Authenticator manager tests proper come in a latter patch.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/Makefile.am 
> fcc62e99b92b9d2e7ab344e561a06dd6de1fef7e 
>   3rdparty/libprocess/include/process/authenticator.hpp PRE-CREATION 
>   3rdparty/libprocess/include/process/event.hpp 
> 16ddbd77afa6efdf6bad201aa497ee102aa863ae 
>   3rdparty/libprocess/include/process/http.hpp 
> fbd6cf7967173495875a8ea90ed28ade88b982a2 
>   3rdparty/libprocess/src/process.cpp 
> 4afa30569b4d235637b49a624602e6b199c32e0e 
> 
> Diff: https://reviews.apache.org/r/37999/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 37999: Implemented http::AuthenticatorManager

[Alexander Rojas]

> On Sept. 23, 2015, 7:39 p.m., Alex Clemmer wrote:
> > Can we please add this file to the CMakeLists.txt file in 
> > 3rdparty/libprocess/src as well?

The included file is a header and so far cmake file doesn't seem to care about 
the include directory (can you correct me if I'm wrong). Still, shouln't the 
headers be there too? otherwise cmake wouldn't do proper dependency analysis.


- Alexander


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


On Sept. 23, 2015, 10:57 a.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37999/
> ---
> 
> (Updated Sept. 23, 2015, 10:57 a.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-3231
> https://issues.apache.org/jira/browse/MESOS-3231
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Introduces the authenticator manager, which is a class which handles the 
> actual authentication procedure during the execution of 
> `ProcessManager::handle()` and it also takes care of the life cycle of 
> instances of http::Authenticator.
> 
> No tests are added at this point since no public API is generated, the goal 
> of this patch is to implement the manager and verify nothing breaks 
> afterwards. Authenticator manager tests proper come in a latter patch.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/Makefile.am 
> fcc62e99b92b9d2e7ab344e561a06dd6de1fef7e 
>   3rdparty/libprocess/include/process/authenticator.hpp PRE-CREATION 
>   3rdparty/libprocess/include/process/event.hpp 
> 16ddbd77afa6efdf6bad201aa497ee102aa863ae 
>   3rdparty/libprocess/include/process/http.hpp 
> fbd6cf7967173495875a8ea90ed28ade88b982a2 
>   3rdparty/libprocess/src/process.cpp 
> 4afa30569b4d235637b49a624602e6b199c32e0e 
> 
> Diff: https://reviews.apache.org/r/37999/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 37999: Implemented http::AuthenticatorManager

[Alex Clemmer]

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


Can we please add this file to the CMakeLists.txt file in 
3rdparty/libprocess/src as well?

- Alex Clemmer


On Sept. 23, 2015, 8:57 a.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37999/
> ---
> 
> (Updated Sept. 23, 2015, 8:57 a.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-3231
> https://issues.apache.org/jira/browse/MESOS-3231
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Introduces the authenticator manager, which is a class which handles the 
> actual authentication procedure during the execution of 
> `ProcessManager::handle()` and it also takes care of the life cycle of 
> instances of http::Authenticator.
> 
> No tests are added at this point since no public API is generated, the goal 
> of this patch is to implement the manager and verify nothing breaks 
> afterwards. Authenticator manager tests proper come in a latter patch.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/Makefile.am 
> fcc62e99b92b9d2e7ab344e561a06dd6de1fef7e 
>   3rdparty/libprocess/include/process/authenticator.hpp PRE-CREATION 
>   3rdparty/libprocess/include/process/event.hpp 
> 16ddbd77afa6efdf6bad201aa497ee102aa863ae 
>   3rdparty/libprocess/include/process/http.hpp 
> fbd6cf7967173495875a8ea90ed28ade88b982a2 
>   3rdparty/libprocess/src/process.cpp 
> 4afa30569b4d235637b49a624602e6b199c32e0e 
> 
> Diff: https://reviews.apache.org/r/37999/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 37999: Implemented http::AuthenticatorManager

[Alexander Rojas]

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

(Updated Sept. 23, 2015, 10:57 a.m.)


Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
Toenshoff.


Changes
---

No longer WIP, opening to the community.


Summary (updated)
-

Implemented http::AuthenticatorManager


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


Repository: mesos


Description (updated)
---

Introduces the authenticator manager, which is a class which handles the actual 
authentication procedure during the execution of `ProcessManager::handle()` and 
it also takes care of the life cycle of instances of http::Authenticator.

No tests are added at this point since no public API is generated, the goal of 
this patch is to implement the manager and verify nothing breaks afterwards. 
Authenticator manager tests proper come in a latter patch.


Diffs
-

  3rdparty/libprocess/include/Makefile.am 
fcc62e99b92b9d2e7ab344e561a06dd6de1fef7e 
  3rdparty/libprocess/include/process/authenticator.hpp PRE-CREATION 
  3rdparty/libprocess/include/process/event.hpp 
16ddbd77afa6efdf6bad201aa497ee102aa863ae 
  3rdparty/libprocess/include/process/http.hpp 
fbd6cf7967173495875a8ea90ed28ade88b982a2 
  3rdparty/libprocess/src/process.cpp 4afa30569b4d235637b49a624602e6b199c32e0e 

Diff: https://reviews.apache.org/r/37999/diff/


Testing
---

make check


Thanks,

Alexander Rojas