keycloak / keycloak-nodejs-connect

Apache License 2.0
682 stars 421 forks source link

Improve redirect URL making script #348

Open yukha-dw opened 2 years ago

yukha-dw commented 2 years ago

Hello,

Please improve redirectURL script on forceLogin function to broaden compatibility with other framework beside Express such as Fastify. Currently, it uses request.hostname to retrieves hostname and request.headers.host to retrieves port number. Unfortunately, it won't work on Fastify Framework because Fastify will assigns same value to request.headers.host and request.hostname (host_name:port_number).

I hope you could retrieves hostname from request.headers.host than request.hostname, thank you

Original https://github.com/keycloak/keycloak-nodejs-connect/blob/773e95a2fd8b4046cb22f6e3d55853169a81e9a8/middleware/protect.js#L20-L27

Improvement

  let headerHost = request.headers.host.split(':');
  let host = headerHost[0];
  let port = headerHost[1] || '';

For example, my redirect url will become: localhost:8080/realms/myrealm/protocol/openid-connect/auth?client_id=spc-client&state=ca6cc925-acf7-474b-8057-46ba40e35f88&redirect_uri=http%3A%2F%2Flocalhost%3A3000%3A3000%2Faltair%3Fauth_callback%3D1&scope=openid openid profile email&response_type=code

marcinmajkowski commented 1 year ago

I have related problem. I am running an application behind a reverse proxy and resulting redirectUrl contains proxy host combined with application port. It is because request.headers.host is used instead of e.g. request.get('x-forwarded-host') (since 'trust proxy' is enabled).

One way to solve this could be providing some way for setting redirectUrl explicitly.

Edit: I've just learned that this package is deprecated, see https://github.com/keycloak/keycloak/discussions/9818