Skip to content

Conversation

@runcom
Copy link
Member

@runcom runcom commented Jan 17, 2017

It's totally wrong to rely on the WWW-Authenticate header from ping - gcr.io for instance always return 401 on /v2/ but that doesn't mean all repos need auth.
This patch clean up a bit this situation by not reading the auth header from ping (nor storing it anywhere in the docker client).
From now on, each request will be challenged for authentication needs (which is probably what the docker client does as well, otherwise I cannot see how this would be possible :))

@mtrmac PTAL? For more background and if I missed anything else: https://bugzilla.redhat.com/show_bug.cgi?id=1413987

Signed-off-by: Antonio Murdaca [email protected]

@runcom runcom force-pushed the fix-docker-setup-auth branch from 83ed97c to 9a9c486 Compare January 17, 2017 15:46
@mtrmac
Copy link
Collaborator

mtrmac commented Jan 17, 2017

(I haven’t actually read the bug and reviewed this in detail, just a first impression): I very much like where this is going, and it would completely handle my concerns about #191.

The one question I keep wondering about in this case is: what if no authentication is necessary, we send and empty body, and that succeeds? It seems to me that ideally we would want to send a request without the body, and wait for the server to either ACK the request and then send the body, or to respond with an authentication error. I vaguely seem to remember that something like that is possible in HTTP, a quick googling suggests sending an Expect: 100-continue header. Again, I haven’t looked into this in any manner of detail yet.

@runcom
Copy link
Member Author

runcom commented Jan 17, 2017

Found out what docker is doing in #211

@runcom runcom closed this Jan 17, 2017
@runcom runcom deleted the fix-docker-setup-auth branch January 17, 2017 18:08
giuseppe pushed a commit to giuseppe/image that referenced this pull request Jan 24, 2017
Vendor after merging mtrmac/image:api-changes and update API use
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants