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

allow creating users from reverse proxy headers #2899

Open
wants to merge 1 commit into
base: Develop
Choose a base branch
from

Conversation

igor47
Copy link

@igor47 igor47 commented Sep 26, 2023

Currently, when authentication happens in the reverse proxy, the user must already be in calibre-web's DB or else nothing happens. Source:

If you pass a username that isn't present in the database, nothing will happen - the user must exist beforehand in order to login.

With this PR, we add an option to also create the user. User creation is modeled on how we do it in LDAP sync. If the user creation fails for whatever reason, still nothing happens, and the failure happens transparently to the user, exactly how it happens today if the user doesn't already exist in the DB.

This is a very-low code PR, but it makes authenticating with reverse proxies much easier. The alternatives to this PR are:

  • use reverse-proxy auth, but create all users manually -- this is tedious, I doubt anyone does it
  • use LDAP sync to create users, and then authenticate via the proxy -- this is the most sensible alternative, but is a lot of work since LDAP is a hot old mess.
  • wait for, or resume, work on the oauth generic provider from feat: support generic oauth2 #2211

i think for people using centralized auth (authentik/authelia) with a reverse proxy (traefik/caddy/nginx) this provides the biggest bang-for-buck without much code to maintain.

We don't do any group setting here, so users will likely be created non-admin. My recommended flow to set up auth:

  • set up calibre-web using the docker images
  • log in with the default admin user and set up reverse proxy auth settings
  • create a user matching the name you'll get from your proxy, and make that user an admin
  • enable auth in your proxy
  • log in with your proxy auth provider. you should be an admin
  • delete the default admin user

@MisterMustache
Copy link

Why hasn't this been merged @OzzieIsaacs?

@igor47
Copy link
Author

igor47 commented May 1, 2024

FWIW, i've been running my own fork of calibre-web with this feature since i opened the PR, with no problems. this PR is open for the benefit of the community. here's how i run it:

Based on the comment in the README:

The last few months, maintaining Calibre-Web has felt more like work than a hobby. I felt pressured and teased by people to solve "their" problems and merge PRs for "their" Calibre-Web. I have turned off all notifications from Github/Discord and will now concentrate undisturbed on the development of “my” Calibre-Web over the next few weeks/months.

i'm assuming this PR will never be merged, so i recommend forking and patching if you want this or other features. BTW @MisterMustache -- your comment came off as pretty aggressive to me, not sure if that was the intent, but i prefer to be more gentle to overworked OSS maintainers.

@MisterMustache
Copy link

BTW @MisterMustache -- your comment came off as pretty aggressive to me, not sure if that was the intent, but i prefer to be more gentle to overworked OSS maintainers.

God no! I'm sorry if It came out that way -- that was was never my intention. I made the comment with genuine curiosity because I haven't seen the README with the note you posted in the comment. I fully support the main maintainer and wish him all the best.

FWIW, i've been running my own fork of calibre-web with this feature since i opened the PR, with no problems. this PR is open for the benefit of the community. here's how i run it:

Thank you for contributing, I really appreciate it! 😄
I've been thinking of making my own fork as well. That being said the links you provided are not public :( and return a 404 to me.

@igor47
Copy link
Author

igor47 commented May 1, 2024

the links you provided are not public :( and return a 404 to me.

oops! relevant section from compose.yaml:

  calibre-web:
    build:
      context: ./custom/calibre
    # image was patched to better support proxy auth
    #image: lscr.io/linuxserver/calibre-web:latest
    restart: unless-stopped
    container_name: calibre
    environment:
      - PUID=116
      - PGID=5004
      - TZ=America/Los_Angeles
      - DOCKER_MODS=linuxserver/mods:universal-calibre
      - OAUTHLIB_RELAX_TOKEN_SCOPE=1
    volumes:
      - ${STORAGE}/calibre/config:/config
      - /media/books:/books
    labels:
      traefik.enable: true
      traefik.http.routers.calibre.rule: Host(`<redacted>`)
      traefik.http.routers.calibre.tls: true
      traefik.http.routers.calibre.tls.certresolver: le
      traefik.http.routers.calibre.entrypoints: https
      traefik.http.services.calibre.loadbalancer.server.port: 8083
      traefik.http.routers.calibre.middlewares: authentik@file

the Dockerfile:

FROM lscr.io/linuxserver/calibre-web:latest
RUN curl -L https://patch-diff.githubusercontent.com/raw/janeczku/calibre-web/pull/2899.patch -o /app/calibre-web/2899.patch && \
  apt-get update && \
  apt-get install -y --no-install-recommends patch && \
  patch -p1 -d /app/calibre-web < /app/calibre-web/2899.patch && \
  rm /app/calibre-web/2899.patch

@nOw-Ay
Copy link

nOw-Ay commented May 28, 2024

Hello @igor47, just had an issue with your PR. I am using Keycloak and Traefik (getting reverse proxy auth thanks to this bridge), I set up X-Forwarded-User as the user header and left the email field blank. Despite my vigilance, I got this error :

  File "/app/calibre-web/cps/usermanagement.py", line 139, in create_user_from_reverse_proxy_header
    user.email = email
UnboundLocalError: local variable 'email' referenced before assignment

Do you have an idea of what might not be working ? Thanks a lot.

EDIT : I managed to solve it in a way, by writing anything in the email field, rp_email_header_name gets a value and user.email can be defined, even if an email isn't specified. You should move the rp_email_header_name test outside of the try: except: block and condition user.email = email to it.

allowing login via reverse proxy auth is convenient, but it's not
convenient to have to create the users in advance. this PR allows users
to be optionally created if they don't already exist. we provide this as
an option in the UI
@igor47 igor47 force-pushed the reverse-proxy-create-users branch from b3adf67 to a326949 Compare June 7, 2024 06:47
@igor47
Copy link
Author

igor47 commented Jun 7, 2024

I set up X-Forwarded-User as the user header and left the email field blank.

yup i wasn't handling this. i updated the PR.

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

Successfully merging this pull request may close these issues.

None yet

3 participants