search

kuali / research-coi

GNU Affero General Public License v3.0
2 stars 3 forks source link

mock auth redirect does not work on non-standard port #13

Closed kgeis closed 8 years ago

kgeis commented 8 years ago

The change made to use req.hostname (to work in conjunction with Express' "trust proxy" setting) instead of req.get('host') in mockAuthClient.js made it so that in the non-proxy case, redirects only go to the standard port (HTTP 80, HTTPS 443). This is because req.get('host') and req.get('X-Forwarded-Host') is the entire Host header (e.g. "localhost:8090") where req.hostname is just the name of the server.

I can't find any way that Express makes it easy to handle both cases without duplicating effort in "trust proxy".

kgeis commented 8 years ago

For comparison, Koa sets req.host and req.hostname when app.proxy is true.

iambrandonn commented 8 years ago

How about we change this line to this?

let returnToValue = encodeURIComponent(req.originalUrl);

It makes the assumption that the mock auth service is on the same host as COI... but it will be, right?

kgeis commented 8 years ago

I think that will work for me. @gouldner, can you test this?

gouldner commented 8 years ago

Tested in dev and on test server and works. Thanks! Brandon !!! @iambrandonn

iambrandonn commented 8 years ago

This was included in 1512.