argoproj / argo-workflows

Workflow Engine for Kubernetes
https://argo-workflows.readthedocs.io/
Apache License 2.0
15.11k stars 3.21k forks source link

fix(sso): use relative redirect URLs. Fixes #13031 #13747

Open MasonM opened 1 month ago

MasonM commented 1 month ago

Fixes #13031

Motivation

When running Argo in plain text mode behind a TLS termination proxy with SSO enabled, you will always be redirected to the workflows page after logging in, regardless of which page you came from. This is happening because server/auth/sso/sso.go is using the scheme and host of the request to determine the redirect URL, which can be different than the original request when a proxy is involved.

Modifications

This is an alternative to https://github.com/argoproj/argo-workflows/pull/13609. @agilgur5 mentioned in https://github.com/argoproj/argo-workflows/pull/13609#discussion_r1781536758 that might be susceptible to open redirects. I can't think of any attack vector where that can happen that wasn't already a viable attack vector, but regardless, this side-steps the whole issue by switching the frontend to using relative redirect URLs (e.g. /workflows) instead of absolute URLs, which removes the need to verify the scheme and host of the URL.

Specifically, it uses what the WHATWG URL standard calls path-absolute-URL strings (which, despite the name, are actually relative URL). This is something that oauth2-proxy also supports, and I copied the validation logic and tests it uses to the backend:

oauth2-proxy is MIT licensed, which is compatible with Apache 2.0, so there shouldn't be any licensing concerns.

The validation logic is hairy because of subtleties with how browsers handle absolute URLs. The article Handling Relative URLs for Redirects / Forwards goes into more detail.

Verification

Testing instructions:

  1. Run make start UI_SECURE=true PROFILE=sso
  2. Visit https://localhost:8080/cluster-workflow-templates
  3. Click the "Login" button
  4. Click "Log in with Example"
  5. Click "Grant Access"
  6. Confirm you're returned to https://localhost:8080/cluster-workflow-templates

https://github.com/user-attachments/assets/6c544d8a-7806-48fe-94fb-9b1f1b3417cb

agilgur5 commented 1 month ago

This is an alternative to #13609. @agilgur5 mentioned in #13609 (comment) that might be susceptible to open redirects. I can't think of any attack vector where that can happen that wasn't already a viable attack vector, but regardless, this side-steps the whole issue by switching the frontend to using relative redirect URLs (e.g. /workflows) instead of absolute URLs, which removes the need to verify the scheme and host of the URL.

I had thought about making the UI use relative URLs too to avoid open redirects entirely, which is something I've done in past jobs before too (~2015ish). I'm not sure if the previous UI logic was exploitable client-side (as in, other than server-side proxy discussed in the linked thread), but I did have a low priority todo to check that

So this approach ostensibly makes sense to me, but I need to carefully check the SSO code in particular