Review#1: rework the way we detect which additional schemes should be…

… supported by the HTTP authentication provider.

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

@@ -106,7 +106,6 @@ const providerMap = new Map<
   [TokenAuthenticationProvider.type, TokenAuthenticationProvider],
   [OIDCAuthenticationProvider.type, OIDCAuthenticationProvider],
   [PKIAuthenticationProvider.type, PKIAuthenticationProvider],
-  [HTTPAuthenticationProvider.type, HTTPAuthenticationProvider],
 ]);
 
 function assertRequest(request: KibanaRequest) {
@@ -221,6 +220,17 @@ export class Authenticator {
         ] as [string, BaseAuthenticationProvider];
       })
     );
+
+    // For the BWC reasons we always include HTTP authentication provider unless it's explicitly disabled.
+    if (this.options.config.authc.http.enabled) {
+      this.setupHTTPAuthenticationProvider(
+        Object.freeze({
+          ...providerCommonOptions,
+          logger: options.loggers.get(HTTPAuthenticationProvider.type),
+        })
+      );
+    }
+
     this.serverBasePath = this.options.basePath.serverBasePath || '/';
 
     this.idleTimeout = this.options.config.session.idleTimeout;
@@ -398,6 +408,33 @@ export class Authenticator {
     return this.providers.has(providerType);
   }
 
+  /**
+   * Initializes HTTP Authentication provider and appends it to the end of the list of enabled
+   * authentication providers.
+   * @param options Common provider options.
+   */
+  private setupHTTPAuthenticationProvider(options: AuthenticationProviderOptions) {
+    const supportedSchemes = new Set(
+      this.options.config.authc.http.schemes.map(scheme => scheme.toLowerCase())
+    );
+
+    // If `autoSchemesEnabled` is set we should allow schemes that other providers use to
+    // authenticate requests with Elasticsearch.
+    if (this.options.config.authc.http.autoSchemesEnabled) {
+      for (const provider of this.providers.values()) {
+        const supportedScheme = provider.getHTTPAuthenticationScheme();
+        if (supportedScheme) {
+          supportedSchemes.add(supportedScheme.toLowerCase());
+        }
+      }
+    }
+
+    this.providers.set(
+      HTTPAuthenticationProvider.type,
+      new HTTPAuthenticationProvider(options, { supportedSchemes })
+    );
+  }
+
   /**
    * Returns provider iterator where providers are sorted in the order of priority (based on the session ownership).
    * @param sessionValue Current session value.

x-pack/plugins/security/server/authentication/providers/base.mock.ts

@@ -23,6 +23,5 @@ export function mockAuthenticationProviderOptions() {
     logger: loggingServiceMock.create().get(),
     basePath,
     tokens: { refresh: jest.fn(), invalidate: jest.fn() },
-    isProviderEnabled: jest.fn(),
   };
 }

x-pack/plugins/security/server/authentication/providers/base.ts

@@ -25,7 +25,6 @@ export interface AuthenticationProviderOptions {
   client: IClusterClient;
   logger: Logger;
   tokens: PublicMethodsOf<Tokens>;
-  isProviderEnabled: (provider: string) => boolean;
 }
 
 /**
@@ -85,6 +84,13 @@ export abstract class BaseAuthenticationProvider {
    */
   abstract logout(request: KibanaRequest, state?: unknown): Promise<DeauthenticationResult>;
 
+  /**
+   * Returns HTTP authentication scheme that provider uses within `Authorization` HTTP header that
+   * it attaches to all successfully authenticated requests to Elasticsearch or `null` in case
+   * provider doesn't attach any additional `Authorization` HTTP headers.
+   */
+  abstract getHTTPAuthenticationScheme(): string | null;
+
   /**
    * Queries Elasticsearch `_authenticate` endpoint to authenticate request and retrieve the user
    * information of authenticated user.

x-pack/plugins/security/server/authentication/providers/basic.test.ts

@@ -188,4 +188,8 @@ describe('BasicAuthenticationProvider', () => {
       );
     });
   });
+
+  it('`getHTTPAuthenticationScheme` method', () => {
+    expect(provider.getHTTPAuthenticationScheme()).toBe('basic');
+  });
 });

x-pack/plugins/security/server/authentication/providers/basic.ts

@@ -110,6 +110,14 @@ export class BasicAuthenticationProvider extends BaseAuthenticationProvider {
     );
   }
 
+  /**
+   * Returns HTTP authentication scheme (`Bearer`) that's used within `Authorization` HTTP header
+   * that provider attaches to all successfully authenticated requests to Elasticsearch.
+   */
+  public getHTTPAuthenticationScheme() {
+    return 'basic';
+  }
+
   /**
    * Tries to extract authorization header from the state and adds it to the request before
    * it's forwarded to Elasticsearch backend.

x-pack/plugins/security/server/authentication/providers/http.test.ts

@@ -38,28 +38,21 @@ describe('HTTPAuthenticationProvider', () => {
   it('throws if `schemes` are not specified', () => {
     const providerOptions = mockAuthenticationProviderOptions();
 
-    expect(() => new HTTPAuthenticationProvider(providerOptions)).toThrowError(
+    expect(() => new HTTPAuthenticationProvider(providerOptions, undefined as any)).toThrowError(
       'Supported schemes should be specified'
     );
-    expect(() => new HTTPAuthenticationProvider(providerOptions, {})).toThrowError(
+    expect(() => new HTTPAuthenticationProvider(providerOptions, {} as any)).toThrowError(
       'Supported schemes should be specified'
     );
-    expect(() => new HTTPAuthenticationProvider(providerOptions, { schemes: [] })).toThrowError(
-      'Supported schemes should be specified'
-    );
-
     expect(
-      () =>
-        new HTTPAuthenticationProvider(providerOptions, { schemes: [], autoSchemesEnabled: false })
+      () => new HTTPAuthenticationProvider(providerOptions, { supportedSchemes: new Set() })
     ).toThrowError('Supported schemes should be specified');
   });
 
   describe('`login` method', () => {
     it('does not handle login', async () => {
       const provider = new HTTPAuthenticationProvider(mockOptions, {
-        enabled: true,
-        autoSchemesEnabled: true,
-        schemes: ['apikey'],
+        supportedSchemes: new Set(['apikey']),
       });
 
       await expect(provider.login()).resolves.toEqual(AuthenticationResult.notHandled());
@@ -70,95 +63,9 @@ describe('HTTPAuthenticationProvider', () => {
   });
 
   describe('`authenticate` method', () => {
-    const testCasesToNotHandle = [
-      {
-        autoSchemesEnabled: false,
-        isProviderEnabled: () => false,
-        schemes: ['basic'],
-        header: 'Bearer xxx',
-      },
-      {
-        autoSchemesEnabled: false,
-        isProviderEnabled: () => false,
-        schemes: ['bearer'],
-        header: 'Basic xxx',
-      },
-      {
-        autoSchemesEnabled: false,
-        isProviderEnabled: () => false,
-        schemes: ['basic', 'apikey'],
-        header: 'Bearer xxx',
-      },
-      {
-        autoSchemesEnabled: true,
-        isProviderEnabled: () => false,
-        schemes: ['basic', 'apikey'],
-        header: 'Bearer xxx',
-      },
-      {
-        autoSchemesEnabled: true,
-        isProviderEnabled: (provider: string) => provider === 'basic',
-        schemes: ['basic'],
-        header: 'Bearer xxx',
-      },
-      {
-        autoSchemesEnabled: true,
-        isProviderEnabled: () => true,
-        schemes: [],
-        header: 'ApiKey xxx',
-      },
-    ];
-
-    const testCasesToHandle = [
-      {
-        autoSchemesEnabled: false,
-        isProviderEnabled: () => false,
-        schemes: ['basic'],
-        header: 'Basic xxx',
-      },
-      {
-        autoSchemesEnabled: false,
-        isProviderEnabled: () => false,
-        schemes: ['bearer'],
-        header: 'Bearer xxx',
-      },
-      {
-        autoSchemesEnabled: false,
-        isProviderEnabled: () => false,
-        schemes: ['basic', 'apikey'],
-        header: 'ApiKey xxx',
-      },
-      {
-        autoSchemesEnabled: false,
-        isProviderEnabled: () => false,
-        schemes: ['some-weird-scheme'],
-        header: 'some-weird-scheme xxx',
-      },
-      ...['saml', 'oidc', 'pki', 'kerberos', 'token'].map(bearerProviderType => ({
-        autoSchemesEnabled: true,
-        isProviderEnabled: (providerType: string) => providerType === bearerProviderType,
-        schemes: ['apikey'],
-        header: 'Bearer xxx',
-      })),
-      {
-        autoSchemesEnabled: true,
-        isProviderEnabled: (provider: string) => provider === 'basic',
-        schemes: ['apikey'],
-        header: 'Basic xxx',
-      },
-      {
-        autoSchemesEnabled: true,
-        isProviderEnabled: () => true,
-        schemes: [],
-        header: 'Bearer xxx',
-      },
-    ];
-
     it('does not handle authentication for requests without `authorization` header.', async () => {
       const provider = new HTTPAuthenticationProvider(mockOptions, {
-        enabled: true,
-        autoSchemesEnabled: true,
-        schemes: ['apikey'],
+        supportedSchemes: new Set(['apikey']),
       });
 
       await expect(provider.authenticate(httpServerMock.createKibanaRequest())).resolves.toEqual(
@@ -171,9 +78,7 @@ describe('HTTPAuthenticationProvider', () => {
 
     it('does not handle authentication for requests with empty scheme in `authorization` header.', async () => {
       const provider = new HTTPAuthenticationProvider(mockOptions, {
-        enabled: true,
-        autoSchemesEnabled: true,
-        schemes: ['apikey'],
+        supportedSchemes: new Set(['apikey']),
       });
 
       await expect(
@@ -187,19 +92,16 @@ describe('HTTPAuthenticationProvider', () => {
     });
 
     it('does not handle authentication via `authorization` header if scheme is not supported.', async () => {
-      for (const {
-        isProviderEnabled,
-        autoSchemesEnabled,
-        schemes,
-        header,
-      } of testCasesToNotHandle) {
+      for (const { schemes, header } of [
+        { schemes: ['basic'], header: 'Bearer xxx' },
+        { schemes: ['bearer'], header: 'Basic xxx' },
+        { schemes: ['basic', 'apikey'], header: 'Bearer xxx' },
+        { schemes: ['basic', 'bearer'], header: 'ApiKey xxx' },
+      ]) {
         const request = httpServerMock.createKibanaRequest({ headers: { authorization: header } });
 
-        mockOptions.isProviderEnabled.mockImplementation(isProviderEnabled);
         const provider = new HTTPAuthenticationProvider(mockOptions, {
-          enabled: true,
-          autoSchemesEnabled,
-          schemes,
+          supportedSchemes: new Set(schemes),
         });
 
         await expect(provider.authenticate(request)).resolves.toEqual(
@@ -215,19 +117,22 @@ describe('HTTPAuthenticationProvider', () => {
 
     it('succeeds if authentication via `authorization` header with supported scheme succeeds.', async () => {
       const user = mockAuthenticatedUser();
-      for (const { isProviderEnabled, autoSchemesEnabled, schemes, header } of testCasesToHandle) {
+      for (const { schemes, header } of [
+        { schemes: ['basic'], header: 'Basic xxx' },
+        { schemes: ['bearer'], header: 'Bearer xxx' },
+        { schemes: ['basic', 'apikey'], header: 'ApiKey xxx' },
+        { schemes: ['some-weird-scheme'], header: 'some-weird-scheme xxx' },
+        { schemes: ['apikey', 'bearer'], header: 'Bearer xxx' },
+      ]) {
         const request = httpServerMock.createKibanaRequest({ headers: { authorization: header } });
 
         const mockScopedClusterClient = elasticsearchServiceMock.createScopedClusterClient();
         mockScopedClusterClient.callAsCurrentUser.mockResolvedValue(user);
         mockOptions.client.asScoped.mockReturnValue(mockScopedClusterClient);
         mockOptions.client.asScoped.mockClear();
 
-        mockOptions.isProviderEnabled.mockImplementation(isProviderEnabled);
         const provider = new HTTPAuthenticationProvider(mockOptions, {
-          enabled: true,
-          autoSchemesEnabled,
-          schemes,
+          supportedSchemes: new Set(schemes),
         });
 
         await expect(provider.authenticate(request)).resolves.toEqual(
@@ -242,19 +147,22 @@ describe('HTTPAuthenticationProvider', () => {
 
     it('fails if authentication via `authorization` header with supported scheme fails.', async () => {
       const failureReason = ElasticsearchErrorHelpers.decorateNotAuthorizedError(new Error());
-      for (const { isProviderEnabled, autoSchemesEnabled, schemes, header } of testCasesToHandle) {
+      for (const { schemes, header } of [
+        { schemes: ['basic'], header: 'Basic xxx' },
+        { schemes: ['bearer'], header: 'Bearer xxx' },
+        { schemes: ['basic', 'apikey'], header: 'ApiKey xxx' },
+        { schemes: ['some-weird-scheme'], header: 'some-weird-scheme xxx' },
+        { schemes: ['apikey', 'bearer'], header: 'Bearer xxx' },
+      ]) {
         const request = httpServerMock.createKibanaRequest({ headers: { authorization: header } });
 
         const mockScopedClusterClient = elasticsearchServiceMock.createScopedClusterClient();
         mockScopedClusterClient.callAsCurrentUser.mockRejectedValue(failureReason);
         mockOptions.client.asScoped.mockReturnValue(mockScopedClusterClient);
         mockOptions.client.asScoped.mockClear();
 
-        mockOptions.isProviderEnabled.mockImplementation(isProviderEnabled);
         const provider = new HTTPAuthenticationProvider(mockOptions, {
-          enabled: true,
-          autoSchemesEnabled,
-          schemes,
+          supportedSchemes: new Set(schemes),
         });
 
         await expect(provider.authenticate(request)).resolves.toEqual(
@@ -271,9 +179,7 @@ describe('HTTPAuthenticationProvider', () => {
   describe('`logout` method', () => {
     it('does not handle logout', async () => {
       const provider = new HTTPAuthenticationProvider(mockOptions, {
-        enabled: true,
-        autoSchemesEnabled: true,
-        schemes: ['apikey'],
+        supportedSchemes: new Set(['apikey']),
       });
 
       await expect(provider.logout()).resolves.toEqual(DeauthenticationResult.notHandled());
@@ -282,4 +188,11 @@ describe('HTTPAuthenticationProvider', () => {
       expect(mockOptions.client.callAsInternalUser).not.toHaveBeenCalled();
     });
   });
+
+  it('`getHTTPAuthenticationScheme` method', () => {
+    const provider = new HTTPAuthenticationProvider(mockOptions, {
+      supportedSchemes: new Set(['apikey']),
+    });
+    expect(provider.getHTTPAuthenticationScheme()).toBeNull();
+  });
 });