Skip to content

Commit

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

Closes #172481

Fixed validation of secret output fields. Updated cypress tests that
validates output secrets.

Create a new remote elasticsearch output and verify that service token
field is required when clicking Save without filling it in.
<img width="673" alt="image"
src="https://github.com/elastic/kibana/assets/90178898/29774463-23de-495a-a1c7-49fd3281877c">

Same for Logstash ssl key:
<img width="678" alt="image"
src="https://github.com/elastic/kibana/assets/90178898/7b41d95f-8066-47d1-a662-2e696399f90c">

Kafka password and ssl key:
<img width="669" alt="image"
src="https://github.com/elastic/kibana/assets/90178898/2ea4dade-beb2-41e8-b068-6d6c42dca498">

In edit mode, when the secret field is empty, the validation is shown
again and goes away when clicking on Cancel changes.
<img width="680" alt="image"
src="https://github.com/elastic/kibana/assets/90178898/93500091-05a5-458c-a42e-97d5f8937d15">
<img width="667" alt="image"
src="https://github.com/elastic/kibana/assets/90178898/b7434126-bf20-40ce-9605-f63d694ea9ca">


### Checklist

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
  • Loading branch information
juliaElastic authored Dec 7, 2023
1 parent 8f7a12c commit a8f8f08
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 a8f8f08

Please sign in to comment.