From 924749ca2270d76f5c1fbd09c604c45f58160ab1 Mon Sep 17 00:00:00 2001 From: Aleh Zasypkin Date: Tue, 1 Dec 2020 11:25:14 +0100 Subject: [PATCH] Review#1: add more helpful logs for the fail cases, use dedicated credentials type to be more explicit. --- .../providers/anonymous.test.ts | 1 + .../authentication/providers/anonymous.ts | 84 ++++++++++++------- x-pack/plugins/security/server/config.test.ts | 80 ++++++++++++------ x-pack/plugins/security/server/config.ts | 27 +++--- .../anonymous_es_anonymous.config.ts | 2 +- 5 files changed, 124 insertions(+), 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 index f1edf2c9a8290a..9674181e187501 100644 --- 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 index 44770d42227afa..6d62d3a909e559 100644 --- 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, anonymousOptions?: Readonly<{ - credentials?: Readonly; + 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 index f4f578e5ad905d..f41e721db33a2e 100644 --- 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 index bd0c654eac31f6..cf9ab3a78ab716 100644 --- 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 index c19b3e6024a708..3fc30c922b742a 100644 --- 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 } }, })}`, ],