Revert "Handle control plane check for federated apis"

[nisan-abeywickrama]

Reverts #598

Summary by CodeRabbit

• Bug Fixes
  • Access control checks are now consistently applied across all API requests, improving authorization enforcement uniformity.

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

[coderabbitai]

Walkthrough

The PR removes federation-based exemptions from control plane authorization checks in the API content controller. Access control enforcement now consistently applies when the control plane is enabled, eliminating special bypass logic previously granted to federated APIs.

Changes

Cohort / File(s)	Summary
Authorization logic simplification src/controllers/apiContentController.js	Removed isFederatedAPI condition from access authorization path in loadAPIContent and loadDocument functions. Eliminated gatewayVendor and isFederatedAPI variable resolution. Authorization now unconditionally enforces control plane checks. Minor syntax adjustment: allowedAPIList.count == 0 instead of === 0.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

• Authorization logic changes: Verify that removing federated API exemptions doesn't unintentionally break legitimate federated API workflows or bypass intended security controls
• Control plane dependency: Confirm the unconditional reliance on config.controlPlane?.enabled !== false is the correct behavior and handles all edge cases
• Broader impact assessment: Consider whether other parts of the codebase depend on the previous federation-based gatekeeping behavior

Poem

🐰 No more gates for the federated few,
All APIs now must pass on through,
Control plane checks, fair and true,
Authorization, clean and new! ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The description is minimal and lacks required template sections including Purpose, Goals, Approach, documentation, testing details, and security checks.	Complete the PR description using the repository template, including Purpose (why this revert), Goals, Approach, and relevant sections like Automation tests and Security checks.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly describes the main change: reverting a previous commit that handled control plane checks for federated APIs.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch revert-598-main

---

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

🧹 Nitpick comments (1)

src/controllers/apiContentController.js (1)

572-576: Use strict equality for consistency.

Line 572 uses loose equality (==) while line 576 uses strict equality (===) for the same type of check on allowedAPIList.count. This inconsistency could lead to unexpected type coercion behavior.

🔎 Proposed fix for consistency

-                if (allowedAPIList.count == 0) {
+                if (allowedAPIList.count === 0) {
                     apiName = apiMetadata.apiInfo.apiName;
                     allowedAPIList = await util.invokeApiRequest(req, 'GET', `${controlPlaneUrl}/apis?query=name:"${apiName}"+version:${version}`, {}, {});
                 }
                 if (allowedAPIList.count === 0) {

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 98aa7c9 and 959cab8.

📒 Files selected for processing (1)

• src/controllers/apiContentController.js

🧰 Additional context used

🧬 Code graph analysis (1)

src/controllers/apiContentController.js (15)

src/app.js (1)

• config (38-38)

src/utils/util.js (1)

• config (31-31)

src/services/apiMetadataService.js (1)

• config (25-25)

src/services/adminService.js (1)

• config (32-32)

src/controllers/authController.js (1)

• config (23-23)

src/controllers/applicationsContentController.js (1)

• config (20-20)

src/controllers/orgContentController.js (1)

• config (26-26)

src/controllers/devportalController.js (1)

• config (20-20)

src/controllers/customContentController.js (1)

• config (20-20)

src/controllers/settingsController.js (1)

• config (26-26)

src/middlewares/ensureAuthenticated.js (1)

• config (21-21)

src/middlewares/registerPartials.js (1)

• config (22-22)

src/services/redisConnectionHelper.js (1)

• config (23-23)

src/db/sequelize.js (1)

• config (23-23)

src/middlewares/passport.js (1)

• config (25-25)

🔇 Additional comments (2)

src/controllers/apiContentController.js (2)

567-567: Verify authorization changes for document access.

Similar to the change in loadAPIContent, the control plane authorization check now runs unconditionally for document access. Ensure this doesn't inadvertently block legitimate access to federated API documentation.

---

232-237: Remove federation exemption from control plane authorization; verify federated API access workflows still function.

The revert removes the federation-based exemption from the control plane authorization check. Line 237 now unconditionally enforces authorization when the control plane is enabled, regardless of whether the API is federated. While this improves security consistency, verify that:

1. Federated APIs can still be properly authorized through the control plane
2. This change aligns with the intended access control model

Minor issues:

• Operator inconsistency: Line 572 uses loose equality (==) while line 576 uses strict equality (===) for the same count comparison. Standardize to ===.
• Dead code: gatewayVendor (line 232) is computed but only serves to derive isFederatedAPI for template rendering. The variable itself is not used directly; consider simplifying if template needs only the boolean result.