Fix to URS authentication #57
Conversation
…redentials are portable
… available (HTTP 404)
|
This PR fixes in addition the ESGF auth tests. These tests were failing because the testing resource had disappeared. |
…ion catch in case of testing.
There was a problem hiding this comment.
Thanks so much for working on this! There are a couple of things that are unclear to me about what this code expects from the server that it's communicating with. Could you document that a little better so that other devs looking at this code can read the process? You've studied this long and hard, so I assume that you can put together some language pretty quickly.
src/pydap/cas/esgf.py
Outdated
| check_url=check_url, | ||
| session=session, | ||
| verify=verify) | ||
| # Connections can be lept alive on the ESGF: |
src/pydap/cas/get_cookies.py
Outdated
| headers = [('User-agent', pydap.lib.__version__)] | ||
| # Connections must be closed since some CAS | ||
| # will cough when connections are kept alive: | ||
| headers = [('User-agent', pydap__version__), |
There was a problem hiding this comment.
Should this be 'Pydap/{}'.format(pydap__version__), e.g. "Pydap/3.2.0" to conform to the user-agent RFC (see section 3.8).
| """ | ||
| This function essentially performs the following command: | ||
| >>> Request.blank(url).get_response(application) | ||
|
|
||
| It however makes sure that the request possesses the same cookies and | ||
| headers as the passed session. | ||
|
|
||
| It recursively follows 302 redirects. |
There was a problem hiding this comment.
What, may I ask, is the key change here? You've removed the 302 following, in exchange for a HEAD request and copying the headers? If that does the job, great, but what headers does NASA/URS set that makes this possible? Would you be able to document that in the code, because it's essentially an external precondition that we'll require to be true. If that ever ceases to be true (and you're not involved in the development), that will be important for other people to know to be able to identify what has gone wrong.
There was a problem hiding this comment.
In fact this change was made to make future development easier. Here's the thinking:
get_cookiesset the proper cookies insession. This is checked using thecheck_urloption, so we know it works.requestsis so well maintained that there are no reason why querying the url should fail: after all, we have checked that the credentials work.requestshandles all the cookies and the redirects. We simply have to use its result and pass it towebobfor the last mile.
Makes sense? I can document that in the code if it does.
| # head of the requested url. Requests will follow redirects and | ||
| # adjust the cookies as needed. We can then use the final url and | ||
| # the final cookies to set up a webob Request object that will | ||
| # be guaranteed to have all the needed credentials: |
There was a problem hiding this comment.
This helps quite a bit. I think most of what I was looking for is what exactly does URS do that we need to work so hard to work around, but this at least explains how this code handles the redirects. Thanks.
There was a problem hiding this comment.
My idea was not to focus on URS but to provide a general method that would always work. Combined with the URS tests, this should be sufficient...
This PR fixes recent problems with the URS authentication.
The new implementation is based more closely on the
requestslibrary and should be much more robust in the future.