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] SAML login nextUrl drops URL hash #831

Closed
markj-db opened this issue Sep 23, 2021 · 6 comments
Closed

[BUG] SAML login nextUrl drops URL hash #831

markj-db opened this issue Sep 23, 2021 · 6 comments
Labels
bug Something isn't working

Comments

@markj-db
Copy link

Describe the bug
OpenSearch Dashboards encodes useful state in the hash component of the URL, for example:

https://<domain>/app/discover#/?_a=(columns:!(level,loggerCategory,messageText,shardName,tags.subDir,workspaceId))

Currently, the SAML component's nextUrl builder includes url.path but omits url.hash, dropping this state. This creates a problem when sharing links to a particular view without first creating a shortlink, in that the state of that view is lost. For the example above, the redirect would be:

https://<domain>/auth/saml/login?nextUrl=%2Fapp%2Fdiscover

and after authentication, the user would land at:

https://<domain>/app/discover

With no preselected columns.

Expected behavior
I believe the user expect to be redirected back to the exact URL, including hash, as they were at before the authentication redirect. I believe it would be preferable to include url.hash, so the nextUrl would become:

https://<domain>/auth/saml/login?nextUrl=%2Fapp%2Fdiscover%23%2F%3F_a%3D%28columns%3A%21%28level%2CloggerCategory%2CmessageText%2CshardName%2Ctags.subDir%2CworkspaceId%29%29

Additional context

@markj-db markj-db added Beta bug Something isn't working untriaged labels Sep 23, 2021
@mvanderlee
Copy link

mvanderlee commented Oct 27, 2021

The problem is more complicated than this. I build my own version that include the hash part.
Only to realize that the SAML auth and nextUrl building happens on the server.
This is very problematic because the server will never get the hash part. So it can not include it.

@mvanderlee
Copy link

Looks like a duplicate of #543

@mvanderlee
Copy link

mvanderlee commented Oct 27, 2021

Elasticsearch Kibana solved this by redirecting to a capture-url-fragment page first, which gets the url fragment and returns that back to server, and only then does it build the SAML redirect url.

Captured and included a har file that showcases this.
es_saml_fragment_capture.zip

@wkruse
Copy link

wkruse commented Jun 10, 2022

This seems like a general issue. The same happens when OpenID is used. The nextUrl parameter drops the URL hash. So for example if you have a dashboard open and your login session expires, you are redirected to /app/dashboards (Dashboards overview). Or if you have a visualization open and you login session expires, you are redirected to /app/visualize (Visualizations overview).

Also enabling state:storeInSessionStorage in the Advanced Settings does not help, at it changes the URL hash.

Similar issue #926.

After security dashboard plugin is enabled, the usability of OpenSearch Dashboards suffers a lot, as the end users do not understand, why they are redirected to unexpected locations after re-login.

@peternied
Copy link
Member

Thanks for commenting on this issue, there is a pull request to save the hash from the url on redirect, #1001 that I believe would address this issue as well as several impacted by the same root cause.

@davidlago
Copy link

This looks like it was successfully backported/released for 2.2, closing

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

No branches or pull requests

5 participants