Skip to content

Commit

Permalink
Deprecate kibana user in favor of kibana_system user (#63186)
Browse files Browse the repository at this point in the history
  • Loading branch information
legrego authored May 5, 2020
1 parent add56be commit 6a6deef
Show file tree
Hide file tree
Showing 23 changed files with 229 additions and 40 deletions.
2 changes: 1 addition & 1 deletion config/kibana.yml
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@
# the username and password that the Kibana server uses to perform maintenance on the Kibana
# index at startup. Your Kibana users still need to authenticate with Elasticsearch, which
# is proxied through the Kibana server.
#elasticsearch.username: "kibana"
#elasticsearch.username: "kibana_system"
#elasticsearch.password: "pass"

# Enables SSL and paths to the PEM-format SSL certificate and SSL key files, respectively.
Expand Down
4 changes: 2 additions & 2 deletions docs/user/security/securing-kibana.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -31,14 +31,14 @@ file:

[source,yaml]
-----------------------------------------------
elasticsearch.username: "kibana"
elasticsearch.username: "kibana_system"
elasticsearch.password: "kibanapassword"
-----------------------------------------------

The {kib} server submits requests as this user to access the cluster monitoring
APIs and the `.kibana` index. The server does _not_ need access to user indices.

The password for the built-in `kibana` user is typically set as part of the
The password for the built-in `kibana_system` user is typically set as part of the
{security} configuration process on {es}. For more information, see
{ref}/built-in-users.html[Built-in users].
--
Expand Down
16 changes: 8 additions & 8 deletions packages/kbn-es/src/utils/native_realm.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ describe('setPasswords', () => {

mockClient.security.getUser.mockImplementation(() => ({
body: {
kibana: {
kibana_system: {
metadata: {
_reserved: true,
},
Expand Down Expand Up @@ -138,7 +138,7 @@ describe('setPasswords', () => {
}));

await nativeRealm.setPasswords({
'password.kibana': 'bar',
'password.kibana_system': 'bar',
});

expect(mockClient.security.changePassword.mock.calls).toMatchInlineSnapshot(`
Expand All @@ -149,7 +149,7 @@ Array [
"password": "bar",
},
"refresh": "wait_for",
"username": "kibana",
"username": "kibana_system",
},
],
Array [
Expand Down Expand Up @@ -188,7 +188,7 @@ describe('getReservedUsers', () => {
it('returns array of reserved usernames', async () => {
mockClient.security.getUser.mockImplementation(() => ({
body: {
kibana: {
kibana_system: {
metadata: {
_reserved: true,
},
Expand All @@ -206,17 +206,17 @@ describe('getReservedUsers', () => {
},
}));

expect(await nativeRealm.getReservedUsers()).toEqual(['kibana', 'logstash_system']);
expect(await nativeRealm.getReservedUsers()).toEqual(['kibana_system', 'logstash_system']);
});
});

describe('setPassword', () => {
it('sets password for provided user', async () => {
await nativeRealm.setPassword('kibana', 'foo');
await nativeRealm.setPassword('kibana_system', 'foo');
expect(mockClient.security.changePassword).toHaveBeenCalledWith({
body: { password: 'foo' },
refresh: 'wait_for',
username: 'kibana',
username: 'kibana_system',
});
});

Expand All @@ -226,7 +226,7 @@ describe('setPassword', () => {
});

await expect(
nativeRealm.setPassword('kibana', 'foo')
nativeRealm.setPassword('kibana_system', 'foo')
).rejects.toThrowErrorMatchingInlineSnapshot(`"SomeError"`);
});
});
2 changes: 1 addition & 1 deletion src/cli/serve/serve.js
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ function applyConfigOverrides(rawConfig, opts, extraCliOptions) {
set('optimize.watch', true);

if (!has('elasticsearch.username')) {
set('elasticsearch.username', 'kibana');
set('elasticsearch.username', 'kibana_system');
}

if (!has('elasticsearch.password')) {
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

13 changes: 11 additions & 2 deletions src/core/server/elasticsearch/elasticsearch_config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -315,12 +315,21 @@ describe('deprecations', () => {
const { messages } = applyElasticsearchDeprecations({ username: 'elastic' });
expect(messages).toMatchInlineSnapshot(`
Array [
"Setting [${CONFIG_PATH}.username] to \\"elastic\\" is deprecated. You should use the \\"kibana\\" user instead.",
"Setting [${CONFIG_PATH}.username] to \\"elastic\\" is deprecated. You should use the \\"kibana_system\\" user instead.",
]
`);
});

it('does not log a warning if elasticsearch.username is set to something besides "elastic"', () => {
it('logs a warning if elasticsearch.username is set to "kibana"', () => {
const { messages } = applyElasticsearchDeprecations({ username: 'kibana' });
expect(messages).toMatchInlineSnapshot(`
Array [
"Setting [${CONFIG_PATH}.username] to \\"kibana\\" is deprecated. You should use the \\"kibana_system\\" user instead.",
]
`);
});

it('does not log a warning if elasticsearch.username is set to something besides "elastic" or "kibana"', () => {
const { messages } = applyElasticsearchDeprecations({ username: 'otheruser' });
expect(messages).toHaveLength(0);
});
Expand Down
8 changes: 6 additions & 2 deletions src/core/server/elasticsearch/elasticsearch_config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ export const configSchema = schema.object({
if (rawConfig === 'elastic') {
return (
'value of "elastic" is forbidden. This is a superuser account that can obfuscate ' +
'privilege-related issues. You should use the "kibana" user instead.'
'privilege-related issues. You should use the "kibana_system" user instead.'
);
}
},
Expand Down Expand Up @@ -131,7 +131,11 @@ const deprecations: ConfigDeprecationProvider = () => [
}
if (es.username === 'elastic') {
log(
`Setting [${fromPath}.username] to "elastic" is deprecated. You should use the "kibana" user instead.`
`Setting [${fromPath}.username] to "elastic" is deprecated. You should use the "kibana_system" user instead.`
);
} else if (es.username === 'kibana') {
log(
`Setting [${fromPath}.username] to "kibana" is deprecated. You should use the "kibana_system" user instead.`
);
}
if (es.ssl?.key !== undefined && es.ssl?.certificate === undefined) {
Expand Down
2 changes: 1 addition & 1 deletion x-pack/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ Elasticsearch will run with a basic license. To run with a trial license, includ

Example: `yarn es snapshot --license trial --password changeme`

By default, this will also set the password for native realm accounts to the password provided (`changeme` by default). This includes that of the `kibana` user which `elasticsearch.username` defaults to in development. If you wish to specific a password for a given native realm account, you can do that like so: `--password.kibana=notsecure`
By default, this will also set the password for native realm accounts to the password provided (`changeme` by default). This includes that of the `kibana_system` user which `elasticsearch.username` defaults to in development. If you wish to specify a password for a given native realm account, you can do that like so: `--password.kibana_system=notsecure`

# Testing
## Running specific tests
Expand Down
2 changes: 1 addition & 1 deletion x-pack/legacy/plugins/monitoring/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ cluster.
% cat config/kibana.dev.yml
monitoring.ui.elasticsearch:
hosts: "http://localhost:9210"
username: "kibana"
username: "kibana_system"
password: "changeme"
```

Expand Down
10 changes: 9 additions & 1 deletion x-pack/plugins/monitoring/server/__tests__/deprecations.js
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,15 @@ describe('monitoring plugin deprecations', function() {
expect(log.called).to.be(true);
});

it('does not log a warning if elasticsearch.username is set to something besides "elastic"', () => {
it('logs a warning if elasticsearch.username is set to "kibana"', () => {
const settings = { elasticsearch: { username: 'kibana' } };

const log = sinon.spy();
transformDeprecations(settings, fromPath, log);
expect(log.called).to.be(true);
});

it('does not log a warning if elasticsearch.username is set to something besides "elastic" or "kibana"', () => {
const settings = { elasticsearch: { username: 'otheruser' } };

const log = sinon.spy();
Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugins/monitoring/server/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ export const configSchema = schema.object({
if (rawConfig === 'elastic') {
return (
'value of "elastic" is forbidden. This is a superuser account that can obfuscate ' +
'privilege-related issues. You should use the "kibana" user instead.'
'privilege-related issues. You should use the "kibana_system" user instead.'
);
}
},
Expand Down
6 changes: 5 additions & 1 deletion x-pack/plugins/monitoring/server/deprecations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,11 @@ export const deprecations = ({
if (es) {
if (es.username === 'elastic') {
logger(
`Setting [${fromPath}.username] to "elastic" is deprecated. You should use the "kibana" user instead.`
`Setting [${fromPath}.username] to "elastic" is deprecated. You should use the "kibana_system" user instead.`
);
} else if (es.username === 'kibana') {
logger(
`Setting [${fromPath}.username] to "kibana" is deprecated. You should use the "kibana_system" user instead.`
);
}
}
Expand Down
2 changes: 2 additions & 0 deletions x-pack/plugins/security/common/model/user.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ export interface User {
enabled: boolean;
metadata?: {
_reserved: boolean;
_deprecated?: boolean;
_deprecated_reason?: string;
};
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,14 @@ const createUser = (username: string, roles = ['idk', 'something']) => {
};
}

if (username === 'deprecated_user') {
user.metadata = {
_reserved: true,
_deprecated: true,
_deprecated_reason: 'beacuse I said so.',
};
}

return user;
};

Expand Down Expand Up @@ -162,6 +170,28 @@ describe('EditUserPage', () => {
expectSaveButton(wrapper);
});

it('warns when viewing a depreciated user', async () => {
const user = createUser('deprecated_user');
const { apiClient, rolesAPIClient } = buildClients(user);
const securitySetup = buildSecuritySetup();

const wrapper = mountWithIntl(
<EditUserPage
username={user.username}
userAPIClient={apiClient}
rolesAPIClient={rolesAPIClient}
authc={securitySetup.authc}
notifications={coreMock.createStart().notifications}
/>
);

await waitForRender(wrapper);
expect(apiClient.getUser).toBeCalledTimes(1);
expect(securitySetup.authc.getCurrentUser).toBeCalledTimes(1);

expect(findTestSubject(wrapper, 'deprecatedUserWarning')).toHaveLength(1);
});

it('warns when user is assigned a deprecated role', async () => {
const user = createUser('existing_user', ['deprecated-role']);
const { apiClient, rolesAPIClient } = buildClients(user);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ import { RolesAPIClient } from '../../roles';
import { ConfirmDeleteUsers, ChangePasswordForm } from '../components';
import { UserValidator, UserValidationResult } from './validate_user';
import { RoleComboBox } from '../../role_combo_box';
import { isUserDeprecated, getExtendedUserDeprecationNotice, isUserReserved } from '../user_utils';
import { UserAPIClient } from '..';

interface Props {
Expand Down Expand Up @@ -244,7 +245,7 @@ export class EditUserPage extends Component<Props, State> {
return (
<Fragment>
<EuiHorizontalRule />
{user.username === 'kibana' ? (
{user.username === 'kibana' || user.username === 'kibana_system' ? (
<Fragment>
<EuiCallOut
title={i18n.translate(
Expand All @@ -257,9 +258,9 @@ export class EditUserPage extends Component<Props, State> {
<p>
<FormattedMessage
id="xpack.security.management.users.editUser.changePasswordUpdateKibanaTitle"
defaultMessage="After you change the password for the kibana user, you must update the {kibana}
defaultMessage="After you change the password for the {username} user, you must update the {kibana}
file and restart Kibana."
values={{ kibana: 'kibana.yml' }}
values={{ kibana: 'kibana.yml', username: user.username }}
/>
</p>
</EuiCallOut>
Expand Down Expand Up @@ -372,7 +373,7 @@ export class EditUserPage extends Component<Props, State> {
isNewUser,
showDeleteConfirmation,
} = this.state;
const reserved = user.metadata && user.metadata._reserved;
const reserved = isUserReserved(user);
if (!user || !roles) {
return null;
}
Expand Down Expand Up @@ -427,15 +428,31 @@ export class EditUserPage extends Component<Props, State> {
</EuiPageContentHeader>
<EuiPageContentBody>
{reserved && (
<EuiText size="s" color="subdued">
<p>
<FormattedMessage
id="xpack.security.management.users.editUser.modifyingReservedUsersDescription"
defaultMessage="Reserved users are built-in and cannot be removed or modified. Only the password
<Fragment>
<EuiText size="s" color="subdued">
<p>
<FormattedMessage
id="xpack.security.management.users.editUser.modifyingReservedUsersDescription"
defaultMessage="Reserved users are built-in and cannot be removed or modified. Only the password
may be changed."
/>
</p>
</EuiText>
/>
</p>
</EuiText>
<EuiSpacer size="s" />
</Fragment>
)}

{isUserDeprecated(user) && (
<Fragment>
<EuiCallOut
data-test-subj="deprecatedUserWarning"
title={getExtendedUserDeprecationNotice(user)}
color="warning"
iconType="alert"
size="s"
/>
<EuiSpacer size="s" />
</Fragment>
)}

{showDeleteConfirmation ? (
Expand Down
53 changes: 53 additions & 0 deletions x-pack/plugins/security/public/management/users/user_utils.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

import { User } from '../../../common/model';
import { isUserReserved, isUserDeprecated, getExtendedUserDeprecationNotice } from './user_utils';

describe('#isUserReserved', () => {
it('returns false for a user with no metadata', () => {
expect(isUserReserved({} as User)).toEqual(false);
});

it('returns false for a user with the reserved flag set to false', () => {
expect(isUserReserved({ metadata: { _reserved: false } } as User)).toEqual(false);
});

it('returns true for a user with the reserved flag set to true', () => {
expect(isUserReserved({ metadata: { _reserved: true } } as User)).toEqual(true);
});
});

describe('#isUserDeprecated', () => {
it('returns false for a user with no metadata', () => {
expect(isUserDeprecated({} as User)).toEqual(false);
});

it('returns false for a user with the deprecated flag set to false', () => {
expect(isUserDeprecated({ metadata: { _deprecated: false } } as User)).toEqual(false);
});

it('returns true for a user with the deprecated flag set to true', () => {
expect(isUserDeprecated({ metadata: { _deprecated: true } } as User)).toEqual(true);
});
});

describe('#getExtendedUserDeprecationNotice', () => {
it('returns a notice when no reason is provided', () => {
expect(
getExtendedUserDeprecationNotice({ username: 'test_user' } as User)
).toMatchInlineSnapshot(`"The test_user user is deprecated. "`);
});

it('returns a notice augmented with reason when provided', () => {
expect(
getExtendedUserDeprecationNotice({
username: 'test_user',
metadata: { _reserved: true, _deprecated_reason: 'some reason' },
} as User)
).toMatchInlineSnapshot(`"The test_user user is deprecated. some reason"`);
});
});
Loading

0 comments on commit 6a6deef

Please sign in to comment.