Skip to content

Commit

Permalink
Require granted API Keys to have a name (#71623)
Browse files Browse the repository at this point in the history
  • Loading branch information
legrego authored Jul 14, 2020
1 parent 801ad64 commit 9356966
Show file tree
Hide file tree
Showing 6 changed files with 84 additions and 19 deletions.
30 changes: 24 additions & 6 deletions x-pack/plugins/alerts/server/alerts_client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
SavedObjectReference,
SavedObject,
} from 'src/core/server';
import _ from 'lodash';
import { ActionsClient } from '../../actions/server';
import {
Alert,
Expand Down Expand Up @@ -53,7 +54,7 @@ interface ConstructorOptions {
spaceId?: string;
namespace?: string;
getUserName: () => Promise<string | null>;
createAPIKey: () => Promise<CreateAPIKeyResult>;
createAPIKey: (name: string) => Promise<CreateAPIKeyResult>;
invalidateAPIKey: (params: InvalidateAPIKeyParams) => Promise<InvalidateAPIKeyResult>;
getActionsClient: () => Promise<ActionsClient>;
}
Expand Down Expand Up @@ -129,7 +130,7 @@ export class AlertsClient {
private readonly taskManager: TaskManagerStartContract;
private readonly savedObjectsClient: SavedObjectsClientContract;
private readonly alertTypeRegistry: AlertTypeRegistry;
private readonly createAPIKey: () => Promise<CreateAPIKeyResult>;
private readonly createAPIKey: (name: string) => Promise<CreateAPIKeyResult>;
private readonly invalidateAPIKey: (
params: InvalidateAPIKeyParams
) => Promise<InvalidateAPIKeyResult>;
Expand Down Expand Up @@ -167,7 +168,10 @@ export class AlertsClient {
const alertType = this.alertTypeRegistry.get(data.alertTypeId);
const validatedAlertTypeParams = validateAlertTypeParams(alertType, data.params);
const username = await this.getUserName();
const createdAPIKey = data.enabled ? await this.createAPIKey() : null;

const createdAPIKey = data.enabled
? await this.createAPIKey(this.generateAPIKeyName(alertType.id, data.name))
: null;

this.validateActions(alertType, data.actions);

Expand Down Expand Up @@ -334,7 +338,9 @@ export class AlertsClient {

const { actions, references } = await this.denormalizeActions(data.actions);
const username = await this.getUserName();
const createdAPIKey = attributes.enabled ? await this.createAPIKey() : null;
const createdAPIKey = attributes.enabled
? await this.createAPIKey(this.generateAPIKeyName(alertType.id, data.name))
: null;
const apiKeyAttributes = this.apiKeyAsAlertAttributes(createdAPIKey, username);

const updatedObject = await this.savedObjectsClient.update<RawAlert>(
Expand Down Expand Up @@ -406,7 +412,10 @@ export class AlertsClient {
id,
{
...attributes,
...this.apiKeyAsAlertAttributes(await this.createAPIKey(), username),
...this.apiKeyAsAlertAttributes(
await this.createAPIKey(this.generateAPIKeyName(attributes.alertTypeId, attributes.name)),
username
),
updatedBy: username,
},
{ version }
Expand Down Expand Up @@ -464,7 +473,12 @@ export class AlertsClient {
{
...attributes,
enabled: true,
...this.apiKeyAsAlertAttributes(await this.createAPIKey(), username),
...this.apiKeyAsAlertAttributes(
await this.createAPIKey(
this.generateAPIKeyName(attributes.alertTypeId, attributes.name)
),
username
),
updatedBy: username,
},
{ version }
Expand Down Expand Up @@ -697,4 +711,8 @@ export class AlertsClient {
references,
};
}

private generateAPIKeyName(alertTypeId: string, alertName: string) {
return _.truncate(`Alerting: ${alertTypeId}/${alertName}`, { length: 256 });
}
}
8 changes: 6 additions & 2 deletions x-pack/plugins/alerts/server/alerts_client_factory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,15 +70,19 @@ export class AlertsClientFactory {
const user = await securityPluginSetup.authc.getCurrentUser(request);
return user ? user.username : null;
},
async createAPIKey() {
async createAPIKey(name: string) {
if (!securityPluginSetup) {
return { apiKeysEnabled: false };
}
// Create an API key using the new grant API - in this case the Kibana system user is creating the
// API key for the user, instead of having the user create it themselves, which requires api_key
// privileges
const createAPIKeyResult = await securityPluginSetup.authc.grantAPIKeyAsInternalUser(
request
request,
{
name,
role_descriptors: {},
}
);
if (!createAPIKeyResult) {
return { apiKeysEnabled: false };
Expand Down
38 changes: 34 additions & 4 deletions x-pack/plugins/security/server/authentication/api_keys.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,10 @@ describe('API Keys', () => {
describe('grantAsInternalUser()', () => {
it('returns null when security feature is disabled', async () => {
mockLicense.isEnabled.mockReturnValue(false);
const result = await apiKeys.grantAsInternalUser(httpServerMock.createKibanaRequest());
const result = await apiKeys.grantAsInternalUser(httpServerMock.createKibanaRequest(), {
name: 'test_api_key',
role_descriptors: {},
});
expect(result).toBeNull();

expect(mockClusterClient.callAsInternalUser).not.toHaveBeenCalled();
Expand All @@ -174,21 +177,33 @@ describe('API Keys', () => {
id: '123',
name: 'key-name',
api_key: 'abc123',
expires: '1d',
});
const result = await apiKeys.grantAsInternalUser(
httpServerMock.createKibanaRequest({
headers: {
authorization: `Basic ${encodeToBase64('foo:bar')}`,
},
})
}),
{
name: 'test_api_key',
role_descriptors: { foo: true },
expiration: '1d',
}
);
expect(result).toEqual({
api_key: 'abc123',
id: '123',
name: 'key-name',
expires: '1d',
});
expect(mockClusterClient.callAsInternalUser).toHaveBeenCalledWith('shield.grantAPIKey', {
body: {
api_key: {
name: 'test_api_key',
role_descriptors: { foo: true },
expiration: '1d',
},
grant_type: 'password',
username: 'foo',
password: 'bar',
Expand All @@ -208,7 +223,12 @@ describe('API Keys', () => {
headers: {
authorization: `Bearer foo-access-token`,
},
})
}),
{
name: 'test_api_key',
role_descriptors: { foo: true },
expiration: '1d',
}
);
expect(result).toEqual({
api_key: 'abc123',
Expand All @@ -217,6 +237,11 @@ describe('API Keys', () => {
});
expect(mockClusterClient.callAsInternalUser).toHaveBeenCalledWith('shield.grantAPIKey', {
body: {
api_key: {
name: 'test_api_key',
role_descriptors: { foo: true },
expiration: '1d',
},
grant_type: 'access_token',
access_token: 'foo-access-token',
},
Expand All @@ -231,7 +256,12 @@ describe('API Keys', () => {
headers: {
authorization: `Digest username="foo"`,
},
})
}),
{
name: 'test_api_key',
role_descriptors: { foo: true },
expiration: '1d',
}
)
).rejects.toThrowErrorMatchingInlineSnapshot(
`"Unsupported scheme \\"Digest\\" for granting API Key"`
Expand Down
12 changes: 9 additions & 3 deletions x-pack/plugins/security/server/authentication/api_keys.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ export interface CreateAPIKeyParams {
}

interface GrantAPIKeyParams {
api_key: CreateAPIKeyParams;
grant_type: 'password' | 'access_token';
username?: string;
password?: string;
Expand Down Expand Up @@ -188,7 +189,7 @@ export class APIKeys {
* Tries to grant an API key for the current user.
* @param request Request instance.
*/
async grantAsInternalUser(request: KibanaRequest) {
async grantAsInternalUser(request: KibanaRequest, createParams: CreateAPIKeyParams) {
if (!this.license.isEnabled()) {
return null;
}
Expand All @@ -200,7 +201,7 @@ export class APIKeys {
`Unable to grant an API Key, request does not contain an authorization header`
);
}
const params = this.getGrantParams(authorizationHeader);
const params = this.getGrantParams(createParams, authorizationHeader);

// User needs `manage_api_key` or `grant_api_key` privilege to use this API
let result: GrantAPIKeyResult;
Expand Down Expand Up @@ -281,9 +282,13 @@ export class APIKeys {
return disabledFeature === 'api_keys';
}

private getGrantParams(authorizationHeader: HTTPAuthorizationHeader): GrantAPIKeyParams {
private getGrantParams(
createParams: CreateAPIKeyParams,
authorizationHeader: HTTPAuthorizationHeader
): GrantAPIKeyParams {
if (authorizationHeader.scheme.toLowerCase() === 'bearer') {
return {
api_key: createParams,
grant_type: 'access_token',
access_token: authorizationHeader.credentials,
};
Expand All @@ -294,6 +299,7 @@ export class APIKeys {
authorizationHeader.credentials
);
return {
api_key: createParams,
grant_type: 'password',
username: basicCredentials.username,
password: basicCredentials.password,
Expand Down
12 changes: 9 additions & 3 deletions x-pack/plugins/security/server/authentication/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -374,7 +374,10 @@ describe('setupAuthentication()', () => {
});

describe('grantAPIKeyAsInternalUser()', () => {
let grantAPIKeyAsInternalUser: (request: KibanaRequest) => Promise<CreateAPIKeyResult | null>;
let grantAPIKeyAsInternalUser: (
request: KibanaRequest,
params: CreateAPIKeyParams
) => Promise<CreateAPIKeyResult | null>;
beforeEach(async () => {
grantAPIKeyAsInternalUser = (await setupAuthentication(mockSetupAuthenticationParams))
.grantAPIKeyAsInternalUser;
Expand All @@ -384,10 +387,13 @@ describe('setupAuthentication()', () => {
const request = httpServerMock.createKibanaRequest();
const apiKeysInstance = jest.requireMock('./api_keys').APIKeys.mock.instances[0];
apiKeysInstance.grantAsInternalUser.mockResolvedValueOnce({ api_key: 'foo' });
await expect(grantAPIKeyAsInternalUser(request)).resolves.toEqual({

const createParams = { name: 'test_key', role_descriptors: {} };

await expect(grantAPIKeyAsInternalUser(request, createParams)).resolves.toEqual({
api_key: 'foo',
});
expect(apiKeysInstance.grantAsInternalUser).toHaveBeenCalledWith(request);
expect(apiKeysInstance.grantAsInternalUser).toHaveBeenCalledWith(request, createParams);
});
});

Expand Down
3 changes: 2 additions & 1 deletion x-pack/plugins/security/server/authentication/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,8 @@ export async function setupAuthentication({
areAPIKeysEnabled: () => apiKeys.areAPIKeysEnabled(),
createAPIKey: (request: KibanaRequest, params: CreateAPIKeyParams) =>
apiKeys.create(request, params),
grantAPIKeyAsInternalUser: (request: KibanaRequest) => apiKeys.grantAsInternalUser(request),
grantAPIKeyAsInternalUser: (request: KibanaRequest, params: CreateAPIKeyParams) =>
apiKeys.grantAsInternalUser(request, params),
invalidateAPIKey: (request: KibanaRequest, params: InvalidateAPIKeyParams) =>
apiKeys.invalidate(request, params),
invalidateAPIKeyAsInternalUser: (params: InvalidateAPIKeyParams) =>
Expand Down

0 comments on commit 9356966

Please sign in to comment.