Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deprecate kibana user in favor of kibana_system user #63186

Merged
merged 12 commits into from
May 5, 2020
Merged
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
4 changes: 2 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,7 @@ 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.`
);
}
legrego marked this conversation as resolved.
Show resolved Hide resolved
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 specific a password for a given native realm account, you can do that like so: `--password.kibana=notsecure`

legrego marked this conversation as resolved.
Show resolved Hide resolved
# 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
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
2 changes: 1 addition & 1 deletion x-pack/plugins/monitoring/server/deprecations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ 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.`
);
}
legrego marked this conversation as resolved.
Show resolved Hide resolved
}
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 @@ -35,6 +35,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 @@ -241,7 +242,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 @@ -254,9 +255,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 @@ -369,7 +370,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 @@ -424,15 +425,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"`);
});
});
14 changes: 14 additions & 0 deletions x-pack/plugins/security/public/management/users/user_utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,20 @@
* you may not use this file except in compliance with the Elastic License.
*/

import { i18n } from '@kbn/i18n';
import { User } from '../../../common/model';

export const isUserReserved = (user: User) => user.metadata?._reserved ?? false;

export const isUserDeprecated = (user: User) => user.metadata?._deprecated ?? false;

export const getExtendedUserDeprecationNotice = (user: User) => {
const reason = user.metadata?._deprecated_reason ?? '';
return i18n.translate('xpack.security.management.users.extendedUserDeprecationNotice', {
defaultMessage: `The {username} user is deprecated. {reason}`,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps this should change so it will be consistent with the reason?

Suggested change
defaultMessage: `The {username} user is deprecated. {reason}`,
defaultMessage: `The [{username}] user is deprecated. {reason}`,

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The current text is consistent with the deprecation message we display for roles today. Gail had actually requested that we remove the brackets from the reason when we did the role deprecations, but then that change would have been inconsistent with the rest of the messages in ES. Overall, I'm 🤷‍♂️ about it, I could go either way.

Given my beautiful backstory, which would you prefer? I'll defer to you 😄

values: {
username: user.username,
reason,
},
});
};
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,38 @@ describe('UsersGridPage', () => {
expect(findTestSubject(wrapper, 'userDisabled')).toHaveLength(1);
});

it('renders deprecated users', async () => {
const apiClientMock = userAPIClientMock.create();
apiClientMock.getUsers.mockImplementation(() => {
return Promise.resolve<User[]>([
{
username: 'foo',
email: '[email protected]',
full_name: 'foo bar',
roles: ['kibana_user'],
enabled: true,
metadata: {
_reserved: true,
_deprecated: true,
_deprecated_reason: 'This user is not cool anymore.',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😄

},
},
]);
});

const wrapper = mountWithIntl(
<UsersGridPage
userAPIClient={apiClientMock}
rolesAPIClient={rolesAPIClientMock.create()}
notifications={coreMock.createStart().notifications}
/>
);

await waitForRender(wrapper);

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

it('renders a warning when a user is assigned a deprecated role', async () => {
const apiClientMock = userAPIClientMock.create();
apiClientMock.getUsers.mockImplementation(() => {
Expand Down
Loading