CorentinTh / enclosed

Minimalistic web app designed for sending private and secure notes.
https://enclosed.cc
Apache License 2.0
683 stars 51 forks source link

Panic using PUBLIC_IS_AUTHENTICATION_REQUIRED #176

Closed X-lem closed 1 month ago

X-lem commented 1 month ago

Describe the bug

I tried testing out the new PUBLIC_IS_AUTHENTICATION_REQUIRED. I set the auth env as follows. Others I left as the default

PUBLIC_IS_AUTHENTICATION_REQUIRED="true"
AUTHENTICATION_JWT_SECRET="[redacted]"
AUTHENTICATION_USERS="test:[redacted]"

What happened?

The docker container crashed. It fails because I'm not using an email for the auth user so I'm assuming some parsing is failing somewhere and panicking. Not sure if you plan on using the email at some point (a username would be fine imo). But if you are wanting to enforce it to be an actual email for future reasons a better error message would be helpful.

image

System information

Self hosting

Where did you encounter the bug?

A self hosted instance

CorentinTh commented 1 month ago

Hi @X-lem

Thanks for bringing this up! I’ll definitely improve the error message so it’s clearer in cases like this.

Looking at your config, the issue seems to be because the value for AUTHENTICATION_USERS isn’t in the proper email format. Right now, the system expects an actual email, so it’s failing when it tries to parse it. You can check out the schema here: config.ts.

If you’re aiming to use a username instead of an email, it might not work since the validation is pretty strict about needing an email right now. If we ever relax that to allow usernames, I’ll make sure to update the docs. For now, using a valid email format should fix the problem