Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Re-authenticate against OCI registry after 403 error #945

Merged
merged 1 commit into from
Jan 22, 2025

Conversation

imphil
Copy link
Contributor

@imphil imphil commented Jan 17, 2025

Writing to an OCI registry, as done with devcontainer features publish requires authentication against the registry with the push OAuth scope. Currently, devcontainer CLI only authenticates or re-authenticates if the registry returns a 401 error (invalid_token). But some registries, notably the IBM Container Registry (icr.io) may also return a 403 error (insufficient_scope) in case of a request being authenticated, but without sufficient scopes (i.e., the token was only valid for the pull scope, but push,pull was required for write access).

Update the code to attempt to re-authenticate in case of a 403 error, just like it's done for a 401 error. The server does supply the correct scope in its WWW-Authenticate header and subsequent requests will then work as expected.

See also RFC 6750, Section 3.1 (Error Codes) for a standards reference.

This improvement makes devcontainer features publish work with IBM Cloud Container Registry.


Test:

$ devcontainer.js features publish --registry icr.io -n my-ns/features ~/my-feature`

HTTP trace (abbreviated) before this change:

-> POST https://icr.io/v2/my-ns/features/my-feature/tags/list
-> 401 Unauthorized
   www-authenticate: Bearer realm="https://icr.io/oauth/token",service="registry",scope="repository:my-ns/features/my-feature:pull"

-> POST https://icr.io/oauth/token
   client_id=devcontainer&grant_type=refresh_token&service=registry&scope=repository%3Amy-ns%2Ffeatures%2Fmy-feature%3Apull&refresh_token=...
<- 200 OK

-> POST https://icr.io/v2/my-ns/features/my-feature/blobs/uploads/
   authorization: Bearer
<- 403 Forbidden:
   www-authenticate: Bearer realm="https://icr.io/oauth/token",service="registry",scope="repository:my-ns/features/my-feature:pull,push",error="insufficient_scope"

HTTP trace (abbreviated) before after change:

-> POST https://icr.io/v2/my-ns/features/my-feature/tags/list
-> 401 Unauthorized
   www-authenticate: Bearer realm="https://icr.io/oauth/token",service="registry",scope="repository:my-ns/features/my-feature:pull"

-> POST https://icr.io/oauth/token
   client_id=devcontainer&grant_type=refresh_token&service=registry&scope=repository%3Amy-ns%2Ffeatures%2Fmy-feature%3Apull&refresh_token=...
<- 200 OK

-> POST https://icr.io/v2/my-ns/features/my-feature/blobs/uploads/
   authorization: Bearer
<- 403 Forbidden:
   www-authenticate: Bearer realm="https://icr.io/oauth/token",service="registry",scope="repository:my-ns/features/my-feature:pull,push",error="insufficient_scope"

-> POST https://icr.io/oauth/token
   client_id=devcontainer&grant_type=refresh_token&service=registry&scope=repository%3Amy-ns%2Ffeatures%2Fmy-feature%3Apull%2Cpush&refresh_token=...
<- 200 OK

Note the second auth request after the 403 response.

Writing to an OCI registry, as done with `devcontainer features publish`
requires authentication against the registry with the `push` OAuth
scope. Currently, devcontainer CLI only authenticates or
re-authenticates if the registry returns a 401 error (invalid_token).
But some registries, notably the IBM Container Registry (icr.io) may
also return a 403 error (insufficient_scope) in case of a request being
authenticated, but without sufficient scopes (i.e., the token was only
valid for the `pull` scope, but `push,pull` was required for write
access).

Update the code to attempt to re-authenticate in case of a 403 error,
just like it's done for a 401 error. The server does supply the correct
scope in its `WWW-Authenticate` header and subsequent requests will then
work as expected.

See also [RFC 6750, Section 3.1 (Error Codes)](https://datatracker.ietf.org/doc/html/rfc6750#section-3.1)
for a standards reference.

This improvement makes `devcontainer features publish` work with IBM
Cloud Container Registry.

---

Test:

```
$ devcontainer.js features publish --registry icr.io -n my-ns/features ~/my-feature`
```

HTTP trace (abbreviated) *before* this change:

```
-> POST https://icr.io/v2/my-ns/features/my-feature/tags/list
-> 401 Unauthorized
   www-authenticate: Bearer realm="https://icr.io/oauth/token",service="registry",scope="repository:my-ns/features/my-feature:pull"

-> POST https://icr.io/oauth/token
   client_id=devcontainer&grant_type=refresh_token&service=registry&scope=repository%3Amy-ns%2Ffeatures%2Fmy-feature%3Apull&refresh_token=...
<- 200 OK

-> POST https://icr.io/v2/my-ns/features/my-feature/blobs/uploads/
   authorization: Bearer
<- 403 Forbidden:
   www-authenticate: Bearer realm="https://icr.io/oauth/token",service="registry",scope="repository:my-ns/features/my-feature:pull,push",error="insufficient_scope"
```

HTTP trace (abbreviated) before *after* change:

```
-> POST https://icr.io/v2/my-ns/features/my-feature/tags/list
-> 401 Unauthorized
   www-authenticate: Bearer realm="https://icr.io/oauth/token",service="registry",scope="repository:my-ns/features/my-feature:pull"

-> POST https://icr.io/oauth/token
   client_id=devcontainer&grant_type=refresh_token&service=registry&scope=repository%3Amy-ns%2Ffeatures%2Fmy-feature%3Apull&refresh_token=...
<- 200 OK

-> POST https://icr.io/v2/my-ns/features/my-feature/blobs/uploads/
   authorization: Bearer
<- 403 Forbidden:
   www-authenticate: Bearer realm="https://icr.io/oauth/token",service="registry",scope="repository:my-ns/features/my-feature:pull,push",error="insufficient_scope"

-> POST https://icr.io/oauth/token
   client_id=devcontainer&grant_type=refresh_token&service=registry&scope=repository%3Amy-ns%2Ffeatures%2Fmy-feature%3Apull%2Cpush&refresh_token=...
<- 200 OK
```

Note the second auth request after the 403 response.
@imphil imphil requested a review from a team as a code owner January 17, 2025 22:00
@imphil
Copy link
Contributor Author

imphil commented Jan 22, 2025

Thanks for your review, @chrmarti!

Just to add an observation I made today: a similar issue also affects the Dev Containers VS Code extension (which vendors the devcontainers-cli code here AFAIK) when pulling more than one feature from ICR. The scenario is as follows:

// from devcontainer.json
	"features": {
		"icr.io/my-ns/features/feature-1:1": {},
		"icr.io/my-ns/features/feature-2:1": {}
	},

During Dev Container Rebuild, I observe the following HTTP requests:

-> GET https://icr.io/v2/my-ns/features/feature-1/manifests/1 
<- 401 Forbidden
   www-authenticate: Bearer realm="https://icr.io/oauth/token",service="registry",scope="repository:my-ns/features/feature-1:pull

-> POST https://icr.io/oauth/token
   client_id=devcontainer&grant_type=refresh_token&service=registry&scope=repository%3Amy-ns%2Ffeatures%2Ffeature-1%3Apull&refresh_token=...
<- 200 OK

-> GET https://icr.io/v2/my-ns/features/feature-1/manifests/1 
<- 200 OK
   www-authenticate: Bearer realm="https://icr.io/oauth/token",service="registry",scope="repository:my-ns/features/feature-1:pull

All good so far. Now a second feature is requested, using the cached auth from the previous step

output.write(`[httpOci] Applying cachedAuthHeader for registry ${ociRef.registry}...`, LogLevel.Trace);

-> GET https://icr.io/v2/my-ns/features/feature-2/manifests/1
<- 403 Forbidden
   www-authenticate: Bearer realm="https://icr.io/oauth/token",service="registry",scope="repository:my-ns/features/feature-2:pull",error="insufficient_scope"

And since that's a 403, Dev Containers would not try to obtain updated credentials and the container startup fails. I'd assume that the PR here also fixes this problem once the code gets vendored in the Remote Development extension (but I obviously have no way to test that).

Underlying to all of this seems the rather tight interpretation of scopes that ICR applies. I'd say that's not super efficient (because we have more roundtrips to obtain new tokens), but it's still in line with what the OAuth spec allows.

@chrmarti
Copy link
Contributor

@imphil Thanks for the PR! I agree, it looks like the extension's issue should be fixed with this from what I see. 👍

(Will get another reviewer and then merge.)

@chrmarti chrmarti enabled auto-merge (rebase) January 22, 2025 18:20
@chrmarti chrmarti merged commit 9291203 into devcontainers:main Jan 22, 2025
17 of 18 checks passed
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.

3 participants