SAML Integration Tests

[expani]

Description

Adds integration tests using selenium web driver for SAML Authentication flows

Category

Integration Tests

What is the old behavior before changes and new behavior after changes?

Integration test were not included.

Issues Resolved

Integration tests for fixes #1058 and #1039

Duplicate of the previous PR #1044

Testing

Integration tests are present in the file saml_auth.test.ts

Check List

• New functionality includes testing
• New functionality has been documented
• Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

[expani]

@peternied The integration tests keep failing because of an issue we have observed during testing as well.
FailedWorkflow

For local testing we performed the following steps :

• Start and stop the OpenSearch Dashboard Server using yarn start
• Run the integration tests using yarn test:jest_server

If we don't run yarn start after a fresh checkout, then the Selenium Web-driver throws up an error ( attached SS ) after logging in via IDP.

A naive solution is to start and stop the OpenSearch Dashboard Server before this

Could you share some insights on what exactly could be causing this ?

[expani]

I have identified the issue for the integration tests failing in Github Actions workflow.

Enabled verbose logging for OSD Server.

logging: {
  silent: false,
  verbose: true
}

Checked out the security-dashboard-plugin and ran yarn osd bootstrap

Ran the jest Integration tests using the command yarn test:jest_server with headless mode disabled for the Selenium web-driver.

When the tests ran, all the ITs failed as after SAML login the error page attached in previous comment was thrown. Upon checking the logs, I could see lot of calls to fetch plugin bundles were failing with the 404 response code.

Checked around OSD Server code base and found that build_opensearch_dashboards_platform_plugins gets invoked for generating these plugins when yarn start command is called.

So, ran the command node scripts/build_opensearch_dashboards_platform_plugins.js after yarn osd bootstrap before starting the Integration tests. The entire suite of selenium web-driver based tests ran without any hiccups.

In order, to make the same run in Github Actions, I think we can add the following line before this

node ./OpenSearch-Dashboards/scripts/build_opensearch_dashboards_platform_plugins.js

This will ensure that all integration tests using selenium web driver with security-dashboards-plugin work seamlessly in the future.

[codecov-commenter]

Codecov Report

Merging #1088 (f247dbb) into main (5d018b0) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #1088   +/-   ##
=======================================
  Coverage   72.43%   72.43%           
=======================================
  Files          88       88           
  Lines        1919     1919           
  Branches      246      246           
=======================================
  Hits         1390     1390           
  Misses        474      474           
  Partials       55       55           

Impacted Files	Coverage Δ
...-plugin/public/apps/account/account-nav-button.tsx	71.42% <0.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[cliu123]

Are \ns needed from line 117 - 122?

[expani]

These are added for debugging, will remove once the ITs succeed.

[cliu123]

Please remove if this is for debugging.

[expani]

These are added for debugging, will remove once the ITs succeed.

[cliu123]

Does securityAuthCookie need to be null here? The return value can be undefined.

[devardee]

yes, since null can valid value for securityAuthCookie

[cliu123]

Is the length of the cookie a constant here? alway equals to 2?

[devardee]

yes, one cookie for set by the idp and one cookie set by the security dashboard plugin. So in total there should be two cookies

[cwperks]

It looks like the confusion here is in the variable name cookie. The call to getCookies is returning all cookies that the browser encounters which is multiple cookies in this case. Perhaps we should rename this variable to cookies?

[peternied]

In order, to make the same run in Github Actions, I think we can add the following line before this
node ./OpenSearch-Dashboards/scripts/build_opensearch_dashboards_platform_plugins.js
This will ensure that all integration tests using selenium web driver with security-dashboards-plugin work seamlessly in the future.

Sure, if we need to update how the configuration is performed we should do so, it looks this added this to the current changes 👍

In the CI run it looks like there are some failures - it seems like there was several API call failures Recieved error from authinfo API StatusCodeError {... that might indicate there was a problem during the test setup

[peternied]

Let's generate these certs so we don't have to check them into the codebase

[peternied]

Doesn't this start the IDP before all jest tests, could we start it in the setup for the saml_auth.tests.ts?

[cwperks]

I'm curious as well, can we spin this up and tear it down as needed for a test or suite of tests?

[peternied]

This looks like debugging info, is this needed during a standard run to diagnose failures? If not could we remove it

[expani]

We will remove it post integration tests run successfully.

[peternied]

Previously there was mention of // 1 Integ Test to test Cookie expiry could you include that test as well?

[opensearch-trigger-bot]

The backport to 2.x failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-2.x 2.x
# Navigate to the new working tree
cd .worktrees/backport-2.x
# Create a new branch
git switch --create backport/backport-1088-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 a4fa35d0008b2652c3922f5f2fa814bfa79e0ff3
# Push it to GitHub
git push --set-upstream origin backport/backport-1088-to-2.x
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-2.x

Then, create a pull request where the base branch is 2.x and the compare/head branch is backport/backport-1088-to-2.x.

[cwperks]

Backport to 2.x failed - manually created backport PR: #1104

[cwperks]

Automatic backport failed because this PR did not get the backport 2.x label: #1039

[opensearch-trigger-bot]

The backport to 1.3 failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-1.3 1.3
# Navigate to the new working tree
cd .worktrees/backport-1.3
# Create a new branch
git switch --create backport/backport-1088-to-1.3
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 a4fa35d0008b2652c3922f5f2fa814bfa79e0ff3
# Push it to GitHub
git push --set-upstream origin backport/backport-1088-to-1.3
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-1.3

Then, create a pull request where the base branch is 1.3 and the compare/head branch is backport/backport-1088-to-1.3.