rhtyd edited a comment on issue #3894: api: Fix count and item issues returned by list APIs URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-590756281 Thanks @weizhouapache I went through your test results, I've gathered a list of APIs that are only supported/affected by this PR for rest of the `list` APIs including `listNetworks` I've logged https://github.com/apache/cloudstack/issues/3897 to be done in future. Here are my comments: - The magic feature of `projectid=-1` (I think documented in one of the old docs) is to allow non-normal-accounts to be able to see all project resources (excluding non-project resources). Domain admins shouldn't be allowed to see all resources including project resources when listall=true and projectid=-1. - A `listall=false`, when combined with `projectid=-1`, will still return all the project-specific resources (for backward compatibility). - The `listNetworks` is not a supported API yet, see #3897 which is why you're not seeing the new behaviour, the PR initially only intended to fix it for `listRouters` for Primate to render pagination/list properly. If this is necessary for the milestone, I can raise another PR. - *BIG ONE* - For the `listRouters` as domain admin called with projectid=-1, I tested both this PR and 4.13.0.0 and indeed I'm able to list routers of other networks. I found this to be true for many other APIs (from the list below). This **should not be allowed** for domain admins, based on quick investigation I think this happens because when projectid=-1 is passed it allows listing of all project resources without enforcing the permittedAccounts check? cc @DaanHoogland @PaulAngus @andrijapanicsb - if we agree I can enforce a check that only the root admin can see all other project resources. - I do see and verified the old behaviour to list shared networks for other domains/projects/accounts, no change in behaviour seen. Based on static code analysis and usages, only the following APIs are affected by the listall=true&projectid=-1 change and only for the root admin account type: - [x] listUsers - [x] listTags - [x] listEvents - [x] listInstanceGroups - [x] listVirtualMachines - [x] listVolumes - [x] listSecurityGroups - [x] listRouters - [x] listProjectInvitations - [x] listAsyncJobs - [x] listTemplates - [x] listIsos - [ ] listAffinityGroups - [ ] listFirewallRules - [ ] listEgressFirewallRules - [ ] listLoadBalancerRules - [ ] listPortForwardingRules - [ ] listNetworkACLLists - [ ] listNetworkACLs - [ ] listVpcs - [ ] listPrivateGateways - [ ] listStaticRoutes - [ ] listVpnUsers - [ ] listRemoteAccessVpns - [ ] listVpnCustomerGateways - [ ] listVpnGateways - [ ] listVpnConnections - [x] listPublicIpAddresses - [ ] listSSHKeyPairs - [ ] listSnapshots - [ ] listVMSnapshot - [ ] listLoadBalancers
---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: [email protected] With regards, Apache Git Services
