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

Introduce OidcRedirectFilter #40600

Merged
merged 1 commit into from
May 17, 2024

Conversation

sberyozkin
Copy link
Member

@sberyozkin sberyozkin commented May 13, 2024

Fixes #40562.

Simple PR which introduces a noteworthy feature, since an ability to customize OIDC redirects dynamically, especially in the presence of multiple OIDC providers, should become useful. Custom providers can add extra query parameters or set response headers as has been tried recently without success, or create secure cookies to help with generating better local redirect error or session expired pages. This PR simply runs every redirect URI through filters if they are available.

Summary for this PR's changes:

  • Get the registered OidcRedirectFilters and run them for every redirect which can be done at the CodeAuthenticationMechanism level
  • Makes 2 OidcUtils method publicly accessible to help custom filters to create or remove cookies if they need to
  • Adds a test where a custom redirect redirect filter encrypts a user name in a cookie and the custom session expired JAX-RS resource handler decrypts this cookie to create a user tailored message. (I had to tweak the test setup a bit to support proper-length encryption client secrets)
  • Updates docs

This PR can be considered a follow up to #40539 - without this PR custom session expired pages can only be produced for single OIDC tenant setups.

Hope it can make to 3.11.0.CR1 or 3.11.0

This comment has been minimized.

This comment has been minimized.

@sberyozkin sberyozkin force-pushed the oidc_redirect_filter branch from 769ccb3 to bd52f6f Compare May 13, 2024 16:48

This comment has been minimized.

Copy link

github-actions bot commented May 13, 2024

🙈 The PR is closed and the preview is expired.

This comment has been minimized.

@sberyozkin sberyozkin marked this pull request as draft May 13, 2024 17:59
@sberyozkin sberyozkin force-pushed the oidc_redirect_filter branch 3 times, most recently from 34ea209 to a68d0ac Compare May 14, 2024 15:23
@sberyozkin sberyozkin force-pushed the oidc_redirect_filter branch from a68d0ac to 5028da3 Compare May 14, 2024 16:45
@sberyozkin sberyozkin marked this pull request as ready for review May 14, 2024 17:39

This comment has been minimized.

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.

LGTM

@sberyozkin
Copy link
Member Author

Thanks @gastaldi, I'd like now to wait a bit in case @pedroigor has some more comments, cheers

@sberyozkin sberyozkin force-pushed the oidc_redirect_filter branch from 5028da3 to 9a910fa Compare May 14, 2024 22:13
@sberyozkin
Copy link
Member Author

@gastaldi Sorry, there was a java docs typo, missing space, and some duplicate text, I only noticed after re-reading it a few times :-)

Copy link

quarkus-bot bot commented May 14, 2024

Status for workflow Quarkus Documentation CI

This is the status report for running Quarkus Documentation CI on commit 9a910fa.

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

⚠️ There are other workflow runs running, you probably need to wait for their status before merging.

Copy link

quarkus-bot bot commented May 14, 2024

Status for workflow Quarkus CI

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

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

Copy link
Contributor

@pedroigor pedroigor left a comment

Choose a reason for hiding this comment

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

LGTM

@sberyozkin
Copy link
Member Author

Thanks @pedroigor.
Indeed, as far as possible optimizations are concerned like having @SessionExpiredPage annotations, it can be handled in a follow up PR

@sberyozkin
Copy link
Member Author

Let me merge now, we can follow up early next week if needed.

@sberyozkin sberyozkin merged commit 6cf3b7e into quarkusio:main May 17, 2024
25 checks passed
@quarkus-bot quarkus-bot bot added this to the 3.12 - main milestone May 17, 2024
@sberyozkin sberyozkin deleted the oidc_redirect_filter branch May 17, 2024 16:42
@quarkus-bot quarkus-bot bot added the kind/enhancement New feature or request label May 17, 2024
@sberyozkin
Copy link
Member Author

Proposing a backport to the 3.11 branch for this PR to make it to final 3.11.0 and to make #40539 useful in a mult-tenant OIDC setup

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Development

Successfully merging this pull request may close these issues.

Support OIDC Redirect filters
4 participants