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

Add session id to audit log #85451

Merged
merged 4 commits into from
Dec 14, 2020
Merged

Add session id to audit log #85451

merged 4 commits into from
Dec 14, 2020

Conversation

thomheymann
Copy link
Contributor

Summary

This PR adds session id output to the audit log.

This is useful for auditors in order to trace different user sessions.

Checklist

Delete any items that are not applicable to this PR.

@thomheymann thomheymann added v8.0.0 release_note:skip Skip the PR/issue when compiling release notes v7.11.0 labels Dec 9, 2020
@thomheymann thomheymann marked this pull request as ready for review December 9, 2020 18:30
@thomheymann thomheymann requested a review from a team as a code owner December 9, 2020 18:30
@azasypkin
Copy link
Member

ACK: will review today

Copy link
Member

@azasypkin azasypkin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, just a couple of nits, thanks!

const sessionCookieValue = await this.options.sessionCookie.get(request);
if (sessionCookieValue) {
return sessionCookieValue.sid;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question/nit: how do you feel about returning null instead of undefined here? So that it's consistent with getCurrentUser, Session.get and etc?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I find null a cause of unintended bugs and behaviours and the distinctions between "not set" (undefined) and "set but empty" (null) to be rarely important.

e.g. null can cause bugs when using optional parameters and default values since the default value will not be used. null is also of type object which is weird and can cause bugs so I rarely use it.

Having said that, I think in this case, we really are dealing with the "not set" scenario so I think undefined is correct.

Why are you using null for the other methods?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are you using null for the other methods?

TL;DR: We don't have any guidelines on that yet and even though I have a slightly stronger opinion on getCurrentUser and Session.get as explained below, I don't have the same strong reasons to use null in this particular case except for consistency. Both would work for me and I trust your judgement here 👍

I think that we historically treated null not as set but empty, but more like an intentional absence of any object value, like the user or session objects are absent in a particular context, it's expected and intentional, and we explicitly manifest that with null. As a side benefit, when you return null; you explicitly define an exit point, where undefined can be either intentional or not (e.g. forgotten return statement).

Regarding default and optional parameters, I'd say it's more about personal preference, I find the code that is explicit about using default and optional parameters a little bit easier to read and understand, and type system will prevent unintentional behavior in case parameter cannot be null.

Copy link
Contributor Author

@thomheymann thomheymann Dec 14, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the definition makes sense* then. For object values use null, for primitives use undefined. That also explains why null is of type object.

* I'm using "sense" in the loosest meaning of the word here since most other programming languages have a single type the denote absence of a value and get by just fine 🤷‍♀️

@thomheymann
Copy link
Contributor Author

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💛 Build succeeded, but was flaky


Test Failures

X-Pack API Integration Tests.x-pack/test/api_integration/apis/security_solution/kpi_network·ts.apis SecuritySolution Endpoints Kpi Network With packetbeat Make sure that we get KpiNetwork networkEvents data

Link to Jenkins

Standard Out

Failed Tests Reporter:
  - Test has not failed recently on tracked branches

[00:00:00]       │
[00:00:00]         └-: apis
[00:00:00]           └-> "before all" hook
[00:05:18]           └-: SecuritySolution Endpoints
[00:05:18]             └-> "before all" hook
[00:05:21]             └-: Kpi Network
[00:05:21]               └-> "before all" hook
[00:05:23]               └-: With packetbeat
[00:05:23]                 └-> "before all" hook
[00:05:23]                 └-> "before all" hook
[00:05:23]                   │ info [packetbeat/default] Loading "mappings.json"
[00:05:23]                   │ info [packetbeat/default] Loading "data.json.gz"
[00:05:23]                   │ info [o.e.c.m.MetadataCreateIndexService] [kibana-ci-immutable-ubuntu-16-tests-xxl-1607937314856367188] [packetbeat-8.0.0-2019.02.19-000001] creating index, cause [api], templates [], shards [1]/[1]
[00:05:24]                   │ info [packetbeat/default] Created index "packetbeat-8.0.0-2019.02.19-000001"
[00:05:24]                   │ debg [packetbeat/default] "packetbeat-8.0.0-2019.02.19-000001" settings {"index":{"lifecycle":{"name":"packetbeat-8.0.0","rollover_alias":"packetbeat-8.0.0"},"mapping":{"total_fields":{"limit":"10000"}},"number_of_replicas":"1","number_of_shards":"1","query":{"default_field":["tags","message","agent.version","agent.name","agent.type","agent.id","agent.ephemeral_id","client.address","client.mac","client.domain","client.geo.continent_name","client.geo.country_name","client.geo.region_name","client.geo.city_name","client.geo.country_iso_code","client.geo.region_iso_code","client.geo.name","cloud.provider","cloud.availability_zone","cloud.region","cloud.instance.id","cloud.instance.name","cloud.machine.type","cloud.account.id","container.runtime","container.id","container.image.name","container.image.tag","container.name","destination.address","destination.mac","destination.domain","destination.geo.continent_name","destination.geo.country_name","destination.geo.region_name","destination.geo.city_name","destination.geo.country_iso_code","destination.geo.region_iso_code","destination.geo.name","ecs.version","error.id","error.message","error.code","event.id","event.kind","event.category","event.action","event.outcome","event.type","event.module","event.dataset","event.hash","event.timezone","file.path","file.target_path","file.extension","file.type","file.device","file.inode","file.uid","file.owner","file.gid","file.group","file.mode","group.id","group.name","host.hostname","host.name","host.id","host.mac","host.type","host.architecture","host.os.platform","host.os.name","host.os.full","host.os.family","host.os.version","host.os.kernel","host.geo.continent_name","host.geo.country_name","host.geo.region_name","host.geo.city_name","host.geo.country_iso_code","host.geo.region_iso_code","host.geo.name","http.request.method","http.request.body.content","http.request.referrer","http.response.body.content","http.version","log.level","network.name","network.type","network.iana_number","network.transport","network.application","network.protocol","network.direction","network.community_id","observer.mac","observer.hostname","observer.vendor","observer.version","observer.serial_number","observer.type","observer.os.platform","observer.os.name","observer.os.full","observer.os.family","observer.os.version","observer.os.kernel","observer.geo.continent_name","observer.geo.country_name","observer.geo.region_name","observer.geo.city_name","observer.geo.country_iso_code","observer.geo.region_iso_code","observer.geo.name","organization.name","organization.id","os.platform","os.name","os.full","os.family","os.version","os.kernel","process.name","process.args","process.executable","process.title","process.working_directory","server.address","server.mac","server.domain","server.geo.continent_name","server.geo.country_name","server.geo.region_name","server.geo.city_name","server.geo.country_iso_code","server.geo.region_iso_code","server.geo.name","service.id","service.name","service.type","service.state","service.version","service.ephemeral_id","source.address","source.mac","source.domain","source.geo.continent_name","source.geo.country_name","source.geo.region_name","source.geo.city_name","source.geo.country_iso_code","source.geo.region_iso_code","source.geo.name","url.original","url.full","url.scheme","url.domain","url.path","url.query","url.fragment","url.username","url.password","user.id","user.name","user.full_name","user.email","user.hash","user.group.id","user.group.name","user_agent.original","user_agent.name","user_agent.version","user_agent.device.name","user_agent.os.platform","user_agent.os.name","user_agent.os.full","user_agent.os.family","user_agent.os.version","user_agent.os.kernel","agent.hostname","error.type","cloud.project.id","kubernetes.pod.name","kubernetes.pod.uid","kubernetes.namespace","kubernetes.node.name","kubernetes.container.name","kubernetes.container.image","type","server.process.name","server.process.args","server.process.executable","server.process.working_directory","server.process.start","client.process.name","client.process.args","client.process.executable","client.process.working_directory","client.process.start","flow.id","status","method","resource","path","query","params","request","response","amqp.reply-text","amqp.exchange","amqp.exchange-type","amqp.consumer-tag","amqp.routing-key","amqp.queue","amqp.content-type","amqp.content-encoding","amqp.delivery-mode","amqp.correlation-id","amqp.reply-to","amqp.expiration","amqp.message-id","amqp.timestamp","amqp.type","amqp.user-id","amqp.app-id","cassandra.request.headers.flags","cassandra.request.headers.stream","cassandra.request.headers.op","cassandra.request.query","cassandra.response.headers.flags","cassandra.response.headers.stream","cassandra.response.headers.op","cassandra.response.result.type","cassandra.response.result.rows.meta.keyspace","cassandra.response.result.rows.meta.table","cassandra.response.result.rows.meta.flags","cassandra.response.result.rows.meta.paging_state","cassandra.response.result.keyspace","cassandra.response.result.schema_change.change","cassandra.response.result.schema_change.keyspace","cassandra.response.result.schema_change.table","cassandra.response.result.schema_change.object","cassandra.response.result.schema_change.target","cassandra.response.result.schema_change.name","cassandra.response.result.schema_change.args","cassandra.response.result.prepared.prepared_id","cassandra.response.result.prepared.req_meta.keyspace","cassandra.response.result.prepared.req_meta.table","cassandra.response.result.prepared.req_meta.flags","cassandra.response.result.prepared.req_meta.paging_state","cassandra.response.result.prepared.resp_meta.keyspace","cassandra.response.result.prepared.resp_meta.table","cassandra.response.result.prepared.resp_meta.flags","cassandra.response.result.prepared.resp_meta.paging_state","cassandra.response.authentication.class","cassandra.response.warnings","cassandra.response.event.type","cassandra.response.event.change","cassandra.response.event.host","cassandra.response.event.schema_change.change","cassandra.response.event.schema_change.keyspace","cassandra.response.event.schema_change.table","cassandra.response.event.schema_change.object","cassandra.response.event.schema_change.target","cassandra.response.event.schema_change.name","cassandra.response.event.schema_change.args","cassandra.response.error.msg","cassandra.response.error.type","cassandra.response.error.details.read_consistency","cassandra.response.error.details.write_type","cassandra.response.error.details.keyspace","cassandra.response.error.details.table","cassandra.response.error.details.stmt_id","cassandra.response.error.details.num_failures","cassandra.response.error.details.function","cassandra.response.error.details.arg_types","dhcpv4.transaction_id","dhcpv4.flags","dhcpv4.client_mac","dhcpv4.server_name","dhcpv4.op_code","dhcpv4.hardware_type","dhcpv4.option.message_type","dhcpv4.option.parameter_request_list","dhcpv4.option.class_identifier","dhcpv4.option.domain_name","dhcpv4.option.hostname","dhcpv4.option.message","dhcpv4.option.boot_file_name","dns.op_code","dns.response_code","dns.question.name","dns.question.type","dns.question.class","dns.question.registered_domain","dns.answers.name","dns.answers.type","dns.answers.class","dns.answers.data","dns.authorities.name","dns.authorities.type","dns.authorities.class","dns.additionals.name","dns.additionals.type","dns.additionals.class","dns.additionals.data","dns.opt.version","dns.opt.ext_rcode","http.response.status_phrase","icmp.version","icmp.request.message","icmp.response.message","memcache.protocol_type","memcache.request.line","memcache.request.command","memcache.response.command","memcache.request.type","memcache.response.type","memcache.response.error_msg","memcache.request.opcode","memcache.response.opcode","memcache.response.status","memcache.request.raw_args","memcache.request.automove","memcache.response.version","mongodb.error","mongodb.fullCollectionName","mongodb.startingFrom","mongodb.query","mongodb.returnFieldsSelector","mongodb.selector","mongodb.update","mongodb.cursorId","mysql.insert_id","mysql.num_fields","mysql.num_rows","mysql.query","mysql.error_message","nfs.tag","nfs.opcode","nfs.status","rpc.xid","rpc.status","rpc.auth_flavor","rpc.cred.gids","rpc.cred.machinename","pgsql.error_message","pgsql.error_severity","pgsql.num_fields","pgsql.num_rows","redis.return_value","redis.error","thrift.params","thrift.service","thrift.return_value","thrift.exceptions","tls.version","tls.resumption_method","tls.client_hello.version","tls.client_hello.extensions.server_name_indication","tls.client_hello.extensions.application_layer_protocol_negotiation","tls.client_hello.extensions.session_ticket","tls.client_hello.extensions.supported_versions","tls.client_hello.extensions.supported_groups","tls.client_hello.extensions.signature_algorithms","tls.client_hello.extensions.ec_points_formats","tls.client_hello.extensions._unparsed_","tls.server_hello.version","tls.server_hello.selected_cipher","tls.server_hello.selected_compression_method","tls.server_hello.session_id","tls.server_hello.extensions.session_ticket","tls.server_hello.extensions.supported_versions","tls.server_hello.extensions.ec_points_formats","tls.server_hello.extensions._unparsed_","tls.client_certificate.serial_number","tls.client_certificate.public_key_algorithm","tls.client_certificate.signature_algorithm","tls.client_certificate.raw","tls.client_certificate.subject.country","tls.client_certificate.subject.organization","tls.client_certificate.subject.organizational_unit","tls.client_certificate.subject.province","tls.client_certificate.subject.common_name","tls.client_certificate.issuer.country","tls.client_certificate.issuer.organization","tls.client_certificate.issuer.organizational_unit","tls.client_certificate.issuer.province","tls.client_certificate.issuer.common_name","tls.client_certificate.fingerprint.md5","tls.client_certificate.fingerprint.sha1","tls.client_certificate.fingerprint.sha256","tls.server_certificate.serial_number","tls.server_certificate.public_key_algorithm","tls.server_certificate.signature_algorithm","tls.server_certificate.raw","tls.server_certificate.subject.country","tls.server_certificate.subject.organization","tls.server_certificate.subject.organizational_unit","tls.server_certificate.subject.province","tls.server_certificate.subject.common_name","tls.server_certificate.issuer.country","tls.server_certificate.issuer.organization","tls.server_certificate.issuer.organizational_unit","tls.server_certificate.issuer.province","tls.server_certificate.issuer.common_name","tls.server_certificate.fingerprint.md5","tls.server_certificate.fingerprint.sha1","tls.server_certificate.fingerprint.sha256","tls.alert_types","tls.fingerprints.ja3.hash","tls.fingerprints.ja3.str","fields.*"]},"refresh_interval":"5s"}}
[00:05:24]                   │ info [o.e.c.m.MetadataMappingService] [kibana-ci-immutable-ubuntu-16-tests-xxl-1607937314856367188] [packetbeat-8.0.0-2019.02.19-000001/4UcLWRURR_OCs5zR9VMxAQ] update_mapping [_doc]
[00:05:24]                   │ info [packetbeat/default] Indexed 665 docs into "packetbeat-8.0.0-2019.02.19-000001"
[00:05:24]                 └-> Make sure that we get KpiNetwork uniqueFlows data
[00:05:24]                   └-> "before each" hook: global before each
[00:05:24]                   └- ✓ pass  (23ms) "apis SecuritySolution Endpoints Kpi Network With packetbeat Make sure that we get KpiNetwork uniqueFlows data"
[00:05:24]                 └-> Make sure that we get KpiNetwork DNS data
[00:05:24]                   └-> "before each" hook: global before each
[00:05:24]                   └- ✓ pass  (18ms) "apis SecuritySolution Endpoints Kpi Network With packetbeat Make sure that we get KpiNetwork DNS data"
[00:05:24]                 └-> Make sure that we get KpiNetwork networkEvents data
[00:05:24]                   └-> "before each" hook: global before each
[00:05:24]                   │ info [o.e.c.m.MetadataCreateIndexService] [kibana-ci-immutable-ubuntu-16-tests-xxl-1607937314856367188] [.async-search] creating index, cause [api], templates [], shards [1]/[0]
[00:05:24]                   └- ✖ fail: apis SecuritySolution Endpoints Kpi Network With packetbeat Make sure that we get KpiNetwork networkEvents data
[00:05:24]                   │       Error: expected 0 to sort of equal 665
[00:05:24]                   │       + expected - actual
[00:05:24]                   │ 
[00:05:24]                   │       -0
[00:05:24]                   │       +665
[00:05:24]                   │       
[00:05:24]                   │       at Assertion.assert (/dev/shm/workspace/parallel/7/kibana/packages/kbn-expect/expect.js:100:11)
[00:05:24]                   │       at Assertion.eql (/dev/shm/workspace/parallel/7/kibana/packages/kbn-expect/expect.js:244:8)
[00:05:24]                   │       at Context.<anonymous> (test/api_integration/apis/security_solution/kpi_network.ts:271:45)
[00:05:24]                   │       at Object.apply (/dev/shm/workspace/parallel/7/kibana/packages/kbn-test/src/functional_test_runner/lib/mocha/wrap_function.js:84:16)
[00:05:24]                   │ 
[00:05:24]                   │ 

Stack Trace

Error: expected 0 to sort of equal 665
    at Assertion.assert (/dev/shm/workspace/parallel/7/kibana/packages/kbn-expect/expect.js:100:11)
    at Assertion.eql (/dev/shm/workspace/parallel/7/kibana/packages/kbn-expect/expect.js:244:8)
    at Context.<anonymous> (test/api_integration/apis/security_solution/kpi_network.ts:271:45)
    at Object.apply (/dev/shm/workspace/parallel/7/kibana/packages/kbn-test/src/functional_test_runner/lib/mocha/wrap_function.js:84:16) {
  actual: '0',
  expected: '665',
  showDiff: true
}

Metrics [docs]

Distributable file count

id before after diff
default 47129 47889 +760

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@thomheymann thomheymann merged commit 5a8a5bf into master Dec 14, 2020
@thomheymann thomheymann deleted the audit/sessionid branch December 14, 2020 11:35
gmmorris added a commit to gmmorris/kibana that referenced this pull request Dec 14, 2020
* master: (116 commits)
  Fix UX E2E tests (elastic#85722)
  Increasing default api key removalDelay to 1h (elastic#85576)
  align cors settings names with elasticsearch (elastic#85738)
  unskip tests and make sure submit is not triggered too quickly (elastic#85567)
  Row trigger 2 (elastic#83167)
  Add session id to audit log (elastic#85451)
  [TSVB] Fields lists do not populate all the times (elastic#85530)
  [Visualize] Removes the external link icon from OSS badges (elastic#85580)
  fixes EQL tests (elastic#85712)
  [APM] enable 'log_level' for Go (elastic#85511)
  ini `1.3.5` -> `1.3.7` (elastic#85707)
  Fix fleet route protections (elastic#85626)
  [Monitoring] Some progress on making alerts better in the UI (elastic#81569)
  [Security Solution] Refactor Timeline Notes to use EuiCommentList (elastic#85256)
  [Security Solution][Detections][Threshold Rules] Threshold rule exceptions (elastic#85103)
  [Security Solution] Alerts details (elastic#83963)
  skip flaky suite (elastic#62060)
  skip flaky suite (elastic#85098)
  skip flaky suite (elastic#84020)
  skip flaky suite (elastic#85671)
  ...
thomheymann added a commit that referenced this pull request Dec 14, 2020
* Add session id to audit log

* fix naming

Co-authored-by: Kibana Machine <[email protected]>

Co-authored-by: Kibana Machine <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release_note:skip Skip the PR/issue when compiling release notes v7.11.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants