diff --git a/openapi2mcp/oas3_testfiles/09-security-acl-conversion-mode.expected.json b/openapi2mcp/oas3_testfiles/09-security-acl-conversion-mode.expected.json new file mode 100644 index 00000000..107034ea --- /dev/null +++ b/openapi2mcp/oas3_testfiles/09-security-acl-conversion-mode.expected.json @@ -0,0 +1,148 @@ +{ + "_format_version": "3.0", + "services": [ + { + "host": "mock-e91bb6429fe84bfeb970620df9499e52.mock.insomnia.run", + "id": "259da73d-d48d-59b5-99fc-dbd49ce1a1d7", + "name": "kongair-flights", + "path": "/", + "plugins": [], + "port": 443, + "protocol": "https", + "routes": [ + { + "id": "6916ad46-4c19-56d8-aaad-0c09dba4b135", + "name": "kongair-flights-mcp", + "paths": [ + "/kongair-flights-mcp" + ], + "plugins": [ + { + "config": { + "default_acl": [ + { + "allow": [ + "flights:read" + ], + "scope": "tools" + } + ], + "mode": "conversion", + "tools": [ + { + "acl": { + "allow": [ + "flights:read" + ] + }, + "annotations": { + "title": "Get KongAir planned flights" + }, + "description": "Returns all the scheduled flights for a given day\n", + "method": "GET", + "name": "get-cool-flights", + "parameters": [ + { + "description": "Filter by date (defaults to current day)", + "in": "query", + "name": "date", + "required": false, + "schema": { + "type": "string" + } + } + ], + "path": "/flights" + }, + { + "acl": { + "allow": [ + "flights:write" + ] + }, + "annotations": { + "title": "Create a new flight" + }, + "description": "Creates a new scheduled flight entry\n", + "method": "POST", + "name": "create-flight", + "path": "/flights", + "request_body": { + "content": { + "application/json": { + "schema": { + "properties": { + "number": { + "type": "string" + }, + "route_id": { + "type": "string" + }, + "scheduled_arrival": { + "type": "string" + }, + "scheduled_departure": { + "type": "string" + } + }, + "required": [ + "number", + "route_id", + "scheduled_departure", + "scheduled_arrival" + ], + "type": "object" + } + } + }, + "required": true + } + }, + { + "acl": { + "allow": [ + "flights:read" + ] + }, + "annotations": { + "title": "Get a specific flight by flight number" + }, + "description": "Returns a specific flight given its flight number\n", + "method": "GET", + "name": "get-flight-by-number", + "parameters": [ + { + "description": "The flight number", + "in": "path", + "name": "flightNumber", + "required": true, + "schema": { + "type": "string" + } + } + ], + "path": "/flights/{flightNumber}" + } + ] + }, + "id": "7d56a807-dd7f-5660-8de3-d7611dd5ad28", + "name": "ai-mcp-proxy", + "tags": [ + "OAS3_import", + "OAS3file_09-security-acl-conversion-mode.yaml" + ] + } + ], + "tags": [ + "OAS3_import", + "OAS3file_09-security-acl-conversion-mode.yaml" + ] + } + ], + "tags": [ + "OAS3_import", + "OAS3file_09-security-acl-conversion-mode.yaml" + ] + } + ] +} \ No newline at end of file diff --git a/openapi2mcp/oas3_testfiles/09-security-acl-conversion-mode.yaml b/openapi2mcp/oas3_testfiles/09-security-acl-conversion-mode.yaml new file mode 100644 index 00000000..89ed8358 --- /dev/null +++ b/openapi2mcp/oas3_testfiles/09-security-acl-conversion-mode.yaml @@ -0,0 +1,103 @@ +--- +# Same spec as 08-security-acl.yaml but used with mode=conversion. +# Expected: tools still get acl.allow, but acl_attribute_type and +# access_token_claim_field must NOT appear in the plugin config. +openapi: 3.0.0 + +info: + description: KongAir Flights service provides the scheduled flights for KongAir + version: 0.1.0 + title: KongAir Flights + +x-kong-name: kongair-flights + +x-kong-mcp-default-acl: + - scope: tools + allow: + - "flights:read" + +servers: + - url: https://mock-e91bb6429fe84bfeb970620df9499e52.mock.insomnia.run + description: KongAir API Server + +paths: + "/flights": + get: + security: + - okta_oauth2: + - flights:read + x-kong-mcp-tool-name: get-cool-flights + summary: Get KongAir planned flights + description: | + Returns all the scheduled flights for a given day + operationId: get-flights + parameters: + - name: date + in: query + description: Filter by date (defaults to current day) + required: false + schema: + type: string + post: + security: + - okta_oauth2: + - flights:write + summary: Create a new flight + description: | + Creates a new scheduled flight entry + operationId: create-flight + requestBody: + required: true + content: + application/json: + schema: + type: object + required: + - number + - route_id + - scheduled_departure + - scheduled_arrival + properties: + number: + type: string + route_id: + type: string + scheduled_departure: + type: string + format: date-time + scheduled_arrival: + type: string + format: date-time + + "/flights/{flightNumber}": + get: + security: + - okta_oauth2: + - flights:read + summary: Get a specific flight by flight number + description: | + Returns a specific flight given its flight number + operationId: get-flight-by-number + parameters: + - name: flightNumber + in: path + description: The flight number + required: true + schema: + type: string + +components: + securitySchemes: + okta_oauth2: + type: oauth2 + x-kong-mcp-acl: + acl_attribute_type: oauth_access_token + access_token_claim_field: scp + flows: + authorizationCode: + authorizationUrl: https://example.okta.com/oauth2/default/v1/authorize + tokenUrl: https://example.okta.com/oauth2/default/v1/token + scopes: + flights:read: Read access to flight resources + flights:write: Write access to flight resources + diff --git a/openapi2mcp/openapi2mcp.go b/openapi2mcp/openapi2mcp.go index bd7fa97b..f8a2f013 100644 --- a/openapi2mcp/openapi2mcp.go +++ b/openapi2mcp/openapi2mcp.go @@ -648,7 +648,10 @@ func Convert(content []byte, opts O2MOptions) (map[string]interface{}, error) { "tools": tools, } - if aclConfig != nil { + // acl_attribute_type and access_token_claim_field are only valid for the + // conversion-listener mode; in conversion-only mode the listener plugin + // upstream is responsible for token validation and these fields must be omitted. + if aclConfig != nil && opts.Mode == ModeConversionListener { if v, ok := aclConfig["acl_attribute_type"]; ok { mcpPluginConfig["acl_attribute_type"] = v } diff --git a/openapi2mcp/openapi2mcp_test.go b/openapi2mcp/openapi2mcp_test.go index 2a876e2b..c8490c4a 100644 --- a/openapi2mcp/openapi2mcp_test.go +++ b/openapi2mcp/openapi2mcp_test.go @@ -596,6 +596,124 @@ paths: assert.Nil(t, tool["acl"], "tool should not have acl") } +// When mode=conversion is used with x-kong-mcp-acl present in the spec, the output: +// - MUST NOT contain acl_attribute_type at the plugin level +// (Kong Gateway rejects this field in conversion-only mode) +// - MUST NOT contain access_token_claim_field at the plugin level +// (same reason — these are only valid for conversion-listener) +// - MUST still contain acl.allow on every tool +// (the upstream listener needs these scopes to enforce per-tool ACL) +func Test_Openapi2mcp_SecurityACL_ConversionMode(t *testing.T) { + fileNameIn := "09-security-acl-conversion-mode.yaml" + fileNameExpected := "09-security-acl-conversion-mode.expected.json" + fileNameOut := "09-security-acl-conversion-mode.generated.json" + + dataIn, err := os.ReadFile(fixturePath + fileNameIn) + if err != nil { + t.Fatalf("Failed to read input file: %v", err) + } + + dataOut, err := Convert(dataIn, O2MOptions{ + Tags: []string{"OAS3_import", "OAS3file_" + fileNameIn}, + Mode: ModeConversion, + }) + assert.NoError(t, err, "should not error for mode=conversion with ACL") + + JSONOut, _ := json.MarshalIndent(dataOut, "", " ") + os.WriteFile(fixturePath+fileNameOut, JSONOut, 0o600) + JSONExpected, err := os.ReadFile(fixturePath + fileNameExpected) + if err != nil { + t.Fatalf("Failed to read expected file: %v", err) + } + + assert.JSONEq(t, string(JSONExpected), string(JSONOut), + "the JSON blobs should be equal for mode=conversion with ACL") + + // --- Programmatic assertions for the fix --- + services := dataOut["services"].([]interface{}) + service := services[0].(map[string]interface{}) + routes := service["routes"].([]interface{}) + route := routes[0].(map[string]interface{}) + plugins := route["plugins"].([]interface{}) + plugin := plugins[0].(map[string]interface{}) + config := plugin["config"].(map[string]interface{}) + + // Core fix: these two fields MUST be absent in conversion mode. + // If either is present, Kong Gateway will reject the config with HTTP 400. + assert.Nil(t, config["acl_attribute_type"], + "acl_attribute_type must NOT be present in conversion mode — Gateway rejects it") + assert.Nil(t, config["access_token_claim_field"], + "access_token_claim_field must NOT be present in conversion mode — Gateway rejects it") + + // Mode must be set correctly + assert.Equal(t, ModeConversion, config["mode"]) + + // Per-tool acl.allow MUST still be present — the upstream listener uses these scopes + tools := config["tools"].([]interface{}) + assert.Len(t, tools, 3, "should have 3 tools") + + tool0 := tools[0].(map[string]interface{}) + assert.Equal(t, "get-cool-flights", tool0["name"]) + acl0 := tool0["acl"].(map[string]interface{}) + assert.Equal(t, []string{"flights:read"}, acl0["allow"], + "tool must still have acl.allow even in conversion mode") + + tool1 := tools[1].(map[string]interface{}) + assert.Equal(t, "create-flight", tool1["name"]) + acl1 := tool1["acl"].(map[string]interface{}) + assert.Equal(t, []string{"flights:write"}, acl1["allow"], + "tool must still have acl.allow even in conversion mode") + + tool2 := tools[2].(map[string]interface{}) + assert.Equal(t, "get-flight-by-number", tool2["name"]) + acl2 := tool2["acl"].(map[string]interface{}) + assert.Equal(t, []string{"flights:read"}, acl2["allow"], + "tool must still have acl.allow even in conversion mode") +} + +// Test_Openapi2mcp_SecurityACL_ConversionListenerMode confirms that +// conversion-listener mode (the default) still emits acl_attribute_type and +// access_token_claim_field — i.e. the fix is backward-compatible. +func Test_Openapi2mcp_SecurityACL_ConversionListenerMode(t *testing.T) { + // Re-use the existing 08 fixture which was designed for conversion-listener + fileNameIn := "08-security-acl.yaml" + dataIn, err := os.ReadFile(fixturePath + fileNameIn) + if err != nil { + t.Fatalf("Failed to read input file: %v", err) + } + + // Explicitly pass ModeConversionListener (same as the default) + dataOut, err := Convert(dataIn, O2MOptions{ + Tags: []string{"OAS3_import", "OAS3file_" + fileNameIn}, + Mode: ModeConversionListener, + }) + if err != nil { + t.Errorf("didn't expect error: %v", err) + return + } + + services := dataOut["services"].([]interface{}) + service := services[0].(map[string]interface{}) + routes := service["routes"].([]interface{}) + route := routes[0].(map[string]interface{}) + plugins := route["plugins"].([]interface{}) + plugin := plugins[0].(map[string]interface{}) + config := plugin["config"].(map[string]interface{}) + + // These MUST still be present in conversion-listener mode (backward compat) + assert.Equal(t, "oauth_access_token", config["acl_attribute_type"], + "acl_attribute_type must still be emitted for conversion-listener mode") + assert.Equal(t, "scp", config["access_token_claim_field"], + "access_token_claim_field must still be emitted for conversion-listener mode") + + // Per-tool acl.allow must also still be present + tools := config["tools"].([]interface{}) + for i, t2 := range tools { + tool := t2.(map[string]interface{}) + assert.NotNil(t, tool["acl"], "tool[%d] must have acl.allow in conversion-listener mode", i) + } +} + func Test_Openapi2mcp_SecurityACL_DocLevelInheritance(t *testing.T) { // Test that operations without security inherit from document-level security dataIn := []byte(`