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

SAML Integration Tests #1088

Merged
merged 46 commits into from
Sep 19, 2022
Merged
Show file tree
Hide file tree
Changes from 44 commits
Commits
Show all changes
46 commits
Select commit Hold shift + click to select a range
861c909
Refactor + add support to run saml based integ tests via selenium web…
devardee Jul 19, 2022
c42dacc
Add plugins.security.unsupported.restapi.allow_securityconfig_modific…
devardee Jul 19, 2022
7415746
Add one more test
devardee Aug 30, 2022
a1afaa5
Added tests for checking tenancy retention after logout in SAML
expani Aug 30, 2022
381de0c
Lint formatting fixes
expani Aug 30, 2022
69c765f
Removed unused imports
expani Aug 30, 2022
2234f77
Add plugins.security.unsupported.restapi.allow_securityconfig_modific…
devardee Jul 19, 2022
776e134
Added License header
expani Aug 30, 2022
4e73818
Added building the plugin bundles while running ITs
expani Sep 1, 2022
d68808a
Merge remote-tracking branch 'upstream/main' into saml_integ_tests_ra…
expani Sep 1, 2022
1a641ba
Signed off the commit
expani Sep 1, 2022
016087c
Added debug loggers for checking IT failures
expani Sep 1, 2022
4c3561a
Added debug loggers for checking IT failures
expani Sep 1, 2022
5c486b4
Added debug loggers for checking IT failures
expani Sep 1, 2022
fc133ae
Added debug loggers for checking IT failures
expani Sep 1, 2022
a39fa6f
Added a new stage for debug loggers before cleanup
expani Sep 1, 2022
15f7483
Added a new stage for debug loggers before cleanup
expani Sep 1, 2022
07688fa
Added logger to print error recieved from auth info during saml login
expani Sep 1, 2022
a0338d4
Added Docker host N/W Config to allow connection to SAML IDP
expani Sep 2, 2022
ad54b42
Added discovery type config to be single node for passing bootstrap c…
expani Sep 5, 2022
abff13a
Debug loggers
expani Sep 5, 2022
60f97f4
Debug loggers
expani Sep 5, 2022
85492d9
Debug loggers
expani Sep 5, 2022
6423819
Reverted run command to see change in error
expani Sep 5, 2022
11dbee5
Trying with full docker image of OS
expani Sep 5, 2022
8062541
Refactored the integration test yaml to use OS Full Docker image
expani Sep 5, 2022
dc52652
Removed all debug loggers
expani Sep 5, 2022
e07ffb9
Added selfSigned package for generating certs and integrated with sam…
expani Sep 5, 2022
9fa7fbf
Deleted checked-in key and cert for saml-idp server
expani Sep 5, 2022
edf19e3
Reverted use of docker image and testing again with manual build
expani Sep 9, 2022
73d5251
Reverted use of docker image and testing again with manual build
expani Sep 9, 2022
ce0708b
Merge remote-tracking branch 'upstream/main' into saml_integ_tests_ra…
expani Sep 9, 2022
ae2f818
Upgraded version from 2.3 to 2.4
expani Sep 9, 2022
9b45c10
Removed debug pointers
expani Sep 9, 2022
22a1ac2
Commented out failing IT temporarily
expani Sep 13, 2022
cd10d35
Rebased with upstream main and removed comment for Lint tests to pass
expani Sep 14, 2022
89950cc
Lint formatting fix
expani Sep 14, 2022
4c696e4
Added the commented failing test back again
expani Sep 14, 2022
8253163
Merge remote-tracking branch 'upstream/main' into saml_integ_tests_ra…
expani Sep 15, 2022
51ad38a
Removed assertion from test again to make it pass
expani Sep 15, 2022
847bc2d
Used a better XPath and improved error logging in tests
expani Sep 15, 2022
c10cf2c
Removed an unused XPath
expani Sep 15, 2022
9af274a
Added back the assertion for failing IT
expani Sep 15, 2022
f6d8032
Added steps to run Selenium based Integ Tests
expani Sep 15, 2022
f247dbb
Commented out the test, will re-enable it again in the fix PR
expani Sep 17, 2022
fba014d
Parameterized the getDriver function
expani Sep 18, 2022
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .github/workflows/integration-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ jobs:
WORKDIR /opensearch/
ENTRYPOINT /docker-host/os-ep.sh
EOF
docker run -d -p 9200:9200 -p 9600:9600 -i opensearch-test:latest
docker run -d --network=host -i opensearch-test:latest

- name: Checkout OpenSearch Dashboard
uses: actions/checkout@v2
Expand Down Expand Up @@ -103,6 +103,7 @@ jobs:
run: |
cd ./OpenSearch-Dashboards
yarn osd bootstrap
node scripts/build_opensearch_dashboards_platform_plugins.js

- name: Run integration tests
run: |
Expand Down
3 changes: 3 additions & 0 deletions DEVELOPER_GUIDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ plugins.security.allow_default_init_securityindex: true
plugins.security.authcz.admin_dn:
- CN=kirk,OU=client,O=client,L=test, C=de

plugins.security.unsupported.restapi.allow_securityconfig_modification: true
plugins.security.audit.type: internal_opensearch
plugins.security.enable_snapshot_restore_privilege: true
plugins.security.check_snapshot_restore_write_privileges: true
Expand Down Expand Up @@ -117,6 +118,8 @@ Next, go to the base directory and run `yarn osd bootstrap` to install any addit

Now, from the base directory and run `yarn start`. This should start dashboard UI successfully. `Cmd+click` the url in the console output (It should look something like `http://0:5601/omf`). Once the page loads, you should be able to log in with user `admin` and password `admin`.

To run selenium based integration tests, download and export the firefox web-driver to your PATH. Also, run `node scripts/build_opensearch_dashboards_platform_plugins.js` or `yarn start` before running the tests. This is essential to generate the bundles.

## Submitting Changes

See [CONTRIBUTING](CONTRIBUTING.md).
Expand Down
12 changes: 8 additions & 4 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,19 +17,23 @@
"lint:es": "node ../../scripts/eslint",
"lint:style": "node ../../scripts/stylelint",
"lint": "yarn run lint:es && yarn run lint:style",
"pretest:jest_server": "node ./test/jest_integration/runIdpServer.js &",
Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Member

Choose a reason for hiding this comment

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

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

"test:jest_server": "node ./test/run_jest_tests.js --config ./test/jest.config.server.js",
"test:jest_ui": "node ./test/run_jest_tests.js --config ./test/jest.config.ui.js"
},
"devDependencies": {
"@elastic/eslint-import-resolver-kibana": "link:../../packages/osd-eslint-import-resolver-opensearch-dashboards",
"typescript": "4.0.2",
"gulp-rename": "2.0.0",
"@testing-library/react-hooks": "^7.0.2",
"@types/hapi__wreck": "^15.0.1"
"@types/hapi__wreck": "^15.0.1",
"gulp-rename": "2.0.0",
"saml-idp": "^1.2.1",
"selenium-webdriver": "^4.0.0-alpha.7",
Copy link
Member

Choose a reason for hiding this comment

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

@peterzhuamazon @bbarani Are there any concerns on building selenium into release?

Copy link
Member

Choose a reason for hiding this comment

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

@anijain-Amazon Would it be possible to add these tests in opensearch-dashboards-functional-test and implement with Cypress? There has already been a well-automated workflow in place where developers can easily implement tests and the tests run with each build. With the workflow, developers do not have to worry about setting up clusters and configurations. They can just focus on implementing tests and the workflow takes care of everything else. In comparison, Selenium would be a brand new test framework that would require some changes in Infra for release build.

Copy link
Contributor

Choose a reason for hiding this comment

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

@cliu123 currently cypress does not support saml auth workflow cypress-io/cypress#5397, so we went with selenium

Copy link
Member

Choose a reason for hiding this comment

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

I see. Thanks for the information!
@kavilla @peterzhuamazon Please be aware of this new test dependency Selenium.

"selfsigned": "^2.0.1",
"typescript": "4.0.2"
},
"dependencies": {
"@hapi/wreck": "^17.1.0",
"@hapi/cryptiles": "5.0.0",
"@hapi/wreck": "^17.1.0",
"html-entities": "1.3.1"
}
}
8 changes: 6 additions & 2 deletions public/apps/account/account-nav-button.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,11 @@ export function AccountNavButton(props: {
<EuiListGroupItem
color="subdued"
key="tenant"
label={<EuiText size="xs">{resolveTenantName(props.tenant || '', username)}</EuiText>}
label={
<EuiText size="xs" id="tenantName">
{resolveTenantName(props.tenant || '', username)}
</EuiText>
}
/>
</EuiFlexItem>
</EuiFlexGroup>
Expand Down Expand Up @@ -140,7 +144,7 @@ export function AccountNavButton(props: {
</div>
);
return (
<EuiHeaderSectionItemButton>
<EuiHeaderSectionItemButton id="user-icon-btn">
<EuiPopover
data-test-subj="account-popover"
id="actionsMenu"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`Account navigation button renders 1`] = `
<EuiHeaderSectionItemButton>
<EuiHeaderSectionItemButton
id="user-icon-btn"
>
<EuiPopover
anchorPosition="downCenter"
button={
Expand Down Expand Up @@ -63,6 +65,7 @@ exports[`Account navigation button renders 1`] = `
key="tenant"
label={
<EuiText
id="tenantName"
size="xs"
>
tenant1
Expand Down
2 changes: 2 additions & 0 deletions server/auth/types/saml/saml_auth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ export class SamlAuthentication extends AuthenticationType {
return escape(path);
}

// Check if we can get the previous tenant information from the expired cookie.
private redirectSAMlCapture = (request: OpenSearchDashboardsRequest, toolkit: AuthToolkit) => {
const nextUrl = this.generateNextUrl(request);
const clearOldVersionCookie = clearOldVersionCookieValue(this.config);
Expand Down Expand Up @@ -97,6 +98,7 @@ export class SamlAuthentication extends AuthenticationType {
};
}

// Can be improved to check if the token is expiring.
async isValidCookie(cookie: SecuritySessionCookie): Promise<boolean> {
return (
cookie.authType === this.type &&
Expand Down
1 change: 1 addition & 0 deletions server/backend/opensearch_security_client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,7 @@ export class SecurityClient {
// location="https://<your-auth-domain.com>/api/saml2/v1/sso?SAMLRequest=<some-encoded-string>"
// requestId="<request_id>"
// '

if (!error.wwwAuthenticateDirective) {
throw error;
}
Expand Down
2 changes: 1 addition & 1 deletion test/helper/cookie.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import { AUTHORIZATION_HEADER_NAME } from '../constant';

export function extractAuthCookie(response: Response) {
const setCookieHeaders = response.header['set-cookie'] as string[];
let securityAuthCookie: string;
let securityAuthCookie: string | null = null;
Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Contributor

Choose a reason for hiding this comment

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

yes, since null can valid value for securityAuthCookie

for (const setCookie of setCookieHeaders) {
if (setCookie.startsWith('security_authentication=')) {
securityAuthCookie = setCookie.split(';')[0];
Expand Down
32 changes: 32 additions & 0 deletions test/jest_integration/runIdpServer.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
/*
* Copyright OpenSearch Contributors
*
* Licensed under the Apache License, Version 2.0 (the "License").
* You may not use this file except in compliance with the License.
* A copy of the License is located at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* or in the "license" file accompanying this file. This file is distributed
* on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
* express or implied. See the License for the specific language governing
* permissions and limitations under the License.
*/

const { runServer } = require('saml-idp');

const { generate } = require('selfsigned');

const pems = generate(null, {
keySize: 2048,
clientCertificateCN: '/C=US/ST=California/L=San Francisco/O=JankyCo/CN=Test Identity Provider',
days: 7300,
});

// Create certificate pair on the fly and pass it to runServer
runServer({
acsUrl: 'http://localhost:5601/_opendistro/_security/saml/acs',
audience: 'https://localhost:9200',
cert: pems.cert,
key: pems.private.toString().replace(/\r\n/, '\n'),
});
Loading