Add Login Selector functional tests. #65705

azasypkin · 2020-05-07T15:04:01Z

Summary

Adding a bunch of pure functional tests to test Login Selector functionality (basic + SAML).

Fixes: #64635

elasticmachine · 2020-05-07T15:04:03Z

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

azasypkin · 2020-05-13T15:27:29Z

x-pack/test/functional/page_objects/security_page.ts

- class LoginPage {
- async login(username, password, options = {}) {
- const [superUsername, superPassword] = config.get('servers.elasticsearch.auth').split(':');
+ interface LoginOptions {


note: I wanted to replace it with LoginExpectedResult initially, but it's used in too many places right now.

azasypkin · 2020-05-13T15:29:18Z

x-pack/test/saml_api_integration/fixtures/saml_provider/metadata.xml

+ <md:SingleSignOnService Binding="urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST"
+ Location="/saml_provider/login"/>
+ <md:SingleSignOnService Binding="urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Redirect"
+ Location="/saml_provider/login"/>


note: needed separate file to make Location's relative.

legrego

Thanks for the TS conversion! Just a couple of questions for you below

legrego · 2020-05-13T18:57:18Z

test/functional/page_objects/login_page.ts

 async login(user: string, pwd: string) {
 await testSubjects.setValue('loginUsername', user);
 await testSubjects.setValue('loginPassword', pwd);
 await testSubjects.click('loginSubmit');
 }
 }

- return new ShieldPage();
+ return new LoginPage();


👏 👏 👏

legrego · 2020-05-13T19:08:48Z

x-pack/test/functional/apps/security/trial_license/index.ts

+import { FtrProviderContext } from '../../../ftr_provider_context';
+
+export default function({ loadTestFile }: FtrProviderContext) {
+ describe('security app - login selector - trial license', function() {


nit: the description for "login selector" should be covered by ./login_selector's describe block

Suggested change

describe('security app - login selector - trial license', function() {

describe('security app - trial license', function() {

Yep, thanks 👍

legrego · 2020-05-13T19:27:35Z

x-pack/test/functional/page_objects/security_page.ts

+
+ async function waitForLoginPage() {
+ await retry.waitForWithTimeout('login page', config.get('timeouts.waitFor') * 5, async () => {
+ const alert = await browser.getAlert();


Is this alert logic here to cleanup any tests that may have a pending browser confirmation dialog?

To be honest, have no idea - just copied existing stuff. But let's try to remove it and see how that goes - we can always return it back (with a proper clarifying comment) if it happens to be necessary.

Okay, now we know why we need this code 🙂

legrego · 2020-05-13T19:36:33Z

x-pack/test/functional/page_objects/security_page.ts

+ });
+ }
+
+ async function waitForLoginForm() {


Do we need to distinguish between waiting for the login form vs the login page? Can we collapse these into a single function?

Do we need to distinguish between waiting for the login form vs the login page?

I believe so, yes. I mean Login Form isn't rendered by default when we have multiple providers and Login Selector isn't rendered when we have just basic/token. And after logout we don't know where we end up (it's used in various tests, with or without Login Selector) so we're waiting for Login Page to appear.

The login-form CSS class we use for the entire Login Page is rather confusing though.

Or what did you mean?

If I understand the distinction correctly, we have:

Login form, which is the username/password input fields

Login page, which is the screen that contains the Login form (could alternatively render the disabled login view if we are preventing users from logging in)

Login selector, which is the new one

To me, it appears like we don't need to make a distinction between Login form and Login page, since the form is just a child of the page, and not a distinct view.

Is there something I'm overlooking or oversimplifying?

Login form, which is the username/password input fields

Correct

Login page, which is the screen that contains the Login form (could alternatively render the disabled login view if we are preventing users from logging in)

It may contain Login Form (including its disabled states) or it may contain Login Selector (we render just one or another)

Login selector, which is the new one

Correct, it's alternative to the Login Form that Login Page may contain

To me, it appears like we don't need to make a distinction between Login form and Login page, since the form is just a child of the page, and not a distinct view.
Is there something I'm overlooking or oversimplifying?

Let me explain two possible cases when I think we need two different things so that you can point to the missing piece in my understanding:

if some test call forceLogout, we should wait for either Login Form or Login Selector to be rendered. But the problem is that we don't know (unless we start analyzing Kibana server config) if Kibana is configured with basic only (in this case Login Page -> Login Form will be rendered) or it's configured with more than one provider (in this case Login Page -> Login Selector will be rendered). Waiting for parent Login Page seems to do the job in both cases.

when we test Login Page itself we want to know that transition between Login Selector and Login Form works, so we have two different methods that wait for one or another to become visible (parent Login Page is always visible/rendered in these tests).

How would you simply this to cover both cases with just two methods? I'm all in to simply stuff as much as possible, I'm just not sure I'm completely following your proposal, sorry about that 🙈

Ah, the piece I was missing was the the Login Page contains the Login Selector. That makes sense, so I think we need to keep both here. Thanks for clarifying this for me!

legrego · 2020-05-13T19:47:19Z

x-pack/test/functional/page_objects/security_page.ts


+ if (expectedResult === 'chrome') {
+ await find.byCssSelector('[data-test-subj="kibanaChrome"] nav:not(.ng-hide) ', 20000);


question The Kibana chrome isn't still using angular, is it? ng-hide probably won't ever exist now that it's migrated to KP. I know this was just a TS conversion, and we have this in other places too, but I wonder what the "correct" test should be. Maybe we just don't need that pseudo-selector anymore?

That's a good point, let me figure that out while I'm here.

It seems what we really need is [data-test-subj="kibanaChrome"] .app-wrapper:not(.hidden-chrome) - changed to this instead.

…ff and properly detect that chrome is visible.

…ser out.

kibanamachine · 2020-05-14T13:15:24Z

💛 Build succeeded, but was flaky

continuous-integration/kibana-ci/pull-request
Commit: 73cc2b9
Flaky suites:
- kibana-ciGroup6

History

💔 Build #47888 failed e8766ee
💚 Build #47645 succeeded 92e8d74
💔 Build #47554 failed 69eb5fb
💔 Build #47415 failed e6c7d78
💔 Build #47360 failed 1b1e134

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

legrego

LGTM!

azasypkin · 2020-05-15T10:31:39Z

Thanks for review @legrego !

It looks like the flakiness of the last CI run isn't related to these changes, so we should be safe:

Node Execute kibana-ciGroup6
ERROR WebDriverError: unknown error: Chrome failed to start: exited abnormally.

# Conflicts: # x-pack/scripts/functional_tests.js

legrego · 2020-05-15T11:13:59Z

It looks like the flakiness of the last CI run isn't related to these changes, so we should safe:

Node Execute kibana-ciGroup6
ERROR WebDriverError: unknown error: Chrome failed to start: exited abnormally.

👍I had double checked too, just in case

…ent/add-support-in-url-for-hidden-toggle * 'master' of github.com:elastic/kibana: (34 commits) [SIEM][CASE] Fix bug when connector is deleted. (elastic#65876) [SIEM][CASE] Improve layout (elastic#66232) [Index Management] Support Hidden Indices (elastic#66422) Add Login Selector functional tests. (elastic#65705) Lens drilldowns (elastic#65675) [ML] Custom template for apiDoc markdown (elastic#66567) Don't bootstrap core type emits (elastic#66377) [Dashboard] Improve loading error handling (elastic#66372) [APM] Minor style fixes for the node strokes (elastic#66574) [Ingest Manager] Fix create data source from integration (elastic#66626) [Metrics UI] Fix default metric alert interval for new conditions (elastic#66610) [Metrics UI] Fix alignment and allow clearing metric value (elastic#66589) Don't return package name for non-package data streams (elastic#66606) [Ingest Manager] Consolidate routing and add breadcrumbs to all pages (elastic#66475) [Docs/Reporting] Have the docs about granular timeout match Cloud docs (elastic#66267) Don't automatically add license header to code inside plugins dir. (elastic#66601) [APM] Don't trigger map layout if no elements (elastic#66625) [Logs UI] Validate ML job setup time ranges (elastic#66426) Fix pagination bugs in CCR and Remote Clusters (elastic#65931) Add cloud icon for supported settings and embed single-sourced getting started (elastic#65610) ... # Conflicts: # x-pack/plugins/index_management/public/application/sections/home/index_list/index_table/index_table.js # x-pack/plugins/index_management/server/lib/fetch_indices.ts

azasypkin · 2020-05-15T13:00:32Z

7.8/7.8.0: 3e740a1
7.x/7.9.0: aa6785e

azasypkin added test Team:Security Team focused on: Auth, Users, Roles, Spaces, Audit Logging, and more! Feature:Security/Authentication Platform Security - Authentication release_note:skip Skip the PR/issue when compiling release notes labels May 7, 2020

azasypkin added v7.8.0 v7.8.1 v7.9.0 labels May 7, 2020

azasypkin force-pushed the issue-xxx-login-selector-tests branch 4 times, most recently from e6c7d78 to 69eb5fb Compare May 13, 2020 07:31

Add Login Selector functional tests.

92e8d74

azasypkin force-pushed the issue-xxx-login-selector-tests branch from 69eb5fb to 92e8d74 Compare May 13, 2020 13:10

azasypkin marked this pull request as ready for review May 13, 2020 15:07

azasypkin requested a review from a team as a code owner May 13, 2020 15:07

azasypkin commented May 13, 2020

View reviewed changes

legrego reviewed May 13, 2020

View reviewed changes

azasypkin added 3 commits May 14, 2020 08:50

Merge branch 'master' into issue-xxx-login-selector-tests

892fbba

Review#1: make sure we are preserving original URL, remove legacy stu…

e8766ee

…ff and properly detect that chrome is visible.

Review#1: return code that closes browser native alerts when we log u…

73cc2b9

…ser out.

azasypkin requested a review from legrego May 14, 2020 13:27

legrego approved these changes May 15, 2020

View reviewed changes

azasypkin merged commit aff7f90 into elastic:master May 15, 2020

azasypkin deleted the issue-xxx-login-selector-tests branch May 15, 2020 10:33

azasypkin mentioned this pull request May 15, 2020

[7.x] Add Login Selector functional tests. (#65705) #66709

Merged

azasypkin added a commit to azasypkin/kibana that referenced this pull request May 15, 2020

Add Login Selector functional tests. (elastic#65705)

dfe605f

# Conflicts: # x-pack/scripts/functional_tests.js

azasypkin mentioned this pull request May 15, 2020

[7.8] Add Login Selector functional tests. (#65705) #66710

Merged

azasypkin added a commit to azasypkin/kibana that referenced this pull request May 15, 2020

Add Login Selector functional tests. (elastic#65705)

4d5555d

# Conflicts: # x-pack/scripts/functional_tests.js

azasypkin added backport pending and removed v7.8.1 labels May 15, 2020

azasypkin added backported and removed backport pending labels May 15, 2020

pheyos mentioned this pull request May 18, 2020

Add welcome screen disabling to common page #66713

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Login Selector functional tests. #65705

Add Login Selector functional tests. #65705

azasypkin commented May 7, 2020 •

edited

Loading

elasticmachine commented May 7, 2020

azasypkin May 13, 2020

azasypkin May 13, 2020

legrego left a comment

legrego May 13, 2020

legrego May 13, 2020

azasypkin May 14, 2020

legrego May 13, 2020

azasypkin May 14, 2020

azasypkin May 14, 2020

legrego May 13, 2020

azasypkin May 14, 2020

legrego May 14, 2020

azasypkin May 14, 2020

legrego May 14, 2020

legrego May 13, 2020

azasypkin May 14, 2020

azasypkin May 14, 2020

kibanamachine commented May 14, 2020

legrego left a comment

azasypkin commented May 15, 2020 •

edited

Loading

legrego commented May 15, 2020

azasypkin commented May 15, 2020

	describe('security app - login selector - trial license', function() {
	describe('security app - trial license', function() {


		if (expectedResult === 'chrome') {
		await find.byCssSelector('[data-test-subj="kibanaChrome"] nav:not(.ng-hide) ', 20000);

Add Login Selector functional tests. #65705

Add Login Selector functional tests. #65705

Conversation

azasypkin commented May 7, 2020 • edited Loading

Summary

elasticmachine commented May 7, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

legrego left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kibanamachine commented May 14, 2020

💛 Build succeeded, but was flaky

History

legrego left a comment

Choose a reason for hiding this comment

azasypkin commented May 15, 2020 • edited Loading

legrego commented May 15, 2020

azasypkin commented May 15, 2020

azasypkin commented May 7, 2020 •

edited

Loading

azasypkin commented May 15, 2020 •

edited

Loading