Skip to content
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

Failing test: X-Pack Security API Integration Tests (Session Idle Timeout).x-pack/test/security_api_integration/tests/session_idle/cleanup·ts - security APIs - Session Idle Session Idle cleanup should properly clean up session expired because of idle timeout #121482

Open
kibanamachine opened this issue Dec 17, 2021 · 30 comments · Fixed by #121961, #126966, #130091 or #153303
Assignees
Labels
failed-test A test failure on a tracked branch, potentially flaky-test Team:Security Team focused on: Auth, Users, Roles, Spaces, Audit Logging, and more!

Comments

@kibanamachine
Copy link
Contributor

kibanamachine commented Dec 17, 2021

A test failed on a tracked branch

Error: expected 200 "OK", got 401 "Unauthorized"
    at Test._assertStatus (/opt/local-ssd/buildkite/builds/kb-n2-4-c3b96ff0bbb5ca35/elastic/kibana-hourly/kibana/node_modules/supertest/lib/test.js:268:12)
    at Test._assertFunction (/opt/local-ssd/buildkite/builds/kb-n2-4-c3b96ff0bbb5ca35/elastic/kibana-hourly/kibana/node_modules/supertest/lib/test.js:283:11)
    at Test.assert (/opt/local-ssd/buildkite/builds/kb-n2-4-c3b96ff0bbb5ca35/elastic/kibana-hourly/kibana/node_modules/supertest/lib/test.js:173:18)
    at assert (/opt/local-ssd/buildkite/builds/kb-n2-4-c3b96ff0bbb5ca35/elastic/kibana-hourly/kibana/node_modules/supertest/lib/test.js:131:12)
    at /opt/local-ssd/buildkite/builds/kb-n2-4-c3b96ff0bbb5ca35/elastic/kibana-hourly/kibana/node_modules/supertest/lib/test.js:128:5
    at Test.Request.callback (/opt/local-ssd/buildkite/builds/kb-n2-4-c3b96ff0bbb5ca35/elastic/kibana-hourly/kibana/node_modules/supertest/node_modules/superagent/lib/node/index.js:718:3)
    at /opt/local-ssd/buildkite/builds/kb-n2-4-c3b96ff0bbb5ca35/elastic/kibana-hourly/kibana/node_modules/supertest/node_modules/superagent/lib/node/index.js:906:18
    at IncomingMessage.<anonymous> (/opt/local-ssd/buildkite/builds/kb-n2-4-c3b96ff0bbb5ca35/elastic/kibana-hourly/kibana/node_modules/supertest/node_modules/superagent/lib/node/parsers/json.js:19:7)
    at IncomingMessage.emit (node:events:402:35)
    at endReadableNT (node:internal/streams/readable:1343:12)

First failure: CI Build - main

@kibanamachine kibanamachine added the failed-test A test failure on a tracked branch, potentially flaky-test label Dec 17, 2021
@botelastic botelastic bot added the needs-team Issues missing a team label label Dec 17, 2021
@afharo afharo added the Team:Security Team focused on: Auth, Users, Roles, Spaces, Audit Logging, and more! label Dec 17, 2021
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-security (Team:Security)

@botelastic botelastic bot removed the needs-team Issues missing a team label label Dec 17, 2021
@azasypkin
Copy link
Member

Error: expected 200 "OK", got 401 "Unauthorized"

I cannot think of any reason why we'd get error like that in this test. Here is the assertion that fails:

const response = await supertest
.post('/internal/security/login')
.set('kbn-xsrf', 'xxx')
.send({
providerType: 'basic',
providerName: 'basic1',
currentURL: '/',
params: { username: basicUsername, password: basicPassword },
})
.expect(200);

It's a simple basic login API request with the credentials of the pre-populated admin test user. I'll keep eye on this issue and either dig deeper if it happens again or close it as an intermittent if we don't see failures during next few weeks.

@kibanamachine
Copy link
Contributor Author

New failure: CI Build - 8.0

@azasypkin
Copy link
Member

Investigating the failure...

@azasypkin
Copy link
Member

azasypkin commented Dec 24, 2021

In this test we take the following steps:

  1. Log in via '/internal/security/login'
  2. Verify that session cookie is valid via '/internal/security/me'
  3. Wait until session expires (idle timeout is 5s) and the cleanup job removes it
  4. Verify that session is no longer valid via '/internal/security/me'

The test fails on the step 2 likely because sometimes Kibana is too slow on CI and it takes more than 5s between steps 1 and 2.

I'm doubling timeouts in configuration for these tests and will see if it helps: https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/37

@kibanamachine
Copy link
Contributor Author

New failure: CI Build - 8.0

@kibanamachine kibanamachine reopened this Dec 28, 2021
@kibanamachine
Copy link
Contributor Author

New failure: CI Build - 8.0

@azasypkin
Copy link
Member

New failure: CI Build - 8.0

Closing again since 8.0/7.x backports haven't been merged yet due to merge conflicts (CI run pending).

@kibanamachine
Copy link
Contributor Author

New failure: CI Build - main

@azasypkin
Copy link
Member

New failure: CI Build - main

Before the fix, the symptom in all failed builds was Error: expected 200 "OK", got 401 "Unauthorized", now it's different: Error: expected 1 to equal 0. Let's wait and see if it's a transient issue or not. If not, we'll need to make sure that's not related to the recent changes in the cleanup logic in #122419

@kibanamachine
Copy link
Contributor Author

New failure: CI Build - main

@azasypkin
Copy link
Member

сс @thomheymann, just in case you can notice what can be the reason here.

@kibanamachine
Copy link
Contributor Author

New failure: CI Build - 8.1

@jportner
Copy link
Contributor

jportner commented Mar 4, 2022

New failure: CI Build - 8.1

This is also the "new error":

└-: security APIs - Session Idle
--
  | └-> "before all" hook: beforeTestSuite.trigger in "security APIs - Session Idle"
  | └-: Session Idle cleanup
  | └-> "before all" hook: beforeTestSuite.trigger for "should properly clean up session expired because of idle timeout"
  | └-> should properly clean up session expired because of idle timeout
  | └-> "before each" hook: global before each for "should properly clean up session expired because of idle timeout"
  | └-> "before each" hook for "should properly clean up session expired because of idle timeout"
  | └- ✖ fail: security APIs - Session Idle Session Idle cleanup should properly clean up session expired because of idle timeout
  | │      Error: expected 1 to equal 0
  | │       at Assertion.assert (/opt/local-ssd/buildkite/builds/kb-n2-4-a6f049047d437185/elastic/kibana-hourly/kibana/node_modules/@kbn/expect/expect.js:100:11)
  | │       at Assertion.be.Assertion.equal (/opt/local-ssd/buildkite/builds/kb-n2-4-a6f049047d437185/elastic/kibana-hourly/kibana/node_modules/@kbn/expect/expect.js:227:8)
  | │       at Assertion.be (/opt/local-ssd/buildkite/builds/kb-n2-4-a6f049047d437185/elastic/kibana-hourly/kibana/node_modules/@kbn/expect/expect.js:69:22)
  | │       at Context.<anonymous> (test/security_api_integration/tests/session_idle/cleanup.ts:111:54)
  | │       at runMicrotasks (<anonymous>)
  | │       at processTicksAndRejections (node:internal/process/task_queues:96:5)
  | │       at Object.apply (/opt/local-ssd/buildkite/builds/kb-n2-4-a6f049047d437185/elastic/kibana-hourly/kibana/node_modules/@kbn/test/target_node/functional_test_runner/lib/mocha/wrap_function.js:87:16)

The assertion that's failing is on line 111 below:

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.
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);

I think I know what's causing the test failure. We changed deleteByQuery to use a combination of find + bulk-delete so that we can log audit records for each deleted session document. In the delete step, we intentionally use refresh: false to ensure it's as fast as possible in case we have to delete multiple batches of sessions:

const bulkResponse = await this.options.elasticsearchClient.bulk(
{
index: this.indexName,
operations,
refresh: false,
},
{ ignore: [409, 404] }
);

However, we don't appear to be triggering an index refresh after the whole cleanup process is finished. I believe we intended to add that but it got lost in the mix.

@jportner jportner linked a pull request Mar 4, 2022 that will close this issue
@kibanamachine kibanamachine reopened this Mar 8, 2022
@kibanamachine
Copy link
Contributor Author

New failure: CI Build - main

@kibanamachine
Copy link
Contributor Author

New failure: CI Build - main

mistic added a commit that referenced this issue Mar 10, 2022
@kibanamachine
Copy link
Contributor Author

New failure: CI Build - main

@kibanamachine
Copy link
Contributor Author

New failure: CI Build - main

@kibanamachine
Copy link
Contributor Author

New failure: CI Build - main

@jbudz
Copy link
Member

jbudz commented Mar 15, 2023

/skip

@kibanamachine
Copy link
Contributor Author

Skipped

main: 3dabc61

@jeramysoucy jeramysoucy self-assigned this Mar 20, 2023
jeramysoucy added a commit that referenced this issue Mar 23, 2023
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
jennypavlova pushed a commit to jennypavlova/kibana that referenced this issue Mar 24, 2023
Closes elastic#152260
Closes elastic#121482
Closes elastic#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
@kibanamachine kibanamachine reopened this May 12, 2023
@kibanamachine
Copy link
Contributor Author

New failure: CI Build - main

@kibanamachine
Copy link
Contributor Author

New failure: CI Build - main

@kibanamachine
Copy link
Contributor Author

New failure: CI Build - 8.8

@kibanamachine
Copy link
Contributor Author

New failure: CI Build - 8.8

@jeramysoucy
Copy link
Contributor

Ran another flaky test runner just to be sure, but this looks tied to a series of CI failures on Friday.

@kibanamachine
Copy link
Contributor Author

New failure: kibana-on-merge - 7.17

@kibanamachine kibanamachine reopened this Aug 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
failed-test A test failure on a tracked branch, potentially flaky-test Team:Security Team focused on: Auth, Users, Roles, Spaces, Audit Logging, and more!
Projects
None yet
8 participants