auth0 / express-openid-connect

An Express.js middleware to protect OpenID Connect web applications.
MIT License
475 stars 143 forks source link

Incorrect forwarding with reverse proxy after callback #475

Closed sebsgr closed 1 year ago

sebsgr commented 1 year ago

Checklist

Description

I want to run the application in a docker container behind a reverse proxy. The application runs behind the reverse proxy with the URL "https://demo.domain.com/app"

My Configuration:

expressWebserver.use(auth({
            baseURL: "https://demo.domain.com/app",
            clientID: "XXX",
            issuerBaseURL: "gitlab.xxx.com",  # Own GitLab as OIDC Provider
            secret: "XXX",
            authorizationParams: {
                response_type: 'code',
                scope: 'openid profile email api write_repository'
            }
        }));

Browser Request:

URL: https://demo.domain.de/app/callback?code=xxxxxx&state=xxxxx
Method: GET
Code: 302 
Remoteadresse: xxxx
content-length: 132
content-type: text/html; charset=utf-8
location: /test

As we can see the request to https://demo.domain.de/app/callback will result in a redirection to /test. my expectation was it a forwarding to /app/test or https://demo.domain.de/app/test

Reproduction

  1. Build Docker Image for the application
    
    FROM node:18-alpine as builder
    WORKDIR /usr
    COPY package*.json ./
    COPY tsconfig.json ./
    RUN npm install
    COPY src ./src
    RUN npm run build

FROM node:18-alpine WORKDIR /usr COPY package*.json ./ RUN npm install --only=production COPY --from=builder /usr/dist . EXPOSE 3000 CMD ["node", "app.js"]

2. Run Container with Treafik

my-container: image: xxx labels: traefik.enable: true

HTTPS

  traefik.http.routers.app-https.rule: Host(`demo.domain.com`) && PathPrefix(`/app`)
  traefik.http.routers.app-https.entrypoints: websecure
  traefik.http.routers.app-https.tls: true
  traefik.http.routers.app-https.middlewares: app-pathreplace
  traefik.http.services.app-https.loadbalancer.server.port: 3000

  traefik.http.middlewares.app-pathreplace.replacepathregex.regex: "^/app/(.*)"
  traefik.http.middlewares.app-pathreplace.replacepathregex.replacement: "/$$1"
networks:
  - proxy

3. Call a route e.g. 'https://demo.domain.com/app/test' 
4. After successfull login the callback request will forward you to 'https://demo.domain.com/test' 

### Additional context

_No response_

### express-openid-connect version

2.16.0

### Express version

4.18.2

### Node.js version

18.11.18
adamjmcgrath commented 1 year ago

Hi @sebsgr - thanks for raising this

For the return to, we use the url of the page according to express: https://github.com/auth0/express-openid-connect/blob/master/lib/hooks/getLoginState.js#L15

traefik.http.middlewares.app-pathreplace.replacepathregex.regex: "^/app/(.*)" traefik.http.middlewares.app-pathreplace.replacepathregex.replacement: "/$$1"

If you accept the url /app/test then rewrite it to get express to serve /test - then it's going to think the page url is /test

Could you not mount your middleware on /app - then it will know where it's being served from. eg expressWebserver.use('/app', auth({ baseURL: 'https://demo.domain.com/app', ...})

ETMayberry commented 1 year ago

@adamjmcgrath I have a similar issue and it's not related to the Express Router object because the reverse proxy is external to the Node Express application. In the OP's example, the first route parameter /app would be proxied by something like an NGINX container which takes /app(/.*) and routes to this Express app using just the captured portion of the path. Express only sees the local route, which is why the redirected route is missing /app in the OP's example.

I feel like it would be helpful to have a configuration parameter for this middleware which allows the application to inject a proxy path as either a string or a function like so:

import { auth } from "express-openid-connect";
import { Router } from "express";

const myRouter = Router();

// static string, for when the proxy path is consistent
myRouter.use(
    auth({ 
        proxyPath: process.env.PROXY_ROUTE ?? '/app' 
    })
);

// function, for when proxy path might be dynamic and we then look to something like a non-standard header
// also, recommend supporting async just in case, even though this example can be done synchronously
async function getOriginalUrl(req: Request) { 
    return req.get('X-OriginalUrl'); 
}

myRouter.use(
    auth({ 
        proxyPath: getOriginalUrl 
    });
);

As far as I'm aware, there is no reliable way for a proxied Express router to identify the client's true original URL unless custom headers are passed along from the proxy or something else is specifically implemented. Thus, I think it's fair to say this middleware should not be responsible for trying to "detect" the original URL.

adamjmcgrath commented 1 year ago

I feel like it would be helpful to have a configuration parameter for this middleware which allows the application to inject a proxy path as either a string or a function like so:

Thanks for the suggestion @ETMayberry - I think there's enough configuration options in the SDK to do this yourself with baseURL and getLoginState - trying to add special options for proxies I think will less effective than just explaining how the existing options can help.

Thus, I think it's fair to say this middleware should not be responsible for trying to "detect" the original URL.

Agreed, am going to close this issue

ETMayberry commented 1 year ago

I think there's enough configuration options in the SDK to do this yourself with baseURL and getLoginState

@adamjmcgrath thanks for your response!

Could you maybe suggest what can be done using baseURL and getLoginState? Both @sebsgr and I have configured baseURL with the full FQDN and included the base proxy route but that seems to be stripped off when the middleware redirects to the originalUrl value from the request. If there is a way to account for the proxy path using the SDK then I'd agree there's no need for anything else but I'm not sure how I can override or modify that path other than just setting https://my-domain.com:8080/my-rev-proxy-route which doesn't seem to stick on that redirect...

adamjmcgrath commented 1 year ago

no worries @ETMayberry

that seems to be stripped off when the middleware redirects to the originalUrl value from the request.

This is the default behaviour, but you can override this using getLoginState

Essentially, the returnTo is set to originalUrl in the default getLoginState hook (here) - you can override this and create your own returnTo eg

app.use(
  auth({
    getLoginState(req) {
      return {
        returnTo: req.get('X-OriginalUrl'),
    };
  })
)

Just make sure you trust whatever value you pass to returnTo, because the SDK will redirect you to that URL (whatever the domain) after login