UI: submit object storage creation using POST - #13768
Conversation
f5a58ef to
fe572b3
Compare
There was a problem hiding this comment.
Pull request overview
This PR fixes adding object storage from the CloudStack UI by switching the addObjectStoragePool API call from a GET (query-string) request to a form-encoded POST request, preventing credential leakage in URLs and ensuring compatibility with POST-enforcing configurations. It also hardens client/server diagnostic logging so that sensitive request contents are not emitted while keeping useful correlation metadata.
Changes:
- UI: submit
addObjectStoragePoolviapostAPI(URLSearchParams body) and mask the secret key field using a password input withautocomplete="off". - UI diagnostics: introduce centralized, safe Axios error logging that avoids serializing request/response bodies and query params.
- Server/API: mark
addObjectStoragePoolas carrying sensitive request info and suppress request-parameter logging (including legacy query strings) for this command; add corresponding unit tests.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| ui/src/views/infra/AddObjectStorage.vue | Switches object storage creation to POST and masks secret key input. |
| ui/src/utils/apiError.js | Adds safe API error summarization + logging to avoid leaking sensitive data. |
| ui/src/utils/request.js | Replaces raw response logging with safe API error logging in the request error path. |
| ui/src/utils/plugins.js | Uses safe API error logging in the global notifier error handler. |
| ui/tests/unit/views/infra/AddObjectStorage.spec.js | Adds unit coverage for POST transport, body encoding, error propagation, and secret-key masking. |
| ui/tests/unit/utils/apiError.spec.js | Adds unit tests ensuring only safe scalar metadata is logged/serialized. |
| server/src/main/java/com/cloud/api/ApiServlet.java | Suppresses parameter logging (and query string logging) for sensitive commands including addObjectStoragePool. |
| server/src/test/java/com/cloud/api/ApiServletTest.java | Adds tests validating parameter/query-string suppression and ordinary-command behavior. |
| api/src/main/java/org/apache/cloudstack/api/command/admin/storage/AddObjectStoragePoolCmd.java | Marks the command request as containing sensitive information. |
| api/src/test/java/org/apache/cloudstack/api/command/admin/storage/AddObjectStoragePoolCmdTest.java | Adds a test asserting the command is annotated as sensitive. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## 4.22 #13768 +/- ##
=========================================
Coverage 17.69% 17.70%
- Complexity 15835 15842 +7
=========================================
Files 5925 5926 +1
Lines 533539 533562 +23
Branches 65274 65277 +3
=========================================
+ Hits 94427 94469 +42
+ Misses 428435 428414 -21
- Partials 10677 10679 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
@blueorangutan package |
|
@DaanHoogland a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 18742 |
|
@blueorangutan test keepEnv |
|
@DaanHoogland a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
|
[SF] Trillian Build Failed (tid-16674) |
Description
This PR fixes object storage creation from the UI by submitting
addObjectStoragePoolas a form-encoded POST request instead of a GET request.The object-storage form carries provider access and secret keys in indexed
detailsparameters. Sending those parameters with GET both fails when POST enforcement is enabled and places credentials in the request URL. The UI now uses the existingpostAPItransport, preserving the command and parameter names while moving them into the request body.Credential handling is hardened across this request's failure and diagnostic paths as part of the same fix:
addObjectStoragePoolis classified as carrying sensitive request information;Ordinary non-sensitive API request logging is unchanged. Requests containing user data continue to receive the same parameter suppression. Object-store discovery, provider behavior, persistence, responses and authorization are unchanged.
Fixes #13679
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
Not applicable.
How Has This Been Tested?
Regression coverage exercises the request transport and each changed diagnostic boundary:
/, no query-parameter payload, preservation of indexed access-key and secret-key fields (including URL-sensitive characters) in the form body, failure propagation, and password masking;console.error;The patch passes
git diff --check. It was tested locally with Temurin Java 17 and Node 16.20.2.How did you try to break this feature and the system with this change?
The transport test uses the real indexed credential parameter names, includes
+,&and=in the secret, and asserts that Axios receives noparamsobject, preventing fallback to a URL query string. A rejected request verifies that backend failures are still propagated.Synthetic credential sentinels were placed in Axios POST bodies, GET parameters and response bodies; none appear in the emitted browser diagnostic. Nested object values were also supplied in fields intended for scalar diagnostics and were dropped rather than serialized. The management-server controls cover a sensitive legacy query string, a user-data request, and ordinary POST/query requests. Sensitive object-storage requests retain correlation metadata without their parameters; ordinary commands retain their normal diagnostics.