diff --git a/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/AbstractPythonConnexionServerCodegen.java b/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/AbstractPythonConnexionServerCodegen.java index f93cad43dfc5..45159efdce93 100644 --- a/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/AbstractPythonConnexionServerCodegen.java +++ b/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/AbstractPythonConnexionServerCodegen.java @@ -86,6 +86,15 @@ public void serialize(Boolean value, JsonGenerator gen, SerializerProvider seria protected boolean usePythonSrcRootInImports = Boolean.FALSE; protected boolean moveTestsUnderPythonSrcRoot = Boolean.FALSE; + // Python function name suffix per security scheme name (e.g. "api-key" -> "api_key"). + // Suffixes are assigned by addSecurityExtensions (called from preprocessOpenAPI): + // scheme names that are already valid identifier parts keep their name, the rest are + // numbered in spec order, so that the x-*Func references written into the spec and the + // function definitions emitted by security_controller.mustache always agree, even when + // the scheme name is not a valid Python identifier. fromSecurity only reads the values + // that addSecurityExtensions has already computed. + private final Map securityFunctionNameSuffixes = new HashMap<>(); + public AbstractPythonConnexionServerCodegen(String templateDirectory, boolean fixBodyNameValue) { super(); @@ -459,26 +468,38 @@ private void addSecurityExtensions(OpenAPI openAPI) { Components components = openAPI.getComponents(); if (components != null && components.getSecuritySchemes() != null) { Map securitySchemes = components.getSecuritySchemes(); + securityFunctionNameSuffixes.clear(); + // Reserve the suffix of names that already are valid identifier parts (no + // character replacement needed) so they keep the same function name as before. + // The remaining names are numbered afterwards, in spec order, avoiding the + // reserved suffixes (e.g. "api-key" listed before "api_key" gets "api_key_2", + // while "api_key" keeps "api_key"). + for (String securityName : securitySchemes.keySet()) { + if (toPythonIdentifierSuffix(securityName).equals(securityName)) { + securityFunctionNameSuffixes.put(securityName, securityName); + } + } for (Map.Entry securitySchemesEntry : securitySchemes.entrySet()) { String securityName = securitySchemesEntry.getKey(); SecurityScheme securityScheme = securitySchemesEntry.getValue(); + String functionSuffix = securityFunctionNameSuffix(securityName); String baseFunctionName = controllerPackage + ".security_controller."; switch (securityScheme.getType()) { case APIKEY: - addSecurityExtension(securityScheme, "x-apikeyInfoFunc", baseFunctionName + "info_from_" + securityName); + addSecurityExtension(securityScheme, "x-apikeyInfoFunc", baseFunctionName + "info_from_" + functionSuffix); break; case HTTP: if ("basic".equals(securityScheme.getScheme())) { - addSecurityExtension(securityScheme, "x-basicInfoFunc", baseFunctionName + "info_from_" + securityName); + addSecurityExtension(securityScheme, "x-basicInfoFunc", baseFunctionName + "info_from_" + functionSuffix); } else if ("bearer".equals(securityScheme.getScheme())) { - addSecurityExtension(securityScheme, "x-bearerInfoFunc", baseFunctionName + "info_from_" + securityName); + addSecurityExtension(securityScheme, "x-bearerInfoFunc", baseFunctionName + "info_from_" + functionSuffix); } break; case OPENIDCONNECT: LOGGER.warn("Security type {} is not supported by connexion yet", securityScheme.getType().toString()); case OAUTH2: - addSecurityExtension(securityScheme, "x-tokenInfoFunc", baseFunctionName + "info_from_" + securityName); - addSecurityExtension(securityScheme, "x-scopeValidateFunc", baseFunctionName + "validate_scope_" + securityName); + addSecurityExtension(securityScheme, "x-tokenInfoFunc", baseFunctionName + "info_from_" + functionSuffix); + addSecurityExtension(securityScheme, "x-scopeValidateFunc", baseFunctionName + "validate_scope_" + functionSuffix); break; default: LOGGER.warn("Unknown security type {}", securityScheme.getType().toString()); @@ -487,6 +508,52 @@ private void addSecurityExtensions(OpenAPI openAPI) { } } + @Override + public List fromSecurity(Map schemes) { + List codegenSecurities = super.fromSecurity(schemes); + for (CodegenSecurity codegenSecurity : codegenSecurities) { + codegenSecurity.vendorExtensions.put("x-python-connexion-security-function-suffix", + securityFunctionNameSuffix(codegenSecurity.name)); + } + return codegenSecurities; + } + + /** + * Returns the Python identifier suffix used for the info_from_/validate_scope_ functions + * of the given security scheme name. The result is stable per scheme name and unique across + * schemes of the same spec. Scheme names that already are valid identifier parts are + * reserved by addSecurityExtensions and keep their name; for the others, if the sanitized + * suffix is already taken by another scheme name (e.g. "api-key" vs "api.key"), "_2", + * "_3", ... is appended in spec order. + */ + private String securityFunctionNameSuffix(String schemeName) { + return securityFunctionNameSuffixes.computeIfAbsent(schemeName, name -> { + String base = toPythonIdentifierSuffix(name); + String suffix = base; + for (int i = 2; securityFunctionNameSuffixes.containsValue(suffix); i++) { + suffix = base + "_" + i; + } + return suffix; + }); + } + + /** + * Maps a security scheme name to a string usable as part of a Python identifier: + * every character that is not ASCII alphanumeric or '_' becomes '_'. The result is + * only used after the "info_from_"/"validate_scope_" prefixes, so a leading digit + * needs no further fix-up. + */ + private static String toPythonIdentifierSuffix(String name) { + StringBuilder suffix = new StringBuilder(name.length()); + for (int i = 0; i < name.length(); i++) { + char c = name.charAt(i); + boolean isIdentifierChar = (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') + || (c >= '0' && c <= '9') || c == '_'; + suffix.append(isIdentifierChar ? c : '_'); + } + return suffix.toString(); + } + private static List getOperations(Map objs) { List result = new ArrayList<>(); ApiInfoMap apiInfo = (ApiInfoMap) objs.get("apiInfo"); diff --git a/modules/openapi-generator/src/main/resources/python-aiohttp/security_controller.mustache b/modules/openapi-generator/src/main/resources/python-aiohttp/security_controller.mustache index df74618a9ffc..ece83fafbdad 100644 --- a/modules/openapi-generator/src/main/resources/python-aiohttp/security_controller.mustache +++ b/modules/openapi-generator/src/main/resources/python-aiohttp/security_controller.mustache @@ -3,7 +3,7 @@ from typing import List {{#authMethods}} {{#isOAuth}} -def info_from_{{name}}(token: str) -> dict: +def info_from_{{{vendorExtensions.x-python-connexion-security-function-suffix}}}(token: str) -> dict: """ Validate and decode token. Returned value will be passed in 'token_info' parameter of your operation function, if there is one. @@ -14,14 +14,14 @@ def info_from_{{name}}(token: str) -> dict: return {'scopes': ['read:pets', 'write:pets'], 'uid': 'user_id'} -def validate_scope_{{name}}(required_scopes: List[str], token_scopes: List[str]) -> bool: +def validate_scope_{{{vendorExtensions.x-python-connexion-security-function-suffix}}}(required_scopes: List[str], token_scopes: List[str]) -> bool: """ Validate required scopes are included in token scope """ return set(required_scopes).issubset(set(token_scopes)) {{/isOAuth}} {{#isApiKey}} -def info_from_{{name}}(api_key: str, required_scopes: None) -> dict: +def info_from_{{{vendorExtensions.x-python-connexion-security-function-suffix}}}(api_key: str, required_scopes: None) -> dict: """ Check and retrieve authentication information from api_key. Returned value will be passed in 'token_info' parameter of your operation function, if there is one. @@ -33,7 +33,7 @@ def info_from_{{name}}(api_key: str, required_scopes: None) -> dict: {{/isApiKey}} {{#isBasicBasic}} -def info_from_{{name}}(username: str, password: str, required_scopes: None) -> dict: +def info_from_{{{vendorExtensions.x-python-connexion-security-function-suffix}}}(username: str, password: str, required_scopes: None) -> dict: """ Check and retrieve authentication information from basic auth. Returned value will be passed in 'token_info' parameter of your operation function, if there is one. @@ -45,7 +45,7 @@ def info_from_{{name}}(username: str, password: str, required_scopes: None) -> d {{/isBasicBasic}} {{#isBasicBearer}} -def info_from_{{name}}(token: str) -> dict: +def info_from_{{{vendorExtensions.x-python-connexion-security-function-suffix}}}(token: str) -> dict: """ Check and retrieve authentication information from custom bearer token. Returned value will be passed in 'token_info' parameter of your operation function, if there is one. diff --git a/modules/openapi-generator/src/main/resources/python-flask/security_controller.mustache b/modules/openapi-generator/src/main/resources/python-flask/security_controller.mustache index a5f3d6d5312e..28da60eb248c 100644 --- a/modules/openapi-generator/src/main/resources/python-flask/security_controller.mustache +++ b/modules/openapi-generator/src/main/resources/python-flask/security_controller.mustache @@ -3,7 +3,7 @@ from typing import List {{#authMethods}} {{#isOAuth}} -def info_from_{{name}}(token): +def info_from_{{{vendorExtensions.x-python-connexion-security-function-suffix}}}(token): """ Validate and decode token. Returned value will be passed in 'token_info' parameter of your operation function, if there is one. @@ -18,7 +18,7 @@ def info_from_{{name}}(token): return {'scopes': ['read:pets', 'write:pets'], 'uid': 'user_id'} -def validate_scope_{{name}}(required_scopes, token_scopes): +def validate_scope_{{{vendorExtensions.x-python-connexion-security-function-suffix}}}(required_scopes, token_scopes): """ Validate required scopes are included in token scope @@ -34,7 +34,7 @@ def validate_scope_{{name}}(required_scopes, token_scopes): {{/isOAuth}} {{#isApiKey}} -def info_from_{{name}}(api_key, required_scopes): +def info_from_{{{vendorExtensions.x-python-connexion-security-function-suffix}}}(api_key, required_scopes): """ Check and retrieve authentication information from api_key. Returned value will be passed in 'token_info' parameter of your operation function, if there is one. @@ -52,7 +52,7 @@ def info_from_{{name}}(api_key, required_scopes): {{/isApiKey}} {{#isBasicBasic}} -def info_from_{{name}}(username, password, required_scopes): +def info_from_{{{vendorExtensions.x-python-connexion-security-function-suffix}}}(username, password, required_scopes): """ Check and retrieve authentication information from basic auth. Returned value will be passed in 'token_info' parameter of your operation function, if there is one. @@ -72,7 +72,7 @@ def info_from_{{name}}(username, password, required_scopes): {{/isBasicBasic}} {{#isBasicBearer}} -def info_from_{{name}}(token): +def info_from_{{{vendorExtensions.x-python-connexion-security-function-suffix}}}(token): """ Check and retrieve authentication information from custom bearer token. Returned value will be passed in 'token_info' parameter of your operation function, if there is one. diff --git a/modules/openapi-generator/src/test/java/org/openapitools/codegen/python/AbstractPythonConnexionServerCodegenTest.java b/modules/openapi-generator/src/test/java/org/openapitools/codegen/python/AbstractPythonConnexionServerCodegenTest.java index f3478726ea1a..baf0f6bbd320 100644 --- a/modules/openapi-generator/src/test/java/org/openapitools/codegen/python/AbstractPythonConnexionServerCodegenTest.java +++ b/modules/openapi-generator/src/test/java/org/openapitools/codegen/python/AbstractPythonConnexionServerCodegenTest.java @@ -4,6 +4,7 @@ import io.swagger.parser.OpenAPIParser; import io.swagger.v3.oas.models.OpenAPI; import io.swagger.v3.oas.models.parameters.Parameter; +import io.swagger.v3.oas.models.security.SecurityScheme; import io.swagger.v3.parser.core.models.ParseOptions; import org.apache.commons.io.FileUtils; import org.apache.commons.lang3.StringUtils; @@ -23,9 +24,13 @@ import java.nio.file.Paths; import java.util.Collections; import java.util.HashSet; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.Objects; +import java.util.regex.Matcher; +import java.util.regex.Pattern; import static org.assertj.core.api.Assertions.assertThat; import static org.openapitools.codegen.TestUtils.assertFileContains; @@ -161,12 +166,18 @@ private static String platformAgnosticPath(String... nodes) { } private static Path generateOutputDir(String generatorName, Map additionalProperties) throws IOException { + return generateOutputDir(generatorName, additionalProperties, + "src/test/resources/3_0/python/connexion-param-sources.yaml"); + } + + private static Path generateOutputDir(String generatorName, Map additionalProperties, + String inputSpec) throws IOException { File output = Files.createTempDirectory("test-" + generatorName).toFile().getCanonicalFile(); output.deleteOnExit(); CodegenConfigurator config = new CodegenConfigurator() .setGeneratorName(generatorName) - .setInputSpec("src/test/resources/3_0/python/connexion-param-sources.yaml") + .setInputSpec(inputSpec) .setOutputDir(output.getAbsolutePath()); for (Map.Entry entry : additionalProperties.entrySet()) { config.addAdditionalProperty(entry.getKey(), entry.getValue()); @@ -559,6 +570,110 @@ public void testFreeFormAndAnyTypeParametersDoNotCallFromDict() throws IOExcepti } } + @Test(description = "security scheme names with '-' or '.' produce valid, unique Python function names that match the x-*Func references") + public void testSecuritySchemeNamesProduceValidPythonFunctions() throws IOException { + String specPath = "src/test/resources/3_0/python-flask/security-scheme-names.yaml"; + String funcBase = "openapi_server.controllers.security_controller."; + + // scheme name -> extension name -> function name the extension must reference. + // "api_key" and "api_key_2" are already valid identifiers listed after names that + // sanitize to their suffix: they keep the unnumbered suffix (the same output as + // before sanitization), so "api-key" is bumped to "api_key_3" and "api.key" to + // "api_key_4". The user-provided x-apikeyInfoFunc on "alias-key" is preserved + // instead of being overwritten with the generated "info_from_alias_key". + Map> expectedFuncRefs = new LinkedHashMap<>(); + expectedFuncRefs.put("api-key", ImmutableMap.of("x-apikeyInfoFunc", "info_from_api_key_3")); + expectedFuncRefs.put("api_key", ImmutableMap.of("x-apikeyInfoFunc", "info_from_api_key")); + expectedFuncRefs.put("petstore.auth", ImmutableMap.of( + "x-tokenInfoFunc", "info_from_petstore_auth", + "x-scopeValidateFunc", "validate_scope_petstore_auth")); + expectedFuncRefs.put("basic-auth", ImmutableMap.of("x-basicInfoFunc", "info_from_basic_auth")); + expectedFuncRefs.put("api_key_2", ImmutableMap.of("x-apikeyInfoFunc", "info_from_api_key_2")); + expectedFuncRefs.put("api.key", ImmutableMap.of("x-apikeyInfoFunc", "info_from_api_key_4")); + expectedFuncRefs.put("1st-key", ImmutableMap.of("x-apikeyInfoFunc", "info_from_1st_key")); + expectedFuncRefs.put("bearer-token", ImmutableMap.of("x-bearerInfoFunc", "info_from_bearer_token")); + expectedFuncRefs.put("alias-key", ImmutableMap.of("x-apikeyInfoFunc", "info_from_api_key")); + + Map> variants = new LinkedHashMap<>(); + variants.put("python-flask", Collections.emptyMap()); + variants.put("python-flask+useConnexion3", ImmutableMap.of("useConnexion3", "true")); + variants.put("python-aiohttp", Collections.emptyMap()); + + for (Map.Entry> variant : variants.entrySet()) { + String generatorName = variant.getKey().replace("+useConnexion3", ""); + Path outputDir = generateOutputDir(generatorName, variant.getValue(), specPath); + String label = variant.getKey(); + + Path securityController = outputDir.resolve( + Paths.get("openapi_server", "controllers", "security_controller.py")); + assertFileExists(securityController); + assertFileNotContains( + securityController, + "def info_from__1st_key(", + "def info_from_1st_key_2("); + + // every generated function name must be a valid Python identifier and unique + String code = Files.readString(securityController); + Matcher defMatcher = + Pattern.compile("def\\s+(\\S+)\\s*\\(").matcher(code); + Set functionNames = new HashSet<>(); + while (defMatcher.find()) { + String functionName = defMatcher.group(1); + assertThat(functionName) + .as("%s: %s must be a valid Python identifier", label, functionName) + .matches("[a-zA-Z_][a-zA-Z0-9_]*"); + functionNames.add(functionName); + } + assertThat(functionNames) + .as("%s: info_from_/validate_scope_ function names must be unique", label) + .containsExactlyInAnyOrder( + "info_from_api_key", + "info_from_api_key_2", + "info_from_api_key_3", + "info_from_api_key_4", + "info_from_petstore_auth", + "validate_scope_petstore_auth", + "info_from_basic_auth", + "info_from_1st_key", + "info_from_bearer_token", + "info_from_alias_key"); + + // each scheme's x-*Func reference in the generated spec must point at the + // function defined for it in security_controller.py + Path openapiYaml = outputDir.resolve( + Paths.get("openapi_server", "openapi", "openapi.yaml")); + assertFileExists(openapiYaml); + OpenAPI generatedSpec = new OpenAPIParser() + .readLocation(openapiYaml.toString(), null, new ParseOptions()) + .getOpenAPI(); + assertThat(generatedSpec) + .as("%s: generated openapi.yaml must parse as OpenAPI", label) + .isNotNull(); + Map securitySchemes = + generatedSpec.getComponents().getSecuritySchemes(); + assertThat(securitySchemes) + .as("%s: generated spec must declare security schemes", label) + .isNotNull(); + assertThat(securitySchemes.keySet()) + .as("%s: generated spec must declare every fixture scheme", label) + .containsExactlyInAnyOrderElementsOf(expectedFuncRefs.keySet()); + for (Map.Entry> schemeEntry : expectedFuncRefs.entrySet()) { + Map extensions = + securitySchemes.get(schemeEntry.getKey()).getExtensions(); + for (Map.Entry funcRef : schemeEntry.getValue().entrySet()) { + assertThat(extensions) + .as("%s: %s.%s must reference the generated function", + label, schemeEntry.getKey(), funcRef.getKey()) + .containsEntry(funcRef.getKey(), funcBase + funcRef.getValue()); + assertThat(functionNames) + .as("%s: %s.%s target must be defined in security_controller.py", + label, schemeEntry.getKey(), funcRef.getKey()) + .contains(funcRef.getValue()); + } + } + } + } + @Test(description = "python-aiohttp generated requirements.txt and setup.py declare python_dateutil so util.deserialize_date/datetime actually parse dates") public void testAiohttpRequirementsIncludePythonDateutil() throws IOException { Path aiohttpDir = generateOutputDir("python-aiohttp", Collections.emptyMap()); diff --git a/modules/openapi-generator/src/test/resources/3_0/python-flask/security-scheme-names.yaml b/modules/openapi-generator/src/test/resources/3_0/python-flask/security-scheme-names.yaml new file mode 100644 index 000000000000..65cac781c8cf --- /dev/null +++ b/modules/openapi-generator/src/test/resources/3_0/python-flask/security-scheme-names.yaml @@ -0,0 +1,183 @@ +openapi: 3.0.3 +info: + title: Security scheme name sanitization test + version: 1.0.0 + description: | + Scheme names containing '-' or '.' are valid OpenAPI component keys but are + not valid Python identifiers. The generated security_controller.py must use + sanitized, unique function names that match the x-*Func references. +servers: + - url: /v1 +paths: + /ping: + get: + tags: [health] + operationId: ping + security: + - api-key: [] + responses: + '200': + description: ok + content: + application/json: + schema: + type: string + /pets: + get: + tags: [pet] + operationId: list_pets + security: + - petstore.auth: [read:pets] + responses: + '200': + description: ok + content: + application/json: + schema: + type: array + items: + type: string + /whoami: + get: + tags: [health] + operationId: whoami + security: + - basic-auth: [] + responses: + '200': + description: ok + content: + application/json: + schema: + type: string + /ping-alt: + get: + tags: [health] + operationId: ping_alt + security: + - api.key: [] + responses: + '200': + description: ok + content: + application/json: + schema: + type: string + /digit: + get: + tags: [health] + operationId: ping_digit + security: + - 1st-key: [] + responses: + '200': + description: ok + content: + application/json: + schema: + type: string + /token: + get: + tags: [health] + operationId: check_token + security: + - bearer-token: [] + responses: + '200': + description: ok + content: + application/json: + schema: + type: string + /alias: + get: + tags: [health] + operationId: ping_alias + security: + - alias-key: [] + responses: + '200': + description: ok + content: + application/json: + schema: + type: string + /collide: + get: + tags: [health] + operationId: ping_collide + security: + - api_key_2: [] + responses: + '200': + description: ok + content: + application/json: + schema: + type: string + /plain: + get: + tags: [health] + operationId: ping_plain + security: + - api_key: [] + responses: + '200': + description: ok + content: + application/json: + schema: + type: string +components: + securitySchemes: + # Sanitizes to "api_key", but that suffix and "api_key_2" are kept by the + # already-valid names below, so this is bumped to "api_key_3". + api-key: + type: apiKey + name: X-API-Key + in: header + # Already a valid identifier even though listed after "api-key", which + # sanitizes to the same suffix: it must keep "api_key" unnumbered (the same + # function name as before the names were sanitized). + api_key: + type: apiKey + name: X-Plain-Key + in: header + petstore.auth: + type: oauth2 + flows: + implicit: + authorizationUrl: https://example.com/api/oauth/dialog + scopes: + write:pets: modify pets in your account + read:pets: read your pets + basic-auth: + type: http + scheme: basic + # Already a valid identifier and literally "api_key_2": it claims that + # suffix, so the colliding "api-key" and "api.key" skip it. + api_key_2: + type: apiKey + name: X-Third-Key + in: header + # Sanitizes to the same suffix as "api-key"; "api_key" through "api_key_3" + # are taken, so this must get "api_key_4". + api.key: + type: apiKey + name: X-API-Key-Alt + in: header + # Starts with a digit; the info_from_/validate_scope_ prefixes keep it a valid + # Python identifier without an extra leading '_'. + 1st-key: + type: apiKey + name: X-First-Key + in: header + bearer-token: + type: http + scheme: bearer + # User-provided x-apikeyInfoFunc must be preserved, not overwritten. + alias-key: + type: apiKey + name: X-Alias-Key + in: header + x-apikeyInfoFunc: openapi_server.controllers.security_controller.info_from_api_key