IAM | Account Server - Block Account Deletion in Case the Account Has Users

[shirady]

Describe the Problem

Add a restriction for deleting an account that has users, so we will not have "dangling" users in the system.

Explain the Changes

1. Add additional condition to check if the account has users (based on the check that we do that an account does not have buckets and the filtering that we have in list_users).

Issues:

1. none

Testing Instructions:

1. Build the images and install NooBaa system on Rancher Desktop (see guide).
   Note: nb is an alias that runs the local operator from build/_output/bin (alias created by devenv).
2. Wait for the default backing store pod to be in state Ready before starting the tests: kubectl wait --for=condition=available backingstore/noobaa-default-backing-store --timeout=6m -n test1
3. I'm using port-forward (in a different tab):

• S3 kubectl port-forward -n test1 service/s3 12443:443
• IAM kubectl port-forward -n test1 service/iam 14443:443

4. Set the debug level to see the added log message: nb system set-debug-level 2 -n test1
5. Create the account: nb account create shira-acc01 -n test1 --show-secrets
6. Create the alias for the account:

• alias account-1-iam='AWS_ACCESS_KEY=<access-key> AWS_SECRET_ACCESS_KEY=<secret-key> aws --no-verify-ssl --endpoint-url https://localhost:14443'

7. Check the connection to the endpoint:

• try to list the users (should throw an error): account-1-iam iam list-users; echo $?

8. Create a user: account-1-iam iam create-user --user-name Robert
9. OPen the logs of the operator and core pods:

• kubectl logs noobaa-operator-6f7b579656-mz9wp -n test1 --tail 0 -f
• kubectl logs noobaa-core-0 -n test1 --tail 0 -f

10. Delete the account: nb account delete shira-acc01 -n test1 - see that it is not finished.
11. See in the logs:

operator logs:

time="2025-11-27T12:46:09Z" level=info msg="✈️  RPC: account.delete_account() Request: {Email:shira-acc01}"
time="2025-11-27T12:46:09Z" level=error msg="⚠️  RPC: account.delete_account() Response Error: Code=FORBIDDEN Message=Cannot delete account that is owner of users"
time="2025-11-27T12:46:09Z" level=info msg="SetPhase: temporary error during phase \"Deleting\"" noobaaaccount=test1/shira-acc01
time="2025-11-27T12:46:09Z" level=warning msg="⏳ Temporary Error: failed to delete account \"shira-acc01\". got error: Cannot delete account that is owner of users" noobaaaccount=test1/shira-acc01
time="2025-11-27T12:46:09Z" level=info msg="UpdateStatus: Done" noobaaaccount=test1/shira-acc0

core logs:

Nov-27 12:46:09.195 [WebServer/33] [ERROR] CONSOLE:: RPC._on_request: ERROR srv account_api.delete_account reqid wss://noobaa-mgmt.test1.svc.cluster.local:443/rpc/-795 connid ws://[::ffff:10.42.0.213]:46292/(8wpljk3) Error: Cannot delete account that is owner of users

Notes:

• In step 1 - deploying the system, I used --use-standalone-db for simplicity (fewer steps for the system in Ready status).

• Doc added/updated

• Tests added

Summary by CodeRabbit

• Bug Fixes
  • Prevents deletion of accounts that would orphan IAM users by blocking deletions when account ownership of users is detected.
  • Returns a forbidden response with a clear message when such a deletion is attempted.
  • Ensures existing protections for special/system accounts and owned resources remain enforced.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

Adds a deletion guard in _verify_can_delete_account: if the target account's owner is undefined, it scans accounts for IAM users owned by that account and forbids deletion if any are found, logging the detection and throwing FORBIDDEN.

Changes

Cohort / File(s)	Summary
Account deletion validation src/server/system_services/account_server.js	Adds an IAM-owned-user check in _verify_can_delete_account: when the target account's owner is undefined, scan accounts for IAM users with owner === target_account._id; log findings and throw FORBIDDEN ("Cannot delete account that is owner of IAM users") if any exist. This complements existing guards (support accounts, system owner, buckets owned).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20–30 minutes

• Inspect the account scan/filter logic for correctness and performance (potential full collection scan).
• Verify the owner === undefined condition is intended and covers edge cases.
• Confirm log message, thrown error type/message, and consistency with existing deletion guards.

Possibly related PRs

• IAM | Use the account server flow #9289 — Modifies the same account deletion/ownership logic in account_server.js; may overlap on IAM ownership checks and deletion flows.
• IAM | Authorize Requests for IAM Users According to IAM User Inline Policy #9281 — Adds account.owner / iam_user_policies handling used by ownership and deletion checks referenced here.
• IAM | Change bucket owner for IAM user to account and more #9268 — Changes IAM ownership representation and user/account delete flows related to the new deletion guard.

Suggested reviewers

• aayushchouhan09
• naveenpaul1
• liranmauda

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and specifically describes the main change: blocking account deletion when an account owns IAM users, which directly matches the code change that adds a deletion guard for accounts with IAM users.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 033d8ff and 658803b.

📒 Files selected for processing (1)

• src/server/system_services/account_server.js (1 hunks)

🧰 Additional context used

🧠 Learnings (3)

📚 Learning: 2025-11-19T15:03:42.260Z

Learnt from: shirady
Repo: noobaa/noobaa-core PR: 9291
File: src/server/common_services/auth_server.js:548-554
Timestamp: 2025-11-19T15:03:42.260Z
Learning: In src/server/common_services/auth_server.js, account objects are loaded directly from system_store (e.g., system_store.data.get_by_id()), so account.owner is an object ID reference with an ._id property, not a string. This differs from s3_rest.js where account.owner is a string due to RPC serialization.

Applied to files:

• src/server/system_services/account_server.js

📚 Learning: 2025-11-13T07:56:23.620Z

Learnt from: shirady
Repo: noobaa/noobaa-core PR: 9281
File: src/server/system_services/account_server.js:1053-1058
Timestamp: 2025-11-13T07:56:23.620Z
Learning: In noobaa-core, account_server.js is only used in containerized deployments, not in NSFS/NC deployments. NSFS/NC deployments have separate account management code in src/manage_nsfs/ directory. Therefore, account_server.js only processes accounts from account_schema.js where owner is an objectid reference, never from nsfs_account_schema.js where owner is a string.

Applied to files:

• src/server/system_services/account_server.js

📚 Learning: 2025-11-12T04:55:42.193Z

Learnt from: naveenpaul1
Repo: noobaa/noobaa-core PR: 9277
File: src/endpoint/s3/s3_rest.js:258-261
Timestamp: 2025-11-12T04:55:42.193Z
Learning: In the context of S3 REST requests (src/endpoint/s3/s3_rest.js), the account.owner field from req.object_sdk.requesting_account is already a string (account ID) because it comes from RPC serialization where owner._id.toString() is applied in account_server.js. No additional .toString() or ._id extraction is needed when passing account.owner to IAM utility functions.

Applied to files:

• src/server/system_services/account_server.js

🧬 Code graph analysis (1)

src/server/system_services/account_server.js (1)

src/endpoint/iam/iam_utils.js (2)

• owner_account_id (78-78)
• owner_account_id (851-851)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)

• GitHub Check: Build Noobaa Image
• GitHub Check: run-jest-unit-tests
• GitHub Check: run-package-lock-validation

🔇 Additional comments (1)

src/server/system_services/account_server.js (1)

1182-1192: LGTM! Deletion guard correctly prevents orphaning IAM users.

The implementation correctly identifies root accounts (via owner === undefined) and scans for owned IAM users before allowing deletion. The logic mirrors the bucket ownership check above and uses the appropriate utility function account_util.get_owner_account_id() for retrieving owner information.

Previous issues flagged in earlier reviews (log message context and comment accuracy) have been properly addressed.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1382037 and 1bee56f.

📒 Files selected for processing (1)

• src/server/system_services/account_server.js (1 hunks)

🧰 Additional context used

🧠 Learnings (3)

📚 Learning: 2025-11-19T15:03:42.260Z

Learnt from: shirady
Repo: noobaa/noobaa-core PR: 9291
File: src/server/common_services/auth_server.js:548-554
Timestamp: 2025-11-19T15:03:42.260Z
Learning: In src/server/common_services/auth_server.js, account objects are loaded directly from system_store (e.g., system_store.data.get_by_id()), so account.owner is an object ID reference with an ._id property, not a string. This differs from s3_rest.js where account.owner is a string due to RPC serialization.

Applied to files:

• src/server/system_services/account_server.js

📚 Learning: 2025-11-13T07:56:23.620Z

Learnt from: shirady
Repo: noobaa/noobaa-core PR: 9281
File: src/server/system_services/account_server.js:1053-1058
Timestamp: 2025-11-13T07:56:23.620Z
Learning: In noobaa-core, account_server.js is only used in containerized deployments, not in NSFS/NC deployments. NSFS/NC deployments have separate account management code in src/manage_nsfs/ directory. Therefore, account_server.js only processes accounts from account_schema.js where owner is an objectid reference, never from nsfs_account_schema.js where owner is a string.

Applied to files:

• src/server/system_services/account_server.js

📚 Learning: 2025-11-12T04:55:42.193Z

Learnt from: naveenpaul1
Repo: noobaa/noobaa-core PR: 9277
File: src/endpoint/s3/s3_rest.js:258-261
Timestamp: 2025-11-12T04:55:42.193Z
Learning: In the context of S3 REST requests (src/endpoint/s3/s3_rest.js), the account.owner field from req.object_sdk.requesting_account is already a string (account ID) because it comes from RPC serialization where owner._id.toString() is applied in account_server.js. No additional .toString() or ._id extraction is needed when passing account.owner to IAM utility functions.

Applied to files:

• src/server/system_services/account_server.js

🧬 Code graph analysis (1)

src/server/system_services/account_server.js (1)

src/util/account_util.js (9)

• _ (4-4)
• account (30-42)
• account (196-196)
• account (304-304)
• account (325-325)
• account (356-356)
• account (622-622)
• owner_account_id (341-341)
• owner_account_id (716-716)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)

• GitHub Check: Build Noobaa Image
• GitHub Check: run-package-lock-validation
• GitHub Check: run-jest-unit-tests