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
Detect cookies on login page #120944
Detect cookies on login page #120944
Conversation
Pinging @elastic/kibana-security (Team:Security) |
@elasticmachine merge upstream |
@elasticmachine merge upstream |
ACK: will review today or tomorrow. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! Just left two questions.
x-pack/plugins/security/public/authentication/login/login_page.tsx
Outdated
Show resolved
Hide resolved
x-pack/plugins/security/public/authentication/login/login_page.test.tsx
Outdated
Show resolved
Hide resolved
x-pack/plugins/security/public/authentication/login/__snapshots__/login_page.test.tsx.snap
Outdated
Show resolved
Hide resolved
x-pack/plugins/security/public/authentication/login/login_page.tsx
Outdated
Show resolved
Hide resolved
….tsx Co-authored-by: gchaps <33642766+gchaps@users.noreply.github.com>
…s__/login_page.test.tsx.snap Co-authored-by: gchaps <33642766+gchaps@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for making this change, I think the different wording will help us troubleshoot SDH issues more quickly, as this has been a problem in the past.
Code review only -- LGTM on green CI, will leave final approval to Oleg
{this.props.sameSiteCookies !== 'None' ? ( | ||
<FormattedMessage | ||
id="xpack.security.loginPage.openInNewWindowOrChangeKibanaConfigTitle" | ||
defaultMessage="To view this content, open it in a new window or ask your administrator to allow cross-origin cookies." | ||
/> | ||
) : ( | ||
<FormattedMessage | ||
id="xpack.security.loginPage.openInNewWindowOrChangeBrowserSettingsTitle" | ||
defaultMessage="To view this content, open it in a new window or adjust your browser settings to allow third-party cookies." | ||
/> | ||
)} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
ACK: reviewing... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great, thanks for making these changes!
x-pack/plugins/security/public/authentication/login/login_page.tsx
Outdated
Show resolved
Hide resolved
💚 Build Succeeded
Metrics [docs]Async chunks
Page load bundle
History
To update your PR or re-run it, just comment with: |
💔 Backport failed
To backport manually run: |
Friendly reminder: Looks like this PR hasn’t been backported yet. |
1 similar comment
Friendly reminder: Looks like this PR hasn’t been backported yet. |
# Conflicts: # x-pack/plugins/security/public/authentication/login/login_app.ts
# Conflicts: # x-pack/plugins/security/public/authentication/login/login_app.ts
Resolves #97200
This PR fixes a bug caused by Safari's "Prevent cross-site tracking" feature.
When this feature is enabled Safari prevents us from setting cookies when embedded inside an iframe.
There's no workaround for this behaviour but we can provide a better user experience by allowing users to open the embedded content inside a new window where they can log in without any issues.
When Kibana can't set cookies and is embedded in an iframe
When Kibana can't set cookies and is not embedded in an iframe
Testing
yarn es snapshot --license trial --ssl
yarn start --ssl