[SECURITY] Remove flakiness around edit user

[XavierM]

Summary

Resolves: #115473

Checklist

• Unit or functional tests were updated or added to match the most common scenarios

[XavierM]

Running https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/162#9b3c6d3d-ba5c-47ef-8d05-7152df0b7038

[XavierM]

@elasticmachine merge upstream

[elasticmachine]

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

[jportner]

Some small nits and a question below, otherwise this is looking great, really nice job!

[jportner]

I did some digging to see exactly where this user's credentials are defined. It's here:

kibana/test/common/services/security/test_user.ts

Lines 18 to 19 in 60340b5

	const TEST_USER_NAME = 'test_user';
	const TEST_USER_PASSWORD = 'changeme';

Docs: https://www.elastic.co/guide/en/kibana/8.0/development-tests.html#_using_the_test_user_service

However, we have a different adminTestUser that is used in our security API integration tests:

kibana/x-pack/test/security_api_integration/tests/session_invalidate/invalidate.ts

Line 10 in 60340b5

import { adminTestUser } from '@kbn/test';

These are defined by the @kbn/test package, they can apparently be changed with environment variables and may be different for Cloud testing:

kibana/packages/kbn-test/src/kbn/users.ts

Lines 9 to 24 in 6693ef3

	const env = process.env;
	export const kibanaTestUser = {
	username: env.TEST_KIBANA_USER || 'elastic',
	password: env.TEST_KIBANA_PASS || 'changeme',
	};
	export const kibanaServerTestUser = {
	username: env.TEST_KIBANA_SERVER_USER || 'kibana',
	password: env.TEST_KIBANA_SERVER_PASS || 'changeme',
	};
	export const adminTestUser = {
	username: env.TEST_ES_USER || 'elastic',
	password: env.TEST_ES_PASS || 'changeme',
	};

---

All that said, using the hardcoded test_user / changeme in this file makes me a bit nervous because 1. I'm not sure if the tests will always be executed with the user defined in test_user.ts, and 2. even so, if that user changes, then this test will fail.

So, maybe we should just export the test username/password from that file so it can be used here, and just call it a day. But I want to know what @azasypkin thinks, in case I'm missing anything else RE: Cloud testing.

[azasypkin]

All that said, using the hardcoded test_user / changeme in this file makes me a bit nervous

Yeah, I share the same concern, but mostly because I treat service test users as immutable to avoid any side effects for the following tests. Is there any reason why we cannot do something like this (semi pseudo-code)?

const user = {
  username: 'throw-away-user',
  password: 'changeme',
  new_password: 'changeme',
  confirm_password: 'changeme',
  roles: ['kibana_admin'],
};

// The API integration tests service to create through API, not UI
await security.user.create(user.username, {
  password: user.password,
  roles: user.roles,
  full_name: 'Admin',
});

// Re-login (should be pretty stable)
await PageObjects.security.forceLogout();
// OR `PageObjects.security.login` that's even simpler to use
await PageObjects.security.loginSelector.login('basic', 'basic', { 
  username: user.username, password: user.password
});

await PageObjects.security.updateUserPassword(user, true);

// Make sure new password works.
await PageObjects.security.forceLogout();
await PageObjects.security.loginSelector.login('basic', 'basic', { 
  username: user.username, password: user.new_password
});

[XavierM]

I was just trying to be lazy here :) since I was not changing the password, but if everyone is concerned about it, I will just login using optionalUser. Not a problem

[jportner]

Yeah my main concern is that we might break Cloud tests. If we do that, we don't find out until after the fact, and it's just a whole thing. Safer to do what Oleg suggested.

[XavierM]

Also I am not going to test if the new password works, I am assuming that we should have API integration for that. What I want to test here that when I am logging as a current user and I am changing my password, and I will have a text field asking for my current password and then I can change my password. We do not want to do too much in one test.

[jportner]

Agreed, that makes sense to me

[azasypkin]

What I want to test here that when I am logging as a current user and I am changing my password, and I will have a text field asking for my current password and then I can change my password. We do not want to do too much in one test.

Sounds good to me as well. Would you mind keeping this comment in the test as well? It'd help future readers to understand what we want to test exactly here.

[XavierM]

Since I changed some of the functional test, I am re-running flaky suite test
https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/171#493c935d-a88a-4815-8bce-6c4e4d246423

We are good here 🟢

[kibanamachine]

💚 Build Succeeded

• Buildkite Build
• Commit: ec1bf53
• Storybooks Preview

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
security	487.1KB	487.3KB	+194.0B

History

• 💔 Build #4865 failed 0d434a3
• 💔 Build #4735 failed c1376cb
• 💚 Build #4510 succeeded e461e7a
• 💔 Build #4422 failed 90c4a4d

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

[kibanamachine]

💔 Backport failed

Status	Branch	Result
✅	8.0
❌	7.16	Commit could not be cherrypicked due to conflicts

Successful backport PRs will be merged automatically after passing CI.

To backport manually run:
node scripts/backport --pr 117558