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

mocking request loses original koa app instance, breaking resolving of req.subdomains, because req.app.subdomainOffset is undefined #129

Open
falkenhawk opened this issue Apr 19, 2019 · 4 comments

Comments

@falkenhawk
Copy link

request.subdomains getter depends on request.app.subdomainOffset, which can be configured after koa is instantiated:

e.g. for hostname app.example.com original ctx.subdomains would resolve to ['app'] (with koa's default subdomainOffset = 2), and mocked req.subdomains would resolve to ['com', 'example', 'app'].

(This is breaking passport authentication for us because we are resolving tenants from subdomains, and at the same time we are checking jwt's issuer if it matches the subdomain)

Of course I can workaround it, but nevertheless I'd consider it as a bug. What's interesting, koa-passport initially delegated original app object in req.app but then, probably with this change: 0b7da65 it began to overwrite app with that almost-empty mock, which can only return trust proxy property and nothing besides that.

@rkusa
Copy link
Owner

rkusa commented Jun 3, 2019

Sorry for the late reply. What is your feeling, what the best solution would be? Extend the app mock to support your (totally valid!) use-case. Or replace the app mock with the original app, but monkey patch the original app to support express' trust proxy setting (thanks for already pointing out the commit where this change has been introduced)?

@netherspite
Copy link

With the latest version of koa (2.11) it also breaks req.ip because it depends on req.app.proxyIpHeader and app is mocked into an almost empty object.
I guess the second approach you've suggested would be the correct one, that's the most logical thing to do in my opinion.

@mrrinot
Copy link

mrrinot commented Sep 28, 2020

Just had this problem when using req.ip in my strategy.
Any news about this one ?

@Manjiz
Copy link

Manjiz commented Nov 25, 2020

Any update?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants