Repository navigation
Allow login while proxing a sub-domain - #2186
code-asher wants to merge 1 commit into
Conversation
|
tbh this feels a little jank to me but I'm not sure how else this could be implemented. |
ed74b04 to
613cc87
Compare
| if (!route.provider.authenticated(request)) { | ||
| // Attempt to determine if it's the user browsing the root and if so fall | ||
| // through to allow the login flow. | ||
| if (request.headers["content-type"] !== "application/json") { |
There was a problem hiding this comment.
Let's check explicitly for text/html here.
There was a problem hiding this comment.
I wanted to do that but Firefox and Chromium don't seem to set this at all. When I logged it the value was undefined.
| // Must be authenticated to use the proxy. | ||
| route.provider.ensureAuthenticated(request) | ||
| if (!route.provider.authenticated(request)) { | ||
| // Attempt to determine if it's the user browsing the root and if so fall |
There was a problem hiding this comment.
| // Attempt to determine if it's the user browsing the root and if so fall | |
| // Attempt to determine if it's the user browsing the route and if so fall |
There was a problem hiding this comment.
I mean root, for example 8080.mydomain.com and not 8080.mydomain.com/some/page. Maybe we should always redirect though even if it's not the root.
There was a problem hiding this comment.
Well I guess we can't do that safely, it would end up causing the problem in #2128 again on account of the whole not being able to detect HTML nonsense.
| // Attempt to determine if it's the user browsing the root and if so fall | ||
| // through to allow the login flow. | ||
| if (request.headers["content-type"] !== "application/json") { | ||
| switch (route.providerBase) { |
There was a problem hiding this comment.
I'm confused by this, don't we pretty always want to fall through to the login flow if text/html is asked for but the user isn't authenticated?
There was a problem hiding this comment.
Yup, if there was a way to reliably detect the request was asking for HTML (and ideally also that it wasn't an xhr request) that would 100% be the way to do it. All this jank is to work around not being able to do that (at least as far as I know, I hope I'm wrong though).
|
Pretty much all of this conflicts with the Express rewrite so I'm going to bundle it into that PR. |
Fixes #2128.