Add support for client-side OIDC interstitial redirect#9669
Merged
mitchellhenke merged 27 commits intomainfrom Dec 6, 2023
Merged
Add support for client-side OIDC interstitial redirect#9669mitchellhenke merged 27 commits intomainfrom
mitchellhenke merged 27 commits intomainfrom
Conversation
aduth
reviewed
Nov 28, 2023
aduth
reviewed
Nov 28, 2023
mitchellhenke
force-pushed
the
mitchellhenke/oidc-interstitial-redirect
branch
from
58ac399 to
5fae0d0
Compare
aduth
reviewed
Nov 29, 2023
mitchellhenke
force-pushed
the
mitchellhenke/oidc-interstitial-redirect
branch
from
5fae0d0 to
4f4c4cb
Compare
spec/controllers/openid_connect/authorization_controller_spec.rb
Outdated
Show resolved
Hide resolved
mitchellhenke
force-pushed
the
mitchellhenke/oidc-interstitial-redirect
branch
2 times, most recently
from
1025204 to
1b776fa
Compare
mitchellhenke
requested review from
a team,
Sgtpluck,
mmagsa and
zachmargolis
aduth
approved these changes
Dec 4, 2023
Comment on lines
10
to
11
Contributor
There was a problem hiding this comment.
I recall you'd mentioned earlier wanting to keep the stylesheets in order to show a blue background. I noticed (testing by removing the <meta> tag locally) that the background is white once the page fully loads, because of styles applied to the <body> tag. Technically we could omit the <body> tag here altogether if we wanted that blue background (which is applied to the <html> tag to be visible).
Contributor
Author
There was a problem hiding this comment.
Added background to the body tag based on internal discussion in 6c71294
mitchellhenke
force-pushed
the
mitchellhenke/oidc-interstitial-redirect
branch
from
6c71294 to
fc95af0
Compare
added 13 commits
December 6, 2023 14:45
changelog: Internal, OpenID Connect, Add support for client-side OIDC redirect
Co-authored-by: Zach Margolis <zachmargolis@users.noreply.github.com>
mitchellhenke
force-pushed
the
mitchellhenke/oidc-interstitial-redirect
branch
from
fc95af0 to
9e60cb8
Compare
Merged
aduth
reviewed
Jan 5, 2024
| @@ -1,3 +1,4 @@ | |||
| # rubocop:disable Layout/LineLength | |||
Contributor
There was a problem hiding this comment.
In retrospect, not sure we should have disabled this for the full file?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🛠 Summary of changes
Note: This is very much a draft and still exploratory
Currently, we must keep track of where we may redirect to OIDC partner applications due to the Content Security Policy (CSP) form-action directive. Some browsers (Chrome and Safari) have a stricter implementation of the
form-actiondirective and require that all hosts in the redirect chain be in theform-actiondirective, which requires us to include all potential redirects in the partner application (since they may redirect on from our redirect).For example, when someone is authenticating with a service provider and successfully submits a form with their OTP code and is successful, there is potential that we may redirect to the partner application, so we must include all of those in the CSP
form-actionheader via apply_secure_headers_override. It isn't always immediately apparent when this is needed and can lead to bugs (#8063) where a user submits a form and it silently fails with a CSP error logged to the console.This PR takes a similar approach to what we did in #6894 for SAML by rendering a page with a link that will be clicked by Javascript (and includes a no Javascript fallback where a user can click the link themselves).
Some of the details still need to be worked out, including the content and visuals of the interstitial page.
This change originated from this proposal which includes more discussion on the partner-facing impacts.