Verified Commit bbb95e6f authored by Kevin Morris's avatar Kevin Morris
Browse files

fix(fastapi): handle sessions more properly



During our BasicAuthBackend execution, we now delete all
stale sessions before performing any checks. This simplifies
our flow.

Because we have simplified session handling, we are now
required to handle stale sessions in any session-altering
code, like account edit or user login.
Signed-off-by: Kevin Morris's avatarKevin Morris <kevr@0cost.org>
parent 7e7a1ead
Pipeline #12572 failed with stage
in 4 minutes and 34 seconds
......@@ -7,13 +7,12 @@ from http import HTTPStatus
import fastapi
from fastapi.responses import RedirectResponse
from sqlalchemy import and_
from starlette.authentication import AuthCredentials, AuthenticationBackend
from starlette.requests import HTTPConnection
import aurweb.config
from aurweb import l10n, util
from aurweb import db, l10n, util
from aurweb.models import Session, User
from aurweb.models.account_type import ACCOUNT_TYPE_ID
from aurweb.templates import make_variable_context, render_template
......@@ -95,29 +94,28 @@ class AnonymousUser:
class BasicAuthBackend(AuthenticationBackend):
async def authenticate(self, conn: HTTPConnection):
from aurweb.db import session
sid = conn.cookies.get("AURSID")
if not sid:
return (None, AnonymousUser())
now_ts = datetime.utcnow().timestamp()
record = session.query(Session).filter(
and_(Session.SessionID == sid,
Session.LastUpdateTS >= now_ts)).first()
# Clear out all expired sessions.
now_ts = int(datetime.utcnow().timestamp())
with db.begin():
db.delete(Session, Session.LastUpdateTS < now_ts)
# If no session with sid and a LastUpdateTS now or later exists.
if not record:
session = db.query(Session).filter(Session.SessionID == sid).first()
if not session:
return (None, AnonymousUser())
# At this point, we cannot have an invalid user if the record
# exists, due to ForeignKey constraints in the schema upheld
# by mysqlclient.
user = session.query(User).filter(User.ID == record.UsersID).first()
user.nonce = util.make_nonce()
user.authenticated = True
session.User.nonce = util.make_nonce()
session.User.authenticated = True
return (AuthCredentials(["authenticated"]), user)
return (AuthCredentials(["authenticated"]), session.User)
def auth_required(is_required: bool = True,
......@@ -165,15 +163,6 @@ def auth_required(is_required: bool = True,
if request.user.is_authenticated() != is_required:
url = "/"
if redirect:
path_params_expr = re.compile(r'\{(\w+)\}')
match = re.findall(path_params_expr, redirect)
args = {k: request.path_params.get(k) for k in match}
url = redirect.format(**args)
if login:
url = "/login?" + util.urlencode({"next": url})
if template:
# template=("template.html",
# ["Some Title", "someFormatted {}"],
......@@ -207,8 +196,18 @@ def auth_required(is_required: bool = True,
context = await make_variable_context(request, title)
return render_template(request, path, context,
status_code=status_code)
elif redirect:
path_params_expr = re.compile(r'\{(\w+)\}')
match = re.findall(path_params_expr, redirect)
args = {k: request.path_params.get(k) for k in match}
url = redirect.format(**args)
if login:
url = "/login?" + util.urlencode({"next": url})
return RedirectResponse(url,
status_code=int(HTTPStatus.SEE_OTHER))
return await func(request, *args, **kwargs)
return wrapper
......
......@@ -14,7 +14,8 @@ class Session(Base):
Integer, ForeignKey("Users.ID", ondelete="CASCADE"),
nullable=False)
User = relationship(
_User, backref=backref("session", uselist=False),
_User, backref=backref("session", uselist=False,
cascade="all, delete"),
foreign_keys=[UsersID])
__mapper_args__ = {"primary_key": [UsersID]}
......
......@@ -12,7 +12,7 @@ import aurweb.config
import aurweb.models.account_type
import aurweb.schema
from aurweb import db
from aurweb import config, db
from aurweb.models.account_type import AccountType as _AccountType
from aurweb.models.ban import is_banned
from aurweb.models.declarative import Base
......@@ -100,11 +100,11 @@ class User(Base):
def _login_approved(self, request: Request):
return not is_banned(request) and not self.Suspended
def login(self, request: Request, password: str, session_time=0):
def login(self, request: Request, password: str,
session_time: int = config.getint("options", "login_timeout")):
""" Login and authenticate a request. """
from aurweb import db
from aurweb.models.session import Session, generate_unique_sid
from aurweb.models import Session
from aurweb.models.session import generate_unique_sid
if not self._login_approved(request):
return None
......@@ -113,30 +113,26 @@ class User(Base):
if not self.authenticated:
return None
now_ts = datetime.utcnow().timestamp()
session_ts = now_ts + (
session_time if session_time
else aurweb.config.getint("options", "login_timeout")
)
sid = None
now_ts = int(datetime.utcnow().timestamp())
session_ts = now_ts + session_time
with db.begin():
self.LastLogin = now_ts
self.LastLoginIPAddress = request.client.host
if not self.session:
sid = generate_unique_sid()
self.session = Session(UsersID=self.ID, SessionID=sid,
if self.session is None:
# If no session exists, create a fresh one.
self.session = Session(UsersID=self.ID,
SessionID=generate_unique_sid(),
LastUpdateTS=session_ts)
db.add(self.session)
else:
last_updated = self.session.LastUpdateTS
if last_updated and last_updated < now_ts:
self.session.SessionID = sid = generate_unique_sid()
else:
# Session is still valid; retrieve the current SID.
sid = self.session.SessionID
# If a session record can be found but is expired,
# use it and update it's SessionID.
if self.session.LastUpdateTS < now_ts:
self.session.SessionID = generate_unique_sid()
# In all cases, we bump LastUpdateTS.
self.session.LastUpdateTS = session_ts
request.cookies["AURSID"] = self.session.SessionID
......
......@@ -467,6 +467,7 @@ async def account_edit_post(request: Request,
ON: bool = Form(default=False), # Owner Notify
passwd: str = Form(default=str())):
from aurweb.db import session
from aurweb.models import Session
user = session.query(models.User).filter(
models.User.Username == username).first()
......@@ -553,6 +554,9 @@ async def account_edit_post(request: Request,
with db.begin():
user.update_password(P)
if user == request.user:
db.delete(Session, Session.UsersID == user.ID)
if user == request.user:
# If the target user is the request user, login with
# the updated password and update AURSID.
......
......@@ -23,13 +23,13 @@ async def login_template(request: Request, next: str, errors: list = None):
@router.get("/login", response_class=HTMLResponse)
@auth_required(False)
@auth_required(False, login=False, redirect="/")
async def login_get(request: Request, next: str = "/"):
return await login_template(request, next)
@router.post("/login", response_class=HTMLResponse)
@auth_required(False)
@auth_required(False, login=False, redirect="/")
async def login_post(request: Request,
next: str = Form(...),
user: str = Form(default=str()),
......@@ -72,7 +72,7 @@ async def login_post(request: Request,
@router.get("/logout")
@auth_required()
@auth_required(login=False, redirect="/")
async def logout(request: Request, next: str = "/"):
""" A GET and POST route for logging out.
......@@ -92,6 +92,6 @@ async def logout(request: Request, next: str = "/"):
@router.post("/logout")
@auth_required()
@auth_required(login=False, redirect="/")
async def logout_post(request: Request, next: str = "/"):
return await logout(request=request, next=next)
......@@ -136,10 +136,15 @@ def test_authenticated_login_forbidden():
allow_redirects=False)
assert response.status_code == int(HTTPStatus.SEE_OTHER)
# Now, let's verify that we receive 403 Forbidden when we
sid = response.cookies.get("AURSID")
with client as request:
# Now, let's verify that we receive 303 See Other when we
# try to get /login as an authenticated user.
response = request.get("/login", allow_redirects=False)
response = request.get("/login", cookies={"AURSID": sid},
allow_redirects=False)
assert response.status_code == int(HTTPStatus.SEE_OTHER)
assert response.headers.get("location") == "/"
def test_unauthenticated_logout_unauthorized():
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment