Fix detection of "system requests" in plugins

src/legacy/core_plugins/kibana/server/lib/__tests__/system_api.js

@@ -22,7 +22,16 @@ import { isSystemApiRequest } from '../system_api';
 
 describe('system_api', () => {
   describe('#isSystemApiRequest', () => {
-    it('returns true for a system API HTTP request', () => {
+    it('returns true for a system HTTP request', () => {
+      const mockHapiRequest = {
+        headers: {
+          'kbn-system-request': true,
+        },
+      };
+      expect(isSystemApiRequest(mockHapiRequest)).to.be(true);
+    });
+
+    it('returns true for a legacy system API HTTP request', () => {
       const mockHapiRequest = {
         headers: {
           'kbn-system-api': true,

src/legacy/core_plugins/kibana/server/lib/system_api.js

@@ -17,7 +17,8 @@
  * under the License.
  */
 
-const SYSTEM_API_HEADER_NAME = 'kbn-system-api';
+const SYSTEM_REQUEST_HEADER_NAME = 'kbn-system-request';
+const LEGACY_SYSTEM_API_HEADER_NAME = 'kbn-system-api';
 
 /**
  * Checks on the *server-side*, if an HTTP request is a system API request
@@ -27,5 +28,8 @@ const SYSTEM_API_HEADER_NAME = 'kbn-system-api';
  * @deprecated Use KibanaRequest#isSystemApi
  */
 export function isSystemApiRequest(request) {
-  return !!request.headers[SYSTEM_API_HEADER_NAME];
+  return (
+    !!request.headers[SYSTEM_REQUEST_HEADER_NAME] ||
+    !!request.headers[LEGACY_SYSTEM_API_HEADER_NAME]
+  );
 }

src/legacy/ui/public/system_api/__tests__/system_api.js

@@ -31,16 +31,25 @@ describe('system_api', () => {
       };
       const newHeaders = addSystemApiHeader(headers);
 
-      expect(newHeaders).to.have.property('kbn-system-api');
-      expect(newHeaders['kbn-system-api']).to.be(true);
+      expect(newHeaders).to.have.property('kbn-system-request');
+      expect(newHeaders['kbn-system-request']).to.be(true);
 
       expect(newHeaders).to.have.property('kbn-version');
       expect(newHeaders['kbn-version']).to.be('4.6.0');
     });
   });
 
   describe('#isSystemApiRequest', () => {
-    it('returns true for a system API HTTP request', () => {
+    it('returns true for a system HTTP request', () => {
+      const mockRequest = {
+        headers: {
+          'kbn-system-request': true,
+        },
+      };
+      expect(isSystemApiRequest(mockRequest)).to.be(true);
+    });
+
+    it('returns true for a legacy system API HTTP request', () => {
       const mockRequest = {
         headers: {
           'kbn-system-api': true,

src/plugins/kibana_legacy/public/utils/system_api.ts

@@ -19,7 +19,8 @@
 
 import { IRequestConfig } from 'angular';
 
-const SYSTEM_API_HEADER_NAME = 'kbn-system-api';
+const SYSTEM_REQUEST_HEADER_NAME = 'kbn-system-request';
+const LEGACY_SYSTEM_API_HEADER_NAME = 'kbn-system-api';
 
 /**
  * Adds a custom header designating request as system API
@@ -28,7 +29,7 @@ const SYSTEM_API_HEADER_NAME = 'kbn-system-api';
  */
 export function addSystemApiHeader(originalHeaders: Record<string, string>) {
   const systemApiHeaders = {
-    [SYSTEM_API_HEADER_NAME]: true,
+    [SYSTEM_REQUEST_HEADER_NAME]: true,
   };
   return {
     ...originalHeaders,
@@ -44,5 +45,7 @@ export function addSystemApiHeader(originalHeaders: Record<string, string>) {
  */
 export function isSystemApiRequest(request: IRequestConfig) {
   const { headers } = request;
-  return headers && !!headers[SYSTEM_API_HEADER_NAME];
+  return (
+    headers && (!!headers[SYSTEM_REQUEST_HEADER_NAME] || !!headers[LEGACY_SYSTEM_API_HEADER_NAME])
+  );
 }

x-pack/legacy/plugins/security/index.js

@@ -115,9 +115,6 @@ export const security = kibana =>
       const xpackInfo = server.plugins.xpack_main.info;
       securityPlugin.__legacyCompat.registerLegacyAPI({
         auditLogger: new AuditLogger(server, 'security', config, xpackInfo),
-        isSystemAPIRequest: server.plugins.kibana.systemApi.isSystemApiRequest.bind(
-          server.plugins.kibana.systemApi
-        ),
       });
 
       // Legacy xPack Info endpoint returns whatever we return in a callback for `registerLicenseCheckResultsGenerator`

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

@@ -28,7 +28,6 @@ function getMockOptions(config: Partial<AuthenticatorOptions['config']> = {}) {
     clusterClient: elasticsearchServiceMock.createClusterClient(),
     basePath: httpServiceMock.createSetupContract().basePath,
     loggers: loggingServiceMock.create(),
-    isSystemAPIRequest: jest.fn(),
     config: {
       session: { idleTimeout: null, lifespan: null },
       authc: { providers: [], oidc: {}, saml: {} },
@@ -286,10 +285,11 @@ describe('Authenticator', () => {
 
     it('creates session whenever authentication provider returns state for system API requests', async () => {
       const user = mockAuthenticatedUser();
-      const request = httpServerMock.createKibanaRequest();
+      const request = httpServerMock.createKibanaRequest({
+        headers: { 'kbn-system-request': 'true' },
+      });
       const authorization = `Basic ${Buffer.from('foo:bar').toString('base64')}`;
 
-      mockOptions.isSystemAPIRequest.mockReturnValue(true);
       mockBasicAuthenticationProvider.authenticate.mockResolvedValue(
         AuthenticationResult.succeeded(user, { state: { authorization } })
       );
@@ -307,10 +307,11 @@ describe('Authenticator', () => {
 
     it('creates session whenever authentication provider returns state for non-system API requests', async () => {
       const user = mockAuthenticatedUser();
-      const request = httpServerMock.createKibanaRequest();
+      const request = httpServerMock.createKibanaRequest({
+        headers: { 'kbn-system-request': 'false' },
+      });
       const authorization = `Basic ${Buffer.from('foo:bar').toString('base64')}`;
 
-      mockOptions.isSystemAPIRequest.mockReturnValue(false);
       mockBasicAuthenticationProvider.authenticate.mockResolvedValue(
         AuthenticationResult.succeeded(user, { state: { authorization } })
       );
@@ -328,9 +329,10 @@ describe('Authenticator', () => {
 
     it('does not extend session for system API calls.', async () => {
       const user = mockAuthenticatedUser();
-      const request = httpServerMock.createKibanaRequest();
+      const request = httpServerMock.createKibanaRequest({
+        headers: { 'kbn-system-request': 'true' },
+      });
 
-      mockOptions.isSystemAPIRequest.mockReturnValue(true);
       mockBasicAuthenticationProvider.authenticate.mockResolvedValue(
         AuthenticationResult.succeeded(user)
       );
@@ -346,9 +348,10 @@ describe('Authenticator', () => {
 
     it('extends session for non-system API calls.', async () => {
       const user = mockAuthenticatedUser();
-      const request = httpServerMock.createKibanaRequest();
+      const request = httpServerMock.createKibanaRequest({
+        headers: { 'kbn-system-request': 'false' },
+      });
 
-      mockOptions.isSystemAPIRequest.mockReturnValue(false);
       mockBasicAuthenticationProvider.authenticate.mockResolvedValue(
         AuthenticationResult.succeeded(user)
       );
@@ -510,9 +513,10 @@ describe('Authenticator', () => {
     });
 
     it('does not touch session for system API calls if authentication fails with non-401 reason.', async () => {
-      const request = httpServerMock.createKibanaRequest();
+      const request = httpServerMock.createKibanaRequest({
+        headers: { 'kbn-system-request': 'true' },
+      });
 
-      mockOptions.isSystemAPIRequest.mockReturnValue(true);
       mockBasicAuthenticationProvider.authenticate.mockResolvedValue(
         AuthenticationResult.failed(new Error('some error'))
       );
@@ -526,9 +530,10 @@ describe('Authenticator', () => {
     });
 
     it('does not touch session for non-system API calls if authentication fails with non-401 reason.', async () => {
-      const request = httpServerMock.createKibanaRequest();
+      const request = httpServerMock.createKibanaRequest({
+        headers: { 'kbn-system-request': 'false' },
+      });
 
-      mockOptions.isSystemAPIRequest.mockReturnValue(false);
       mockBasicAuthenticationProvider.authenticate.mockResolvedValue(
         AuthenticationResult.failed(new Error('some error'))
       );
@@ -544,9 +549,10 @@ describe('Authenticator', () => {
     it('replaces existing session with the one returned by authentication provider for system API requests', async () => {
       const user = mockAuthenticatedUser();
       const newState = { authorization: 'Basic yyy' };
-      const request = httpServerMock.createKibanaRequest();
+      const request = httpServerMock.createKibanaRequest({
+        headers: { 'kbn-system-request': 'true' },
+      });
 
-      mockOptions.isSystemAPIRequest.mockReturnValue(true);
       mockBasicAuthenticationProvider.authenticate.mockResolvedValue(
         AuthenticationResult.succeeded(user, { state: newState })
       );
@@ -567,9 +573,10 @@ describe('Authenticator', () => {
     it('replaces existing session with the one returned by authentication provider for non-system API requests', async () => {
       const user = mockAuthenticatedUser();
       const newState = { authorization: 'Basic yyy' };
-      const request = httpServerMock.createKibanaRequest();
+      const request = httpServerMock.createKibanaRequest({
+        headers: { 'kbn-system-request': 'false' },
+      });
 
-      mockOptions.isSystemAPIRequest.mockReturnValue(false);
       mockBasicAuthenticationProvider.authenticate.mockResolvedValue(
         AuthenticationResult.succeeded(user, { state: newState })
       );
@@ -588,9 +595,10 @@ describe('Authenticator', () => {
     });
 
     it('clears session if provider failed to authenticate system API request with 401 with active session.', async () => {
-      const request = httpServerMock.createKibanaRequest();
+      const request = httpServerMock.createKibanaRequest({
+        headers: { 'kbn-system-request': 'true' },
+      });
 
-      mockOptions.isSystemAPIRequest.mockReturnValue(true);
       mockBasicAuthenticationProvider.authenticate.mockResolvedValue(
         AuthenticationResult.failed(Boom.unauthorized())
       );
@@ -604,9 +612,10 @@ describe('Authenticator', () => {
     });
 
     it('clears session if provider failed to authenticate non-system API request with 401 with active session.', async () => {
-      const request = httpServerMock.createKibanaRequest();
+      const request = httpServerMock.createKibanaRequest({
+        headers: { 'kbn-system-request': 'false' },
+      });
 
-      mockOptions.isSystemAPIRequest.mockReturnValue(false);
       mockBasicAuthenticationProvider.authenticate.mockResolvedValue(
         AuthenticationResult.failed(Boom.unauthorized())
       );
@@ -635,9 +644,10 @@ describe('Authenticator', () => {
     });
 
     it('does not clear session if provider can not handle system API request authentication with active session.', async () => {
-      const request = httpServerMock.createKibanaRequest();
+      const request = httpServerMock.createKibanaRequest({
+        headers: { 'kbn-system-request': 'true' },
+      });
 
-      mockOptions.isSystemAPIRequest.mockReturnValue(true);
       mockBasicAuthenticationProvider.authenticate.mockResolvedValue(
         AuthenticationResult.notHandled()
       );
@@ -651,9 +661,10 @@ describe('Authenticator', () => {
     });
 
     it('does not clear session if provider can not handle non-system API request authentication with active session.', async () => {
-      const request = httpServerMock.createKibanaRequest();
+      const request = httpServerMock.createKibanaRequest({
+        headers: { 'kbn-system-request': 'false' },
+      });
 
-      mockOptions.isSystemAPIRequest.mockReturnValue(false);
       mockBasicAuthenticationProvider.authenticate.mockResolvedValue(
         AuthenticationResult.notHandled()
       );
@@ -667,9 +678,10 @@ describe('Authenticator', () => {
     });
 
     it('clears session for system API request if it belongs to not configured provider.', async () => {
-      const request = httpServerMock.createKibanaRequest();
+      const request = httpServerMock.createKibanaRequest({
+        headers: { 'kbn-system-request': 'true' },
+      });
 
-      mockOptions.isSystemAPIRequest.mockReturnValue(true);
       mockBasicAuthenticationProvider.authenticate.mockResolvedValue(
         AuthenticationResult.notHandled()
       );
@@ -683,9 +695,10 @@ describe('Authenticator', () => {
     });
 
     it('clears session for non-system API request if it belongs to not configured provider.', async () => {
-      const request = httpServerMock.createKibanaRequest();
+      const request = httpServerMock.createKibanaRequest({
+        headers: { 'kbn-system-request': 'false' },
+      });
 
-      mockOptions.isSystemAPIRequest.mockReturnValue(false);
       mockBasicAuthenticationProvider.authenticate.mockResolvedValue(
         AuthenticationResult.notHandled()
       );

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

@@ -88,7 +88,6 @@ export interface AuthenticatorOptions {
   loggers: LoggerFactory;
   clusterClient: IClusterClient;
   sessionStorageFactory: SessionStorageFactory<ProviderSession>;
-  isSystemAPIRequest: (request: KibanaRequest) => boolean;
 }
 
 // Mapping between provider key defined in the config and authentication
@@ -310,7 +309,7 @@ export class Authenticator {
 
       this.updateSessionValue(sessionStorage, {
         providerType,
-        isSystemAPIRequest: this.options.isSystemAPIRequest(request),
+        isSystemRequest: request.isSystemRequest,
         authenticationResult,
         existingSession: ownsSession ? existingSession : null,
       });
@@ -434,12 +433,12 @@ export class Authenticator {
       providerType,
       authenticationResult,
       existingSession,
-      isSystemAPIRequest,
+      isSystemRequest,
     }: {
       providerType: string;
       authenticationResult: AuthenticationResult;
       existingSession: ProviderSession | null;
-      isSystemAPIRequest: boolean;
+      isSystemRequest: boolean;
     }
   ) {
     if (!existingSession && !authenticationResult.shouldUpdateState()) {
@@ -451,7 +450,7 @@ export class Authenticator {
     // state we should store it in the session regardless of whether it's a system API request or not.
     const sessionCanBeUpdated =
       (authenticationResult.succeeded() || authenticationResult.redirected()) &&
-      (authenticationResult.shouldUpdateState() || !isSystemAPIRequest);
+      (authenticationResult.shouldUpdateState() || !isSystemRequest);
 
     // If provider owned the session, but failed to authenticate anyway, that likely means that
     // session is not valid and we should clear it. Also provider can specifically ask to clear

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

@@ -32,7 +32,6 @@ import {
 } from '../../../../../src/core/server';
 import { AuthenticatedUser } from '../../common/model';
 import { ConfigType, createConfig$ } from '../config';
-import { LegacyAPI } from '../plugin';
 import { AuthenticationResult } from './authentication_result';
 import { setupAuthentication } from '.';
 import {
@@ -47,7 +46,6 @@ describe('setupAuthentication()', () => {
   let mockSetupAuthenticationParams: {
     config: ConfigType;
     loggers: LoggerFactory;
-    getLegacyAPI(): Pick<LegacyAPI, 'isSystemAPIRequest'>;
     http: jest.Mocked<CoreSetup['http']>;
     clusterClient: jest.Mocked<IClusterClient>;
     license: jest.Mocked<SecurityLicense>;
@@ -73,7 +71,6 @@ describe('setupAuthentication()', () => {
       clusterClient: elasticsearchServiceMock.createClusterClient(),
       license: licenseMock.create(),
       loggers: loggingServiceMock.create(),
-      getLegacyAPI: jest.fn(),
     };
 
     mockScopedClusterClient = elasticsearchServiceMock.createScopedClusterClient();

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

@@ -14,7 +14,6 @@ import { AuthenticatedUser } from '../../common/model';
 import { ConfigType } from '../config';
 import { getErrorStatusCode } from '../errors';
 import { Authenticator, ProviderSession } from './authenticator';
-import { LegacyAPI } from '../plugin';
 import { APIKeys, CreateAPIKeyParams, InvalidateAPIKeyParams } from './api_keys';
 import { SecurityLicense } from '../../common/licensing';
 
@@ -36,7 +35,6 @@ interface SetupAuthenticationParams {
   config: ConfigType;
   license: SecurityLicense;
   loggers: LoggerFactory;
-  getLegacyAPI(): Pick<LegacyAPI, 'isSystemAPIRequest'>;
 }
 
 export type Authentication = UnwrapPromise<ReturnType<typeof setupAuthentication>>;
@@ -47,7 +45,6 @@ export async function setupAuthentication({
   config,
   license,
   loggers,
-  getLegacyAPI,
 }: SetupAuthenticationParams) {
   const authLogger = loggers.get('authentication');
 
@@ -83,7 +80,6 @@ export async function setupAuthentication({
     clusterClient,
     basePath: http.basePath,
     config: { session: config.session, authc: config.authc },
-    isSystemAPIRequest: (request: KibanaRequest) => getLegacyAPI().isSystemAPIRequest(request),
     loggers,
     sessionStorageFactory: await http.createCookieSessionStorageFactory({
       encryptionKey: config.encryptionKey,