Repository navigation
feat(auth): accept instance authenticators on Cargo endpoints - #61
BaptisteCentreon wants to merge 3 commits into
Conversation
87aa0b6 to
7875c0b
Compare
| class CargoAuthMixin: | ||
| """Authenticate the Cargo token first, then the deployment's configured authenticators. | ||
|
|
||
| Cargo endpoints must not shadow ``DEFAULT_AUTHENTICATION_CLASSES``: hardcoding only |
There was a problem hiding this comment.
Avoid double-backticks please. And please disclose AI use in accordance with our AI policy https://github.com/pulp/pulp-docs/blob/main/pulpproject.org/help/more/governance/ai_policy.md
|
Apologies for taking some time to getting around to review these. It has been a busy couple of weeks |
| "dl": self.base_download_url, | ||
| "api": self.base_api_url, | ||
| "auth-required": False, | ||
| "auth-required": bool(self.distribution.content_guard_id), |
There was a problem hiding this comment.
Is this a general change, or do you specifically have in mind a use case for mixing content guards with cargo download?
There was a problem hiding this comment.
Hey, I am super busy at the moment, a colleague will be helping on the fixes you asked for.
About the change itself, I added to pulp core the equivalent of https://crates.io/docs/trusted-publishing, it allows CIs to get time and context bound privileges, this helps fight against Arrakis wildlife by ensuring CIs job don't use hardcoded tokens and have limited privileges.
Porting this feature here would duplicate code, so I looked at how other part of pulp (like docker for instance) handle delegating privileges managent to pulp core and replicated it here.
I hope it answers your question :)
Baptiste
|
Review from Codex, acting on behalf of @dralley. The endpoint-authentication part looks sound: prepend CargoTokenAuthentication, then retain the deployment default authentication classes. That preserves I have a blocker on deriving The new test only checks the JSON flag, so it cannot establish the advertised behavior. Please split or restrict this part until it has an end-to-end Minor wording point: crates.io trusted publishing exchanges an OIDC assertion for a short-lived publishing token; it is not raw OIDC authentication automatically performed by Cargo against an alternate registry. Pulp workload identity can be a supported case only when Cargo is configured to send the header form that authenticator expects (for example, |
Skip token scope checks when the request is not authenticated with a Cargo token. Since the Cargo views now accept the instance's default authenticators, request.auth can be None (Basic, session) or another object, which made publish and yank fail with a server error. Also shorten the mixin docstring. Assisted By: Claude Opus 5.5
Today the Cargo views (/me, publish, download/yank) hardcode authentication_classes = [CargoTokenAuthentication].
This overrides DEFAULT_AUTHENTICATION_CLASSES. So the endpoints only accept crg_ tokens. Every other credential the deployment supports (Basic, session, SSO, OIDC) gets rejected.
Authorization is not the problem: the views already call request.user.has_perm(...), which works for any user. Only authentication blocks them.
The fix
This PR replaces the hardcoded list with a small mixin. It keeps CargoTokenAuthentication first, then adds the instance's configured authenticators. So cargo login keeps working, and any other configured credential also reaches the Cargo endpoints. Nothing ties the plugin to a specific authenticator.
It also sets the index config.json auth-required flag from the distribution's content guard. A guarded distribution now tells cargo to send its credentials on the index and downloads, not only on publish.
Tests
I added functional tests. They check that Basic auth reaches /me, and that auth-required follows the content guard.