Add security schemes filter normalizer option#23174
Conversation
|
I found one logging issue and I would like to fix it since I touched this code. The problem is normalizer logs message like |
|
thanks for the draft. please put it on hold for the time being |
|
@wing328 any ideas on how to proceed with this feature? |
|
let me try to open a PR later this week... |
I take this as you don't like the proposal in this PR. This is fine. Just let me know if you need any help from my side. |
|
@limentas when you've time, can you please PM me via Slack to further discuss this? |
|
here is the link to join our slack: https://join.slack.com/t/openapi-generator/shared_invite/zt-36ucx4ybl-jYrN6euoYn6zxXNZdldoZA |
can you please elaborate on this one? I thought this should be done as part of the fix as the goal is remove the security definition (that the users do not want for whatever reasons) from the whole spec as if it has never been defined in the security schema section. |
I do not completely this one. Please elaborate. |
|
I think your PR is almost done so let's push it through the finish line (my approach is too idealistic so I don't think I've time to finish it) |
At this moment I remove security schemas definitions only from components->securitySchemes. But these schemas are referenced in top-level security declaration and also can be referenced in operations. For example in petstore example there is petstore_auth security schema. Let's say new filter removes it. Then this schema is referenced also in POST
If I remember correctly the problem is that FILTER logging is incorrect. |
Add test for it Optimize old filter logic
4f34e3d to
bf6cac9
Compare
|
I have implemented the logic to clean up all references of removed security schemes. |
There was a problem hiding this comment.
3 issues found across 4 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="docs/customization.md">
<violation number="1" location="docs/customization.md:759">
P2: SECURITY_SCHEMES_FILTER is documented as setting x-internal on operations, but the implementation removes security schemes and cleans their references instead.</violation>
</file>
<file name="modules/openapi-generator/src/main/java/org/openapitools/codegen/OpenAPINormalizer.java">
<violation number="1" location="modules/openapi-generator/src/main/java/org/openapitools/codegen/OpenAPINormalizer.java:694">
P1: Security-scheme cleanup is incomplete: it only strips keys from operation security and does not prune empty requirements or clean `openAPI.security`, which can leave dangling refs and alter auth semantics.</violation>
<violation number="2" location="modules/openapi-generator/src/main/java/org/openapitools/codegen/OpenAPINormalizer.java:2509">
P1: `SECURITY_SCHEMES_FILTER` can throw an NPE when a security scheme has no type because `scheme.getType()` is dereferenced without a null check.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
|
Fixed code review remarks |
There was a problem hiding this comment.
1 issue found across 4 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="modules/openapi-generator/src/main/java/org/openapitools/codegen/OpenAPINormalizer.java">
<violation number="1" location="modules/openapi-generator/src/main/java/org/openapitools/codegen/OpenAPINormalizer.java:693">
P2: Security-scheme cleanup skips callback operations, so callback `security` requirements can still reference schemes that were removed from `components.securitySchemes`.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
…-generator into security-schemes-filter
|
Added clean up code for callbacks and webhooks |
There was a problem hiding this comment.
2 issues found across 4 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="modules/openapi-generator/src/main/java/org/openapitools/codegen/OpenAPINormalizer.java">
<violation number="1" location="modules/openapi-generator/src/main/java/org/openapitools/codegen/OpenAPINormalizer.java:354">
P2: Renaming the protected FILTER factory without a compatibility bridge breaks downstream subclass overrides of `createFilter`, silently disabling custom filtering.</violation>
<violation number="2" location="modules/openapi-generator/src/main/java/org/openapitools/codegen/OpenAPINormalizer.java:729">
P2: Security-scheme cleanup does not recurse into callback operations nested under regular operations, so deleted schemes can remain referenced there.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
- Make the changes backward compatible with custom filters - Iterate over nested callbacks in path items
|
thanks for the PR which has been merged 👍 |
This PR adds new normalizer option to remove unnecessary security schemes. I have decided to make it similar to
FILTERoption to have this functionality more universal and allow to filter schemes by name and by type.I have tried to follow DRY principle and uniform the logic with current
FILTERimplementation that's why it required to touch existent logic.I have also moved
FILTERparse logic outside loop to not repeat it every iteration.TODO:
SET_BEARER_AUTH_FOR_NAMEoptionmarked as internal only (x-internal: true) by the {} filterResolves #22954
PR checklist
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*.IMPORTANT: Do NOT purge/delete any folders/files (e.g. tests) when regenerating the samples as manually written tests may be removed.
master(upcoming7.x.0minor release - breaking changes with fallbacks),8.0.x(breaking changes without fallbacks)"fixes #123"present in the PR description)Summary by cubic
Adds
SECURITY_SCHEMES_FILTERto keep selected security schemes bykeyortype, remove others fromcomponents.securitySchemes, and clean all references globally, per operation, webhooks, callbacks (including nested), and components’ path items. Also adds a reusable, backward‑compatible filter framework with single-pass parsing and clearer errors/logging. Resolves #22954.New Features
SECURITY_SCHEMES_FILTERsupportskey:/type:(e.g.,key:api_key1|api_key2;type:oauth2; types:apiKey|http|mutualTLS|oauth2|openIdConnect). Removes non-matching schemes and their references, drops emptied requirements while keeping original empty ones, and works withSET_BEARER_AUTH_FOR_NAME(conversion happens before filtering). Cleans references across global security, operations, webhooks, callbacks (incl. nested), and components’ callbacks and path items.Refactors
BaseFilter;Filteris now the operations filter and remains backward compatible with custom filters; addsSecuritySchemesFilter. New hooks:createOperationsFilter,createSecuritySchemesFilter; parseFILTER/SECURITY_SCHEMES_FILTERonce; clearer usage-and-input errors and log messages.Written for commit 5ff2e3d. Summary will update on new commits.