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

Rename OidcClientRequestFilter filter to OidcRequestFilter #36915

Conversation

sberyozkin
Copy link
Member

This is a follow up to #36634, this code is currently on main only, so I'd like to apply these changes before 3.6.0.CR1 is out:

  • I've found it confusing today that this filter is called OidcClientRequestFilter since we already have OidcClientRequestFilter whose job is not to intercept OIDC requests but act as an OidcClient to acquire tokens and then propagate it to the target services. In fact, the OidcClientRequestFilter filter introduced in Introduce OidcRequestFilter #36634 can intercept this existiing OidcClientRequestFilter doing the OIDC requests - it can confuse everybody, so I renamed to a more appropriate OidcRequestFilter and clarified it in the docs
  • Update the method signature with a new parameter, contextProperties, which is simply a Map wrapper at this time and will have more type specific getters like getStringProperty or may be get(String name, Class<T> type) etc. I'll fill it in a bit more in a follow up enhancement. The main reason is to support use cases where for example a JWK set acquisition has to be delayed and when this filter will need to analyze some context properties, example, a token or something else. Also, it is a protection in case something else has been missed in which case we will pass it as an extra property

CC @calvernaz

Copy link
Contributor

@michalvavrik michalvavrik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@quarkus-bot

This comment has been minimized.

@sberyozkin sberyozkin force-pushed the rename_oidc_client_request_filter_filter branch from 862a264 to c22ba5a Compare November 7, 2023 13:14
@quarkus-bot quarkus-bot bot added area/docstyle issues related for manual docstyle review area/documentation labels Nov 7, 2023
@quarkus-bot quarkus-bot bot added this to To do in Quarkus Documentation Nov 7, 2023
@sberyozkin
Copy link
Member Author

Apologies for the noise, I realized that beyond updating JavaDocs, I had to fix the reference guides mentioning the old interface name

@sberyozkin sberyozkin force-pushed the rename_oidc_client_request_filter_filter branch from 87e0d2e to db5d540 Compare November 7, 2023 13:17
Copy link

github-actions bot commented Nov 7, 2023

🙈 The PR is closed and the preview is expired.

@quarkus-bot
Copy link

quarkus-bot bot commented Nov 7, 2023

✔️ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

Quarkus Documentation automation moved this from To do to Reviewer approved Nov 7, 2023
@sberyozkin sberyozkin merged commit ff281c8 into quarkusio:main Nov 7, 2023
24 checks passed
Quarkus Documentation automation moved this from Reviewer approved to Done Nov 7, 2023
@sberyozkin sberyozkin deleted the rename_oidc_client_request_filter_filter branch November 7, 2023 15:10
@quarkus-bot quarkus-bot bot added this to the 3.6 - main milestone Nov 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/docstyle issues related for manual docstyle review area/documentation area/oidc
Development

Successfully merging this pull request may close these issues.

None yet

3 participants