fix: make AURSID a session cookie if "remember me" is not checked

This should match more closely the expectation of a user.
A session cookie should vanish on browser close
and you thus they need to authenticate again.

There is no need to bump the expiration of AURSID either,
so we can remove that part.

Signed-off-by: moson-mo <mo-son@mailbox.org>

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

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:

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))
+    return 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)

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
+off of the "Remember Me" checkbox on the login page. If it was not
-use their own independent configuration for the number of seconds
+checked, we don't set Max-Age and it becomes a session cookie.
-that each type of session should stay alive.
+Otherwise we make it a persistent cookie and for the expiry date
-
+we use `options.persistent_cookie_timeout`.
-- "Remember Me" unchecked while logging in
+It indicates the number of seconds the session should live.
-    - `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.
 
 ### 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`

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
-
+            assert cookie.value == "False"
-    # 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
 
     # 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