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] 1.3.12 Short URL raises 400 error during SAML login #1627

Closed
bgoerzig opened this issue Oct 21, 2023 · 4 comments · Fixed by #1744
Closed

[BUG] 1.3.12 Short URL raises 400 error during SAML login #1627

bgoerzig opened this issue Oct 21, 2023 · 4 comments · Fixed by #1744
Assignees
Labels
bug Something isn't working help wanted Extra attention is needed, need help from community triaged

Comments

@bgoerzig
Copy link

bgoerzig commented Oct 21, 2023

What is the bug?

When opening a short URL pointing to a SAML-enabled OSD instance the SAML login flow won't be initiated and clients will receive the following response:

{"statusCode":400,"error":"Bad Request","message":"[request query.security_tenant]: definition for this key is missing"}

What's interesting is that the nextURL won't include the HTTP-encoded security_tenant parameter.
Instead, an empty security_tenant parameter will be included in the request itself.

For example, opening the short URL

http://localhost:5601/goto/3eb6108934e0f3208a0f7f0afe0c69c5?security_tenant=global

will redirect the client to

http://localhost:5601/goto/3eb6108934e0f3208a0f7f0afe0c69c5?security_tenant=global
auth/saml/captureUrlFragment?nextUrl=%2Fgoto%2F3eb6108934e0f3208a0f7f0afe0c69c5&security_tenant=

Verbose flow using curl:

$ curl 'http://localhost:5601/goto/3a572e1e320b3486e9708efa9dce0a2f?security_tenant=global' -vL
*   Trying 127.0.0.1:5601...
* Connected to localhost (127.0.0.1) port 5601 (#0)
> GET /goto/3a572e1e320b3486e9708efa9dce0a2f?security_tenant=global HTTP/1.1
> Host: localhost:5601
> User-Agent: curl/8.0.1
> Accept: */*
> 
< HTTP/1.1 302 Found
< location: /auth/saml/captureUrlFragment?nextUrl=%2Fgoto%2F3a572e1e320b3486e9708efa9dce0a2f&security_tenant=
< set-cookie: security_authentication=; Max-Age=0; Expires=Thu, 01 Jan 1970 00:00:00 GMT; HttpOnly; Path=/
< set-cookie: security_authentication=; Max-Age=0; Expires=Thu, 01 Jan 1970 00:00:00 GMT; HttpOnly; Path=/
< osd-name: kibana-deployment-5584d77474-dgwzp
< cache-control: private, no-cache, no-store, must-revalidate
< content-length: 0
< Date: Sat, 21 Oct 2023 10:03:33 GMT
< Connection: keep-alive
< 
* Connection #0 to host localhost left intact
* Issue another request to this URL: 'http://localhost:5601/auth/saml/captureUrlFragment?nextUrl=%2Fgoto%2F3a572e1e320b3486e9708efa9dce0a2f&security_tenant='
* Found bundle for host: 0x10a4100 [serially]
* Can not multiplex, even if we wanted to
* Re-using existing connection #0 with host localhost
> GET /auth/saml/captureUrlFragment?nextUrl=%2Fgoto%2F3a572e1e320b3486e9708efa9dce0a2f&security_tenant= HTTP/1.1
> Host: localhost:5601
> User-Agent: curl/8.0.1
> Accept: */*
> 
< HTTP/1.1 400 Bad Request
< osd-name: kibana-deployment-5584d77474-dgwzp
< content-type: application/json; charset=utf-8
< cache-control: private, no-cache, no-store, must-revalidate
< content-length: 120
< Date: Sat, 21 Oct 2023 10:03:33 GMT
< Connection: keep-alive
< 
* Connection #0 to host localhost left intact
{"statusCode":400,"error":"Bad Request","message":"[request query.security_tenant]: definition for this key is missing"}

How can one reproduce the bug?

Steps to reproduce the behavior:

  1. Set up a SAML enabled OSD instance
  2. Log in to the instance
  3. Create a short URL in the global tenant
  4. Log out of your OSD instance
  5. Open the short URL
  6. See error

What is the expected behavior?

I expected the SAML login flow to be triggered with the correct nextURL.

What is your host/environment?

  • Client: macOS Sonoma 14.0, Chrome 118.0.5993.88 (arm64), curl 8.0.1 (x86_64-koji-linux-gnu)
  • Server: Kubernetes 1.25.12, workers using gardenlinux 934.10.0, pods using opensearchproject/opensearch-dashboards:1.3.12 (Amazon Linux 2)
  • Plugins: Security Dashboards Plugin

Do you have any screenshots?
Not applicable

Do you have any additional context?

Obviously, authenticated clients will not be affected. One can work around the issue by HTTP-encoding the security_tenant parameter and appending it to the value of nextURL to properly trigger the SAML login flow manually:

$ curl -vL 'http://localhost:5601/auth/saml/captureUrlFragment?nextUrl=%2Fgoto%2F3eb6108934e0f3208a0f7f0afe0c69c5%3Fsecurity_tenant%3Dglobal'
*   Trying 127.0.0.1:5601...
* Connected to localhost (127.0.0.1) port 5601 (#0)
> GET /auth/saml/captureUrlFragment?nextUrl=%2Fgoto%2F3eb6108934e0f3208a0f7f0afe0c69c5%3Fsecurity_tenant%3Dglobal HTTP/1.1
> Host: localhost:5601
> User-Agent: curl/8.0.1
> Accept: */*
> 
< HTTP/1.1 200 OK
< content-type: text/html; charset=utf-8
< content-security-policy: script-src 'unsafe-eval' 'self'; worker-src blob: 'self'; style-src 'unsafe-inline' 'self'
< osd-name: kibana-deployment-5584d77474-dgwzp
< cache-control: private, no-cache, no-store, must-revalidate
< set-cookie: security_authentication=; Max-Age=0; Expires=Thu, 01 Jan 1970 00:00:00 GMT; HttpOnly; Path=/
< content-length: 196
< accept-ranges: bytes
< Date: Sat, 21 Oct 2023 10:27:19 GMT
< Connection: keep-alive
< 

            <!DOCTYPE html>
            <title>OSD SAML Capture</title>
            <link rel="icon" href="data:,">
            <script src="/auth/saml/captureUrlFragment.js"></script>
* Connection #0 to host localhost left intact
          

I have a hunch that the generateNextURL function simply drops all parameters because it only takes into account the request.url.pathname without parameters: https://github.com/opensearch-project/security-dashboards-plugin/blob/1.3.12.0/server/auth/types/saml/saml_auth.ts#L54-L59

This could be wrong though because the problem seems to affect short URLs only (or any tenant-scoped resource, I didn't test this yet). I couldn't test this claim though because I had problems running the unit tests for the project.

@bgoerzig bgoerzig added bug Something isn't working untriaged labels Oct 21, 2023
@bgoerzig bgoerzig changed the title [BUG] 1.13.12 Short URL raises 400 error during SAML login [BUG] 1.3.12 Short URL raises 400 error during SAML login Oct 21, 2023
@cwperks cwperks added triaged help wanted Extra attention is needed, need help from community and removed untriaged labels Oct 23, 2023
@cwperks
Copy link
Member

cwperks commented Oct 23, 2023

[Triage] @bgoerzig Thank you for filing this issue and providing rich description of the issue to reproduce. I have marked this as triaged and help wanted.

@devardee
Copy link
Contributor

I can fix this

@adrian-golawski-eliatra

@davidlago, could you, please, assign the task to me?

@MioOgbeni
Copy link

MioOgbeni commented Dec 13, 2023

Hi, I also encountered this bug at AWS OSS 2.8.0. At the same time, I'm using another AWS OSS 2.8.0 instance, but with OIDC authentication, where opening shortened links works. Looking at the code, it looks like a quick fix.

Why SAML auth use own generateNextUrl method, when OpenID auth use composeNextUrlQeuryParam function from global utils?

Can it be rewrited to this global utils function, or not? It seems to work with this function.

EDIT: Sorry, I looked more closely and find that OpenID implementation was rewrited in this PR to same as SAML have. And yes I also encountered bug from this PR at OpenID auth. 😄 It seems that both SAML and OpenID must be now fixed on master branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed, need help from community triaged
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants