[FTR] enable roles management testing for Observability project #196514

dmlemeshko · 2024-10-16T09:56:13Z

Summary

This PR makes changes in FTR saml_auth service to allow creating custom role for Oblt serverless project, when roles management is explicitly enabled with --xpack.security.roleManagementEnabled=true in Kibana server arguments.

I also added role_management/custom_role_access.ts as a test example. Currently roles management is enabled in x-pack/test_serverless/functional/test_suites/observability/config.feature_flags.ts and after this PR is merged, more tests with custom roles can be added for Oblt project.

How to run tests:

node scripts/functional_tests --config x-pack/test_serverless/functional/test_suites/observability/config.feature_flags.ts

dmlemeshko · 2024-10-16T09:57:39Z

x-pack/test_serverless/functional/test_suites/search/custom_role_access.ts

@@ -53,6 +53,8 @@ export default function ({ getPageObjects, getService }: FtrProviderContext) {
      if (roleAuthc) {
        await samlAuth.invalidateM2mApiKeyWithRoleScope(roleAuthc);
      }
+      // delete custom role
+      await samlAuth.deleteCustomRole();


we need to delete role, because platform security tests validate security roles UI and roles list.

dmlemeshko · 2024-10-16T10:07:57Z

...k/test_serverless/functional/test_suites/observability/role_management/custom_role_access.ts

+      // TODO: 'Add data' and 'Project Settings' should be hidden
+      // await testSubjects.missingOrFail('~nav-item-id-observabilityOnboarding');
+      // await testSubjects.missingOrFail('~nav-item-id-project_settings_project_nav');


I believe both Add Data and Project Settings should be hidden when role has no access to it.

wayneseymour · 2024-10-16T10:48:49Z

packages/kbn-ftr-common-functional-services/services/saml_auth/serverless/auth_provider.ts

+  );
+
+  // Return true if the value is explicitly set to 'true', otherwise false
+  return roleManagementArg?.split('=')[1] === 'true' || false;


Why is true a string literal, but false is a boolean?

I get that coercion will make true become a boolean, but that's if coercion is used.

we are looking for the exact --xpack.security.roleManagementEnabled=true match in Kibana server arguments to understand if roles management was explicitly enabled. I believe Kibana won't start if you try to pass this flag with something other than true or false

I mean only 'true' is expected to enable the functionality, so I set flag to false for any other case (including argument is completely missing)

So the return type of boolean, is only actually a boolean when it's false.
So if it's the string literal 'true' and not a boolean true, it seems like the return type should be false || 'true', and not boolean right?

I guess I'm used to seeing a boolean return type with all return calls being an actual boolean, not a string literal depending on type coercion.

Perhaps it's just a nit at this point.

I don't think you can start Kibana with --xpack.security.roleManagementEnabled='true', schema validation won't allow it. I don't try validate the cases for Kibana server, I believe FTR won't even start if literal 'true' or 'false' will be in arguments

I was right, there is no way to face literal 'true' in place:

proc [kibana] [2024-10-16T14:13:30.865+02:00][INFO ][root] Kibana is shutting down {"service":{"node":{"roles":["background_tasks","ui"]}}} proc [kibana] [2024-10-16T14:13:30.884+02:00][FATAL][root] Reason: [config validation of [xpack.security].roleManagementEnabled]: expected value of type [boolean] but got [string] proc [kibana] Error: [config validation of [xpack.security].roleManagementEnabled]: expected value of type [boolean] but got [string] proc [kibana] at ObjectType.validate (type.ts:171:13) proc [kibana] at ConfigService.validateAtPath (config_service.ts:334:19) proc [kibana] at config_service.ts:354:28 proc [kibana] at /Users/dmle/github/kibana/node_modules/rxjs/src/internal/operators/map.ts:58:33 proc [kibana] at OperatorSubscriber._this._next (/Users/dmle/github/kibana/node_modules/rxjs/src/internal/operators/OperatorSubscriber.ts:70:13)

wayneseymour · 2024-10-16T10:50:06Z

x-pack/test_serverless/README.md

 await samlAuth.invalidateM2mApiKeyWithRoleScope(roleAuthc);
+await samlAuth.deleteCustomRole();


So now the devs have to make two extra calls to finish?
Possible to make this a single call?

I don't think we need to combine it.

There is nothing new in the workflow: API key and Kibana role are 2 different entities and we already handle "cleaning" in stateful separately. In theory API key is still valid after role deletion and can be used on its own.

But now I realised it makes sense to delete cached cookie for custom role whenever setCustomRole or deleteCustomRole is called. Cookies are cached per role on the first call to speedup test execution, but as custom role is re-created we need to regenerate cookie as well. I will test it (maybe cookie will remains valid with new privileges in place) and address in the new PR

small fix dbdcd59

tl; dr; generated cookie remains valid after role is re-created after deletion or its privileges were updated. I don't think we need a logic to drop it from cache at the moment

The change I did: we need to make sure descriptors is not null aka contains some valid privileges.

wayneseymour · 2024-10-16T14:59:31Z

packages/kbn-ftr-common-functional-services/services/saml_auth/saml_auth_provider.ts

+          .set(INTERNAL_REQUEST_HEADERS)
+          .set(adminCookieHeader);
+
+        expect(status).to.be(204);


If the status is not a 204, is it still possible that it was indeed deleted?

Interesting point.
I guess both create and delete operations are tested within security plugin testing scope. 204 status is successful request by design, so I don't think we need an extra ES query to validate Kibana APIs did their job.

btw I checked that it is deleted via UI, but I think we are good to rely on status code only

Indeed, I was just wondering if that null assignment within the map symbol below could be a problem if the 204 check fails, but the item was actually deleted, then the map would be out of date and could be a difficult bug to track down.
But if you think it's a non-issue, I'm good with it.

Aha, I didn't relate it to the supportedRoleDescriptors not being updated. You are right, we should re-set it for the following test file that might use custom roles as well. Let me have a look

Moved before actual delete call bcfc49f

…m:dmlemeshko/kibana into ftr/support-custom-roles-for-oblt-project

wayneseymour

CR only, LGTM

jbudz · 2024-10-16T19:09:03Z

packages/kbn-ftr-common-functional-services/services/saml_auth/serverless/auth_provider.ts

@@ -34,8 +34,18 @@ const getDefaultServerlessRole = (projectType: string) => {
  }
 };

+const isRoleManagementExplicitlyEnabled = (args: string[]): boolean => {
+  const roleManagementArg = args.find((arg) =>
+    arg.startsWith('--xpack.security.roleManagementEnabled=')


It may simplify things to pull in the getopts package for parsing these. I see --serverless is parsed similarly elsewhere, so don't see a need to change it now, but making a note in case we refactor later.

mohamedhamed-ahmed

LGTM!!! Thanks for doing the effort here.
Tried it in a couple of tests and works as expected.

elasticmachine · 2024-10-18T11:01:05Z

💚 Build Succeeded

Buildkite Build
Commit: dbf62b3

Metrics [docs]

✅ unchanged

History

💚 Build #243233 succeeded a67c858
💔 Build #243069 failed 072f0dc
💔 Build #243020 failed 07dc5be

cc @dmlemeshko

kibanamachine · 2024-10-18T12:17:31Z

Starting backport for target branches: 8.16

https://github.com/elastic/kibana/actions/runs/11403529950

…tic#196514) ## Summary This PR makes changes in FTR `saml_auth` service to allow creating custom role for Oblt serverless project, when roles management is explicitly enabled with `--xpack.security.roleManagementEnabled=true` in Kibana server arguments. I also added [role_management/custom_role_access.ts ](x-pack/test_serverless/functional/test_suites/observability/role_management/custom_role_access.ts) as a test example. Currently roles management is enabled in `x-pack/test_serverless/functional/test_suites/observability/config.feature_flags.ts` and after this PR is merged, more tests with custom roles can be added for Oblt project. How to run tests: ``` node scripts/functional_tests --config x-pack/test_serverless/functional/test_suites/observability/config.feature_flags.ts ``` (cherry picked from commit 16c965f)

kibanamachine · 2024-10-18T12:22:08Z

💚 All backports created successfully

Status	Branch	Result
✅	8.16

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

#196514) (#196862) # Backport This will backport the following commits from `main` to `8.16`: - [[FTR] enable roles management testing for Observability project (#196514)](#196514)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)  Co-authored-by: Dzmitry Lemechko <[email protected]>

jbudz · 2024-10-18T19:58:29Z

@dmlemeshko there's a version gap with this backport. Can you check if this should also be backported to 8.x/8.17?

kibanamachine · 2024-10-25T14:52:42Z

Starting backport for target branches: 8.16, 8.x

https://github.com/elastic/kibana/actions/runs/11520516599

…tic#196514) ## Summary This PR makes changes in FTR `saml_auth` service to allow creating custom role for Oblt serverless project, when roles management is explicitly enabled with `--xpack.security.roleManagementEnabled=true` in Kibana server arguments. I also added [role_management/custom_role_access.ts ](x-pack/test_serverless/functional/test_suites/observability/role_management/custom_role_access.ts) as a test example. Currently roles management is enabled in `x-pack/test_serverless/functional/test_suites/observability/config.feature_flags.ts` and after this PR is merged, more tests with custom roles can be added for Oblt project. How to run tests: ``` node scripts/functional_tests --config x-pack/test_serverless/functional/test_suites/observability/config.feature_flags.ts ``` (cherry picked from commit 16c965f)

kibanamachine · 2024-10-25T14:57:39Z

💔 Some backports could not be created

Status	Branch	Result
❌	8.16	Cherrypick failed because the selected commit (`16c965f`) is empty. It looks like the commit was already backported in #196862
✅	8.x

Note: Successful backport PRs will be merged automatically after passing CI.

Manual backport

To create the backport manually run:

node scripts/backport --pr 196514

Questions ?

Please refer to the Backport tool documentation

…#196514) (#197845) # Backport This will backport the following commits from `main` to `8.x`: - [[FTR] enable roles management testing for Observability project (#196514)](#196514)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) \n\n### Questions ?\nPlease refer to the [Backport tool\ndocumentation](https://github.com/sqren/backport)\n\n\n\nCo-authored-by: Dzmitry Lemechko <[email protected]>"}},{"branch":"8.x","label":"v8.17.0","branchLabelMappingKey":"^v8.17.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}] BACKPORT--> Co-authored-by: Dzmitry Lemechko <[email protected]>

[ftr] allow to explicitly enable roles management for svl oblt project

95f0e45

dmlemeshko requested a review from mohamedhamed-ahmed October 16, 2024 09:56

Merge branch 'main' into ftr/support-custom-roles-for-oblt-project

07dc5be

dmlemeshko commented Oct 16, 2024

View reviewed changes

dmlemeshko self-assigned this Oct 16, 2024

dmlemeshko added release_note:skip Skip the PR/issue when compiling release notes v9.0.0 v8.16.0 backport:version Backport to applied version labels labels Oct 16, 2024

dmlemeshko marked this pull request as ready for review October 16, 2024 10:36

dmlemeshko requested review from a team as code owners October 16, 2024 10:36

wayneseymour reviewed Oct 16, 2024

View reviewed changes

Merge branch 'main' into ftr/support-custom-roles-for-oblt-project

072f0dc

dmlemeshko requested a review from wayneseymour October 16, 2024 12:14

dmlemeshko added 2 commits October 16, 2024 16:53

fix check if role descriptors are defined

dbdcd59

Merge branch 'main' into ftr/support-custom-roles-for-oblt-project

9aaa355

wayneseymour reviewed Oct 16, 2024

View reviewed changes

dmlemeshko added 2 commits October 16, 2024 17:19

reset descriptors for role before actual delete call

bcfc49f

Merge branch 'ftr/support-custom-roles-for-oblt-project' of github.co…

4a5b45d

…m:dmlemeshko/kibana into ftr/support-custom-roles-for-oblt-project

wayneseymour approved these changes Oct 16, 2024

View reviewed changes

Merge branch 'main' into ftr/support-custom-roles-for-oblt-project

a67c858

jbudz reviewed Oct 16, 2024

View reviewed changes

jbudz approved these changes Oct 16, 2024

View reviewed changes

mohamedhamed-ahmed approved these changes Oct 16, 2024

View reviewed changes

mohamedhamed-ahmed mentioned this pull request Oct 16, 2024

[Dataset Quality] Enhance Serverless FTR Tests #196599

Closed

Merge branch 'main' into ftr/support-custom-roles-for-oblt-project

9dc3648

dmlemeshko enabled auto-merge (squash) October 17, 2024 15:24

Merge branch 'main' into ftr/support-custom-roles-for-oblt-project

dbf62b3

delanni disabled auto-merge October 18, 2024 12:17

delanni merged commit 16c965f into elastic:main Oct 18, 2024
26 checks passed

kibanamachine mentioned this pull request Oct 18, 2024

[8.16] [FTR] enable roles management testing for Observability project (#196514) #196862

Merged

pheyos added v8.17.0 backport:version Backport to applied version labels and removed backport:version Backport to applied version labels labels Oct 25, 2024

kibanamachine mentioned this pull request Oct 25, 2024

[8.x] [FTR] enable roles management testing for Observability project (#196514) #197845

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[FTR] enable roles management testing for Observability project #196514

[FTR] enable roles management testing for Observability project #196514

dmlemeshko commented Oct 16, 2024 •

edited by kibanamachine

Loading

dmlemeshko Oct 16, 2024

dmlemeshko Oct 16, 2024

wayneseymour Oct 16, 2024

wayneseymour Oct 16, 2024

dmlemeshko Oct 16, 2024

dmlemeshko Oct 16, 2024

wayneseymour Oct 16, 2024

dmlemeshko Oct 16, 2024 •

edited

Loading

dmlemeshko Oct 16, 2024

wayneseymour Oct 16, 2024

dmlemeshko Oct 16, 2024 •

edited

Loading

dmlemeshko Oct 16, 2024 •

edited

Loading

wayneseymour Oct 16, 2024

dmlemeshko Oct 16, 2024

dmlemeshko Oct 16, 2024

wayneseymour Oct 16, 2024

dmlemeshko Oct 16, 2024 •

edited

Loading

dmlemeshko Oct 16, 2024

wayneseymour left a comment

jbudz Oct 16, 2024

mohamedhamed-ahmed left a comment

elasticmachine commented Oct 18, 2024

kibanamachine commented Oct 18, 2024

kibanamachine commented Oct 18, 2024

jbudz commented Oct 18, 2024

kibanamachine commented Oct 25, 2024

kibanamachine commented Oct 25, 2024

		await samlAuth.invalidateM2mApiKeyWithRoleScope(roleAuthc);
		await samlAuth.deleteCustomRole();

[FTR] enable roles management testing for Observability project #196514

[FTR] enable roles management testing for Observability project #196514

Conversation

dmlemeshko commented Oct 16, 2024 • edited by kibanamachine Loading

Summary

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

dmlemeshko Oct 16, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dmlemeshko Oct 16, 2024 • edited Loading

Choose a reason for hiding this comment

dmlemeshko Oct 16, 2024 • edited Loading

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

dmlemeshko Oct 16, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wayneseymour left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mohamedhamed-ahmed left a comment

Choose a reason for hiding this comment

elasticmachine commented Oct 18, 2024

💚 Build Succeeded

Metrics [docs]

History

kibanamachine commented Oct 18, 2024

kibanamachine commented Oct 18, 2024

💚 All backports created successfully

Questions ?

jbudz commented Oct 18, 2024

kibanamachine commented Oct 25, 2024

kibanamachine commented Oct 25, 2024

💔 Some backports could not be created

Manual backport

Questions ?

dmlemeshko commented Oct 16, 2024 •

edited by kibanamachine

Loading

dmlemeshko Oct 16, 2024 •

edited

Loading

dmlemeshko Oct 16, 2024 •

edited

Loading

dmlemeshko Oct 16, 2024 •

edited

Loading

dmlemeshko Oct 16, 2024 •

edited

Loading