-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[FTR] enable roles management testing for Observability project #196514
Changes from 5 commits
95f0e45
07dc5be
072f0dc
dbdcd59
9aaa355
bcfc49f
4a5b45d
a67c858
9dc3648
dbf62b3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -34,8 +34,18 @@ const getDefaultServerlessRole = (projectType: string) => { | |
} | ||
}; | ||
|
||
const isRoleManagementExplicitlyEnabled = (args: string[]): boolean => { | ||
const roleManagementArg = args.find((arg) => | ||
arg.startsWith('--xpack.security.roleManagementEnabled=') | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It may simplify things to pull in the |
||
); | ||
|
||
// Return true if the value is explicitly set to 'true', otherwise false | ||
return roleManagementArg?.split('=')[1] === 'true' || false; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I get that coercion will make true become a boolean, but that's if coercion is used. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we are looking for the exact There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I mean only 'true' is expected to enable the functionality, so I set flag to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So the return type of boolean, is only actually a boolean when it's false. 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think you can start Kibana with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was right, there is no way to face literal 'true' in place:
|
||
}; | ||
|
||
export class ServerlessAuthProvider implements AuthProvider { | ||
private readonly projectType: string; | ||
private readonly roleManagementEnabled: boolean; | ||
private readonly rolesDefinitionPath: string; | ||
|
||
constructor(config: Config) { | ||
|
@@ -45,6 +55,10 @@ export class ServerlessAuthProvider implements AuthProvider { | |
return acc + (match ? match[1] : ''); | ||
}, '') as ServerlessProjectType; | ||
|
||
// Indicates whether role management was explicitly enabled using | ||
// the `--xpack.security.roleManagementEnabled=true` flag. | ||
this.roleManagementEnabled = isRoleManagementExplicitlyEnabled(kbnServerArgs); | ||
|
||
if (!isServerlessProjectType(this.projectType)) { | ||
throw new Error(`Unsupported serverless projectType: ${this.projectType}`); | ||
} | ||
|
@@ -70,7 +84,9 @@ export class ServerlessAuthProvider implements AuthProvider { | |
} | ||
|
||
isCustomRoleEnabled() { | ||
return projectTypesWithCustomRolesEnabled.includes(this.projectType); | ||
return ( | ||
projectTypesWithCustomRolesEnabled.includes(this.projectType) || this.roleManagementEnabled | ||
); | ||
} | ||
|
||
getCustomRole() { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -207,6 +207,8 @@ describe("my internal APIs test suite", async function() { | |
With custom native roles now enabled for the Security and Search projects on MKI, the FTR supports | ||
defining and authenticating with custom roles in both UI functional tests and API integration tests. | ||
|
||
To test role management within the Observability project, you can execute the tests using the existing [config.feature_flags.ts](x-pack/test_serverless/functional/test_suites/observability/config.feature_flags.ts), where this functionality is explicitly enabled. Though the config is not run on MKI, it provides the ability to test custom roles in Kibana CI before the functionality is enabled in MKI. When roles management is enabled on MKI, these tests can be migrated to the regular FTR config and will be run on MKI. | ||
|
||
For compatibility with MKI, the role name `customRole` is reserved for use in tests. The test user is automatically assigned to this role, but before logging in via the browser, generating a cookie header, or creating an API key in each test suite, the role’s privileges must be updated. | ||
|
||
Note: We are still working on a solution to run these tests against MKI. In the meantime, please tag the suite with `skipMKI`. | ||
|
@@ -229,6 +231,9 @@ await samlAuth.setCustomRole({ | |
}); | ||
// Then, log in via the browser as a user with the newly defined privileges | ||
await pageObjects.svlCommonPage.loginWithCustomRole(); | ||
|
||
// Make sure to delete the custom role in the 'after' hook | ||
await samlAuth.deleteCustomRole(); | ||
``` | ||
|
||
FTR api_integration test example: | ||
|
@@ -251,8 +256,9 @@ await samlAuth.setCustomRole({ | |
// Then, generate an API key with the newly defined privileges | ||
const roleAuthc = await samlAuth.createM2mApiKeyWithRoleScope('customRole'); | ||
|
||
// Remember to invalidate the API key after use | ||
// Remember to invalidate the API key after use and delete the custom role | ||
await samlAuth.invalidateM2mApiKeyWithRoleScope(roleAuthc); | ||
await samlAuth.deleteCustomRole(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So now the devs have to make two extra calls to finish? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 |
||
``` | ||
|
||
### Testing with feature flags | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,100 @@ | ||
/* | ||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
* or more contributor license agreements. Licensed under the Elastic License | ||
* 2.0; you may not use this file except in compliance with the Elastic License | ||
* 2.0. | ||
*/ | ||
|
||
import expect from '@kbn/expect'; | ||
import { FtrProviderContext } from '../../../ftr_provider_context'; | ||
import { RoleCredentials } from '../../../../shared/services'; | ||
|
||
export default function ({ getPageObjects, getService }: FtrProviderContext) { | ||
const pageObjects = getPageObjects(['svlCommonPage', 'timePicker', 'common', 'header']); | ||
const samlAuth = getService('samlAuth'); | ||
const supertestWithoutAuth = getService('supertestWithoutAuth'); | ||
const esArchiver = getService('esArchiver'); | ||
const kibanaServer = getService('kibanaServer'); | ||
const testSubjects = getService('testSubjects'); | ||
let roleAuthc: RoleCredentials; | ||
|
||
describe('With custom role', function () { | ||
// skipping on MKI while we are working on a solution | ||
this.tags(['skipMKI']); | ||
before(async () => { | ||
await esArchiver.loadIfNeeded('test/functional/fixtures/es_archiver/logstash_functional'); | ||
await kibanaServer.importExport.load('test/functional/fixtures/kbn_archiver/discover'); | ||
await kibanaServer.uiSettings.update({ | ||
defaultIndex: 'logstash-*', | ||
}); | ||
await samlAuth.setCustomRole({ | ||
elasticsearch: { | ||
indices: [{ names: ['logstash-*'], privileges: ['read', 'view_index_metadata'] }], | ||
}, | ||
kibana: [ | ||
{ | ||
feature: { | ||
discover: ['read'], | ||
}, | ||
spaces: ['*'], | ||
}, | ||
], | ||
}); | ||
// login with custom role | ||
await pageObjects.svlCommonPage.loginWithCustomRole(); | ||
await pageObjects.svlCommonPage.assertUserAvatarExists(); | ||
}); | ||
|
||
after(async () => { | ||
await esArchiver.unload('test/functional/fixtures/es_archiver/logstash_functional'); | ||
await kibanaServer.importExport.unload('test/functional/fixtures/kbn_archiver/discover'); | ||
await kibanaServer.uiSettings.replace({}); | ||
await kibanaServer.savedObjects.cleanStandardList(); | ||
if (roleAuthc) { | ||
await samlAuth.invalidateM2mApiKeyWithRoleScope(roleAuthc); | ||
} | ||
// delete custom role | ||
await samlAuth.deleteCustomRole(); | ||
}); | ||
|
||
it('should have limited navigation menu', async () => { | ||
await pageObjects.svlCommonPage.assertUserAvatarExists(); | ||
// discover navigation link is present | ||
await testSubjects.existOrFail('~nav-item-id-last-used-logs-viewer'); | ||
|
||
// all other links in navigation menu are hidden | ||
await testSubjects.missingOrFail('~nav-item-id-dashboards'); | ||
await testSubjects.missingOrFail('~nav-item-id-observability-overview:alerts'); | ||
await testSubjects.missingOrFail('~nav-item-id-observability-overview:cases'); | ||
await testSubjects.missingOrFail('~nav-item-id-slo'); | ||
await testSubjects.missingOrFail('~nav-item-id-aiops'); | ||
await testSubjects.missingOrFail('~nav-item-id-inventory'); | ||
await testSubjects.missingOrFail('~nav-item-id-apm'); | ||
await testSubjects.missingOrFail('~nav-item-id-metrics'); | ||
await testSubjects.missingOrFail('~nav-item-id-synthetics'); | ||
|
||
// 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'); | ||
Comment on lines
+76
to
+78
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
}); | ||
|
||
it('should access Discover app', async () => { | ||
await pageObjects.common.navigateToApp('discover'); | ||
await pageObjects.timePicker.setDefaultAbsoluteRange(); | ||
await pageObjects.header.waitUntilLoadingHasFinished(); | ||
expect(await testSubjects.exists('unifiedHistogramChart')).to.be(true); | ||
expect(await testSubjects.exists('discoverQueryHits')).to.be(true); | ||
}); | ||
|
||
it('should access console with API key', async () => { | ||
roleAuthc = await samlAuth.createM2mApiKeyWithRoleScope('customRole'); | ||
const { body } = await supertestWithoutAuth | ||
.get('/api/console/api_server') | ||
.set(roleAuthc.apiKeyHeader) | ||
.set(samlAuth.getInternalRequestHeader()) | ||
.set({ 'kbn-xsrf': 'true' }) | ||
.expect(200); | ||
expect(body.es).to.be.ok(); | ||
}); | ||
}); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
/* | ||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
* or more contributor license agreements. Licensed under the Elastic License | ||
* 2.0; you may not use this file except in compliance with the Elastic License | ||
* 2.0. | ||
*/ | ||
|
||
import { FtrProviderContext } from '../../../ftr_provider_context'; | ||
|
||
export default function ({ loadTestFile }: FtrProviderContext) { | ||
describe('Role Management', function () { | ||
loadTestFile(require.resolve('./custom_role_access')); | ||
}); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -53,6 +53,8 @@ export default function ({ getPageObjects, getService }: FtrProviderContext) { | |
if (roleAuthc) { | ||
await samlAuth.invalidateM2mApiKeyWithRoleScope(roleAuthc); | ||
} | ||
// delete custom role | ||
await samlAuth.deleteCustomRole(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we need to delete role, because platform security tests validate security roles UI and roles list. |
||
}); | ||
|
||
it('should have limited navigation menu', async () => { | ||
|
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.
If the status is not a 204, is it still possible that it was indeed deleted?
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.
Interesting point.
I guess both
create
anddelete
operations are tested within security plugin testing scope. 204 status issuccessful request
by design, so I don't think we need an extra ES query to validate Kibana APIs did their job.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.
btw I checked that it is deleted via UI, but I think we are good to rely on status code only
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.
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.
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.
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 lookThere 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.
Moved before actual delete call bcfc49f