aurora-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Giulio Eulisse <giulio.euli...@cern.ch>
Subject Re: Review Request 51893: Allow cookie based authentication
Date Tue, 11 Oct 2016 13:57:49 GMT


> On Oct. 7, 2016, 3:47 p.m., Joshua Cohen wrote:
> > src/main/python/apache/aurora/common/cookie_auth_module.py, line 31
> > <https://reviews.apache.org/r/51893/diff/12-13/?file=1525264#file1525264line31>
> >
> >     You should be able to drop the expanduser call now that you're using `DEFAULT_SEARCH_PATHS`,
we can rely it already having expanded the paths.

Ok.


> On Oct. 7, 2016, 3:47 p.m., Joshua Cohen wrote:
> > src/test/python/apache/aurora/common/test_cookie_auth_module.py, lines 4-6
> > <https://reviews.apache.org/r/51893/diff/13/?file=1526286#file1526286line4>
> >
> >     This is not much of a test ;).
> >     
> >     Can you verify, e.g., that invoking `CookieAuth` with a request properly adds
the cookies from the cookie jar?
> >     
> >     This will probably involve a little bit of mocking. You'll need to create a
tempdir and write a cookie jar there for it to load. The easiest way to accomplish this probably
something like:
> >     
> >         import mock
> >         import os
> >         
> >         from twitter.common.contextutil import temporary_file
> >         
> >         
> >         def test_cookie_auth():
> >           with temporary_file() as fp:
> >           fp.write(...) # write cookiejar format to tempfile
> >           
> >           c = CookieAuth((os.path.dirname(fp.name),))
> >           
> >           mock_request = mock.Mock(spec=requests.PreparedRequest)
> >           c(mock_request)
> >           
> >           # Now assert that mock_request.prepare_cookies was invoked with the expected
cookies.
> >     
> >     
> >     Note: this will also require changing the `CookieAuth` constructor to allow
for injection of `search_paths`. E.g.
> >     
> >         class CookieAuth(AuthBase):
> >         
> >           def __init__(self, search_paths = DEFAULT_SEARCH_PATHS):
> >             ...
> >     
> >     Same would apply to the test for `CookieAuthModule` where we should essentially
assert the return values from its various methods (these tests are less valuable, so if you're
not feeling that ambitious I'd be ok to punt on them).

Ok. I've added the requested tests.


- Giulio


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


On Oct. 7, 2016, 10:49 a.m., Giulio Eulisse wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51893/
> -----------------------------------------------------------
> 
> (Updated Oct. 7, 2016, 10:49 a.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Stephan Erb.
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Allow cookie based authentication
> 
> This allows aurora client to connect to servers which are behind a frontend which expects
some sort of cookie to autheticate and authorize users. The cookie should be stored in MozillanCookieJar
format in a file named `~/.aurora-token`.
> 
> 
> Diffs
> -----
> 
>   RELEASE-NOTES.md 1819eaa20cf5014228643a1e120316d646cc2824 
>   docs/operations/security.md 46e0b8a9db654f52467f9adf36307a6a97a7a3ec 
>   src/main/python/apache/aurora/admin/aurora_admin.py fbebbab8c827b5695042d18770d850e31fc38122

>   src/main/python/apache/aurora/client/cli/client.py fa0c2648c5ff7ea6c9d949cf8cd9b9795d452e98

>   src/main/python/apache/aurora/common/cookie_auth_module.py PRE-CREATION 
>   src/test/python/apache/aurora/common/test_cookie_auth_module.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/51893/diff/
> 
> 
> Testing
> -------
> 
> $ cat ~/aurora/clusters.json
> [
> {
>   "name": "build",
>   "scheduler_uri": "https://aliaurora.cern.ch",
>   "auth_mechanism": "COOKIE"
> }
> ]
> $ dist/aurora.pex quota get build/root
> 
> 
> Thanks,
> 
> Giulio Eulisse
> 
>


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