diff --git a/.env.example b/.env.example new file mode 100644 index 00000000..02a302c6 --- /dev/null +++ b/.env.example @@ -0,0 +1,11 @@ +# Required +# Supports `dev`, `prod`, or `test` +ENVIRONMENT=dev +DB_PORT=5444 + +# Authentication +COOKIE_SECURE=false +AUTH_URL=https://cas.sfu.ca/cas/serviceValidate + +# Redirect after authentication +FRONTEND_ORIGIN=http://localhost:8080 diff --git a/.github/workflows/alembic.yml b/.github/workflows/alembic.yml index 860422c2..3ab50195 100644 --- a/.github/workflows/alembic.yml +++ b/.github/workflows/alembic.yml @@ -4,6 +4,11 @@ on: pull_request jobs: alembic_upgrade_head: runs-on: ubuntu-latest + env: + ENVIRONMENT: dev + DB_PORT: "5432" + COOKIE_SECURE: "false" + FRONTEND_ORIGIN: http://localhost:8080 services: postgres: @@ -32,19 +37,18 @@ jobs: sleep 1 done + - uses: astral-sh/setup-uv@v6 + with: + enable-cache: true + + - name: Install Python + run: uv python install 3.13 + - name: Install dependencies - run: | - sudo add-apt-repository ppa:deadsnakes/ppa - sudo apt install python3.13 python3.13-venv - python3.13 -m pip install --upgrade pip - python3.13 -m venv venv - source ./venv/bin/activate - pip install . - - # This will fail if there are divergent heads and alembic gets confused; - # e.g., un-sanitarily merging main into a dev branch. + run: uv sync --locked + + # This will fail if there are divergent heads and alembic gets confused + # e.g. merging into main without passing this check - name: Run alembic upgrade head - run: | - source ./venv/bin/activate - cd src - alembic upgrade head + working-directory: src + run: uv run alembic upgrade head diff --git a/.github/workflows/pytest_unit.yml b/.github/workflows/pytest_unit.yml index d0cebdba..32cc172b 100644 --- a/.github/workflows/pytest_unit.yml +++ b/.github/workflows/pytest_unit.yml @@ -5,18 +5,21 @@ jobs: unit_tests: runs-on: ubuntu-latest timeout-minutes: 5 + env: + ENVIRONMENT: test + COOKIE_SECURE: "false" + FRONTEND_ORIGIN: http://localhost:8080 steps: - uses: actions/checkout@v6 - - uses: actions/setup-python@v6 - with: - python-version: '3.13' - - uses: astral-sh/setup-uv@v6 with: enable-cache: true + - name: Install Python + run: uv python install 3.13 + - name: Install dependencies run: uv sync --extra test --locked diff --git a/.github/workflows/ruff.yml b/.github/workflows/ruff.yml index b213240b..f8999fa7 100644 --- a/.github/workflows/ruff.yml +++ b/.github/workflows/ruff.yml @@ -5,4 +5,16 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@v6 - - uses: astral-sh/ruff-action@v3 + + - uses: astral-sh/setup-uv@v6 + with: + enable-cache: true + + - name: Install Python + run: uv python install 3.13 + + - name: Install dependencies + run: uv sync --extra dev --locked + + - name: Run Ruff + run: uv run ruff check . diff --git a/README.md b/README.md index d2d0cb5c..61e1c54c 100755 --- a/README.md +++ b/README.md @@ -33,26 +33,27 @@ pip install ".[dev, test]" # or: uv sync --all-extras ``` 5. Follow the database setup instructions on the [wiki](https://github.com/CSSS/csss-site-backend/wiki/1.-Local-Setup#database-setup). The recommended way is to do it through Docker, but both should work. -6. You will need to set the following environment variables +6. You will need to set the following environment variables set ```bash -export DB_PORT=5444 # If you're using Docker -export LOCAL=true # Should be true if you're running this locally +DB_PORT=5444 # If you're using Docker +ENVIRONMENT=dev # Set this to `test` if you want to use the test database instead ``` +You can also create a `.env` file and set those in there. See `.env.example` for more information. ## Important Directories - `config/` configuration files for the server machine - `src/` - - `alembic` for database migrations - - `access/` for controlling officer access to the google drive, bitwarden, and github. TODO: discord as well? - - `blog/` for running an editable csss-blog - - `dashboard/` for controlling the server, website, jobs, and access to services & stuff. - - `elections/` for the mangement of current elections & past elections - - `jobs/` for cronjobs that run regularly on the server - - `misc/` for anything that can't be easily categorized or is very small - - `officers/` for officer contact information + photos -- `test/` for html pages which interact with the backend's local api + - `alembic` database migrations + - `auth/` controlling authentication and sessions + - `candidates/` management of those who run in large elections + - `elections/` mangement of current elections & past elections + - `event/` management of events + - `nominees/` management of nominees to large elections + - `officers/` management of officer information and terms + - `translink/` management of TransLink REST API +- `test/` unit and integration tests ## Developer Tools diff --git a/pyproject.toml b/pyproject.toml index 88fcc07f..9eb1503b 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -37,14 +37,22 @@ Homepage = "https://api.sfucsss.org/" # Pytest: Test framework [tool.pytest.ini_options] pythonpath = ["src"] +testpaths = ["tests"] +norecursedirs = ["tests/wip"] # Don't test these paths +asyncio_mode = "auto" +asyncio_default_fixture_loop_scope = "function" +asyncio_default_test_loop_scope = "function" log_cli = true log_cli_level = "INFO" -testpaths = [ - "tests", +addopts = [ + "--strict-markers", + "--strict-config", + "--tb=short" +] +markers = [ + "integration: tests that require database or full app setup", + "unit: isolated tests that do not require external services" ] -norecursedirs = "tests/wip" -asyncio_mode = "auto" -asyncio_default_fixture_loop_scope = "function" # Ruff: Formatter and linter [tool.ruff] diff --git a/src/alembic/versions/060623cf940f_change_session_id_to_be_32_bytes_of_.py b/src/alembic/versions/060623cf940f_change_session_id_to_be_32_bytes_of_.py new file mode 100644 index 00000000..1cc75c83 --- /dev/null +++ b/src/alembic/versions/060623cf940f_change_session_id_to_be_32_bytes_of_.py @@ -0,0 +1,38 @@ +"""change session ID to be 32 bytes of base64 + +Revision ID: 060623cf940f +Revises: f0c99d0db277 +Create Date: 2026-07-01 01:17:28.430756 + +""" +from typing import Sequence, Union + +from alembic import op +import sqlalchemy as sa + +from constants import SESSION_ID_LEN + + +# revision identifiers, used by Alembic. +revision: str = '060623cf940f' +down_revision: Union[str, None] = 'f0c99d0db277' +branch_labels: Union[str, Sequence[str], None] = None +depends_on: Union[str, Sequence[str], None] = None + + +def upgrade() -> None: + # ### commands auto generated by Alembic - please adjust! ### + op.alter_column('user_session', 'session_id', + existing_type=sa.VARCHAR(length=512), + type_=sa.String(length=SESSION_ID_LEN), + existing_nullable=False) + # ### end Alembic commands ### + + +def downgrade() -> None: + # ### commands auto generated by Alembic - please adjust! ### + op.alter_column('user_session', 'session_id', + existing_type=sa.String(length=SESSION_ID_LEN), + type_=sa.VARCHAR(length=512), + existing_nullable=False) + # ### end Alembic commands ### diff --git a/src/alembic/versions/caa26d56d842_remove_site_profile_pics.py b/src/alembic/versions/caa26d56d842_remove_site_profile_pics.py new file mode 100644 index 00000000..50f0b33b --- /dev/null +++ b/src/alembic/versions/caa26d56d842_remove_site_profile_pics.py @@ -0,0 +1,30 @@ +"""remove site profile pics + +Revision ID: caa26d56d842 +Revises: 060623cf940f +Create Date: 2026-07-01 02:01:26.412179 + +""" +from typing import Sequence, Union + +from alembic import op +import sqlalchemy as sa + + +# revision identifiers, used by Alembic. +revision: str = 'caa26d56d842' +down_revision: Union[str, None] = '060623cf940f' +branch_labels: Union[str, Sequence[str], None] = None +depends_on: Union[str, Sequence[str], None] = None + + +def upgrade() -> None: + # ### commands auto generated by Alembic - please adjust! ### + op.drop_column('site_user', 'profile_picture_url') + # ### end Alembic commands ### + + +def downgrade() -> None: + # ### commands auto generated by Alembic - please adjust! ### + op.add_column('site_user', sa.Column('profile_picture_url', sa.TEXT(), autoincrement=False, nullable=True)) + # ### end Alembic commands ### diff --git a/src/auth/__init__.py b/src/auth/__init__.py deleted file mode 100644 index 2bad3942..00000000 --- a/src/auth/__init__.py +++ /dev/null @@ -1 +0,0 @@ -from auth import crud diff --git a/src/auth/constants.py b/src/auth/constants.py new file mode 100644 index 00000000..de5b3bfb --- /dev/null +++ b/src/auth/constants.py @@ -0,0 +1,5 @@ +from datetime import timedelta + +COOKIE_SESSION_KEY = "session_id" +COOKIE_MAX_AGE = 60 * 60 * 2 # 2 hours in seconds +SESSION_MAX_AGE = timedelta(seconds=COOKIE_MAX_AGE) diff --git a/src/auth/crud.py b/src/auth/crud.py index 88f28bb6..e9536eeb 100644 --- a/src/auth/crud.py +++ b/src/auth/crud.py @@ -1,9 +1,11 @@ import logging -from datetime import UTC, datetime, timedelta +from datetime import UTC, datetime import sqlalchemy +from sqlalchemy.dialects.postgresql import insert from sqlalchemy.ext.asyncio import AsyncSession +from auth.constants import SESSION_MAX_AGE from auth.tables import SiteUserDB, UserSessionDB _logger = logging.getLogger(__name__) @@ -15,56 +17,66 @@ async def create_user_session(db_session: AsyncSession, session_id: str, computi Also, adds the new user to the SiteUser table if it's their first time logging in. """ - existing_user_session = await db_session.scalar( - sqlalchemy.select(UserSessionDB).where(UserSessionDB.computing_id == computing_id) + now = datetime.now(UTC) + + # Upsert the site user + # Create a new user if it's their first login... + user_query = insert(SiteUserDB).values( + computing_id=computing_id, + first_logged_in=now, + last_logged_in=now, ) - existing_user = await db_session.scalar( - sqlalchemy.select(SiteUserDB).where(SiteUserDB.computing_id == computing_id) + # ...or just update their "last_logged_in" time + user_query = user_query.on_conflict_do_update( + index_elements=[SiteUserDB.computing_id], set_={"last_logged_in": now} ) - - if existing_user is None: - if existing_user_session is not None: - # log this strange case that shouldn't be possible - _logger.warning(f"User session {session_id} exists for non-existent user {computing_id} ... !") - - # add new user to User table if it's their first time logging in - db_session.add( - SiteUserDB(computing_id=computing_id, first_logged_in=datetime.now(UTC), last_logged_in=datetime.now(UTC)) - ) - - if existing_user_session is not None: - existing_user_session.issue_time = datetime.now(UTC) - existing_user_session.session_id = session_id - if existing_user is not None: - # update the last time the user logged in to now - existing_user.last_logged_in = datetime.now(UTC) - else: - db_session.add( - UserSessionDB( - session_id=session_id, - computing_id=computing_id, - issue_time=datetime.now(UTC), - ) - ) + await db_session.execute(user_query) + + # Upsert the user session + # Create a new session... + session_query = insert(UserSessionDB).values( + session_id=session_id, + computing_id=computing_id, + issue_time=now, + ) + # ...or update their current session + session_query = session_query.on_conflict_do_update( + index_elements=[UserSessionDB.computing_id], set_={"session_id": session_id, "issue_time": now} + ) + await db_session.execute(session_query) async def remove_user_session(db_session: AsyncSession, session_id: str): query = sqlalchemy.select(UserSessionDB).where(UserSessionDB.session_id == session_id) - user_session = await db_session.scalars(query) - await db_session.delete(user_session.first()) + user_session = await db_session.scalar(query) + if user_session is not None: + await db_session.delete(user_session) + +async def get_session_computing_id(db_session: AsyncSession, session_id: str) -> str | None: + """ + Retrieves the computing ID from a session. + + Args: + db_session: database transaction + session_id: session ID the computing ID is using -async def get_computing_id(db_session: AsyncSession, session_id: str) -> str | None: + Returns: + The computing ID associated with the session, or None if the session is invalid or expired. + """ query = sqlalchemy.select(UserSessionDB).where(UserSessionDB.session_id == session_id) existing_user_session = (await db_session.scalars(query)).first() - return existing_user_session.computing_id if existing_user_session else None + if existing_user_session is None or existing_user_session.issue_time < datetime.now(UTC) - SESSION_MAX_AGE: + return None + + return existing_user_session.computing_id # remove all out of date user sessions async def task_clean_expired_user_sessions(db_session: AsyncSession): - one_day_ago = datetime.now(UTC) - timedelta(days=0.5) + expiration = datetime.now(UTC) - SESSION_MAX_AGE - query = sqlalchemy.delete(UserSessionDB).where(UserSessionDB.issue_time < one_day_ago) + query = sqlalchemy.delete(UserSessionDB).where(UserSessionDB.issue_time < expiration) await db_session.execute(query) await db_session.commit() @@ -73,7 +85,8 @@ async def task_clean_expired_user_sessions(db_session: AsyncSession): async def get_site_user(db_session: AsyncSession, session_id: str) -> SiteUserDB | None: query = sqlalchemy.select(UserSessionDB).where(UserSessionDB.session_id == session_id) user_session = await db_session.scalar(query) - if user_session is None: + + if user_session is None or user_session.issue_time < datetime.now(UTC) - SESSION_MAX_AGE: return None query = sqlalchemy.select(SiteUserDB).where(SiteUserDB.computing_id == user_session.computing_id) @@ -81,22 +94,5 @@ async def get_site_user(db_session: AsyncSession, session_id: str) -> SiteUserDB async def site_user_exists(db_session: AsyncSession, computing_id: str) -> bool: - user = await db_session.scalar(sqlalchemy.select(SiteUserDB).where(SiteUserDB.computing_id == computing_id)) + user = await db_session.get(SiteUserDB, computing_id) return user is not None - - -# update the optional user info for a given site user (e.g., display name, profile picture, ...) -async def update_site_user(db_session: AsyncSession, session_id: str, profile_picture_url: str) -> bool: - query = sqlalchemy.select(UserSessionDB).where(UserSessionDB.session_id == session_id) - user_session = await db_session.scalar(query) - if user_session is None: - return False - - query = ( - sqlalchemy.update(SiteUserDB) - .where(SiteUserDB.computing_id == user_session.computing_id) - .values(profile_picture_url=profile_picture_url) - ) - await db_session.execute(query) - - return True diff --git a/src/auth/models.py b/src/auth/models.py index 15bfd35a..f3647f2a 100644 --- a/src/auth/models.py +++ b/src/auth/models.py @@ -1,26 +1,28 @@ from datetime import datetime -from pydantic import BaseModel, Field +from pydantic import BaseModel, ConfigDict, Field + +from constants import COMPUTING_ID_LEN, SESSION_ID_LEN class LoginBodyParams(BaseModel): service: str = Field(description="Service URL used for SFU's CAS system") ticket: str = Field(description="Ticket return from SFU's CAS system") - redirect_url: str | None = Field(None, description="Optional redirect URL") -class UpdateUserParams(BaseModel): - profile_picture_url: str +class UserBaseModel(BaseModel): + computing_id: str = Field(..., max_length=COMPUTING_ID_LEN, description="Student's computing ID") + + +class UserSession(UserBaseModel): + model_config = ConfigDict(from_attributes=True) + issue_time: datetime = Field(..., description="Time the session was created") + session_id: str = Field(..., max_length=SESSION_ID_LEN, description="Unique session ID of the user") -class UserSessionModel(BaseModel): - computing_id: str - issue_time: datetime - session_id: str +class SiteUser(UserBaseModel): + model_config = ConfigDict(from_attributes=True) -class SiteUserModel(BaseModel): - computing_id: str - first_logged_in: datetime - last_logged_in: datetime - profile_picture_url: str | None = None + first_logged_in: datetime | None = Field(..., description="Time the user was created") + last_logged_in: datetime | None = Field(..., description="Time the user last logged in") diff --git a/src/auth/tables.py b/src/auth/tables.py index f553f3a7..22d6c124 100644 --- a/src/auth/tables.py +++ b/src/auth/tables.py @@ -1,6 +1,6 @@ from datetime import datetime -from sqlalchemy import DateTime, ForeignKey, String, Text, func +from sqlalchemy import DateTime, ForeignKey, String, func from sqlalchemy.orm import Mapped, mapped_column from constants import COMPUTING_ID_LEN, SESSION_ID_LEN @@ -23,7 +23,7 @@ class UserSessionDB(Base): session_id: Mapped[str] = mapped_column( String(SESSION_ID_LEN), nullable=False, unique=True - ) # the space needed to store 256 bytes in base64 + ) # the space needed to store 32 bytes in base64 class SiteUserDB(Base): @@ -39,15 +39,3 @@ class SiteUserDB(Base): # first and last time logged into the CSSS API first_logged_in: Mapped[datetime | None] = mapped_column(DateTime(timezone=True), nullable=True) last_logged_in: Mapped[datetime | None] = mapped_column(DateTime(timezone=True), nullable=True) - - # optional user information for display purposes - profile_picture_url: Mapped[str | None] = mapped_column(Text, nullable=True) - - def serialize(self) -> dict[str, str | int | bool | None]: - res = {"computing_id": self.computing_id, "profile_picture_url": self.profile_picture_url} - if self.first_logged_in is not None: - res["first_logged_in"] = self.first_logged_in.isoformat() - if self.last_logged_in is not None: - res["last_logged_in"] = self.last_logged_in.isoformat() - - return res diff --git a/src/auth/types.py b/src/auth/types.py deleted file mode 100644 index 5e478e3d..00000000 --- a/src/auth/types.py +++ /dev/null @@ -1,17 +0,0 @@ -from dataclasses import dataclass - - -@dataclass -class SiteUserData: - computing_id: str - first_logged_in: str - last_logged_in: str - profile_picture_url: None | str - - def serializable_dict(self): - return { - "computing_id": self.computing_id, - "first_logged_in": self.first_logged_in, - "last_logged_in": self.last_logged_in, - "profile_picture_url": self.profile_picture_url, - } diff --git a/src/auth/urls.py b/src/auth/urls.py index cb7b622d..30b43d80 100644 --- a/src/auth/urls.py +++ b/src/auth/urls.py @@ -4,13 +4,14 @@ import urllib.parse import xmltodict -from fastapi import APIRouter, BackgroundTasks, HTTPException, Request, Response -from fastapi.responses import JSONResponse, RedirectResponse +from fastapi import APIRouter, BackgroundTasks, HTTPException, Request, Response, status +from fastapi.responses import JSONResponse import database from auth import crud -from auth.models import LoginBodyParams, SiteUserModel, UpdateUserParams -from constants import DOMAIN, IS_PROD, SAMESITE +from auth.constants import COOKIE_MAX_AGE, COOKIE_SESSION_KEY +from auth.models import LoginBodyParams, SiteUser +from config import settings from utils.shared_models import DetailModel, MessageModel _logger = logging.getLogger(__name__) @@ -19,9 +20,8 @@ # utils -# ex: rsa4096 is 512 bytes -def generate_session_id_b64(num_bytes: int) -> str: - return base64.b64encode(os.urandom(num_bytes)).decode("utf-8") +def _generate_session_id() -> str: + return base64.urlsafe_b64encode(os.urandom(32)).decode("utf-8").rstrip("=") # ----------------------- # @@ -37,12 +37,12 @@ def generate_session_id_b64(num_bytes: int) -> str: @router.post( "/login", description="Create a login session.", - response_description="Successfully validated with SFU's CAS", - response_model=str, + status_code=status.HTTP_204_NO_CONTENT, responses={ - 307: {"description": "Successful validation, with redirect"}, - 400: {"description": "Origin is missing.", "model": DetailModel}, + 204: {"description": "Successfully validated with SFU's CAS"}, 401: {"description": "Failed to validate ticket with SFU's CAS", "model": DetailModel}, + 502: {"description": "Failed to connect to SFU's CAS", "model": DetailModel}, + 503: {"description": "Authentication not configured", "model": DetailModel}, }, operation_id="login", ) @@ -50,39 +50,55 @@ async def login_user( request: Request, db_session: database.DBSession, background_tasks: BackgroundTasks, body: LoginBodyParams ): # verify the ticket is valid - service_url = body.service - service = urllib.parse.quote(service_url) - service_validate_url = f"https://cas.sfu.ca/cas/serviceValidate?service={service}&ticket={body.ticket}" + service = urllib.parse.quote(body.service) + ticket = urllib.parse.quote(body.ticket) + if not settings.auth_url: + raise HTTPException(status_code=status.HTTP_503_SERVICE_UNAVAILABLE, detail="authentication error") + service_validate_url = f"{settings.auth_url}?service={service}&ticket={ticket}" client = request.app.state.http_client - response = await client.get(service_validate_url) - cas_response = xmltodict.parse(response.text) - if "cas:authenticationFailure" in cas_response["cas:serviceResponse"]: - _logger.info(f"User failed to login, with response {cas_response}") - raise HTTPException(status_code=401, detail="authentication error") - else: - session_id = generate_session_id_b64(256) - computing_id = cas_response["cas:serviceResponse"]["cas:authenticationSuccess"]["cas:user"] - - await crud.create_user_session(db_session, session_id, computing_id) - await db_session.commit() - - # clean old sessions after sending the response - background_tasks.add_task(crud.task_clean_expired_user_sessions, db_session) - - if body.redirect_url: - origin = request.headers.get("origin") - if origin: - response = RedirectResponse(origin + body.redirect_url) - else: - raise HTTPException(status_code=400, detail="bad origin") - else: - response = Response() - - response.set_cookie( - key="session_id", value=session_id, secure=IS_PROD, httponly=True, samesite=SAMESITE, domain=DOMAIN - ) # this overwrites any past, possibly invalid, session_id - return response + try: + response = await client.get(service_validate_url) + response.raise_for_status() + cas_response = xmltodict.parse(response.text) + except Exception: + _logger.exception(f"CAS Login failure: service={body.service}") + raise HTTPException(status_code=status.HTTP_502_BAD_GATEWAY, detail="authentication error") from None + + service_response = cas_response.get("cas:serviceResponse") + if not isinstance(service_response, dict): + raise HTTPException(status_code=status.HTTP_502_BAD_GATEWAY, detail="authentication error") + + if "cas:authenticationFailure" in service_response: + _logger.info(f"CAS Login failure: service={body.service}") + raise HTTPException(status_code=status.HTTP_401_UNAUTHORIZED, detail="authentication error") + + auth_success = service_response.get("cas:authenticationSuccess") + if not isinstance(auth_success, dict): + raise HTTPException(status_code=status.HTTP_502_BAD_GATEWAY, detail="authentication error") + + session_id = _generate_session_id() + computing_id = auth_success.get("cas:user") + if not isinstance(computing_id, str) or not computing_id: + raise HTTPException(status_code=status.HTTP_502_BAD_GATEWAY, detail="authentication error") + + # clean old sessions after sending the response + # TODO: Convert this to a daily CRON job + background_tasks.add_task(crud.task_clean_expired_user_sessions, db_session) + + response = Response(status_code=status.HTTP_204_NO_CONTENT) + response.set_cookie( + key=COOKIE_SESSION_KEY, + value=session_id, + secure=settings.cookie_secure, + httponly=True, + samesite="strict", + domain=settings.cookie_domain, + max_age=COOKIE_MAX_AGE, + ) # this overwrites any past, possibly invalid, session_id + await crud.create_user_session(db_session, session_id, computing_id) + await db_session.commit() + return response @router.get( @@ -96,16 +112,21 @@ async def logout_user( db_session: database.DBSession, ): session_id = request.cookies.get("session_id", None) + response_dict = {"message": "logout successful"} - if session_id: - await crud.remove_user_session(db_session, session_id) - await db_session.commit() - response_dict = {"message": "logout successful"} - else: - response_dict = {"message": "user was not logged in"} + if not session_id: + return JSONResponse(response_dict) + await crud.remove_user_session(db_session, session_id) response = JSONResponse(response_dict) - response.delete_cookie(key="session_id") + response.delete_cookie( + key=COOKIE_SESSION_KEY, + domain=settings.cookie_domain, + secure=settings.cookie_secure, + httponly=True, + samesite="strict", + ) + await db_session.commit() return response @@ -113,7 +134,7 @@ async def logout_user( "/user", operation_id="get_user", description="Get info about the current user. Only accessible by that user", - response_model=SiteUserModel, + response_model=SiteUser, responses={401: {"description": "Not logged in.", "model": DetailModel}}, ) async def get_user( @@ -125,36 +146,12 @@ async def get_user( """ session_id = request.cookies.get("session_id", None) if session_id is None: - raise HTTPException(status_code=401, detail="user must be authenticated to get their info") + raise HTTPException( + status_code=status.HTTP_401_UNAUTHORIZED, detail="user must be authenticated to get their info" + ) user_info = await crud.get_site_user(db_session, session_id) if user_info is None: - raise HTTPException(status_code=401, detail="could not find user with session_id, please log in") - - return JSONResponse(user_info.serialize()) - + raise HTTPException(status_code=status.HTTP_401_UNAUTHORIZED, detail="could not find user with session_id") -# TODO: We should change this so that the admins can change people's pictures too, so they can remove offensive stuff -@router.patch( - "/user", - operation_id="update_user", - description="Update information for the currently logged in user. Only accessible by that user", - response_model=str, - responses={401: {"description": "Not logged in.", "model": DetailModel}}, -) -async def update_user( - body: UpdateUserParams, - request: Request, - db_session: database.DBSession, -): - """ - Returns the info stored in the site_user table in the auth module, if the user is logged in. - """ - session_id = request.cookies.get("session_id") - if session_id is None: - raise HTTPException(status_code=401, detail="user must be authenticated to get their info") - - ok = await crud.update_site_user(db_session, session_id, body.profile_picture_url) - await db_session.commit() - if not ok: - raise HTTPException(status_code=401, detail="could not find user with session_id, please log in") + return user_info diff --git a/src/config.py b/src/config.py index 903d81fe..63fec33b 100644 --- a/src/config.py +++ b/src/config.py @@ -1,10 +1,22 @@ +from pathlib import Path + from pydantic_settings import BaseSettings, SettingsConfigDict +ENV_FILE = Path(__file__).parent.parent / ".env" + class Settings(BaseSettings): - model_config = SettingsConfigDict(env_file=".env") + model_config = SettingsConfigDict(env_file=ENV_FILE) + + environment: str + db_port: int | None = None + + cookie_secure: bool + cookie_domain: str | None = None + frontend_origin: str + auth_url: str | None = None translink_api_key: str | None = None -settings = Settings() +settings = Settings() # pyright: ignore[reportCallIssue] diff --git a/src/constants.py b/src/constants.py index 7a8fc546..48e2cd31 100644 --- a/src/constants.py +++ b/src/constants.py @@ -1,21 +1,17 @@ -import os from zoneinfo import ZoneInfo # TODO(future): replace new.sfucsss.org with sfucsss.org during migration # TODO(far-future): branch-specific root IP addresses (e.g., devbranch.sfucsss.org) -ENV_LOCAL = os.environ.get("LOCAL") -IS_PROD = True if not ENV_LOCAL or ENV_LOCAL.lower() != "true" else False -GITHUB_ORG_NAME = "CSSS-Test-Organization" if not IS_PROD else "CSSS" +# GITHUB_ORG_NAME = "CSSS-Test-Organization" if not IS_PROD else "CSSS" TZ_INFO = ZoneInfo("America/Vancouver") W3_GUILD_ID = "1260652618875797504" CSSS_GUILD_ID = "228761314644852736" -ACTIVE_GUILD_ID = W3_GUILD_ID if not IS_PROD else CSSS_GUILD_ID +# ACTIVE_GUILD_ID = W3_GUILD_ID if not IS_PROD else CSSS_GUILD_ID -SESSION_ID_LEN = 512 +SESSION_ID_LEN = 43 # technically a max of 8 digits https://www.sfu.ca/computing/about/support/tips/sfu-userid.html -COMPUTING_ID_LEN = 32 -COMPUTING_ID_MAX = 8 +COMPUTING_ID_LEN = 8 # see https://support.discord.com/hc/en-us/articles/4407571667351-How-to-Find-User-IDs-for-Law-Enforcement#:~:text=Each%20Discord%20user%20is%20assigned,user%20and%20cannot%20be%20changed. # NOTE: the length got updated to 19 in july 2024. See https://www.reddit.com/r/discordapp/comments/ucrp1r/only_3_months_until_discord_ids_hit_19_digits/ @@ -28,7 +24,3 @@ # https://docs.github.com/en/enterprise-server@3.10/admin/identity-and-access-management/iam-configuration-reference/username-considerations-for-external-authentication GITHUB_USERNAME_LEN = 39 - -# COOKIE -SAMESITE = "none" if IS_PROD else "lax" -DOMAIN = ".sfucsss.org" if IS_PROD else None diff --git a/src/database.py b/src/database.py index 54ddc356..6f434607 100644 --- a/src/database.py +++ b/src/database.py @@ -10,6 +10,8 @@ from sqlalchemy.ext.asyncio import AsyncConnection, AsyncSession, async_sessionmaker, create_async_engine from sqlalchemy.orm import DeclarativeBase +from config import settings + convention = { "ix": "ix_%(column_0_label)s", # index "uq": "uq_%(table_name)s_%(column_0_name)s", # unique @@ -84,11 +86,10 @@ async def session(self) -> AsyncGenerator[AsyncSession]: await session.close() -if os.environ.get("DB_PORT") is not None: +if settings.db_port: # using a remote (or docker) database - db_port = os.environ.get("DB_PORT") - SQLALCHEMY_DATABASE_URL = f"postgresql+asyncpg://localhost:{db_port}/main" - SQLALCHEMY_TEST_DATABASE_URL = f"postgresql+asyncpg://localhost:{db_port}/test" + SQLALCHEMY_DATABASE_URL = f"postgresql+asyncpg://localhost:{settings.db_port}/main" + SQLALCHEMY_TEST_DATABASE_URL = f"postgresql+asyncpg://localhost:{settings.db_port}/test" else: SQLALCHEMY_DATABASE_URL = "postgresql+asyncpg:///main" SQLALCHEMY_TEST_DATABASE_URL = "postgresql+asyncpg:///test" @@ -99,7 +100,7 @@ async def session(self) -> AsyncGenerator[AsyncSession]: def setup_database(): global sessionmanager - db_url = SQLALCHEMY_TEST_DATABASE_URL if os.environ.get("ENV") == "test" else SQLALCHEMY_DATABASE_URL + db_url = SQLALCHEMY_TEST_DATABASE_URL if settings.environment == "test" else SQLALCHEMY_DATABASE_URL # TODO: where is sys.stdout piped to? I want all these to go to a specific logs folder sessionmanager = DatabaseSessionManager( db_url, diff --git a/src/dependencies.py b/src/dependencies.py index 0f43390e..c5c0c52b 100644 --- a/src/dependencies.py +++ b/src/dependencies.py @@ -3,6 +3,7 @@ from fastapi import Cookie, Depends, HTTPException, status import auth +import auth.crud import database from utils.permissions import is_user_election_admin, is_user_website_admin @@ -11,7 +12,7 @@ async def user(db_session: database.DBSession, session_id: Annotated[str | None, if session_id is None: return None - session_computing_id = await auth.crud.get_computing_id(db_session, session_id) + session_computing_id = await auth.crud.get_session_computing_id(db_session, session_id) return session_computing_id @@ -23,7 +24,7 @@ async def logged_in_user(db_session: database.DBSession, session_id: Annotated[s if session_id is None: raise HTTPException(status_code=status.HTTP_401_UNAUTHORIZED, detail="no session id") - session_computing_id = await auth.crud.get_computing_id(db_session, session_id) + session_computing_id = await auth.crud.get_session_computing_id(db_session, session_id) if session_computing_id is None: raise HTTPException(status_code=status.HTTP_401_UNAUTHORIZED, detail="no computing id") diff --git a/src/event/urls.py b/src/event/urls.py index 76968d69..98214909 100644 --- a/src/event/urls.py +++ b/src/event/urls.py @@ -1,8 +1,5 @@ -from datetime import date, datetime - from fastapi import APIRouter, Depends, HTTPException, status from fastapi.encoders import jsonable_encoder -from fastapi.responses import JSONResponse from pydantic import ValidationError import database @@ -10,7 +7,7 @@ from dependencies import perm_admin from event.models import Event, EventCreate, EventDelete, EventUpdate from event.tables import EventDB -from utils.shared_models import DetailModel, SuccessResponse +from utils.shared_models import DetailModel router = APIRouter( prefix="/event", diff --git a/src/load_test_db.py b/src/load_test_db.py index 024d1830..53b80cd0 100644 --- a/src/load_test_db.py +++ b/src/load_test_db.py @@ -10,7 +10,7 @@ # NOTE: make sure you import from a file in your module which (at least) indirectly contains those # tables, or the current python context will not be able to find them & they won't be loaded -from auth.crud import create_user_session, update_site_user +from auth.crud import create_user_session from candidates.crud import add_candidate from candidates.tables import CandidateDB from database import SQLALCHEMY_TEST_DATABASE_URL, Base, DatabaseSessionManager @@ -72,7 +72,6 @@ async def reset_db(engine): async def load_test_auth_data(db_session: AsyncSession): await create_user_session(db_session, "temp_id_314", "abc314") - await update_site_user(db_session, "temp_id_314", "www.my_profile_picture_url.ca/test") await db_session.commit() diff --git a/src/main.py b/src/main.py index d7b30145..6f0b2956 100755 --- a/src/main.py +++ b/src/main.py @@ -18,7 +18,7 @@ import officers.urls import permission.urls import translink.urls -from constants import IS_PROD +from config import settings logging.basicConfig(level=logging.DEBUG) database.setup_database() @@ -37,38 +37,32 @@ async def lifespan(app: FastAPI): await database.sessionmanager.close() -# Enable OpenAPI docs only for local development -if not IS_PROD: - print("Running local environment") - origins = [ - "http://localhost:4200", # default Angular - "http://localhost:8080", # for existing applications/sites - ] +# If on production, disable viewing the docs +if settings.environment == "prod": + print("Running production environment") app = FastAPI( lifespan=lifespan, title="CSSS Site Backend", root_path="/api", + docs_url=None, # disables Swagger UI + redoc_url=None, # disables ReDoc + openapi_url=None, # disables OpenAPI schema ) -# if on production, disable viewing the docs +# Enable OpenAPI docs only for local development else: - print("Running production environment") - origins = [ - "https://sfucsss.org", - "https://test.sfucsss.org", - "https://admin.sfucsss.org", - "https://madness.sfucsss.org", - ] + print("Running local environment") app = FastAPI( lifespan=lifespan, title="CSSS Site Backend", root_path="/api", - docs_url=None, # disables Swagger UI - redoc_url=None, # disables ReDoc - openapi_url=None, # disables OpenAPI schema ) app.add_middleware( - CORSMiddleware, allow_origins=origins, allow_credentials=True, allow_methods=["*"], allow_headers=["*"] + CORSMiddleware, + allow_origins=[settings.frontend_origin], + allow_credentials=True, + allow_methods=["*"], + allow_headers=["*"], ) app.include_router(auth.urls.router) diff --git a/src/officers/urls.py b/src/officers/urls.py index aed1b741..2701c31a 100755 --- a/src/officers/urls.py +++ b/src/officers/urls.py @@ -37,7 +37,7 @@ async def _has_officer_private_info_access( if session_id is None: return False, None - computing_id = await auth.crud.get_computing_id(db_session, session_id) + computing_id = await auth.crud.get_session_computing_id(db_session, session_id) if computing_id is None: return False, None diff --git a/src/utils/__init__.py b/src/utils/__init__.py index 2384cb4b..7adb385a 100644 --- a/src/utils/__init__.py +++ b/src/utils/__init__.py @@ -1,5 +1,5 @@ import re -from datetime import date, datetime +from datetime import date from sqlalchemy import Select @@ -9,14 +9,6 @@ from officers.tables import OfficerTermDB -def is_iso_format(date_str: str) -> bool: - try: - datetime.fromisoformat(date_str) - return True - except ValueError: - return False - - def is_active_officer(query: Select) -> Select: """ An active officer is one who is currently part of the CSSS officer team. diff --git a/src/utils/permissions.py b/src/utils/permissions.py index 37cdd0fa..7ffe5d47 100644 --- a/src/utils/permissions.py +++ b/src/utils/permissions.py @@ -3,8 +3,9 @@ from fastapi import HTTPException, Request, status import auth +import auth.crud import database -import officers +import officers.crud from officers.constants import OfficerPositionEnum WEBSITE_ADMIN_POSITIONS: list[OfficerPositionEnum] = [ @@ -54,7 +55,7 @@ async def get_user(request: Request, db_session: database.DBSession) -> tuple[st if session_id is None: raise HTTPException(status_code=status.HTTP_401_UNAUTHORIZED, detail="no session id") - session_computing_id = await auth.crud.get_computing_id(db_session, session_id) + session_computing_id = await auth.crud.get_session_computing_id(db_session, session_id) if session_computing_id is None: raise HTTPException(status_code=status.HTTP_401_UNAUTHORIZED, detail="no computing id") diff --git a/tests/integration/test_auth.py b/tests/integration/test_auth.py new file mode 100644 index 00000000..e4969ed6 --- /dev/null +++ b/tests/integration/test_auth.py @@ -0,0 +1,167 @@ +from datetime import UTC, datetime +from http import HTTPStatus +from unittest.mock import AsyncMock + +import pytest +import sqlalchemy +from httpx import AsyncClient, Request, Response +from sqlalchemy.ext.asyncio import AsyncSession + +from auth.constants import COOKIE_MAX_AGE, COOKIE_SESSION_KEY, SESSION_MAX_AGE +from auth.crud import create_user_session +from auth.tables import UserSessionDB +from config import settings +from main import app + +pytestmark = pytest.mark.asyncio(loop_scope="session") + +TEST_SERVICE = "http://localhost:8080" +TEST_TICKET = "ST-test-ticket" +TEST_COMPUTING_ID = "auth123" + + +@pytest.fixture(autouse=True) +def auth_settings(monkeypatch: pytest.MonkeyPatch): + monkeypatch.setattr(settings, "auth_url", "https://cas.sfu.ca/cas/serviceValidate") + monkeypatch.setattr(settings, "cookie_secure", False) + monkeypatch.setattr(settings, "cookie_domain", None) + + +def _cas_response(content: str, status_code: int = HTTPStatus.OK) -> Response: + return Response( + status_code=status_code, + text=content, + request=Request("GET", settings.auth_url or "https://cas.example.test/serviceValidate"), + ) + + +def _mock_cas_response(monkeypatch: pytest.MonkeyPatch, content: str, status_code: int = HTTPStatus.OK) -> AsyncMock: + client = AsyncMock(spec=AsyncClient) + client.get = AsyncMock(return_value=_cas_response(content, status_code)) + monkeypatch.setattr(app.state, "http_client", client, raising=False) + return client + + +def _successful_cas_xml(computing_id: str = TEST_COMPUTING_ID) -> str: + return f""" + + + {computing_id} + + + """ + + +def _failed_cas_xml() -> str: + return """ + + + Ticket validation failed + + + """ + + +async def test__login_creates_session_cookie_and_get_user(client: AsyncClient, monkeypatch: pytest.MonkeyPatch): + client.cookies.clear() + cas_client = _mock_cas_response(monkeypatch, _successful_cas_xml()) + + response = await client.post("/auth/login", json={"service": TEST_SERVICE, "ticket": TEST_TICKET}) + + assert response.status_code == HTTPStatus.NO_CONTENT + assert response.content == b"" + assert COOKIE_SESSION_KEY in client.cookies + + set_cookie = response.headers["set-cookie"] + assert f"{COOKIE_SESSION_KEY}=" in set_cookie + assert f"Max-Age={COOKIE_MAX_AGE}" in set_cookie + assert "HttpOnly" in set_cookie + assert "SameSite=strict" in set_cookie + + cas_client.get.assert_awaited_once() + + response = await client.get("/auth/user") + + assert response.status_code == HTTPStatus.OK + assert response.json()["computing_id"] == TEST_COMPUTING_ID + assert response.json()["first_logged_in"] is not None + assert response.json()["last_logged_in"] is not None + + +async def test__login_returns_unauthorized_for_failed_cas_ticket(client: AsyncClient, monkeypatch: pytest.MonkeyPatch): + client.cookies.clear() + _mock_cas_response(monkeypatch, _failed_cas_xml()) + + response = await client.post("/auth/login", json={"service": TEST_SERVICE, "ticket": TEST_TICKET}) + + assert response.status_code == HTTPStatus.UNAUTHORIZED + assert COOKIE_SESSION_KEY not in client.cookies + + +async def test__login_returns_bad_gateway_for_cas_connection_failure( + client: AsyncClient, monkeypatch: pytest.MonkeyPatch +): + client.cookies.clear() + _mock_cas_response(monkeypatch, "CAS unavailable", HTTPStatus.SERVICE_UNAVAILABLE) + + response = await client.post("/auth/login", json={"service": TEST_SERVICE, "ticket": TEST_TICKET}) + + assert response.status_code == HTTPStatus.BAD_GATEWAY + assert COOKIE_SESSION_KEY not in client.cookies + + +async def test__login_returns_service_unavailable_when_auth_is_not_configured( + client: AsyncClient, monkeypatch: pytest.MonkeyPatch +): + client.cookies.clear() + cas_client = _mock_cas_response(monkeypatch, _successful_cas_xml()) + monkeypatch.setattr(settings, "auth_url", None) + + response = await client.post("/auth/login", json={"service": TEST_SERVICE, "ticket": TEST_TICKET}) + + assert response.status_code == HTTPStatus.SERVICE_UNAVAILABLE + cas_client.get.assert_not_awaited() + assert COOKIE_SESSION_KEY not in client.cookies + + +async def test__logout_is_idempotent_and_deletes_session_cookie(client: AsyncClient, monkeypatch: pytest.MonkeyPatch): + client.cookies.clear() + + response = await client.get("/auth/logout") + + assert response.status_code == HTTPStatus.OK + assert response.json() == {"message": "logout successful"} + + _mock_cas_response(monkeypatch, _successful_cas_xml()) + response = await client.post("/auth/login", json={"service": TEST_SERVICE, "ticket": TEST_TICKET}) + assert response.status_code == HTTPStatus.NO_CONTENT + assert COOKIE_SESSION_KEY in client.cookies + + response = await client.get("/auth/logout") + + assert response.status_code == HTTPStatus.OK + assert response.json() == {"message": "logout successful"} + assert COOKIE_SESSION_KEY not in client.cookies + assert f"{COOKIE_SESSION_KEY}=" in response.headers["set-cookie"] + assert "Max-Age=0" in response.headers["set-cookie"] + + response = await client.get("/auth/user") + assert response.status_code == HTTPStatus.UNAUTHORIZED + + +async def test__expired_session_cannot_get_user(client: AsyncClient, db_session: AsyncSession): + client.cookies.clear() + session_id = "expired-session-id" + computing_id = "expired" + await create_user_session(db_session, session_id, computing_id) + await db_session.execute( + sqlalchemy.update(UserSessionDB) + .where(UserSessionDB.session_id == session_id) + .values(issue_time=datetime.now(UTC) - (2 * SESSION_MAX_AGE)) + ) + await db_session.commit() + client.cookies.set(COOKIE_SESSION_KEY, session_id) + + response = await client.get("/auth/user") + + assert response.status_code == HTTPStatus.UNAUTHORIZED diff --git a/tests/unit/test_translink.py b/tests/unit/test_translink.py index 1cf9b13d..de5fa350 100644 --- a/tests/unit/test_translink.py +++ b/tests/unit/test_translink.py @@ -1,6 +1,6 @@ import io import zipfile -from datetime import date, datetime, timedelta +from datetime import datetime, timedelta from unittest.mock import AsyncMock, MagicMock, patch import httpx @@ -466,6 +466,7 @@ async def test__get_or_fetch_realtime_feed_returns_none_on_http_error_status(): async def test__get_or_fetch_static_schedule_cache_hit(): """When the DB has today's row, no HTTP call should be made.""" + today = datetime.now(tz=TZ_INFO).date() cached_records = [ { "trip_id": "trip_143", @@ -475,13 +476,13 @@ async def test__get_or_fetch_static_schedule_cache_hit(): "departure_seconds": 82800, } ] - cached_row = TransLinkStaticScheduleDB(id=1, date_fetched=date.today(), schedule=cached_records) + cached_row = TransLinkStaticScheduleDB(id=1, date_fetched=today, schedule=cached_records) session = mock_db_session(cached_row=cached_row) client = AsyncMock(spec=AsyncClient) result_date, result_df = await get_or_fetch_static_schedule(session, client) - assert result_date == date.today() + assert result_date == today assert not result_df.empty assert result_df.iloc[0]["bus_number"] == "143" client.get.assert_not_called() @@ -489,12 +490,13 @@ async def test__get_or_fetch_static_schedule_cache_hit(): async def test__get_or_fetch_static_schedule_cache_miss_fetches(): """On a cache miss the function should call the API and persist the result.""" + today = datetime.now(tz=TZ_INFO).date() session = mock_db_session(cached_row=None) client = mock_http_client(make_gtfs_zip()) result_date, result_df = await get_or_fetch_static_schedule(session, client) - assert result_date == date.today() + assert result_date == today assert not result_df.empty session.merge.assert_awaited_once() session.commit.assert_awaited_once() @@ -504,13 +506,14 @@ async def test__get_or_fetch_static_schedule_db_write_failure_still_returns(): """If the DB write fails, the function should still return the fetched data.""" import sqlalchemy.exc + today = datetime.now(tz=TZ_INFO).date() session = mock_db_session(cached_row=None) session.merge = AsyncMock(side_effect=sqlalchemy.exc.SQLAlchemyError("disk full")) client = mock_http_client(make_gtfs_zip()) result_date, result_df = await get_or_fetch_static_schedule(session, client) - assert result_date == date.today() + assert result_date == today assert not result_df.empty session.rollback.assert_awaited_once() @@ -578,7 +581,7 @@ async def test__endpoint_realtime_returns_200(client): async def test__endpoint_static_returns_schedule(client): - today = date.today() + today = datetime.now(tz=TZ_INFO).date() mock_df = pd.DataFrame( [ {