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.

Sign up for GitHub

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

Jump to bottom

Incorrect redirect after Login when terminating TLS with reverse proxy #13031

Open
4 tasks done
ryancurrah opened this issue May 10, 2024 · 4 comments · May be fixed by #13609 or #13747
Open
4 tasks done

Incorrect redirect after Login when terminating TLS with reverse proxy #13031

ryancurrah opened this issue May 10, 2024 · 4 comments · May be fixed by #13609 or #13747
Assignees
@MasonM
Labels
area/server area/sso-rbac P2 Important. All bugs with >=3 thumbs up that aren’t P0 or P1, plus: Any other bugs deemed important solution/workaround There's a workaround, might not be great, but exists type/bug

Comments

@ryancurrah
Copy link
Contributor

ryancurrah commented May 10, 2024 •
edited
Loading

Pre-requisites

  • I have double-checked my configuration
  • I have tested with the :latest image tag (i.e. quay.io/argoproj/workflow-controller:latest) and can confirm the issue still exists on :latest. If not, I have explained why, in detail, in my description below.
  • I have searched existing issues and could not find a match for this bug
  • I'd like to contribute the fix myself (see contributing guide)

What happened/what did you expect to happen?

Description:

When clicking an Argo Workflows link, if I'm not logged in, I'm redirected to the login page. After logging in, I'm redirected to the workflows page instead of the original page I was trying to access.

Steps to reproduce:

  1. Click an Argo Workflows link to a specific page
  2. If not logged in, click the "Login" button
  3. After logging in, observe that you are redirected to the workflows page (not the page you intended to visit)

Debugging information:

Expected behavior:

After logging in, the user should be redirected to the original project they were trying to access, not the last viewed project.

Version: Argo Workflows v3.5.5

Version

v3.5.5

Paste a small workflow that reproduces the issue. We must be able to run the workflow; don't enter a workflows that uses private images.

n/a

Logs from the workflow controller

kubectl logs -n argo deploy/workflow-controller | grep ${workflow}

Logs from in your workflow's wait container

kubectl logs -n argo -c wait -l workflows.argoproj.io/workflow=${workflow},workflow.argoproj.io/phase!=Succeeded
@ryancurrah ryancurrah changed the title Users Not Returned to Original Page After Login Users Not Returned to Original Page After Login When Using Reverse Proxy and No TLS May 10, 2024
@ryancurrah ryancurrah changed the title Users Not Returned to Original Page After Login When Using Reverse Proxy and No TLS Users Not Returned to Original Page After Login When Using Reverse Proxy with No TLS Configured May 10, 2024
@agilgur5 agilgur5 changed the title Users Not Returned to Original Page After Login When Using Reverse Proxy with No TLS Configured Incorrect redirect after Login with terminating TLS with reverse proxy May 10, 2024
@agilgur5 agilgur5 changed the title Incorrect redirect after Login with terminating TLS with reverse proxy Incorrect redirect after Login when terminating TLS with reverse proxy May 10, 2024
@agilgur5
Copy link

agilgur5 commented May 11, 2024 •
edited
Loading

Follow-up to this Slack thread

We could potentially use the request's protocol to determine this.

But secure: false has other implications, like the Cookie setting and potentially others that we should be careful around and ensure properly handle ingress TLS termination.

EDIT: Here's another redirect line that depends on it that should be changed too.

@agilgur5 agilgur5 added the P3 Low priority label May 11, 2024
@ejsuncy
Copy link

ejsuncy commented Jun 27, 2024

Thanks for opening this ticket from my slack help request! subscribing for updates...

@agilgur5 agilgur5 added the solution/workaround There's a workaround, might not be great, but exists label Jun 27, 2024
@agilgur5
Copy link

Workaround is to use a self-signed cert on the Server with secure: true, per the Slack thread

@Joibel Joibel mentioned this issue Jul 1, 2024
Closed
4 tasks
@agilgur5 agilgur5 added P2 Important. All bugs with >=3 thumbs up that aren’t P0 or P1, plus: Any other bugs deemed important and removed P3 Low priority labels Jul 2, 2024
@bmooso

This comment was marked as spam.

Sign in to view
MasonM added a commit to MasonM/argo-workflows that referenced this issue Sep 16, 2024
@MasonM
build(ui): support TLS for UI and Argo Server
78d52c4 
This adds a new `make start UI_SECURE=true` flag that can be used to
test Argo with a TLS termination proxy in front, which is needed for argoproj#13031

Also, `make start SECURE=true UI=true` was broken because
`ui/src/app/webpack.config.js` wasn't respecting the `ARGO_SECURE` flag,
so I fixed that too.

Signed-off-by: Mason Malone <[email protected]>
MasonM added a commit to MasonM/argo-workflows that referenced this issue Sep 16, 2024
@MasonM
fix(sso): respect X-Forwarded-* headers for redirect URL. Fixes argop…
75854e5 
…roj#13031

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 protocol and host of the
request to determine the redirect URL, which can be different than the
original request when a proxy is used.

`oauth2-proxy` had a similar issue, which they fixed in
oauth2-proxy/oauth2-proxy#957 by reading from
the
[X-Forwarded-Host](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/X-Forwarded-Host)
and
[X-Forwarded-Proto](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/X-Forwarded-Proto)
headers, if set. This applies the same fix. The only major difference is
that `oauth2-proxy` only reads those headers when the `--reverse-proxy`
option is set, but I didn't add a corresponding option here. The only
reason I can think of for doing so is to preserve
backwards-compatibility, since it's theoretically possible this could
break someone's setup. I can't think of any real-world situations where
that can happen, but let me know if I should put this behind a option
and I'm happy to do so.

Tested using `make start UI_SECURE=true PROFILE=sso` and changing this
line: https://github.com/argoproj/argo-workflows/blob/29ea3518a30b2b605290d0ba38f95bfb75a2901f/manifests/quick-start/sso/overlays/workflow-controller-configmap.yaml#L15
to `redirectUrl: https://localhost:8080/oauth2/callback`. I tried to see
if I could automate this, but I couldn't figure out a clean way of doing
so.

Signed-off-by: Mason Malone <[email protected]>
@MasonM MasonM linked a pull request Sep 16, 2024 that will close this issue
Open
MasonM added a commit to MasonM/argo-workflows that referenced this issue Sep 28, 2024
@MasonM
fix(sso): respect X-Forwarded-* headers for redirect URL. Fixes argop…
649d561 
…roj#13031

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 protocol and host of the
request to determine the redirect URL, which can be different than the
original request when a proxy is used.

`oauth2-proxy` had a similar issue, which they fixed in
oauth2-proxy/oauth2-proxy#957 by reading from
the
[X-Forwarded-Host](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/X-Forwarded-Host)
and
[X-Forwarded-Proto](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/X-Forwarded-Proto)
headers, if set. This applies the same fix. The only major difference is
that `oauth2-proxy` only reads those headers when the `--reverse-proxy`
option is set, but I didn't add a corresponding option here. The only
reason I can think of for doing so is to preserve
backwards-compatibility, since it's theoretically possible this could
break someone's setup. I can't think of any real-world situations where
that can happen, but let me know if I should put this behind a option
and I'm happy to do so.

Tested using `make start UI_SECURE=true PROFILE=sso` and changing this
line: https://github.com/argoproj/argo-workflows/blob/29ea3518a30b2b605290d0ba38f95bfb75a2901f/manifests/quick-start/sso/overlays/workflow-controller-configmap.yaml#L15
to `redirectUrl: https://localhost:8080/oauth2/callback`. I tried to see
if I could automate this, but I couldn't figure out a clean way of doing
so.

Signed-off-by: Mason Malone <[email protected]>
@MasonM MasonM mentioned this issue Sep 28, 2024
Merged
MasonM added a commit to MasonM/argo-workflows that referenced this issue Oct 12, 2024
@MasonM
fix(sso): use relative redirect URLs. Fixed argoproj#13031
a34602f 
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 protocol and host of the request to
determine the redirect URL, which can be different than the original request
when a proxy is involved.

This switches the frontend to redirect using relative redirect URLs, e.g.
`/workflows`. Specifically, it uses what the [WHATWG URL
standard](https://url.spec.whatwg.org/#path-absolute-url-string) 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:
* https://github.com/oauth2-proxy/oauth2-proxy/blob/ab448cf38e7c1f0740b3cc2448284775e39d9661/pkg/app/redirect/validator.go#L14-L16
* https://github.com/oauth2-proxy/oauth2-proxy/blob/ab448cf38e7c1f0740b3cc2448284775e39d9661/pkg/app/redirect/validator.go#L47

`oauth2-proxy` is [MIT
licensed](https://github.com/oauth2-proxy/oauth2-proxy/blob/ab448cf38e7c1f0740b3cc2448284775e39d9661/LICENSE),
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](https://nikola.dev/posts/2021-05-10/handling_relative_urls)
goes into more detail.

Signed-off-by: Mason Malone <[email protected]>
MasonM added a commit to MasonM/argo-workflows that referenced this issue Oct 12, 2024
@MasonM
fix(sso): use relative redirect URLs. Fixed argoproj#13031
30c69fe 
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 protocol and host of the request to
determine the redirect URL, which can be different than the original request
when a proxy is involved.

This switches the frontend to redirect using relative redirect URLs, e.g.
`/workflows`. Specifically, it uses what the [WHATWG URL
standard](https://url.spec.whatwg.org/#path-absolute-url-string) 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:
* https://github.com/oauth2-proxy/oauth2-proxy/blob/ab448cf38e7c1f0740b3cc2448284775e39d9661/pkg/app/redirect/validator.go#L14-L16
* https://github.com/oauth2-proxy/oauth2-proxy/blob/ab448cf38e7c1f0740b3cc2448284775e39d9661/pkg/app/redirect/validator.go#L47

`oauth2-proxy` is [MIT
licensed](https://github.com/oauth2-proxy/oauth2-proxy/blob/ab448cf38e7c1f0740b3cc2448284775e39d9661/LICENSE),
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](https://nikola.dev/posts/2021-05-10/handling_relative_urls)
goes into more detail.

Signed-off-by: Mason Malone <[email protected]>
@MasonM MasonM linked a pull request Oct 12, 2024 that will close this issue
Open
@MasonM MasonM self-assigned this Dec 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@MasonM MasonM

Labels
area/server area/sso-rbac P2 Important. All bugs with >=3 thumbs up that aren’t P0 or P1, plus: Any other bugs deemed important solution/workaround There's a workaround, might not be great, but exists type/bug
Projects
None yet
Milestone
No milestone
5 participants
@MasonM @bmooso @agilgur5 @ejsuncy @ryancurrah