diff --git a/aurweb/routers/auth.py b/aurweb/routers/auth.py index e4864424..4aca9304 100644 --- a/aurweb/routers/auth.py +++ b/aurweb/routers/auth.py @@ -59,7 +59,10 @@ async def login_post(request: Request, response = RedirectResponse(url=next, status_code=int(HTTPStatus.SEE_OTHER)) - response.set_cookie("AURSID", sid, expires=expires_at) + + secure_cookies = aurweb.config.getboolean("options", "disable_http_login") + response.set_cookie("AURSID", sid, expires=expires_at, + secure=secure_cookies, httponly=True) return response diff --git a/aurweb/routers/html.py b/aurweb/routers/html.py index 890aff88..ed0c039b 100644 --- a/aurweb/routers/html.py +++ b/aurweb/routers/html.py @@ -6,6 +6,8 @@ from http import HTTPStatus from fastapi import APIRouter, Form, HTTPException, Request from fastapi.responses import HTMLResponse, RedirectResponse +import aurweb.config + from aurweb.templates import make_context, render_template router = APIRouter() @@ -45,7 +47,9 @@ 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=int(HTTPStatus.SEE_OTHER)) - response.set_cookie("AURLANG", set_lang) + secure_cookies = aurweb.config.getboolean("options", "disable_http_login") + response.set_cookie("AURLANG", set_lang, + secure=secure_cookies, httponly=True) return response diff --git a/aurweb/routers/sso.py b/aurweb/routers/sso.py index 4b12b932..093807fe 100644 --- a/aurweb/routers/sso.py +++ b/aurweb/routers/sso.py @@ -131,13 +131,15 @@ async def authenticate(request: Request, redirect: str = None, conn=Depends(aurw elif len(aur_accounts) == 1: sid = open_session(request, conn, aur_accounts[0][Users.c.ID]) response = RedirectResponse(redirect if redirect and is_aur_url(redirect) else "/") + secure_cookies = aurweb.config.getboolean("options", "disable_http_login") response.set_cookie(key="AURSID", value=sid, httponly=True, - secure=request.url.scheme == "https") + secure=secure_cookies) if "id_token" in token: # We save the id_token for the SSO logout. It’s not too important # though, so if we can’t find it, we can live without it. - response.set_cookie(key="SSO_ID_TOKEN", value=token["id_token"], path="/sso/", - httponly=True, secure=request.url.scheme == "https") + response.set_cookie(key="SSO_ID_TOKEN", value=token["id_token"], + path="/sso/", httponly=True, + secure=secure_cookies) return response else: # We’ve got a severe integrity violation. diff --git a/aurweb/templates.py b/aurweb/templates.py index 015f8c9f..640b9447 100644 --- a/aurweb/templates.py +++ b/aurweb/templates.py @@ -88,7 +88,10 @@ def render_template(request: Request, template = templates.get_template(path) rendered = template.render(context) - response = HTMLResponse(rendered, status_code=int(status_code)) - response.set_cookie("AURLANG", context.get("language")) - response.set_cookie("AURTZ", context.get("timezone")) + response = HTMLResponse(rendered, status_code=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 response diff --git a/aurweb/util.py b/aurweb/util.py index 0aec6f45..1da85606 100644 --- a/aurweb/util.py +++ b/aurweb/util.py @@ -85,8 +85,9 @@ def valid_ssh_pubkey(pk): def migrate_cookies(request, response): + secure_cookies = aurweb.config.getboolean("options", "disable_http_login") for k, v in request.cookies.items(): - response.set_cookie(k, v) + response.set_cookie(k, v, secure=secure_cookies, httponly=True) return response diff --git a/test/test_auth_routes.py b/test/test_auth_routes.py index 360b48cc..a443be72 100644 --- a/test/test_auth_routes.py +++ b/test/test_auth_routes.py @@ -1,5 +1,6 @@ from datetime import datetime from http import HTTPStatus +from unittest import mock import pytest @@ -70,6 +71,55 @@ def test_login_logout(): assert "AURSID" not in response.cookies +def mock_getboolean(a, b): + if a == "options" and b == "disable_http_login": + return True + return bool(aurweb.config.get(a, b)) + + +@mock.patch("aurweb.config.getboolean", side_effect=mock_getboolean) +def test_secure_login(mock): + """ In this test, we check to verify the course of action taken + by starlette when providing secure=True to a response cookie. + This is achieved by mocking aurweb.config.getboolean to return + True (or 1) when looking for `options.disable_http_login`. + When we receive a response with `disable_http_login` enabled, + we check the fields in cookies received for the secure and + httponly fields, in addition to the rest of the fields given + on such a request. """ + + # Create a local TestClient here since we mocked configuration. + client = TestClient(app) + + # Data used for our upcoming http post request. + post_data = { + "user": user.Username, + "passwd": "testPassword", + "next": "/" + } + + # Perform a login request with the data matching our user. + with client as request: + response = request.post("/login", data=post_data, + allow_redirects=False) + + # Make sure we got the expected status out of it. + assert response.status_code == int(HTTPStatus.SEE_OTHER) + + # Let's check what we got in terms of cookies for AURSID. + # Make sure that a secure cookie got passed to us. + cookie = next(c for c in response.cookies if c.name == "AURSID") + assert cookie.secure is True + assert cookie.has_nonstandard_attr("HttpOnly") is True + assert cookie.value is not None and len(cookie.value) > 0 + + # Let's make sure we actually have a session relationship + # with the AURSID we ended up with. + record = query(Session, Session.SessionID == cookie.value).first() + assert record is not None and record.User == user + assert user.session == record + + def test_authenticated_login_forbidden(): post_data = { "user": "test",