Skip to content

docker_client: Handle "invalid_scope" errors - #1640

Merged
mtrmac merged 1 commit into
containers:mainfrom
dcermak:handle-invalid-scope
Aug 30, 2022
Merged

docker_client: Handle "invalid_scope" errors#1640
mtrmac merged 1 commit into
containers:mainfrom
dcermak:handle-invalid-scope

Conversation

@dcermak

@dcermak dcermak commented Aug 25, 2022

Copy link
Copy Markdown
Contributor

This is a rebase of #1079 and fixes #1478

@dcermak
dcermak force-pushed the handle-invalid-scope branch from 4005291 to ac2ca5d Compare August 25, 2022 09:20

@mtrmac mtrmac left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for reviving this!

Let’s get this finally done.

The overall approach LGTM.

Comment thread docker/docker_client.go Outdated
Comment thread docker/docker_client.go Outdated
Comment thread docker/docker_client.go Outdated
Comment thread docker/docker_client.go Outdated
Comment thread docker/docker_client.go
Comment thread docker/docker_client.go Outdated
Comment thread docker/docker_client.go
Comment thread docker/docker_client.go Outdated
Comment thread docker/docker_client.go Outdated
@dcermak
dcermak force-pushed the handle-invalid-scope branch 3 times, most recently from 1e05f27 to 12b2398 Compare August 26, 2022 09:36
@dcermak

dcermak commented Aug 26, 2022

Copy link
Copy Markdown
Contributor Author

@mtrmac Thank you for the review! I have tried addressing your comments and added a few unit tests.

@mtrmac mtrmac left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! That’s a great amount of tests.

Comment thread docker/docker_client.go Outdated
Comment thread docker/docker_client_test.go
@dcermak
dcermak force-pushed the handle-invalid-scope branch from 12b2398 to 2fd3305 Compare August 29, 2022 08:02
Comment thread docker/docker_client.go Outdated
@dcermak
dcermak requested a review from mtrmac August 29, 2022 08:06

@mtrmac mtrmac left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks:

Last round from me, I promise: #1640 (comment) — the panic is a bit of a risk.

#1640 (comment) would be nice but is not blocking for me.

By default docker_client just uses the auth challenges from the /v2/
ping request to request a Bearer Token. For some requests (e.g. for
/v2/_catalog on some registries) this might not be sufficient and return a
a HTTP Unauthorized Error with the "www-authenticate" header including
an "insufficient_scope" error. In that case the client will now retry
the request and fetch a new token with updated challenges to have the
"scope" matching for what the endpoint needs.

This fixes containers#1478

Signed-off-by: Ralf Haferkamp <rhafer@suse.com>
Signed-off-by: Ralf Haferkamp <ralf@h4kamp.de>
Signed-off-by: Dan Čermák <dcermak@suse.com>
Signed-off-by: Miloslav Trmač <mitr@redhat.com>

Co-authored-by: Miloslav Trmač <mitr@redhat.com>
Co-authored-by: Ralf Haferkamp <ralf@h4kamp.de>
@dcermak
dcermak force-pushed the handle-invalid-scope branch from 2fd3305 to 3ce7f05 Compare August 30, 2022 07:02
@dcermak
dcermak requested a review from mtrmac August 30, 2022 07:05

@mtrmac mtrmac left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks again!

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.

Fix connecting to registries with non-hardcoded auth scopes

2 participants