Skip to content

Commit

Permalink
Fixes session idle timeout API test (#153303)
Browse files Browse the repository at this point in the history
Closes #152260
Closes #121482
Closes #136688

## Description
- Adds security index refresh to `getSessionInfo` to ensure each query
is running on updated data
- Triggers the cleanup routine just before checking idle session
timeouts to increase determinism (same methodology used in concurrent
sessions tests)
- Adds a short static delay when testing session extend to ensure the
original session time has somewhat elapsed (when this was failing it was
only by a few milliseconds)

### Tests
x-pack/test/security_api_integration/session_idle.config.ts...
- 'should properly clean up session expired because of idle timeout'
- 'should properly clean up session expired because of idle timeout when
providers override global session config'
- 'should extend the session'

### Flaky Test Runner
https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/2031
  • Loading branch information
jeramysoucy authored Mar 23, 2023
1 parent 552e472 commit fb67e5a
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 26 deletions.
3 changes: 3 additions & 0 deletions x-pack/test/security_api_integration/session_idle.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ export default async function ({ readConfigFile }: FtrConfigProviderContext) {
const kibanaPort = xPackAPITestsConfig.get('servers.kibana.port');
const idpPath = require.resolve('@kbn/security-api-integration-helpers/saml/idp_metadata.xml');

const testEndpointsPlugin = resolve(__dirname, '../security_functional/plugins/test_endpoints');

return {
testFiles: [resolve(__dirname, './tests/session_idle')],
services,
Expand All @@ -41,6 +43,7 @@ export default async function ({ readConfigFile }: FtrConfigProviderContext) {
...xPackAPITestsConfig.get('kbnTestServer'),
serverArgs: [
...xPackAPITestsConfig.get('kbnTestServer.serverArgs'),
`--plugin-path=${testEndpointsPlugin}`,
'--xpack.security.session.idleTimeout=10s',
'--xpack.security.session.cleanupInterval=20s',
`--xpack.security.authc.providers=${JSON.stringify({
Expand Down
56 changes: 37 additions & 19 deletions x-pack/test/security_api_integration/tests/session_idle/cleanup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ export default function ({ getService }: FtrProviderContext) {
const es = getService('es');
const esDeleteAllIndices = getService('esDeleteAllIndices');
const config = getService('config');
const retry = getService('retry');
const log = getService('log');
const randomness = getService('randomness');
const { username: basicUsername, password: basicPassword } = adminTestUser;
Expand Down Expand Up @@ -48,6 +49,7 @@ export default function ({ getService }: FtrProviderContext) {
}

async function getNumberOfSessionDocuments() {
await es.indices.refresh({ index: '.kibana_security_session*' });
return (
// @ts-expect-error doesn't handle total as number
(await es.search({ index: '.kibana_security_session*' })).hits.total.value as number
Expand Down Expand Up @@ -79,9 +81,19 @@ export default function ({ getService }: FtrProviderContext) {
return cookie;
}

// Failing: See https://github.com/elastic/kibana/issues/121482
// Failing: See https://github.com/elastic/kibana/issues/152260
describe.skip('Session Idle cleanup', () => {
async function runCleanupTaskSoon() {
// In most cases, an error would mean the task is currently running so let's run it again
await retry.tryForTime(30000, async () => {
await supertest
.post('/session/_run_cleanup')
.set('kbn-xsrf', 'xxx')
.auth(adminTestUser.username, adminTestUser.password)
.send()
.expect(200);
});
}

describe('Session Idle cleanup', () => {
beforeEach(async () => {
await es.cluster.health({ index: '.kibana_security_session*', wait_for_status: 'green' });
await esDeleteAllIndices('.kibana_security_session*');
Expand All @@ -101,20 +113,23 @@ export default function ({ getService }: FtrProviderContext) {
params: { username: basicUsername, password: basicPassword },
})
.expect(200);
await es.indices.refresh({ index: '.kibana_security_session*' });

const sessionCookie = parseCookie(response.headers['set-cookie'][0])!;
await checkSessionCookie(sessionCookie, basicUsername, { type: 'basic', name: 'basic1' });
expect(await getNumberOfSessionDocuments()).to.be(1);

// Cleanup routine runs every 20s, and idle timeout threshold is three times larger than 10s
// idle timeout, let's wait for 60s to make sure cleanup routine runs when idle timeout
// threshold is exceeded.
// Poke the background task to run
await runCleanupTaskSoon();
log.debug('Waiting for cleanup job to run...');
await setTimeoutAsync(60000);

// Session info is removed from the index and cookie isn't valid anymore
expect(await getNumberOfSessionDocuments()).to.be(0);
// Cleanup routine runs every 20s, and idle timeout threshold is three times larger than 10s
// idle timeout (so, 30s). We just triggered the cleanup, so we'll wait for 40s to make sure
// cleanup routine runs after the idle timeout threshold is exceeded. Then we'll wait for a
// correct response.
await setTimeoutAsync(40000);
await retry.tryForTime(20000, async () => {
// Session info is removed from the index and cookie isn't valid anymore
expect(await getNumberOfSessionDocuments()).to.be(0);
});

log.debug(`Authenticating as ${basicUsername} with invalid session cookie.`);
await supertest
Expand Down Expand Up @@ -144,7 +159,6 @@ export default function ({ getService }: FtrProviderContext) {
params: { username: basicUsername, password: basicPassword },
})
.expect(200);
await es.indices.refresh({ index: '.kibana_security_session*' });

const basicSessionCookie = parseCookie(response.headers['set-cookie'][0])!;
await checkSessionCookie(basicSessionCookie, basicUsername, {
Expand All @@ -153,14 +167,19 @@ export default function ({ getService }: FtrProviderContext) {
});
expect(await getNumberOfSessionDocuments()).to.be(4);

// Cleanup routine runs every 20s, and idle timeout threshold is three times larger than 10s
// idle timeout, let's wait for 60s to make sure cleanup routine runs when idle timeout
// threshold is exceeded.
// Poke the background task to run
await runCleanupTaskSoon();
log.debug('Waiting for cleanup job to run...');
await setTimeoutAsync(60000);
// Cleanup routine runs every 20s, and idle timeout threshold is three times larger than 10s
// idle timeout (so, 30s). We just triggered the cleanup, so we'll wait for 40s to make sure
// cleanup routine runs after the idle timeout threshold is exceeded. Then we'll wait for a
// correct response.
await setTimeoutAsync(40000);
await retry.tryForTime(20000, async () => {
// Session for basic and SAML that used global session settings should not be valid anymore.
expect(await getNumberOfSessionDocuments()).to.be(2);
});

// Session for basic and SAML that used global session settings should not be valid anymore.
expect(await getNumberOfSessionDocuments()).to.be(2);
await supertest
.get('/internal/security/me')
.set('kbn-xsrf', 'xxx')
Expand Down Expand Up @@ -196,7 +215,6 @@ export default function ({ getService }: FtrProviderContext) {
params: { username: basicUsername, password: basicPassword },
})
.expect(200);
await es.indices.refresh({ index: '.kibana_security_session*' });

let sessionCookie = parseCookie(response.headers['set-cookie'][0])!;
await checkSessionCookie(sessionCookie, basicUsername, { type: 'basic', name: 'basic1' });
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

import { parse as parseCookie, Cookie } from 'tough-cookie';
import expect from '@kbn/expect';
import { setTimeout as setTimeoutAsync } from 'timers/promises';
import { FtrProviderContext } from '../../ftr_provider_context';

export default function ({ getService }: FtrProviderContext) {
Expand All @@ -23,38 +24,40 @@ export default function ({ getService }: FtrProviderContext) {
// session id used during a test run, and we might have lots of unrelated
// sessions in the index created by the tests that didn't clean up the index.
async function getSessionsCreatedAt() {
await es.indices.refresh({ index: '.kibana_security_session*' });
const searchResponse = await es.search<{ createdAt: number }>({
index: '.kibana_security_session*',
});

return searchResponse.hits.hits.map((hit) => hit._source!.createdAt).sort();
}

// Failing: See https://github.com/elastic/kibana/issues/136688
describe.skip('Session', () => {
describe('Session', () => {
let sessionCookie: Cookie;

const saveCookie = async (response: any) => {
// save the response cookie, and pass back the result
sessionCookie = parseCookie(response.headers['set-cookie'][0])!;
return response;
};
const getSessionInfo = async () =>
supertestWithoutAuth
const getSessionInfo = async () => {
return supertestWithoutAuth
.get('/internal/security/session')
.set('kbn-xsrf', 'xxx')
.set('kbn-system-request', 'true')
.set('Cookie', sessionCookie.cookieString())
.send()
.expect(200);
const extendSession = async () =>
supertestWithoutAuth
};
const extendSession = async () => {
return supertestWithoutAuth
.post('/internal/security/session')
.set('kbn-xsrf', 'xxx')
.set('Cookie', sessionCookie.cookieString())
.send()
.expect(302)
.then(saveCookie);
};

beforeEach(async () => {
await supertestWithoutAuth
Expand Down Expand Up @@ -94,14 +97,19 @@ export default function ({ getService }: FtrProviderContext) {
it('should extend the session', async () => {
// browsers will follow the redirect and return the new session info, but this testing framework does not
// we simulate that behavior in this test by sending another GET request
const { body } = await getSessionInfo();

// Make sure that all sessions have populated `createdAt` field.
const sessionsCreatedAtBeforeExtension = await getSessionsCreatedAt();
expect(sessionsCreatedAtBeforeExtension.every((createdAt) => createdAt > 0)).to.be(true);

// Make sure that the session time has somewhat elapsed to ensure
// there is a noticable difference after extending the session
await setTimeoutAsync(200);
const { body } = await getSessionInfo();
await extendSession();
const { body: body2 } = await getSessionInfo();

// Check that the extended session has more time left than before
expect(body2.expiresInMs).not.to.be.lessThan(body.expiresInMs);

// Check that session extension didn't alter `createdAt`.
Expand Down

0 comments on commit fb67e5a

Please sign in to comment.