Security Fix: Remove Company_id From Request Body

by Alex Johnson 50 views

In today's interconnected digital landscape, ensuring the security of applications and data is paramount. One critical aspect of application security is multi-tenancy isolation, which prevents users from one organization or tenant from accessing the data and resources of another. A recent security issue highlighted the importance of enforcing this isolation by removing company_id from request bodies and exclusively extracting it from JWT (JSON Web Tokens). This article will delve into the specifics of this vulnerability, its potential impact, and the recommended solutions to mitigate the risk.

Understanding the Security Vulnerability

The core issue revolves around how some application endpoints were handling the company_id. Instead of relying solely on the company_id embedded within the authenticated JWT, these endpoints were accepting the company_id directly from the request body. This seemingly minor detail creates a significant security vulnerability, opening the door to several potential exploits.

The Danger of Accepting company_id from Request Body

Allowing the company_id to be passed in the request body bypasses the intended security measures. A malicious user could craft requests that specify a company_id different from the one associated with their authenticated JWT. This could lead to:

  1. Unauthorized Resource Creation: An attacker could create resources (e.g., organizational units, positions, users) within another company's namespace, effectively infiltrating their system.
  2. Multi-Tenant Isolation Breach: This is a direct violation of multi-tenancy principles, as it allows users to operate outside their designated boundaries.
  3. Data Access and Modification: The most severe consequence is the potential for unauthorized access to sensitive data belonging to other tenants. An attacker might be able to read, modify, or even delete data they shouldn't have access to, leading to significant data breaches and compliance violations.

Illustrative Attack Scenario

Consider a scenario where an attacker authenticates as a user from "Company A." Their JWT correctly identifies them as belonging to Company A. However, when making a request to create a new organizational unit, they include "company_id": "Company B" in the request body. If the endpoint accepts this company_id, the organizational unit will be created under Company B's namespace, a clear breach of security.

# Attacker authenticated as user from company-A (JWT contains company-A)
# But sends company_id=company-B in request body

POST /organization-units
Cookie: access_token=<jwt_for_company_A>
{
  "name": "Malicious Unit",
  "company_id": "company-B"  # ⚠️ DIFFERENT from JWT!
}

# Result: Creates organization unit in company-B's namespace!
# This is a CRITICAL multi-tenant isolation breach

Affected Endpoints: A Detailed Look

Identifying the specific endpoints vulnerable to this issue is crucial for remediation. Several endpoints within the identity service were flagged as potentially affected, requiring immediate attention.

1. OrganizationUnitListResource.post()

This endpoint, located in services/identity_service/app/resources/organization_unit.py, was identified as directly accepting the company_id from the request body. The code snippet below demonstrates the vulnerability:

@require_jwt_auth()
@check_access_required("create")
def post(self):
    json_data = request.get_json()  # ❌ Accepts company_id from body
    org_unit_schema = OrganizationUnitSchema(session=db.session)
    new_org_unit = org_unit_schema.load(json_data)

The corresponding schema, organization_unit_schema.py, further confirms the issue by explicitly requiring the company_id in the JSON body:

company_id = fields.String(
    required=True,  # ❌ Required in JSON body
    validate=validate.Length(equal=36),
)

2. PositionListResource.post()

Located in services/identity_service/app/resources/position.py, this endpoint also requires verification but is suspected to be vulnerable. The schema, position_schema.py, includes a similar definition for company_id:

company_id = fields.String(
    required=True,  # ❌ Required in JSON body
    validate=validate.Regexp(...)
)

3. User.post(): A Complex Case

The User.post() endpoint, found in services/identity_service/app/resources/user.py, presents a more complex scenario. It involves manual JWT decoding and intricate company_id logic, necessitating a thorough review to ensure security.

The Secure Pattern: Learning from Success

Fortunately, a secure pattern was already in use within the application, specifically in the handling of Customer and Subcontractor resources. This pattern provides a blueprint for mitigating the vulnerability in other endpoints.

The Customer & Subcontractor Resources Approach

The secure pattern ensures that the company_id is always derived from the JWT, effectively overwriting any value provided in the request body. This is achieved by explicitly setting the company_id in the json_data to the value extracted from the JWT (g.company_id).

@require_jwt_auth()
@check_access_required("create")
def post(self):
    json_data = request.get_json()
    json_data["company_id"] = g.company_id  # ✅ FORCE JWT company_id
    
    schema = CustomerSchema(session=db.session)
    new_customer = schema.load(json_data)
    # ...

While the schema still defines company_id as required, the endpoint's logic ensures that the JWT's company_id takes precedence, preventing manipulation.

Recommended Solutions: A Phased Approach

To address this security vulnerability effectively, a two-phased approach is recommended, prioritizing immediate mitigation and long-term security enhancement.

Phase 1: Immediate Mitigation (Overwrite Pattern)

The first phase focuses on quickly addressing the vulnerability using the Overwrite Pattern. This involves overriding any client-provided company_id with the company_id extracted from the JWT. This approach offers several advantages:

Pros:

  • Simple to Implement: The code changes required are relatively straightforward, allowing for rapid deployment.
  • Works with Existing Schemas: No modifications to the existing schemas are necessary, minimizing disruption.
  • Clear Security Boundary: The pattern establishes a clear and unambiguous security boundary, ensuring the JWT's company_id is always authoritative.

Implementation:

The implementation involves adding a few lines of code to the affected endpoints. First, extract the company_id from the JWT (g.company_id). Then, overwrite the company_id in the json_data with this value. Additionally, implement security logging to detect and alert on any attempts to manipulate the company_id.

@require_jwt_auth()
@check_access_required("create")
def post(self):
    json_data = request.get_json()
    
    # ALWAYS override with JWT company_id - ignore client value
    json_data["company_id"] = g.company_id
    
    # Log security warning if client tried to send different company_id
    if "company_id" in request.get_json():  # Original body
        client_company_id = request.get_json().get("company_id")
        if client_company_id and client_company_id != g.company_id:
            logger.warning(
                "Security: Client attempted to set company_id=%s but JWT has %s",
                client_company_id,
                g.company_id
            )
    
    schema = OrganizationUnitSchema(session=db.session)
    new_org_unit = schema.load(json_data)
    # ...

Phase 2: Long-Term Enhancement (Schema Enforcement)

The second phase focuses on a more robust, long-term solution: Schema Enforcement. This involves modifying the schemas to explicitly prevent the company_id from being accepted during the loading process.

Pros:

  • Schema Rejects company_id in JSON: The schema acts as a first line of defense, preventing malicious input from even reaching the application logic.
  • Clear Separation of Concerns: This approach clearly separates input validation (schema) from application logic (endpoint), enhancing maintainability.
  • Prevents Accidental Acceptance: Even if the endpoint logic were to inadvertently bypass the overwrite, the schema would still prevent the incorrect company_id from being used.

Cons:

  • More Changes Required: Implementing schema enforcement necessitates modifications to both the schemas and the endpoints.
  • Need to Update API Documentation: The API documentation must be updated to reflect the change in how company_id is handled.

Implementation:

The recommended approach is to make the company_id field dump_only in the schema. This means the field will be included in the output (dump) but not accepted as input (load). Additionally, the company_id should be set from the JWT after the data has been loaded.

# Schema changes
class OrganizationUnitSchema(ma.SQLAlchemyAutoSchema):
    class Meta:
        model = OrganizationUnit
        include_fk = True
        dump_only = ("id", "created_at", "updated_at", "company_id")  # ✅ Add company_id
        unknown = RAISE

    # Remove company_id field definition - will be auto-generated as dump_only

# Endpoint changes
@require_jwt_auth()
@check_access_required("create")
def post(self):
    json_data = request.get_json()
    
    schema = OrganizationUnitSchema(session=db.session)
    new_org_unit = schema.load(json_data)
    
    # Set company_id from JWT after load
    new_org_unit.company_id = g.company_id
    
    db.session.add(new_org_unit)
    # ...

Testing Requirements: Ensuring Security and Functionality

Thorough testing is crucial to ensure that the implemented solutions effectively address the vulnerability without introducing regressions. Testing should include both security-specific tests and general regression tests.

Security Tests (CRITICAL)

Security tests should focus on verifying that the company_id from the JWT is enforced and that attempts to manipulate it are unsuccessful. A key test case involves attempting to create a resource in another company's namespace.

def test_cannot_create_org_unit_in_other_company():
    """Test that company_id from JWT is enforced."""
    # Authenticate as user from company-A
    login_response = client.post("/auth/login", json={
        "email": "user@company-a.com",
        "password": "password"
    })
    
    # Try to create org unit with company_id=company-B in body
    response = client.post("/organization-units", 
        cookies={"access_token": login_response.cookies.get("access_token")},
        json={
            "name": "Malicious Unit",
            "company_id": "company-B-uuid"  # Different from JWT!
        }
    )
    
    # Should succeed but use company-A (from JWT)
    assert response.status_code == 201
    data = response.get_json()
    assert data["company_id"] == "company-A-uuid"  # NOT company-B!

Regression Tests

Regression tests should verify that existing functionality remains intact after the security fix. This includes ensuring that normal resource creation works, the company_id is correctly set from the JWT, other fields are accepted, and all existing tests continue to pass.

Files to Modify: A Clear Roadmap

To facilitate the implementation of the solutions, a clear roadmap of files to modify is essential.

Endpoints

  • services/identity_service/app/resources/organization_unit.py
    • OrganizationUnitListResource.post()
  • services/identity_service/app/resources/position.py
    • PositionListResource.post() (verify needed)

Tests

  • tests/api/test_organization_units.py - Add security test
  • tests/api/test_positions.py - Add security test

Acceptance Criteria: Defining Success

To ensure the successful remediation of the vulnerability, clear acceptance criteria must be defined.

  • All endpoints override company_id with g.company_id from JWT.
  • Security logging is added for manipulation attempts.
  • Tests verify JWT company_id is enforced.
  • Tests verify client-provided company_id is ignored.
  • No regression in existing functionality.
  • Documentation updated (API docs, OpenAPI spec).

Related Issues: Connecting the Dots

This security vulnerability is related to other ongoing efforts to enhance the security and architecture of the identity service. Specifically, it is linked to:

  • #17 - Fix company_id Architecture in Identity Service (User endpoint)
  • guardian_service #5 - CRITICAL SECURITY - Multi-tenant isolation

Conclusion

Addressing the vulnerability of accepting company_id from request bodies is critical for maintaining the security and integrity of the application. By implementing the recommended phased approach, which prioritizes immediate mitigation and long-term security enhancement, organizations can effectively protect against potential attacks and ensure multi-tenant isolation. This comprehensive strategy, encompassing code modifications, thorough testing, and clear documentation, will fortify the application's security posture and safeguard sensitive data. Remember to regularly consult resources like the OWASP (Open Web Application Security Project) for the latest security best practices.