openSUSE / osc2

The next Command Line Interface to work with an openSUSE Build Service
http://openbuildservice.org/
17 stars 9 forks source link

Automatically send Authorization header #22

Closed nijel closed 9 years ago

nijel commented 9 years ago

We're dealing with known API which requires authorization in all cases. So it makes sense to send the Authorization header automatically and not wait for round trip to server to tell us that it needs it.

With Python's HTTPBasicAuthHandler the request is first tried without authorization and if that fails with 401, it is retried with it. This leads to invoking two HTTP requests for every operation.

marcus-h commented 9 years ago

On 2015-05-28 02:32:10 -0700, Michal Čihař wrote:

We're dealing with known API which requires authorization in all cases. So it makes sense to send the Authorization header automatically and not wait for round trip to server to tell us that it needs it.

Sounds reasonable - some time ago, we discussed this also for osc https://github.com/openSUSE/open-build-service/issues/833

With Python's HTTPBasicAuthHandler the request is first tried without authorization and if that fails with 401, it is retried with it. This leads to invoking two HTTP requests for every operation.

I just implemented it a bit differently (see commit a2a145a898b78513049d51382e8c9e3b3bb1997e). This way, we are a bit more "flexible" (for instance, it is possible to restore the old behavior (see the testcases in commit 171c7c32df37a965a8ba1e74d551ce9dee855b3f)). A small issue with your patch w.r.t. security is that it sends the credentials even if we got redirected to a different host (no apiurl). This might happen if we fetch the build dependencies for a local build.

If you encounter any problems with the current implementation, please tell me.

marcus-h commented 9 years ago

Btw., thanks for the pull request!:)