Mailing-List: contact reviews-help@mesos.apache.org; run by ezmlm
Precedence: bulk
Reply-To: reviews@mesos.apache.org
Content-Type: multipart/alternative;
 boundary="===============4407614221657887578=="
MIME-Version: 1.0
Subject: Re: Review Request 37999: Implemented http::AuthenticatorManager
From: "Alexander Rojas" <alexander@mesosphere.io>
To: "Benjamin Hindman" <benh@berkeley.edu>, "Adam B" <adam@mesosphere.io>,
 "Bernd Mathiske" <bernd@mesosphere.io>, "Till Toenshoff" <toenshoff@me.com>
Cc: "mesos" <reviews@mesos.apache.org>,
 "Alexander Rojas" <alexander@mesosphere.io>
Date: Wed, 04 Nov 2015 11:25:56 -0000
Message-ID: <20151104112556.28714.94940@reviews.apache.org>
Auto-Submitted: auto-generated
Sender: "Alexander Rojas" <noreply@reviews.apache.org>
References: <20151021114602.1667.75435@reviews.apache.org>
In-Reply-To: <20151021114602.1667.75435@reviews.apache.org>
Reply-To: "Alexander Rojas" <alexander@mesosphere.io>

--===============4407614221657887578==
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit


> On Oct. 21, 2015, 1:46 p.m., Bernd Mathiske wrote:
> > 3rdparty/libprocess/src/process.cpp, line 885
> > <https://reviews.apache.org/r/37999/diff/9/?file=1102348#file1102348line885>
> >
> >     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
> > <https://reviews.apache.org/r/37999/diff/9/?file=1102348#file1102348line886>
> >
> >     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
> 
>


--===============4407614221657887578==--