Skip to content

UI: submit object storage creation using POST - #13768

Open
Dogface2k wants to merge 3 commits into
apache:4.22from
Dogface2k:fix/object-storage-post
Open

UI: submit object storage creation using POST#13768
Dogface2k wants to merge 3 commits into
apache:4.22from
Dogface2k:fix/object-storage-post

Conversation

@Dogface2k

@Dogface2k Dogface2k commented Aug 2, 2026

Copy link
Copy Markdown

Description

This PR fixes object storage creation from the UI by submitting addObjectStoragePool as a form-encoded POST request instead of a GET request.

The object-storage form carries provider access and secret keys in indexed details parameters. Sending those parameters with GET both fails when POST enforcement is enabled and places credentials in the request URL. The UI now uses the existing postAPI transport, 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:

  • addObjectStoragePool is classified as carrying sensitive request information;
  • management-server START/END logging keeps the client, HTTP method and command but excludes request parameters for this command, whether they arrived through POST or a legacy query string;
  • browser error diagnostics remain enabled and retain error name/code, integer HTTP status, method and CloudStack command, without serializing Axios request bodies, query parameters, response bodies or unexpected nested objects;
  • the secret-key field uses a password input with autocomplete disabled.

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

  • Breaking change (fix or feature that would cause existing functionality to change)
  • New feature (non-breaking change which adds functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • Enhancement (improves an existing feature and functionality)
  • Cleanup (Code refactoring and cleanup, that may add test cases)
  • Build/CI
  • Test (unit or integration test code)

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

  • Major
  • Minor

Bug Severity

  • BLOCKER
  • Critical
  • Major
  • Minor
  • Trivial

Screenshots (if appropriate):

Not applicable.

How Has This Been Tested?

Regression coverage exercises the request transport and each changed diagnostic boundary:

  • the UI component test verifies one POST to /, 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;
  • browser logging tests pass synthetic access, secret and session keys through POST and GET Axios error objects and verify that only safe scalar diagnostic metadata reaches console.error;
  • servlet tests verify parameter suppression for object-storage requests sent as POST or legacy query strings, user-data suppression, and ordinary-command controls;
  • the API command test verifies that the request is classified as containing sensitive information.

The patch passes git diff --check. It was tested locally with Temurin Java 17 and Node 16.20.2.

API module:
Tests run: 715, Failures: 0, Errors: 0, Skipped: 0
Checkstyle violations: 0

Server module:
Tests run: 2920, Failures: 0, Errors: 0, Skipped: 9
Checkstyle violations: 0

Focused ApiServletTest:
Tests run: 28, Failures: 0, Errors: 0, Skipped: 0

UI production build:
Build complete. The dist directory is ready to be deployed.

UI lint:
No lint errors found

Full UI unit suite:
Test Suites: 6 passed, 6 total
Tests: 182 passed, 182 total

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 no params object, 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.

@Dogface2k
Dogface2k marked this pull request as ready for review August 2, 2026 13:18
@Dogface2k
Dogface2k marked this pull request as draft August 2, 2026 13:53
@Dogface2k
Dogface2k marked this pull request as ready for review August 2, 2026 14:02
@Dogface2k
Dogface2k marked this pull request as draft August 2, 2026 15:36
@Dogface2k
Dogface2k marked this pull request as ready for review August 2, 2026 15:58

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 addObjectStoragePool via postAPI (URLSearchParams body) and mask the secret key field using a password input with autocomplete="off".
  • UI diagnostics: introduce centralized, safe Axios error logging that avoids serializing request/response bodies and query params.
  • Server/API: mark addObjectStoragePool as 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

codecov Bot commented Aug 3, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 84.61538% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 17.70%. Comparing base (5328528) to head (fe572b3).

Files with missing lines Patch % Lines
server/src/main/java/com/cloud/api/ApiServlet.java 84.61% 1 Missing and 1 partial ⚠️
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     
Flag Coverage Δ
uitests 3.77% <ø> (+0.07%) ⬆️
unittests 18.77% <84.61%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@DaanHoogland

Copy link
Copy Markdown
Contributor

@blueorangutan package

@blueorangutan

Copy link
Copy Markdown

@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.

Comment thread ui/src/utils/request.js
@blueorangutan

Copy link
Copy Markdown

Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 18742

@DaanHoogland

Copy link
Copy Markdown
Contributor

@blueorangutan test keepEnv

@blueorangutan

Copy link
Copy Markdown

@DaanHoogland a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests

@blueorangutan

Copy link
Copy Markdown

[SF] Trillian Build Failed (tid-16674)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants