linuxserver / docker-bookstack

A Docker container for the BookStack documentation wiki
GNU General Public License v3.0
747 stars 108 forks source link

[BUG] MFA Setup gets unknown internal error #162

Closed ReVoLt112 closed 1 year ago

ReVoLt112 commented 1 year ago

Is there an existing issue for this?

Current Behavior

When i click on setup mfa (https:///mfa/totp/generate) i get an http 500

Expected Behavior

Setup process for TOTP

Steps To Reproduce

latest version of: image: lscr.io/linuxserver/bookstack Setup as described ob docker hub.

Click in User Profile on setup MFA, follow steps, error

Environment

- OS: CentOS 7.9.2009 (3.10.0-1160.83.1.el7.x86_64)
- How docker service was installed: official docker repo

CPU architecture

x86-64

Docker creation

---
version: "2"
services:
  bookstack:
    image: lscr.io/linuxserver/bookstack
    container_name: bookstack
    environment:
      - PUID=1000
      - PGID=1000
      - APP_URL=https://bookstack.<URL>.de
      - APP_LANG=de
      - APP_TIMEZONE=Europe/Berlin
      - APP_PROXIES=10.250.0.1
      - DB_HOST=10.250.0.5
      - DB_PORT=3306
      - DB_USER=<USER>
      - DB_PASS=<PASS>
      - DB_DATABASE=bookstack
      - MAIL_DRIVER=smtp
      # Host, Port & Encryption mechanism to use
      - MAIL_HOST=<SMTP>.<URL>.de
      - MAIL_PORT=587
      - MAIL_ENCRYPTION=tls
      # Authentication details for your SMTP service
      - MAIL_USERNAME=bookstack
      - MAIL_PASSWORD=<PASS>
      # The "from" email address for outgoing email
      - MAIL_FROM=bookstack@<URL>.de  
      # The "from" name used for outgoing email
      - MAIL_FROM_NAME=BookStack
      - CACHE_DRIVER=database
      - SESSION_DRIVER=database
      - DRAWIO=true
    volumes:
      - docker_share:/config
    ports:
      - 6875:80
    restart: unless-stopped

volumes:
    docker_share:
      driver: local
      driver_opts:
         type: nfs
         o: "nfsvers=4,addr=192.168.1.2,rw"
         device: ":/volume1/docker/bookstack"

Container logs

/config/www/laravel.log:
[2023-02-27 07:03:10] production.ERROR: Call to undefined function BaconQrCode\Encoder\iconv() {"userId":4,"exception":"[object] (Error(code: 0): Call to undefined function BaconQrCode\\Encoder\\iconv() at /app/www/vendor/bacon/bacon-qr-code/src/Encoder/Encoder.php:611)

Only error message i found
github-actions[bot] commented 1 year ago

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

ReVoLt112 commented 1 year ago

i could get it running by manual login into shell of the docker container and installing php81-iconv into container...

not solved yet because it is no persistent solution

ssddanbrown commented 1 year ago

Just been looking into this after hearing an additional report. I was a bit confused by this, since while iconv is not listed as a dependancy for BookStack directly, the package this is used for does indicate iconv as a required extension so should be flagged during the composer install step. The iconv extension is typically included by default in PHP.

Looking at the latest build console output I saw the below after composer install is ran:

#10 71.72 **** cleanup ****
#10 71.73 WARNING: Ignoring http://dl-cdn.alpinelinux.org/alpine/v3.17/main: No such file or directory
#10 71.73 WARNING: Ignoring http://dl-cdn.alpinelinux.org/alpine/v3.17/community: No such file or directory
#10 71.73 (1/5) Purging build-dependencies (20230227.202549)
#10 71.73 (2/5) Purging composer (2.4.4-r0)
#10 71.73 (3/5) Purging php81-iconv (8.1.16-r0)
#10 71.73 (4/5) Purging php81-zip (8.1.16-r0)
#10 71.73 (5/5) Purging libzip (1.9.2-r2)

I believe the above is from these lines: https://github.com/linuxserver/docker-bookstack/blob/9c8985d8c7b9777f05c75739237f1c9034aff740/Dockerfile#L54-L56

So maybe that apk delete & purge is removing the iconv extension as a side effect. Have not tested if it would do this if the iconv extension is explicity installed earlier in the script. I Have not yet looked to understand why this is arising at this time either. Could be our recent update in framework/dependancies (maybe leading to iconv not being used until now) otherwise not sure if there's been other changes on the base image side that might have affected this.

thespad commented 1 year ago

It looks like php81-iconv gets installed as a composer dep, which means it also gets removed, as we're not installing it explicitly as a runtime dep. Easy enough to change if it's needed as a runtime package now.

ssddanbrown commented 1 year ago

@thespad Thanks for the response.

I've been digging deep into this to understand what's changed, since it was winding me up as I was sure the MFA process and libraries involved have not changed recently. Turns out an unrelated dependency would bring in a iconv polyfill library, which was covering for the lack of iconv extension. That dependency was recently dropped, therefore the polyfill was also dropped, resulting in this scenario.

While the polyfill could be re-required as a dependency, it'd probably be best to just use the extension since it's fairly core and often default to PHP. I've updated the install requirements for BookStack to reflect the iconv requirement.

I've just opened #164 as a PR to address this.

ssddanbrown commented 1 year ago

@ReVoLt112 A new container image is now available. Pull down and use the latest image and hopefully this should now be fixed.

ReVoLt112 commented 1 year ago

Fixed, was a pleasure to cooperate with you