Initial OIDC support by jtama · Pull Request #15417 · OpenAPITools/openapi-generator

jtama · 2023-05-04T15:48:46Z

Adds initial support for OpenID Connect security.

PR checklist

Read the contribution guidelines.
Pull Request title clearly describes the work in the pull request and Pull Request description provides details about how to validate the work. Missing information here may result in delayed response from the community.
Run the following to build the project and update samples:
```
./mvnw clean package 
./bin/generate-samples.sh
./bin/utils/export_docs_generators.sh
```
Commit all changed files.
This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master.
These must match the expectations made by your contribution.
You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example ./bin/generate-samples.sh bin/configs/java*.
For Windows users, please run the script in Git BASH.
In case you are adding a new generator, run the following additional script :
```
./bin/utils/ensure-up-to-date.sh
```
Commit all changed files.
File the PR against the correct branch: master (6.3.0) (minor release - breaking changes with fallbacks), 7.0.x (breaking changes without fallbacks)
If your PR is targeting a particular programming language, @mention the technical committee members, so they are more likely to review the pull request.

wing328 · 2023-05-09T08:32:34Z

@jtama thanks for the PR. There's another PR that adds OpenID support. Can you please take a look at that to see if that meets your requirement?

#15407

jtama · 2023-05-09T10:00:33Z

I don't see any scope taken in account for oidc, which means they wont be able to generate @RolesAllowed annotations, or in my dedicate case, won't be able to generate proper documentation...

wing328 · 2023-05-09T12:20:49Z

what about we merge #15407 and they you can submit a PR on top to add support for scope?

jtama · 2023-05-09T12:22:04Z

Yes I can rebase after it has been merged.

wing328 · 2023-05-09T16:56:20Z

Just merged #15407.

Please rebase this PR when you've time.

Changes OIDC to OpenIdConnect Adds generated files

ahmednfwela

there were some OIDC shortcuts

wing328 · 2023-05-10T06:05:26Z

[ERROR] COMPILATION ERROR : 
[ERROR] /home/circleci/OpenAPITools/openapi-generator/modules/openapi-generator/src/main/java/org/openapitools/codegen/CodegenSecurity.java:[47,19] variable openIdConnectUrl is already defined in class org.openapitools.codegen.CodegenSecurity
[ERROR] /home/circleci/OpenAPITools/openapi-generator/modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java:[5307,19] cannot find symbol
  symbol:   variable isOpenId
  location: variable cs of type org.openapitools.codegen.CodegenSecurity
[ERROR] /home/circleci/OpenAPITools/openapi-generator/modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java:[5374,74] cannot find symbol
  symbol:   variable isOpenId
  location: variable cs of type org.openapitools.codegen.CodegenSecurity
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.10.1:compile (default-compile) on project openapi-generator: Compilation failure: Compilation failure: 
[ERROR] /home/circleci/OpenAPITools/openapi-generator/modules/openapi-generator/src/main/java/org/openapitools/codegen/CodegenSecurity.java:[47,19] variable openIdConnectUrl is already defined in class org.openapitools.codegen.CodegenSecurity
[ERROR] /home/circleci/OpenAPITools/openapi-generator/modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java:[5307,19] cannot find symbol
[ERROR]   symbol:   variable isOpenId
[ERROR]   location: variable cs of type org.openapitools.codegen.CodegenSecurity
[ERROR] /home/circleci/OpenAPITools/openapi-generator/modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java:[5374,74] cannot find symbol
[ERROR]   symbol:   variable isOpenId
[ERROR]   location: variable cs of type org.openapitools.codegen.CodegenSecurity
[ERROR] -> [Help 1]
[ERROR] 
[ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.
[ERROR] Re-run Maven using the -X switch to enable full debug logging.
[ERROR] 
[ERROR] For more information about the errors and possible solutions, please read the following articles:
[ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoFailureException
[ERROR] 
[ERROR] After correcting the problems, you can resume the build with the command

the project wont' build. can you please take a look?

wing328 · 2023-05-10T06:06:50Z

 |BasicAuth|✓|OAS2,OAS3
 |ApiKey|✓|OAS2,OAS3
-|OpenIDConnect|✗|OAS3
+|OpenIDConnect|✓|OAS3


do these generators really support OpenIDConnect as part of the PR?

Nope, but the pr check list said : Commit all changed files..

I wanted to separate preparatory work from generators work, cut into small chuncks ;)

wing328 · 2023-05-10T06:08:15Z

    public String type;
    public String scheme;
-    public Boolean isBasic, isOAuth, isApiKey, isOpenId;
+    public Boolean isBasic, isOAuth, isApiKey, isOpenIdConnect;


Sorry I may not be very clear before.

I think using OpenID is fine to represent the security definition type.

isn't OpenId different from OpenIdConnect ? @wing328

also according to the spec, it's openIdConnect

it's the same thing.

also according to the spec, it's openIdConnect

Yup, so isOAuth should be named isOAuth2 instead if we simply follow that (but it's not the case)

I want to avoid mustache tags with long name if possible. That's why I suggest isOpenID instead.

that's why I simply want to keep the naming introduced in #15407

it looks straightforward to me.

what do you guys think?

I don't mind using OpenIdConnect at all if that's what you guys prefer.

I mean I don't mind, but it might confuse people to have some places with OpenId and others with OpenIdConnect, considering they are 2 different specs.
but an argument can be made here is that OpenId is deprecated, and it defaults to OpenIdConnect anyway.

so both is fine by me

So I corrected all my rebase errors and left isOpenID too ease mustache templates. Just tell me If I should use isOpenIdConnect instead.

wing328 · 2023-05-10T08:51:33Z

looks good. I'll fix https://github.com/OpenAPITools/openapi-generator/actions/runs/4934037685/jobs/8819091825?pr=15417 after merging this PR.

ahmednfwela reviewed May 5, 2023

View reviewed changes

Comment thread modules/openapi-generator/src/main/java/org/openapitools/codegen/CodegenSecurity.java Outdated

jtama added 2 commits May 10, 2023 07:54

Initial OIDC support

1fb7a96

Removes 'println'

63e711d

Changes OIDC to OpenIdConnect Adds generated files

ahmednfwela reviewed May 10, 2023

View reviewed changes

wing328 reviewed May 10, 2023

View reviewed changes

ahmednfwela reviewed May 10, 2023

View reviewed changes

Comment thread modules/openapi-generator/src/main/java/org/openapitools/codegen/CodegenSecurity.java Outdated

wing328 reviewed May 10, 2023

View reviewed changes

fix rebase errors

06fb73f

wing328 merged commit 6e74725 into OpenAPITools:master May 10, 2023

wing328 added Enhancement: Security Feature: Generator labels May 10, 2023

wing328 added this to the 6.6.0 milestone May 10, 2023

Felk mentioned this pull request Jul 15, 2024

[BUG] [Java] Authentication undefined: my-auth-name in ApiClient.java #19168

Open

6 tasks

Uh oh!

Conversation

jtama commented May 4, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR checklist

Uh oh!

Uh oh!

wing328 commented May 9, 2023

Uh oh!

jtama commented May 9, 2023

Uh oh!

wing328 commented May 9, 2023

Uh oh!

jtama commented May 9, 2023

Uh oh!

wing328 commented May 9, 2023

Uh oh!

ahmednfwela left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

wing328 commented May 10, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wing328 May 10, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wing328 commented May 10, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jtama commented May 4, 2023 •

edited

Loading

wing328 May 10, 2023 •

edited

Loading