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] support for bearer authentication #457

Closed
lorenzleutgeb opened this issue Jul 4, 2018 · 10 comments
Closed

[Java] support for bearer authentication #457

lorenzleutgeb opened this issue Jul 4, 2018 · 10 comments

Comments

@lorenzleutgeb
Copy link

lorenzleutgeb commented Jul 4, 2018

Please see swagger-api/swagger-codegen-generators#113

The output that OpenAPI generator 020883f produces suffers from the same defect. However I did not look into your codebase. I understand that referring to the Swagger Codegen project might not be the most sensible thing to do, and if you rather do not want to look at it, I could look for the cause in this codebase.

@jmini
Copy link
Member

jmini commented Jul 4, 2018

Welcome here!

Thank you for this issue.

There is no problem to mention Swagger-Codegen. The codebase are similar, Swagger-Codegen v3 and OpenAPI-Generator are both evolutions of Swagger-Codegen 2.x.
My personal summary of the story is that the opinions about next steps went apart...


You have located the correct section in the template. In this code base it is here:

authentications = new HashMap<String, Authentication>();{{#authMethods}}{{#isBasic}}
authentications.put("{{name}}", new HttpBasicAuth());{{/isBasic}}{{#isApiKey}}
authentications.put("{{name}}", new ApiKeyAuth({{#isKeyInHeader}}"header"{{/isKeyInHeader}}{{^isKeyInHeader}}"query"{{/isKeyInHeader}}, "{{keyParamName}}"));{{/isApiKey}}{{#isOAuth}}
authentications.put("{{name}}", new OAuth());{{/isOAuth}}{{/authMethods}}

My take is that there is no support for bearer yet. Is this a new feature of OpenAPI 3?


Could you please post in this issue a full OAS3 example, you can adapt the very ping.yaml example, where you put the values for bearer as they should be for your case.

From your post I understood that you generate a java client, but can you post the command you are using (cli, maven, gradle or java integration of the core)? The base library you are using is the important point...

@jmini jmini changed the title DefaultCodeGen is too aggressive in assuming Basic authentication [Java] support for bearer authentication Jul 4, 2018
@lorenzleutgeb
Copy link
Author

lorenzleutgeb commented Jul 4, 2018

From your post I understood that you generate a java client, but can you post the command you are using (cli, maven, gradle or java integration of the core)?

I cloned/checked out 020883f and ran mvn clean package. With the resulting JAR I do

java -jar openapi-generator-cli.jar generate -l java -i api.yml -o jclient

I do not understand why you categorized this to affect the Java compilation target specifically. From my limited understanding of the codebase here, it seems that the error is actually in the frontend, i.e. in the shared Java code that underpins generation and the issue therefore affects all targets. In the issue for the Swagger implementation I mention that I think the error is in their io.swagger.codegen.languages.DefaultCodegen class (lines highlighted) which translates to your org.openapitools.codegen.DefaultCodegen (lines highlighted) in straight forward ways. In the OpenAPI Tools codebase, the assumption that HTTP authentication implies HTTP authentication using the basic authentication scheme is valid just like in the Swagger codebase, so the root causes align.

Could you please post in this issue a full OAS3 example, you can adapt the very ping.yaml example, where you put the values for bearer as they should be for your case.

openapi: 3.0.1
info:
  title: ping test
  version: '1.0'
servers:
  - url: 'http://localhost:8000/'
paths:
  /ping:
    get:
      operationId: pingGet
      responses:
        '201':
          description: OK

# Modifications are below this line. Above is ping.yaml.

components:
  securitySchemes:
    bearer:
      type: http
      scheme: bearer

security:
  - bearer: []

@jmini
Copy link
Member

jmini commented Jul 4, 2018

You are right, I did not investigate it yet, but one part of the fix will be in the Codegen-layer (OAS3-Feature label) and the java template needs to be adjusted too.

Or I have missed something and I will be happy to change that.

@lorenzleutgeb
Copy link
Author

I see. Thanks.

@chrisesharp
Copy link

chrisesharp commented Jul 5, 2018

+1 on this. I came across exactly the same issue this week trying to generate a client with a Bearer JWT token:

securitySchemes:
    JWT_auth:
      type: http
      description: JWT authentication needed
      scheme: bearer
      bearerFormat: JWT

@jason-cohen
Copy link
Contributor

jason-cohen commented Aug 23, 2018

@jmini has there been any further development into this issue?

Like you pointed out, it's not exactly an issue with the mustache templates. It primarily arises from the DefaultGenerator assuming HTTP security type implies BASIC Auth, without any further investigation as to the scheme provided.

A simple minimal solution might be to replace (DefaultCodegen.java line 2953):

} else if (SecurityScheme.Type.HTTP.equals(securityScheme.getType())) {
    cs.isKeyInHeader = cs.isKeyInQuery = cs.isKeyInCookie = cs.isApiKey = cs.isOAuth = false;
    cs.isBasic = true;
}

with

} else if (SecurityScheme.Type.HTTP.equals(securityScheme.getType())) {
    cs.isKeyInHeader = cs.isKeyInQuery = cs.isKeyInCookie = cs.isApiKey = cs.isBasic = cs.isOAuth = false;
    if ("basic".equals(securityScheme.getScheme())) cs.isBasic = true;
    if ("bearer".equals(securityScheme.getScheme())) cs.isOAuth = true;
}

However I haven't investigated as to the implications of having isOAuth true without the other oauth fields set.

@lorenzleutgeb
Copy link
Author

@jason-cohen Just hijacking the OAuth security scheme is a bad idea. For example, the spec requires an OAuth flow to be set on security schemes of this type, which is something that cannot be inferred (and might even not apply) in the case of a different HTTP authentication scheme. What you suggest as a solution here is a workaround that will immediately fail as soon as someone wants to use a different scheme than Bearer... I strongly urge @jmini to not further follow this proposal.

@jason-cohen
Copy link
Contributor

@lorenzleutgeb Very good point. I agree, hijacking the OAuth scheme is a bad idea as it couples them, when the very point of the http-bearer scheme was to have an authorization bearer header not coupled with OAuth.

Maybe a more viable option would be to add a new field isBearer that could be used in the templates in a manner similar to the isBasic.

@davidwcarlson
Copy link
Contributor

I am running into the same issue with BearerAuth. It looks like the recent commit 80ca67c took care of the changes to DefaultGenerator to correctly set the existing BasicBasic and BasicBearer variables.

In response to @jason-cohen's concerns about highjacking OAuth, my pull request #1930 adds a new BearerAuth object and uses it if the BasicBearer is set to true.

davidwcarlson added a commit to davidwcarlson/openapi-generator that referenced this issue Jan 24, 2019
See OpenAPITools#457
Also OpenAPITools#1446 for typescript, OpenAPITools#1577 for python

Specs defined as follows currently generate BasicAuth and send "Authorization: Basic [base64Encode(username + ":" + password)]"
components:
  securitySchemes:
    bearer:
      type: http
      scheme: bearer

This change will generate an OAuth header, which will send a "Authorization: Bearer [accessToken]"
This is a smaller, less-impactful change than introducing a BearerAuth object, but this change doesn't support scheme values other than bearer
See also OpenAPITools#1930
davidwcarlson added a commit to davidwcarlson/openapi-generator that referenced this issue Jan 24, 2019
See OpenAPITools#457
Also OpenAPITools#1446 for typescript, OpenAPITools#1577 for python

Specs defined as follows currently generate BasicAuth and send an "Authorization: Basic [base64Encode(username + ":" + password)]" header
    components:
      securitySchemes:
        bearer:
          type: http
          scheme: bearer

This change will generate code which uses a new HttpBearerAuth class, which will send a "Authorization: [scheme] [accessToken]" header.
This change is slightly larger and more impactful than simply using OAuth for bearerBearer, but it allows for scheme values other than bearer.

This fix was enabled by the recent commit of OpenAPITools@80ca67c

This PR is an alternative to OpenAPITools#1972
@wing328 wing328 closed this as completed in ef7b28d Feb 9, 2019
@wing328
Copy link
Member

wing328 commented Feb 9, 2019

#1972 by @davidwcarlson has been merged into master. Please give it a try with the latest master.

@wing328 wing328 added this to the 4.0.0 milestone Feb 9, 2019
A-Joshi pushed a commit to ihsmarkitoss/openapi-generator that referenced this issue Feb 27, 2019
* fix OpenAPITools#457 by introducing an HttpBearerAuth object

See OpenAPITools#457
Also OpenAPITools#1446 for typescript, OpenAPITools#1577 for python

Specs defined as follows currently generate BasicAuth and send an "Authorization: Basic [base64Encode(username + ":" + password)]" header
    components:
      securitySchemes:
        bearer:
          type: http
          scheme: bearer

This change will generate code which uses a new HttpBearerAuth class, which will send a "Authorization: [scheme] [accessToken]" header.
This change is slightly larger and more impactful than simply using OAuth for bearerBearer, but it allows for scheme values other than bearer.

This fix was enabled by the recent commit of OpenAPITools@80ca67c

This PR is an alternative to OpenAPITools#1972

* update petstore samples

* Update HttpBearerAuth mustache templates and samples

* correct the expected number of generated java client files

* update the retrofit2 HttpBearerAuth template and samples

* Add resttemplate-specific HttpBearerAuth mustache and samples

* add vertx-specific HttpBearerAuth template and samples

* add java webclient-specific HttpBearerAuth template and samples
therve pushed a commit to DataDog/datadog-api-client-java that referenced this issue Mar 23, 2021
* fix #457 by introducing an HttpBearerAuth object

See OpenAPITools/openapi-generator#457
Also OpenAPITools/openapi-generator#1446 for typescript, OpenAPITools/openapi-generator#1577 for python

Specs defined as follows currently generate BasicAuth and send an "Authorization: Basic [base64Encode(username + ":" + password)]" header
    components:
      securitySchemes:
        bearer:
          type: http
          scheme: bearer

This change will generate code which uses a new HttpBearerAuth class, which will send a "Authorization: [scheme] [accessToken]" header.
This change is slightly larger and more impactful than simply using OAuth for bearerBearer, but it allows for scheme values other than bearer.

This fix was enabled by the recent commit of OpenAPITools/openapi-generator@80ca67c

This PR is an alternative to OpenAPITools/openapi-generator#1972

* update petstore samples

* Update HttpBearerAuth mustache templates and samples

* correct the expected number of generated java client files

* update the retrofit2 HttpBearerAuth template and samples

* Add resttemplate-specific HttpBearerAuth mustache and samples

* add vertx-specific HttpBearerAuth template and samples

* add java webclient-specific HttpBearerAuth template and samples
api-clients-generation-pipeline bot pushed a commit to DataDog/datadog-api-client-java that referenced this issue Sep 8, 2021
* fix #457 by introducing an HttpBearerAuth object

See OpenAPITools/openapi-generator#457
Also OpenAPITools/openapi-generator#1446 for typescript, OpenAPITools/openapi-generator#1577 for python

Specs defined as follows currently generate BasicAuth and send an "Authorization: Basic [base64Encode(username + ":" + password)]" header
    components:
      securitySchemes:
        bearer:
          type: http
          scheme: bearer

This change will generate code which uses a new HttpBearerAuth class, which will send a "Authorization: [scheme] [accessToken]" header.
This change is slightly larger and more impactful than simply using OAuth for bearerBearer, but it allows for scheme values other than bearer.

This fix was enabled by the recent commit of OpenAPITools/openapi-generator@80ca67c

This PR is an alternative to OpenAPITools/openapi-generator#1972

* update petstore samples

* Update HttpBearerAuth mustache templates and samples

* correct the expected number of generated java client files

* update the retrofit2 HttpBearerAuth template and samples

* Add resttemplate-specific HttpBearerAuth mustache and samples

* add vertx-specific HttpBearerAuth template and samples

* add java webclient-specific HttpBearerAuth template and samples
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants