Added suggestions from code review

docs/settings/security-settings.asciidoc

@@ -269,7 +269,7 @@ To enable the <<xpack-security-ecs-audit-logging, ECS audit logger>>, specify wh
 [cols="2*<"]
 |===
 | `xpack.security.audit.appender`
-| experimental[] Optional. Specifies where audit logs should be written to and how they should be formatted.
+| Optional. Specifies where audit logs should be written to and how they should be formatted.
 
 2+a| For example:
 
@@ -283,7 +283,7 @@ xpack.security.audit.appender:
 ----------------------------------------
 
 | `xpack.security.audit.appender.kind`
-| experimental[] Required. Specifies where audit logs should be written to. Allowed values are `console` or `file`.
+| Required. Specifies where audit logs should be written to. Allowed values are `console` or `file`.
 |===
 
 [float]
@@ -295,10 +295,10 @@ The file appender can be configured using the following settings:
 [cols="2*<"]
 |===
 | `xpack.security.audit.appender.path`
-| experimental[] Required. Full file path the log file should be written to.
+| Required. Full file path the log file should be written to.
 
 | `xpack.security.audit.appender.layout.kind`
-| experimental[] Required. Specifies how audit logs should be formatted. Allowed values are `json` or `pattern`.
+| Required. Specifies how audit logs should be formatted. Allowed values are `json` or `pattern`.
 |===
 
 [float]
@@ -310,10 +310,10 @@ The pattern layout can be configured using the following settings:
 [cols="2*<"]
 |===
 | `xpack.security.audit.appender.layout.highlight`
-| experimental[] Optional. Set to `true` to enable highlighting log messages with colors.
+| Optional. Set to `true` to enable highlighting log messages with colors.
 
 | `xpack.security.audit.appender.layout.pattern`
-| experimental[] Optional. Specifies how the log line should be formatted. *Default:* `[%date][%level][%logger]%meta %message`
+| Optional. Specifies how the log line should be formatted. *Default:* `[%date][%level][%logger]%meta %message`
 |===
 
 [float]
@@ -323,7 +323,7 @@ The pattern layout can be configured using the following settings:
 [cols="2*<"]
 |===
 | `xpack.security.audit.ignore_filters[]`
-| experimental[] List of filters that determine which events should be excluded from the audit log. An event will get filtered out if at least one of the provided filters matches.
+| List of filters that determine which events should be excluded from the audit log. An event will get filtered out if at least one of the provided filters matches.
 
 2+a| For example:
 
@@ -338,14 +338,14 @@ xpack.security.audit.ignore_filters:
 <2> Filters out any data write events
 
 | `xpack.security.audit.ignore_filters[].actions[]`
-| experimental[] List of values matched against the `event.action` field of an audit event. Refer to <<xpack-security-audit-logging>> for a list of available events.
+| List of values matched against the `event.action` field of an audit event. Refer to <<xpack-security-audit-logging>> for a list of available events.
 
 | `xpack.security.audit.ignore_filters[].categories[]`
-| experimental[] List of values matched against the `event.category` field of an audit event. Refer to https://www.elastic.co/guide/en/ecs/1.5/ecs-allowed-values-event-category.html[ECS categorization field] for allowed values.
+| List of values matched against the `event.category` field of an audit event. Refer to https://www.elastic.co/guide/en/ecs/1.5/ecs-allowed-values-event-category.html[ECS categorization field] for allowed values.
 
 | `xpack.security.audit.ignore_filters[].types[]`
-| experimental[] List of values matched against the `event.type` field of an audit event. Refer to https://www.elastic.co/guide/en/ecs/1.5/ecs-allowed-values-event-type.html[ECS type field] for allowed values.
+| List of values matched against the `event.type` field of an audit event. Refer to https://www.elastic.co/guide/en/ecs/1.5/ecs-allowed-values-event-type.html[ECS type field] for allowed values.
 
 | `xpack.security.audit.ignore_filters[].outcomes[]`
-| experimental[] List of values matched against the `event.outcome` field of an audit event. Refer to https://www.elastic.co/guide/en/ecs/1.5/ecs-allowed-values-event-outcome.html[ECS outcome field] for allowed values.
+| List of values matched against the `event.outcome` field of an audit event. Refer to https://www.elastic.co/guide/en/ecs/1.5/ecs-allowed-values-event-outcome.html[ECS outcome field] for allowed values.
 |===

x-pack/plugins/security/server/audit/audit_events.test.ts

@@ -127,6 +127,7 @@ describe('#userLoginEvent', () => {
           "authentication_realm": "native1",
           "authentication_type": "basic",
           "lookup_realm": "native1",
+          "space_id": undefined,
         },
         "message": "User [user] has logged in using basic provider [name=basic1]",
         "user": Object {
@@ -162,6 +163,7 @@ describe('#userLoginEvent', () => {
           "authentication_realm": undefined,
           "authentication_type": "basic",
           "lookup_realm": undefined,
+          "space_id": undefined,
         },
         "message": "Failed attempt to login using basic provider [name=basic1]",
         "user": undefined,

x-pack/plugins/security/server/audit/audit_events.ts

@@ -123,7 +123,7 @@ export function httpRequestEvent({ request }: HttpRequestParams): AuditEvent {
       domain: request.url.hostname,
       path: pathname,
       port: request.url.port ? parseInt(request.url.port, 10) : undefined,
-      query: search?.slice(1) ?? undefined,
+      query: search?.slice(1) || undefined,
       scheme: request.url.protocol,
     },
   };
@@ -154,6 +154,7 @@ export function userLoginEvent({
       roles: authenticationResult.user.roles,
     },
     kibana: {
+      space_id: undefined, // Ensure this does not get populated by audit service
       authentication_provider: authenticationProvider,
       authentication_type: authenticationType,
       authentication_realm: authenticationResult.user?.authentication_realm.name,

x-pack/plugins/security/server/audit/audit_service.ts

@@ -5,7 +5,7 @@
  */
 
 import { Subscription } from 'rxjs';
-import { map } from 'rxjs/operators';
+import { map, distinctUntilKeyChanged } from 'rxjs/operators';
 import {
   Logger,
   LoggingServiceSetup,
@@ -86,8 +86,13 @@ export class AuditService {
       });
     }
 
-    // Configure logging
-    logging.configure(license.features$.pipe(createLoggingConfig(config)));
+    // Configure logging during setup and when license changes
+    logging.configure(
+      license.features$.pipe(
+        distinctUntilKeyChanged('allowAuditLogging'),
+        createLoggingConfig(config)
+      )
+    );
 
     /**
      * Creates an {@link AuditLogger} scoped to the current request.

x-pack/test/security_api_integration/tests/audit/audit_log.ts

@@ -30,9 +30,10 @@ class FileWrapper {
   }
 }
 
-export default function ({ getPageObjects, getService }: FtrProviderContext) {
+export default function ({ getService }: FtrProviderContext) {
   const supertest = getService('supertest');
   const retry = getService('retry');
+  const { username, password } = getService('config').get('servers.kibana');
 
   describe('Audit Log', function () {
     const logFilePath = Path.resolve(__dirname, '../../fixtures/audit/audit.log');
@@ -42,32 +43,76 @@ export default function ({ getPageObjects, getService }: FtrProviderContext) {
       await logFile.reset();
     });
 
-    it('logs audit events from saved objects client', async () => {
-      await supertest.get('/audit_log').set('kbn-xsrf', 'foo').expect(204);
-
-      await retry.waitFor('logs event in the dest file', async () => {
-        return await logFile.isNotEmpty();
-      });
+    it('logs audit events when reading and writing saved objects', async () => {
+      await supertest.get('/audit_log?query=param').set('kbn-xsrf', 'foo').expect(204);
+      await retry.waitFor('logs event in the dest file', async () => await logFile.isNotEmpty());
 
       const content = await logFile.readJSON();
 
       const httpEvent = content.find((c) => c.event.action === 'http_request');
       expect(httpEvent).to.be.ok();
       expect(httpEvent.trace.id).to.be.ok();
-      expect(httpEvent.user.name).to.be('elastic');
+      expect(httpEvent.user.name).to.be(username);
       expect(httpEvent.kibana.space_id).to.be('default');
+      expect(httpEvent.http.request.method).to.be('get');
+      expect(httpEvent.url.path).to.be('/audit_log');
+      expect(httpEvent.url.query).to.be('query=param');
 
       const createEvent = content.find((c) => c.event.action === 'saved_object_create');
       expect(createEvent).to.be.ok();
       expect(createEvent.trace.id).to.be.ok();
-      expect(createEvent.user.name).to.be('elastic');
+      expect(createEvent.user.name).to.be(username);
       expect(createEvent.kibana.space_id).to.be('default');
 
       const findEvent = content.find((c) => c.event.action === 'saved_object_find');
       expect(findEvent).to.be.ok();
       expect(findEvent.trace.id).to.be.ok();
-      expect(findEvent.user.name).to.be('elastic');
+      expect(findEvent.user.name).to.be(username);
       expect(findEvent.kibana.space_id).to.be('default');
     });
+
+    it('logs audit events when logging in successfully', async () => {
+      await supertest
+        .post('/internal/security/login')
+        .set('kbn-xsrf', 'xxx')
+        .send({
+          providerType: 'basic',
+          providerName: 'basic',
+          currentURL: '/',
+          params: { username, password },
+        })
+        .expect(200);
+      await retry.waitFor('logs event in the dest file', async () => await logFile.isNotEmpty());
+
+      const content = await logFile.readJSON();
+
+      const loginEvent = content.find((c) => c.event.action === 'user_login');
+      expect(loginEvent).to.be.ok();
+      expect(loginEvent.event.outcome).to.be('success');
+      expect(loginEvent.trace.id).to.be.ok();
+      expect(loginEvent.user.name).to.be(username);
+    });
+
+    it('logs audit events when failing to log in', async () => {
+      await supertest
+        .post('/internal/security/login')
+        .set('kbn-xsrf', 'xxx')
+        .send({
+          providerType: 'basic',
+          providerName: 'basic',
+          currentURL: '/',
+          params: { username, password: 'invalid_password' },
+        })
+        .expect(401);
+      await retry.waitFor('logs event in the dest file', async () => await logFile.isNotEmpty());
+
+      const content = await logFile.readJSON();
+
+      const loginEvent = content.find((c) => c.event.action === 'user_login');
+      expect(loginEvent).to.be.ok();
+      expect(loginEvent.event.outcome).to.be('failure');
+      expect(loginEvent.trace.id).to.be.ok();
+      expect(loginEvent.user).not.to.be.ok();
+    });
   });
 }