Skip to content

Conversation

@kevalmahajan
Copy link
Member

@kevalmahajan kevalmahajan commented Jul 25, 2025

🐛 Bug-fix PR


📌 Summary

Closes #601

Masks all the authentication values in the response of gateways APIs

💡 Fix Description

  1. Implemented a masked_GatewayRead function to apply masking to all necessary fields and validate data using the GatewayRead Pydantic model.
  2. Integrated this function into the API response to ensure all outgoing data is properly masked and validated before being sent.

🧪 Verification

Check Command Status
Lint suite make lint
Unit tests make test
Coverage ≥ 90 % make coverage
Manual regression no longer fails steps / screenshots

📐 MCP Compliance (if relevant)

  • Matches current MCP spec
  • No breaking change to MCP clients

✅ Checklist

  • Code formatted (make black isort pre-commit)
  • No secrets/credentials committed

Signed-off-by: Keval Mahajan <[email protected]>
Signed-off-by: Keval Mahajan <[email protected]>
Signed-off-by: Keval Mahajan <[email protected]>
@crivetimihai crivetimihai self-assigned this Jul 25, 2025
@crivetimihai
Copy link
Member

crivetimihai commented Jul 25, 2025

Passed:

  • make doctest

  • make test

  • make smoketest

  • make docker-run-ssl-host # UI works

  • make docker-stop compose-stop compose-rm compose-up compose-logs compose-stop

  • Checking the masking is effective:

    # start an mcp server
    python3 -m mcpgateway.translate --stdio "uvx mcp-server-git" --port 8103
    
    # export a token
    export MCPGATEWAY_BEARER_TOKEN=$(python3 -m mcpgateway.utils.create_jwt_token -u admin --secret my-test-key)
    
    # start a local gateway
    make serve
    
    # Register the server
    curl -v -X POST http://127.0.0.1:4444/gateways \
    -H "Authorization: Bearer $MCPGATEWAY_BEARER_TOKEN" \
    -H "Content-Type: application/json" \
    -d '{"name":"aaa","url":"http://127.0.0.1:8103/sse","transport":"SSE"}' | jq 
    
    {
      "id": "416b5071f2c44694ae521de818f249ca",
      "name": "aaa",
      "url": "http://127.0.0.1:8103/sse",
      "description": null,
      "transport": "SSE",
      "capabilities": {
        "experimental": {},
        "tools": {
          "listChanged": false
        }
      },
      "createdAt": "2025-07-25T07:47:55.272610",
      "updatedAt": "2025-07-25T07:47:55.272611",
      "enabled": true,
      "reachable": true,
      "lastSeen": "2025-07-25T07:47:55.271885",
      "authType": null,
      "authValue": null,
      "authUsername": null,
      "authPassword": null,
      "authToken": null,
      "authHeaderKey": null,
      "authHeaderValue": null,
      "slug": "aaa"
    }
  • Check if the token gets masked

    curl -v -X POST http://127.0.0.1:4444/gateways \
      -H "Authorization: Bearer $MCPGATEWAY_BEARER_TOKEN" \
      -H "Content-Type: application/json" \
      -d '{
        "name": "ccc",
        "url": "http://127.0.0.1:8103/sse",
        "transport": "SSE",
        "auth": {
          "type": "bearer",
          "token": "YOUR_MCP_SERVER_TOKEN"
        }
      }' | jq
    {
      "id": "2e02bf0c194a4c4cbe60dbcf37925734",
      "name": "ccc",
      "url": "http://127.0.0.1:8103/sse",
      "description": null,
      "transport": "SSE",
      "capabilities": {
        "experimental": {},
        "tools": {
          "listChanged": false
        }
      },
      "createdAt": "2025-07-25T07:52:32.812655",
      "updatedAt": "2025-07-25T07:52:32.812656",
      "enabled": true,
      "reachable": true,
      "lastSeen": "2025-07-25T07:52:32.812032",
      "authType": null,
      "authValue": null,
      "authUsername": null,
      "authPassword": null,
      "authToken": null,
      "authHeaderKey": null,
      "authHeaderValue": null,
      "slug": "ccc"
    }

@crivetimihai
Copy link
Member

crivetimihai commented Jul 26, 2025

This PR implements a security enhancement to mask sensitive authentication fields when returning gateway information through the API. Here's what's being changed:

Key Changes:

  1. New masked_gateway_read Method (in gateway_service.py):

    • Adds a new method that masks sensitive authentication fields before returning gateway data
    • Masks these fields with "*****":
      • auth_username, auth_password, auth_token
      • auth_header_value, auth_value
      • Both snake_case and camelCase versions of these fields
    • Accepts either a single GatewayRead object or a list of them
    • Returns the same type as the input (single or list)
  2. Schema Update (in schemas.py):

    • Modifies the _populate_auth validator to skip validation logic if it encounters the masked value "*****"
    • This prevents validation errors when processing masked authentication data
  3. API Endpoint Updates:

    • Admin endpoints (admin.py):
      • admin_list_gateways: Now masks sensitive fields before returning gateway list
      • admin_get_gateway: Now masks sensitive fields before returning single gateway
    • Regular endpoints (main.py):
      • list_gateways: Now masks sensitive fields
      • get_gateway: Now masks sensitive fields
  4. Test Updates (test_admin.py):

    • Updates tests to expect the camelCase field names in responses (e.g., authType instead of auth_type)
    • Adds required fields like name and url to test mocks
    • Ensures tests pass with the new masking behavior

Purpose:

This PR enhances security by ensuring that sensitive authentication credentials (passwords, tokens, API keys) are never exposed through the API responses. When clients request gateway information, they'll see:

  • The authentication type (basic, bearer, headers)
  • Non-sensitive fields like usernames or header keys
  • But all sensitive values (passwords, tokens, header values) will be masked as "*****"

This is a common security best practice to prevent accidental exposure of credentials through API responses, logs, or debugging output.

@crivetimihai
Copy link
Member

crivetimihai commented Jul 26, 2025

1. Test Bearer Auth - Check if authValue is Masked

# First, create a gateway with bearer auth
curl -X POST http://127.0.0.1:4444/gateways \
  -H "Authorization: Bearer $MCPGATEWAY_BEARER_TOKEN" \
  -H "Content-Type: application/json" \
  -d '{
    "name": "bearer-test",
    "url": "http://127.0.0.1:8103/sse",
    "transport": "SSE",
    "auth_type": "bearer",
    "auth_token": "secret-token-12345"
  }' | jq

# Get the gateway ID from response, then fetch it
curl -X GET http://127.0.0.1:4444/gateways/{gateway-id} \
  -H "Authorization: Bearer $MCPGATEWAY_BEARER_TOKEN" | jq

# BUG: Check if authValue is masked (it should be "*****" per issue #601)
# Expected: "authValue": "*****"
# Actual: "authValue": "ey123123..." (base64 encoded token - NOT MASKED!)

2. Test Basic Auth - All Fields Masked

# Create gateway with basic auth
curl -X POST http://127.0.0.1:4444/gateways \
  -H "Authorization: Bearer $MCPGATEWAY_BEARER_TOKEN" \
  -H "Content-Type: application/json" \
  -d '{
    "name": "basic-test",
    "url": "http://127.0.0.1:8103/sse",
    "transport": "SSE",
    "auth_type": "basic",
    "auth_username": "admin",
    "auth_password": "secretpass"
  }' | jq

# Fetch and check masking
curl -X GET http://127.0.0.1:4444/gateways/{gateway-id} \
  -H "Authorization: Bearer $MCPGATEWAY_BEARER_TOKEN" | jq

# Check these fields:
# - "authPassword": "*****" ✓ (should be masked)
# - "authValue": "ey123123..." ✗ (should be "*****" but isn't)

3. Test Custom Headers Auth

# Create gateway with custom headers
curl -X POST http://127.0.0.1:4444/gateways \
  -H "Authorization: Bearer $MCPGATEWAY_BEARER_TOKEN" \
  -H "Content-Type: application/json" \
  -d '{
    "name": "headers-test",
    "url": "http://127.0.0.1:8103/sse",
    "transport": "SSE",
    "auth_type": "authheaders",
    "auth_header_key": "X-API-Key",
    "auth_header_value": "my-secret-api-key"
  }' | jq

# Fetch and verify masking
curl -X GET http://127.0.0.1:4444/gateways/{gateway-id} \
  -H "Authorization: Bearer $MCPGATEWAY_BEARER_TOKEN" | jq

# Check:
# - "authHeaderValue": "*****" ✓ (should be masked)
# - "authValue": "eyJ..." ✗ (should be "*****" but isn't)

4. Test List Endpoints - Multiple Gateways

# List all gateways (regular endpoint)
curl -X GET http://127.0.0.1:4444/gateways \
  -H "Authorization: Bearer $MCPGATEWAY_BEARER_TOKEN" | jq

# List all gateways (admin endpoint)
curl -X GET http://127.0.0.1:4444/admin/gateways \
  -H "Authorization: Bearer $MCPGATEWAY_BEARER_TOKEN" | jq

# Both should mask sensitive fields for ALL gateways in the list

5. Test Edit Flow - Masked Values Problem

# Step 1: Get a gateway with auth
curl -X GET http://127.0.0.1:4444/admin/gateways/{gateway-id} \
  -H "Authorization: Bearer $MCPGATEWAY_BEARER_TOKEN" | jq

# You'll see: "authPassword": "*****"

# Step 2: Try to update the gateway without changing password
curl -X PUT http://127.0.0.1:4444/gateways/{gateway-id} \
  -H "Authorization: Bearer $MCPGATEWAY_BEARER_TOKEN" \
  -H "Content-Type: application/json" \
  -d '{
    "name": "basic-test-updated",
    "auth_type": "basic",
    "auth_username": "admin",
    "auth_password": "*****"
  }' | jq

# BUG: This might literally set the password to "*****" instead of keeping the existing one!

6. Test Validation Error with Masked Bearer Token

# This tests if re-validation works with masked values
# Create a test script to check if the masked response can be re-validated:

echo '{
  "name": "test",
  "url": "http://test.com",
  "auth_type": "bearer",
  "auth_value": "eyJBdXRob3JpemF0aW9uIjogIkJlYXJlciBhYmMxMjMifQ==",
  "auth_token": "*****"
}' | curl -X POST http://127.0.0.1:4444/test-validation \
  -H "Authorization: Bearer $MCPGATEWAY_BEARER_TOKEN" \
  -H "Content-Type: application/json" \
  -d @-

# This would need a test endpoint to verify, but the validation will likely fail

7. Quick Test Script - All Auth Types

#!/bin/bash

# Test all auth types and check if authValue is properly masked

echo "Testing Bearer Auth..."
BEARER_ID=$(curl -s -X POST http://127.0.0.1:4444/gateways \
  -H "Authorization: Bearer $MCPGATEWAY_BEARER_TOKEN" \
  -H "Content-Type: application/json" \
  -d '{
    "name": "bearer-test-'$(date +%s)'",
    "url": "http://127.0.0.1:8103/sse",
    "transport": "SSE",
    "auth_type": "bearer",
    "auth_token": "secret-token"
  }' | jq -r '.id')

echo "Fetching Bearer Gateway..."
curl -s -X GET "http://127.0.0.1:4444/gateways/$BEARER_ID" \
  -H "Authorization: Bearer $MCPGATEWAY_BEARER_TOKEN" | jq '{
    authType: .authType,
    authValue_is_masked: (.authValue == "*****"),
    authToken_is_masked: (.authToken == "*****"),
    authValue: .authValue
  }'

echo -e "\nTesting Basic Auth..."
BASIC_ID=$(curl -s -X POST http://127.0.0.1:4444/gateways \
  -H "Authorization: Bearer $MCPGATEWAY_BEARER_TOKEN" \
  -H "Content-Type: application/json" \
  -d '{
    "name": "basic-test-'$(date +%s)'",
    "url": "http://127.0.0.1:8103/sse",
    "transport": "SSE",
    "auth_type": "basic",
    "auth_username": "admin",
    "auth_password": "secret"
  }' | jq -r '.id')

echo "Fetching Basic Gateway..."
curl -s -X GET "http://127.0.0.1:4444/gateways/$BASIC_ID" \
  -H "Authorization: Bearer $MCPGATEWAY_BEARER_TOKEN" | jq '{
    authType: .authType,
    authValue_is_masked: (.authValue == "*****"),
    authPassword_is_masked: (.authPassword == "*****"),
    authValue: .authValue
  }'

Expected vs Actual Results

Run these tests and look for:

  1. authValue should ALWAYS be "*" for any auth type (per issue [Bug]: APIs for gateways in admin and main do not mask auth values #601)
  2. All sensitive fields (password, token, header values) should be "*****"
  3. Edit operations shouldn't break when receiving masked values
  4. The API should handle re-validation of masked data properly

The main bug is that authValue is not being masked, which is explicitly required by issue #601.

PR Checklist: Done ✅ / Missing ❌

✅ Done (Partially):

  • Created masked_gateway_read method in gateway_service.py
  • Updated GET /gateways/{id} to use masking
  • Updated GET /gateways to use masking
  • Updated GET /admin/gateways/{id} to use masking
  • Updated GET /admin/gateways to use masking
  • Added validation skip for "*****" in schemas.py

❌ Missing - Critical:

  1. POST endpoints not masked:

    • POST /gateways - Returns unmasked data after creation
    • POST /admin/gateways - Returns unmasked data after creation
  2. PUT endpoints not masked:

    • PUT /gateways/{id} - Returns unmasked data after update
    • PUT /admin/gateways/{id} - Returns unmasked data after update
  3. Other endpoints to check:

    • Any other endpoints that return GatewayRead objects

❌ Potential Code Issues:

  1. Validation logic flaw - The check if auth_value_encoded == "*****" won't work because auth_value is base64 encoded, not masked directly

❌ Not Tested:

  1. Whether masked data can be properly re-validated without errors
  2. Whether edit forms still work with masked values
  3. Whether the frontend handles masked values correctly

Summary:

All POST and PUT endpoints still expose sensitive data, and the authValue field is not masked.

@crivetimihai crivetimihai self-requested a review July 26, 2025 13:01
@kevalmahajan
Copy link
Member Author

  1. Masking has been implemented for response data in POST and PUT APIs.
  2. Masking is now also applied to other APIs that use the GatewayRead model, such as toggle_gateway_status.
  3. The masking function has been moved into the Pydantic model class instead of being a separate utility, making it easier to use and maintain.
  4. The edit functionality in the UI has been corrected. Previously, masked values like ***** were being stored when edits were made. Now, if no changes are made to the auth fields, the original values are retained and returned correctly; if changes are made, the updated values are stored as expected.

@crivetimihai crivetimihai merged commit 45c897d into main Jul 30, 2025
37 checks passed
@crivetimihai crivetimihai deleted the mask_auth branch July 30, 2025 09:16
vk-playground pushed a commit to vk-playground/mcp-context-forge that referenced this pull request Sep 14, 2025
* masked auth values in gateways apis

Signed-off-by: Keval Mahajan <[email protected]>

* linting

Signed-off-by: Keval Mahajan <[email protected]>

* masked admin-gateways api auth values

Signed-off-by: Keval Mahajan <[email protected]>

* mask first and convert to the alias values

Signed-off-by: Keval Mahajan <[email protected]>

* linting

Signed-off-by: Keval Mahajan <[email protected]>

* flake8 issues resolved

Signed-off-by: Keval Mahajan <[email protected]>

* improved masking auth values

Signed-off-by: Keval Mahajan <[email protected]>

* updated test cases

Signed-off-by: Keval Mahajan <[email protected]>

* updated doctest for masked

Signed-off-by: Keval Mahajan <[email protected]>

* resolved flake8 issues

Signed-off-by: Keval Mahajan <[email protected]>

---------

Signed-off-by: Keval Mahajan <[email protected]>
vk-playground pushed a commit to vk-playground/mcp-context-forge that referenced this pull request Sep 14, 2025
* masked auth values in gateways apis

Signed-off-by: Keval Mahajan <[email protected]>

* linting

Signed-off-by: Keval Mahajan <[email protected]>

* masked admin-gateways api auth values

Signed-off-by: Keval Mahajan <[email protected]>

* mask first and convert to the alias values

Signed-off-by: Keval Mahajan <[email protected]>

* linting

Signed-off-by: Keval Mahajan <[email protected]>

* flake8 issues resolved

Signed-off-by: Keval Mahajan <[email protected]>

* improved masking auth values

Signed-off-by: Keval Mahajan <[email protected]>

* updated test cases

Signed-off-by: Keval Mahajan <[email protected]>

* updated doctest for masked

Signed-off-by: Keval Mahajan <[email protected]>

* resolved flake8 issues

Signed-off-by: Keval Mahajan <[email protected]>

---------

Signed-off-by: Keval Mahajan <[email protected]>
vk-playground pushed a commit to vk-playground/mcp-context-forge that referenced this pull request Sep 16, 2025
* masked auth values in gateways apis

Signed-off-by: Keval Mahajan <[email protected]>

* linting

Signed-off-by: Keval Mahajan <[email protected]>

* masked admin-gateways api auth values

Signed-off-by: Keval Mahajan <[email protected]>

* mask first and convert to the alias values

Signed-off-by: Keval Mahajan <[email protected]>

* linting

Signed-off-by: Keval Mahajan <[email protected]>

* flake8 issues resolved

Signed-off-by: Keval Mahajan <[email protected]>

* improved masking auth values

Signed-off-by: Keval Mahajan <[email protected]>

* updated test cases

Signed-off-by: Keval Mahajan <[email protected]>

* updated doctest for masked

Signed-off-by: Keval Mahajan <[email protected]>

* resolved flake8 issues

Signed-off-by: Keval Mahajan <[email protected]>

---------

Signed-off-by: Keval Mahajan <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: APIs for gateways in admin and main do not mask auth values

3 participants