Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -184,6 +184,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