Skip to content

Commit

Permalink
[kbn-test] call Kibana API to fetch current user profile (#186279)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
dmlemeshko authored Jun 17, 2024
1 parent a9f5375 commit dce29e2
Show file tree
Hide file tree
Showing 10 changed files with 81 additions and 100 deletions.
11 changes: 4 additions & 7 deletions packages/kbn-test/src/auth/saml_auth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -239,7 +237,7 @@ export const finishSAMLHandshake = async ({
);
};

const getSecurityProfile = async ({
export const getSecurityProfile = async ({
kbnHost,
cookie,
log,
Expand Down Expand Up @@ -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) => {
Expand All @@ -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);
};
17 changes: 14 additions & 3 deletions packages/kbn-test/src/auth/session_manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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;
}
}
71 changes: 55 additions & 16 deletions packages/kbn-test/src/auth/sesson_manager.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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');

Expand All @@ -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 = {
Expand All @@ -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'
)!;
Expand Down Expand Up @@ -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 () => {
Expand All @@ -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,
Expand All @@ -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);
});
});

Expand Down Expand Up @@ -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);
});

Expand All @@ -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);
});
Expand All @@ -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 () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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'));
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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'));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});

Expand Down

This file was deleted.

4 changes: 4 additions & 0 deletions x-pack/test_serverless/shared/services/svl_user_manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
},
Expand Down

0 comments on commit dce29e2

Please sign in to comment.