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

[Security Solution] [Cypress] SAML for Serverless login and role testing #172655

Merged
merged 18 commits into from
Dec 13, 2023

Conversation

MadameSheema
Copy link
Member

@MadameSheema MadameSheema commented Dec 6, 2023

Relates to:

Summary

In this PR we are using the code implemented on #170417 and #172678 to allow SAML and role testing inside Cypress.

  • We are creating a Cypress task to use the above-developed code and be able to retrieve a session cookie given a role.
  • We updated the login task to know how we should perform the login depending if we are in Serverless (MKI or serverless FTR) or ESS
  • In the parallel serverless script:
    • We are updating the BASE_ENV_URL variable to use the proper QA environment (pending to be done in follow-up PRs, to extract this value so it is not hardcoded cc @dkirchan )
    • We are adding the IS_SERVERLESS environment variable needed for the logic on the login task. This changed implied to update the es_archiver file to continue work as expected.
    • We have added the TEST_CLOUD_HOST_NAME environment variable needed for the code we are reusing to retrieve the session cookie for MKI.
  • We have updated the Security Solution quality gate script to set the role_users.json file needed by the code we are reusing to get the different session cookies on MKI
  • We have adjusted the tests because the username now follows the pattern test <role> (@dmlemeshko is it possible to have as username just the role? Is this something that can impact other tests and teams?)
  • We have skipped a test that got unstable after the changes.

How to test it in your machine

Serverless FTR

  1. Navigate to x-pack/test/security_solution_cypress
  2. Execute yarn cypress:open:serverless
  3. Click on E2E testing
  4. Click on any test to execute it

Serverless MKI

Setup a valid Elastic Cloud API key for QA environment:

  1. Navigate to QA environment.
  2. Click on the User menu button located on the top right of the header.
  3. Click on Organization.
  4. Click on the API keys tab.
  5. Click on Create API key button.
  6. Add a name, set an expiration date, assign an organization owner role.
  7. Click on Create API key
  8. Save the value of the key

Store the saved key on ~/.elastic/cloud.json using the following format:

{
  "api_key": {
    "qa": "<API_KEY>"
  }
}

Store the email and password of the account you used to login in the QA Environment at the root directory of your Kibana project on .ftr/role_users.json, using the following format:

{
  "admin": {
    "email": "<email>",
    "password": "<password>"
  }
}

If you want to execute a test with a role different from the default one, make sure you have created the user under your organization and is added to the above json following the format:

{
  "admin": {
    "email": "<email>",
    "password": "<password>"
  },
  "<roleName>": {
    "email": "<email>",
    "password": "<password>"
  }
}
  1. Navigate to x-pack/test/security_solution_cypress
  2. Execute yarn cypress:open:qa:serverless
  3. Click on E2E testing
  4. Click on any test to execute it

Copy link
Member

@pheyos pheyos left a comment

Choose a reason for hiding this comment

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

As discussed in slack, we should try to use some shared functionality to handle all this SAML stuff. This will make it more stable and easier to maintain in the future. There's currently work going on to move the SAML test logic into the kbn-test package so it can also be used by Cypress tests, see #172678.
@MadameSheema is there a reason to NOT use the shared functionality?

@MadameSheema
Copy link
Member Author

As discussed in slack, we should try to use some shared functionality to handle all this SAML stuff. This will make it more stable and easier to maintain in the future. There's currently work going on to move the SAML test logic into the kbn-test package so it can also be used by Cypress tests, see #172678.
@MadameSheema is there a reason to NOT use the shared functionality?

Not at all :) In this draft PR I'm reusing the initial code Dima wrote, I was not aware of the kbn-test package effort when I opened this draft.

I already synced with @dmlemeshko and will wait for the above-mentioned PR to finalize the integration with Cypress.

@MadameSheema
Copy link
Member Author

/ci

@MadameSheema
Copy link
Member Author

/ci

@MadameSheema
Copy link
Member Author

/ci

@MadameSheema
Copy link
Member Author

/ci

@MadameSheema
Copy link
Member Author

/ci

@MadameSheema MadameSheema changed the title integrating SAML authentication in Cypress [Security Solution] [Cypress] SAML for Serverless login and role testing Dec 12, 2023
@MadameSheema MadameSheema self-assigned this Dec 12, 2023
@MadameSheema MadameSheema added release_note:skip Skip the PR/issue when compiling release notes Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. v8.13.0 labels Dec 12, 2023
@MadameSheema MadameSheema marked this pull request as ready for review December 12, 2023 16:05
@elasticmachine
Copy link
Contributor

Pinging @elastic/security-solution (Team: SecuritySolution)

@MadameSheema
Copy link
Member Author

/ci

Copy link
Contributor

@charlie-pichette charlie-pichette left a comment

Choose a reason for hiding this comment

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

Looks good. 👍🏼

@@ -571,6 +571,7 @@ ${JSON.stringify(cypressConfigFile, null, 2)}
KIBANA_PASSWORD: credentials.password,

CLOUD_SERVERLESS: true,
IS_SERVERLESS: true,
Copy link
Contributor

Choose a reason for hiding this comment

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

Cant we use only one of these two flags in lines 573 and 574? If this is not a big rework.

Copy link
Member Author

Choose a reason for hiding this comment

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

These flags represent two different things.

  • IS_SERVERLESS is used for both FTR and cloud environments.
  • CLOUD_SERVERLESS is used for just deployed projects on the cloud, this cannot be used for the FTR serverless environment.

Copy link
Contributor

@stephmilovic stephmilovic left a comment

Choose a reason for hiding this comment

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

LGTM for explore changes, thanks!

@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] FTR Configs #29 / discover Discover Saved Queries Manage saved queries updates a saved query
  • [job] [logs] Serverless Rule Management - Security Solution Cypress Tests #2 / Related integrations related Integrations Advanced Setting is disabled rules management table should not display a badge with the installed integrations should not display a badge with the installed integrations

Metrics [docs]

✅ unchanged

History

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

cc @MadameSheema

@@ -16,6 +16,9 @@ export JOB=kibana-security-solution-chrome

buildkite-agent meta-data set "${BUILDKITE_JOB_ID}_is_test_execution_step" "true"

mkdir .ftr
retry 5 5 vault kv get -format=json -field=data secret/kibana-issues/dev/security-quality-gate/role-users > .ftr/role_users.json
Copy link
Contributor

Choose a reason for hiding this comment

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

We recently implemented a change where we're accessing vault through a variable path. I think for this to be future-proof to some degree, we should also care for that branching in here. However, the utility function that we use cannot be parameterized with extra args in its current state.

If this is urgent, we can go with this.
If you have some time to chisel the edges, I'll follow up with the details, so you can add it to this PR.

Copy link
Member Author

Choose a reason for hiding this comment

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

@dkirchan can you please take a look at @delanni comment and estimate how long is going to take to make the changes? Thanks!

Copy link
Contributor

Choose a reason for hiding this comment

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

@delanni as discussed private, lets unblock this PR and I will add these changes as part of my PR that I am preparing: #173005

Copy link
Contributor

@delanni delanni left a comment

Choose a reason for hiding this comment

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

approved with suggested fix incoming in #173005

Copy link
Contributor

@vitaliidm vitaliidm left a comment

Choose a reason for hiding this comment

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

Detection Response area LGTM. Thanks for this PR implementation

Successfully log in by different roles, created for test qa depoloyment

Note: in PR description , when testing MKI deployment, script in second step should be

Execute yarn cypress:open:qa:serverless

Observation: most of the tests against MKI failed when I run it locally, Chrome just crashed

We detected that the Chrome Renderer process just crashed.

We have failed the current spec but will continue running the next spec.

This can happen for a number of different reasons.

If you're running lots of tests on a memory intense application.
  - Try increasing the CPU/memory on the machine you're running on.
  - Try enabling experimentalMemoryManagement in your config file.
  - Try lowering numTestsKeptInMemory in your config file during 'cypress open'.

You can learn more here:

https://on.cypress.io/renderer-process-crashed

Have you seen it before? Is it possibly related to this PR?

}
```

As role names please use:
Copy link
Contributor

Choose a reason for hiding this comment

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

should list of these role match ROLES enum?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is something I want to double check with @maximpn when he is back. The listed roles are the ones available on serverless and are the ones we can retrieve cookies from so I've the feeling we might be missing roles on that enum.

@MadameSheema
Copy link
Member Author

@vitaliidm you are right around the script!! thanks for noticing! about the failure, I didn't see it before.

@MadameSheema MadameSheema merged commit c580213 into elastic:main Dec 13, 2023
41 checks passed
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Dec 13, 2023
@MadameSheema MadameSheema deleted the saml-cypress-integration branch December 13, 2023 16:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting release_note:skip Skip the PR/issue when compiling release notes Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. v8.13.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.