Updatable API keys - logging audit trail event#88276
Updatable API keys - logging audit trail event#88276n1v0lg merged 203 commits intoelastic:masterfrom
Conversation
|
Pinging @elastic/es-security (Team:Security) |
|
Hi @n1v0lg, I've created a changelog YAML for you. |
...security/src/main/java/org/elasticsearch/xpack/security/audit/logfile/LoggingAuditTrail.java
Outdated
Show resolved
Hide resolved
| withRoleDescriptor(builder, roleDescriptor); | ||
| } | ||
| builder.endArray(); | ||
| } |
There was a problem hiding this comment.
It's an oversight that metadata is not included in the auditing message because metadata support was added after audit for security-config-change. Our view has contiuned to be that audit message should be as complete as possible. It can get very verbose. But we generally consider the leaving out information without clear context is more risky. Taking the API key metadata as an example, Fleet uses it to identify the host where an agent runs on and this is actually an important piece of information.
That said, I am happy for it to be a separate PR. In fact, I can take this one so that you can focus on the main tasks.
| ["read","maintenance"]},{"names":["in*","alias*"],"privileges":["read"], | ||
| "field_security":{"grant":["field1*","@timestamp"],"except":["field11"]}}], | ||
| "applications":[],"run_as":[]},{"cluster":["all"],"indices":[{"names": | ||
| ["index-b*"],"privileges":["all"]}],"applications":[],"run_as":[]}]}}} |
There was a problem hiding this comment.
Note: no metadata here - will need to add it when we add metadata to the actual event.
| } | ||
|
|
||
| private void withRequestBody(final XContentBuilder builder, final UpdateApiKeyRequest updateApiKeyRequest) throws IOException { | ||
| builder.startObject("apikey").field("id", updateApiKeyRequest.getId()); |
There was a problem hiding this comment.
I am thinking how this should play when we have the bulk update API. My intuition would be having a separate apikeys schema to avoid having both id and ids in the same schema. What do you think?
There was a problem hiding this comment.
Yup separate schema makes sense. The apikeys schema already exists for invalidation so we could re-use that for instance.
* upstream/master: (38 commits) Simplify map copying (elastic#88432) Make DiffableUtils.diff implementation agnostic (elastic#88403) Ingest: Start separating Metadata from IngestSourceAndMetadata (elastic#88401) Move runtime fields base scripts out of scripting fields api package. (elastic#88488) Enable TRACE Logging for test and increase timeout (elastic#88477) Mute ReactiveStorageIT#testScaleDuringSplitOrClone (elastic#88480) Track the count of failed invocations since last successful policy snapshot (elastic#88398) Avoid noisy exceptions on data nodes when aborting snapshots (elastic#88476) Fix ReactiveStorageDeciderServiceTests testNodeSizeForDataBelowLowWatermark (elastic#88452) INFO logging of snapshot restore and completion (elastic#88257) unmute test (elastic#88454) Updatable API keys - noop check (elastic#88346) Corrected an incomplete sentence. (elastic#86542) Use consistent shard map type in IndexService (elastic#88465) Stop registering TestGeoShapeFieldMapperPlugin in ESIntegTestCase (elastic#88460) TSDB: RollupShardIndexer logging improvements (elastic#88416) Audit API key ID when create or grant API keys (elastic#88456) Bound random negative size test in SearchSourceBuilderTests#testNegativeSizeErrors (elastic#88457) Updatable API keys - logging audit trail event (elastic#88276) Polish reworked LoggedExec task (elastic#88424) ... # Conflicts: # x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/v2/RollupShardIndexer.java
This PR adds a new audit trail event for when API keys are updated.