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

[Security/APIKey Service] Internal API endpoint do determine if user has API keys #172884

Merged
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
24b53e9
Extend Security APIKey service
tsullivan Dec 7, 2023
b730c9f
backing out of sprawling test updates
tsullivan Dec 14, 2023
89a92fd
Tests
tsullivan Dec 14, 2023
a469842
fix test
tsullivan Dec 14, 2023
1a332f6
Merge branch 'main' into security/api-keys-service-has-api-keys
tsullivan Dec 18, 2023
9fd5b10
remove non-trivial test
tsullivan Dec 18, 2023
bdf8987
Merge branch 'main' into security/api-keys-service-has-api-keys
tsullivan Dec 18, 2023
09496b5
use naming consistent with _enabled
tsullivan Dec 18, 2023
c0fb932
Update x-pack/plugins/security/server/routes/api_keys/has.ts
tsullivan Dec 18, 2023
f39999c
Merge branch 'security/api-keys-service-has-api-keys' of github.com:t…
tsullivan Dec 18, 2023
598f141
fix path of the new endpoint
tsullivan Dec 18, 2023
5b51b06
Merge branch 'main' into security/api-keys-service-has-api-keys
tsullivan Dec 18, 2023
474ed25
Move new test outside of serverless area
tsullivan Dec 18, 2023
83ea4c9
Merge branch 'main' into security/api-keys-service-has-api-keys
tsullivan Dec 20, 2023
423037a
Merge branch 'main' into security/api-keys-service-has-api-keys
tsullivan Dec 21, 2023
c1aeb86
Update x-pack/plugins/security/server/routes/api_keys/has_active.test.ts
tsullivan Dec 21, 2023
27a823a
Update x-pack/test/security_api_integration/tests/api_keys/has_active…
tsullivan Dec 21, 2023
eeb8e10
Add active_only param
tsullivan Dec 21, 2023
7640ed4
Merge branch 'security/api-keys-service-has-api-keys' of github.com:t…
tsullivan Dec 21, 2023
d15c932
Allow core to handle error thrown from route handler
tsullivan Dec 21, 2023
c6461cf
update test per removal of error catching in the route handler
tsullivan Dec 21, 2023
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
125 changes: 125 additions & 0 deletions x-pack/plugins/security/server/routes/api_keys/has.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/
import Boom from '@hapi/boom';

import { kibanaResponseFactory } from '@kbn/core/server';
import type { RequestHandler } from '@kbn/core/server';
import type { CustomRequestHandlerMock, ScopedClusterClientMock } from '@kbn/core/server/mocks';
import { coreMock, httpServerMock } from '@kbn/core/server/mocks';
import { licensingMock } from '@kbn/licensing-plugin/server/mocks';
import type { DeeplyMockedKeys } from '@kbn/utility-types-jest';

import { defineHasApiKeysRoutes } from './has';
import type { InternalAuthenticationServiceStart } from '../../authentication';
import { authenticationServiceMock } from '../../authentication/authentication_service.mock';
import { routeDefinitionParamsMock } from '../index.mock';

describe('Has API Keys route', () => {
let routeHandler: RequestHandler<any, any, any, any>;
let authc: DeeplyMockedKeys<InternalAuthenticationServiceStart>;
let esClientMock: ScopedClusterClientMock;
let mockContext: CustomRequestHandlerMock<unknown>;

beforeEach(async () => {
const mockRouteDefinitionParams = routeDefinitionParamsMock.create();
authc = authenticationServiceMock.createStart();
mockRouteDefinitionParams.getAuthenticationService.mockReturnValue(authc);
defineHasApiKeysRoutes(mockRouteDefinitionParams);
[[, routeHandler]] = mockRouteDefinitionParams.router.get.mock.calls;
mockContext = coreMock.createCustomRequestHandlerContext({
core: coreMock.createRequestHandlerContext(),
licensing: licensingMock.createRequestHandlerContext(),
});

esClientMock = (await mockContext.core).elasticsearch.client;

authc.apiKeys.areAPIKeysEnabled.mockResolvedValue(true);
authc.apiKeys.areCrossClusterAPIKeysEnabled.mockResolvedValue(true);

esClientMock.asCurrentUser.security.getApiKey.mockResponse({
api_keys: [
{ id: '123', invalidated: false },
{ id: '456', invalidated: true },
],
} as any);
});

it('should calculate when user has API keys', async () => {
const response = await routeHandler(
mockContext,
httpServerMock.createKibanaRequest(),
kibanaResponseFactory
);

expect(response.payload).toEqual(
expect.objectContaining({
hasApiKeys: true,
})
);
});

it('should calculate when user does not have API keys', async () => {
esClientMock.asCurrentUser.security.getApiKey.mockResponse({
api_keys: [],
});

const response = await routeHandler(
mockContext,
httpServerMock.createKibanaRequest(),
kibanaResponseFactory
);

expect(response.payload).toEqual(
expect.objectContaining({
hasApiKeys: false,
})
);
});

it('should filter out invalidated API keys', async () => {
const response = await routeHandler(
mockContext,
httpServerMock.createKibanaRequest(),
kibanaResponseFactory
);

expect(response.status).toBe(200);
expect(response.payload?.hasApiKeys).toBe(true);
});

it('should return `404` if API keys are disabled', async () => {
authc.apiKeys.areAPIKeysEnabled.mockResolvedValue(false);

const response = await routeHandler(
mockContext,
httpServerMock.createKibanaRequest(),
kibanaResponseFactory
);

expect(response.status).toBe(404);
expect(response.payload).toEqual({
message:
"API keys are disabled in Elasticsearch. To use API keys enable 'xpack.security.authc.api_key.enabled' setting.",
});
});

it('should forward error from Elasticsearch GET API keys endpoint', async () => {
const error = Boom.forbidden('test not acceptable message');
esClientMock.asCurrentUser.security.getApiKey.mockResponseImplementation(() => {
throw error;
});

const response = await routeHandler(
mockContext,
httpServerMock.createKibanaRequest(),
kibanaResponseFactory
);

expect(response.status).toBe(403);
expect(response.payload).toEqual(error);
});
});
65 changes: 65 additions & 0 deletions x-pack/plugins/security/server/routes/api_keys/has.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

import type { RouteDefinitionParams } from '..';
import { wrapIntoCustomErrorResponse } from '../../errors';
import { createLicensedRouteHandler } from '../licensed_route_handler';

/**
* Response of Kibana Has API keys endpoint.
*/
export interface HasAPIKeysResult {
hasApiKeys: boolean;
}

export function defineHasApiKeysRoutes({
tsullivan marked this conversation as resolved.
Show resolved Hide resolved
router,
getAuthenticationService,
}: RouteDefinitionParams) {
router.get(
{
path: '/internal/security/has_api_keys',
tsullivan marked this conversation as resolved.
Show resolved Hide resolved
validate: false,
options: {
access: 'internal',
},
},
createLicensedRouteHandler(async (context, _request, response) => {
try {
// copied logic from get.ts
tsullivan marked this conversation as resolved.
Show resolved Hide resolved
const esClient = (await context.core).elasticsearch.client;
const authenticationService = getAuthenticationService();

const areApiKeysEnabled = await authenticationService.apiKeys.areAPIKeysEnabled();

if (!areApiKeysEnabled) {
return response.notFound({
body: {
message:
"API keys are disabled in Elasticsearch. To use API keys enable 'xpack.security.authc.api_key.enabled' setting.",
},
});
}

const apiResponse = await esClient.asCurrentUser.security.getApiKey({
owner: true,
});
Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not sure the owner field needs any conditions, but perhaps I'm not aware.

Copy link
Member

Choose a reason for hiding this comment

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

Hmm, I'm not aware of any conditions either.


const validKeys = apiResponse.api_keys.filter(({ invalidated }) => !invalidated);
Copy link
Member

Choose a reason for hiding this comment

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

question: you filter out invalidated keys, but not expired ones - is it intentional? If not, can we use active_only request parameter to avoid this additional filtering?

Copy link
Member Author

Choose a reason for hiding this comment

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

@azasypkin I see your point, but I don't see the active_only parameter supported in the SecurityGetApiKeyRequest typings.

Copy link
Member

Choose a reason for hiding this comment

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

Hmm, it should be available since 8.10.0 - elastic/elasticsearch#98259 - let me figure this out. In the worst case I'd prefer to do something like this to filter keys on ES side instead:

const apiResponse = await esClient.asCurrentUser.security.getApiKey({
  owner: true,
  // @ts-expect-error @elastic/elasticsearch SecurityGetApiKeyRequest.active_only: boolean | undefined
  active_only: true,
});

return response.ok<HasAPIKeysResult>({
  body: {
    hasApiKeys: apiResponse.api_keys.length > 0,
  },
});

Copy link
Member

Choose a reason for hiding this comment

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

Okay, we'll get this definition in the client soon (elastic/elasticsearch#103621), let's move forward with active_only and @ts-expect-error (if needed) for now (Client will include the parameter regardless wrong type definition).


// simply return true if the result array is non-empty
return response.ok<HasAPIKeysResult>({
body: {
hasApiKeys: validKeys.length > 0,
},
});
} catch (error) {
return response.customError(wrapIntoCustomErrorResponse(error));
}
})
);
}
2 changes: 2 additions & 0 deletions x-pack/plugins/security/server/routes/api_keys/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import { defineCreateApiKeyRoutes } from './create';
import { defineEnabledApiKeysRoutes } from './enabled';
import { defineGetApiKeysRoutes } from './get';
import { defineHasApiKeysRoutes } from './has';
import { defineInvalidateApiKeysRoutes } from './invalidate';
import { defineUpdateApiKeyRoutes } from './update';
import type { RouteDefinitionParams } from '..';
Expand All @@ -24,6 +25,7 @@ export type { GetAPIKeysResult } from './get';
export function defineApiKeysRoutes(params: RouteDefinitionParams) {
defineEnabledApiKeysRoutes(params);
defineGetApiKeysRoutes(params);
defineHasApiKeysRoutes(params);
defineCreateApiKeyRoutes(params);
defineUpdateApiKeyRoutes(params);
defineInvalidateApiKeysRoutes(params);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,31 @@ export default function ({ getService }: FtrProviderContext) {
expect(status).toBe(200);
});

it('has_api_keys', async () => {
tsullivan marked this conversation as resolved.
Show resolved Hide resolved
let body: unknown;
let status: number;

({ body, status } = await supertest
.get('/internal/security/has_api_keys')
.set(svlCommonApi.getCommonRequestHeader()));
// expect a rejection because we're not using the internal header
expect(body).toEqual({
statusCode: 400,
error: 'Bad Request',
message: expect.stringContaining(
'method [get] exists but is not available with the current configuration'
),
});
expect(status).toBe(400);

({ body, status } = await supertest
.get('/internal/security/has_api_keys')
.set(svlCommonApi.getInternalRequestHeader()));
// expect success because we're using the internal header
expect(status).toBe(200);
expect(body).toEqual({ hasApiKeys: true });
});

it('invalidate', async () => {
let body: unknown;
let status: number;
Expand Down