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

[BUG] OpenID Connect login nextUrl drops URL hash #1141

Closed
wkruse opened this issue Oct 14, 2022 · 4 comments
Closed

[BUG] OpenID Connect login nextUrl drops URL hash #1141

wkruse opened this issue Oct 14, 2022 · 4 comments
Assignees
Labels
bug Something isn't working triaged

Comments

@wkruse
Copy link

wkruse commented Oct 14, 2022

What is the bug?
In Opensearch Dashboards URL hash encodes the state of a visualization (state could be the filters to apply, the time range etc). Currently for OpenID Connect based authentication when user logins/re-logins the url hash is being dropped because of redirecting to the IDP.

How can one reproduce the bug?

Steps to reproduce the behavior:

  1. Setup OpenID Connect based authentication
  2. Open any dashboard
  3. Copy the URL
  4. Logout
  5. Open new private window
  6. Paste the URL
  7. You are redirected to /app/dashboards (dashboards overview)

What is the expected behavior?
The state of the dashboard/visualization must not be lost.

What is your host/environment?

  • Version 2.3.0

Do you have any additional context?
The same issue for SAML was fixed.

@wkruse wkruse added bug Something isn't working untriaged labels Oct 14, 2022
@stephen-crawford
Copy link
Contributor

[TRIAGE] @cliu123 tagging you for follow-up with Jimish. Added to sprint backlog for reviewing the provided fix and then adapting the pull request to match new issue.

@davidlago
Copy link

@cwperks please add details to criteria including fixing Cypress in our repo to be able to fully test this change

@cwperks cwperks self-assigned this Aug 22, 2023
@cwperks
Copy link
Member

cwperks commented Aug 22, 2023

There's a few associated items that will help to make changes in the OIDC authentication flow with confidence. There are unit tests in the security repo around the OIDC Authenticator and unit tests in this repo with the OIDC flow, but there are no integration tests that test the flows with a sample OIDC Identity Provider. For SAML, there are integration tests in this repo that spin up a node-based IdP and use selenium to test common use-cases:

  1. Login to app/opensearch_dashboards_overview#/ when SAML is enabled
  2. Login to app/dev_tools#/console when SAML is enabled
  3. Login to Dashboard with Hash
  4. Tenancy persisted after Logout in SAML

There are no equivalent tests for OIDC, but there should be and an issue was created to track the work: #1150

Function tests for dashboards-plugins are typically written with Cypress in the opensearch-dashboards-functional-test repo and the selenium tests in this repo should migrate to the FTR (functional test repository) and be referenced in a github action like this: https://github.com/opensearch-project/security-dashboards-plugin/blob/main/.github/workflows/cypress-test.yml

There is one issue with Cypress in the FTR repo now. The FTR repo is on CypressV9 which does not support cross origin testing. There is a PR in the FTR repo to upgrade Cypress to V12 but it is still a WIP to identify tests that need to be updated: opensearch-project/opensearch-dashboards-functional-test#668

Work has been done in the FTR repo to add OIDC tests, but it is blocked by the CypressV12 upgrade: opensearch-project/opensearch-dashboards-functional-test#595

@derek-ho
Copy link
Collaborator

Closing - was fixed in: #1563

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working triaged
Projects
None yet
Development

No branches or pull requests

6 participants