UI: submit object storage creation using POST

Author: Dogface2k

api/src/main/java/org/apache/cloudstack/api/command/admin/storage/AddObjectStoragePoolCmd.java

@@ -32,7 +32,7 @@
 import java.util.Map;
 
 @APICommand(name = "addObjectStoragePool", description = "Adds a object storage pool", responseObject = ObjectStoreResponse.class, since = "4.19.0",
-        requestHasSensitiveInfo = false, responseHasSensitiveInfo = false)
+        requestHasSensitiveInfo = true, responseHasSensitiveInfo = false)
 public class AddObjectStoragePoolCmd extends BaseCmd {
 
     /////////////////////////////////////////////////////

api/src/test/java/org/apache/cloudstack/api/command/admin/storage/AddObjectStoragePoolCmdTest.java

@@ -20,6 +20,7 @@
 
 import com.cloud.exception.DiscoveryException;
 import com.cloud.storage.StorageService;
+import org.apache.cloudstack.api.APICommand;
 import org.apache.cloudstack.api.ResponseGenerator;
 import org.apache.cloudstack.api.response.ObjectStoreResponse;
 import org.apache.cloudstack.context.CallContext;
@@ -38,6 +39,8 @@
 import java.util.HashMap;
 import java.util.Map;
 
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
 import static org.mockito.ArgumentMatchers.any;
 
 @RunWith(MockitoJUnitRunner.class)
@@ -98,4 +101,12 @@ public void testAddObjectStore() throws DiscoveryException {
         Mockito.verify(storageService, Mockito.times(1))
                 .discoverObjectStore(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any());
     }
+
+    @Test
+    public void testRequestIsMarkedAsContainingSensitiveInformation() {
+        APICommand apiCommand = AddObjectStoragePoolCmd.class.getAnnotation(APICommand.class);
+
+        assertNotNull(apiCommand);
+        assertTrue(apiCommand.requestHasSensitiveInfo());
+    }
 }

server/src/main/java/com/cloud/api/ApiServlet.java

@@ -100,6 +100,7 @@ public class ApiServlet extends HttpServlet {
             "updaterolepermission",
             "updateprojectrolepermission",
             "createstoragepool",
+            "addobjectstoragepool",
             "addhost",
             "updatehostpassword",
             "addcluster",
@@ -241,7 +242,7 @@ void processRequestInContext(final HttpServletRequest req, final HttpServletResp
         if (LOGGER.isDebugEnabled()) {
             reqStr = auditTrailSb.toString() + " " + cleanQueryString;
             if (req.getMethod().equalsIgnoreCase("POST") && org.apache.commons.lang3.StringUtils.isNotBlank(command)) {
-                if (!POST_REQUESTS_TO_DISABLE_LOGGING.contains(command.toLowerCase()) && !reqParams.containsKey(ApiConstants.USER_DATA)) {
+                if (shouldLogPostRequestParameters(command, reqParams)) {
                     String cleanParamsString = getCleanParamsString(reqParams);
                     if (org.apache.commons.lang3.StringUtils.isNotBlank(cleanParamsString)) {
                         reqStr += "\n" + cleanParamsString;
@@ -771,4 +772,8 @@ private String getCleanParamsString(Map<String, String[]> reqParams) {
 
         return cleanParamsString.toString();
     }
+
+    protected boolean shouldLogPostRequestParameters(String command, Map<String, String[]> reqParams) {
+        return !POST_REQUESTS_TO_DISABLE_LOGGING.contains(command.toLowerCase()) && !reqParams.containsKey(ApiConstants.USER_DATA);
+    }
 }

server/src/test/java/com/cloud/api/ApiServletTest.java

@@ -461,4 +461,28 @@ public void testVerify2FAWhenExpectedCommandIsNotCalled() throws UnknownHostExce
 
         Assert.assertEquals(false, result);
     }
+
+    @Test
+    public void shouldNotLogPostRequestParametersForAddObjectStoragePool() {
+        boolean result = servlet.shouldLogPostRequestParameters("addObjectStoragePool", new HashMap<>());
+
+        Assert.assertFalse(result);
+    }
+
+    @Test
+    public void shouldLogPostRequestParametersForCommandWithoutSensitiveParameters() {
+        boolean result = servlet.shouldLogPostRequestParameters("listZones", new HashMap<>());
+
+        Assert.assertTrue(result);
+    }
+
+    @Test
+    public void shouldNotLogPostRequestParametersContainingUserData() {
+        Map<String, String[]> params = new HashMap<>();
+        params.put(ApiConstants.USER_DATA, new String[] {"sensitive-user-data"});
+
+        boolean result = servlet.shouldLogPostRequestParameters("deployVirtualMachine", params);
+
+        Assert.assertFalse(result);
+    }
 }

ui/src/views/infra/AddObjectStorage.vue

@@ -106,7 +106,7 @@
 </template>
 <script>
 import { ref, reactive, toRaw } from 'vue'
-import { getAPI } from '@/api'
+import { postAPI } from '@/api'
 import { mixinForm } from '@/utils/mixin'
 import ResourceIcon from '@/components/view/ResourceIcon'
 import TooltipLabel from '@/components/widgets/TooltipLabel'
@@ -209,7 +209,7 @@ export default {
     },
     addObjectStore (params) {
       return new Promise((resolve, reject) => {
-        getAPI('addObjectStoragePool', params).then(json => {
+        postAPI('addObjectStoragePool', params).then(json => {
           resolve()
         }).catch(error => {
           reject(error)

ui/tests/unit/views/infra/AddObjectStorage.spec.js

@@ -0,0 +1,71 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+import mockAxios from '../../../mock/mockAxios'
+import AddObjectStorage from '@/views/infra/AddObjectStorage'
+
+jest.mock('axios', () => mockAxios)
+jest.mock('@/vue-app', () => ({
+  vueProps: {
+    $localStorage: {
+      get: jest.fn(() => null)
+    }
+  }
+}))
+
+describe('Views > infra > AddObjectStorage.vue', () => {
+  beforeEach(() => {
+    mockAxios.mockReset()
+  })
+
+  it('submits addObjectStoragePool using POST with the request in the body', async () => {
+    mockAxios.mockResolvedValue({})
+
+    const params = {
+      name: 'test-store',
+      provider: 'MinIO',
+      url: 'https://object-storage.example.test',
+      'details[0].key': 'accesskey',
+      'details[0].value': 'test-access-key',
+      'details[1].key': 'secretkey',
+      'details[1].value': 'test-secret-key'
+    }
+
+    await AddObjectStorage.methods.addObjectStore(params)
+
+    expect(mockAxios).toHaveBeenCalledTimes(1)
+    const request = mockAxios.mock.calls[0][0]
+    expect(request).toMatchObject({
+      url: '/',
+      method: 'POST'
+    })
+    expect(request.params).toBeUndefined()
+    expect(request.data).toBeInstanceOf(URLSearchParams)
+    expect(request.data.get('command')).toBe('addObjectStoragePool')
+    expect(request.data.get('response')).toBe('json')
+    expect(request.data.get('url')).toBe(params.url)
+    expect(request.data.get('details[0].value')).toBe(params['details[0].value'])
+    expect(request.data.get('details[1].value')).toBe(params['details[1].value'])
+  })
+
+  it('propagates an addObjectStoragePool API failure', async () => {
+    const error = new Error('request failed')
+    mockAxios.mockRejectedValue(error)
+
+    await expect(AddObjectStorage.methods.addObjectStore({})).rejects.toBe(error)
+  })
+})