-
Notifications
You must be signed in to change notification settings - Fork 278
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
mod: what should happen when an OCI server returns a 401 response? #2955
Labels
Comments
What do other clients do in this scenario, e.g. |
@mvdan The difference is that CUE tries to look up multiple OCI repos, while |
porcuepine
pushed a commit
to cue-labs/oci
that referenced
this issue
Mar 27, 2024
Some registries will return a 401 Unauthorized error, indicating that no valid auth credentials have been provided, even when valid auth credentials _have_ been provided. Although this goes against [the HTTP status code conventions](https://stackoverflow.com/a/6937030) we need to deal with this somehow, because otherwise a client cannot distinguish between a "bad auth credentials error" (meaning that if the user does authenticate, the error might go away) and an "auth credentials not valid for resource error" (meaning that the user is authenticated but cannot access the resource despite that). The `ociauth` package is in a unique position to be able to make this determination because it the only place that knows that auth credentials have been freshly acquired, therefore a subsequent 401 error is almost certainly because the privileges were insufficient for the authenticated user rather than because there is no authenticated user. So, we change `ociauth` to return 403 Forbidden in this case. Fixes cue-lang/cue#2955 Signed-off-by: Roger Peppe <rogpeppe@gmail.com> Change-Id: Ie50bb826e266d3b26f06881d41da36f740bc43ab
porcuepine
pushed a commit
to cue-labs/oci
that referenced
this issue
Mar 27, 2024
Some registries will return a 401 Unauthorized error, indicating that no valid auth credentials have been provided, even when valid auth credentials _have_ been provided. Although this goes against [the HTTP status code conventions](https://stackoverflow.com/a/6937030) we need to deal with this somehow, because otherwise a client cannot distinguish between a "bad auth credentials error" (meaning that if the user does authenticate, the error might go away) and an "auth credentials not valid for resource error" (meaning that the user is authenticated but cannot access the resource despite that). The `ociauth` package is in a unique position to be able to make this determination because it the only place that knows that auth credentials have been freshly acquired, therefore a subsequent 401 error is almost certainly because the privileges were insufficient for the authenticated user rather than because there is no authenticated user. So, we change `ociauth` to return 403 Forbidden in this case. Fixes cue-lang/cue#2955 Signed-off-by: Roger Peppe <rogpeppe@gmail.com> Change-Id: Ie50bb826e266d3b26f06881d41da36f740bc43ab
porcuepine
pushed a commit
to cue-labs/oci-trybot
that referenced
this issue
Mar 27, 2024
Some registries will return a 401 Unauthorized error, indicating that no valid auth credentials have been provided, even when valid auth credentials _have_ been provided. Although this goes against [the HTTP status code conventions](https://stackoverflow.com/a/6937030) we need to deal with this somehow, because otherwise a client cannot distinguish between a "bad auth credentials error" (meaning that if the user does authenticate, the error might go away) and an "auth credentials not valid for resource error" (meaning that the user is authenticated but cannot access the resource despite that). The `ociauth` package is in a unique position to be able to make this determination because it the only place that knows that auth credentials have been freshly acquired, therefore a subsequent 401 error is almost certainly because the privileges were insufficient for the authenticated user rather than because there is no authenticated user. So, we change `ociauth` to return 403 Forbidden in this case. Fixes cue-lang/cue#2955 Signed-off-by: Roger Peppe <rogpeppe@gmail.com> Change-Id: Ie50bb826e266d3b26f06881d41da36f740bc43ab Dispatch-Trailer: {"type":"trybot","CL":1188182,"patchset":2,"ref":"refs/changes/82/1188182/2","targetBranch":"main"}
porcuepine
pushed a commit
to cue-labs/oci
that referenced
this issue
Mar 27, 2024
Some registries will return a 401 Unauthorized error, indicating that no valid auth credentials have been provided, even when valid auth credentials _have_ been provided. Although this goes against [the HTTP status code conventions](https://stackoverflow.com/a/6937030) we need to deal with this somehow, because otherwise a client cannot distinguish between a "bad auth credentials error" (meaning that if the user does authenticate, the error might go away) and an "auth credentials not valid for resource error" (meaning that the user is authenticated but cannot access the resource despite that). The `ociauth` package is in a unique position to be able to make this determination because it the only place that knows that auth credentials have been freshly acquired, therefore a subsequent 401 error is almost certainly because the privileges were insufficient for the authenticated user rather than because there is no authenticated user. So, we change `ociauth` to return 403 Forbidden in this case. Fixes cue-lang/cue#2955 Signed-off-by: Roger Peppe <rogpeppe@gmail.com> Change-Id: Ie50bb826e266d3b26f06881d41da36f740bc43ab
porcuepine
pushed a commit
to cue-labs/oci-trybot
that referenced
this issue
Mar 27, 2024
Some registries will return a 401 Unauthorized error, indicating that no valid auth credentials have been provided, even when valid auth credentials _have_ been provided. Although this goes against [the HTTP status code conventions](https://stackoverflow.com/a/6937030) we need to deal with this somehow, because otherwise a client cannot distinguish between a "bad auth credentials error" (meaning that if the user does authenticate, the error might go away) and an "auth credentials not valid for resource error" (meaning that the user is authenticated but cannot access the resource despite that). The `ociauth` package is in a unique position to be able to make this determination because it the only place that knows that auth credentials have been freshly acquired, therefore a subsequent 401 error is almost certainly because the privileges were insufficient for the authenticated user rather than because there is no authenticated user. So, we change `ociauth` to return 403 Forbidden in this case. Fixes cue-lang/cue#2955 Signed-off-by: Roger Peppe <rogpeppe@gmail.com> Change-Id: Ie50bb826e266d3b26f06881d41da36f740bc43ab Dispatch-Trailer: {"type":"trybot","CL":1188182,"patchset":3,"ref":"refs/changes/82/1188182/3","targetBranch":"main"}
porcuepine
pushed a commit
to cue-labs/oci
that referenced
this issue
Mar 28, 2024
Some registries will return a 401 Unauthorized error, indicating that no valid auth credentials have been provided, even when valid auth credentials _have_ been provided. Although this goes against [the HTTP status code conventions](https://stackoverflow.com/a/6937030) we need to deal with this somehow, because otherwise a client cannot distinguish between a "bad auth credentials error" (meaning that if the user does authenticate, the error might go away) and an "auth credentials not valid for resource error" (meaning that the user is authenticated but cannot access the resource despite that). The `ociauth` package is in a unique position to be able to make this determination because it the only place that knows that auth credentials have been freshly acquired, therefore a subsequent 401 error is almost certainly because the privileges were insufficient for the authenticated user rather than because there is no authenticated user. So, we change `ociauth` to return 403 Forbidden in this case. Fixes cue-lang/cue#2955 Signed-off-by: Roger Peppe <rogpeppe@gmail.com> Change-Id: Ie50bb826e266d3b26f06881d41da36f740bc43ab
porcuepine
pushed a commit
to cue-labs/oci-trybot
that referenced
this issue
Mar 28, 2024
Some registries will return a 401 Unauthorized error, indicating that no valid auth credentials have been provided, even when valid auth credentials _have_ been provided. Although this goes against [the HTTP status code conventions](https://stackoverflow.com/a/6937030) we need to deal with this somehow, because otherwise a client cannot distinguish between a "bad auth credentials error" (meaning that if the user does authenticate, the error might go away) and an "auth credentials not valid for resource error" (meaning that the user is authenticated but cannot access the resource despite that). The `ociauth` package is in a unique position to be able to make this determination because it the only place that knows that auth credentials have been freshly acquired, therefore a subsequent 401 error is almost certainly because the privileges were insufficient for the authenticated user rather than because there is no authenticated user. So, we change `ociauth` to return 403 Forbidden in this case. Fixes cue-lang/cue#2955 Signed-off-by: Roger Peppe <rogpeppe@gmail.com> Change-Id: Ie50bb826e266d3b26f06881d41da36f740bc43ab Dispatch-Trailer: {"type":"trybot","CL":1188182,"patchset":4,"ref":"refs/changes/82/1188182/4","targetBranch":"main"}
porcuepine
pushed a commit
to cue-labs/oci
that referenced
this issue
Mar 28, 2024
Some registries will return a 401 Unauthorized error, indicating that no valid auth credentials have been provided, even when valid auth credentials _have_ been provided. Although this goes against [the HTTP status code conventions](https://stackoverflow.com/a/6937030) we need to deal with this somehow, because otherwise a client cannot distinguish between a "bad auth credentials error" (meaning that if the user does authenticate, the error might go away) and an "auth credentials not valid for resource error" (meaning that the user is authenticated but cannot access the resource despite that). The `ociauth` package is in a unique position to be able to make this determination because it the only place that knows that auth credentials have been freshly acquired, therefore a subsequent 401 error is almost certainly because the privileges were insufficient for the authenticated user rather than because there is no authenticated user. So, we change `ociauth` to return 403 Forbidden in this case. Fixes cue-lang/cue#2955 Signed-off-by: Roger Peppe <rogpeppe@gmail.com> Change-Id: Ie50bb826e266d3b26f06881d41da36f740bc43ab
porcuepine
pushed a commit
to cue-labs/oci-trybot
that referenced
this issue
Mar 28, 2024
Some registries will return a 401 Unauthorized error, indicating that no valid auth credentials have been provided, even when valid auth credentials _have_ been provided. Although this goes against [the HTTP status code conventions](https://stackoverflow.com/a/6937030) we need to deal with this somehow, because otherwise a client cannot distinguish between a "bad auth credentials error" (meaning that if the user does authenticate, the error might go away) and an "auth credentials not valid for resource error" (meaning that the user is authenticated but cannot access the resource despite that). The `ociauth` package is in a unique position to be able to make this determination because it the only place that knows that auth credentials have been freshly acquired, therefore a subsequent 401 error is almost certainly because the privileges were insufficient for the authenticated user rather than because there is no authenticated user. So, we change `ociauth` to return 403 Forbidden in this case. Fixes cue-lang/cue#2955 Signed-off-by: Roger Peppe <rogpeppe@gmail.com> Change-Id: Ie50bb826e266d3b26f06881d41da36f740bc43ab Dispatch-Trailer: {"type":"trybot","CL":1188182,"patchset":5,"ref":"refs/changes/82/1188182/5","targetBranch":"main"}
porcuepine
pushed a commit
to cue-labs/oci
that referenced
this issue
Mar 28, 2024
Some registries will return a 401 Unauthorized error, indicating that no valid auth credentials have been provided, even when valid auth credentials _have_ been provided. Although this goes against [the HTTP status code conventions](https://stackoverflow.com/a/6937030) we need to deal with this somehow, because otherwise a client cannot distinguish between a "bad auth credentials error" (meaning that if the user does authenticate, the error might go away) and an "auth credentials not valid for resource error" (meaning that the user is authenticated but cannot access the resource despite that). The `ociauth` package is in a unique position to be able to make this determination because it the only place that knows that auth credentials have been freshly acquired, therefore a subsequent 401 error is almost certainly because the privileges were insufficient for the authenticated user rather than because there is no authenticated user. So, we change `ociauth` to return 403 Forbidden in this case. Fixes cue-lang/cue#2955 Signed-off-by: Roger Peppe <rogpeppe@gmail.com> Change-Id: Ie50bb826e266d3b26f06881d41da36f740bc43ab Reviewed-on: https://review.gerrithub.io/c/cue-labs/oci/+/1188182 TryBot-Result: CUE porcuepine <cue.porcuepine@gmail.com> Reviewed-by: Tianon Gravi <admwiggin@gmail.com> Reviewed-by: Paul Jolly <paul@myitcv.io>
porcuepine
pushed a commit
to cue-labs/oci
that referenced
this issue
Apr 4, 2024
The current logic triggers the 401->403 logic even when the credentials are not acquired from a token server. We want to be more specific than that. For cue-lang/cue#2955. Signed-off-by: Roger Peppe <rogpeppe@gmail.com> Change-Id: I8d826cff31554ecdb78bb580e72a2d8258ae6565
porcuepine
pushed a commit
to cue-labs/oci-trybot
that referenced
this issue
Apr 4, 2024
The current logic triggers the 401->403 logic even when the credentials are not acquired from a token server. We want to be more specific than that. For cue-lang/cue#2955. Signed-off-by: Roger Peppe <rogpeppe@gmail.com> Change-Id: I8d826cff31554ecdb78bb580e72a2d8258ae6565 Dispatch-Trailer: {"type":"trybot","CL":1191614,"patchset":2,"ref":"refs/changes/14/1191614/2","targetBranch":"main"}
porcuepine
pushed a commit
to cue-labs/oci-trybot
that referenced
this issue
Apr 4, 2024
The current logic triggers the 401->403 logic even when the credentials are not acquired from a token server. We want to be more specific than that. For cue-lang/cue#2955. Signed-off-by: Roger Peppe <rogpeppe@gmail.com> Change-Id: I8d826cff31554ecdb78bb580e72a2d8258ae6565 Dispatch-Trailer: {"type":"trybot","CL":1191614,"patchset":3,"ref":"refs/changes/14/1191614/3","targetBranch":"main"}
porcuepine
pushed a commit
to cue-labs/oci
that referenced
this issue
Apr 4, 2024
The current logic triggers the 401->403 logic even when the credentials are not acquired from a token server. We want to be more specific than that. For cue-lang/cue#2955. Signed-off-by: Roger Peppe <rogpeppe@gmail.com> Change-Id: I8d826cff31554ecdb78bb580e72a2d8258ae6565
cueckoo
pushed a commit
that referenced
this issue
Apr 4, 2024
When considering what repositories might contain a module, the module resolution logic is careful to distinguish between a simple "not found" error (discarded silently) and any other kind of error (triggers an error in the whole module resolution process). When a user has authenticated but does not have access to a given module, the HTTP standard allows the server to return a 403 (Forbidden) error to indicate that the user doesn't have permission to access a page. When considering possible candidates for modules, we don't want to fail if there are some modules that exist that we can't access, so this CL changes the logic to treat any 403 error the same as "not found" in this respect. We also add tests for this and for token-server-based authentication, verifying that with the OCI dependency updated as it is in this CL, issue #2955 is actually fixed. Fixes #2955. Signed-off-by: Roger Peppe <rogpeppe@gmail.com> Change-Id: I665b1aa28be255bb2e4a00bfc606b0899575fec1
cueckoo
pushed a commit
that referenced
this issue
Apr 4, 2024
When considering what repositories might contain a module, the module resolution logic is careful to distinguish between a simple "not found" error (discarded silently) and any other kind of error (triggers an error in the whole module resolution process). When a user has authenticated but does not have access to a given module, the HTTP standard allows the server to return a 403 (Forbidden) error to indicate that the user doesn't have permission to access a page. When considering possible candidates for modules, we don't want to fail if there are some modules that exist that we can't access, so this CL changes the logic to treat any 403 error the same as "not found" in this respect. We also add tests for this and for token-server-based authentication, verifying that with the OCI dependency updated as it is in this CL, issue #2955 is actually fixed. The `registrytest` package API is changed to allow this, because the previous `AuthHandler` function was not sufficient to allow an independent token handler server listening on a different port. Fixes #2955. Signed-off-by: Roger Peppe <rogpeppe@gmail.com> Change-Id: I665b1aa28be255bb2e4a00bfc606b0899575fec1
cueckoo
pushed a commit
that referenced
this issue
Apr 4, 2024
When considering what repositories might contain a module, the module resolution logic is careful to distinguish between a simple "not found" error (discarded silently) and any other kind of error (triggers an error in the whole module resolution process). When a user has authenticated but does not have access to a given module, the HTTP standard allows the server to return a 403 (Forbidden) error to indicate that the user doesn't have permission to access a page. When considering possible candidates for modules, we don't want to fail if there are some modules that exist that we can't access, so this CL changes the logic to treat any 403 error the same as "not found" in this respect. We also make the error message when there is a permanent error more specific so it actually mentions the module that it's trying to resolve. We also add tests for this and for token-server-based authentication, verifying that with the OCI dependency updated as it is in this CL, issue #2955 is actually fixed. The `registrytest` package API is changed to allow this, because the previous `AuthHandler` function was not sufficient to allow an independent token handler server listening on a different port. Fixes #2955. Signed-off-by: Roger Peppe <rogpeppe@gmail.com> Change-Id: I665b1aa28be255bb2e4a00bfc606b0899575fec1
porcuepine
pushed a commit
to cue-labs/oci
that referenced
this issue
Apr 4, 2024
The current logic triggers the 401->403 logic even when the credentials are not acquired from a token server. We want to be more specific than that. For cue-lang/cue#2955. Signed-off-by: Roger Peppe <rogpeppe@gmail.com> Change-Id: I8d826cff31554ecdb78bb580e72a2d8258ae6565 Reviewed-on: https://review.gerrithub.io/c/cue-labs/oci/+/1191614 TryBot-Result: CUE porcuepine <cue.porcuepine@gmail.com> Reviewed-by: Rustam Abdullaev <rustam@cue.works>
cueckoo
pushed a commit
that referenced
this issue
Apr 4, 2024
When considering what repositories might contain a module, the module resolution logic is careful to distinguish between a simple "not found" error (discarded silently) and any other kind of error (triggers an error in the whole module resolution process). When a user has authenticated but does not have access to a given module, the HTTP standard allows the server to return a 403 (Forbidden) error to indicate that the user doesn't have permission to access a page. When considering possible candidates for modules, we don't want to fail if there are some modules that exist that we can't access, so this CL changes the logic to treat any 403 error the same as "not found" in this respect. We also make the error message when there is a permanent error more specific so it actually mentions the module that it's trying to resolve. We also add tests for this and for token-server-based authentication, verifying that with the OCI dependency updated as it is in this CL, issue #2955 is actually fixed. The `registrytest` package API is changed to allow this, because the previous `AuthHandler` function was not sufficient to allow an independent token handler server listening on a different port. Fixes #2955. Signed-off-by: Roger Peppe <rogpeppe@gmail.com> Change-Id: I665b1aa28be255bb2e4a00bfc606b0899575fec1
cueckoo
pushed a commit
that referenced
this issue
Apr 5, 2024
When considering what repositories might contain a module, the module resolution logic is careful to distinguish between a simple "not found" error (discarded silently) and any other kind of error (triggers an error in the whole module resolution process). When a user has authenticated but does not have access to a given module, the HTTP standard allows the server to return a 403 (Forbidden) error to indicate that the user doesn't have permission to access a page. When considering possible candidates for modules, we don't want to fail if there are some modules that exist that we can't access, so this CL changes the logic to treat any 403 error the same as "not found" in this respect. We also make the error message when there is a permanent error more specific so it actually mentions the module that it's trying to resolve. We also add tests for this and for token-server-based authentication, verifying that with the OCI dependency updated as it is in this CL, issue #2955 is actually fixed. The `registrytest` package API is changed to allow this, because the previous `AuthHandler` function was not sufficient to allow an independent token handler server listening on a different port. Fixes #2955. Signed-off-by: Roger Peppe <rogpeppe@gmail.com> Change-Id: I665b1aa28be255bb2e4a00bfc606b0899575fec1
cueckoo
pushed a commit
that referenced
this issue
Apr 5, 2024
When considering what repositories might contain a module, the module resolution logic is careful to distinguish between a simple "not found" error (discarded silently) and any other kind of error (triggers an error in the whole module resolution process). When a user has authenticated but does not have access to a given module, the HTTP standard allows the server to return a 403 (Forbidden) error to indicate that the user doesn't have permission to access a page. When considering possible candidates for modules, we don't want to fail if there are some modules that exist that we can't access, so this CL changes the logic to treat any 403 error the same as "not found" in this respect. We also make the error message when there is a permanent error more specific so it actually mentions the module that it's trying to resolve. We also add tests for this and for token-server-based authentication, verifying that with the OCI dependency updated as it is in this CL, issue #2955 is actually fixed. The `registrytest` package API is changed to allow this, because the previous `AuthHandler` function was not sufficient to allow an independent token handler server listening on a different port. Fixes #2955. Signed-off-by: Roger Peppe <rogpeppe@gmail.com> Change-Id: I665b1aa28be255bb2e4a00bfc606b0899575fec1
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
What version of CUE are you using (
cue version
)?Does this issue reproduce with the latest stable release?
N/A
What did you do?
This isn't the actual reproducer because I don't have access to the registry, but
this is the gist:
What did you expect to see?
An error saying that the module is unavailable.
What did you see instead?
The server is returning a 401 error rather than a 404, even though the client has acquired a valid auth token.
The HTTP log looks like this (names changed, cleaned up somewhat and run through
jq .
):The text was updated successfully, but these errors were encountered: