From dce29e277b27f19b13ce90c4a74cac66c9fc2c79 Mon Sep 17 00:00:00 2001 From: Dzmitry Lemechko Date: Mon, 17 Jun 2024 19:57:47 +0200 Subject: [PATCH] [kbn-test] call Kibana API to fetch current user profile (#186279) Related to #185870 In this PR I move user_profile fetching out of SAML session creation to its `sessionManager.getUserData()`: - rely on Kibana Security API for both local/Kibana CI and MKI cases (currently it is cloud and cached on saml session creation) - do not cache profile data in test service, Kibana API is fast enough I deleted the tests that no longer relevant --- packages/kbn-test/src/auth/saml_auth.ts | 11 ++- packages/kbn-test/src/auth/session_manager.ts | 17 ++++- .../kbn-test/src/auth/sesson_manager.test.ts | 71 ++++++++++++++----- .../common/platform_security/index.ts | 1 - .../platform_security/request_as_viewer.ts | 36 ---------- .../page_objects/svl_common_page.ts | 6 +- .../common/platform_security/index.ts | 1 - .../user_profiles/user_profiles.ts | 2 +- .../platform_security/viewer_role_login.ts | 32 --------- .../shared/services/svl_user_manager.ts | 4 ++ 10 files changed, 81 insertions(+), 100 deletions(-) delete mode 100644 x-pack/test_serverless/api_integration/test_suites/common/platform_security/request_as_viewer.ts delete mode 100644 x-pack/test_serverless/functional/test_suites/common/platform_security/viewer_role_login.ts diff --git a/packages/kbn-test/src/auth/saml_auth.ts b/packages/kbn-test/src/auth/saml_auth.ts index a88d11c6a5bc90..99343c8c6912b6 100644 --- a/packages/kbn-test/src/auth/saml_auth.ts +++ b/packages/kbn-test/src/auth/saml_auth.ts @@ -24,11 +24,9 @@ import { export class Session { readonly cookie; readonly email; - readonly fullname; - constructor(cookie: Cookie, email: string, fullname: string) { + constructor(cookie: Cookie, email: string) { this.cookie = cookie; this.email = email; - this.fullname = fullname; } getCookieValue() { @@ -239,7 +237,7 @@ export const finishSAMLHandshake = async ({ ); }; -const getSecurityProfile = async ({ +export const getSecurityProfile = async ({ kbnHost, cookie, log, @@ -274,8 +272,7 @@ export const createCloudSAMLSession = async (params: CloudSamlSessionParams) => const { location, sid } = await createSAMLRequest(kbnHost, kbnVersion, log); const samlResponse = await createSAMLResponse({ location, ecSession, email, kbnHost, log }); const cookie = await finishSAMLHandshake({ kbnHost, samlResponse, sid, log }); - const userProfile = await getSecurityProfile({ kbnHost, cookie, log }); - return new Session(cookie, email, userProfile.full_name); + return new Session(cookie, email); }; export const createLocalSAMLSession = async (params: LocalSamlSessionParams) => { @@ -288,5 +285,5 @@ export const createLocalSAMLSession = async (params: LocalSamlSessionParams) => roles: [role], }); const cookie = await finishSAMLHandshake({ kbnHost, samlResponse, log }); - return new Session(cookie, email, fullname); + return new Session(cookie, email); }; diff --git a/packages/kbn-test/src/auth/session_manager.ts b/packages/kbn-test/src/auth/session_manager.ts index a8edca981c1d4d..1783c3f389aa1f 100644 --- a/packages/kbn-test/src/auth/session_manager.ts +++ b/packages/kbn-test/src/auth/session_manager.ts @@ -13,7 +13,12 @@ import { resolve } from 'path'; import Url from 'url'; import { KbnClient } from '../kbn_client'; import { readCloudUsersFromFile } from './helper'; -import { createCloudSAMLSession, createLocalSAMLSession, Session } from './saml_auth'; +import { + createCloudSAMLSession, + createLocalSAMLSession, + getSecurityProfile, + Session, +} from './saml_auth'; import { Role, User } from './types'; export interface HostOptions { @@ -146,8 +151,14 @@ export class SamlSessionManager { return session.getCookieValue(); } + async getEmail(role: string) { + const session = await this.getSessionByRole(role); + return session.email; + } + async getUserData(role: string) { - const { email, fullname } = await this.getSessionByRole(role); - return { email, fullname }; + const { cookie } = await this.getSessionByRole(role); + const profileData = await getSecurityProfile({ kbnHost: this.kbnHost, cookie, log: this.log }); + return profileData; } } diff --git a/packages/kbn-test/src/auth/sesson_manager.test.ts b/packages/kbn-test/src/auth/sesson_manager.test.ts index f0679917654a0d..df037ebdf04e48 100644 --- a/packages/kbn-test/src/auth/sesson_manager.test.ts +++ b/packages/kbn-test/src/auth/sesson_manager.test.ts @@ -12,7 +12,7 @@ import { Session } from './saml_auth'; import { SamlSessionManager } from './session_manager'; import * as samlAuth from './saml_auth'; import * as helper from './helper'; -import { Role, User } from './types'; +import { Role, User, UserProfile } from './types'; import { SERVERLESS_ROLES_ROOT_PATH } from '@kbn/es'; const log = new ToolingLog(); @@ -23,6 +23,7 @@ const roleEditor = 'editor'; const createLocalSAMLSessionMock = jest.spyOn(samlAuth, 'createLocalSAMLSession'); const createCloudSAMLSessionMock = jest.spyOn(samlAuth, 'createCloudSAMLSession'); +const getSecurityProfileMock = jest.spyOn(samlAuth, 'getSecurityProfile'); const readCloudUsersFromFileMock = jest.spyOn(helper, 'readCloudUsersFromFile'); const isValidHostnameMock = jest.spyOn(helper, 'isValidHostname'); @@ -42,7 +43,7 @@ describe('SamlSessionManager', () => { .KbnClient.mockImplementation(() => ({ version: { get } })); get.mockImplementation(() => Promise.resolve('8.12.0')); - createLocalSAMLSessionMock.mockResolvedValue(new Session(cookieInstance, email, fullname)); + createLocalSAMLSessionMock.mockResolvedValue(new Session(cookieInstance, testEmail)); }); const hostOptions = { @@ -59,8 +60,8 @@ describe('SamlSessionManager', () => { log, supportedRoles, }; - const email = 'testuser@elastic.com'; - const fullname = 'Test User'; + const testEmail = 'testuser@elastic.com'; + const testFullname = 'Test User'; const cookieInstance = Cookie.parse( 'sid=kbn_cookie_value; Path=/; Expires=Wed, 01 Oct 2023 07:00:00 GMT' )!; @@ -91,10 +92,26 @@ describe('SamlSessionManager', () => { expect(createCloudSAMLSessionMock.mock.calls).toHaveLength(0); }); - test(`'getUserData' should return the correct email & fullname`, async () => { + test(`'getEmail' return the correct email`, async () => { const samlSessionManager = new SamlSessionManager(samlSessionManagerOptions); - const data = await samlSessionManager.getUserData(roleViewer); - expect(data).toEqual({ email, fullname }); + const email = await samlSessionManager.getEmail(roleEditor); + expect(email).toBe(testEmail); + }); + + test(`'getUserData' should call security API and return user profile data`, async () => { + const testData: UserProfile = { + username: '6ta90xc', + roles: [roleEditor], + full_name: testFullname, + email: testEmail, + enabled: true, + elastic_cloud_user: false, + }; + getSecurityProfileMock.mockResolvedValueOnce(testData); + const samlSessionManager = new SamlSessionManager(samlSessionManagerOptions); + const userData = await samlSessionManager.getUserData(roleViewer); + + expect(userData).toEqual(testData); }); test(`throws error when role is not in 'supportedRoles'`, async () => { @@ -117,6 +134,15 @@ describe('SamlSessionManager', () => { test(`doesn't throw error when supportedRoles is not defined`, async () => { const nonExistingRole = 'tester'; + const testData: UserProfile = { + username: '6ta90xc', + roles: [nonExistingRole], + full_name: testFullname, + email: testEmail, + enabled: true, + elastic_cloud_user: false, + }; + getSecurityProfileMock.mockResolvedValueOnce(testData); const samlSessionManager = new SamlSessionManager({ hostOptions, log, @@ -127,6 +153,7 @@ describe('SamlSessionManager', () => { await samlSessionManager.getUserData(nonExistingRole); expect(createLocalSAMLSessionMock.mock.calls).toHaveLength(1); expect(createCloudSAMLSessionMock.mock.calls).toHaveLength(0); + expect(getSecurityProfileMock.mock.calls).toHaveLength(1); }); }); @@ -184,9 +211,7 @@ describe('SamlSessionManager', () => { .KbnClient.mockImplementation(() => ({ version: { get } })); get.mockImplementationOnce(() => Promise.resolve('8.12.0')); - createCloudSAMLSessionMock.mockResolvedValue( - new Session(cloudCookieInstance, cloudEmail, cloudFullname) - ); + createCloudSAMLSessionMock.mockResolvedValue(new Session(cloudCookieInstance, cloudEmail)); readCloudUsersFromFileMock.mockReturnValue(cloudUsers); }); @@ -201,9 +226,7 @@ describe('SamlSessionManager', () => { test(`'getSessionCookieForRole' should return the actual cookie value`, async () => { const samlSessionManager = new SamlSessionManager(samlSessionManagerOptions); - createCloudSAMLSessionMock.mockResolvedValue( - new Session(cloudCookieInstance, cloudEmail, cloudFullname) - ); + createCloudSAMLSessionMock.mockResolvedValue(new Session(cloudCookieInstance, cloudEmail)); const cookie = await samlSessionManager.getSessionCookieForRole(roleViewer); expect(cookie).toBe(cloudCookieInstance.value); }); @@ -223,10 +246,26 @@ describe('SamlSessionManager', () => { expect(createCloudSAMLSessionMock.mock.calls).toHaveLength(2); }); - test(`'getUserData' should return the correct email & fullname`, async () => { + test(`'getEmail' return the correct email`, async () => { const samlSessionManager = new SamlSessionManager(samlSessionManagerOptions); - const data = await samlSessionManager.getUserData(roleViewer); - expect(data).toEqual({ email: cloudEmail, fullname: cloudFullname }); + const email = await samlSessionManager.getEmail(roleViewer); + expect(email).toBe(cloudEmail); + }); + + test(`'getUserData' should call security API and return user profile data`, async () => { + const testData: UserProfile = { + username: '92qab123', + roles: [roleViewer], + full_name: cloudFullname, + email: cloudEmail, + enabled: true, + elastic_cloud_user: true, + }; + getSecurityProfileMock.mockResolvedValueOnce(testData); + const samlSessionManager = new SamlSessionManager(samlSessionManagerOptions); + const userData = await samlSessionManager.getUserData(roleViewer); + + expect(userData).toEqual(testData); }); test(`throws error for non-existing role when 'supportedRoles' is defined`, async () => { diff --git a/x-pack/test_serverless/api_integration/test_suites/common/platform_security/index.ts b/x-pack/test_serverless/api_integration/test_suites/common/platform_security/index.ts index fab6e47e87969a..e5f3f4a86a9235 100644 --- a/x-pack/test_serverless/api_integration/test_suites/common/platform_security/index.ts +++ b/x-pack/test_serverless/api_integration/test_suites/common/platform_security/index.ts @@ -22,7 +22,6 @@ export default function ({ loadTestFile }: FtrProviderContext) { loadTestFile(require.resolve('./role_mappings')); loadTestFile(require.resolve('./sessions')); loadTestFile(require.resolve('./users')); - loadTestFile(require.resolve('./request_as_viewer')); loadTestFile(require.resolve('./user_profiles')); loadTestFile(require.resolve('./views')); loadTestFile(require.resolve('./feature_check')); diff --git a/x-pack/test_serverless/api_integration/test_suites/common/platform_security/request_as_viewer.ts b/x-pack/test_serverless/api_integration/test_suites/common/platform_security/request_as_viewer.ts deleted file mode 100644 index 7931f28f55df78..00000000000000 --- a/x-pack/test_serverless/api_integration/test_suites/common/platform_security/request_as_viewer.ts +++ /dev/null @@ -1,36 +0,0 @@ -/* - * 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 type { FtrProviderContext } from '../../../ftr_provider_context'; - -export default function ({ getService }: FtrProviderContext) { - describe('security/me as viewer', () => { - const svlUserManager = getService('svlUserManager'); - const svlCommonApi = getService('svlCommonApi'); - const supertestWithoutAuth = getService('supertestWithoutAuth'); - let credentials: { Cookie: string }; - - before(async () => { - // get auth header for Viewer role - credentials = await svlUserManager.getApiCredentialsForRole('viewer'); - }); - - it('returns valid user data for authenticated request', async () => { - const { body, status } = await supertestWithoutAuth - .get('/internal/security/me') - .set(svlCommonApi.getInternalRequestHeader()) - .set(credentials); - - const userData = await svlUserManager.getUserData('viewer'); - - expect(status).to.be(200); - expect(body.full_name).to.be(userData.fullname); - expect(body.email).to.be(userData.email); - }); - }); -} diff --git a/x-pack/test_serverless/functional/page_objects/svl_common_page.ts b/x-pack/test_serverless/functional/page_objects/svl_common_page.ts index 49113e5565435d..827821c128daa2 100644 --- a/x-pack/test_serverless/functional/page_objects/svl_common_page.ts +++ b/x-pack/test_serverless/functional/page_objects/svl_common_page.ts @@ -100,14 +100,14 @@ export function SvlCommonPageProvider({ getService, getPageObjects }: FtrProvide .set(svlCommonApi.getInternalRequestHeader()) .set({ Cookie: `sid=${browserCookies[0].value}` }); - const userData = await svlUserManager.getUserData(role); + const email = await svlUserManager.getEmail(role); // email returned from API call must match the email for the specified role - if (body.email === userData.email) { + if (body.email === email) { log.debug(`The new cookie is properly set for '${role}' role`); } else { log.debug(`API response body: ${JSON.stringify(body)}`); throw new Error( - `Cookie is not set properly, expected email is '${userData.email}', but found '${body.email}'` + `Cookie is not set properly, expected email is '${email}', but found '${body.email}'` ); } // Verifying that we are logged in diff --git a/x-pack/test_serverless/functional/test_suites/common/platform_security/index.ts b/x-pack/test_serverless/functional/test_suites/common/platform_security/index.ts index 7427384fc73bf1..bbb66a98418682 100644 --- a/x-pack/test_serverless/functional/test_suites/common/platform_security/index.ts +++ b/x-pack/test_serverless/functional/test_suites/common/platform_security/index.ts @@ -9,7 +9,6 @@ import { FtrProviderContext } from '../../../ftr_provider_context'; export default function ({ loadTestFile }: FtrProviderContext) { describe('Serverless Common UI - Platform Security', function () { - loadTestFile(require.resolve('./viewer_role_login')); loadTestFile(require.resolve('./api_keys')); loadTestFile(require.resolve('./navigation/avatar_menu')); loadTestFile(require.resolve('./user_profiles/user_profiles')); diff --git a/x-pack/test_serverless/functional/test_suites/common/platform_security/user_profiles/user_profiles.ts b/x-pack/test_serverless/functional/test_suites/common/platform_security/user_profiles/user_profiles.ts index c9c61d5d3a6a95..50313bc4da7cf9 100644 --- a/x-pack/test_serverless/functional/test_suites/common/platform_security/user_profiles/user_profiles.ts +++ b/x-pack/test_serverless/functional/test_suites/common/platform_security/user_profiles/user_profiles.ts @@ -33,7 +33,7 @@ export default function ({ getPageObjects, getService }: FtrProviderContext) { const actualFullname = await pageObjects.userProfiles.getProfileFullname(); const actualEmail = await pageObjects.userProfiles.getProfileEmail(); - expect(actualFullname).to.be(userData.fullname); + expect(actualFullname).to.be(userData.full_name); expect(actualEmail).to.be(userData.email); }); diff --git a/x-pack/test_serverless/functional/test_suites/common/platform_security/viewer_role_login.ts b/x-pack/test_serverless/functional/test_suites/common/platform_security/viewer_role_login.ts deleted file mode 100644 index 767abe540c2f50..00000000000000 --- a/x-pack/test_serverless/functional/test_suites/common/platform_security/viewer_role_login.ts +++ /dev/null @@ -1,32 +0,0 @@ -/* - * 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'; - -const VIEWER_ROLE = 'viewer'; - -export default function ({ getPageObject, getService }: FtrProviderContext) { - describe(`Login as ${VIEWER_ROLE}`, function () { - const svlCommonPage = getPageObject('svlCommonPage'); - const testSubjects = getService('testSubjects'); - const svlUserManager = getService('svlUserManager'); - - before(async () => { - await svlCommonPage.loginWithRole(VIEWER_ROLE); - }); - - it('should be able to see correct profile', async () => { - await svlCommonPage.assertProjectHeaderExists(); - await svlCommonPage.assertUserAvatarExists(); - await svlCommonPage.clickUserAvatar(); - await svlCommonPage.assertUserMenuExists(); - const actualFullname = await testSubjects.getVisibleText('contextMenuPanelTitle'); - const userData = await svlUserManager.getUserData(VIEWER_ROLE); - expect(actualFullname).to.be(userData.fullname); - }); - }); -} diff --git a/x-pack/test_serverless/shared/services/svl_user_manager.ts b/x-pack/test_serverless/shared/services/svl_user_manager.ts index 3568da0c6b87b1..12fa18eee66731 100644 --- a/x-pack/test_serverless/shared/services/svl_user_manager.ts +++ b/x-pack/test_serverless/shared/services/svl_user_manager.ts @@ -78,6 +78,10 @@ export function SvlUserManagerProvider({ getService }: FtrProviderContext) { async getApiCredentialsForRole(role: string) { return sessionManager.getApiCredentialsForRole(role); }, + async getEmail(role: string) { + return sessionManager.getEmail(role); + }, + async getUserData(role: string) { return sessionManager.getUserData(role); },