diff --git a/aurweb/cookies.py b/aurweb/cookies.py index cb4396d7..022cff1e 100644 --- a/aurweb/cookies.py +++ b/aurweb/cookies.py @@ -1,6 +1,3 @@ -from fastapi import Request -from fastapi.responses import Response - from aurweb import config @@ -33,33 +30,3 @@ def timeout(extended: bool) -> int: if bool(extended): timeout = config.getint("options", "persistent_cookie_timeout") return timeout - - -def update_response_cookies( - request: Request, - response: Response, - 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 aursid: Optional AURSID cookie value - :returns: Updated response - """ - secure = config.getboolean("options", "disable_http_login") - if aursid: - remember_me = request.cookies.get("AURREMEMBER") == "True" - response.set_cookie( - "AURSID", - aursid, - secure=secure, - httponly=secure, - max_age=timeout(remember_me), - samesite=samesite(), - ) - return response diff --git a/aurweb/routers/auth.py b/aurweb/routers/auth.py index 98a655e3..46dee3a4 100644 --- a/aurweb/routers/auth.py +++ b/aurweb/routers/auth.py @@ -69,7 +69,12 @@ async def login_post( if user.Suspended: return await login_template(request, next, errors=["Account Suspended"]) - cookie_timeout = cookies.timeout(remember_me) + # If "remember me" was not ticked, we set a session cookie for AURSID, + # otherwise we make it a persistent cookie + cookie_timeout = None + if remember_me: + cookie_timeout = aurweb.config.getint("options", "persistent_cookie_timeout") + perma_timeout = aurweb.config.getint("options", "permanent_cookie_timeout") sid = _retry_login(request, user, passwd, cookie_timeout) if not sid: diff --git a/aurweb/templates.py b/aurweb/templates.py index 89316d6d..d20cbe85 100644 --- a/aurweb/templates.py +++ b/aurweb/templates.py @@ -10,7 +10,7 @@ from fastapi import Request from fastapi.responses import HTMLResponse import aurweb.config -from aurweb import cookies, l10n, time +from aurweb import l10n, time # Prepare jinja2 objects. _loader = jinja2.FileSystemLoader( @@ -145,13 +145,4 @@ def render_template( ): """Render a template as an HTMLResponse.""" rendered = render_raw_template(request, path, context) - response = HTMLResponse(rendered, status_code=int(status_code)) - - 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) + return HTMLResponse(rendered, status_code=int(status_code)) diff --git a/doc/web-auth.md b/doc/web-auth.md index dbb4403d..c8604fed 100644 --- a/doc/web-auth.md +++ b/doc/web-auth.md @@ -22,17 +22,11 @@ in the following ways: ### Max-Age The value used for the `AURSID` Max-Age attribute is decided based -off of the "Remember Me" checkbox on the login page. Both paths -use their own independent configuration for the number of seconds -that each type of session should stay alive. - -- "Remember Me" unchecked while logging in - - `options.login_timeout` is used -- "Remember Me" checked while logging in - - `options.persistent_cookie_timeout` is used - -Both `options.login_timeout` and `options.persistent_cookie_timeout` -indicate the number of seconds the session should live. +off of the "Remember Me" checkbox on the login page. If it was not +checked, we don't set Max-Age and it becomes a session cookie. +Otherwise we make it a persistent cookie and for the expiry date +we use `options.persistent_cookie_timeout`. +It indicates the number of seconds the session should live. ### Notes @@ -89,7 +83,7 @@ The following list of steps describes exactly how this verification works: 1. Was the `AURSID` cookie delivered? 1. No, the algorithm ends, you are considered unauthenticated 2. Yes, move on to 2 -2. Was the `AURREMEMBER` cookie delivered with a value of 1? +2. Was the `AURREMEMBER` cookie delivered with a value of `True`? 1. No, set the expected session timeout **T** to `options.login_timeout` 2. Yes, set the expected session timeout **T** to `options.persistent_cookie_timeout` diff --git a/test/test_cookies.py b/test/test_cookies.py index a0ace68f..dd4143cb 100644 --- a/test/test_cookies.py +++ b/test/test_cookies.py @@ -1,4 +1,5 @@ from datetime import datetime +from http import HTTPStatus import pytest from fastapi.testclient import TestClient @@ -56,7 +57,6 @@ def test_cookies_login(client: TestClient, user: User): resp = request.post("/login", data=data) local_time = int(datetime.now().timestamp()) - expected_timeout = local_time + config.getint("options", "login_timeout") expected_permanent = local_time + config.getint( "options", "permanent_cookie_timeout" ) @@ -64,22 +64,15 @@ def test_cookies_login(client: TestClient, user: User): # Check if we got permanent cookies with expected expiry date. # Allow 1 second difference to account for timing issues # between the request and current time. + # AURSID should be a session cookie (no expiry date) assert "AURSID", "AURREMEMBER" in resp.cookies for cookie in resp.cookies.jar: if cookie.name == "AURSID": - assert abs(cookie.expires - expected_timeout) < 2 + assert cookie.expires is None if cookie.name == "AURREMEMBER": assert abs(cookie.expires - expected_permanent) < 2 - - # Make some random http call. - # We should get an (updated) AURSID cookie with each request. - sid = resp.cookies.get("AURSID") - with client as request: - request.cookies = {"AURSID": sid} - resp = request.get("/") - - assert "AURSID" in resp.cookies + assert cookie.value == "False" # Log out with client as request: @@ -102,14 +95,27 @@ def test_cookies_login(client: TestClient, user: User): # Check if we got a permanent cookie with expected expiry date. # Allow 1 second difference to account for timing issues # between the request and current time. + # AURSID should be a persistent cookie expected_persistent = local_time + config.getint( "options", "persistent_cookie_timeout" ) - assert "AURSID" in resp.cookies + assert "AURSID", "AURREMEMBER" in resp.cookies for cookie in resp.cookies.jar: if cookie.name in "AURSID": assert abs(cookie.expires - expected_persistent) < 2 + if cookie.name == "AURREMEMBER": + assert abs(cookie.expires - expected_permanent) < 2 + assert cookie.value == "True" + + # log in again even though we already have a session + with client as request: + resp = request.post("/login", data=data) + + # we are logged in already and should have been redirected + assert resp.status_code == int(HTTPStatus.SEE_OTHER) + assert resp.headers.get("location") == "/" + def test_cookie_language(client: TestClient, user: User): # Unauthenticated reqeuests should set AURLANG cookie