From d67f3d982b3548cabb5f3fbf379927f596c03976 Mon Sep 17 00:00:00 2001 From: Tim Farrell Date: Thu, 1 Feb 2024 13:40:59 -0600 Subject: [PATCH 01/10] Start by renaming variables to something more generic. This will give us a bit more flexibility as we look to other session management mechanisms. --- Dockerfile | 2 +- backend/config.py | 9 ++++++--- backend/utils/utils.py | 6 +++--- docker-compose.yaml | 1 + 4 files changed, 11 insertions(+), 7 deletions(-) diff --git a/Dockerfile b/Dockerfile index 3d4e380a..d292716b 100644 --- a/Dockerfile +++ b/Dockerfile @@ -25,7 +25,7 @@ ENV OLLAMA_API_BASE_URL "/ollama/api" ENV OPENAI_API_BASE_URL "" ENV OPENAI_API_KEY "" -ENV WEBUI_JWT_SECRET_KEY "SECRET_KEY" +ENV WEBUI_SECRET_KEY "" WORKDIR /app/backend diff --git a/backend/config.py b/backend/config.py index 05733f88..18e0824d 100644 --- a/backend/config.py +++ b/backend/config.py @@ -98,12 +98,15 @@ WEBUI_VERSION = os.environ.get("WEBUI_VERSION", "v1.0.0-alpha.61") WEBUI_AUTH = True #################################### -# WEBUI_JWT_SECRET_KEY +# WEBUI_SECRET_KEY #################################### -WEBUI_JWT_SECRET_KEY = os.environ.get("WEBUI_JWT_SECRET_KEY", "t0p-s3cr3t") +WEBUI_SECRET_KEY = os.environ.get( + "WEBUI_SECRET_KEY", + os.environ.get("WEBUI_JWT_SECRET_KEY", "t0p-s3cr3t") # DEPRECATED: remove at next major version +) -if WEBUI_AUTH and WEBUI_JWT_SECRET_KEY == "": +if WEBUI_AUTH and WEBUI_SECRET_KEY == "": raise ValueError(ERROR_MESSAGES.ENV_VAR_NOT_FOUND) #################################### diff --git a/backend/utils/utils.py b/backend/utils/utils.py index be500ce3..768a2ebf 100644 --- a/backend/utils/utils.py +++ b/backend/utils/utils.py @@ -14,7 +14,7 @@ import config logging.getLogger("passlib").setLevel(logging.ERROR) -JWT_SECRET_KEY = config.WEBUI_JWT_SECRET_KEY +SESSION_SECRET = config.WEBUI_SECRET_KEY ALGORITHM = "HS256" ############## @@ -42,13 +42,13 @@ def create_token(data: dict, expires_delta: Union[timedelta, None] = None) -> st expire = datetime.utcnow() + expires_delta payload.update({"exp": expire}) - encoded_jwt = jwt.encode(payload, JWT_SECRET_KEY, algorithm=ALGORITHM) + encoded_jwt = jwt.encode(payload, SESSION_SECRET, algorithm=ALGORITHM) return encoded_jwt def decode_token(token: str) -> Optional[dict]: try: - decoded = jwt.decode(token, JWT_SECRET_KEY, options={"verify_signature": False}) + decoded = jwt.decode(token, SESSION_SECRET, options={"verify_signature": False}) return decoded except Exception as e: return None diff --git a/docker-compose.yaml b/docker-compose.yaml index 7cd1bde0..eb0a8a90 100644 --- a/docker-compose.yaml +++ b/docker-compose.yaml @@ -26,6 +26,7 @@ services: - ${OLLAMA_WEBUI_PORT-3000}:8080 environment: - 'OLLAMA_API_BASE_URL=http://ollama:11434/api' + - 'WEBUI_SECRET_KEY=' extra_hosts: - host.docker.internal:host-gateway restart: unless-stopped From 03a7e35967dde9f75f2cc3e752ecb74c887feae5 Mon Sep 17 00:00:00 2001 From: Tim Farrell Date: Thu, 1 Feb 2024 13:43:54 -0600 Subject: [PATCH 02/10] Default docker installations should generate a random key instead of using a static secret that everyone can see. --- Dockerfile | 3 +++ backend/start.sh | 7 ++++++- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/Dockerfile b/Dockerfile index d292716b..39933fd4 100644 --- a/Dockerfile +++ b/Dockerfile @@ -53,4 +53,7 @@ COPY --from=build /app/build /app/build # copy backend files COPY ./backend . +# Generate a random value to use as a WEBUI_SECRET_KEY in case the user didn't provide one. +RUN echo $(head -c 12 /dev/random | base64) > docker_secret_key + CMD [ "bash", "start.sh"] \ No newline at end of file diff --git a/backend/start.sh b/backend/start.sh index 09a791fc..515e6c93 100755 --- a/backend/start.sh +++ b/backend/start.sh @@ -4,4 +4,9 @@ SCRIPT_DIR=$( cd -- "$( dirname -- "${BASH_SOURCE[0]}" )" &> /dev/null && pwd ) cd "$SCRIPT_DIR" || exit PORT="${PORT:-8080}" -exec uvicorn main:app --host 0.0.0.0 --port "$PORT" --forwarded-allow-ips '*' +if test -f docker_secret_key && test "$WEBUI_SECRET_KEY" = ""; then + echo Using generated DOCKER_SECRET_KEY + WEBUI_SECRET_KEY=`cat docker_secret_key` +fi + +WEBUI_SECRET_KEY="$WEBUI_SECRET_KEY" exec uvicorn main:app --host 0.0.0.0 --port "$PORT" --forwarded-allow-ips '*' From 2c1dacb9b67b35840da8f812d6ddc0deb74d712d Mon Sep 17 00:00:00 2001 From: Tim Farrell Date: Thu, 1 Feb 2024 13:46:45 -0600 Subject: [PATCH 03/10] We should verify signatures to make the whole session secret meaningful. --- backend/utils/utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/utils/utils.py b/backend/utils/utils.py index 768a2ebf..8b722554 100644 --- a/backend/utils/utils.py +++ b/backend/utils/utils.py @@ -48,7 +48,7 @@ def create_token(data: dict, expires_delta: Union[timedelta, None] = None) -> st def decode_token(token: str) -> Optional[dict]: try: - decoded = jwt.decode(token, SESSION_SECRET, options={"verify_signature": False}) + decoded = jwt.decode(token, SESSION_SECRET) return decoded except Exception as e: return None From 8c37edd027b52426ffdaf715da7cbbe613b4df8e Mon Sep 17 00:00:00 2001 From: Tim Farrell Date: Thu, 1 Feb 2024 14:04:48 -0600 Subject: [PATCH 04/10] Even though "User.email" is enforced as unique at signup, it is not a unique field in the database. Let's use "User.id" instead. This also makes it more difficult to do a session stealing attack. --- backend/apps/web/routers/auths.py | 4 ++-- backend/utils/utils.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/backend/apps/web/routers/auths.py b/backend/apps/web/routers/auths.py index f45c67ac..d06539f8 100644 --- a/backend/apps/web/routers/auths.py +++ b/backend/apps/web/routers/auths.py @@ -93,7 +93,7 @@ async def update_password( async def signin(form_data: SigninForm): user = Auths.authenticate_user(form_data.email.lower(), form_data.password) if user: - token = create_token(data={"email": user.email}) + token = create_token(data={"id": user.id}) return { "token": token, @@ -132,7 +132,7 @@ async def signup(request: Request, form_data: SignupForm): ) if user: - token = create_token(data={"email": user.email}) + token = create_token(data={"id": user.id}) # response.set_cookie(key='token', value=token, httponly=True) return { diff --git a/backend/utils/utils.py b/backend/utils/utils.py index 8b722554..9b146bbc 100644 --- a/backend/utils/utils.py +++ b/backend/utils/utils.py @@ -60,8 +60,8 @@ def extract_token_from_auth_header(auth_header: str): def get_current_user(auth_token: HTTPAuthorizationCredentials = Depends(HTTPBearer())): data = decode_token(auth_token.credentials) - if data != None and "email" in data: - user = Users.get_user_by_email(data["email"]) + if data != None and "id" in data: + user = Users.get_user_by_id(data["id"]) if user is None: raise HTTPException( status_code=status.HTTP_401_UNAUTHORIZED, From e15dbdc46ac82960e88e99f0225a5197dd9bff78 Mon Sep 17 00:00:00 2001 From: Tim Farrell Date: Thu, 1 Feb 2024 14:52:11 -0600 Subject: [PATCH 05/10] Pass the instance we're using. --- backend/utils/utils.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/backend/utils/utils.py b/backend/utils/utils.py index 9b146bbc..546799cb 100644 --- a/backend/utils/utils.py +++ b/backend/utils/utils.py @@ -21,7 +21,7 @@ ALGORITHM = "HS256" # Auth Utils ############## -bearer_scheme = HTTPBearer() +bearer_security = HTTPBearer() pwd_context = CryptContext(schemes=["bcrypt"], deprecated="auto") @@ -58,7 +58,7 @@ def extract_token_from_auth_header(auth_header: str): return auth_header[len("Bearer ") :] -def get_current_user(auth_token: HTTPAuthorizationCredentials = Depends(HTTPBearer())): +def get_current_user(auth_token: HTTPAuthorizationCredentials = Depends(bearer_security)): data = decode_token(auth_token.credentials) if data != None and "id" in data: user = Users.get_user_by_id(data["id"]) From 4fceb404bdbc8e250360a16b4108058af8aa50e0 Mon Sep 17 00:00:00 2001 From: Tim Farrell Date: Thu, 1 Feb 2024 14:52:46 -0600 Subject: [PATCH 06/10] Call `jwt.decode` with the expected algorithms --- backend/utils/utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/utils/utils.py b/backend/utils/utils.py index 546799cb..2795a613 100644 --- a/backend/utils/utils.py +++ b/backend/utils/utils.py @@ -48,7 +48,7 @@ def create_token(data: dict, expires_delta: Union[timedelta, None] = None) -> st def decode_token(token: str) -> Optional[dict]: try: - decoded = jwt.decode(token, SESSION_SECRET) + decoded = jwt.decode(token, SESSION_SECRET, algorithms=[ALGORITHM]) return decoded except Exception as e: return None From 44799e2018274acc1467d74360efff0288818721 Mon Sep 17 00:00:00 2001 From: Tim Farrell Date: Thu, 1 Feb 2024 14:53:13 -0600 Subject: [PATCH 07/10] Remove some extraneous imports --- backend/apps/web/models/auths.py | 7 +------ backend/apps/web/routers/chats.py | 3 --- 2 files changed, 1 insertion(+), 9 deletions(-) diff --git a/backend/apps/web/models/auths.py b/backend/apps/web/models/auths.py index 367db3ff..02d2ab86 100644 --- a/backend/apps/web/models/auths.py +++ b/backend/apps/web/models/auths.py @@ -5,12 +5,7 @@ import uuid from peewee import * from apps.web.models.users import UserModel, Users -from utils.utils import ( - verify_password, - get_password_hash, - bearer_scheme, - create_token, -) +from utils.utils import verify_password from apps.web.internal.db import DB diff --git a/backend/apps/web/routers/chats.py b/backend/apps/web/routers/chats.py index 29214229..d2830095 100644 --- a/backend/apps/web/routers/chats.py +++ b/backend/apps/web/routers/chats.py @@ -25,9 +25,6 @@ from apps.web.models.tags import ( Tags, ) -from utils.utils import ( - bearer_scheme, -) from constants import ERROR_MESSAGES router = APIRouter() From e2d481d99a7009a71dd27f132ada7d18e5f72037 Mon Sep 17 00:00:00 2001 From: Tim Farrell Date: Thu, 1 Feb 2024 20:55:59 -0600 Subject: [PATCH 08/10] Move the random secret generation to start.sh. This way the random secret is created on first run instead of docker build. We don't really want all standard imaages to share a password anymore than we want a static password. --- Dockerfile | 3 --- backend/start.sh | 16 +++++++++++++--- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/Dockerfile b/Dockerfile index 39933fd4..d292716b 100644 --- a/Dockerfile +++ b/Dockerfile @@ -53,7 +53,4 @@ COPY --from=build /app/build /app/build # copy backend files COPY ./backend . -# Generate a random value to use as a WEBUI_SECRET_KEY in case the user didn't provide one. -RUN echo $(head -c 12 /dev/random | base64) > docker_secret_key - CMD [ "bash", "start.sh"] \ No newline at end of file diff --git a/backend/start.sh b/backend/start.sh index 515e6c93..03fe792a 100755 --- a/backend/start.sh +++ b/backend/start.sh @@ -3,10 +3,20 @@ SCRIPT_DIR=$( cd -- "$( dirname -- "${BASH_SOURCE[0]}" )" &> /dev/null && pwd ) cd "$SCRIPT_DIR" || exit +KEY_FILE=.webui_secret_key + PORT="${PORT:-8080}" -if test -f docker_secret_key && test "$WEBUI_SECRET_KEY" = ""; then - echo Using generated DOCKER_SECRET_KEY - WEBUI_SECRET_KEY=`cat docker_secret_key` +if ["$WEBUI_SECRET_KEY" = ""]; then + echo No WEBUI_SECRET_KEY provided + + if ! [ -e "$KEY_FILE" ]; then + echo Generating WEBUI_SECRET_KEY + # Generate a random value to use as a WEBUI_SECRET_KEY in case the user didn't provide one. + echo $(head -c 12 /dev/random | base64) > $KEY_FILE + fi + + echo Loading WEBUI_SECRET_KEY from $KEY_FILE + WEBUI_SECRET_KEY=`cat $KEY_FILE` fi WEBUI_SECRET_KEY="$WEBUI_SECRET_KEY" exec uvicorn main:app --host 0.0.0.0 --port "$PORT" --forwarded-allow-ips '*' From 1031638d8296fdd6b8a7e6e5d6495a5f8d111f8c Mon Sep 17 00:00:00 2001 From: Tim Farrell Date: Thu, 1 Feb 2024 21:04:10 -0600 Subject: [PATCH 09/10] Maintain backward compatibility with WEBUI_JWT_SECRET_KEY for the time being --- backend/start.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/start.sh b/backend/start.sh index 03fe792a..ac5d48a7 100755 --- a/backend/start.sh +++ b/backend/start.sh @@ -6,7 +6,7 @@ cd "$SCRIPT_DIR" || exit KEY_FILE=.webui_secret_key PORT="${PORT:-8080}" -if ["$WEBUI_SECRET_KEY" = ""]; then +if ["$WEBUI_SECRET_KEY $WEBUI_JWT_SECRET_KEY" = " "]; then echo No WEBUI_SECRET_KEY provided if ! [ -e "$KEY_FILE" ]; then From 8298cefd62cad4b66b2225852547b5b58f32c0a5 Mon Sep 17 00:00:00 2001 From: Tim Farrell Date: Fri, 2 Feb 2024 08:45:47 -0600 Subject: [PATCH 10/10] Fix bash condition formatting --- backend/start.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/start.sh b/backend/start.sh index ac5d48a7..c9f5a05b 100755 --- a/backend/start.sh +++ b/backend/start.sh @@ -6,7 +6,7 @@ cd "$SCRIPT_DIR" || exit KEY_FILE=.webui_secret_key PORT="${PORT:-8080}" -if ["$WEBUI_SECRET_KEY $WEBUI_JWT_SECRET_KEY" = " "]; then +if test "$WEBUI_SECRET_KEY $WEBUI_JWT_SECRET_KEY" = " "; then echo No WEBUI_SECRET_KEY provided if ! [ -e "$KEY_FILE" ]; then