mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Alexander Rojas" <alexan...@mesosphere.io>
Subject Re: Review Request 37999: Implemented http::AuthenticatorManager
Date Fri, 06 Nov 2015 17:05:38 GMT


> On Nov. 6, 2015, 4:22 p.m., Bernd Mathiske wrote:
> > 3rdparty/libprocess/src/process.cpp, line 750
> > <https://reviews.apache.org/r/37999/diff/12/?file=1116827#file1116827line750>
> >
> >     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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message