Preserve URL Hash for SAML based login

[devardee]

Description

In Opensearch Dashboards URL Hash encodes the state of a visualization (state could be the filters to apply, the time range etc). Currently for SAML based authentication when User logins-for-the-first-time/re-authenticates the url hash is being dropped because of redirecting to the IDP. This PR ensures that URL Hash is preserved after authenticating with the IDP.

Old Closed PR for reference : #1001.

Category

Enhancement

Why these changes are required?

This PR ensures that state of visualization remains consistent after authenticating via IDP.

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

In the old behavior when the dashboard plugin redirects browser to IDP the hash is lost and User looses their unsaved work. In the new behavior, before redirecting to IDP the dashboard plugin will store the hash in browser's local storage, later after the User is authenticated the hash is restored from the browser's local storage.

Issues Resolved

#543 and #831

Testing

Intergation Testing:

• I have written selenium based integration test, which will simulate the Login Flow. I have not included it in this PR. Will raise a separate PR for that.

Manual Testing:

To Test the flow manually we need 3 servers.

1. Opensearch Server is running with the following settings in the yml file.

plugins.security.ssl.transport.pemkey_filepath: esnode-key.pem
plugins.security.ssl.transport.pemtrustedcas_filepath: root-ca.pem
plugins.security.ssl.transport.enforce_hostname_verification: false
plugins.security.ssl.http.enabled: true
plugins.security.ssl.http.pemcert_filepath: esnode.pem
plugins.security.ssl.http.pemkey_filepath: esnode-key.pem
plugins.security.ssl.http.pemtrustedcas_filepath: root-ca.pem
plugins.security.allow_unsafe_democertificates: true
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
plugins.security.restapi.roles_enabled: ["all_access", "security_rest_api_access"]
plugins.security.system_indices.enabled: true
plugins.security.system_indices.indices: [".plugins-ml-model", ".plugins-ml-task", ".opendistro-alerting-config", ".opendistro-alerting-alert*", ".opendistro-anomaly-results*", ".opendistro-anomaly-detector*", ".opendistro-anomaly-checkpoints", ".opendistro-anomaly-detection-state", ".opendistro-reports-*", ".opensearch-notifications-*", ".opensearch-notebooks", ".opensearch-observability", ".opendistro-asynchronous-search-response*", ".replication-metadata-store"]

2. For SAML IDP, Clone this repo and run the following commands in a separate terminal

openssl req -x509 -new -newkey rsa:2048 -nodes \
>     -subj '/C=US/ST=California/L=San Francisco/O=JankyCo/CN=Test Identity Provider' \
>     -keyout idp-private-key.pem \
>     -out idp-public-cert.pem -days 7300

node app.js --acsUrl http://localhost:5601/woq --audience https://localhost:9200

3. Now start the opensearch dashboard server with the following settings in the dashboards.yml file:

server.host: "localhost"
server.basePath: "/woq"
server.xsrf.whitelist: ["/_opendistro/_security/saml/acs/idpinitiated", "/_opendistro/_security/saml/acs",
                        "/_opendistro/_security/saml/logout"]
opensearch.hosts: ["https://localhost:9200"]
opensearch.ssl.verificationMode: none
opensearch.username: "kibanaserver"
opensearch.password: "kibanaserver"
opensearch.requestHeadersWhitelist: [ authorization,securitytenant ]
opensearch_security.multitenancy.enabled: true
opensearch_security.multitenancy.tenants.preferred: ["Private", "Global"]
opensearch_security.readonly_mode.roles: ["kibana_read_only"]
opensearch_security.auth.type: "saml"
opensearch_security.auth.anonymous_auth_enabled: true

# Use this setting if you are running opensearch-dashboards without https
opensearch_security.cookie.secure: false

4. Perform a PATCH request to Openserach Server to enable SAML

curl -X PATCH https://localhost:9200/_plugins/_security/api/securityconfig -H 'Content-Type: application/json' -H 'Accept: application/json' -d '[{"op": "add","path": "/config/dynamic/authc/saml_auth_domain","value": {"http_enabled": "true","transport_enabled": "false","order": 5,"http_authenticator": {"challenge": true,"type": "saml","config": {"idp": {"metadata_url": "http://localhost:7000/metadata","entity_id": "urn:example:idp"},"sp": {"entity_id": "https://localhost:9200"},"kibana_url": "http://localhost:5601/woq","exchange_key": "6aff3042-1327-4f3d-82f0-40a157ac4464"}}, "authentication_backend":{"type": "noop","config": {}}}}]' -k -u 'admin:admin'

5. [email protected] is the default username, that can be changed in the config of the IDP. PATCH Request to map all_access role to that user for testing purpose.

curl -X PATCH https://localhost:9200/_plugins/_security/api/rolesmapping/all_access -H 'Content-Type: application/json' -H 'Accept: application/json' -d '[{"op":"add", "path": "/users", "value": ["[email protected]"]}]'

6. Login as the user and download the sample data and verify if the hash is being preserved after re-logging/loginng-for-the-first-time.

HAR File:

SamlHashPreserve.har.zip

Video demonstrating the flow:

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.

[devardee]

cc: @varun-lodaya

[codecov-commenter]

Codecov Report

Merging #1039 (983e6f5) into main (cc5b763) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #1039   +/-   ##
=======================================
  Coverage   72.27%   72.27%           
=======================================
  Files          87       87           
  Lines        1915     1915           
  Branches      244      244           
=======================================
  Hits         1384     1384           
  Misses        478      478           
  Partials       53       53           

Help us with your feedback. Take ten seconds to tell us how you rate us.

[DarshitChanpura]

Great Addition! One small request, can you add one-liner comments to explain the functionality you added in routes.ts

[devardee]

@DarshitChanpura, I have added comments.

[peternied]

Thanks for this contribution - is there a reason you'd like to add the test cases in a separate PR? I'd like to merge this fix with its associated tests at once

[varun-lodaya]

peternied@ the plugin does not have a SAML integration test framework currently. Adding a new framework to handle redirects is very involved. devardee@ has started some work but it's taking longer than expected and we don't want to block this while waiting on the framework.

[peternied]

I know the manual steps were called out in the PR description, but we don't have the capacity or process to execute manual test cases, this change needs to wait until there is a way to validate it works.

Without such validation the functionality could easily be broken and we would not know why. There was a mention of trouble getting the tests up and running, please open a draft PR if you'd like to brainstorm on how to make progress in this space before adding those changes to this PR.

[peternied]

About the SAML testing, when we setup the cluster for integration testing a docker instance is used for OpenSearch. By expanding on the docker-compose example with a test SAML IdP this might be an easier to integrate into tests into the CI systems see link.

[cliu123]

Please remove this if not needed.

[devardee]

ack

[devardee]

@peternied , @DarshitChanpura and @cliu123 I have addressed the comments, can you please take another look

[peternied]

#1044 is nearly there, lets get those updates and include them in this PR so we can merge everything at once.

[peternied]

Merging, seperately we will see about getting the tests integrated