[Cloud Security] Added textarea secret fields for GCP JSON file #187022

animehart · 2024-06-26T20:27:44Z

Summary

Currently GCP Json Blob field might contain input variables that can be considered secrets, and we are not hiding it. As such user can see the value when previewing the integration after saving them.

This PR makes it so that the JSON Blob field acts like a Password field once user save them as in they won't be able to see it when editing it (they can only replace but not edit)

It also encrypt the content on Preview, preventing User from seeing it from the Preview.

Screen.Recording.2024-07-02.at.7.50.57.PM.mov

TODO:

Add Follow up ticket to change the manifest file, We need to change gcp.credentials.json secret value from false to true in the manifest [Cloud Security] change secret value for gcp json blob to true integrations#10479
When working on this issue, we realized that mocking Lazyloading component for jest seems to be problematic and we haven't been doing it correctly before, as such we are skipping all jest test that has lazy loading component in it and will address it on separate ticket ( we will need to decide on what would be the best way to proceed ) [Debt] Need to fix tests that uses Lazy loaded component #187930

elasticmachine · 2024-06-27T04:08:23Z

Pinging @elastic/kibana-cloud-security-posture (Team:Cloud Security)

elasticmachine · 2024-06-27T04:08:45Z

Pinging @elastic/fleet (Team:Fleet)

…on separate ticket probably

…xtarea-secret

kibana-ci · 2024-06-27T23:25:08Z

💛 Build succeeded, but was flaky

Buildkite Build
Commit: 3d32c85

Failed CI Steps

Metrics [docs]

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id	before	after	diff
`fleet`	1226	1227	+1

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`cloudSecurityPosture`	450.4KB	450.9KB	+485.0B
`fleet`	1.4MB	1.4MB	+25.0B
total			+510.0B

Unknown metric groups

API count

id	before	after	diff
`fleet`	1348	1349	+1

ESLint disabled line counts

id	before	after	diff
`fleet`	45	46	+1

Total ESLint disabled count

id	before	after	diff
`fleet`	58	59	+1

History

💔 Build #218102 failed 29b30d7

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

JordanSh

can you add a video of the component when creating new and then when editing?

JordanSh · 2024-07-02T10:21:33Z

...gins/cloud_security_posture/public/components/fleet_extensions/policy_template_form.test.tsx

+      // const credentialsJsonField = queryByTestId(
+      //   CIS_GCP_INPUT_FIELDS_TEST_SUBJECTS.CREDENTIALS_JSON
+      // );
+      // await waitFor(() =>
+      //   expect(getByTestId(CIS_GCP_INPUT_FIELDS_TEST_SUBJECTS.CREDENTIALS_JSON)).toBeInTheDocument()
+      // );


remove this if its not needed

woops
i missed this

JordanSh · 2024-07-02T10:30:14Z

...rity_posture/public/components/fleet_extensions/gcp_credentials_form/gcp_credential_form.tsx

+                <LazyPackagePolicyInputVarField
+                  data-test-subj={CIS_GCP_INPUT_FIELDS_TEST_SUBJECTS.CREDENTIALS_JSON}
+                  varDef={{
+                    ...findVariableDef(packageInfo, credentialJSONFields.id)!,


using this assertion is a red flag, i suggest to handle the situation of when its missing so we can actually know its there instead if asserting that

we did this same thing with Azure and AWS secret component, can't quite remember how we ended up with this assertion

JordanSh · 2024-07-02T10:34:42Z

...rity_posture/public/components/fleet_extensions/gcp_credentials_form/gcp_credential_form.tsx

+                  onChange={(value) => {
+                    onChange(credentialJSONFields.id, value);
+                  }}
+                  errors={[]}


im not familiar with this component, is this on purpose? what does this effects?

this error props is basically being used on EuiFormRow component inside this component

JordanSh · 2024-07-02T10:36:43Z

...rity_posture/public/components/fleet_extensions/gcp_credentials_form/gcp_credential_form.tsx

+                  }}
+                  errors={[]}
+                  forceShowErrors={false}
+                  isEditPage={true}


should this always be set to true? shouldn't it be conditional, using our isEdit parameter?

juliaElastic

Fleet change LGTM

opauloh

overall looks ok, just got some questions before approving

opauloh · 2024-07-09T16:39:38Z

...gins/cloud_security_posture/public/components/fleet_extensions/policy_template_form.test.tsx

-        CIS_GCP_INPUT_FIELDS_TEST_SUBJECTS.CREDENTIALS_JSON
-      );
+
+      screen.logTestingPlaygroundURL();


better remove this log, even if the test is skipped

opauloh · 2024-07-09T16:42:30Z

...cy/create_package_policy_page/components/steps/components/package_policy_input_var_field.tsx

@@ -185,7 +185,8 @@ function getInputComponent({
  fieldTestSelector,
  setIsDirty,
 }: InputComponentProps) {
-  const { multi, type, options } = varDef;
+  // eslint-disable-next-line @typescript-eslint/naming-convention
+  const { multi, type, options, full_width } = varDef;


would that work instead of disabling the rule?

const { multi, type, options, full_width : fullWidth } = varDef;

opauloh · 2024-07-09T16:45:37Z

...gins/cloud_security_posture/public/components/fleet_extensions/policy_template_form.test.tsx

@@ -629,7 +637,7 @@ describe('<CspPolicyTemplateForm />', () => {
      });
    });

-    it(`renders ${CLOUDBEAT_EKS} Direct Access Keys fields`, async () => {
+    it.skip(`renders ${CLOUDBEAT_EKS} Direct Access Keys fields`, async () => {


why all the tests are skipped?

opauloh · 2024-07-09T16:46:45Z

...gins/cloud_security_posture/public/components/fleet_extensions/policy_template_form.test.tsx

@@ -1577,7 +1541,7 @@ describe('<CspPolicyTemplateForm />', () => {
      expect(setupTechnologySelector).toBeInTheDocument();
      expect(setupTechnologySelector).toHaveTextContent(/agentless/i);
      expect(orgIdField).toBeInTheDocument();
-      expect(credentialsJsonField).toBeInTheDocument();
+      // expect(credentialsJsonField).toBeInTheDocument();


the LazyPackagePolicyInputVarField mock at the beginning of the file suggests this assertion should not be commented

opauloh · 2024-07-09T16:56:20Z

...ck/test/cloud_security_posture_functional/pages/cis_integrations/cspm/cis_integration_gcp.ts

-        await cisIntegration.navigateToIntegrationCspList();
-        expect(
-          (await cisIntegration.getFieldValueInEditPage(CREDENTIALS_JSON_TEST_ID)) ===
-            credentialJsonName
-        ).to.be(true);


I think instead of just removing the assertion, we should test that it's really a secret field, when going to the edit page, we shouldn't be able to see the credential anywhere. Instead, we should be able to see the following message: "The saved secret access key is hidden."

opauloh · 2024-07-09T16:56:56Z

...ck/test/cloud_security_posture_functional/pages/cis_integrations/cspm/cis_integration_gcp.ts

-        await cisIntegration.navigateToIntegrationCspList();
-        expect(
-          (await cisIntegration.getFieldValueInEditPage(CREDENTIALS_JSON_TEST_ID)) ===
-            credentialJsonName
-        ).to.be(true);


same comment are before

elasticmachine · 2024-07-10T02:28:09Z

💚 Build Succeeded

Buildkite Build
Commit: 114e42a

Metrics [docs]

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id	before	after	diff
`fleet`	1226	1227	+1

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`cloudSecurityPosture`	452.9KB	453.4KB	+522.0B
`fleet`	1.8MB	1.8MB	+25.0B
total			+547.0B

Unknown metric groups

API count

id	before	after	diff
`fleet`	1348	1349	+1

History

💛 Build #220487 was flaky b0ce7a8
💚 Build #220117 succeeded 698c114
💚 Build #219269 succeeded 8cde8ad

opauloh

LGTM - Just need to link the tech debt for removing the tests skips to this PR

animehart added 2 commits June 26, 2024 13:15

added secret text area

d36dbf7

added property to handle full width text area

1249962

animehart marked this pull request as ready for review June 26, 2024 21:04

animehart requested review from a team as code owners June 26, 2024 21:04

animehart added v8.15.0 Team:Cloud Security Cloud Security team related labels Jun 27, 2024

animehart added the release_note:skip Skip the PR/issue when compiling release notes label Jun 27, 2024

Merge branch 'main' into textarea-secret

29b30d7

botelastic bot added the Team:Fleet Team label for Observability Data Collection Fleet team label Jun 27, 2024

animehart added 2 commits June 27, 2024 14:53

skipping test thats affected by lazy load, will need to address this …

a8ba211

…on separate ticket probably

Merge branch 'textarea-secret' of github.com:animehart/kibana into te…

3d32c85

…xtarea-secret

JordanSh requested changes Jul 2, 2024

View reviewed changes

pr comments pt 1

8cde8ad

animehart requested a review from JordanSh July 4, 2024 09:29

JordanSh approved these changes Jul 4, 2024

View reviewed changes

juliaElastic approved these changes Jul 5, 2024

View reviewed changes

animehart added 2 commits July 6, 2024 03:20

fix conflict

18d90dd

Merge branch 'main' into textarea-secret

698c114

animehart added the v8.16.0 label Jul 9, 2024

opauloh reviewed Jul 9, 2024

View reviewed changes

animehart removed the v8.15.0 label Jul 9, 2024

animehart added 3 commits July 9, 2024 15:36

ftr update

b0ce7a8

reverting jest

16de911

fix conflict

114e42a

animehart mentioned this pull request Jul 10, 2024

[Debt] Need to fix tests that uses Lazy loaded component #187930

Open

opauloh approved these changes Jul 10, 2024

View reviewed changes

animehart merged commit 0e62867 into elastic:main Jul 10, 2024
21 checks passed

kibanamachine added the backport:skip This commit does not require backporting label Jul 10, 2024

animehart mentioned this pull request Jul 12, 2024

[Cloud Security] change secret value for gcp json blob to true elastic/integrations#10479

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Cloud Security] Added textarea secret fields for GCP JSON file #187022

[Cloud Security] Added textarea secret fields for GCP JSON file #187022

animehart commented Jun 26, 2024 •

edited

Loading

elasticmachine commented Jun 27, 2024

elasticmachine commented Jun 27, 2024

kibana-ci commented Jun 27, 2024

API count

ESLint disabled line counts

Total ESLint disabled count

JordanSh left a comment

JordanSh Jul 2, 2024

animehart Jul 2, 2024

JordanSh Jul 2, 2024

animehart Jul 4, 2024

JordanSh Jul 2, 2024

animehart Jul 2, 2024 •

edited

Loading

JordanSh Jul 2, 2024

juliaElastic left a comment

opauloh left a comment

opauloh Jul 9, 2024

opauloh Jul 9, 2024

opauloh Jul 9, 2024

opauloh Jul 9, 2024

opauloh Jul 9, 2024

opauloh Jul 9, 2024

elasticmachine commented Jul 10, 2024

API count

opauloh left a comment

[Cloud Security] Added textarea secret fields for GCP JSON file #187022

[Cloud Security] Added textarea secret fields for GCP JSON file #187022

Conversation

animehart commented Jun 26, 2024 • edited Loading

Summary

elasticmachine commented Jun 27, 2024

elasticmachine commented Jun 27, 2024

kibana-ci commented Jun 27, 2024

💛 Build succeeded, but was flaky

Failed CI Steps

Metrics [docs]

Public APIs missing comments

Async chunks

API count

ESLint disabled line counts

Total ESLint disabled count

History

JordanSh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

animehart Jul 2, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

juliaElastic left a comment

Choose a reason for hiding this comment

opauloh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elasticmachine commented Jul 10, 2024

💚 Build Succeeded

Metrics [docs]

Public APIs missing comments

Async chunks

API count

History

opauloh left a comment

Choose a reason for hiding this comment

animehart commented Jun 26, 2024 •

edited

Loading

animehart Jul 2, 2024 •

edited

Loading