From 034288711b6c74b49080b1eaf54d18c069977fa8 Mon Sep 17 00:00:00 2001 From: Kevin Morris Date: Thu, 28 Oct 2021 00:11:23 -0700 Subject: [PATCH] fix(fastapi): rework cookies - do not re-emit generically This change removes cookie re-emission of AURLANG and AURTZ, adds the AURREMEMBER cookie (the state of the "Remember Me" checkbox on login), and re-emits AURSID based on the AURREMEMBER cookie. Previously, re-emission of AURSID was forcefully modifying the expiration of the AURSID cookie. The introduction of AURREMEMBER allows us to deduct the correct cookie expiration timing based on configuration variables. With this addition, we now re-emit the AURSID cookie with an updated expiration based on the "Remember Me" checkbox on login. Signed-off-by: Kevin Morris --- aurweb/cookies.py | 68 ++++++++++++++++++++++++++++++++++++++ aurweb/routers/accounts.py | 11 +++--- aurweb/routers/auth.py | 22 ++++++------ aurweb/routers/html.py | 9 ++--- aurweb/templates.py | 17 ++++++---- aurweb/util.py | 23 ------------- 6 files changed, 100 insertions(+), 50 deletions(-) create mode 100644 aurweb/cookies.py diff --git a/aurweb/cookies.py b/aurweb/cookies.py new file mode 100644 index 00000000..442a4c0a --- /dev/null +++ b/aurweb/cookies.py @@ -0,0 +1,68 @@ +from fastapi import Request +from fastapi.responses import Response + +from aurweb import config + + +def samesite() -> str: + """ Produce cookie SameSite value based on options.disable_http_login. + + When options.disable_http_login is True, "strict" is returned. Otherwise, + "lax" is returned. + + :returns "strict" if options.disable_http_login else "lax" + """ + secure = config.getboolean("options", "disable_http_login") + return "strict" if secure else "lax" + + +def timeout(extended: bool) -> int: + """ Produce a session timeout based on `remember_me`. + + This method returns one of AUR_CONFIG's options.persistent_cookie_timeout + and options.login_timeout based on the `extended` argument. + + The `extended` argument is typically the value of the AURREMEMBER + cookie, defaulted to False. + + If `extended` is False, options.login_timeout is returned. Otherwise, + if `extended` is True, options.persistent_cookie_timeout is returned. + + :param extended: Flag which generates an extended timeout when True + :returns: Cookie timeout based on configuration options + """ + timeout = config.getint("options", "login_timeout") + if bool(extended): + timeout = config.getint("options", "persistent_cookie_timeout") + return timeout + + +def update_response_cookies(request: Request, response: Response, + aurtz: str = None, aurlang: str = None, + aursid: str = None) -> Response: + """ Update session cookies. This method is particularly useful + when updating a cookie which was already set. + + The AURSID cookie's expiration is based on the AURREMEMBER cookie, + which is retrieved from `request`. + + :param request: FastAPI request + :param response: FastAPI response + :param aurtz: Optional AURTZ cookie value + :param aurlang: Optional AURLANG cookie value + :param aursid: Optional AURSID cookie value + :returns: Updated response + """ + secure = config.getboolean("options", "disable_http_login") + if aurtz: + response.set_cookie("AURTZ", aurtz, secure=secure, httponly=secure, + samesite=samesite()) + if aurlang: + response.set_cookie("AURLANG", aurlang, secure=secure, httponly=secure, + samesite=samesite()) + if aursid: + remember_me = bool(request.cookies.get("AURREMEMBER", False)) + response.set_cookie("AURSID", aursid, secure=secure, httponly=secure, + max_age=timeout(remember_me), + samesite=samesite()) + return response diff --git a/aurweb/routers/accounts.py b/aurweb/routers/accounts.py index 70be2510..61c125e0 100644 --- a/aurweb/routers/accounts.py +++ b/aurweb/routers/accounts.py @@ -10,7 +10,7 @@ from sqlalchemy import and_, func, or_ import aurweb.config -from aurweb import db, l10n, logging, models, time, util +from aurweb import cookies, db, l10n, logging, models, time, util from aurweb.auth import account_type_required, auth_required from aurweb.captcha import get_captcha_answer, get_captcha_salts, get_captcha_token from aurweb.l10n import get_translator_for_request @@ -585,16 +585,19 @@ async def account_edit_post(request: Request, user.update_password(P) if user == request.user: + remember_me = request.cookies.get("AURREMEMBER", False) + # If the target user is the request user, login with - # the updated password and update AURSID. - request.cookies["AURSID"] = user.login(request, P) + # the updated password to update the Session record. + user.login(request, P, cookies.timeout(remember_me)) if not errors: context["complete"] = True # Update cookies with requests, in case they were changed. response = render_template(request, "account/edit.html", context) - return util.migrate_cookies(request, response) + return cookies.update_response_cookies(request, response, + aurtz=TZ, aurlang=L) account_template = ( diff --git a/aurweb/routers/auth.py b/aurweb/routers/auth.py index b3f9cc68..4e6a416a 100644 --- a/aurweb/routers/auth.py +++ b/aurweb/routers/auth.py @@ -6,7 +6,7 @@ from fastapi.responses import HTMLResponse, RedirectResponse import aurweb.config -from aurweb import util +from aurweb import cookies from aurweb.auth import auth_required from aurweb.models import User from aurweb.templates import make_variable_context, render_template @@ -42,12 +42,7 @@ async def login_post(request: Request, return await login_template(request, next, errors=["Bad username or password."]) - cookie_timeout = 0 - - if remember_me: - cookie_timeout = aurweb.config.getint( - "options", "persistent_cookie_timeout") - + cookie_timeout = cookies.timeout(remember_me) sid = user.login(request, passwd, cookie_timeout) if not sid: return await login_template(request, next, @@ -61,14 +56,17 @@ async def login_post(request: Request, response = RedirectResponse(url=next, status_code=HTTPStatus.SEE_OTHER) - secure_cookies = aurweb.config.getboolean("options", "disable_http_login") + secure = aurweb.config.getboolean("options", "disable_http_login") response.set_cookie("AURSID", sid, expires=expires_at, - secure=secure_cookies, httponly=True) + secure=secure, httponly=secure, + samesite=cookies.samesite()) response.set_cookie("AURTZ", user.Timezone, - secure=secure_cookies, httponly=True) + secure=secure, httponly=secure, + samesite=cookies.samesite()) response.set_cookie("AURLANG", user.LangPreference, - secure=secure_cookies, httponly=True) - return util.add_samesite_fields(response, "strict") + secure=secure, httponly=secure, + samesite=cookies.samesite()) + return response @router.get("/logout") diff --git a/aurweb/routers/html.py b/aurweb/routers/html.py index ed249794..c749ca67 100644 --- a/aurweb/routers/html.py +++ b/aurweb/routers/html.py @@ -11,7 +11,7 @@ from sqlalchemy import and_, case, or_ import aurweb.config import aurweb.models.package_request -from aurweb import db, models, util +from aurweb import cookies, db, models, util from aurweb.cache import db_count_cache from aurweb.models.account_type import TRUSTED_USER_AND_DEV_ID, TRUSTED_USER_ID from aurweb.models.package_request import PENDING_ID @@ -53,10 +53,11 @@ async def language(request: Request, # In any case, set the response's AURLANG cookie that never expires. response = RedirectResponse(url=f"{next}{query_string}", status_code=HTTPStatus.SEE_OTHER) - secure_cookies = aurweb.config.getboolean("options", "disable_http_login") + secure = aurweb.config.getboolean("options", "disable_http_login") response.set_cookie("AURLANG", set_lang, - secure=secure_cookies, httponly=True) - return util.add_samesite_fields(response, "strict") + secure=secure, httponly=secure, + samesite=cookies.samesite()) + return response @router.get("/", response_class=HTMLResponse) diff --git a/aurweb/templates.py b/aurweb/templates.py index 42e335b1..0039535d 100644 --- a/aurweb/templates.py +++ b/aurweb/templates.py @@ -16,7 +16,7 @@ from fastapi.responses import HTMLResponse import aurweb.config -from aurweb import captcha, l10n, time, util +from aurweb import captcha, cookies, l10n, time, util # Prepare jinja2 objects. _loader = jinja2.FileSystemLoader(os.path.join( @@ -148,9 +148,12 @@ def render_template(request: Request, """ Render a template as an HTMLResponse. """ rendered = render_raw_template(request, path, context) response = HTMLResponse(rendered, status_code=int(status_code)) - secure_cookies = aurweb.config.getboolean("options", "disable_http_login") - response.set_cookie("AURLANG", context.get("language"), - secure=secure_cookies, httponly=True) - response.set_cookie("AURTZ", context.get("timezone"), - secure=secure_cookies, httponly=True) - return util.add_samesite_fields(response, "strict") + + sid = None + if request.user.is_authenticated(): + sid = request.cookies.get("AURSID") + + # Re-emit SID via update_response_cookies with an updated expiration. + # This extends the life of a user session based on the AURREMEMBER + # cookie, which is always set to the "Remember Me" state on login. + return cookies.update_response_cookies(request, response, aursid=sid) diff --git a/aurweb/util.py b/aurweb/util.py index c24cc1d0..dd7491d3 100644 --- a/aurweb/util.py +++ b/aurweb/util.py @@ -14,7 +14,6 @@ from zoneinfo import ZoneInfo import fastapi from email_validator import EmailNotValidError, EmailUndeliverableError, validate_email -from fastapi.responses import Response from jinja2 import pass_context import aurweb.config @@ -103,16 +102,6 @@ def valid_ssh_pubkey(pk): return base64.b64encode(base64.b64decode(tokens[1])).decode() == tokens[1] -def migrate_cookies(request, response): - whitelist = {"AURSID", "AURTZ", "AURLANG"} - - secure_cookies = aurweb.config.getboolean("options", "disable_http_login") - for k, v in request.cookies.items(): - if k in whitelist: - response.set_cookie(k, v, secure=secure_cookies, httponly=True) - return add_samesite_fields(response, "strict") - - @pass_context def account_url(context, user): request = context.get("request") @@ -159,18 +148,6 @@ def jsonify(obj): return obj -def add_samesite_fields(response: Response, value: str): - """ Set the SameSite field on all cookie headers found. - Taken from https://github.com/tiangolo/fastapi/issues/1099. """ - for idx, header in enumerate(response.raw_headers): - if header[0].decode() == "set-cookie": - cookie = header[1].decode() - if f"SameSite={value}" not in cookie: - cookie += f"; SameSite={value}" - response.raw_headers[idx] = (header[0], cookie.encode()) - return response - - def get_ssh_fingerprints(): return aurweb.config.get_section("fingerprints") or {}