You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by "kevin-lii (via GitHub)" <gi...@apache.org> on 2025/04/15 22:16:27 UTC
[PR] Modify State Changing Requests to Utilize POST Request [cloudstack-go]
kevin-lii opened a new pull request, #107:
URL: https://github.com/apache/cloudstack-go/pull/107
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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Modify State Changing Requests to Utilize POST Request [cloudstack-go]
Posted by "sureshanaparti (via GitHub)" <gi...@apache.org>.
sureshanaparti commented on code in PR #107:
URL: https://github.com/apache/cloudstack-go/pull/107#discussion_r2149668170
##########
cloudstack/UserService.go:
##########
@@ -3133,7 +3133,7 @@ func (s *UserService) NewVerifyOAuthCodeAndGetUserParams(provider string) *Verif
// Verify the OAuth Code and fetch the corresponding user from provider
func (s *UserService) VerifyOAuthCodeAndGetUser(p *VerifyOAuthCodeAndGetUserParams) (*VerifyOAuthCodeAndGetUserResponse, error) {
- resp, err := s.cs.newPostRequest("verifyOAuthCodeAndGetUser", p.toURLValues())
+ resp, err := s.cs.newRequest("verifyOAuthCodeAndGetUser", p.toURLValues())
Review Comment:
yes, this is changed with generate. it's a GET call, valid one @weizhouapache
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Modify State Changing Requests to Utilize POST Request [cloudstack-go]
Posted by "kevin-lii (via GitHub)" <gi...@apache.org>.
kevin-lii commented on code in PR #107:
URL: https://github.com/apache/cloudstack-go/pull/107#discussion_r2047779261
##########
generate/generate.go:
##########
@@ -560,7 +560,7 @@ func (as *allServices) GeneralCode() ([]byte, error) {
pn("// Execute the request against a CS API. Will return the raw JSON data returned by the API and nil if")
pn("// no error occurred. If the API returns an error the result will be nil and the HTTP error code and CS")
pn("// error details. If a processing (code) error occurs the result will be nil and the generated error")
- pn("func (cs *CloudStackClient) newRequest(api string, params url.Values) (json.RawMessage, error) {")
+ pn("func (cs *CloudStackClient) newPostRequest(api string, params url.Values) (json.RawMessage, error) {")
Review Comment:
Sorry I think I accidentally replaced that. I will fix that.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Modify State Changing Requests to Utilize POST Request [cloudstack-go]
Posted by "sureshanaparti (via GitHub)" <gi...@apache.org>.
sureshanaparti commented on code in PR #107:
URL: https://github.com/apache/cloudstack-go/pull/107#discussion_r2149679383
##########
generate/generate.go:
##########
@@ -1752,17 +1755,22 @@ func (s *service) generateNewAPICallFunc(a *API) {
pn("")
pn(" // We should be able to retry on failure as this call is idempotent")
pn(" for i := 0; i < 3; i++ {")
- pn(" resp, err = s.cs.newPostRequest(\"%s\", p.toURLValues())", a.Name)
+ pn(" resp, err = s.cs.newRequest(\"%s\", p.toURLValues())", a.Name)
pn(" if err == nil {")
pn(" break")
pn(" }")
pn(" time.Sleep(500 * time.Millisecond)")
pn(" }")
} else {
- if requiresPostMethod[a.Name] {
+ isGetRequest, _ := regexp.MatchString("^(get|list|query|find)(\\w+)+$", strings.ToLower(a.Name))
+ getRequestList := map[string]struct{}{"isaccountallowedtocreateofferingswithtags": {}, "readyforshutdown": {}, "cloudianisenabled": {}, "quotabalance": {},
+ "quotasummary": {}, "quotatarifflist": {}, "quotaisenabled": {}, "quotastatement": {}, "verifyoauthcodeandgetuser": {}}
+ _, isInGetRequestList := getRequestList[strings.ToLower(a.Name)]
+
+ if requiresPostMethod[a.Name] || !(isGetRequest || isInGetRequestList) {
pn(" resp, err := s.cs.newPostRequest(\"%s\", p.toURLValues())", a.Name)
} else {
- pn(" resp, err := s.cs.newPostRequest(\"%s\", p.toURLValues())", a.Name)
+ pn(" resp, err := s.cs.newRequest(\"%s\", p.toURLValues())", a.Name)
Review Comment:
newRequest, which is GET call
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Modify State Changing Requests to Utilize POST Request [cloudstack-go]
Posted by "sureshanaparti (via GitHub)" <gi...@apache.org>.
sureshanaparti commented on code in PR #107:
URL: https://github.com/apache/cloudstack-go/pull/107#discussion_r2056084151
##########
cloudstack/ManagementService.go:
##########
@@ -804,7 +804,7 @@ func (s *ManagementService) NewReadyForShutdownParams() *ReadyForShutdownParams
// Returns the status of CloudStack, whether a shutdown has been triggered and if ready to shutdown
func (s *ManagementService) ReadyForShutdown(p *ReadyForShutdownParams) (*ReadyForShutdownResponse, error) {
- resp, err := s.cs.newRequest("readyForShutdown", p.toURLValues())
+ resp, err := s.cs.newPostRequest("readyForShutdown", p.toURLValues())
Review Comment:
```suggestion
resp, err := s.cs.newRequest("readyForShutdown", p.toURLValues())
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Modify State Changing Requests to Utilize POST Request [cloudstack-go]
Posted by "sureshanaparti (via GitHub)" <gi...@apache.org>.
sureshanaparti commented on code in PR #107:
URL: https://github.com/apache/cloudstack-go/pull/107#discussion_r2056117130
##########
cloudstack/QuotaService.go:
##########
@@ -192,7 +192,7 @@ func (s *QuotaService) NewQuotaBalanceParams(account string, domainid string) *Q
// Create a quota balance statement
func (s *QuotaService) QuotaBalance(p *QuotaBalanceParams) (*QuotaBalanceResponse, error) {
- resp, err := s.cs.newRequest("quotaBalance", p.toURLValues())
+ resp, err := s.cs.newPostRequest("quotaBalance", p.toURLValues())
Review Comment:
```suggestion
resp, err := s.cs.newRequest("quotaBalance", p.toURLValues())
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Modify State Changing Requests to Utilize POST Request [cloudstack-go]
Posted by "sureshanaparti (via GitHub)" <gi...@apache.org>.
sureshanaparti commented on code in PR #107:
URL: https://github.com/apache/cloudstack-go/pull/107#discussion_r2056077237
##########
cloudstack/CloudianService.go:
##########
@@ -51,7 +51,7 @@ func (s *CloudianService) NewCloudianIsEnabledParams() *CloudianIsEnabledParams
// Checks if the Cloudian Connector is enabled
func (s *CloudianService) CloudianIsEnabled(p *CloudianIsEnabledParams) (*CloudianIsEnabledResponse, error) {
- resp, err := s.cs.newRequest("cloudianIsEnabled", p.toURLValues())
+ resp, err := s.cs.newPostRequest("cloudianIsEnabled", p.toURLValues())
Review Comment:
```suggestion
resp, err := s.cs.newRequest("cloudianIsEnabled", p.toURLValues())
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Modify State Changing Requests to Utilize POST Request [cloudstack-go]
Posted by "sureshanaparti (via GitHub)" <gi...@apache.org>.
sureshanaparti commented on code in PR #107:
URL: https://github.com/apache/cloudstack-go/pull/107#discussion_r2056111744
##########
cloudstack/QuotaService.go:
##########
@@ -408,7 +408,7 @@ func (s *QuotaService) NewQuotaIsEnabledParams() *QuotaIsEnabledParams {
// Return true if the plugin is enabled
func (s *QuotaService) QuotaIsEnabled(p *QuotaIsEnabledParams) (*QuotaIsEnabledResponse, error) {
- resp, err := s.cs.newRequest("quotaIsEnabled", p.toURLValues())
+ resp, err := s.cs.newPostRequest("quotaIsEnabled", p.toURLValues())
Review Comment:
```suggestion
resp, err := s.cs.newRequest("quotaIsEnabled", p.toURLValues())
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Modify State Changing Requests to Utilize POST Request [cloudstack-go]
Posted by "sureshanaparti (via GitHub)" <gi...@apache.org>.
sureshanaparti commented on code in PR #107:
URL: https://github.com/apache/cloudstack-go/pull/107#discussion_r2056112815
##########
cloudstack/QuotaService.go:
##########
@@ -598,7 +598,7 @@ func (s *QuotaService) NewQuotaStatementParams(account string, domainid string,
// Create a quota statement
func (s *QuotaService) QuotaStatement(p *QuotaStatementParams) (*QuotaStatementResponse, error) {
- resp, err := s.cs.newRequest("quotaStatement", p.toURLValues())
+ resp, err := s.cs.newPostRequest("quotaStatement", p.toURLValues())
Review Comment:
```suggestion
resp, err := s.cs.newRequest("quotaStatement", p.toURLValues())
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Modify State Changing Requests to Utilize POST Request [cloudstack-go]
Posted by "sureshanaparti (via GitHub)" <gi...@apache.org>.
sureshanaparti commented on code in PR #107:
URL: https://github.com/apache/cloudstack-go/pull/107#discussion_r2056114303
##########
cloudstack/QuotaService.go:
##########
@@ -801,7 +801,7 @@ func (s *QuotaService) NewQuotaSummaryParams() *QuotaSummaryParams {
// Lists balance and quota usage for all accounts
func (s *QuotaService) QuotaSummary(p *QuotaSummaryParams) (*QuotaSummaryResponse, error) {
- resp, err := s.cs.newRequest("quotaSummary", p.toURLValues())
+ resp, err := s.cs.newPostRequest("quotaSummary", p.toURLValues())
Review Comment:
```suggestion
resp, err := s.cs.newRequest("quotaSummary", p.toURLValues())
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Modify State Changing Requests to Utilize POST Request [cloudstack-go]
Posted by "weizhouapache (via GitHub)" <gi...@apache.org>.
weizhouapache commented on code in PR #107:
URL: https://github.com/apache/cloudstack-go/pull/107#discussion_r2149635362
##########
cloudstack/UserService.go:
##########
@@ -3133,7 +3133,7 @@ func (s *UserService) NewVerifyOAuthCodeAndGetUserParams(provider string) *Verif
// Verify the OAuth Code and fetch the corresponding user from provider
func (s *UserService) VerifyOAuthCodeAndGetUser(p *VerifyOAuthCodeAndGetUserParams) (*VerifyOAuthCodeAndGetUserResponse, error) {
- resp, err := s.cs.newPostRequest("verifyOAuthCodeAndGetUser", p.toURLValues())
+ resp, err := s.cs.newRequest("verifyOAuthCodeAndGetUser", p.toURLValues())
Review Comment:
should this be changed ?
@sureshanaparti
##########
generate/generate.go:
##########
@@ -1752,17 +1755,22 @@ func (s *service) generateNewAPICallFunc(a *API) {
pn("")
pn(" // We should be able to retry on failure as this call is idempotent")
pn(" for i := 0; i < 3; i++ {")
- pn(" resp, err = s.cs.newPostRequest(\"%s\", p.toURLValues())", a.Name)
+ pn(" resp, err = s.cs.newRequest(\"%s\", p.toURLValues())", a.Name)
pn(" if err == nil {")
pn(" break")
pn(" }")
pn(" time.Sleep(500 * time.Millisecond)")
pn(" }")
} else {
- if requiresPostMethod[a.Name] {
+ isGetRequest, _ := regexp.MatchString("^(get|list|query|find)(\\w+)+$", strings.ToLower(a.Name))
+ getRequestList := map[string]struct{}{"isaccountallowedtocreateofferingswithtags": {}, "readyforshutdown": {}, "cloudianisenabled": {}, "quotabalance": {},
+ "quotasummary": {}, "quotatarifflist": {}, "quotaisenabled": {}, "quotastatement": {}, "verifyoauthcodeandgetuser": {}}
+ _, isInGetRequestList := getRequestList[strings.ToLower(a.Name)]
+
+ if requiresPostMethod[a.Name] || !(isGetRequest || isInGetRequestList) {
pn(" resp, err := s.cs.newPostRequest(\"%s\", p.toURLValues())", a.Name)
} else {
- pn(" resp, err := s.cs.newPostRequest(\"%s\", p.toURLValues())", a.Name)
+ pn(" resp, err := s.cs.newRequest(\"%s\", p.toURLValues())", a.Name)
Review Comment:
newPostRequest or newRequest ?
@sureshanaparti
##########
generate/requiredParams.go:
##########
@@ -49,7 +49,11 @@ var requiredParams = map[string][]string{
"displaytext",
},
"createVPC": {
+ "name",
"displaytext",
+ "vpcofferingid",
+ "zoneid",
+ "cidr",
Review Comment:
actually only need to add `cidr`
but no issues if add others (`name`, `zoneid`, `vpcofferingid`)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Modify State Changing Requests to Utilize POST Request [cloudstack-go]
Posted by "sureshanaparti (via GitHub)" <gi...@apache.org>.
sureshanaparti commented on code in PR #107:
URL: https://github.com/apache/cloudstack-go/pull/107#discussion_r2056053999
##########
cloudstack/AccountService.go:
##########
@@ -1104,7 +1104,7 @@ func (s *AccountService) NewIsAccountAllowedToCreateOfferingsWithTagsParams() *I
// Return true if the specified account is allowed to create offerings with tags.
func (s *AccountService) IsAccountAllowedToCreateOfferingsWithTags(p *IsAccountAllowedToCreateOfferingsWithTagsParams) (*IsAccountAllowedToCreateOfferingsWithTagsResponse, error) {
- resp, err := s.cs.newRequest("isAccountAllowedToCreateOfferingsWithTags", p.toURLValues())
+ resp, err := s.cs.newPostRequest("isAccountAllowedToCreateOfferingsWithTags", p.toURLValues())
Review Comment:
```suggestion
resp, err := s.cs.newRequest("isAccountAllowedToCreateOfferingsWithTags", p.toURLValues())
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Modify State Changing Requests to Utilize POST Request [cloudstack-go]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #107:
URL: https://github.com/apache/cloudstack-go/pull/107#issuecomment-2912048663
@kevin-lii , can you rebase on latest to have the post requests tested against a simulator running in a container, please? (see #111 )
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Modify State Changing Requests to Utilize POST Request [cloudstack-go]
Posted by "sureshanaparti (via GitHub)" <gi...@apache.org>.
sureshanaparti merged PR #107:
URL: https://github.com/apache/cloudstack-go/pull/107
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Modify State Changing Requests to Utilize POST Request [cloudstack-go]
Posted by "rohityadavcloud (via GitHub)" <gi...@apache.org>.
rohityadavcloud commented on code in PR #107:
URL: https://github.com/apache/cloudstack-go/pull/107#discussion_r2046235561
##########
generate/generate.go:
##########
@@ -560,7 +560,7 @@ func (as *allServices) GeneralCode() ([]byte, error) {
pn("// Execute the request against a CS API. Will return the raw JSON data returned by the API and nil if")
pn("// no error occurred. If the API returns an error the result will be nil and the HTTP error code and CS")
pn("// error details. If a processing (code) error occurs the result will be nil and the generated error")
- pn("func (cs *CloudStackClient) newRequest(api string, params url.Values) (json.RawMessage, error) {")
+ pn("func (cs *CloudStackClient) newPostRequest(api string, params url.Values) (json.RawMessage, error) {")
Review Comment:
@kevin-lii should all API calls be POST by default? Or, we should compare if it's a list/get API ?
cc @Pearl1594 @weizhouapache @sureshanaparti @vishesh92
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Modify State Changing Requests to Utilize POST Request [cloudstack-go]
Posted by "sureshanaparti (via GitHub)" <gi...@apache.org>.
sureshanaparti commented on code in PR #107:
URL: https://github.com/apache/cloudstack-go/pull/107#discussion_r2056115362
##########
cloudstack/QuotaService.go:
##########
@@ -1449,7 +1449,7 @@ func (s *QuotaService) NewQuotaTariffListParams() *QuotaTariffListParams {
// Lists all quota tariff plans
func (s *QuotaService) QuotaTariffList(p *QuotaTariffListParams) (*QuotaTariffListResponse, error) {
- resp, err := s.cs.newRequest("quotaTariffList", p.toURLValues())
+ resp, err := s.cs.newPostRequest("quotaTariffList", p.toURLValues())
Review Comment:
```suggestion
resp, err := s.cs.newRequest("quotaTariffList", p.toURLValues())
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Modify State Changing Requests to Utilize POST Request [cloudstack-go]
Posted by "vishesh92 (via GitHub)" <gi...@apache.org>.
vishesh92 commented on PR #107:
URL: https://github.com/apache/cloudstack-go/pull/107#issuecomment-2912049804
@kevin-lii Can you reabase your PR against the latest main branch. I added a github action (https://github.com/apache/cloudstack-go/pull/111) which will help in testing this PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org