[Backport 1.3] Preserve URL Hash for SAML based login #1226

cwperks · 2022-11-23T16:18:09Z

Description

Backport of #1039, #1058, #1088 to 1.3

Note: This PR checks in yarn.lock to lock the version of dependencies so that yarn does not try to install the latest version of dependencies which may not be compatible with node 10.24.1. This specifically locks selenium-webdriver to 4.0.0-alpha.7 which is compatible with node 10.24.1. Without the lockfile, yarn resolves this to 4.6.1 and will not build due to node version compatibility because [email protected] requires node >= 12.

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.

* Refactor + add support to run saml based integ tests via selenium web driver Signed-off-by: Deepak Devarakonda <[email protected]> * Add plugins.security.unsupported.restapi.allow_securityconfig_modification in developer guide Signed-off-by: Deepak Devarakonda <[email protected]> * Add one more test Signed-off-by: Deepak Devarakonda <[email protected]> * Added tests for checking tenancy retention after logout in SAML Signed-off-by: Aniketh Jain <[email protected]> * Lint formatting fixes Signed-off-by: Aniketh Jain <[email protected]> * Removed unused imports Signed-off-by: Aniketh Jain <[email protected]> * Add plugins.security.unsupported.restapi.allow_securityconfig_modification in developer guide Signed-off-by: Deepak Devarakonda <[email protected]> * Added License header Signed-off-by: Aniketh Jain <[email protected]> * Added building the plugin bundles while running ITs Signed-off-by: Aniketh Jain <[email protected]> * Signed off the commit Removed a comment no longer required Signed-off-by: Aniketh Jain <[email protected]> * Added debug loggers for checking IT failures Signed-off-by: Aniketh Jain <[email protected]> * Added debug loggers for checking IT failures Signed-off-by: Aniketh Jain <[email protected]> * Added debug loggers for checking IT failures Signed-off-by: Aniketh Jain <[email protected]> * Added debug loggers for checking IT failures Signed-off-by: Aniketh Jain <[email protected]> * Added a new stage for debug loggers before cleanup Signed-off-by: Aniketh Jain <[email protected]> * Added a new stage for debug loggers before cleanup Signed-off-by: Aniketh Jain <[email protected]> * Added logger to print error recieved from auth info during saml login Signed-off-by: Aniketh Jain <[email protected]> * Added Docker host N/W Config to allow connection to SAML IDP Signed-off-by: Aniketh Jain <[email protected]> * Added discovery type config to be single node for passing bootstrap checks Signed-off-by: Aniketh Jain <[email protected]> * Debug loggers Signed-off-by: Aniketh Jain <[email protected]> * Debug loggers Signed-off-by: Aniketh Jain <[email protected]> * Debug loggers Signed-off-by: Aniketh Jain <[email protected]> * Reverted run command to see change in error Signed-off-by: Aniketh Jain <[email protected]> * Trying with full docker image of OS Signed-off-by: Aniketh Jain <[email protected]> * Refactored the integration test yaml to use OS Full Docker image Signed-off-by: Aniketh Jain <[email protected]> * Removed all debug loggers Signed-off-by: Aniketh Jain <[email protected]> * Added selfSigned package for generating certs and integrated with saml-idp Signed-off-by: Aniketh Jain <[email protected]> * Deleted checked-in key and cert for saml-idp server Signed-off-by: Aniketh Jain <[email protected]> * Reverted use of docker image and testing again with manual build Signed-off-by: Aniketh Jain <[email protected]> * Reverted use of docker image and testing again with manual build Signed-off-by: Aniketh Jain <[email protected]> * Upgraded version from 2.3 to 2.4 Signed-off-by: Aniketh Jain <[email protected]> * Removed debug pointers Signed-off-by: Aniketh Jain <[email protected]> * Commented out failing IT temporarily Signed-off-by: Aniketh Jain <[email protected]> * Lint formatting fix Signed-off-by: Aniketh Jain <[email protected]> * Added the commented failing test back again Signed-off-by: Aniketh Jain <[email protected]> * Removed assertion from test again to make it pass Signed-off-by: Aniketh Jain <[email protected]> * Used a better XPath and improved error logging in tests Signed-off-by: Aniketh Jain <[email protected]> * Removed an unused XPath Signed-off-by: Aniketh Jain <[email protected]> * Added back the assertion for failing IT Signed-off-by: Aniketh Jain <[email protected]> * Added steps to run Selenium based Integ Tests Signed-off-by: Aniketh Jain <[email protected]> * Commented out the test, will re-enable it again in the fix PR Signed-off-by: Aniketh Jain <[email protected]> * Parameterized the getDriver function Signed-off-by: Aniketh Jain <[email protected]> Signed-off-by: Deepak Devarakonda <[email protected]> Signed-off-by: Aniketh Jain <[email protected]> Co-authored-by: Deepak Devarakonda <[email protected]>

…flow (opensearch-project#1058) * Fix for picking up tenancy from local storage in SAML AuthN flow Signed-off-by: Aniketh Jain <[email protected]>

Signed-off-by: Craig Perkins <[email protected]>

…ecurity-dashboards-plugin into backport-saml-hash-fix

Signed-off-by: Craig Perkins <[email protected]>

…ecurity-dashboards-plugin into backport-saml-hash-fix

Signed-off-by: Craig Perkins <[email protected]>

* Preserve URL HASH after user logs via SAML IDP Co-authored-by: Darshit Chanpura <[email protected]>

cwperks · 2022-11-23T16:22:45Z

@scrawfor99 Thank you for the help with this backport! I took a look into the node compatibility issue and we are able to get around it by checking in a yarn.lock file to keep the dependency resolution predictable at build time.

This PR is a duplicate of #1220.

DarshitChanpura

LGTM!

DarshitChanpura · 2022-11-23T16:25:28Z

@scrawfor99 Thank you for the help with this backport! I took a look into the node compatibility issue and we are able to get around it by checking in a yarn.lock file to keep the dependency resolution predictable at build time.

This PR is a duplicate of #1220.

Should we leave this PR open or close it?

cwperks · 2022-11-23T16:29:42Z

FYI other dashboards plugins check in the yarn.lock file, but security-dashboards-plugin does not. Is there a reason why this repo does not checkin yarn.lock? I have not found any reason by searching through the issues of this repository.

See 3 other dashboards plugin repos that all contain yarn.lock:

cwperks · 2022-11-23T16:30:58Z

.github/workflows/integration-test.yml


      - name: Checkout OpenSearch Dashboard
        uses: actions/checkout@v2
        with:
          path: OpenSearch-Dashboards
          repository: opensearch-project/OpenSearch-Dashboards
-          ref: '1.3'
+          ref: '1.3.6'


This ref needs to be updated when a build becomes available.

cwperks · 2022-11-23T16:32:20Z

@DarshitChanpura I converted this to a Draft since there is a reference to the 1.3.6 tag in here. Once a build of 1.3.7 is available I will update the reference and change this to Ready for Review.

cwperks · 2022-11-23T16:33:40Z

yarn.lock

+  resolved "https://registry.yarnpkg.com/@types/scheduler/-/scheduler-0.16.2.tgz#1a62f89525723dde24ba1b01b092bf5df8ad4d39"
+  integrity sha512-hppQEBDmlwhFAXKJX2KnWLYu5yMfi91yazPb2l+lbJiwW+wdo1gNeRA+3RgNSO39WYX2euey41KEwnqesU2Jew==
+
+"@types/selenium-webdriver@^4.0.9":


This was manually added to yarn.lock to make sure there is a compatible version with node 10.24.1

cwperks · 2022-11-23T16:33:48Z

yarn.lock

+    ajv-errors "^1.0.0"
+    ajv-keywords "^3.1.0"
+
+selenium-webdriver@^4.0.0-alpha.7:


This was manually added to yarn.lock to make sure there is a compatible version with node 10.24.1

stephen-crawford · 2022-11-23T17:23:31Z

This all looks good. I think once we have the correct build an the DCO is fixed we should be all set.

Signed-off-by: Craig Perkins <[email protected]>

cwperks · 2022-11-23T17:45:46Z

All of the commits are signed. The cherry-picked commits or the merge commit may be causing the DCO failure.

Signed-off-by: Craig Perkins <[email protected]>

stephen-crawford · 2022-11-23T21:24:33Z

It looks like it is the merge commit.

stephen-crawford

This all looks good; we will just need to update the version like you said.

peternied · 2022-11-29T19:43:17Z

@cwperks Anything you are waiting on before we merge this change?

cwperks · 2022-11-29T19:53:18Z

@peternied Yes, I'm waiting on a 1.3.7 build of OSD so that I can update the ref here: #1226 (comment)

Signed-off-by: Craig Perkins <[email protected]>

rishabh6788 · 2022-12-01T01:55:30Z

@peternied @cwperks The OpenSearch and Dashboards distribution build was successful for 1.3.7. Re-ran the CI workflow and it is successful now.

cwperks · 2022-12-01T14:14:39Z

Thank you for the update @rishabh6788! Merging this change now.

expani and others added 20 commits November 21, 2022 15:33

Fix for Tenancy info getting lost on re-login in SAML Authentication …

53fb127

…flow (opensearch-project#1058) * Fix for picking up tenancy from local storage in SAML AuthN flow Signed-off-by: Aniketh Jain <[email protected]>

Add comma

adc2e80

Signed-off-by: Craig Perkins <[email protected]>

Remove comma after selfsigned

d2ca598

Signed-off-by: Craig Perkins <[email protected]>

Use saml-idp 1.1.0

00a2512

Signed-off-by: Craig Perkins <[email protected]>

Add resolutions in package.json

47daa15

Signed-off-by: Craig Perkins <[email protected]>

Use selenium-webdriver 3.6.0

4f3aa4b

Signed-off-by: Craig Perkins <[email protected]>

Use selenium-webdriver 4.4.0

5232362

Signed-off-by: Craig Perkins <[email protected]>

Use exact version

2458f08

Signed-off-by: Craig Perkins <[email protected]>

Add resolutions above

6186f80

Signed-off-by: Craig Perkins <[email protected]>

List contents of OpenSearch-Dashboards

c4e7110

Signed-off-by: Craig Perkins <[email protected]>

Set working directory

3ebb0c6

Signed-off-by: Craig Perkins <[email protected]>

Perform yarn install first to use lockfile

aba1ee5

Signed-off-by: Craig Perkins <[email protected]>

Add --frozen-lockfile

b20a003

Signed-off-by: Craig Perkins <[email protected]>

Try adding yarn.lock file

7ef6214

Signed-off-by: Craig Perkins <[email protected]>

Merge branch 'backport-saml-hash-fix' of https://github.com/cwperks/s…

0333474

…ecurity-dashboards-plugin into backport-saml-hash-fix

Add semi-colon

b766075

Signed-off-by: Craig Perkins <[email protected]>

Merge branch 'backport-saml-hash-fix' of https://github.com/cwperks/s…

249e477

…ecurity-dashboards-plugin into backport-saml-hash-fix

Update ref to 1.3.6 tag

b2c00f7

Signed-off-by: Craig Perkins <[email protected]>

Preserve URL Hash for SAML based login (opensearch-project#1039)

228dfba

* Preserve URL HASH after user logs via SAML IDP Co-authored-by: Darshit Chanpura <[email protected]>

cwperks requested a review from a team November 23, 2022 16:18

DarshitChanpura approved these changes Nov 23, 2022

View reviewed changes

cwperks marked this pull request as draft November 23, 2022 16:30

cwperks commented Nov 23, 2022

View reviewed changes

Explicitly setup geckodriver

9755d92

Signed-off-by: Craig Perkins <[email protected]>

Setup firefox

ebe57c2

Signed-off-by: Craig Perkins <[email protected]>

stephen-crawford mentioned this pull request Nov 23, 2022

[Backport 1.3] Preserve URL Hash for SAML based login with Tests #1220

Closed

3 tasks

stephen-crawford approved these changes Nov 23, 2022

View reviewed changes

peternied approved these changes Nov 29, 2022

View reviewed changes

cwperks added 2 commits November 30, 2022 16:18

Update ref to 1.3

47477a1

Signed-off-by: Craig Perkins <[email protected]>

Merge branch '1.3' into backport-saml-hash-fix

d211266

DarshitChanpura marked this pull request as ready for review November 30, 2022 21:37

cwperks mentioned this pull request Nov 30, 2022

Version bump to 1.3.7 #1233

Merged

3 tasks

cwperks merged commit c34dc55 into opensearch-project:1.3 Dec 1, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Backport 1.3] Preserve URL Hash for SAML based login #1226

[Backport 1.3] Preserve URL Hash for SAML based login #1226

cwperks commented Nov 23, 2022

cwperks commented Nov 23, 2022

DarshitChanpura left a comment

DarshitChanpura commented Nov 23, 2022

cwperks commented Nov 23, 2022

cwperks Nov 23, 2022

cwperks commented Nov 23, 2022

cwperks Nov 23, 2022

cwperks Nov 23, 2022

stephen-crawford commented Nov 23, 2022

cwperks commented Nov 23, 2022

stephen-crawford commented Nov 23, 2022

stephen-crawford left a comment

peternied commented Nov 29, 2022

cwperks commented Nov 29, 2022

rishabh6788 commented Dec 1, 2022

cwperks commented Dec 1, 2022

[Backport 1.3] Preserve URL Hash for SAML based login #1226

[Backport 1.3] Preserve URL Hash for SAML based login #1226

Conversation

cwperks commented Nov 23, 2022

Description

Check List

cwperks commented Nov 23, 2022

DarshitChanpura left a comment

Choose a reason for hiding this comment

DarshitChanpura commented Nov 23, 2022

cwperks commented Nov 23, 2022

cwperks Nov 23, 2022

Choose a reason for hiding this comment

cwperks commented Nov 23, 2022

cwperks Nov 23, 2022

Choose a reason for hiding this comment

cwperks Nov 23, 2022

Choose a reason for hiding this comment

stephen-crawford commented Nov 23, 2022

cwperks commented Nov 23, 2022

stephen-crawford commented Nov 23, 2022

stephen-crawford left a comment

Choose a reason for hiding this comment

peternied commented Nov 29, 2022

cwperks commented Nov 29, 2022

rishabh6788 commented Dec 1, 2022

cwperks commented Dec 1, 2022