Skip to content

[Security] Add message to login page - #51557

Merged
sorenlouv merged 7 commits into
elastic:masterfrom
sorenlouv:add-login-assistance-message
Nov 26, 2019
Merged

sorenlouv merged 7 commits into
elastic:masterfrom
sorenlouv:add-login-assistance-message

Conversation

@sorenlouv

@sorenlouv sorenlouv commented Nov 24, 2019 •

Copy link
Copy Markdown
Contributor

This change makes it possible to add an optional message to the login page. This is useful for announcing scheduled maintenance, and providing help with credentials - something we currently need the ability to do for observability.

Example 1: Maintenance announcements

Example 2: Help with credentials (corporate)

Example 3: Help with credentials (test cluster)

@sorenlouv
sorenlouv requested review from a team as code owners November 24, 2019 21:01
@elasticmachine

Copy link
Copy Markdown
Contributor

💔 Build Failed

@sorenlouv sorenlouv added the Team:Security Platform Security: Auth, Users, Roles, Spaces, Audit Logging, etc t// label Nov 25, 2019
@elasticmachine

Copy link
Copy Markdown
Contributor

Pinging @elastic/kibana-security (Team:Security)

@elasticmachine

Copy link
Copy Markdown
Contributor

💔 Build Failed

@elasticmachine

Copy link
Copy Markdown
Contributor

💔 Build Failed

@watson

watson commented Nov 25, 2019

Copy link
Copy Markdown
Contributor

For scheduled maintenance announcements, I think it would be better to have a banner that also appears after login and not only during login. A user can be logged in for a long time and not see the login screen. So I'm not sure about how big a benefit this will be only on the login screen.

However, I like the idea of having a message like this related to the actual login procedure.

@sorenlouv

sorenlouv commented Nov 25, 2019 •

Copy link
Copy Markdown
Contributor Author

@watson True, maintenance announcements might not be the best use case. Help to login is specifically the problem we are trying to solve. For the Kibana instances I'm responsible for I'm regularly pinged by people who want to know how they can login. For this specific purpose it would be very beneficial to be able to add login instructions (or link to such) directly above the login box.

I'm sure there are also other use cases people might want to use this message for so tried to keep it a little generic.

@elasticmachine

Copy link
Copy Markdown
Contributor

💚 Build Succeeded

@jbudz

jbudz commented Nov 25, 2019

Copy link
Copy Markdown
Contributor

We do have
image too, only after login but could be reused for a global. I do think there's utility with a server defined message like this too

@legrego

legrego commented Nov 25, 2019

Copy link
Copy Markdown
Member

@alexfrancoeur would this satisfy #36586?

I think there is a small part of #17298 in here too, but that is mostly concerned with post-login messages

@sorenlouv

sorenlouv commented Nov 25, 2019 •

Copy link
Copy Markdown
Contributor Author

More generally, the ability to add a custom html site link to login page for admin user to specify text of the link and link to external page through the management interface in Kibana.
#36586 (comment)

Users cannot use custom html but they can use markdown, so links and formatting is possible (and I'd argue safer and easier than allowing custom html).

@legrego legrego left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It looks like the "logout message" prevents the new login assistance message from rendering:

image

I'd expect both messages to appear, but they're both using the primary EuiCallOut color, so I worry one might get lost next to the other. This might be ok, but maybe @elastic/kibana-design has an opinion?

image

@legrego

legrego commented Nov 25, 2019

Copy link
Copy Markdown
Member

Users cannot use custom html but they can use markdown, so links and formatting is possible (and I'd argue safer and easier than allowing custom html).

++ I'd be wary of custom HTML too, and I think markdown allows for a sufficient level of control here.

@legrego legrego added the v8.0.0 label Nov 25, 2019
@cchaos

cchaos commented Nov 25, 2019

Copy link
Copy Markdown
Contributor

I would not put the custom message inside of an EuiCallOut. Rendering can be tricky with custom content (even just Markdown) as seen by @legrego's example. Though if it is just plain Markdown, it should get wrapped in an EuiText (size="s") block.

@sorenlouv

sorenlouv commented Nov 25, 2019 •

Copy link
Copy Markdown
Contributor Author

@cchaos Better?

image

@sorenlouv

sorenlouv commented Nov 25, 2019 •

Copy link
Copy Markdown
Contributor Author

It looks like the "logout message" prevents the new login assistance message from rendering:

@legrego Good point. Since c229c4c it now renders the login message independently of the other messages.

@cchaos cchaos left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@sqren Yes, I think it's fair to say that not all custom messages time or content sensitive. Which is why pulling it out of the EuiCallOut makes the most sense. If it is, they can add their own emphasis via text formatting.

Co-Authored-By: Caroline Horn <549577+cchaos@users.noreply.github.com>
@sorenlouv

sorenlouv commented Nov 25, 2019 •

Copy link
Copy Markdown
Contributor Author

Login Page with logout callout and login assistance message:

image

Edit: text made smaller since I took the screenshot

@elasticmachine

Copy link
Copy Markdown
Contributor

💔 Build Failed

@sorenlouv

Copy link
Copy Markdown
Contributor Author

I would assume we'd want our callout though to be closest to the login form (just above the panel).

Okay, I'll flip them around.

@sorenlouv

Copy link
Copy Markdown
Contributor Author

image

@cchaos cchaos left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good from my end. 👍

@elasticmachine

Copy link
Copy Markdown
Contributor

💚 Build Succeeded

@watson watson left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM 👍

@sorenlouv

Copy link
Copy Markdown
Contributor Author

I'll merge this for now to unblock some things. LMK if I should change the name xpack.security.loginAssistanceMessage to something else (naming is hard) and I can do that in a follow-up PR.

@sorenlouv
sorenlouv merged commit e8e5174 into elastic:master Nov 26, 2019
@sorenlouv
sorenlouv deleted the add-login-assistance-message branch November 26, 2019 12:19
sorenlouv added a commit that referenced this pull request Nov 26, 2019
* [Security] Add loginAssistanceMessage to login page

* Fix tests

* Fix login_page.test.tsx

* Fix defaultValue

* Render login assistance message independently of other messages and use EuiText instead of EuiCallOut

* Use small text

Co-Authored-By: Caroline Horn <549577+cchaos@users.noreply.github.com>

* Flip order of message around
@kresss

kresss commented Jan 6, 2020

Copy link
Copy Markdown

Do users still see this new message if they are logging into a PKI protected kibana? Ideally they should have to acknowledge the banner before being logged in even if they are not providing username/password style credentials.

@kobelb

kobelb commented Jan 7, 2020 •

Copy link
Copy Markdown
Contributor

Do users still see this new message if they are logging into a PKI protected kibana? Ideally they should have to acknowledge the banner before being logged in even if they are not providing username/password style credentials.

They do not. This message is only shown on the login page, which only works with the Reserved/Native/File realms in Elasticsearch.

Would you mind chiming in on either #18176 and/or #17298 with your specific needs?

@kresss

kresss commented Jan 7, 2020

Copy link
Copy Markdown

Would you mind chiming in on either #18176 and/or #17298 with your specific needs?

@kobelb I think that our requirements are stated in #18176. The comment from @mbarretta restates the requirement.

@kobelb

kobelb commented Jan 7, 2020

Copy link
Copy Markdown
Contributor

Thanks @kresss

@cauemarcondes

Copy link
Copy Markdown
Contributor

Tests:
IE: ✅
Chrome: ✅
Firefox: ✅
Safari: ✅
Screenshot 2020-01-20 at 12 03 22

@cauemarcondes cauemarcondes added the apm:test-plan-done Pull request that was successfully tested during the test plan label Jan 20, 2020
@watson watson mentioned this pull request Jan 27, 2020
@azasypkin

Copy link
Copy Markdown
Contributor

6.8/6.8.9: 2baa83b

patrykkopycinski pushed a commit to patrykkopycinski/kibana that referenced this pull request May 6, 2026
* [Security] Add loginAssistanceMessage to login page

* Fix tests

* Fix login_page.test.tsx

* Fix defaultValue

* Render login assistance message independently of other messages and use EuiText instead of EuiCallOut

* Use small text

Co-Authored-By: Caroline Horn <549577+cchaos@users.noreply.github.com>

* Flip order of message around
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

apm:test-plan-done Pull request that was successfully tested during the test plan backported release_note:enhancement Team:Security Platform Security: Auth, Users, Roles, Spaces, Audit Logging, etc t// v6.8.9 v7.6.0 v8.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.