[BUG] Issue setting up proxy_set_header for IP Address in nginx log

[Nounoursdestavernes]

Is there an existing issue for this?

• [X] I have searched the existing issues

Current Behavior

I have setup APP_PROXIES="*" in order to add an anti-bruteforce software based on logs. It work perfectly in the Audit page in bookstack through the header x-forwarded-for . But Nginx logs are still loging docker's IP and not the client one.

I found a work around by modifying the nginx.conf file by adding new logs, but it is not satsifying for me.

Expected Behavior

When the APP_PROXIES is set then Nginx log the IP through x-forwarded-for header as bookstack do.

Steps To Reproduce

1. Use default docker-compose to set up an instance
2. Set up the APP_PROXIES variable to "*"
3. Check that in the audit page the real IP is read from x-forwarded-for
4. Nginx logs don't use the header to log the correct IP.

Environment

- OS: Ubuntu 22.04
- How docker service was installed: Docker compose

CPU architecture

x86-64

Docker creation

name: bookstack
services:
  bookstack:
    image: lscr.io/linuxserver/bookstack:24.05.3
    container_name: bookstack
    environment:
      - PUID=${PUID}
      - PGID=${PGID}
      - TZ=${TZ}
      - APP_URL=${APP_URL}
      - DB_HOST=${DB_HOST}
      - DB_PORT=${DB_PORT}
      - DB_USER=${DB_USER}
      - DB_PASS=${DB_PASS}
      - DB_DATABASE=${DB_DATABASE}
      - STORAGE_TYPE=${STORAGE_TYPE}
      - LOG_FAILED_LOGIN_MESSAGE=${LOG_FAILED_LOGIN_MESSAGE}
      - APP_PROXIES=${APP_PROXIES}
    volumes:
      - /data/bookstack/bookstack_app_data:/config
    ports:
      - 127.0.0.1:8080:80
    restart: unless-stopped
    depends_on:
      - bookstack_db

  bookstack_db:
    image: lscr.io/linuxserver/mariadb:10.11.8
    container_name: bookstack_db
    environment:
      - PUID=${PUID}
      - PGID=${PGID}
      - TZ=${TZ}
      - MYSQL_ROOT_PASSWORD=${DB_ROOT_PASSWORD}
      - MYSQL_DATABASE=${DB_DATABASE}
      - MYSQL_USER=${DB_USER}
      - MYSQL_PASSWORD=${DB_PASS}
    volumes:
      - /data/bookstack/bookstack_db_data:/config
    restart: unless-stopped

Container logs

2024/07/21 18:11:45 [error] 311#311: *3 FastCGI sent in stderr: "PHP message: Failed login for po@po.po" while reading response header from upstream, client: 172.18.0.1, server: _, request: "POST /login HTTP/1.0", upstream: "fastcgi://127.0.0.1:9000", host: "127.0.0.1:8080", referrer: "https://bookstack.mywebsite.fr/login"

[github-actions[bot]]

Thanks for opening your first issue here! Be sure to follow the relevant issue templates, or risk having this issue marked as invalid.

[ssddanbrown]

I found a work around by modifying the nginx.conf file by adding new logs, but it is not satsifying for me.

Why is that not satisfying? The linxuserver team helpfully provide the nginx config so you're able to customize and make things work where needed.

I don't understand why there's an expectation for them to also support & handle BookStack's app-level settings, which is something I'd advise against since the value format may not align with the intended purpose here and/or might be something changed at the app-level in future.

I don't see this as a bug, but a feature request or a bug in expectations.

[Nounoursdestavernes]

Hi @ssddanbrown

In my opinion it is not satisfying for two reasons:

• I had to handle a part of the configuration on top of a environment variable that require some knowledge. I got it so that ok for me but I think it should be easier for people with less knowledge.
• For me, a docker compose that help to deploy a specific app should be adapted to this one. In consequence, when an environment variable change the behavior of the app, the rest of the environment could be aligned on this behavior.

I understand the fact that they don't have to support and handle BookStack's app-level settings. But this is not my vision of docker images and docker compose. For me they are a layer that is design to help to deploy and configure the app and it's environment more easily.

With your explanation of your vision I agree that is more a feature request than a bug.

BTW I am available to help to dev this feature.

EDIT : After debating with a friend I change my vision and this is clearly a feature and this is far from being mandatory or needed. This is more a bonus and could effectively could create breaking change with future versions of BookStack.

[Roxedus]

Solving this would need to edit files we have no intention to automatically change. As Dan points out, we expose the nginx configuration to the user, for this very purpose.

Implementing this change OOTB raises multiple issues.

• It deviates from how we treat the config for all php based applications we package.
• It will be breaking for people that does not expect the real ip in the logs.
• It is based on a application setting we (currently) don't document, leading to unexpected changes for the user when enabled.

Steps that can be taken are:

• Document this better on our readme.
• Create a dockermod, which makes this automatic change opt-in.

[Nounoursdestavernes]

Hi @Roxedus

Thanks for the explanation. I better understand the issues raised by my proposition and why it shouldn't be implemented as it was proposed.