Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[JAVA/Kotlin] Bug invalid code for ApiClient generated when ONLY using an http authentication which is neither basic nor bearer #14876

Closed
tiffmaelite opened this issue Mar 3, 2023 · 0 comments · Fixed by #15220

Comments

@tiffmaelite
Copy link
Contributor

tiffmaelite commented Mar 3, 2023

Description

Depending on the defined authentication scheme, then the generated ApiClient class does not compile. Indeed, it may incorrectly create a new HttpBearerAuth object, but correctly detect that this class does not need to be imported.

It was detected when trying to generate an OpenApi client using kerberos negotiation with type http and scheme negotiate together together with with bearer auth and generator java with library resttemplate. Additionally to the "missing" HttpBearerAuth import, the wrong values were set into the created authentication object. With some other java generators, the class was imported but there was still an incorrectly created HttpBearerAuth object. Because the root cause of both issues is the same, I group them together here.

openapi-generator version

Initially observed with version 4.2.3, but examples below reproduced with version 6.4.0, generator java with library resttemplate, but I think that it also affects following libraries: java/apache-httpclient, java/feign, java/okhttp-gson, java/resteasy, java/retrofit, java/retrofit2, java/vertx, java/webclient, kotlin-client/jvm-ktor, kotlin-client/multiplatform

Related issues/PRs

Could not find anything related existing.
Opened #15220

Suggest a fix/enhancement

When searching for the root cause of the observed behavior, I noticed the following parts of ApiClient mustache templates:

  • for java/apache-httpclient:
{{#hasHttpBearerMethods}}
import {{invokerPackage}}.auth.HttpBearerAuth;
{{/hasHttpBearerMethods}}
...
{{#isBasic}}{{#isBasicBasic}}
    authentications.put("{{name}}", new HttpBasicAuth());{{/isBasicBasic}}{{^isBasicBasic}}
    authentications.put("{{name}}", new HttpBearerAuth("{{scheme}}"));{{/isBasicBasic}}{{/isBasic}}
  • for java/feign:
import {{invokerPackage}}.auth.HttpBearerAuth;
...
{{#isBasic}}
      {{#isBasicBasic}}
        auth = new HttpBasicAuth();
      {{/isBasicBasic}}
      {{^isBasicBasic}}
        auth = new HttpBearerAuth("{{scheme}}");
      {{/isBasicBasic}}
      {{/isBasic}}
  • for java/vertx
import {{invokerPackage}}.auth.HttpBearerAuth;
...
{{#isBasic}}{{#isBasicBasic}}
        authentications.put("{{name}}", new HttpBasicAuth());{{/isBasicBasic}}{{^isBasicBasic}}
        authentications.put("{{name}}", new HttpBearerAuth("{{scheme}}"));{{/isBasicBasic}}{{/isBasic}}
...
{{#isBasic}}{{#isBasicBasic}}

        public void add{{#lambda.titlecase}}{{name}}{{/lambda.titlecase}}Authentication(String username, String password) {
            HttpBasicAuth auth = new HttpBasicAuth();
            auth.setUsername(username);
            auth.setPassword(password);
            authentications.put("{{name}}", auth);
        }{{/isBasicBasic}}{{^isBasicBasic}}

        public void add{{#lambda.titlecase}}{{name}}{{/lambda.titlecase}}Authentication(String bearerToken) {
           HttpBearerAuth auth = new
           HttpBearerAuth("{{scheme}}");
           auth.setBearerToken(bearerToken);
           authentications.put("{{name}}", auth);
        }{{/isBasicBasic}}{{/isBasic}}
...
{{#isBasic}}{{#isBasicBasic}}

        public static AuthInfo for{{#lambda.titlecase}}{{name}}{{/lambda.titlecase}}(String username, String password) {
            AuthInfo authInfo = new AuthInfo();
            authInfo.add{{#lambda.titlecase}}{{name}}{{/lambda.titlecase}}Authentication(username, password);
            return authInfo;
        }{{/isBasicBasic}}{{^isBasicBasic}}

        public static AuthInfo for{{#lambda.titlecase}}{{name}}{{/lambda.titlecase}}Authentication(String bearerToken) {
            AuthInfo authInfo = new AuthInfo();
            authInfo.add{{#lambda.titlecase}}{{name}}{{/lambda.titlecase}}Authentication(bearerToken);
            return authInfo;
        }{{/isBasicBasic}}{{/isBasic}}

However, the implementation behind hasHttpBearerMethods is as follows:

    /**
     * Returns true if the specified OAS model has at least one operation with HTTP bearer.
     *
     * @param authMethods List of auth methods.
     * @return True if at least one operation has HTTP bearer security scheme defined
     */
    public static boolean hasHttpBearerMethods(List<CodegenSecurity> authMethods) {
        if (authMethods != null && !authMethods.isEmpty()) {
            for (CodegenSecurity cs : authMethods) {
                if (Boolean.TRUE.equals(cs.isBasicBearer)) {
                    return true;
                }
            }
        }
        return false;
    }

As a result, I think that it would be more appropriate for the ApiClient mustache templates to contain:

  • for java/apache-httpclient:
{{#hasHttpBearerMethods}}
import {{invokerPackage}}.auth.HttpBearerAuth;
{{/hasHttpBearerMethods}}
...
{{#isBasic}}{{#isBasicBasic}}
    authentications.put("{{name}}", new HttpBasicAuth());{{/isBasicBasic}}{{#isBasicBearer}}
    authentications.put("{{name}}", new HttpBearerAuth("{{scheme}}"));{{/isBasicBearer}}{{/isBasic}}
  • for java/feign:
import {{invokerPackage}}.auth.HttpBearerAuth;
...
     {{#isBasic}}
      {{#isBasicBasic}}
        auth = new HttpBasicAuth();
      {{/isBasicBasic}}
      {{#isBasicBearer}}
        auth = new HttpBearerAuth("{{scheme}}");
      {{/isBasicBearer}}
      {{/isBasic}}
  • for java/vertx:
import {{invokerPackage}}.auth.HttpBearerAuth;
...
{{#isBasic}}{{#isBasicBasic}}
        authentications.put("{{name}}", new HttpBasicAuth());{{/isBasicBasic}}{{#isBasicBearer}}
        authentications.put("{{name}}", new HttpBearerAuth("{{scheme}}"));{{/isBasicBearer}}{{/isBasic}}
...
{{#isBasic}}{{#isBasicBasic}}

        public void add{{#lambda.titlecase}}{{name}}{{/lambda.titlecase}}Authentication(String username, String password) {
            HttpBasicAuth auth = new HttpBasicAuth();
            auth.setUsername(username);
            auth.setPassword(password);
            authentications.put("{{name}}", auth);
        }{{/isBasicBasic}}{{#isBasicBearer}}

        public void add{{#lambda.titlecase}}{{name}}{{/lambda.titlecase}}Authentication(String bearerToken) {
           HttpBearerAuth auth = new
           HttpBearerAuth("{{scheme}}");
           auth.setBearerToken(bearerToken);
           authentications.put("{{name}}", auth);
        }{{/isBasicBearer}}{{/isBasic}}
...
{{#isBasic}}{{#isBasicBasic}}

        public static AuthInfo for{{#lambda.titlecase}}{{name}}{{/lambda.titlecase}}(String username, String password) {
            AuthInfo authInfo = new AuthInfo();
            authInfo.add{{#lambda.titlecase}}{{name}}{{/lambda.titlecase}}Authentication(username, password);
            return authInfo;
        }{{/isBasicBasic}}{{#isBasicBearer}}

        public static AuthInfo for{{#lambda.titlecase}}{{name}}{{/lambda.titlecase}}Authentication(String bearerToken) {
            AuthInfo authInfo = new AuthInfo();
            authInfo.add{{#lambda.titlecase}}{{name}}{{/lambda.titlecase}}Authentication(bearerToken);
            return authInfo;
        }{{/isBasicBearer}}{{/isBasic}}

or for hasHttpBearerMethods to be defined as follows:

    /**
     * Returns true if the specified OAS model has at least one operation with HTTP bearer.
     *
     * @param authMethods List of auth methods.
     * @return True if at least one operation has HTTP bearer security scheme defined
     */
    public static boolean hasHttpBearerMethods(List<CodegenSecurity> authMethods) {
        if (authMethods != null && !authMethods.isEmpty()) {
            for (CodegenSecurity cs : authMethods) {
                if (Boolean.TRUE.equals(cs.isBasic) && Boolean.FALSE.equals(cs.isBasicBasic)) {
                    return true;
                }
            }
        }
        return false;
    }

The PR linked to this issue proposes the first option as correct fix, since it avoids creating HttpBearerAuth objects for unknown types of authentications.

Command line used for generation

The below examples were built with openapi-generator-maven-plugin and following maven plugin configuration:

    <build>
        <plugins>
            <plugin>
                <groupId>org.openapitools</groupId>
                <artifactId>openapi-generator-maven-plugin</artifactId>
                <version>6.4.0</version>
                <executions>
                    <execution>
                        <id>client</id>
                        <phase>generate-sources</phase>
                        <goals>
                            <goal>generate</goal>
                        </goals>
                        <configuration>
                            <inputSpec>${project.basedir}/src/main/resources/openapi.yaml</inputSpec>
                            <apiPackage>com.example.commons.myapi.openapi.api</apiPackage>
                            <invokerPackage>com.example.commons.myapi.openapi.client</invokerPackage>
                            <modelPackage>com.example.commons.myapi.openapi.model</modelPackage>
                            <generatorName>java</generatorName>
                            <library>resttemplate</library>
                            <generateApiTests>false</generateApiTests>
                            <generateModelTests>false</generateModelTests>
                            <artifactId>myapi-openapi-client</artifactId>
                            <artifactVersion>${project.version}</artifactVersion>
                            <output>${project.basedir}/myapi-openapi-client</output>
                            <groupId>com.example.commons</groupId>
                            <configOptions>
                                <dateLibrary>java8</dateLibrary>
                                <java8>true</java8>
                                <sourceFolder>target/generated-sources</sourceFolder>
                                <parentArtifactId>myapi-openapi</parentArtifactId>
                                <parentGroupId>com.juliusbaer.gcmt.commons</parentGroupId>
                                <parentVersion>${project.version}</parentVersion>
                            </configOptions>
                        </configuration>
                    </execution>
                    <execution>
                        <id>mock</id>
                        <phase>generate-sources</phase>
                        <goals>
                            <goal>generate</goal>
                        </goals>
                        <configuration>
                            <inputSpec>${project.basedir}/src/main/resources/openapi.yaml</inputSpec>
                            <apiPackage>com.example.commons.myapi.openapi.mock.api</apiPackage>
                            <invokerPackage>com.example.commons.myapi.openapi.mock.client</invokerPackage>
                            <modelPackage>com.example.commons.myapi.openapi.model</modelPackage>
                            <generatorName>spring</generatorName>
                            <library>spring-boot</library>
                            <generateApiDocumentation>false</generateApiDocumentation>
                            <generateModelDocumentation>false</generateModelDocumentation>
                            <generateModels>false</generateModels>
                            <generateApiTests>false</generateApiTests>
                            <generateModelTests>false</generateModelTests>
                            <artifactId>myapi-openapi-mock</artifactId>
                            <artifactVersion>${project.version}</artifactVersion>
                            <output>${project.basedir}/myapi-openapi-mock</output>
                            <groupId>com.example.commons</groupId>
                            <configOptions>
                                <dateLibrary>java8</dateLibrary>
                                <java8>true</java8>
                                <sourceFolder>target/generated-sources</sourceFolder>
                                <parentArtifactId>myapi-openapi</parentArtifactId>
                                <parentGroupId>com.juliusbaer.gcmt.commons</parentGroupId>
                                <parentVersion>${project.version}</parentVersion>
                            </configOptions>
                        </configuration>
                    </execution>
                </executions>
            </plugin>
        </plugins>
    </build>
Steps to reproduce
Scenario 1: using both bearerAuth and kerberosNegotiation

Side-note: According to https://github.com/OAI/OpenAPI-Specification/blob/main/versions/3.0.3.md#securitySchemeObject any scheme listed on https://www.iana.org/assignments/http-authschemes/http-authschemes.xhtml is supported, which includes scheme "negotiate", which is widely used by corporate APIs. But this is not yet supported by openapi-generator with java libraries, so I would expect it to be simply ignored in this case.

OpenAPI declaration file content

openapi: 3.0.3
security:
  - kerberosNegotiation: []
  - bearerAuth: []
components:
  securitySchemes:
    kerberosNegotiation:
      type: http
      scheme: negotiate
    bearerAuth:
      type: http
      scheme: bearer

Observed result

...
import com.example.commons.myapi.openapi.client.auth.Authentication;
import com.example.commons.myapi.openapi.client.auth.HttpBearerAuth;

@javax.annotation.Generated(value = "org.openapitools.codegen.languages.JavaClientCodegen", date = "2023-03-03T15:27:28.078+01:00[Europe/Berlin]")
@Component("com.example.commons.myapi.openapi.client.ApiClient")
public class ApiClient extends JavaTimeFormatter {
...
    private Map<String, Authentication> authentications;
...
    protected void init() {
...
        authentications = new HashMap<String, Authentication>();
        authentications.put("kerberosNegotiation", new HttpBearerAuth("negotiate"));
        authentications.put("bearerAuth", new HttpBearerAuth("bearer"));
        // Prevent the authentications from being modified.
        authentications = Collections.unmodifiableMap(authentications);
    }
}

Expected result

...
import com.example.commons.myapi.openapi.client.auth.Authentication;
import com.example.commons.myapi.openapi.client.auth.HttpBearerAuth;

@javax.annotation.Generated(value = "org.openapitools.codegen.languages.JavaClientCodegen", date = "2023-03-03T15:27:28.078+01:00[Europe/Berlin]")
@Component("com.example.commons.myapi.openapi.client.ApiClient")
public class ApiClient extends JavaTimeFormatter {
...
    private Map<String, Authentication> authentications;
...
    protected void init() {
...
        authentications = new HashMap<String, Authentication>();
        authentications.put("bearerAuth", new HttpBearerAuth("bearer"));
        // Prevent the authentications from being modified.
        authentications = Collections.unmodifiableMap(authentications);
    }
}
Scenario 2: using only bearerAuth

OpenAPI declaration file content

openapi: 3.0.3
security:
  - bearerAuth: []
components:
  securitySchemes:
    bearerAuth:
      type: http
      scheme: bearer

Observed result

...
import com.example.commons.myapi.openapi.client.auth.Authentication;
import com.example.commons.myapi.openapi.client.auth.HttpBearerAuth;

@javax.annotation.Generated(value = "org.openapitools.codegen.languages.JavaClientCodegen", date = "2023-03-03T15:31:17.066+01:00[Europe/Berlin]")
@Component("com.example.commons.myapi.openapi.client.ApiClient")
public class ApiClient extends JavaTimeFormatter {
...
    private Map<String, Authentication> authentications;
...
    protected void init() {
...
        authentications = new HashMap<String, Authentication>();
        authentications.put("bearerAuth", new HttpBearerAuth("bearer"));
        // Prevent the authentications from being modified.
        authentications = Collections.unmodifiableMap(authentications);
    }
}

Expected result same as observed

...
import com.example.commons.myapi.openapi.client.auth.Authentication;
import com.example.commons.myapi.openapi.client.auth.HttpBearerAuth;

@javax.annotation.Generated(value = "org.openapitools.codegen.languages.JavaClientCodegen", date = "2023-03-03T15:31:17.066+01:00[Europe/Berlin]")
@Component("com.example.commons.myapi.openapi.client.ApiClient")
public class ApiClient extends JavaTimeFormatter {
...
    private Map<String, Authentication> authentications;
...
    protected void init() {
...
        authentications = new HashMap<String, Authentication>();
        authentications.put("bearerAuth", new HttpBearerAuth("bearer"));
        // Prevent the authentications from being modified.
        authentications = Collections.unmodifiableMap(authentications);
    }
}
Scenario 3: using only kerberosNegotiation

OpenAPI declaration file content

openapi: 3.0.3
security:
  - kerberosNegotiation: []
components:
  securitySchemes:
    kerberosNegotiation:
      type: http
      scheme: negotiate

Observed

import com.example.commons.myapi.openapi.client.auth.Authentication;

@javax.annotation.Generated(value = "org.openapitools.codegen.languages.JavaClientCodegen", date = "2023-03-03T15:42:11.398+01:00[Europe/Berlin]")
@Component("com.example.commons.myapi.openapi.client.ApiClient")
public class ApiClient extends JavaTimeFormatter {
...
    private Map<String, Authentication> authentications;
...
    protected void init() {
...
        authentications = new HashMap<String, Authentication>();
        authentications.put("kerberosNegotiation", new HttpBearerAuth("negotiate"));
        // Prevent the authentications from being modified.
        authentications = Collections.unmodifiableMap(authentications);
    }
}
[ERROR] COMPILATION ERROR : 
[INFO] -------------------------------------------------------------
[ERROR] /C:/dev/myapi-example/myapi-openapi/myapi-openapi-client/target/generated-sources/com/example/commons/myapi/openapi/client/ApiClient.java:[112,56] cannot find symbol
  symbol:   class HttpBearerAuth
  location: class com.example.commons.myapi.openapi.client.ApiClient

Expected

import com.example.commons.myapi.openapi.client.auth.Authentication;

@javax.annotation.Generated(value = "org.openapitools.codegen.languages.JavaClientCodegen", date = "2023-03-03T15:42:11.398+01:00[Europe/Berlin]")
@Component("com.example.commons.myapi.openapi.client.ApiClient")
public class ApiClient extends JavaTimeFormatter {
...
    private Map<String, Authentication> authentications;
...
    protected void init() {
...
        authentications = new HashMap<String, Authentication>();
        // Prevent the authentications from being modified.
        authentications = Collections.unmodifiableMap(authentications);
    }
}
Scenario 4: using only signatureAuth

Side-note: I could not find it listed on https://www.iana.org/assignments/http-authschemes/http-authschemes.xhtml but the open api generator documentation and tests contain mention of this http authentication type

OpenAPI declaration file content

openapi: 3.0.3
security:
  - signatureAuth: []
components:
  securitySchemes:
    signatureAuth:
      type: http
      scheme: signature

Observed result

import com.example.commons.myapi.openapi.client.auth.Authentication;

@javax.annotation.Generated(value = "org.openapitools.codegen.languages.JavaClientCodegen", date = "2023-03-03T15:42:11.398+01:00[Europe/Berlin]")
@Component("com.example.commons.myapi.openapi.client.ApiClient")
public class ApiClient extends JavaTimeFormatter {
...
    private Map<String, Authentication> authentications;
...
    protected void init() {
...
        authentications = new HashMap<String, Authentication>();
        authentications.put("signatureAuth", new HttpBearerAuth("signature"));
        // Prevent the authentications from being modified.
        authentications = Collections.unmodifiableMap(authentications);
    }
}
[ERROR] COMPILATION ERROR : 
[INFO] -------------------------------------------------------------
[ERROR] /C:/dev/myapi-example/myapi-openapi/myapi-openapi-client/target/generated-sources/com/example/commons/myapi/openapi/client/ApiClient.java:[112,50] cannot find symbol
  symbol:   class HttpBearerAuth
  location: class com.example.commons.myapi.openapi.client.ApiClient

Expected result

I'm not really sure of what should be expected in this case.
maybe the same with the missing import added:

import com.example.commons.myapi.openapi.client.auth.Authentication;
import com.example.commons.myapi.openapi.client.auth.HttpBearerAuth;

@javax.annotation.Generated(value = "org.openapitools.codegen.languages.JavaClientCodegen", date = "2023-03-03T15:42:11.398+01:00[Europe/Berlin]")
@Component("com.example.commons.myapi.openapi.client.ApiClient")
public class ApiClient extends JavaTimeFormatter {
...
    private Map<String, Authentication> authentications;
...
    protected void init() {
...
        authentications = new HashMap<String, Authentication>();
        authentications.put("signatureAuth", new HttpBearerAuth("signature"));
        // Prevent the authentications from being modified.
        authentications = Collections.unmodifiableMap(authentications);
    }
}

or rather without the authentication listed at all. Is it still a missing feature for the resttemplate library? (I see that a class called HttpSignatureAuth exists for generators jersey2 and jersey3)? If so, I would expect the following code to be generated:

import com.example.commons.myapi.openapi.client.auth.Authentication;

@javax.annotation.Generated(value = "org.openapitools.codegen.languages.JavaClientCodegen", date = "2023-03-03T15:42:11.398+01:00[Europe/Berlin]")
@Component("com.example.commons.myapi.openapi.client.ApiClient")
public class ApiClient extends JavaTimeFormatter {
...
    private Map<String, Authentication> authentications;
...
    protected void init() {
...
        authentications = new HashMap<String, Authentication>();
        // Prevent the authentications from being modified.
        authentications = Collections.unmodifiableMap(authentications);
    }
}
@tiffmaelite tiffmaelite changed the title [JAVA/Kotlin] Bug invalid code for ApiClient generated when using an http authentication which is neither basic nor bearer [JAVA/Kotlin] Bug invalid code for ApiClient generated when ONLY using an http authentication which is neither basic nor bearer Mar 3, 2023
wing328 pushed a commit that referenced this issue May 22, 2023
…ttpBearerAuth (or HttpBasicAuth) for other http auth methods (such as http signature auth or custom schemes) (#15220)

* remove http signature from test yaml when not supported

* do not use HttpBearerAuth for signature auth or other unsupported http auth method

ignore unsupported http auth method unless generated code would not compile (in which case, an exception is thrown)

* [Java] fix use of isBasic condition

* [kotlin] fix use of isBasic condition
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant