Add user profile ID to audit log events (#141092)

* Add user profile ID to audit log events * Fix merge conflict * Fix integration tests * Fix integration tests * Fix type * Refactor * updated functional tests * updated functional tests * Added tests for enrichment logic Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
elastic · Oct 3, 2022 · c1d0b93 · c1d0b93
1 parent bea4228
commit c1d0b93
Show file tree

Hide file tree

Showing 18 changed files with 282 additions and 87 deletions.
diff --git a/docs/user/security/audit-logging.asciidoc b/docs/user/security/audit-logging.asciidoc
@@ -325,6 +325,9 @@ Possible values:
 | *Field*
 | *Description*
 
+| `user.id`
+| Unique identifier of the user across sessions (See {ref}/user-profile.html[user profiles]).
+
 | `user.name`
 | Login name of the user.
 

diff --git a/x-pack/plugins/security/common/model/authenticated_user.mock.ts b/x-pack/plugins/security/common/model/authenticated_user.mock.ts
@@ -24,6 +24,7 @@ export function mockAuthenticatedUser(user: MockAuthenticatedUserProps = {}) {
     authentication_provider: { type: 'basic', name: 'basic1' },
     authentication_type: 'realm',
     elastic_cloud_user: false,
+    profile_uid: 'uid',
     metadata: { _reserved: false },
     ...user,
   };

diff --git a/x-pack/plugins/security/common/model/authenticated_user.ts b/x-pack/plugins/security/common/model/authenticated_user.ts
@@ -57,6 +57,11 @@ export interface AuthenticatedUser extends User {
    * Indicates whether user is authenticated via Elastic Cloud built-in SAML realm.
    */
   elastic_cloud_user: boolean;
+
+  /**
+   * User profile ID of this user.
+   */
+  profile_uid?: string;
 }
 
 export function isUserAnonymous(user: Pick<AuthenticatedUser, 'authentication_provider'>) {

diff --git a/x-pack/plugins/security/server/audit/audit_events.test.ts b/x-pack/plugins/security/server/audit/audit_events.test.ts
@@ -240,6 +240,7 @@ describe('#userLoginEvent', () => {
         authenticationProvider: 'basic1',
         authenticationType: 'basic',
         sessionId: '123',
+        userProfileId: 'uid',
       })
     ).toMatchInlineSnapshot(`
       Object {
@@ -261,6 +262,7 @@ describe('#userLoginEvent', () => {
         },
         "message": "User [user] has logged in using basic provider [name=basic1]",
         "user": Object {
+          "id": "uid",
           "name": "user",
           "roles": Array [
             "user-role",
@@ -311,6 +313,7 @@ describe('#userLogoutEvent', () => {
       userLogoutEvent({
         username: 'elastic',
         provider: { name: 'basic1', type: 'basic' },
+        userProfileId: 'uid',
       })
     ).toMatchInlineSnapshot(`
       Object {
@@ -327,6 +330,7 @@ describe('#userLogoutEvent', () => {
         },
         "message": "User [elastic] is logging out using basic provider [name=basic1]",
         "user": Object {
+          "id": "uid",
           "name": "elastic",
         },
       }

diff --git a/x-pack/plugins/security/server/audit/audit_events.ts b/x-pack/plugins/security/server/audit/audit_events.ts
@@ -98,13 +98,15 @@ export interface UserLoginParams {
   authenticationProvider?: string;
   authenticationType?: string;
   sessionId?: string;
+  userProfileId?: string;
 }
 
 export function userLoginEvent({
   authenticationResult,
   authenticationProvider,
   authenticationType,
   sessionId,
+  userProfileId,
 }: UserLoginParams): AuditEvent {
   return {
     message: authenticationResult.user
@@ -116,6 +118,7 @@ export function userLoginEvent({
       outcome: authenticationResult.user ? 'success' : 'failure',
     },
     user: authenticationResult.user && {
+      id: userProfileId,
       name: authenticationResult.user.username,
       roles: authenticationResult.user.roles as string[],
     },
@@ -137,21 +140,28 @@ export function userLoginEvent({
 export interface UserLogoutParams {
   username?: string;
   provider: AuthenticationProvider;
+  userProfileId?: string;
 }
 
-export function userLogoutEvent({ username, provider }: UserLogoutParams): AuditEvent {
+export function userLogoutEvent({
+  username,
+  provider,
+  userProfileId,
+}: 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,
+    user:
+      userProfileId || username
+        ? {
+            id: userProfileId,
+            name: username,
+          }
+        : undefined,
     kibana: {
       authentication_provider: provider.name,
       authentication_type: provider.type,

diff --git a/x-pack/plugins/security/server/audit/audit_service.test.ts b/x-pack/plugins/security/server/audit/audit_service.test.ts
@@ -38,7 +38,9 @@ const createAuditConfig = (settings: Partial<ConfigType['audit']>) => {
 const config = createAuditConfig({ enabled: true });
 const { logging } = coreMock.createSetup();
 const http = httpServiceMock.createSetupContract();
-const getCurrentUser = jest.fn().mockReturnValue({ username: 'jdoe', roles: ['admin'] });
+const getCurrentUser = jest
+  .fn()
+  .mockReturnValue({ username: 'jdoe', roles: ['admin'], profile_uid: 'uid' });
 const getSpaceId = jest.fn().mockReturnValue('default');
 const getSID = jest.fn().mockResolvedValue('SESSION_ID');
 const recordAuditLoggingUsage = jest.fn();
@@ -192,7 +194,7 @@ describe('#asScoped', () => {
       event: { action: 'ACTION' },
       kibana: { space_id: 'default', session_id: 'SESSION_ID' },
       trace: { id: 'REQUEST_ID' },
-      user: { name: 'jdoe', roles: ['admin'] },
+      user: { id: 'uid', name: 'jdoe', roles: ['admin'] },
     });
     audit.stop();
   });

diff --git a/x-pack/plugins/security/server/audit/audit_service.ts b/x-pack/plugins/security/server/audit/audit_service.ts
@@ -166,6 +166,7 @@ export class AuditService {
           ...event,
           user:
             (user && {
+              id: user.profile_uid,
               name: user.username,
               roles: user.roles as string[],
             }) ||

diff --git a/x-pack/plugins/security/server/authentication/authentication_service.test.ts b/x-pack/plugins/security/server/authentication/authentication_service.test.ts
@@ -273,6 +273,33 @@ describe('AuthenticationService', () => {
         expect(authenticate).toHaveBeenCalledWith(mockRequest);
       });
 
+      it('sets authenticated state correctly with user profile id', async () => {
+        const mockRequest = httpServerMock.createKibanaRequest();
+        const mockResponse = httpServerMock.createLifecycleResponseFactory();
+        const mockUser = mockAuthenticatedUser();
+        const mockAuthHeaders = { authorization: 'Basic xxx' };
+        const mockAuthResponseHeaders = { 'WWW-Authenticate': 'Negotiate' };
+
+        authenticate.mockResolvedValue(
+          AuthenticationResult.succeeded(
+            { ...mockUser, profile_uid: 'USER_PROFILE_ID' },
+            {
+              authHeaders: mockAuthHeaders,
+              authResponseHeaders: mockAuthResponseHeaders,
+            }
+          )
+        );
+
+        await authHandler(mockRequest, mockResponse, mockAuthToolkit);
+
+        expect(mockAuthToolkit.authenticated).toHaveBeenCalledTimes(1);
+        expect(mockAuthToolkit.authenticated).toHaveBeenCalledWith({
+          state: { ...mockUser, profile_uid: 'USER_PROFILE_ID' },
+          requestHeaders: mockAuthHeaders,
+          responseHeaders: mockAuthResponseHeaders,
+        });
+      });
+
       it('redirects user if redirection is requested by the authenticator preserving authentication response headers if any', async () => {
         const mockResponse = httpServerMock.createLifecycleResponseFactory();
         authenticate.mockResolvedValue(

diff --git a/x-pack/plugins/security/server/authentication/authenticator.test.ts b/x-pack/plugins/security/server/authentication/authenticator.test.ts
@@ -39,7 +39,7 @@ import type { UserProfileGrant } from '../user_profile';
 import { userProfileServiceMock } from '../user_profile/user_profile_service.mock';
 import { AuthenticationResult } from './authentication_result';
 import type { AuthenticatorOptions } from './authenticator';
-import { Authenticator } from './authenticator';
+import { Authenticator, enrichWithUserProfileId } from './authenticator';
 import { DeauthenticationResult } from './deauthentication_result';
 import type { BasicAuthenticationProvider, SAMLAuthenticationProvider } from './providers';
 
@@ -379,6 +379,29 @@ describe('Authenticator', () => {
       expectAuditEvents({ action: 'user_login', outcome: 'success' });
     });
 
+    it('returns user enriched with user profile id.', async () => {
+      const request = httpServerMock.createKibanaRequest();
+      const user = mockAuthenticatedUser({ profile_uid: undefined });
+      mockOptions.session.create.mockResolvedValue(
+        sessionMock.createValue({
+          userProfileId: 'PROFILE_ID',
+        })
+      );
+
+      mockBasicAuthenticationProvider.login.mockResolvedValue(
+        AuthenticationResult.succeeded(user, {
+          state: {}, // to ensure a new session is created
+        })
+      );
+
+      const result = await authenticator.login(request, { provider: { type: 'basic' }, value: {} });
+      expect(result.user).toEqual(
+        expect.objectContaining({
+          profile_uid: 'PROFILE_ID',
+        })
+      );
+    });
+
     describe('user_login audit events', () => {
       // Every other test case includes audit event assertions, but the user_login event is a bit special.
       // We have these separate, detailed test cases to ensure that the session ID is included for user_login success events.
@@ -2560,3 +2583,65 @@ describe('Authenticator', () => {
     });
   });
 });
+
+describe('enrichWithUserProfileId', () => {
+  it('should enrich succeeded authentication results with user profile id', () => {
+    const authenticationResult = AuthenticationResult.succeeded(
+      mockAuthenticatedUser({ profile_uid: undefined })
+    );
+    const sessionValue = sessionMock.createValue({ userProfileId: 'uid' });
+    expect(enrichWithUserProfileId(authenticationResult, sessionValue)).toEqual(
+      expect.objectContaining({
+        user: expect.objectContaining({
+          profile_uid: 'uid',
+        }),
+      })
+    );
+  });
+
+  it('should enrich redirected authentication results with user profile id', () => {
+    const authenticationResult = AuthenticationResult.redirectTo('/redirect/to', {
+      user: mockAuthenticatedUser({ profile_uid: undefined }),
+    });
+    const sessionValue = sessionMock.createValue({ userProfileId: 'uid' });
+    expect(enrichWithUserProfileId(authenticationResult, sessionValue)).toEqual(
+      expect.objectContaining({
+        user: expect.objectContaining({
+          profile_uid: 'uid',
+        }),
+      })
+    );
+  });
+
+  it('should not change unhandled authentication results', () => {
+    const authenticationResult = AuthenticationResult.notHandled();
+    const sessionValue = sessionMock.createValue();
+    expect(enrichWithUserProfileId(authenticationResult, sessionValue)).toBe(authenticationResult);
+  });
+
+  it('should not change failed authentication results', () => {
+    const authenticationResult = AuthenticationResult.failed(new Error('Authentication error'));
+    const sessionValue = sessionMock.createValue();
+    expect(enrichWithUserProfileId(authenticationResult, sessionValue)).toBe(authenticationResult);
+  });
+
+  it('should not change redirected authentication results without user', () => {
+    const authenticationResult = AuthenticationResult.redirectTo('/redirect/to');
+    const sessionValue = sessionMock.createValue();
+    expect(enrichWithUserProfileId(authenticationResult, sessionValue)).toBe(authenticationResult);
+  });
+
+  it('should not change succeeded authentication result if session has no user profile id', () => {
+    const authenticationResult = AuthenticationResult.succeeded(mockAuthenticatedUser());
+    const sessionValue = sessionMock.createValue({ userProfileId: undefined });
+    expect(enrichWithUserProfileId(authenticationResult, sessionValue)).toBe(authenticationResult);
+  });
+
+  it('should not change succeeded authentication result if user profile ids already match', () => {
+    const authenticationResult = AuthenticationResult.succeeded(
+      mockAuthenticatedUser({ profile_uid: 'uid' })
+    );
+    const sessionValue = sessionMock.createValue({ userProfileId: 'uid' });
+    expect(enrichWithUserProfileId(authenticationResult, sessionValue)).toBe(authenticationResult);
+  });
+});