No longer use generated IDs for X-Opaque-ID / requestId

docs/development/core/server/kibana-plugin-core-server.kibanarequest.md

@@ -26,8 +26,8 @@ export declare class KibanaRequest<Params = unknown, Query = unknown, Body = unk
 |  [body](./kibana-plugin-core-server.kibanarequest.body.md) |  | Body |  |
 |  [events](./kibana-plugin-core-server.kibanarequest.events.md) |  | KibanaRequestEvents | Request events [KibanaRequestEvents](./kibana-plugin-core-server.kibanarequestevents.md) |
 |  [headers](./kibana-plugin-core-server.kibanarequest.headers.md) |  | Headers | Readonly copy of incoming request headers. |
-|  [id](./kibana-plugin-core-server.kibanarequest.id.md) |  | string | A identifier to identify this request. |
 |  [isSystemRequest](./kibana-plugin-core-server.kibanarequest.issystemrequest.md) |  | boolean | Whether or not the request is a "system request" rather than an application-level request. Can be set on the client using the <code>HttpFetchOptions#asSystemRequest</code> option. |
+|  [opaqueId?](./kibana-plugin-core-server.kibanarequest.opaqueid.md) |  | string | <i>(Optional)</i> The (optional) opaqueId of this request. |
 |  [params](./kibana-plugin-core-server.kibanarequest.params.md) |  | Params |  |
 |  [query](./kibana-plugin-core-server.kibanarequest.query.md) |  | Query |  |
 |  [rewrittenUrl?](./kibana-plugin-core-server.kibanarequest.rewrittenurl.md) |  | URL | <i>(Optional)</i> URL rewritten in onPreRouting request interceptor. |

docs/development/core/server/kibana-plugin-core-server.kibanarequest.opaqueid.md

@@ -0,0 +1,18 @@
+<!-- Do not edit this file. It is automatically generated by API Documenter. -->
+
+[Home](./index.md) &gt; [kibana-plugin-core-server](./kibana-plugin-core-server.md) &gt; [KibanaRequest](./kibana-plugin-core-server.kibanarequest.md) &gt; [opaqueId](./kibana-plugin-core-server.kibanarequest.opaqueid.md)
+
+## KibanaRequest.opaqueId property
+
+The (optional) opaqueId of this request.
+
+<b>Signature:</b>
+
+```typescript
+readonly opaqueId?: string;
+```
+
+## Remarks
+
+This value is sourced from the incoming request's `X-Opaque-Id` header which is not guaranteed to be unique per request. When present, it should contain a userId in any format.
+

packages/kbn-server-http-tools/src/get_request_id.test.ts

@@ -8,20 +8,14 @@
 
 import { getRequestId } from './get_request_id';
 
-jest.mock('uuid', () => ({
-  v4: jest.fn().mockReturnValue('xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx'),
-}));
-
 describe('getRequestId', () => {
   describe('when allowFromAnyIp is true', () => {
-    it('generates a UUID if no x-opaque-id header is present', () => {
+    it('returns undefined if no x-opaque-id header is present', () => {
       const request = {
         headers: {},
         raw: { req: { socket: { remoteAddress: '1.1.1.1' } } },
       } as any;
-      expect(getRequestId(request, { allowFromAnyIp: true, ipAllowlist: [] })).toEqual(
-        'xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx'
-      );
+      expect(getRequestId(request, { allowFromAnyIp: true, ipAllowlist: [] })).toBeUndefined();
     });
 
     it('uses x-opaque-id header value if present', () => {
@@ -39,14 +33,12 @@ describe('getRequestId', () => {
 
   describe('when allowFromAnyIp is false', () => {
     describe('and ipAllowlist is empty', () => {
-      it('generates a UUID even if x-opaque-id header is present', () => {
+      it('returns undefined even if x-opaque-id header is present', () => {
         const request = {
           headers: { 'x-opaque-id': 'id from header' },
           raw: { req: { socket: { remoteAddress: '1.1.1.1' } } },
         } as any;
-        expect(getRequestId(request, { allowFromAnyIp: false, ipAllowlist: [] })).toEqual(
-          'xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx'
-        );
+        expect(getRequestId(request, { allowFromAnyIp: false, ipAllowlist: [] })).toBeUndefined();
       });
     });
 
@@ -61,24 +53,24 @@ describe('getRequestId', () => {
         );
       });
 
-      it('generates a UUID if request comes from untrusted IP address', () => {
+      it('does not use x-opaque-id header if request comes from untrusted IP address', () => {
         const request = {
           headers: { 'x-opaque-id': 'id from header' },
           raw: { req: { socket: { remoteAddress: '5.5.5.5' } } },
         } as any;
-        expect(getRequestId(request, { allowFromAnyIp: false, ipAllowlist: ['1.1.1.1'] })).toEqual(
-          'xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx'
-        );
+        expect(
+          getRequestId(request, { allowFromAnyIp: false, ipAllowlist: ['1.1.1.1'] })
+        ).toBeUndefined();
       });
 
-      it('generates UUID if request comes from trusted IP address but no x-opaque-id header is present', () => {
+      it('returns undefined if request comes from trusted IP address but no x-opaque-id header is present', () => {
         const request = {
           headers: {},
           raw: { req: { socket: { remoteAddress: '1.1.1.1' } } },
         } as any;
-        expect(getRequestId(request, { allowFromAnyIp: false, ipAllowlist: ['1.1.1.1'] })).toEqual(
-          'xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx'
-        );
+        expect(
+          getRequestId(request, { allowFromAnyIp: false, ipAllowlist: ['1.1.1.1'] })
+        ).toBeUndefined();
       });
     });
   });

packages/kbn-server-http-tools/src/get_request_id.ts

@@ -7,16 +7,20 @@
  */
 
 import { Request } from '@hapi/hapi';
-import uuid from 'uuid';
 
+/**
+ * Return the requestId for this request from its `x-opaque-id` header,
+ * depending on the `server.requestId` configuration, or undefined
+ * if the value should not be used.
+ */
 export function getRequestId(
   request: Request,
   { allowFromAnyIp, ipAllowlist }: { allowFromAnyIp: boolean; ipAllowlist: string[] }
-): string {
+): string | undefined {
   const remoteAddress = request.raw.req.socket?.remoteAddress;
   return allowFromAnyIp ||
     // socket may be undefined in integration tests that connect via the http listener directly
     (remoteAddress && ipAllowlist.includes(remoteAddress))
-    ? request.headers['x-opaque-id'] ?? uuid.v4()
-    : uuid.v4();
+    ? request.headers['x-opaque-id']
+    : undefined;
 }

src/core/server/elasticsearch/client/cluster_client.ts

@@ -137,12 +137,13 @@ export class ClusterClient implements ICustomClusterClient {
     let scopedHeaders: Headers;
     if (isRealRequest(request)) {
       const requestHeaders = ensureRawRequest(request).headers ?? {};
-      const requestIdHeaders = isKibanaRequest(request) ? { 'x-opaque-id': request.id } : {};
+      const opaqueIdHeaders =
+        isKibanaRequest(request) && request.opaqueId ? { 'x-opaque-id': request.opaqueId } : {};
       const authHeaders = this.authHeaders ? this.authHeaders.get(request) : {};
 
       scopedHeaders = {
         ...filterHeaders(requestHeaders, this.config.requestHeadersWhitelist),
-        ...requestIdHeaders,
+        ...opaqueIdHeaders,
         ...authHeaders,
       };
     } else {

src/core/server/execution_context/execution_context_service.ts

@@ -79,8 +79,8 @@ export class ExecutionContextService
 
   constructor(private readonly coreContext: CoreContext) {
     this.log = coreContext.logger.get('execution_context');
-    this.contextStore = new AsyncLocalStorage<IExecutionContextContainer>();
-    this.requestIdStore = new AsyncLocalStorage<{ requestId: string }>();
+    this.contextStore = new AsyncLocalStorage();
+    this.requestIdStore = new AsyncLocalStorage();
   }
 
   setup(): InternalExecutionContextSetup {
@@ -154,7 +154,7 @@ export class ExecutionContextService
 
   private getAsHeader(): string | undefined {
     if (!this.enabled) return;
-    // requestId may not be present in the case of FakeRequest
+    // requestId may not be present if unspecified by the client or in the case of FakeRequest
     const requestId = this.requestIdStore.getStore()?.requestId ?? 'unknownId';
     const executionContext = this.contextStore.getStore()?.toString();
     const executionContextStr = executionContext ? `;kibana:${executionContext}` : '';

src/core/server/http/http_server.ts

@@ -340,7 +340,9 @@ export class HttpServer {
       const parentContext = executionContext?.getParentContextFrom(request.headers);
       if (parentContext) executionContext?.set(parentContext);
 
-      executionContext?.setRequestId(requestId);
+      if (requestId) {
+        executionContext?.setRequestId(requestId);
+      }
 
       request.app = {
         ...(request.app ?? {}),

src/core/server/http/integration_tests/request.test.ts

@@ -344,7 +344,7 @@ describe('KibanaRequest', () => {
       const { server: innerServer, createRouter } = await server.setup(setupDeps);
       const router = createRouter('/');
       router.get({ path: '/', validate: false }, async (context, req, res) => {
-        return res.ok({ body: { requestId: req.id } });
+        return res.ok({ body: { requestId: req.opaqueId } });
       });
       await server.start();
 

src/core/server/http/router/request.test.ts

@@ -16,31 +16,31 @@ import { httpServerMock } from '../http_server.mocks';
 import { schema } from '@kbn/config-schema';
 
 describe('KibanaRequest', () => {
-  describe('id property', () => {
+  describe('opaqueId property', () => {
     it('uses the request.app.requestId property if present', () => {
       const request = httpServerMock.createRawRequest({
         app: { requestId: 'fakeId' },
       });
       const kibanaRequest = KibanaRequest.from(request);
-      expect(kibanaRequest.id).toEqual('fakeId');
+      expect(kibanaRequest.opaqueId).toEqual('fakeId');
     });
 
-    it('generates a new UUID if request.app property is not present', () => {
+    it('is undefined if request.app property is not present', () => {
       // Undefined app property
       const request = httpServerMock.createRawRequest({
         app: undefined,
       });
       const kibanaRequest = KibanaRequest.from(request);
-      expect(kibanaRequest.id).toEqual('xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx');
+      expect(kibanaRequest.opaqueId).toBeUndefined();
     });
 
-    it('generates a new UUID if request.app.requestId property is not present', () => {
+    it('is undefined if request.app.requestId property is not present', () => {
       // Undefined app.requestId property
       const request = httpServerMock.createRawRequest({
         app: {},
       });
       const kibanaRequest = KibanaRequest.from(request);
-      expect(kibanaRequest.id).toEqual('xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx');
+      expect(kibanaRequest.opaqueId).toBeUndefined();
     });
   });
 

src/core/server/http/router/request.ts

@@ -32,7 +32,7 @@ export interface KibanaRouteOptions extends RouteOptionsApp {
  * @internal
  */
 export interface KibanaRequestState extends RequestApplicationState {
-  requestId: string;
+  requestId?: string;
   requestUuid: string;
   rewrittenUrl?: URL;
   traceId?: string;
@@ -127,21 +127,20 @@ export class KibanaRequest<
     const body = routeValidator.getBody(req.payload, 'request body');
     return { query, params, body };
   }
+
   /**
-   * A identifier to identify this request.
+   * The (optional) opaqueId of this request.
    *
-   * @remarks
-   * Depending on the user's configuration, this value may be sourced from the
-   * incoming request's `X-Opaque-Id` header which is not guaranteed to be unique
-   * per request.
+   * @remarks This value is sourced from the incoming request's `X-Opaque-Id` header
+   *          which is not guaranteed to be unique per request. When present, it should
+   *          contain a userId in any format.
    */
-  public readonly id: string;
+  public readonly opaqueId?: string;
   /**
    * A UUID to identify this request.
    *
-   * @remarks
-   * This value is NOT sourced from the incoming request's `X-Opaque-Id` header. it
-   * is always a UUID uniquely identifying the request.
+   * @remarks This value is NOT sourced from the incoming request's `X-Opaque-Id` header.
+   *          it is always a UUID uniquely identifying the request.
    */
   public readonly uuid: string;
   /** a WHATWG URL standard object. */
@@ -186,11 +185,11 @@ export class KibanaRequest<
     // until that time we have to expose all the headers
     private readonly withoutSecretHeaders: boolean
   ) {
-    // The `requestId` and `requestUuid` properties will not be populated for requests that are 'faked' by internal systems that leverage
+    // The `opaqueId` and `requestUuid` properties will not be populated for requests that are 'faked' by internal systems that leverage
     // KibanaRequest in conjunction with scoped Elasticsearch and SavedObjectsClient in order to pass credentials.
     // In these cases, the ids default to a newly generated UUID.
     const appState = request.app as KibanaRequestState | undefined;
-    this.id = appState?.requestId ?? uuid.v4();
+    this.opaqueId = appState?.requestId;
     this.uuid = appState?.requestUuid ?? uuid.v4();
     this.rewrittenUrl = appState?.rewrittenUrl;
 

src/core/server/saved_objects/routes/integration_tests/import.test.ts

@@ -488,8 +488,7 @@ describe(`POST ${URL}`, () => {
       const mockUuid = jest.requireMock('uuid');
       mockUuid.v4 = jest
         .fn()
-        .mockReturnValueOnce('foo') // a uuid.v4() is generated for the request.id
-        .mockReturnValueOnce('foo') // another uuid.v4() is used for the request.uuid
+        .mockReturnValueOnce('foo') // a uuid.v4() is used for the request.uuid
         .mockReturnValueOnce('new-id-1')
         .mockReturnValueOnce('new-id-2');
       savedObjectsClient.bulkGet.mockResolvedValueOnce({ saved_objects: [mockIndexPattern] });

src/core/server/server.api.md

@@ -1350,8 +1350,8 @@ export class KibanaRequest<Params = unknown, Query = unknown, Body = unknown, Me
     // @internal
     static from<P, Q, B>(req: Request_2, routeSchemas?: RouteValidator<P, Q, B> | RouteValidatorFullConfig<P, Q, B>, withoutSecretHeaders?: boolean): KibanaRequest<P, Q, B, any>;
     readonly headers: Headers_2;
-    readonly id: string;
     readonly isSystemRequest: boolean;
+    readonly opaqueId?: string;
     // (undocumented)
     readonly params: Params;
     // (undocumented)

src/plugins/data/server/search/expressions/esaggs.test.ts

@@ -57,7 +57,7 @@ describe('esaggs expression function - server', () => {
     jest.clearAllMocks();
     mockHandlers = {
       abortSignal: jest.fn() as unknown as jest.Mocked<AbortSignal>,
-      getKibanaRequest: jest.fn().mockReturnValue({ id: 'hi' } as KibanaRequest),
+      getKibanaRequest: jest.fn().mockReturnValue({ opaqueId: 'hi' } as KibanaRequest),
       getSearchContext: jest.fn(),
       getSearchSessionId: jest.fn().mockReturnValue('abc123'),
       getExecutionContext: jest.fn(),
@@ -81,7 +81,7 @@ describe('esaggs expression function - server', () => {
   test('calls getStartDependencies with the KibanaRequest', async () => {
     await definition().fn(null, args, mockHandlers).toPromise();
 
-    expect(getStartDependencies).toHaveBeenCalledWith({ id: 'hi' });
+    expect(getStartDependencies).toHaveBeenCalledWith({ opaqueId: 'hi' });
   });
 
   test('calls indexPatterns.create with the values provided by the subexpression arg', async () => {

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

@@ -166,7 +166,7 @@ export class AuditService {
             session_id: sessionId,
             ...event.kibana,
           },
-          trace: { id: request.id },
+          trace: { id: request.opaqueId },
         });
       },
     });