Skip to content

Commit

Permalink
[8.12] [Fleet] fix validation of output secret fields (#172795) (#172812
Browse files Browse the repository at this point in the history
)

# Backport

This will backport the following commits from `main` to `8.12`:
- [[Fleet] fix validation of output secret fields
(#172795)](#172795)

<!--- Backport version: 8.9.7 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Julia
Bardi","email":"[email protected]"},"sourceCommit":{"committedDate":"2023-12-07T13:14:56Z","message":"[Fleet]
fix validation of output secret fields (#172795)\n\n##
Summary\r\n\r\nCloses
https://github.com/elastic/kibana/issues/172481\r\n\r\nFixed validation
of secret output fields. Updated cypress tests that\r\nvalidates output
secrets.\r\n\r\nCreate a new remote elasticsearch output and verify that
service token\r\nfield is required when clicking Save without filling it
in.\r\n<img width=\"673\"
alt=\"image\"\r\nsrc=\"https://github.com/elastic/kibana/assets/90178898/29774463-23de-495a-a1c7-49fd3281877c\">\r\n\r\nSame
for Logstash ssl key:\r\n<img width=\"678\"
alt=\"image\"\r\nsrc=\"https://github.com/elastic/kibana/assets/90178898/7b41d95f-8066-47d1-a662-2e696399f90c\">\r\n\r\nKafka
password and ssl key:\r\n<img width=\"669\"
alt=\"image\"\r\nsrc=\"https://github.com/elastic/kibana/assets/90178898/2ea4dade-beb2-41e8-b068-6d6c42dca498\">\r\n\r\nIn
edit mode, when the secret field is empty, the validation is
shown\r\nagain and goes away when clicking on Cancel changes.\r\n<img
width=\"680\"
alt=\"image\"\r\nsrc=\"https://github.com/elastic/kibana/assets/90178898/93500091-05a5-458c-a42e-97d5f8937d15\">\r\n<img
width=\"667\"
alt=\"image\"\r\nsrc=\"https://github.com/elastic/kibana/assets/90178898/b7434126-bf20-40ce-9605-f63d694ea9ca\">\r\n\r\n\r\n###
Checklist\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common
scenarios","sha":"a8f8f087d3ba622970ff9075beab2a1693e3a7d3","branchLabelMapping":{"^v8.13.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","Team:Fleet","v8.12.0","v8.13.0"],"number":172795,"url":"https://github.com/elastic/kibana/pull/172795","mergeCommit":{"message":"[Fleet]
fix validation of output secret fields (#172795)\n\n##
Summary\r\n\r\nCloses
https://github.com/elastic/kibana/issues/172481\r\n\r\nFixed validation
of secret output fields. Updated cypress tests that\r\nvalidates output
secrets.\r\n\r\nCreate a new remote elasticsearch output and verify that
service token\r\nfield is required when clicking Save without filling it
in.\r\n<img width=\"673\"
alt=\"image\"\r\nsrc=\"https://github.com/elastic/kibana/assets/90178898/29774463-23de-495a-a1c7-49fd3281877c\">\r\n\r\nSame
for Logstash ssl key:\r\n<img width=\"678\"
alt=\"image\"\r\nsrc=\"https://github.com/elastic/kibana/assets/90178898/7b41d95f-8066-47d1-a662-2e696399f90c\">\r\n\r\nKafka
password and ssl key:\r\n<img width=\"669\"
alt=\"image\"\r\nsrc=\"https://github.com/elastic/kibana/assets/90178898/2ea4dade-beb2-41e8-b068-6d6c42dca498\">\r\n\r\nIn
edit mode, when the secret field is empty, the validation is
shown\r\nagain and goes away when clicking on Cancel changes.\r\n<img
width=\"680\"
alt=\"image\"\r\nsrc=\"https://github.com/elastic/kibana/assets/90178898/93500091-05a5-458c-a42e-97d5f8937d15\">\r\n<img
width=\"667\"
alt=\"image\"\r\nsrc=\"https://github.com/elastic/kibana/assets/90178898/b7434126-bf20-40ce-9605-f63d694ea9ca\">\r\n\r\n\r\n###
Checklist\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common
scenarios","sha":"a8f8f087d3ba622970ff9075beab2a1693e3a7d3"}},"sourceBranch":"main","suggestedTargetBranches":["8.12"],"targetPullRequestStates":[{"branch":"8.12","label":"v8.12.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.13.0","labelRegex":"^v8.13.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/172795","number":172795,"mergeCommit":{"message":"[Fleet]
fix validation of output secret fields (#172795)\n\n##
Summary\r\n\r\nCloses
https://github.com/elastic/kibana/issues/172481\r\n\r\nFixed validation
of secret output fields. Updated cypress tests that\r\nvalidates output
secrets.\r\n\r\nCreate a new remote elasticsearch output and verify that
service token\r\nfield is required when clicking Save without filling it
in.\r\n<img width=\"673\"
alt=\"image\"\r\nsrc=\"https://github.com/elastic/kibana/assets/90178898/29774463-23de-495a-a1c7-49fd3281877c\">\r\n\r\nSame
for Logstash ssl key:\r\n<img width=\"678\"
alt=\"image\"\r\nsrc=\"https://github.com/elastic/kibana/assets/90178898/7b41d95f-8066-47d1-a662-2e696399f90c\">\r\n\r\nKafka
password and ssl key:\r\n<img width=\"669\"
alt=\"image\"\r\nsrc=\"https://github.com/elastic/kibana/assets/90178898/2ea4dade-beb2-41e8-b068-6d6c42dca498\">\r\n\r\nIn
edit mode, when the secret field is empty, the validation is
shown\r\nagain and goes away when clicking on Cancel changes.\r\n<img
width=\"680\"
alt=\"image\"\r\nsrc=\"https://github.com/elastic/kibana/assets/90178898/93500091-05a5-458c-a42e-97d5f8937d15\">\r\n<img
width=\"667\"
alt=\"image\"\r\nsrc=\"https://github.com/elastic/kibana/assets/90178898/b7434126-bf20-40ce-9605-f63d694ea9ca\">\r\n\r\n\r\n###
Checklist\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common
scenarios","sha":"a8f8f087d3ba622970ff9075beab2a1693e3a7d3"}}]}]
BACKPORT-->

Co-authored-by: Julia Bardi <[email protected]>
  • Loading branch information
kibanamachine and juliaElastic authored Dec 7, 2023
1 parent 7837193 commit 33884c2
Show file tree
Hide file tree
Showing 8 changed files with 118 additions and 9 deletions.
79 changes: 77 additions & 2 deletions x-pack/plugins/fleet/cypress/e2e/fleet_settings_outputs.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import {
loadESOutput,
loadKafkaOutput,
loadLogstashOutput,
loadRemoteESOutput,
resetKafkaOutputForm,
selectESOutput,
selectKafkaOutput,
Expand Down Expand Up @@ -134,6 +135,80 @@ describe('Outputs', () => {
});
});

describe('Remote ES', () => {
it('displays proper error messages', () => {
selectRemoteESOutput();
cy.getBySel(SETTINGS_SAVE_BTN).click();

cy.contains('Name is required');
cy.contains('URL is required');
cy.contains('Service Token is required');
shouldDisplayError(SETTINGS_OUTPUTS.NAME_INPUT);
shouldDisplayError('serviceTokenSecretInput');
});

describe('Form submit', () => {
let outputId: string;

before(() => {
interceptOutputId((id) => {
outputId = id;
});
});

after(() => {
cleanupOutput(outputId);
});

it('saves the output', () => {
selectRemoteESOutput();

cy.getBySel(SETTINGS_OUTPUTS.NAME_INPUT).type('name');
cy.get('[placeholder="Specify host URL"').clear().type('https://localhost:5000');
cy.getBySel('serviceTokenSecretInput').type('service_token');

cy.intercept('POST', '**/api/fleet/outputs').as('saveOutput');

cy.getBySel(SETTINGS_SAVE_BTN).click();

cy.wait('@saveOutput').then((interception) => {
const responseBody = interception.response?.body;
cy.visit(`/app/fleet/settings/outputs/${responseBody?.item?.id}`);
expect(responseBody?.item.service_token).to.equal(undefined);
expect(responseBody?.item.secrets.service_token.id).not.to.equal(undefined);
});

cy.get('[placeholder="Specify host URL"').should('have.value', 'https://localhost:5000');
cy.getBySel(SETTINGS_OUTPUTS.NAME_INPUT).should('have.value', 'name');
});
});

describe('Form edit', () => {
let outputId: string;

before(() => {
loadRemoteESOutput().then((data) => {
outputId = data.item.id;
});
});
after(() => {
cleanupOutput(outputId);
});

it('edits the output', () => {
visit(`/app/fleet/settings/outputs/${outputId}`);

cy.get('[placeholder="Specify host URL"').clear().type('https://localhost:5001');

cy.getBySel(SETTINGS_SAVE_BTN).click();
cy.getBySel(SETTINGS_CONFIRM_MODAL_BTN).click();
visit(`/app/fleet/settings/outputs/${outputId}`);

cy.get('[placeholder="Specify host URL"').should('have.value', 'https://localhost:5001');
});
});
});

describe('Kafka', () => {
describe('Form validation', () => {
it('renders all form fields', () => {
Expand Down Expand Up @@ -301,7 +376,7 @@ describe('Outputs', () => {
cy.contains('Name is required');
cy.contains('Host is required');
cy.contains('Username is required');
// cy.contains('Password is required');
cy.contains('Password is required');
cy.contains('Default topic is required');
cy.contains('Topic is required');
cy.contains(
Expand All @@ -310,7 +385,7 @@ describe('Outputs', () => {
cy.contains('Must be a key, value pair i.e. "http.response.code: 200"');
shouldDisplayError(SETTINGS_OUTPUTS.NAME_INPUT);
shouldDisplayError(SETTINGS_OUTPUTS_KAFKA.AUTHENTICATION_USERNAME_INPUT);
// shouldDisplayError(SETTINGS_OUTPUTS_KAFKA.AUTHENTICATION_PASSWORD_INPUT); // TODO
shouldDisplayError(SETTINGS_OUTPUTS_KAFKA.AUTHENTICATION_PASSWORD_INPUT);
shouldDisplayError(SETTINGS_OUTPUTS_KAFKA.TOPICS_DEFAULT_TOPIC_INPUT);
shouldDisplayError(SETTINGS_OUTPUTS_KAFKA.TOPICS_CONDITION_INPUT);
shouldDisplayError(SETTINGS_OUTPUTS_KAFKA.TOPICS_TOPIC_INPUT);
Expand Down
11 changes: 11 additions & 0 deletions x-pack/plugins/fleet/cypress/screens/fleet_outputs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,17 @@ export const loadESOutput = () =>
hosts: ['https://bla.co'],
});

export const loadRemoteESOutput = () =>
loadOutput({
name: 'remote_es',
type: 'remote_elasticsearch',
is_default: false,
is_default_monitoring: false,
hosts: ['https://bla.co'],
secrets: { service_token: 'token' },
preset: 'balanced',
});

export const loadLogstashOutput = () =>
loadOutput({
name: 'ls',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,7 @@ export const EditOutputFlyout: React.FunctionComponent<EditOutputFlyoutProps> =
})}
{...inputs.sslKeySecretInput.formRowProps}
onUsePlainText={onUsePlainText}
cancelEdit={inputs.sslKeySecretInput.cancelEdit}
>
<EuiTextArea
fullWidth
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,7 @@ export const OutputFormKafkaAuthentication: React.FunctionComponent<{
)}
{...inputs.kafkaSslKeySecretInput.formRowProps}
onUsePlainText={onUsePlainText}
cancelEdit={inputs.kafkaSslKeySecretInput.cancelEdit}
>
<EuiTextArea
fullWidth
Expand Down Expand Up @@ -247,6 +248,7 @@ export const OutputFormKafkaAuthentication: React.FunctionComponent<{
)}
{...inputs.kafkaAuthPasswordSecretInput.formRowProps}
onUsePlainText={onUsePlainText}
cancelEdit={inputs.kafkaAuthPasswordSecretInput.cancelEdit}
>
<EuiFieldPassword
type={'dual'}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ export const OutputFormRemoteEsSection: React.FunctionComponent<Props> = (props)
defaultMessage: 'Service Token',
})}
{...inputs.serviceTokenSecretInput.formRowProps}
cancelEdit={inputs.serviceTokenSecretInput.cancelEdit}
onUsePlainText={onUsePlainText}
>
<EuiFieldText
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ describe('SecretFormRow', () => {
const initialValue = 'initial value';
const clear = jest.fn();
const onUsePlainText = jest.fn();
const cancelEdit = jest.fn();

it('should switch to edit mode when the replace button is clicked', () => {
const { getByText, queryByText, container } = render(
Expand All @@ -23,6 +24,7 @@ describe('SecretFormRow', () => {
initialValue={initialValue}
clear={clear}
onUsePlainText={onUsePlainText}
cancelEdit={cancelEdit}
>
<input id="myinput" type="text" value={initialValue} />
</SecretFormRow>
Expand All @@ -38,13 +40,14 @@ describe('SecretFormRow', () => {
expect(queryByText(initialValue)).not.toBeInTheDocument();
});

it('should call the clear function when the cancel button is clicked', () => {
it('should call the cancelEdit function when the cancel button is clicked', () => {
const { getByText } = render(
<SecretFormRow
title={title}
initialValue={initialValue}
clear={clear}
onUsePlainText={onUsePlainText}
cancelEdit={cancelEdit}
>
<input type="text" value={initialValue} />
</SecretFormRow>
Expand All @@ -53,12 +56,17 @@ describe('SecretFormRow', () => {
fireEvent.click(getByText('Replace Test Secret'));
fireEvent.click(getByText('Cancel Test Secret change'));

expect(clear).toHaveBeenCalled();
expect(cancelEdit).toHaveBeenCalled();
});

it('should call the onUsePlainText function when the revert link is clicked', () => {
const { getByText } = render(
<SecretFormRow title={title} clear={clear} onUsePlainText={onUsePlainText}>
<SecretFormRow
title={title}
clear={clear}
onUsePlainText={onUsePlainText}
cancelEdit={cancelEdit}
>
<input type="text" />
</SecretFormRow>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,18 @@ export const SecretFormRow: React.FC<{
clear: () => void;
initialValue?: any;
onUsePlainText: () => void;
}> = ({ fullWidth, error, isInvalid, children, clear, title, initialValue, onUsePlainText }) => {
cancelEdit: () => void;
}> = ({
fullWidth,
error,
isInvalid,
children,
clear,
title,
initialValue,
onUsePlainText,
cancelEdit,
}) => {
const hasInitialValue = initialValue !== undefined;
const [editMode, setEditMode] = useState(!initialValue);
const valueHiddenPanel = (
Expand Down Expand Up @@ -66,7 +77,7 @@ export const SecretFormRow: React.FC<{
<EuiButtonEmpty
onClick={() => {
setEditMode(false);
clear();
cancelEdit();
}}
color="primary"
iconType="refresh"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,11 @@ import { safeLoad } from 'js-yaml';
const toSecretValidator =
(validator: (value: string) => string[] | undefined) =>
(value: string | { id: string } | undefined) => {
if (!value || typeof value === 'object') {
if (typeof value === 'object') {
return undefined;
}

return validator(value);
return validator(value ?? '');
};

export function validateKafkaHosts(value: string[]) {
Expand Down

0 comments on commit 33884c2

Please sign in to comment.