Capture user logout events in audit log (#121455) (#121828)

* Audit user logout events

* Capture all user initiated logout events

* Added suggestions from code review

Co-authored-by: Thom Heymann <190132+thomheymann@users.noreply.github.com>

docs/user/security/audit-logging.asciidoc

@@ -50,6 +50,9 @@ Refer to the corresponding {es} logs for potential write errors.
 | `success` | User has logged in successfully.
 | `failure` | Failed login attempt (e.g. due to invalid credentials).
 
+| `user_logout`
+| `unknown` | User is logging out.
+
 | `access_agreement_acknowledged`
 | N/A | User has acknowledged the access agreement.
 

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

@@ -18,6 +18,7 @@ import {
   SpaceAuditAction,
   spaceAuditEvent,
   userLoginEvent,
+  userLogoutEvent,
 } from './audit_events';
 
 describe('#savedObjectEvent', () => {
@@ -300,6 +301,57 @@ describe('#userLoginEvent', () => {
   });
 });
 
+describe('#userLogoutEvent', () => {
+  test('creates event with `unknown` outcome', () => {
+    expect(
+      userLogoutEvent({
+        username: 'elastic',
+        provider: { name: 'basic1', type: 'basic' },
+      })
+    ).toMatchInlineSnapshot(`
+      Object {
+        "event": Object {
+          "action": "user_logout",
+          "category": Array [
+            "authentication",
+          ],
+          "outcome": "unknown",
+        },
+        "kibana": Object {
+          "authentication_provider": "basic1",
+          "authentication_type": "basic",
+        },
+        "message": "User [elastic] is logging out using basic provider [name=basic1]",
+        "user": Object {
+          "name": "elastic",
+        },
+      }
+    `);
+
+    expect(
+      userLogoutEvent({
+        provider: { name: 'basic1', type: 'basic' },
+      })
+    ).toMatchInlineSnapshot(`
+      Object {
+        "event": Object {
+          "action": "user_logout",
+          "category": Array [
+            "authentication",
+          ],
+          "outcome": "unknown",
+        },
+        "kibana": Object {
+          "authentication_provider": "basic1",
+          "authentication_type": "basic",
+        },
+        "message": "User [undefined] is logging out using basic provider [name=basic1]",
+        "user": undefined,
+      }
+    `);
+  });
+});
+
 describe('#httpRequestEvent', () => {
   test('creates event with `unknown` outcome', () => {
     expect(

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

@@ -131,6 +131,31 @@ export function userLoginEvent({
   };
 }
 
+export interface UserLogoutParams {
+  username?: string;
+  provider: AuthenticationProvider;
+}
+
+export function userLogoutEvent({ username, provider }: UserLogoutParams): AuditEvent {
+  return {
+    message: `User [${username}] is logging out using ${provider.type} provider [name=${provider.name}]`,
+    event: {
+      action: 'user_logout',
+      category: ['authentication'],
+      outcome: 'unknown',
+    },
+    user: username
+      ? {
+          name: username,
+        }
+      : undefined,
+    kibana: {
+      authentication_provider: provider.name,
+      authentication_type: provider.type,
+    },
+  };
+}
+
 export interface AccessAgreementAcknowledgedParams {
   username: string;
   provider: AuthenticationProvider;

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

@@ -10,6 +10,7 @@ export { AuditService } from './audit_service';
 export type { AuditEvent } from './audit_events';
 export {
   userLoginEvent,
+  userLogoutEvent,
   accessAgreementAcknowledgedEvent,
   httpRequestEvent,
   savedObjectEvent,

x-pack/plugins/security/server/authentication/authenticator.test.ts

@@ -1091,9 +1091,15 @@ describe('Authenticator', () => {
     let authenticator: Authenticator;
     let mockOptions: ReturnType<typeof getMockOptions>;
     let mockSessVal: SessionValue;
+    const auditLogger = {
+      log: jest.fn(),
+    };
+
     beforeEach(() => {
+      auditLogger.log.mockClear();
       mockOptions = getMockOptions({ providers: { basic: { basic1: { order: 0 } } } });
       mockOptions.session.get.mockResolvedValue(null);
+      mockOptions.audit.asScoped.mockReturnValue(auditLogger);
       mockSessVal = sessionMock.createValue({ state: { authorization: 'Basic xxx' } });
 
       authenticator = new Authenticator(mockOptions);
@@ -1377,6 +1383,26 @@ describe('Authenticator', () => {
       expect(mockOptions.session.extend).not.toHaveBeenCalled();
     });
 
+    it('adds audit event when invalidating session.', async () => {
+      const request = httpServerMock.createKibanaRequest();
+
+      mockBasicAuthenticationProvider.authenticate.mockResolvedValue(
+        AuthenticationResult.redirectTo('some-url', { state: null })
+      );
+      mockOptions.session.get.mockResolvedValue(mockSessVal);
+
+      await expect(authenticator.authenticate(request)).resolves.toEqual(
+        AuthenticationResult.redirectTo('some-url', { state: null })
+      );
+
+      expect(auditLogger.log).toHaveBeenCalledTimes(1);
+      expect(auditLogger.log).toHaveBeenCalledWith(
+        expect.objectContaining({
+          event: { action: 'user_logout', category: ['authentication'], outcome: 'unknown' },
+        })
+      );
+    });
+
     it('does not clear session if provider can not handle system API request authentication with active session.', async () => {
       const request = httpServerMock.createKibanaRequest({
         headers: { 'kbn-system-request': 'true' },
@@ -1796,8 +1822,14 @@ describe('Authenticator', () => {
     let authenticator: Authenticator;
     let mockOptions: ReturnType<typeof getMockOptions>;
     let mockSessVal: SessionValue;
+    const auditLogger = {
+      log: jest.fn(),
+    };
+
     beforeEach(() => {
+      auditLogger.log.mockClear();
       mockOptions = getMockOptions({ providers: { basic: { basic1: { order: 0 } } } });
+      mockOptions.audit.asScoped.mockReturnValue(auditLogger);
       mockSessVal = sessionMock.createValue({ state: { authorization: 'Basic xxx' } });
 
       authenticator = new Authenticator(mockOptions);
@@ -1836,6 +1868,25 @@ describe('Authenticator', () => {
       expect(mockOptions.session.invalidate).toHaveBeenCalled();
     });
 
+    it('adds audit event.', async () => {
+      const request = httpServerMock.createKibanaRequest();
+      mockBasicAuthenticationProvider.logout.mockResolvedValue(
+        DeauthenticationResult.redirectTo('some-url')
+      );
+      mockOptions.session.get.mockResolvedValue(mockSessVal);
+
+      await expect(authenticator.logout(request)).resolves.toEqual(
+        DeauthenticationResult.redirectTo('some-url')
+      );
+
+      expect(auditLogger.log).toHaveBeenCalledTimes(1);
+      expect(auditLogger.log).toHaveBeenCalledWith(
+        expect.objectContaining({
+          event: { action: 'user_logout', category: ['authentication'], outcome: 'unknown' },
+        })
+      );
+    });
+
     it('if session does not exist but provider name is valid, returns whatever authentication provider returns.', async () => {
       const request = httpServerMock.createKibanaRequest({
         query: { provider: 'basic1' },

x-pack/plugins/security/server/authentication/authenticator.ts

@@ -21,7 +21,7 @@ import type { SecurityLicense } from '../../common/licensing';
 import type { AuthenticatedUser, AuthenticationProvider } from '../../common/model';
 import { shouldProviderUseLoginForm } from '../../common/model';
 import type { AuditServiceSetup } from '../audit';
-import { accessAgreementAcknowledgedEvent, userLoginEvent } from '../audit';
+import { accessAgreementAcknowledgedEvent, userLoginEvent, userLogoutEvent } from '../audit';
 import type { ConfigType } from '../config';
 import { getErrorStatusCode } from '../errors';
 import type { SecurityFeatureUsageServiceStart } from '../feature_usage';
@@ -418,7 +418,7 @@ export class Authenticator {
       sessionValue?.provider.name ??
       request.url.searchParams.get(LOGOUT_PROVIDER_QUERY_STRING_PARAMETER);
     if (suggestedProviderName) {
-      await this.invalidateSessionValue(request);
+      await this.invalidateSessionValue(request, sessionValue);
 
       // Provider name may be passed in a query param and sourced from the browser's local storage;
       // hence, we can't assume that this provider exists, so we have to check it.
@@ -567,7 +567,7 @@ export class Authenticator {
       this.logger.warn(
         `Attempted to retrieve session for the "${existingSessionValue.provider.type}/${existingSessionValue.provider.name}" provider, but it is not configured.`
       );
-      await this.invalidateSessionValue(request);
+      await this.invalidateSessionValue(request, existingSessionValue);
       return null;
     }
 
@@ -601,7 +601,7 @@ export class Authenticator {
     // attempt didn't fail.
     if (authenticationResult.shouldClearState()) {
       this.logger.debug('Authentication provider requested to invalidate existing session.');
-      await this.invalidateSessionValue(request);
+      await this.invalidateSessionValue(request, existingSessionValue);
       return null;
     }
 
@@ -615,7 +615,7 @@ export class Authenticator {
     if (authenticationResult.failed()) {
       if (ownsSession && getErrorStatusCode(authenticationResult.error) === 401) {
         this.logger.debug('Authentication attempt failed, existing session will be invalidated.');
-        await this.invalidateSessionValue(request);
+        await this.invalidateSessionValue(request, existingSessionValue);
       }
       return null;
     }
@@ -653,17 +653,17 @@ export class Authenticator {
       this.logger.debug(
         'Authentication provider has changed, existing session will be invalidated.'
       );
-      await this.invalidateSessionValue(request);
+      await this.invalidateSessionValue(request, existingSessionValue);
       existingSessionValue = null;
     } else if (sessionHasBeenAuthenticated) {
       this.logger.debug(
         'Session is authenticated, existing unauthenticated session will be invalidated.'
       );
-      await this.invalidateSessionValue(request);
+      await this.invalidateSessionValue(request, existingSessionValue);
       existingSessionValue = null;
     } else if (usernameHasChanged) {
       this.logger.debug('Username has changed, existing session will be invalidated.');
-      await this.invalidateSessionValue(request);
+      await this.invalidateSessionValue(request, existingSessionValue);
       existingSessionValue = null;
     }
 
@@ -699,8 +699,19 @@ export class Authenticator {
   /**
    * Invalidates session value associated with the specified request.
    * @param request Request instance.
+   * @param sessionValue Value of the existing session if any.
    */
-  private async invalidateSessionValue(request: KibanaRequest) {
+  private async invalidateSessionValue(request: KibanaRequest, sessionValue: SessionValue | null) {
+    if (sessionValue) {
+      const auditLogger = this.options.audit.asScoped(request);
+      auditLogger.log(
+        userLogoutEvent({
+          username: sessionValue.username,
+          provider: sessionValue.provider,
+        })
+      );
+    }
+
     await this.session.invalidate(request, { match: 'current' });
   }