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

Support for OIDC session expired page #40539

Merged
merged 1 commit into from
May 10, 2024

Conversation

sberyozkin
Copy link
Member

@sberyozkin sberyozkin commented May 9, 2024

Currently, a user whose session has expired or no longer can be refreshed (for example, RT itself is no longer valid) is redirected to the OIDC provider to re-authenticate which can offer a poor UX. For example, imagine a user who has authenticated is accessing an application page after some idle time and is seeing an authentication challenge screen, instead of a friendly page which informs the user, your session has expired, follow this link to re-authenticate.

This is exactly what this PR does, lets users configure a session expired page where a user whose session has expired or no longer can be refreshed is redirected to this page from where the user can again re-login, but in a normal interactive way.
One of the tests has been updated to confirm a redirect to such a page is initiated. Docs have been updated.

CC @calvernaz

@quarkus-bot

This comment has been minimized.

Copy link

github-actions bot commented May 9, 2024

🙈 The PR is closed and the preview is expired.

@quarkus-bot

This comment has been minimized.

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.

Nice, looks like a nice UX improvement.

Quarkus Documentation automation moved this from To do to Reviewer approved May 10, 2024
@sberyozkin
Copy link
Member Author

Hi @pedroigor, @gastaldi, thanks, yes, hope it will give users an easy option to control the UX better once the user session has expired.
George, thanks for catching the doc typo, anything else you'd like to suggest, let me know, take your time please. I only hope it will make it to 3.11.0.CR1 :-)

@quarkus-bot
Copy link

quarkus-bot bot commented May 10, 2024

Status for workflow Quarkus Documentation CI

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

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

@quarkus-bot
Copy link

quarkus-bot bot commented May 10, 2024

Status for workflow Quarkus CI

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

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

@gastaldi
Copy link
Contributor

LGTM, but I wonder if these pages shouldn't be under a specific config. Eg. quarkus.oidc.authentication.redirect-pages.session-expired

@sberyozkin
Copy link
Member Author

Hi @gastaldi, thanks,

LGTM, but I wonder if these pages shouldn't be under a specific config. Eg. quarkus.oidc.authentication.redirect-pages.session-expired

Sure, it could've been done, I could've introduced an explicit group for it. But we already have an error-path property at the authentication group level, this is a local resource to where a user is redirected if the OIDC authentication has failed, I should've asked you to review that earlier PR :-) This is why I added this property immediately under error-path :-). So I'd keep at under authentication. too, just to avoid any migration and deprecation for the error-path.

Also, now that you have mentioned this idea, we also have a redirect-path property, which is not an ideal property name, because it indicates a callback path to where the user should land first to after a redirect from OIDC.

Not an ideal situation, but hope we can compensate for that with JavaDocs and docs :-)

@gastaldi gastaldi merged commit 4d6425b into quarkusio:main May 10, 2024
25 checks passed
Quarkus Documentation automation moved this from Reviewer approved to Done May 10, 2024
@quarkus-bot quarkus-bot bot added this to the 3.11 - main milestone May 10, 2024
@quarkus-bot quarkus-bot bot added the kind/enhancement New feature or request label May 10, 2024
@sberyozkin sberyozkin deleted the oidc_session_expired_page branch May 10, 2024 18:41
@sberyozkin
Copy link
Member Author

sberyozkin commented May 10, 2024

Hey @gastaldi @pedroigor Thanks, I already have an exciting follow-up enhancement request in mind :-)

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

Successfully merging this pull request may close these issues.

Support for the OIDC session-expired page
3 participants