Fix checkRoleEscalation performance and bugs in access checking

[nvazquez]

Description

This PR fixes performance issues and bugs in AccountManagerImpl.checkRoleEscalation(Account, Account).

Performance problem: The method iterated all ~742 API commands individually, calling checkApiAccess for each command through 2 API checkers. DynamicRoleBasedAPIAccessChecker.checkAccess(Account, String) made a redundant uncached DB call on every invocation, ignoring already-cached data from
getRolePermissionsUsingCache(). Additionally, ProjectRoleBasedApiAccessChecker.checkAccess(Account, String) always returns true but was still called 742 times.

Impact before fix: ~1,484 DB calls per checkRoleEscalation invocation for non-admin roles.
Impact after fix: ~2 DB calls (or 0 with warm cache).

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):

How Has This Been Tested?

How did you try to break this feature and the system with this change?

[codecov]

Codecov Report

❌ Patch coverage is 56.94444% with 31 lines in your changes missing coverage. Please review.
✅ Project coverage is 17.70%. Comparing base (4708121) to head (81df63e).
⚠️ Report is 201 commits behind head on 4.22.

Files with missing lines	Patch %	Lines
...ain/java/org/apache/cloudstack/acl/APIChecker.java	6.66%	14 Missing ⚠️
...loudstack/acl/StaticRoleBasedAPIAccessChecker.java	0.00%	9 Missing ⚠️
...c/main/java/com/cloud/user/AccountManagerImpl.java	85.18%	4 Missing ⚠️
...oudstack/acl/ProjectRoleBasedApiAccessChecker.java	0.00%	3 Missing ⚠️
...oudstack/acl/DynamicRoleBasedAPIAccessChecker.java	94.44%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               4.22   #12973      +/-   ##
============================================
+ Coverage     17.60%   17.70%   +0.09%     
- Complexity    15677    15855     +178     
============================================
  Files          5918     5926       +8     
  Lines        531681   533592    +1911     
  Branches      65005    65283     +278     
============================================
+ Hits          93623    94490     +867     
- Misses       427498   428421     +923     
- Partials      10560    10681     +121     

Flag	Coverage Δ
uitests	3.69% <ø> (-0.02%)	⬇️
unittests	18.78% <56.94%> (+0.10%)	⬆️

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.

[nvazquez]

@blueorangutan package

[blueorangutan]

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

[blueorangutan]

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

[nvazquez]

@blueorangutan test

[blueorangutan]

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

[blueorangutan]

[SF] Trillian test result (tid-15819)
Environment: kvm-ol8 (x2), zone: Advanced Networking with Mgmt server ol8
Total time taken: 49520 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr12973-t15819-kvm-ol8.zip
Smoke tests completed. 149 look OK, 0 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test	Result	Time (s)	Test File

[DaanHoogland]

@blueorangutan package

[blueorangutan]

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

[blueorangutan]

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

[RosiKyu]

I partially tested this PR while testing PR #13044 (which moves checkRoleEscalation outside the DB transaction in createAccount). During that testing, I hit a case where a default Domain Admin successfully created a User-type account and assigned it the Root Admin role. The escalation went through and the account was persisted in the DB with role_id=1.

I reproduced the same case on a baseline build and got the exact same result, so the issue is pre-existing in checkRoleEscalation and not introduced by PR #13044. This PR addresses checkRoleEscalation, so I built a third environment with it and tested two scenarios:

Scenario	Without this PR (baseline)	With this PR
1. Default Domain Admin → User account with Root Admin role	Account is created with role_id=1 (escalation succeeds)	Blocked with HTTP 531 + "role null or unknown" - the error message could be clearer
2. Restricted Domain Admin -> default Domain Admin account (#5781)	Escalated APIs available to the created account	Blocked with HTTP 531 + clear "Escalated APIs: [createServiceOffering]" message

In both scenarios the request fails before anything is written to the database - the log shows the account only in memory (id:0), so no rows are inserted. That part works correctly.

Suggestions:

1. The error message in Scenario 1 could be clearer. "The account has role null or unknown" doesn't tell the user that this was an escalation attempt. Consider catching the null-role case in getApisAllowedToAccount and returning a clearer message like "cannot assign this role from your Domain Admin account context" - and if possible a 4xx HTTP code instead of 531.

2. Scenario 1's block currently depends on findRole hiding Admin roles from non-Admins. If that visibility ever changes (for example, to support a UI feature listing roles), the new size comparison would be the only thing left protecting against this case - and with the default Domain Admin role's broad ALLOW rules, it might not catch it. Worth considering whether validateRoleChange (which already exists and runs in updateAccount) should also run in the createAccount path so there's a clear role-type check before the permission scan.

---

Detailed test cases

Build under test: 4.22.1.0-SNAPSHOT with PR #12973 applied on Oracle Linux 9. Baseline reproductions confirmed on a mainline 4.22.1.0 build without this PR (escalation succeeds).

Setup on the PR build

• Test domain: test-pr12973 (id: 915effa9-4149-4afc-8987-dd37a858cf78)
• Default Domain Admin user: tpr12973-domadmin (account id: 7141f4b5-2781-4985-9207-01caef71b95c, user id: 3e4f5ef9-944b-46a9-aa5c-f3497c0d153b)
• Restricted custom Domain Admin role: tpr12973-da-restricted (id: 572a677a-64a3-492d-9ed9-263c8ead139a)
  • Permission 1 (sortorder 1): createServiceOffering DENY
  • Permission 2 (sortorder 2): * ALLOW
• Restricted user with the role above: tpr12973-da-restricted-user (account id: 59fb08dd-e09c-4603-bdb5-e2d034419b71, user id: 4c0585c9-3732-4c2b-8ec0-9e2ee6a11899)
• Default Root Admin role id: f9de0c41-4270-11f1-9812-1e001100045b
• Default Domain Admin role id: f9de56a1-4270-11f1-9812-1e001100045b

Scenario 1 - Default Domain Admin tries to create a User account with the Root Admin role

Run as tpr12973-domadmin (default Domain Admin role).

(tpr12973-domadmin) 🐱 > sync
Discovered 420 APIs

(tpr12973-domadmin) 🐱 > create account accounttype=0 username=tc12973-escalation password=Password123 firstname=Test lastname=Escalation email=tc12973-escalation@test.local domainid=915effa9-4149-4afc-8987-dd37a858cf78 roleid=f9de0c41-4270-11f1-9812-1e001100045b
🙈 Error: (HTTP 531, error code 4365) The account [Account [{"accountName":"tc12973-escalation","id":0,"uuid":"98338144-9c43-4940-bd99-dc64610955c6"}]] has role null or unknown.

mysql> SELECT id, uuid, account_name FROM account WHERE account_name='tc12973-escalation';
Empty set (0.00 sec)

mysql> SELECT id, uuid, username FROM user WHERE username='tc12973-escalation';
Empty set (0.00 sec)

**Management server log:**

2026-04-27 20:04:19,950 DEBUG [c.c.u.AccountManagerImpl] (qtp2038105753-18:[ctx-8d78c1a7, ctx-a39d7d92, ctx-dc8c5b13]) (logid:9a87f5b8) Verifying whether the caller has the correct privileges based on the user's role type and API permissions: Account [{"accountName":"tc12973-escalation","id":0,"uuid":"98338144-9c43-4940-bd99-dc64610955c6"}]
2026-04-27 20:04:19,951 DEBUG [c.c.u.AccountManagerImpl] (qtp2038105753-18:[ctx-8d78c1a7, ctx-a39d7d92, ctx-dc8c5b13]) (logid:9a87f5b8) Checking calling account [tpr12973-domadmin, 7141f4b5-2781-4985-9207-01caef71b95c] permission to perform this operation for user account [tc12973-escalation, 98338144-9c43-4940-bd99-dc64610955c6]
2026-04-27 20:04:19,951 DEBUG [c.c.u.AccountManagerImpl] (qtp2038105753-18:[ctx-8d78c1a7, ctx-a39d7d92, ctx-dc8c5b13]) (logid:9a87f5b8) Checking if user of account tpr12973-domadmin [7141f4b5-2781-4985-9207-01caef71b95c] with role-id [3] can create an account of type tc12973-escalation [98338144-9c43-4940-bd99-dc64610955c6] with role-id [1]
2026-04-27 20:04:19,969 INFO  [c.c.a.ApiServer] (qtp2038105753-18:[ctx-8d78c1a7, ctx-a39d7d92, ctx-dc8c5b13]) (logid:9a87f5b8) PermissionDenied: The account [Account [{"accountName":"tc12973-escalation","id":0,"uuid":"98338144-9c43-4940-bd99-dc64610955c6"}]] has role null or unknown. on objs: []

Result: The escalation is blocked. No rows in account or user tables.

But the error message is confusing. The user sees "The account has role null or unknown" with HTTP 531, which doesn't say anything about a privilege escalation. It sounds like a missing-role bug.

What's happening: When a Domain Admin asks about the Root Admin role, findRole returns null because Admin roles are hidden from non-Admin callers. The new code throws on that null and stops the operation. So the escalation IS blocked, but it's blocked because the Domain Admin can't even see the Root Admin role - not because the new permission check decided it was an escalation. The actual size comparison (the new logic this PR adds) never runs in this scenario.

On the baseline build, the same null-role exception happens, but the old code's try/catch... continue swallowed it for every API and let the loop finish, so the escalation went through. This PR removes that catch-and-continue, which is what closes the gap here - but as a side effect, not by design.

Scenario 2 - Restricted custom Domain Admin tries to create a default Domain Admin account (issue #5781)

Run as tpr12973-da-restricted-user. The role allows * and denies only createServiceOffering.

(tpr12973-da-restricted-user) 🐱 > sync
Discovered 878 APIs

(tpr12973-da-restricted-user) 🐱 > list serviceofferings
{
  "count": 2,
  "serviceoffering": [
    {
      "cpunumber": 1,
      "cpuspeed": 500,
      "created": "2026-04-27T19:41:43+0000",
      "defaultuse": false,
      "diskofferingdisplaytext": "Small Instance",
      "diskofferingid": "91c1d067-bbe1-457f-81dd-3fb8d66fb612",
      "diskofferingname": "Small Instance",
      "diskofferingstrictness": false,
      "displaytext": "Small Instance",
      "dynamicscalingenabled": true,
      "encryptroot": false,
      "hasannotations": false,
      "id": "81a91cf3-aa46-4612-9fbf-f8f2437e1d32",
      "iscustomized": false,
      "issystem": false,
      "isvolatile": false,
      "limitcpuuse": false,
      "memory": 512,
      "name": "Small Instance",
      "offerha": false,
      "provisioningtype": "thin",
      "rootdisksize": 0,
      "state": "Active",
      "storagetype": "shared"
    },
    {
      "cpunumber": 1,
      "cpuspeed": 1000,
      "created": "2026-04-27T19:41:43+0000",
      "defaultuse": false,
      "diskofferingdisplaytext": "Medium Instance",
      "diskofferingid": "d1557163-589a-47f5-981c-648861ded62c",
      "diskofferingname": "Medium Instance",
      "diskofferingstrictness": false,
      "displaytext": "Medium Instance",
      "dynamicscalingenabled": true,
      "encryptroot": false,
      "hasannotations": false,
      "id": "816dcd5c-f9a2-4657-a1e5-713a99750a09",
      "iscustomized": false,
      "issystem": false,
      "isvolatile": false,
      "limitcpuuse": false,
      "memory": 1024,
      "name": "Medium Instance",
      "offerha": false,
      "provisioningtype": "thin",
      "rootdisksize": 0,
      "state": "Active",
      "storagetype": "shared"
    }
  ]
}

(tpr12973-da-restricted-user) 🐱 > create serviceoffering name=tpr12973-test-offering displaytext=Test cpunumber=1 cpuspeed=1000 memory=512
🙈 Error: unknown command or API requested

(tpr12973-da-restricted-user) 🐱 > create account accounttype=2 username=tc12973-da-escalation password=Password123 firstname=Escaped lastname=DA email=tc12973-da-escalation@test.local domainid=915effa9-4149-4afc-8987-dd37a858cf78 roleid=f9de56a1-4270-11f1-9812-1e001100045b
🙈 Error: (HTTP 531, error code 4365) User of Account Account [{"accountName":"tpr12973-da-restricted-user","id":6,"uuid":"59fb08dd-e09c-4603-bdb5-e2d034419b71"}] and domain Domain {"id":2,"name":"test-pr12973","path":"\/test-pr12973\/","uuid":"915effa9-4149-4afc-8987-dd37a858cf78"} cannot create an account with access to more privileges than they have. Escalated APIs: [createServiceOffering]

mysql> SELECT id, uuid, account_name FROM account WHERE account_name='tc12973-da-escalation';
Empty set (0.00 sec)

mysql> SELECT id, uuid, username FROM user WHERE username='tc12973-da-escalation';
Empty set (0.01 sec)

**Management server log:**

2026-04-27 20:12:09,227 DEBUG [c.c.u.AccountManagerImpl] (qtp2038105753-19:[ctx-b111f5f0, ctx-992dd474, ctx-ee71b5ca]) (logid:3ffca8c0) Verifying whether the caller has the correct privileges based on the user's role type and API permissions: Account [{"accountName":"tc12973-da-escalation","id":0,"uuid":"3721d72d-0906-490d-9424-c1b4232a32a3"}]
2026-04-27 20:12:09,230 DEBUG [c.c.u.AccountManagerImpl] (qtp2038105753-19:[ctx-b111f5f0, ctx-992dd474, ctx-ee71b5ca]) (logid:3ffca8c0) Checking calling account [tpr12973-da-restricted-user, 59fb08dd-e09c-4603-bdb5-e2d034419b71] permission to perform this operation for user account [tc12973-da-escalation, 3721d72d-0906-490d-9424-c1b4232a32a3]
2026-04-27 20:12:09,230 DEBUG [c.c.u.AccountManagerImpl] (qtp2038105753-19:[ctx-b111f5f0, ctx-992dd474, ctx-ee71b5ca]) (logid:3ffca8c0) Checking if user of account tpr12973-da-restricted-user [59fb08dd-e09c-4603-bdb5-e2d034419b71] with role-id [10] can create an account of type tc12973-da-escalation [3721d72d-0906-490d-9424-c1b4232a32a3] with role-id [3]
2026-04-27 20:12:09,605 WARN  [c.c.u.AccountManagerImpl] (qtp2038105753-19:[ctx-b111f5f0, ctx-992dd474, ctx-ee71b5ca]) (logid:3ffca8c0) User of Account Account [{"accountName":"tpr12973-da-restricted-user","id":6,"uuid":"59fb08dd-e09c-4603-bdb5-e2d034419b71"}] and domain Domain {"id":2,"name":"test-pr12973","path":"\/test-pr12973\/","uuid":"915effa9-4149-4afc-8987-dd37a858cf78"} cannot create an account with access to more privileges than they have. Escalated APIs: [createServiceOffering]
2026-04-27 20:12:09,622 INFO  [c.c.a.ApiServer] (qtp2038105753-19:[ctx-b111f5f0, ctx-992dd474, ctx-ee71b5ca]) (logid:3ffca8c0) PermissionDenied: User of Account Account [{"accountName":"tpr12973-da-restricted-user","id":6,"uuid":"59fb08dd-e09c-4603-bdb5-e2d034419b71"}] and domain Domain {"id":2,"name":"test-pr12973","path":"\/test-pr12973\/","uuid":"915effa9-4149-4afc-8987-dd37a858cf78"} cannot create an account with access to more privileges than they have. Escalated APIs: [createServiceOffering] on objs: []

Result: Blocked, with a clear and helpful error message that names exactly which API was the problem (createServiceOffering).

The user sees 878 of 879 APIs (the one excluded by the deny rule), confirming the deny rule is enforced. DB confirms no rows persisted.

[nvazquez]

Thanks @RosiKyu - have fixed the scenario 1 as per your comments, scenario 2 seems to be working fine

@blueorangutan package

[blueorangutan]

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

[blueorangutan]

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

[nvazquez]

@blueorangutan test

[blueorangutan]

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

[blueorangutan]

[SF] Trillian test result (tid-16735)
Environment: kvm-ol8 (x2), zone: Advanced Networking with Mgmt server ol8
Total time taken: 54899 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr12973-t16735-kvm-ol8.zip
Smoke tests completed. 147 look OK, 2 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test	Result	Time (s)	Test File
test_01_deployVMInSharedNetwork	Failure	504.42	test_network.py
test_01_redundant_vpc_site2site_vpn	Failure	403.44	test_vpc_vpn.py

[RosiKyu]

@nvazquez, I have re-tested the PR on a 4.22 KVM environment, on a build that includes the latest fix commit, alongside a 4.22 build without the PR. Everything below is a measured before and after.

The performance fix works as expected.

• One createAccount by a Domain Admin, counted from the MySQL query log, on both environments with dynamic.apichecker.cache.period=0:

	without the PR	with the PR
database queries for role permissions	10939	5
total database queries	11421	212
time for the API call to return	10633 ms	~1300 ms

A single account creation took over 10 seconds on the build without the PR.

• Turning the cache on drops it further still. Three createAccount calls in a row with dynamic.apichecker.cache.period=300:

run 1 (first call after restart)   role permission queries = 2
run 2                              role permission queries = 0
run 3                              role permission queries = 0

The first call reads the permissions once, and later calls reuse them from memory until the cache period runs out.

Escalation is still correctly blocked, and the new Escalated APIs: [...] message is a real improvement because it names the exact API responsible:

Error: (HTTP 531, error code 4365) User of Account Account [{"accountName":"restricted-da-user",...}] and domain Domain {...} cannot create an account with access to more privileges than they have. Escalated APIs: [createServiceOffering]

The improved message for the missing role case works. A default Domain Admin creating a User account with the Root Admin role now gets:

Error: (HTTP 531, error code 4365) User of account: test-domadmin cannot assign this role on the requested account: test-account

instead of the previous has role null or unknown, with the original detail kept in the log. Much clearer.

No regressions found. All 17 account and user APIs affected by this change were exercised as a Domain Admin against an account it owns: create, update, enable, disable and delete account; create, update, enable, disable and delete user; register and get user keys; move user; update resource limit and count. All pass. Valid account creations are not wrongly refused, including a restricted role creating an account that carries that same role.

Two additional issues found

1. Revoking a permission does not take effect until the cache expires

Only happens when dynamic.apichecker.cache.period is greater than 0. The default is 0, so default installations are not affected.

After a permission is revoked from a role, account creation carries on using the remembered copy. A restricted Domain Admin can create an account that still has the revoked permission, for as long as the cache period lasts. The window is temporary but the account created in it is permanent, so this reopens #5781 for that period.

Steps to reproduce, with cache.period=300 and the management server restarted so nothing is remembered yet:

1. Grant createServiceOffering to the restricted user's role (a copy of Domain Admin with that one rule denied)
2. Check in the database that the rule reads ALLOW
3. As that user, create an account with the default Domain Admin role. This is allowed at this point, and it is the call that puts the permission into the cache
4. Revoke createServiceOffering from the restricted user's role
5. Check in the database that the rule now reads DENY
6. Immediately create another account with the default Domain Admin role

Step 6 should be refused with Escalated APIs: [createServiceOffering]. (It succeeds instead.)

Steps 3 to 6 have to be done within the cache period, counted from step 3, because step 3 is what puts the permission into memory. If that time runs out in between, the database is read again correctly and the problem does not show.

Observed, 49 seconds after step 3, with the database reading DENY:

account_name    role            created
test-acct-1     Domain Admin    09:37:27      <- step 3, allowed
test-acct-2     Domain Admin    09:38:16      <- step 6, should have been refused

restricted user's role permission in database: DENY
test-acct-2 has createServiceOffering=ALLOW:   yes

The same sequence on a build without the PR refuses it immediately, with the same cache setting:

cache.period = 1800

[10:46:56] grant createServiceOffering          database = ALLOW
[10:46:57] create #1 (puts permission in cache) -> CREATED
[10:47:01] revoke createServiceOffering         database = DENY
[10:47:01] create #2 immediately                -> REFUSED
[10:47:01] create #3 immediately                -> REFUSED

Setting cache.period back to 0 on the PR build also makes it refuse correctly every time.

This works in both directions. A newly granted permission also fails to take effect until the cache period runs out. That is not a security problem, but it will be reported as permissions not working.

Suggestion. Clear the remembered permissions for a role when they are changed, or read them from the database for this particular check.

2. A long list of escalated APIs makes the API return HTTP 500 with an empty response

This one does not need the cache. It happens on a default installation.

Every differing API name is put into the error message with no limit. Past a certain length the API stops returning the error at all.

Tested by calling createAccount directly and checking the response, with dynamic.apichecker.cache.period=0:

escalated APIs	HTTP status	readable error
326	531	yes
786	500	no

The identical call on a build without the PR returns a proper error:

	without the PR	with the PR
HTTP status	531	500
readable error	yes	no

Steps to reproduce, no special configuration needed:

1. Create a Domain Admin role with only a few rules allowed (listApis, listAccounts, listDomains, createAccount, createUser, listUsers, listConfigurations). An account on that role can see about 140 APIs.
2. Create an account with that role and generate API keys for it.
3. As that user, run createAccount with a much more privileged role. Using the default Domain Admin role gives 326 escalated APIs and returns a proper error. Using a role with a * ALLOW rule gives 786 and returns 500 with an empty response.

cmk shows the failing case as:

(tpr_minimal) 🐱 > create account accounttype=2 username=tc-chk-c password=Password123456 firstname=T lastname=T email=c@test.local domainid=2a43e16e-f8b6-4a93-ba87-74ed1a5ecd9f roleid=5c7e8978-d79c-43f8-b2a2-5201e7192d08
🙈 Error: failed to decode response

So the user gets no error text at all, and a 500 instead of the intended 531, which looks like the server broke rather than a permission being refused.

The escalation is still correctly refused and nothing is written to the database, so this is not a security issue.

This also hides the improved message from 81df63e whenever the list is long, so that improvement is only visible when the list is short.

Suggestion. Limit the list in the message, for example the first 10 followed by (+N more), keeping the full list in the log if it is needed.