You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Anand Mazumdar <ma...@gmail.com> on 2016/04/13 00:26:15 UTC

Review Request 46116: Added basic authentication scheme to the scheduler library.

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

Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
-------

This change adds basic scheme AuthN support to the library.
It would be good to add support for additional schemes in the
future.


Diffs
-----

  include/mesos/v1/scheduler.hpp 6603075fe187ac7d1b56381ffc6e18f14adbfb5a 
  src/scheduler/scheduler.cpp f9d54f99f3086a8a4349abd3892413fa6bd44437 

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


Testing
-------

make check


Thanks,

Anand Mazumdar


Re: Review Request 46116: Added basic authentication scheme to the scheduler library.

Posted by Anand Mazumdar <ma...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46116/
-----------------------------------------------------------

(Updated April 15, 2016, 12:11 a.m.)


Review request for mesos and Vinod Kone.


Changes
-------

Review comment from Vinod


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


Repository: mesos


Description
-------

This change adds basic scheme AuthN support to the library.
It would be good to add support for additional schemes in the
future.


Diffs (updated)
-----

  include/mesos/v1/scheduler.hpp 6603075fe187ac7d1b56381ffc6e18f14adbfb5a 
  src/scheduler/scheduler.cpp f9d54f99f3086a8a4349abd3892413fa6bd44437 

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


Testing
-------

make check


Thanks,

Anand Mazumdar


Re: Review Request 46116: Added basic authentication scheme to the scheduler library.

Posted by Vinod Kone <vi...@gmail.com>.

> On April 13, 2016, 10:05 p.m., Vinod Kone wrote:
> >

Also, I'm wondering if it would be better for the library to take Authorization string as a parameter instead of Credential. That way it is more generic because the library can just set "Authorization: <authorization>". The idea is that scheduler will use a library to do any type of auth it needs (e.g. kerberos, JWT) and finally gives the scheduler library the AuthZ header. We could maybe make it even more generic by taking a list of headers.


- Vinod


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


On April 12, 2016, 10:26 p.m., Anand Mazumdar wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46116/
> -----------------------------------------------------------
> 
> (Updated April 12, 2016, 10:26 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-4459
>     https://issues.apache.org/jira/browse/MESOS-4459
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This change adds basic scheme AuthN support to the library.
> It would be good to add support for additional schemes in the
> future.
> 
> 
> Diffs
> -----
> 
>   include/mesos/v1/scheduler.hpp 6603075fe187ac7d1b56381ffc6e18f14adbfb5a 
>   src/scheduler/scheduler.cpp f9d54f99f3086a8a4349abd3892413fa6bd44437 
> 
> Diff: https://reviews.apache.org/r/46116/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>


Re: Review Request 46116: Added basic authentication scheme to the scheduler library.

Posted by Anand Mazumdar <ma...@gmail.com>.

> On April 13, 2016, 10:05 p.m., Vinod Kone wrote:
> >
> 
> Vinod Kone wrote:
>     Also, I'm wondering if it would be better for the library to take Authorization string as a parameter instead of Credential. That way it is more generic because the library can just set "Authorization: <authorization>". The idea is that scheduler will use a library to do any type of auth it needs (e.g. kerberos, JWT) and finally gives the scheduler library the AuthZ header. We could maybe make it even more generic by taking a list of headers.

IIUC, the client still needs to send an initial request to the server with the login credentials. If the login request succeeds, the server would return a token back to the client. All subsequent requests from the client can then include the token as part of the `Authorization` header. So, we would still need to pass the credentials to the library. We can later change the logic in the library to look for the token as part of the response from server and then send it for subsequent requests.


- Anand


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


On April 12, 2016, 10:26 p.m., Anand Mazumdar wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46116/
> -----------------------------------------------------------
> 
> (Updated April 12, 2016, 10:26 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-4459
>     https://issues.apache.org/jira/browse/MESOS-4459
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This change adds basic scheme AuthN support to the library.
> It would be good to add support for additional schemes in the
> future.
> 
> 
> Diffs
> -----
> 
>   include/mesos/v1/scheduler.hpp 6603075fe187ac7d1b56381ffc6e18f14adbfb5a 
>   src/scheduler/scheduler.cpp f9d54f99f3086a8a4349abd3892413fa6bd44437 
> 
> Diff: https://reviews.apache.org/r/46116/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>


Re: Review Request 46116: Added basic authentication scheme to the scheduler library.

Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46116/#review128764
-----------------------------------------------------------


Fix it, then Ship it!





include/mesos/v1/scheduler.hpp (line 62)
<https://reviews.apache.org/r/46116/#comment192227>

    Can you add a comment here mentioning what happens if Credential is some?


- Vinod Kone


On April 12, 2016, 10:26 p.m., Anand Mazumdar wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46116/
> -----------------------------------------------------------
> 
> (Updated April 12, 2016, 10:26 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-4459
>     https://issues.apache.org/jira/browse/MESOS-4459
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This change adds basic scheme AuthN support to the library.
> It would be good to add support for additional schemes in the
> future.
> 
> 
> Diffs
> -----
> 
>   include/mesos/v1/scheduler.hpp 6603075fe187ac7d1b56381ffc6e18f14adbfb5a 
>   src/scheduler/scheduler.cpp f9d54f99f3086a8a4349abd3892413fa6bd44437 
> 
> Diff: https://reviews.apache.org/r/46116/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>