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.

Sign up for GitHub

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

Jump to bottom

Make OidcRequestContextProperties modifiable #44203

Merged
merged 1 commit into from
Oct 31, 2024
Merged

Make OidcRequestContextProperties modifiable #44203

merged 1 commit into from
Oct 31, 2024

Conversation

sberyozkin
Copy link
Member

Fixes #44156.

OidcRequestContextProperties can be used as a medium for OidcRequestFilter and OidcResponseFilter to attach some metadata to the current request and analyze it.
However it is rather limiting right now - some useful properties are there such as the tenant id, request URI, but request filters can not use OidcRequestContextProperties to pass their own properties to response filters.
This is because OidcRequestContextProperties is unmodifiable - I did it so without any particular reason, but only to start from something...

So this PR relaxes it and adds OidcRequestContextProperties#set and adds a test confirming it works...

While it is technically an enhancement, it fixes a serious enough limitation with a very simple update, so that should warrant a backport to 3.16 once the PR reviewed/merged, but Guillaume, please drop the label if you have any concerns about it, thanks

@sberyozkin sberyozkin requested a review from gastaldi October 30, 2024 18:27
@sberyozkin
Copy link
Member Author

sberyozkin commented Oct 30, 2024 •
edited
Loading

Also, once it is fixed and the release with the fix is done, I'll work on a blog covering all the OIDC filters.

@quarkus-bot quarkus-bot

This comment has been minimized.

Sign in to view
gastaldi
gastaldi approved these changes Oct 30, 2024
View reviewed changes
Copy link
Contributor

@gastaldi gastaldi left a comment

Choose a reason for hiding this comment

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

Approved, but I'd be inclined to avoid any API changes and return a modifiable map instead

@sberyozkin sberyozkin force-pushed the modify_oidc_request_context_props branch from 1b910b3 to b666094 Compare October 31, 2024 11:22
@sberyozkin
Copy link
Member Author

Hey George @gastaldi, I reverted making the Map modifiable and kept the shortcut (renamed to put), you listed that option as well earlier so I took it :-), as it feels a bit nicer to me when users do contextProps.put("a", "b")... I've looked at RoutingContext, it has shortcuts, without exposing the internal Map at all (something I should've avoided I guess)...

Hope it is a good compromise and you will be happy enough with it :-)

@sberyozkin sberyozkin force-pushed the modify_oidc_request_context_props branch from b666094 to 8fa03e9 Compare October 31, 2024 11:52
@sberyozkin sberyozkin force-pushed the modify_oidc_request_context_props branch from 8fa03e9 to cb9b8a2 Compare October 31, 2024 11:57
@quarkus-bot quarkus-bot
Copy link

quarkus-bot bot commented Oct 31, 2024 •
edited by github-actions bot
Loading

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit cb9b8a2.

✅ 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.

You can consult the Develocity build scans.

@sberyozkin sberyozkin merged commit 5401847 into quarkusio:main Oct 31, 2024
23 checks passed
@sberyozkin sberyozkin deleted the modify_oidc_request_context_props branch October 31, 2024 12:49
@quarkus-bot quarkus-bot bot added the kind/enhancement New feature or request label Oct 31, 2024
@quarkus-bot quarkus-bot bot added this to the 3.17 - main milestone Oct 31, 2024
@gsmet gsmet modified the milestones: 3.17 - main, 3.16.2 Nov 5, 2024
benkard pushed a commit to benkard/quarkus-googlecloud-jsonlogging that referenced this pull request Nov 13, 2024
chore(deps): update all non-major dependencies to v3.16.3 (mulk/quark…
83041b9 
…us-googlecloud-jsonlogging!24)

This MR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [io.quarkus:quarkus-extension-processor](https://github.com/quarkusio/quarkus) |  | patch | `3.16.1` -> `3.16.3` |
| [io.quarkus:quarkus-extension-maven-plugin](https://github.com/quarkusio/quarkus) | build | patch | `3.16.1` -> `3.16.3` |
| [io.quarkus:quarkus-bom](https://github.com/quarkusio/quarkus) | import | patch | `3.16.1` -> `3.16.3` |
| [io.quarkus:quarkus-maven-plugin](https://github.com/quarkusio/quarkus) | build | patch | `3.16.1` -> `3.16.3` |

---

### Release Notes

<details>
<summary>quarkusio/quarkus</summary>

### [`v3.16.3`](quarkusio/quarkus@3.16.2...3.16.3)

[Compare Source](quarkusio/quarkus@3.16.2...3.16.3)

### [`v3.16.2`](https://github.com/quarkusio/quarkus/releases/tag/3.16.2)

[Compare Source](quarkusio/quarkus@3.16.1...3.16.2)

##### Complete changelog

-   [#&#8203;34824](quarkusio/quarkus#34824) - AmazonLambdaRecorder Handler Discovery Erroneously Considers Decorators
-   [#&#8203;38086](quarkusio/quarkus#38086) - Documentation about `RecordCodecProvider` in MongoDB with Panache
-   [#&#8203;42149](quarkusio/quarkus#42149) - Upgrade Postgres 16
-   [#&#8203;44039](quarkusio/quarkus#44039) - WebSockets Next: create a new event loop context for each client
-   [#&#8203;44132](quarkusio/quarkus#44132) - Update getting-started-reactive.adoc
-   [#&#8203;44149](quarkusio/quarkus#44149) - Fix Config Error Screen
-   [#&#8203;44152](quarkusio/quarkus#44152) - Do not throw NPE in AfterAll interceptor if application didn't start
-   [#&#8203;44155](quarkusio/quarkus#44155) - Amazon Lambda - Support decorators
-   [#&#8203;44156](quarkusio/quarkus#44156) - Make OidcRequestContextProperties modifiable
-   [#&#8203;44178](quarkusio/quarkus#44178) - Bump com.fasterxml.jackson:jackson-bom from 2.18.0 to 2.18.1
-   [#&#8203;44183](quarkusio/quarkus#44183) - Properly apply the update recipes in version order
-   [#&#8203;44184](quarkusio/quarkus#44184) - Add Support for Trusted Proxy Detection on Forwarded Requests
-   [#&#8203;44185](quarkusio/quarkus#44185) - Repeating `@PermissionsAllowed` annotations totally disable method authentication
-   [#&#8203;44189](quarkusio/quarkus#44189) - Small improvements to the Deploying to Google Cloud guide
-   [#&#8203;44190](quarkusio/quarkus#44190) - ResteasyReactiveProcessor#setupEndpoints reports duplicate endpoints when a rest client matches a resource
-   [#&#8203;44191](quarkusio/quarkus#44191) - Update PostgreSQL image to 17
-   [#&#8203;44201](quarkusio/quarkus#44201) - Broken link
-   [#&#8203;44202](quarkusio/quarkus#44202) - Broken link?
-   [#&#8203;44203](quarkusio/quarkus#44203) - Make OidcRequestContextProperties modifiable
-   [#&#8203;44204](quarkusio/quarkus#44204) - Fix broken doc links
-   [#&#8203;44207](quarkusio/quarkus#44207) - Bump bouncycastle.version from 1.78.1 to 1.79
-   [#&#8203;44209](quarkusio/quarkus#44209) - Bump io.quarkus.develocity:quarkus-project-develocity-extension from 1.1.6 to 1.1.7
-   [#&#8203;44221](quarkusio/quarkus#44221) - Add extension description for websockets next
-   [#&#8203;44227](quarkusio/quarkus#44227) - Add stork-configuration-generator as an annotationProcessorPath
-   [#&#8203;44229](quarkusio/quarkus#44229) - ContainerResponseFilter with `@Priority(Integer.MIN_VALUE)` will be actually invoked with max priority
-   [#&#8203;44232](quarkusio/quarkus#44232) - Quartz: use a more reasonable default for quarkus.quartz.thread-count
-   [#&#8203;44235](quarkusio/quarkus#44235) - 3.16: `@WithTestResource` starts all test resources (regression)
-   [#&#8203;44237](quarkusio/quarkus#44237) - Properly implement priority of ContainerResponseFilter
-   [#&#8203;44238](quarkusio/quarkus#44238) - Refactor SecurityTransformerUtils to consider repeated annotations
-   [#&#8203;44239](quarkusio/quarkus#44239) - Replace oidc auth facebook screenshots with generic ones
-   [#&#8203;44244](quarkusio/quarkus#44244) - Bump `quarkiverse-parent` to 18
-   [#&#8203;44245](quarkusio/quarkus#44245) - Delete disabled job
-   [#&#8203;44248](quarkusio/quarkus#44248) - Ignore client interfaces when detecting duplicate endpoints
-   [#&#8203;44263](quarkusio/quarkus#44263) - Quarkus Dev UI - Clicking on gRPC - Services - service implementation class  Uncaught exception received by Vert.x
-   [#&#8203;44277](quarkusio/quarkus#44277) - Dev UI Open in IDE make sure lineNumber is in quotes
-   [#&#8203;44279](quarkusio/quarkus#44279) - Limit `MATCHING_RESOURCES` TestResources to the test that declares them
-   [#&#8203;44281](quarkusio/quarkus#44281) - Included pages within a fragment ignores rendered=false property.
-   [#&#8203;44298](quarkusio/quarkus#44298) - Qute: fix rendered=false if a fragment includes nested fragment
-   [#&#8203;44300](quarkusio/quarkus#44300) - When testing request payload is populated with string "null" if enable-reflection-free-serializers enabled
-   [#&#8203;44309](quarkusio/quarkus#44309) - Avoid deserializing null nodes in reflection free Jackson serialization
-   [#&#8203;44316](quarkusio/quarkus#44316) - Duplicated field serialization using the generated reflection free Jackson serializers
-   [#&#8203;44317](quarkusio/quarkus#44317) - Avoid duplicated field serialization in reflection free Jackson serializers
-   [#&#8203;44321](quarkusio/quarkus#44321) - Use Java 21 by default in the Deploying to Google Cloud guide
-   [#&#8203;44322](quarkusio/quarkus#44322) - Explain in MongoDB docs that records are supported
-   [#&#8203;44324](quarkusio/quarkus#44324) - Take config annotation when trying to match test resources

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever MR is behind base branch, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this MR and you won't be reminded about these updates again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this MR, check this box

---

This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNC4yNC4wIiwidXBkYXRlZEluVmVyIjoiMzQuMjQuMCJ9-->
@sberyozkin sberyozkin mentioned this pull request Dec 5, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gastaldi gastaldi gastaldi approved these changes

Assignees
No one assigned
Labels
area/oidc kind/enhancement New feature or request
Projects
None yet
Milestone
3.16.2
Development

Successfully merging this pull request may close these issues.

Make OidcRequestContextProperties modifiable
3 participants
@sberyozkin @gastaldi @gsmet