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

[Telemetry] Settings Collector: redact sensitive reported values #88675

Merged
merged 18 commits into from
Jan 27, 2021
Merged
Show file tree
Hide file tree
Changes from 7 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 @@ -26,6 +26,7 @@ export interface UiSettingsParams<T = unknown>
| [readonly](./kibana-plugin-core-public.uisettingsparams.readonly.md) | <code>boolean</code> | a flag indicating that value cannot be changed |
| [requiresPageReload](./kibana-plugin-core-public.uisettingsparams.requirespagereload.md) | <code>boolean</code> | a flag indicating whether new value applying requires page reloading |
| [schema](./kibana-plugin-core-public.uisettingsparams.schema.md) | <code>Type&lt;T&gt;</code> | |
| [sensitive](./kibana-plugin-core-public.uisettingsparams.sensitive.md) | <code>boolean</code> | a flag indicating that value might contain user sensitive data. used by telemetry to mask the value of the setting when sent. |
| [type](./kibana-plugin-core-public.uisettingsparams.type.md) | <code>UiSettingsType</code> | defines a type of UI element [UiSettingsType](./kibana-plugin-core-public.uisettingstype.md) |
| [validation](./kibana-plugin-core-public.uisettingsparams.validation.md) | <code>ImageValidation &#124; StringValidation</code> | |
| [value](./kibana-plugin-core-public.uisettingsparams.value.md) | <code>T</code> | default value to fall back to if a user doesn't provide any |
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
<!-- Do not edit this file. It is automatically generated by API Documenter. -->

[Home](./index.md) &gt; [kibana-plugin-core-public](./kibana-plugin-core-public.md) &gt; [UiSettingsParams](./kibana-plugin-core-public.uisettingsparams.md) &gt; [sensitive](./kibana-plugin-core-public.uisettingsparams.sensitive.md)

## UiSettingsParams.sensitive property

a flag indicating that value might contain user sensitive data. used by telemetry to mask the value of the setting when sent.

<b>Signature:</b>

```typescript
sensitive?: boolean;
```
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
<!-- Do not edit this file. It is automatically generated by API Documenter. -->

[Home](./index.md) &gt; [kibana-plugin-core-server](./kibana-plugin-core-server.md) &gt; [IUiSettingsClient](./kibana-plugin-core-server.iuisettingsclient.md) &gt; [isSensitive](./kibana-plugin-core-server.iuisettingsclient.issensitive.md)

## IUiSettingsClient.isSensitive property

Shows whether the uiSetting is a sensitive value. Used by telemetry to not send sensitive values.

<b>Signature:</b>

```typescript
isSensitive: (key: string) => boolean;
```
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ export interface IUiSettingsClient
| [getRegistered](./kibana-plugin-core-server.iuisettingsclient.getregistered.md) | <code>() =&gt; Readonly&lt;Record&lt;string, PublicUiSettingsParams&gt;&gt;</code> | Returns registered uiSettings values [UiSettingsParams](./kibana-plugin-core-server.uisettingsparams.md) |
| [getUserProvided](./kibana-plugin-core-server.iuisettingsclient.getuserprovided.md) | <code>&lt;T = any&gt;() =&gt; Promise&lt;Record&lt;string, UserProvidedValues&lt;T&gt;&gt;&gt;</code> | Retrieves a set of all uiSettings values set by the user. |
| [isOverridden](./kibana-plugin-core-server.iuisettingsclient.isoverridden.md) | <code>(key: string) =&gt; boolean</code> | Shows whether the uiSettings value set by the user. |
| [isSensitive](./kibana-plugin-core-server.iuisettingsclient.issensitive.md) | <code>(key: string) =&gt; boolean</code> | Shows whether the uiSetting is a sensitive value. Used by telemetry to not send sensitive values. |
| [remove](./kibana-plugin-core-server.iuisettingsclient.remove.md) | <code>(key: string) =&gt; Promise&lt;void&gt;</code> | Removes uiSettings value by key. |
| [removeMany](./kibana-plugin-core-server.iuisettingsclient.removemany.md) | <code>(keys: string[]) =&gt; Promise&lt;void&gt;</code> | Removes multiple uiSettings values by keys. |
| [set](./kibana-plugin-core-server.iuisettingsclient.set.md) | <code>(key: string, value: any) =&gt; Promise&lt;void&gt;</code> | Writes uiSettings value and marks it as set by the user. |
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ export interface UiSettingsParams<T = unknown>
| [readonly](./kibana-plugin-core-server.uisettingsparams.readonly.md) | <code>boolean</code> | a flag indicating that value cannot be changed |
| [requiresPageReload](./kibana-plugin-core-server.uisettingsparams.requirespagereload.md) | <code>boolean</code> | a flag indicating whether new value applying requires page reloading |
| [schema](./kibana-plugin-core-server.uisettingsparams.schema.md) | <code>Type&lt;T&gt;</code> | |
| [sensitive](./kibana-plugin-core-server.uisettingsparams.sensitive.md) | <code>boolean</code> | a flag indicating that value might contain user sensitive data. used by telemetry to mask the value of the setting when sent. |
| [type](./kibana-plugin-core-server.uisettingsparams.type.md) | <code>UiSettingsType</code> | defines a type of UI element [UiSettingsType](./kibana-plugin-core-server.uisettingstype.md) |
| [validation](./kibana-plugin-core-server.uisettingsparams.validation.md) | <code>ImageValidation &#124; StringValidation</code> | |
| [value](./kibana-plugin-core-server.uisettingsparams.value.md) | <code>T</code> | default value to fall back to if a user doesn't provide any |
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
<!-- Do not edit this file. It is automatically generated by API Documenter. -->

[Home](./index.md) &gt; [kibana-plugin-core-server](./kibana-plugin-core-server.md) &gt; [UiSettingsParams](./kibana-plugin-core-server.uisettingsparams.md) &gt; [sensitive](./kibana-plugin-core-server.uisettingsparams.sensitive.md)

## UiSettingsParams.sensitive property

a flag indicating that value might contain user sensitive data. used by telemetry to mask the value of the setting when sent.

<b>Signature:</b>

```typescript
sensitive?: boolean;
```
17 changes: 17 additions & 0 deletions packages/kbn-dev-utils/src/kbn_client/kbn_client_ui_settings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { ToolingLog } from '../tooling_log';
import { KbnClientRequester, uriencode } from './kbn_client_requester';

export type UiSettingValues = Record<string, string | number | boolean>;

interface UiSettingsApiResponse {
settings: {
[key: string]: {
Expand All @@ -20,6 +21,10 @@ interface UiSettingsApiResponse {
};
}

interface UiSettingsWithRegisteredApiResponse<V> extends UiSettingsApiResponse {
registered: Record<string, V>;
}

export class KbnClientUiSettings {
constructor(
private readonly log: ToolingLog,
Expand Down Expand Up @@ -79,6 +84,18 @@ export class KbnClientUiSettings {
});
}

/**
* Returns all registered kibana settings
*/
async getAllRegistered<T = unknown>() {
const { data } = await this.requester.request<UiSettingsWithRegisteredApiResponse<T>>({
path: '/api/kibana/settings?_allRegistered=true',
method: 'GET',
});

return data.registered;
}

/**
* Add fields to the config doc (like setting timezone and defaultIndex)
*/
Expand Down
2 changes: 2 additions & 0 deletions packages/kbn-telemetry-tools/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,5 @@

export { runTelemetryCheck } from './cli/run_telemetry_check';
export { runTelemetryExtract } from './cli/run_telemetry_extract';
export { getInterfacesDescriptors } from './tools/interface_to_object';
export { loadProgram } from './tools/utils';
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* and the Server Side Public License, v 1; you may not use this file except in
* compliance with, at your election, the Elastic License or the Server Side
* Public License, v 1.
*/

export interface UsageStats {
withBooleanValue: boolean;
withStringValue: string;
withNumberValue: number;
withStringArrayValue: string[];
withObjectValue: { withBooleanValue: boolean };
withObjectArrayValue: Array<{ withNumberValue: number }>;
}
61 changes: 61 additions & 0 deletions packages/kbn-telemetry-tools/src/tools/interface_to_object.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* and the Server Side Public License, v 1; you may not use this file except in
* compliance with, at your election, the Elastic License or the Server Side
* Public License, v 1.
*/

import * as ts from 'typescript';
import * as path from 'path';
import { loadProgram } from './utils';
import { getInterfacesDescriptors } from './interface_to_object';

describe('getInterfacesDescriptors', () => {
it('creates object descriptors from interfaces', async () => {
const fixturePath = path.resolve(__dirname, '__fixture__', 'usage_stats_type.ts');
const { program } = loadProgram('./', [fixturePath]);
const sourceFile = program.getSourceFile(fixturePath);
if (!sourceFile) {
throw Error('sourceFile is undefined!');
}

const results = getInterfacesDescriptors(sourceFile, program, 'UsageStats');
expect(results).toEqual([
{
withBooleanValue: {
kind: ts.SyntaxKind.BooleanKeyword,
type: 'BooleanKeyword',
},
withStringValue: {
kind: ts.SyntaxKind.StringKeyword,
type: 'StringKeyword',
},
withNumberValue: {
kind: ts.SyntaxKind.NumberKeyword,
type: 'NumberKeyword',
},
withStringArrayValue: {
items: {
kind: ts.SyntaxKind.StringKeyword,
type: 'StringKeyword',
},
},
withObjectValue: {
withBooleanValue: {
kind: ts.SyntaxKind.BooleanKeyword,
type: 'BooleanKeyword',
},
},
withObjectArrayValue: {
items: {
withNumberValue: {
kind: ts.SyntaxKind.NumberKeyword,
type: 'NumberKeyword',
},
},
},
},
]);
});
});
53 changes: 53 additions & 0 deletions packages/kbn-telemetry-tools/src/tools/interface_to_object.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
* and the Server Side Public License, v 1; you may not use this file except in
* compliance with, at your election, the Elastic License or the Server Side
* Public License, v 1.
*/

import * as ts from 'typescript';
import { getDescriptor, Descriptor, DescriptorValue } from './serializer';

export type InterfaceObjectType = Record<string, Descriptor | DescriptorValue>;

export function getInterfacesDescriptors(
sourceFile: ts.SourceFile,
program: ts.Program,
interfaceName: string
): InterfaceObjectType[] {
const interfacesObjects: InterfaceObjectType[] = [];
const typeChecker = program.getTypeChecker();

ts.forEachChild(sourceFile, (node) => {
if (ts.isInterfaceDeclaration(node)) {
if (ts.getNameOfDeclaration(node)?.getText() === interfaceName) {
const symbols = typeChecker.getSymbolsInScope(node, ts.SymbolFlags.Interface);

const symbolDeclaration = symbols
.find((symbol) => symbol.name === interfaceName)
?.getDeclarations()
?.find(ts.isInterfaceDeclaration)
?.getChildren()
.filter((child) => child.kind === 334)
.pop();

const interfaceObject = symbolDeclaration?.getChildren().reduce((acc, child) => {
if (ts.isPropertySignature(child)) {
const childNameText = child.name.getText();
const key = childNameText.replace(/["']/g, '');
return { ...acc, [key]: getDescriptor(child, program) };
}

throw new Error(`Unrecognized property in interface of kind: ${child.kind}.`);
}, {} as InterfaceObjectType);

if (interfaceObject) {
interfacesObjects.push(interfaceObject);
}
}
}
});

return interfacesObjects;
}
21 changes: 19 additions & 2 deletions packages/kbn-telemetry-tools/src/tools/serializer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ export enum TelemetryKinds {
Date = 10001,
}

interface DescriptorValue {
export interface DescriptorValue {
kind: ts.SyntaxKind | TelemetryKinds;
type: keyof typeof ts.SyntaxKind | keyof typeof TelemetryKinds;
}
Expand All @@ -42,6 +42,13 @@ export function isObjectDescriptor(value: any) {
return false;
}

export function descriptorToObject(descriptor: Descriptor | DescriptorValue) {
return Object.entries(descriptor).reduce((acc, [key, value]) => {
acc[key] = value.kind ? kindToDescriptorName(value.kind) : descriptorToObject(value);
return acc;
}, {} as Record<string, any>);
}

export function kindToDescriptorName(kind: number) {
switch (kind) {
case ts.SyntaxKind.StringKeyword:
Expand Down Expand Up @@ -158,6 +165,16 @@ export function getDescriptor(node: ts.Node, program: ts.Program): Descriptor |
if (symbolName === 'Date') {
return { kind: TelemetryKinds.Date, type: 'Date' };
}

// Support Array<T>
if (symbolName === 'Array') {
if (node.typeArguments?.length !== 1) {
throw Error('Array type only supports 1 type parameter Array<T>');
}
Comment on lines +170 to +173
Copy link
Contributor

Choose a reason for hiding this comment

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

UltraNIT: better safe than sorry I guess, but I still wonder if this assertion is really useful.

Copy link
Member Author

Choose a reason for hiding this comment

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

It is not needed but to stay consistent with other symbols we check for.

const typeArgument = node.typeArguments[0];
return { items: getDescriptor(typeArgument, program) };
}

// Support `Record<string, SOMETHING>`
if (symbolName === 'Record') {
const descriptor = getDescriptor(node.typeArguments![1], program);
Expand Down Expand Up @@ -243,7 +260,7 @@ export function getDescriptor(node: ts.Node, program: ts.Program): Descriptor |
case ts.SyntaxKind.UnionType:
case ts.SyntaxKind.AnyKeyword:
default:
throw new Error(`Unknown type ${ts.SyntaxKind[node.kind]}; ${node.getText()}`);
throw new Error(`Unknown type ${ts.SyntaxKind[node.kind]}`);
}
}

Expand Down
10 changes: 10 additions & 0 deletions packages/kbn-telemetry-tools/src/tools/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,16 @@ export const readFileAsync = promisify(readFile);
export const writeFileAsync = promisify(writeFile);
export const globAsync = promisify(glob);

export function loadProgram(configPath: string, files: string[]) {
const tsConfig = ts.findConfigFile('./', ts.sys.fileExists, 'tsconfig.json');
if (!tsConfig) {
throw new Error('Could not find a valid tsconfig.json.');
}
const program = ts.createProgram(files, tsConfig as any);
const typeChecker = program.getTypeChecker();
return { program, typeChecker };
}

export function isPropertyWithKey(property: ts.Node, identifierName: string) {
if (ts.isPropertyAssignment(property) || ts.isMethodDeclaration(property)) {
if (ts.isIdentifier(property.name)) {
Expand Down
1 change: 1 addition & 0 deletions src/core/public/public.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -1502,6 +1502,7 @@ export interface UiSettingsParams<T = unknown> {
requiresPageReload?: boolean;
// (undocumented)
schema: Type<T>;
sensitive?: boolean;
type?: UiSettingsType;
// (undocumented)
validation?: ImageValidation | StringValidation;
Expand Down
2 changes: 2 additions & 0 deletions src/core/server/server.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -1195,6 +1195,7 @@ export interface IUiSettingsClient {
getRegistered: () => Readonly<Record<string, PublicUiSettingsParams>>;
getUserProvided: <T = any>() => Promise<Record<string, UserProvidedValues<T>>>;
isOverridden: (key: string) => boolean;
isSensitive: (key: string) => boolean;
remove: (key: string) => Promise<void>;
removeMany: (keys: string[]) => Promise<void>;
set: (key: string, value: any) => Promise<void>;
Expand Down Expand Up @@ -3100,6 +3101,7 @@ export interface UiSettingsParams<T = unknown> {
requiresPageReload?: boolean;
// (undocumented)
schema: Type<T>;
sensitive?: boolean;
type?: UiSettingsType;
// (undocumented)
validation?: ImageValidation | StringValidation;
Expand Down
13 changes: 12 additions & 1 deletion src/core/server/ui_settings/routes/get.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,28 @@
* Public License, v 1.
*/

import { schema } from '@kbn/config-schema';
import { IRouter } from '../../http';
import { SavedObjectsErrorHelpers } from '../../saved_objects';

export function registerGetRoute(router: IRouter) {
router.get(
{ path: '/api/kibana/settings', validate: false },
{
path: '/api/kibana/settings',
validate: {
query: schema.object({
_allRegistered: schema.maybe(schema.boolean()),
Copy link
Member

@afharo afharo Jan 26, 2021

Choose a reason for hiding this comment

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

We are exposing this only for test purposes. What do you think about following a similar approach to the Application Usage's functional tests?

  1. A test plugin uses core's public APIs, and calls the existing method uiSettingsClient.getAll() and saves it in the window object.
  2. Then the mocha tests retrieve that property and validate their content.

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 agree exposing an API just for testing might not be ideal. I'm curious if anyone else from the team ( @elastic/kibana-core ) has different ideas.

Using the browser might be ok, even an api in the test plugin that exposes those settings might be even better too. Since api_integration tests dont open the browser and using an API is more reliable.

}),
},
},
async (context, request, response) => {
try {
const { _allRegistered } = request.query;
const uiSettingsClient = context.core.uiSettings.client;

return response.ok({
body: {
registered: _allRegistered ? uiSettingsClient.getRegistered() : undefined,
settings: await uiSettingsClient.getUserProvided(),
},
});
Expand Down
1 change: 1 addition & 0 deletions src/core/server/ui_settings/settings/notifications.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ export const getNotificationsSettings = (): Record<string, UiSettingsParams> =>
},
}),
category: ['notifications'],
sensitive: true,
schema: schema.string(),
},
'notifications:lifetime:banner': {
Expand Down
4 changes: 4 additions & 0 deletions src/core/server/ui_settings/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,10 @@ export interface IUiSettingsClient {
* Shows whether the uiSettings value set by the user.
*/
isOverridden: (key: string) => boolean;
/**
* Shows whether the uiSetting is a sensitive value. Used by telemetry to not send sensitive values.
*/
isSensitive: (key: string) => boolean;
}

/** @internal */
Expand Down
Loading