Make cache service work with authenticated asset downloads #6120
+67
−55
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Commit 4c8ed39 introduced an option to to restrict asset downloads to logged-in users. This was literally only allowing asset downloads to users that were logged-in using the web UI. To move this forward and implement https://progress.opensuse.org/issues/174301 it is required to allow other authentication our other API authentication methods as well. This change therefore enables the use of our normal API authentication methods for read-only asset routes by simply making use of the auth controller we normally use for API requests. If authentication for asset is not configured this has of course no effect. This allows queries like to return the expected 200/302 response (or 403 in case the specified credentials are wrong). ``` curl -i -u Demo:…:… 'http://localhost:9526/assets/other/openSUSE-Tumbleweed-DVD-x86_64-Snapshot20230109-Media.iso.sha256' curl -i -u Demo:…:… 'http://localhost:9526/tests/4416/asset/other/openSUSE-Tumbleweed-DVD-x86_64-Snapshot20250112-Media.iso.sha256' ``` Before, these queries would instead always redirect to the login route of the web UI. The change to the web API routes alone would break asset downloads via links on the web UI as those routes would now always require the CSRF token to be supplied. This commit therefore also changes the API auth controller to allow GET requests without CSRF token. We already allow GET requests without CSRF token on all web UI routes in the session controller so this should be fine. (Additionally, browsers disallow cross-site scripting out of the box and the GET API routes never require any authentication anyway.)
* Add an additional route that returns whether a user is authenticated * Use `X-Original-URI` for the HMAC computation in this additional route so the token auth works despite the changed request URL * Extend configuration template for NGINX to show an example configuration * See https://docs.nginx.com/nginx/admin-guide/security-controls/configuring-subrequest-authentication * See https://nginx.org/en/docs/http/ngx_http_auth_request_module.html * See https://progress.opensuse.org/issues/174301
* Use API credentials in cache service downloads * See https://progress.opensuse.org/issues/170380
* Use signatures * Use more compact coding style
The openQA user agent code so far preserves manually set headers including the `X-API-…` headers used for authentication. This is probably not required and has the problematic side-effect that those headers are not updated when following redirections. That means authentication fails when a redirection is in place. This is the case when querying assets of a job as done by the cache service. In order to make the cache service work when authentication for assets is enabled via 4c8ed39 this change updates the `X-API-…` headers regardless of whether they are already present. Any custom values will be overridden but preserving custom values is not required anyways. (The accepted header will still be preserved.) Related ticket: https://progress.opensuse.org/issues/174154
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #6120 +/- ##
=======================================
Coverage 98.98% 98.98%
=======================================
Files 396 396
Lines 39556 39557 +1
=======================================
+ Hits 39156 39157 +1
Misses 400 400 ☔ View full report in Codecov by Sentry. |
okurz
approved these changes
Jan 15, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR incorporates #6114 which should probably be merged first.
It will tweak the cache service and the openQA user agent in general so both work when authenticated asset downloads are enabled. See https://progress.opensuse.org/issues/170380 and individual commit messages for details.
If wanted I can also extend tests. (Maybe I'll have to adapt tests anyway.) I tested this locally. Before this change it didn't work and with it it works when connecting directly and when using NGINX.