Skip to content

Modify State Changing Requests to Utilize POST Request #107

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Jun 19, 2025

Conversation

kevin-lii
Copy link

Currently State Changing Request utilize GET request but this is a potential security vulnerability since attackers may be able to intercept and extract the query parameters of GET requests. Utilizing POST requests will help mitigate this security vulnerability since the parameters of the POST body will be encrypted in transport.

Copy link
Contributor

@sureshanaparti sureshanaparti left a comment

Choose a reason for hiding this comment

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

code lgtm

@DaanHoogland
Copy link
Contributor

@kevin-lii , can you rebase on latest to have the post requests tested against a simulator running in a container, please? (see #111 )

Copy link

@kiranchavala kiranchavala left a comment

Choose a reason for hiding this comment

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

LGTM

Steps performed to check

  1. Enabled the global setting "enforce.post.requests.and.timestamps” to true

  2. Execute the

go test ci/ci_test.go -v

 go test ci/ci_test.go -v
=== RUN   TestCloudstackAPI
    ci_test.go:91: Using zone ID: 932cfb3e-be7e-4d6f-8730-3b0f520c58bf
    ci_test.go:91: Using service offering IDs: c4557d27-24c0-4631-aeb1-b62226562ae0, 468c8d4c-38e2-4a0a-b262-862412ed90b7
    ci_test.go:91: Using network offering ID: eac1aff2-fb81-4050-af1a-6ed8c7d41fa9
    ci_test.go:91: Created test network with ID: a7ee1fc1-a293-491e-9ff7-8243f0e5cce6
    ci_test.go:91: Using template ID: 77b89c09-17bd-4229-8d55-a48c26f3fe0f
    ci_test.go:91: Created test VM ci-test-vm-1 with ID: e1056c91-2f90-4c8c-a702-92fdd7123378
    ci_test.go:91: Created test VM ci-test-vm-2 with ID: c1dd264a-bf27-4617-892f-4cab0fc87d10
=== RUN   TestCloudstackAPI/BasicTypes
=== RUN   TestCloudstackAPI/BasicTypes/StringParameters
    ci_test.go:144: ListZones with keyword='zone' successful, found 0 zones
=== RUN   TestCloudstackAPI/BasicTypes/BooleanParameters
    ci_test.go:169: ListVirtualMachines with listall=true successful, found 2 VMs
    ci_test.go:182: ListVirtualMachines with listall=false successful, found 2 VMs
=== RUN   TestCloudstackAPI/BasicTypes/IntegerParameters
    ci_test.go:205: ListVirtualMachines with pagesize=5, page=1 successful, found 2 VMs
=== RUN   TestCloudstackAPI/BasicTypes/LongParameters
    ci_test.go:228: ChangeServiceForVirtualMachine with maxiops=1000 successful, job ID:
=== RUN   TestCloudstackAPI/BasicTypes/DateParameters
    ci_test.go:259: ListUsageRecords with startdate='2024-01-01' and enddate='2024-01-31' successful, found 0 usage records
=== RUN   TestCloudstackAPI/BasicTypes/ListParameters
    ci_test.go:279: ListVirtualMachines with ids=[e1056c91-2f90-4c8c-a702-92fdd7123378 c1dd264a-bf27-4617-892f-4cab0fc87d10] successful, found 2 VMs
=== RUN   TestCloudstackAPI/BasicTypes/MapParameters
    ci_test.go:317: AddNicToVirtualMachine with dhcpoptions=map[option1:value1 option2:value2] successful, job ID: 14866cf9-46eb-481d-9adb-e8e93e812f14
=== RUN   TestCloudstackAPI/BasicTypes/UUIDParameters
    ci_test.go:337: ListServiceOfferings with zoneid=932cfb3e-be7e-4d6f-8730-3b0f520c58bf successful, found 2 service offerings
    ci_test.go:344: Service offering ID c4557d27-24c0-4631-aeb1-b62226562ae0 is valid UUID format
=== RUN   TestCloudstackAPI/BasicTypes/ShortParameters
    ci_test.go:377: RemoveRawUsageRecords with interval=365 successful:
=== NAME  TestCloudstackAPI
    ci_test.go:100: Successfully destroyed test VM e1056c91-2f90-4c8c-a702-92fdd7123378
    ci_test.go:100: Successfully destroyed test VM c1dd264a-bf27-4617-892f-4cab0fc87d10
    ci_test.go:100: Successfully deleted test network a7ee1fc1-a293-491e-9ff7-8243f0e5cce6
--- PASS: TestCloudstackAPI (16.37s)
    --- PASS: TestCloudstackAPI/BasicTypes (2.27s)
        --- PASS: TestCloudstackAPI/BasicTypes/StringParameters (0.21s)
        --- PASS: TestCloudstackAPI/BasicTypes/BooleanParameters (0.47s)
        --- PASS: TestCloudstackAPI/BasicTypes/IntegerParameters (0.23s)
        --- PASS: TestCloudstackAPI/BasicTypes/LongParameters (0.27s)
        --- PASS: TestCloudstackAPI/BasicTypes/DateParameters (0.22s)
        --- PASS: TestCloudstackAPI/BasicTypes/ListParameters (0.23s)
        --- PASS: TestCloudstackAPI/BasicTypes/MapParameters (0.23s)
        --- PASS: TestCloudstackAPI/BasicTypes/UUIDParameters (0.22s)
        --- PASS: TestCloudstackAPI/BasicTypes/ShortParameters (0.21s)
PASS

Copy link
Member

@vishesh92 vishesh92 left a comment

Choose a reason for hiding this comment

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

clgtm. didn't test.

@sureshanaparti sureshanaparti merged commit 0bc11ec into apache:main Jun 19, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants