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

Long redirect URLs cause tsh login to fail #7467

Closed
russjones opened this issue Jul 3, 2021 · 6 comments · Fixed by #8293
Closed

Long redirect URLs cause tsh login to fail #7467

russjones opened this issue Jul 3, 2021 · 6 comments · Fixed by #8293
Assignees
Labels
bug c-sq Internal Customer Reference feature-request Used for new features in Teleport, improvements to current should be #enhancements

Comments

@russjones
Copy link
Contributor

Problem

Use reported seeing the following errors when attempting to use tsh login with their SSO provider. Web UI login works fine.

ERRO [WEB]       Error while processing callback. auth:oidc error:[
ERROR REPORT:
Original Error: *trace.RawTrace invalid_grant: Invalid or expired code parameter.

This error was coming from their identity provider because the code token has already been consumed.

Upon investigation, it was discovered that the entire SSO login flow is successful until the 302 console redirect to transfer credentials from the browser to disk. This was due to the user running a middleware component that intercepts all requests and drop requests with headers larger than 8 kb and Teleport was sending a Location header that was 16 kb.

The middleware would accept the request, drop it, then the 302 console redirect would be tried again and this time fail at the IdP (hence the error above) but succeed because the header was now smaller than 8 kb.

User reports that this is not the middleware they are running, but other popular Cloud services, like AWS ALB, also enforce 8 kb limits on headers.

Proposed Solution

This process starts with the IdP issuing a POST /webapi/saml/acs request which eventually becomes the 302 console redirect. Instead of performing a 302 console redirect, Teleport could return a small Javascript application that uses window.location to perform the redirect. We use a similar approach in Application Access.

https://github.com/gravitational/teleport/blob/master/lib/web/app/redirect.go

This would solve the problem because this request would not be caught by any middleware and instead happen local to the users workstation only.

@russjones russjones added bug feature-request Used for new features in Teleport, improvements to current should be #enhancements c-sq Internal Customer Reference labels Jul 3, 2021
@jsonmorris
Copy link

This also applies to oidc login redirects in addition to saml redirects.

@russjones
Copy link
Contributor Author

@codingllama Let's sync about this on Thursday, talked with @klizhentas, there is another way to solve this problem that works well with the future work on Teleport Terminal.

@codingllama
Copy link
Contributor

Relevant: #7493.

@russjones
Copy link
Contributor Author

Assigned to @atburke the window.location approach which will act as a stopgap solution until we can implement #7493.

@atburke
Copy link
Contributor

atburke commented Sep 8, 2021

I've tested the SSO workflow, and although most of it works without javascript, the final success/error pages don't. I think it's safe to assume that users won't have javascript disabled and that the window.location approach shouldn't break any existing workflows (and if not, we could try HTML Redirects).

As for implementation, I think the simplest approach would be to add a new version of Handler.WithRedirect that serves the javascript app instead of returning 302.

@russjones
Copy link
Contributor Author

@alex-kovoy What do you think above the above? I can provide you more details tomorrow if you'd like.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug c-sq Internal Customer Reference feature-request Used for new features in Teleport, improvements to current should be #enhancements
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants