Review#1: add more helpful logs for the fail cases, use dedicated cre…

…dentials type to be more explicit.
elastic · Dec 1, 2020 · 924749c · 924749c
1 parent 3fbc785
commit 924749c
Show file tree

Hide file tree

Showing 5 changed files with 124 additions and 70 deletions.
diff --git a/x-pack/plugins/security/server/authentication/providers/anonymous.test.ts b/x-pack/plugins/security/server/authentication/providers/anonymous.test.ts
@@ -66,6 +66,7 @@ describe('AnonymousAuthenticationProvider', () => {
             authorization = new HTTPAuthorizationHeader('ApiKey', 'some-apiKey').toString();
             break;
           default:
+            credentials = 'elasticsearch_anonymous_user' as 'elasticsearch_anonymous_user';
             break;
         }
 

diff --git a/x-pack/plugins/security/server/authentication/providers/anonymous.ts b/x-pack/plugins/security/server/authentication/providers/anonymous.ts
@@ -4,7 +4,7 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 
-import { KibanaRequest } from '../../../../../../src/core/server';
+import { KibanaRequest, LegacyElasticsearchErrorHelpers } from '../../../../../../src/core/server';
 import { AuthenticationResult } from '../authentication_result';
 import { canRedirectRequest } from '../can_redirect_request';
 import { DeauthenticationResult } from '../deauthentication_result';
@@ -29,6 +29,11 @@ interface APIKeyCredentials {
   apiKey: { id: string; key: string } | string;
 }
 
+/**
+ * Credentials that imply authentication based on the Elasticsearch native anonymous user.
+ */
+type ElasticsearchAnonymousUserCredentials = 'elasticsearch_anonymous_user';
+
 /**
  * Checks whether current request can initiate a new session.
  * @param request Request instance.
@@ -44,7 +49,10 @@ function canStartNewSession(request: KibanaRequest) {
  * @param credentials
  */
 function isAPIKeyCredentials(
-  credentials: UsernameAndPasswordCredentials | APIKeyCredentials
+  credentials:
+    | ElasticsearchAnonymousUserCredentials
+    | APIKeyCredentials
+    | UsernameAndPasswordCredentials
 ): credentials is APIKeyCredentials {
   return !!(credentials as APIKeyCredentials).apiKey;
 }
@@ -67,37 +75,41 @@ export class AnonymousAuthenticationProvider extends BaseAuthenticationProvider
   constructor(
     protected readonly options: Readonly<AuthenticationProviderOptions>,
     anonymousOptions?: Readonly<{
-      credentials?: Readonly<UsernameAndPasswordCredentials | APIKeyCredentials>;
+      credentials?: Readonly<
+        ElasticsearchAnonymousUserCredentials | UsernameAndPasswordCredentials | APIKeyCredentials
+      >;
     }>
   ) {
     super(options);
 
     const credentials = anonymousOptions?.credentials;
-    if (credentials) {
-      if (isAPIKeyCredentials(credentials)) {
-        this.logger.debug('Anonymous requests will be authenticated via API key.');
-        this.httpAuthorizationHeader = new HTTPAuthorizationHeader(
-          'ApiKey',
-          typeof credentials.apiKey === 'string'
-            ? credentials.apiKey
-            : new BasicHTTPAuthorizationHeaderCredentials(
-                credentials.apiKey.id,
-                credentials.apiKey.key
-              ).toString()
-        );
-      } else {
-        this.logger.debug('Anonymous requests will be authenticated via username and password.');
-        this.httpAuthorizationHeader = new HTTPAuthorizationHeader(
-          'Basic',
-          new BasicHTTPAuthorizationHeaderCredentials(
-            credentials.username,
-            credentials.password
-          ).toString()
-        );
-      }
-    } else {
+    if (!credentials) {
+      throw new Error('Credentials must be specified');
+    }
+
+    if (credentials === 'elasticsearch_anonymous_user') {
       this.logger.debug(
-        'Anonymous requests will be authenticated using Elasticsearch native anonymous access.'
+        'Anonymous requests will be authenticated using Elasticsearch native anonymous user.'
+      );
+    } else if (isAPIKeyCredentials(credentials)) {
+      this.logger.debug('Anonymous requests will be authenticated via API key.');
+      this.httpAuthorizationHeader = new HTTPAuthorizationHeader(
+        'ApiKey',
+        typeof credentials.apiKey === 'string'
+          ? credentials.apiKey
+          : new BasicHTTPAuthorizationHeaderCredentials(
+              credentials.apiKey.id,
+              credentials.apiKey.key
+            ).toString()
+      );
+    } else {
+      this.logger.debug('Anonymous requests will be authenticated via username and password.');
+      this.httpAuthorizationHeader = new HTTPAuthorizationHeader(
+        'Basic',
+        new BasicHTTPAuthorizationHeaderCredentials(
+          credentials.username,
+          credentials.password
+        ).toString()
       );
     }
   }
@@ -178,7 +190,23 @@ export class AnonymousAuthenticationProvider extends BaseAuthenticationProvider
       // Create session only if it doesn't exist yet, otherwise keep it unchanged.
       return AuthenticationResult.succeeded(user, { authHeaders, state: state ? undefined : {} });
     } catch (err) {
-      this.logger.debug(`Failed to authenticate request : ${err.message}`);
+      if (LegacyElasticsearchErrorHelpers.isNotAuthorizedError(err)) {
+        if (!this.httpAuthorizationHeader) {
+          this.logger.error(
+            `Failed to authenticate anonymous request using Elasticsearch reserved anonymous user. Anonymous access may not be properly configured in Elasticsearch: ${err.message}`
+          );
+        } else if (this.httpAuthorizationHeader.scheme.toLowerCase() === 'basic') {
+          this.logger.error(
+            `Failed to authenticate anonymous request using provided username/password credentials. The user with the provided username may not exist or the password is wrong: ${err.message}`
+          );
+        } else {
+          this.logger.error(
+            `Failed to authenticate anonymous request using provided API key. The key may not exist or expired: ${err.message}`
+          );
+        }
+      } else {
+        this.logger.error(`Failed to authenticate request : ${err.message}`);
+      }
       return AuthenticationResult.failed(err);
     }
   }

diff --git a/x-pack/plugins/security/server/config.test.ts b/x-pack/plugins/security/server/config.test.ts
@@ -877,27 +877,15 @@ describe('config schema', () => {
         );
       });
 
-      it('does not require `credentials`', () => {
-        expect(
+      it('requires `credentials`', () => {
+        expect(() =>
           ConfigSchema.validate({
             authc: { providers: { anonymous: { anonymous1: { order: 0 } } } },
-          }).authc.providers
-        ).toMatchInlineSnapshot(`
-          Object {
-            "anonymous": Object {
-              "anonymous1": Object {
-                "description": "Continue as Guest",
-                "enabled": true,
-                "hint": "For anonymous users",
-                "icon": "globe",
-                "order": 0,
-                "session": Object {
-                  "idleTimeout": null,
-                },
-                "showInSelector": true,
-              },
-            },
-          }
+          })
+        ).toThrowErrorMatchingInlineSnapshot(`
+          "[authc.providers]: types that failed validation:
+          - [authc.providers.0]: expected value of type [array] but got [Object]
+          - [authc.providers.1.anonymous.anonymous1.credentials]: expected at least one defined value but got [undefined]"
         `);
       });
 
@@ -914,8 +902,9 @@ describe('config schema', () => {
           "[authc.providers]: types that failed validation:
           - [authc.providers.0]: expected value of type [array] but got [Object]
           - [authc.providers.1.anonymous.anonymous1.credentials]: types that failed validation:
-           - [credentials.0.password]: expected value of type [string] but got [undefined]
-           - [credentials.1.apiKey]: expected at least one defined value but got [undefined]"
+           - [credentials.0]: expected value to equal [elasticsearch_anonymous_user]
+           - [credentials.1.password]: expected value of type [string] but got [undefined]
+           - [credentials.2.apiKey]: expected at least one defined value but got [undefined]"
         `);
 
         expect(() =>
@@ -930,8 +919,9 @@ describe('config schema', () => {
           "[authc.providers]: types that failed validation:
           - [authc.providers.0]: expected value of type [array] but got [Object]
           - [authc.providers.1.anonymous.anonymous1.credentials]: types that failed validation:
-           - [credentials.0.username]: expected value of type [string] but got [undefined]
-           - [credentials.1.apiKey]: expected at least one defined value but got [undefined]"
+           - [credentials.0]: expected value to equal [elasticsearch_anonymous_user]
+           - [credentials.1.username]: expected value of type [string] but got [undefined]
+           - [credentials.2.apiKey]: expected at least one defined value but got [undefined]"
         `);
       });
 
@@ -985,8 +975,9 @@ describe('config schema', () => {
           "[authc.providers]: types that failed validation:
           - [authc.providers.0]: expected value of type [array] but got [Object]
           - [authc.providers.1.anonymous.anonymous1.credentials]: types that failed validation:
-           - [credentials.0.username]: expected value of type [string] but got [undefined]
-           - [credentials.1.apiKey]: types that failed validation:
+           - [credentials.0]: expected value to equal [elasticsearch_anonymous_user]
+           - [credentials.1.username]: expected value of type [string] but got [undefined]
+           - [credentials.2.apiKey]: types that failed validation:
             - [credentials.apiKey.0.key]: expected value of type [string] but got [undefined]
             - [credentials.apiKey.1]: expected value of type [string] but got [Object]"
         `);
@@ -1005,8 +996,9 @@ describe('config schema', () => {
           "[authc.providers]: types that failed validation:
           - [authc.providers.0]: expected value of type [array] but got [Object]
           - [authc.providers.1.anonymous.anonymous1.credentials]: types that failed validation:
-           - [credentials.0.username]: expected value of type [string] but got [undefined]
-           - [credentials.1.apiKey]: types that failed validation:
+           - [credentials.0]: expected value to equal [elasticsearch_anonymous_user]
+           - [credentials.1.username]: expected value of type [string] but got [undefined]
+           - [credentials.2.apiKey]: types that failed validation:
             - [credentials.apiKey.0.id]: expected value of type [string] but got [undefined]
             - [credentials.apiKey.1]: expected value of type [string] but got [Object]"
         `);
@@ -1085,6 +1077,40 @@ describe('config schema', () => {
         `);
       });
 
+      it('can be successfully validated with `elasticsearch_anonymous_user` credentials', () => {
+        expect(
+          ConfigSchema.validate({
+            authc: {
+              providers: {
+                anonymous: {
+                  anonymous1: {
+                    order: 0,
+                    credentials: 'elasticsearch_anonymous_user',
+                  },
+                },
+              },
+            },
+          }).authc.providers
+        ).toMatchInlineSnapshot(`
+          Object {
+            "anonymous": Object {
+              "anonymous1": Object {
+                "credentials": "elasticsearch_anonymous_user",
+                "description": "Continue as Guest",
+                "enabled": true,
+                "hint": "For anonymous users",
+                "icon": "globe",
+                "order": 0,
+                "session": Object {
+                  "idleTimeout": null,
+                },
+                "showInSelector": true,
+              },
+            },
+          }
+        `);
+      });
+
       it('can be successfully validated with session config overrides', () => {
         expect(
           ConfigSchema.validate({

diff --git a/x-pack/plugins/security/server/config.ts b/x-pack/plugins/security/server/config.ts
@@ -149,20 +149,19 @@ const providersConfigSchema = schema.object(
         }),
       },
       {
-        credentials: schema.maybe(
-          schema.oneOf([
-            schema.object({
-              username: schema.string(),
-              password: schema.string(),
-            }),
-            schema.object({
-              apiKey: schema.oneOf([
-                schema.object({ id: schema.string(), key: schema.string() }),
-                schema.string(),
-              ]),
-            }),
-          ])
-        ),
+        credentials: schema.oneOf([
+          schema.literal('elasticsearch_anonymous_user'),
+          schema.object({
+            username: schema.string(),
+            password: schema.string(),
+          }),
+          schema.object({
+            apiKey: schema.oneOf([
+              schema.object({ id: schema.string(), key: schema.string() }),
+              schema.string(),
+            ]),
+          }),
+        ]),
       }
     ),
   },

diff --git a/x-pack/test/security_api_integration/anonymous_es_anonymous.config.ts b/x-pack/test/security_api_integration/anonymous_es_anonymous.config.ts
@@ -31,7 +31,7 @@ export default async function ({ readConfigFile }: FtrConfigProviderContext) {
           .get('kbnTestServer.serverArgs')
           .filter((arg: string) => !arg.startsWith('--xpack.security.authc.providers')),
         `--xpack.security.authc.providers=${JSON.stringify({
-          anonymous: { anonymous1: { order: 0 } },
+          anonymous: { anonymous1: { order: 0, credentials: 'elasticsearch_anonymous_user' } },
           basic: { basic1: { order: 1 } },
         })}`,
       ],