Skip to content

Commit

Permalink
Expose ability to check if API Keys are enabled (#63454)
Browse files Browse the repository at this point in the history
* expose ability to check if API Keys are enabled

* fix mock

* Fix typo in test name

* simplify key check

* fix privilege check

* remove unused variable

* address PR feedback

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
  • Loading branch information
legrego and elasticmachine committed Apr 23, 2020
1 parent a53d533 commit 44f9cbc
Show file tree
Hide file tree
Showing 22 changed files with 479 additions and 87 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,11 @@ export interface AuthenticationServiceSetup {
* Returns currently authenticated user and throws if current user isn't authenticated.
*/
getCurrentUser: () => Promise<AuthenticatedUser>;

/**
* Determines if API Keys are currently enabled.
*/
areAPIKeysEnabled: () => Promise<boolean>;
}

export class AuthenticationService {
Expand All @@ -37,11 +42,15 @@ export class AuthenticationService {
const getCurrentUser = async () =>
(await http.get('/internal/security/me', { asSystemRequest: true })) as AuthenticatedUser;

const areAPIKeysEnabled = async () =>
((await http.get('/internal/security/api_key/_enabled')) as { apiKeysEnabled: boolean })
.apiKeysEnabled;

loginApp.create({ application, config, getStartServices, http });
logoutApp.create({ application, http });
loggedOutApp.create({ application, getStartServices, http });
overwrittenSessionApp.create({ application, authc: { getCurrentUser }, getStartServices });

return { getCurrentUser };
return { getCurrentUser, areAPIKeysEnabled };
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,6 @@ import { AuthenticationServiceSetup } from './authentication_service';
export const authenticationMock = {
createSetup: (): jest.Mocked<AuthenticationServiceSetup> => ({
getCurrentUser: jest.fn(),
areAPIKeysEnabled: jest.fn(),
}),
};
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { AuthenticationServiceSetup } from '../authentication_service';

interface CreateDeps {
application: ApplicationSetup;
authc: AuthenticationServiceSetup;
authc: Pick<AuthenticationServiceSetup, 'getCurrentUser'>;
getStartServices: StartServicesAccessor;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import { AuthenticationStatePage } from '../components';

interface Props {
basePath: IBasePath;
authc: AuthenticationServiceSetup;
authc: Pick<AuthenticationServiceSetup, 'getCurrentUser'>;
}

export function OverwrittenSessionPage({ authc, basePath }: Props) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { ApiKey, ApiKeyToInvalidate } from '../../../common/model';
interface CheckPrivilegesResponse {
areApiKeysEnabled: boolean;
isAdmin: boolean;
canManage: boolean;
}

interface InvalidateApiKeysResponse {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import { APIKeysGridPage } from './api_keys_grid_page';
import { coreMock } from '../../../../../../../src/core/public/mocks';
import { apiKeysAPIClientMock } from '../index.mock';

const mock403 = () => ({ body: { statusCode: 403 } });
const mock500 = () => ({ body: { error: 'Internal Server Error', message: '', statusCode: 500 } });

const waitForRender = async (
Expand Down Expand Up @@ -48,6 +47,7 @@ describe('APIKeysGridPage', () => {
apiClientMock.checkPrivileges.mockResolvedValue({
isAdmin: true,
areApiKeysEnabled: true,
canManage: true,
});
apiClientMock.getApiKeys.mockResolvedValue({
apiKeys: [
Expand Down Expand Up @@ -82,6 +82,7 @@ describe('APIKeysGridPage', () => {
it('renders a callout when API keys are not enabled', async () => {
apiClientMock.checkPrivileges.mockResolvedValue({
isAdmin: true,
canManage: true,
areApiKeysEnabled: false,
});

Expand All @@ -95,7 +96,11 @@ describe('APIKeysGridPage', () => {
});

it('renders permission denied if user does not have required permissions', async () => {
apiClientMock.checkPrivileges.mockRejectedValue(mock403());
apiClientMock.checkPrivileges.mockResolvedValue({
canManage: false,
isAdmin: false,
areApiKeysEnabled: true,
});

const wrapper = mountWithIntl(<APIKeysGridPage {...getViewProperties()} />);

Expand Down Expand Up @@ -152,6 +157,7 @@ describe('APIKeysGridPage', () => {
beforeEach(() => {
apiClientMock.checkPrivileges.mockResolvedValue({
isAdmin: false,
canManage: true,
areApiKeysEnabled: true,
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ import {
import { i18n } from '@kbn/i18n';
import { FormattedMessage } from '@kbn/i18n/react';
import moment from 'moment-timezone';
import _ from 'lodash';
import { NotificationsStart } from 'src/core/public';
import { SectionLoading } from '../../../../../../../src/plugins/es_ui_shared/public';
import { ApiKey, ApiKeyToInvalidate } from '../../../../common/model';
Expand All @@ -47,10 +46,10 @@ interface State {
isLoadingApp: boolean;
isLoadingTable: boolean;
isAdmin: boolean;
canManage: boolean;
areApiKeysEnabled: boolean;
apiKeys: ApiKey[];
selectedItems: ApiKey[];
permissionDenied: boolean;
error: any;
}

Expand All @@ -63,9 +62,9 @@ export class APIKeysGridPage extends Component<Props, State> {
isLoadingApp: true,
isLoadingTable: false,
isAdmin: false,
canManage: false,
areApiKeysEnabled: false,
apiKeys: [],
permissionDenied: false,
selectedItems: [],
error: undefined,
};
Expand All @@ -77,19 +76,15 @@ export class APIKeysGridPage extends Component<Props, State> {

public render() {
const {
permissionDenied,
isLoadingApp,
isLoadingTable,
areApiKeysEnabled,
isAdmin,
canManage,
error,
apiKeys,
} = this.state;

if (permissionDenied) {
return <PermissionDenied />;
}

if (isLoadingApp) {
return (
<EuiPageContent>
Expand All @@ -103,6 +98,10 @@ export class APIKeysGridPage extends Component<Props, State> {
);
}

if (!canManage) {
return <PermissionDenied />;
}

if (error) {
const {
body: { error: errorTitle, message, statusCode },
Expand Down Expand Up @@ -495,26 +494,25 @@ export class APIKeysGridPage extends Component<Props, State> {

private async checkPrivileges() {
try {
const { isAdmin, areApiKeysEnabled } = await this.props.apiKeysAPIClient.checkPrivileges();
this.setState({ isAdmin, areApiKeysEnabled });
const {
isAdmin,
canManage,
areApiKeysEnabled,
} = await this.props.apiKeysAPIClient.checkPrivileges();
this.setState({ isAdmin, canManage, areApiKeysEnabled });

if (areApiKeysEnabled) {
this.initiallyLoadApiKeys();
} else {
// We're done loading and will just show the "Disabled" error.
if (!canManage || !areApiKeysEnabled) {
this.setState({ isLoadingApp: false });
}
} catch (e) {
if (_.get(e, 'body.statusCode') === 403) {
this.setState({ permissionDenied: true, isLoadingApp: false });
} else {
this.props.notifications.toasts.addDanger(
i18n.translate('xpack.security.management.apiKeys.table.fetchingApiKeysErrorMessage', {
defaultMessage: 'Error checking privileges: {message}',
values: { message: _.get(e, 'body.message', '') },
})
);
this.initiallyLoadApiKeys();
}
} catch (e) {
this.props.notifications.toasts.addDanger(
i18n.translate('xpack.security.management.apiKeys.table.fetchingApiKeysErrorMessage', {
defaultMessage: 'Error checking privileges: {message}',
values: { message: e.body?.message ?? '' },
})
);
}
}

Expand Down
4 changes: 2 additions & 2 deletions x-pack/plugins/security/public/plugin.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ describe('Security Plugin', () => {
)
).toEqual({
__legacyCompat: { logoutUrl: '/some-base-path/logout', tenant: '/some-base-path' },
authc: { getCurrentUser: expect.any(Function) },
authc: { getCurrentUser: expect.any(Function), areAPIKeysEnabled: expect.any(Function) },
license: {
isEnabled: expect.any(Function),
getFeatures: expect.any(Function),
Expand All @@ -63,7 +63,7 @@ describe('Security Plugin', () => {

expect(setupManagementServiceMock).toHaveBeenCalledTimes(1);
expect(setupManagementServiceMock).toHaveBeenCalledWith({
authc: { getCurrentUser: expect.any(Function) },
authc: { getCurrentUser: expect.any(Function), areAPIKeysEnabled: expect.any(Function) },
license: {
isEnabled: expect.any(Function),
getFeatures: expect.any(Function),
Expand Down
76 changes: 76 additions & 0 deletions x-pack/plugins/security/server/authentication/api_keys.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,82 @@ describe('API Keys', () => {
});
});

describe('areAPIKeysEnabled()', () => {
it('returns false when security feature is disabled', async () => {
mockLicense.isEnabled.mockReturnValue(false);

const result = await apiKeys.areAPIKeysEnabled();
expect(result).toEqual(false);
expect(mockScopedClusterClient.callAsCurrentUser).not.toHaveBeenCalled();
expect(mockScopedClusterClient.callAsInternalUser).not.toHaveBeenCalled();
expect(mockClusterClient.callAsInternalUser).not.toHaveBeenCalled();
});

it('returns false when the exception metadata indicates api keys are disabled', async () => {
mockLicense.isEnabled.mockReturnValue(true);
const error = new Error();
(error as any).body = {
error: { 'disabled.feature': 'api_keys' },
};
mockClusterClient.callAsInternalUser.mockRejectedValue(error);
const result = await apiKeys.areAPIKeysEnabled();
expect(mockClusterClient.callAsInternalUser).toHaveBeenCalledTimes(1);
expect(result).toEqual(false);
});

it('returns true when the operation completes without error', async () => {
mockLicense.isEnabled.mockReturnValue(true);
mockClusterClient.callAsInternalUser.mockResolvedValue({});
const result = await apiKeys.areAPIKeysEnabled();
expect(mockClusterClient.callAsInternalUser).toHaveBeenCalledTimes(1);
expect(result).toEqual(true);
});

it('throws the original error when exception metadata does not indicate that api keys are disabled', async () => {
mockLicense.isEnabled.mockReturnValue(true);
const error = new Error();
(error as any).body = {
error: { 'disabled.feature': 'something_else' },
};

mockClusterClient.callAsInternalUser.mockRejectedValue(error);
expect(apiKeys.areAPIKeysEnabled()).rejects.toThrowError(error);
expect(mockClusterClient.callAsInternalUser).toHaveBeenCalledTimes(1);
});

it('throws the original error when exception metadata does not contain `disabled.feature`', async () => {
mockLicense.isEnabled.mockReturnValue(true);
const error = new Error();
(error as any).body = {};

mockClusterClient.callAsInternalUser.mockRejectedValue(error);
expect(apiKeys.areAPIKeysEnabled()).rejects.toThrowError(error);
expect(mockClusterClient.callAsInternalUser).toHaveBeenCalledTimes(1);
});

it('throws the original error when exception contains no metadata', async () => {
mockLicense.isEnabled.mockReturnValue(true);
const error = new Error();

mockClusterClient.callAsInternalUser.mockRejectedValue(error);
expect(apiKeys.areAPIKeysEnabled()).rejects.toThrowError(error);
expect(mockClusterClient.callAsInternalUser).toHaveBeenCalledTimes(1);
});

it('calls callCluster with proper parameters', async () => {
mockLicense.isEnabled.mockReturnValue(true);
mockClusterClient.callAsInternalUser.mockResolvedValueOnce({});

const result = await apiKeys.areAPIKeysEnabled();
expect(result).toEqual(true);
expect(mockClusterClient.callAsInternalUser).toHaveBeenCalledWith('shield.invalidateAPIKey', {
body: {
id: 'kibana-api-key-service-test',
},
});
});
});

describe('create()', () => {
it('returns null when security feature is disabled', async () => {
mockLicense.isEnabled.mockReturnValue(false);
Expand Down
34 changes: 34 additions & 0 deletions x-pack/plugins/security/server/authentication/api_keys.ts
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,35 @@ export class APIKeys {
this.license = license;
}

/**
* Determines if API Keys are enabled in Elasticsearch.
*/
async areAPIKeysEnabled(): Promise<boolean> {
if (!this.license.isEnabled()) {
return false;
}

const id = `kibana-api-key-service-test`;

this.logger.debug(
`Testing if API Keys are enabled by attempting to invalidate a non-existant key: ${id}`
);

try {
await this.clusterClient.callAsInternalUser('shield.invalidateAPIKey', {
body: {
id,
},
});
return true;
} catch (e) {
if (this.doesErrorIndicateAPIKeysAreDisabled(e)) {
return false;
}
throw e;
}
}

/**
* Tries to create an API key for the current user.
* @param request Request instance.
Expand Down Expand Up @@ -247,6 +276,11 @@ export class APIKeys {
return result;
}

private doesErrorIndicateAPIKeysAreDisabled(e: Record<string, any>) {
const disabledFeature = e.body?.error?.['disabled.feature'];
return disabledFeature === 'api_keys';
}

private getGrantParams(authorizationHeader: HTTPAuthorizationHeader): GrantAPIKeyParams {
if (authorizationHeader.scheme.toLowerCase() === 'bearer') {
return {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ export const authenticationMock = {
login: jest.fn(),
logout: jest.fn(),
isProviderTypeEnabled: jest.fn(),
areAPIKeysEnabled: jest.fn(),
createAPIKey: jest.fn(),
getCurrentUser: jest.fn(),
grantAPIKeyAsInternalUser: jest.fn(),
Expand Down
1 change: 1 addition & 0 deletions x-pack/plugins/security/server/authentication/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,7 @@ export async function setupAuthentication({
getSessionInfo: authenticator.getSessionInfo.bind(authenticator),
isProviderTypeEnabled: authenticator.isProviderTypeEnabled.bind(authenticator),
getCurrentUser,
areAPIKeysEnabled: () => apiKeys.areAPIKeysEnabled(),
createAPIKey: (request: KibanaRequest, params: CreateAPIKeyParams) =>
apiKeys.create(request, params),
grantAPIKeyAsInternalUser: (request: KibanaRequest) => apiKeys.grantAsInternalUser(request),
Expand Down
1 change: 1 addition & 0 deletions x-pack/plugins/security/server/plugin.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ describe('Security Plugin', () => {
"registerPrivilegesWithCluster": [Function],
},
"authc": Object {
"areAPIKeysEnabled": [Function],
"createAPIKey": [Function],
"getCurrentUser": [Function],
"getSessionInfo": [Function],
Expand Down
Loading

0 comments on commit 44f9cbc

Please sign in to comment.