diff --git a/exceptions/http_exceptions.py b/exceptions/http_exceptions.py index 1aabdc9..f745596 100644 --- a/exceptions/http_exceptions.py +++ b/exceptions/http_exceptions.py @@ -146,16 +146,6 @@ def __init__(self): ) -class ActiveInvitationExistsError(HTTPException): - """Raised when trying to invite a user for whom an active invitation already exists.""" - - def __init__(self): - super().__init__( - status_code=409, - detail="An active invitation already exists for this email address in this organization.", - ) - - class InvalidRoleForOrganizationError(HTTPException): """Raised when a role provided does not belong to the target organization. Note: If the role ID simply doesn't exist, a standard 404 RoleNotFoundError should be raised. @@ -186,10 +176,31 @@ def __init__(self): class InvalidInvitationTokenError(HTTPException): - """Raised when an invitation token is invalid, expired, or not found.""" + """Raised when an invitation token is missing, superseded, or already used.""" + + def __init__(self): + super().__init__( + status_code=404, + detail=( + "This invitation link is no longer valid. " + "If you were invited again recently, use the link from the " + "most recent invitation email." + ), + ) + + +class ExpiredInvitationTokenError(HTTPException): + """Raised when an invitation token exists but has passed its expiry date.""" def __init__(self): - super().__init__(status_code=404, detail="Invitation not found or expired") + super().__init__( + status_code=404, + detail=( + "This invitation link has expired. Ask your organization " + "administrator to send a new invitation, then use the link in " + "the latest email." + ), + ) class InvitationEmailMismatchError(HTTPException): diff --git a/routers/core/account.py b/routers/core/account.py index ee4f047..7d93e7c 100644 --- a/routers/core/account.py +++ b/routers/core/account.py @@ -45,6 +45,7 @@ get_account_from_recovery_token, get_account_from_credentials, require_unauthenticated_client, + require_unauthenticated_unless_invitation_warning, get_verified_account, ) from exceptions.http_exceptions import ( @@ -54,14 +55,17 @@ EmailNotVerifiedError, MaxEmailsReachedError, PasswordValidationError, - InvalidInvitationTokenError, InvitationEmailMismatchError, InvitationProcessingError, ) from routers.core.dashboard import router as dashboard_router from routers.core.user import router as user_router from routers.core.organization import router as org_router -from utils.core.invitations import process_invitation +from utils.core.invitations import ( + process_invitation, + require_active_invitation_by_token, + get_invitation_token_warning, +) from utils.core.rate_limit import ( check_login_ip_rate_limit, check_login_email_rate_limit, @@ -147,37 +151,56 @@ def logout( @router.get("/login") async def read_login( request: Request, - _: None = Depends(require_unauthenticated_client), + _: None = Depends(require_unauthenticated_unless_invitation_warning), invitation_token: Optional[str] = Query(None), + user: Optional[User] = Depends(get_optional_user), + session: Session = Depends(get_session), ): """ Render login page or redirect to dashboard if already logged in. """ + invitation_token_warning = ( + get_invitation_token_warning(session, invitation_token) + if invitation_token + else None + ) return templates.TemplateResponse( request, "account/login.html", - {"user": None, "invitation_token": invitation_token}, + { + "user": user, + "invitation_token": invitation_token, + "invitation_token_warning": invitation_token_warning, + }, ) @router.get("/register") async def read_register( request: Request, - _: None = Depends(require_unauthenticated_client), + _: None = Depends(require_unauthenticated_unless_invitation_warning), email: Optional[EmailStr] = Query(None), invitation_token: Optional[str] = Query(None), + user: Optional[User] = Depends(get_optional_user), + session: Session = Depends(get_session), ): """ Render registration page or redirect to dashboard if already logged in. """ + invitation_token_warning = ( + get_invitation_token_warning(session, invitation_token) + if invitation_token + else None + ) return templates.TemplateResponse( request, "account/register.html", { - "user": None, + "user": user, "password_pattern": HTML_PASSWORD_PATTERN, "email": email, "invitation_token": invitation_token, + "invitation_token_warning": invitation_token_warning, }, ) @@ -270,6 +293,18 @@ async def register( """ Register a new user account, optionally processing an invitation. """ + pending_invitation: Optional[Invitation] = None + if invitation_token: + pending_invitation = require_active_invitation_by_token( + session, invitation_token + ) + if email != pending_invitation.invitee_email: + logger.warning( + f"Invitation email mismatch for token {invitation_token} during registration. " + f"Account: {email}, Invitation: {pending_invitation.invitee_email}" + ) + raise InvitationEmailMismatchError() + # Check if the email is already registered existing_account: Optional[Account] = session.exec( select(Account).where(Account.email == email) @@ -313,46 +348,27 @@ async def register( redirect_url = dashboard_router.url_path_for("read_dashboard") # Process invitation if token is provided (BEFORE final commit) - if invitation_token: + if pending_invitation: logger.info( f"Registration attempt with invitation token: {invitation_token} for email {email}" ) - # Fetch the invitation - statement = select(Invitation).where(Invitation.token == invitation_token) - invitation = session.exec(statement).first() - - if not invitation or not invitation.is_active(): - logger.warning( - f"Invalid or inactive invitation token provided during registration: {invitation_token}" - ) - # Consider raising a more generic error to avoid exposing token validity - raise InvalidInvitationTokenError() - - # Verify email matches - if email != invitation.invitee_email: - logger.warning( - f"Invitation email mismatch for token {invitation_token} during registration. " - f"Account: {email}, Invitation: {invitation.invitee_email}" - ) - # Consider raising a more generic error to avoid confirming email existence - raise InvitationEmailMismatchError() # Process the invitation (adds changes to the session) try: logger.info( - f"Processing invitation {invitation.id} for new user {new_user.name} ({email}) during registration." + f"Processing invitation {pending_invitation.id} for new user {new_user.name} ({email}) during registration." ) - process_invitation(invitation, new_user, session) + process_invitation(pending_invitation, new_user, session) # Set redirect to the organization page redirect_url = org_router.url_path_for( - "read_organization", org_id=invitation.organization_id + "read_organization", org_id=pending_invitation.organization_id ) logger.info( - f"Redirecting new user {new_user.name} to organization {invitation.organization_id} after accepting invitation {invitation.id}." + f"Redirecting new user {new_user.name} to organization {pending_invitation.organization_id} after accepting invitation {pending_invitation.id}." ) except Exception as e: logger.error( - f"Error processing invitation {invitation.id} for new user {new_user.name} ({email}) during registration: {e}", + f"Error processing invitation {pending_invitation.id} for new user {new_user.name} ({email}) during registration: {e}", exc_info=True, ) session.rollback() @@ -434,15 +450,7 @@ async def login( logger.info( f"Login attempt with invitation token: {invitation_token} for account {account.email}" ) - # Fetch the invitation - statement = select(Invitation).where(Invitation.token == invitation_token) - invitation = session.exec(statement).first() - - if not invitation or not invitation.is_active(): - logger.warning( - f"Invalid or inactive invitation token provided during login: {invitation_token}" - ) - raise InvalidInvitationTokenError() + invitation = require_active_invitation_by_token(session, invitation_token) # Verify email matches (check primary and any verified secondary emails) account_emails = session.exec( diff --git a/routers/core/invitation.py b/routers/core/invitation.py index 07f4554..473ec42 100644 --- a/routers/core/invitation.py +++ b/routers/core/invitation.py @@ -1,3 +1,4 @@ +from datetime import timedelta from uuid import uuid4 from typing import Optional from fastapi import APIRouter, Depends, Form, Query, Request, status @@ -13,17 +14,19 @@ get_optional_user, get_session, ) -from utils.core.models import User, Role, Account, Invitation, Organization +from utils.core.models import User, Role, Account, Invitation, Organization, utc_now from utils.core.enums import ValidPermissions from utils.app.enums import AppPermissions -from utils.core.invitations import send_invitation_email, process_invitation +from utils.core.invitations import ( + send_invitation_email, + process_invitation, + require_active_invitation_by_token, +) from exceptions.http_exceptions import ( UserIsAlreadyMemberError, - ActiveInvitationExistsError, InvalidRoleForOrganizationError, OrganizationNotFoundError, InvitationEmailSendError, - InvalidInvitationTokenError, InvitationNotFoundError, InsufficientPermissionsError, RoleNotFoundError, @@ -34,7 +37,6 @@ from routers.core.account import router as account_router from routers.core.organization import router as org_router -# Setup logger logger = getLogger("uvicorn.error") templates = Jinja2Templates(directory="templates") @@ -45,16 +47,64 @@ ) -# Dependency to get a valid invitation def get_valid_invitation( token: str = Query(...), session: Session = Depends(get_session) ) -> Invitation: """Dependency to retrieve a valid, active invitation based on the token.""" - statement = select(Invitation).where(Invitation.token == token) - invitation = session.exec(statement).first() - if not invitation or not invitation.is_active(): - raise InvalidInvitationTokenError() - return invitation + return require_active_invitation_by_token(session, token) + + +def _redirect_for_inactive_invitation( + invitation: Optional[Invitation], + token: str, + session: Session, +) -> RedirectResponse: + """Send user to register/login so invitation_token_warning banners can display.""" + if invitation: + existing_account = session.exec( + select(Account).where(Account.email == invitation.invitee_email) + ).first() + if existing_account: + login_url = account_router.url_path_for("read_login") + redirect_url = f"{login_url}?invitation_token={token}" + else: + register_url = account_router.url_path_for("read_register") + redirect_url = ( + f"{register_url}?email={invitation.invitee_email}" + f"&invitation_token={token}" + ) + else: + login_url = account_router.url_path_for("read_login") + redirect_url = f"{login_url}?invitation_token={token}" + + return RedirectResponse(url=redirect_url, status_code=status.HTTP_303_SEE_OTHER) + + +def _members_table_response( + request: Request, + session: Session, + organization_id: int, + current_user: User, + toast_message: str | None = None, +) -> Response: + organization, user_permissions, pending_invitations = load_org_for_members_partial( + session, organization_id, current_user + ) + response = templates.TemplateResponse( + request, + "organization/partials/members_table.html", + { + "organization": organization, + "pending_invitations": pending_invitations, + "user": current_user, + "user_permissions": user_permissions, + "ValidPermissions": ValidPermissions, + "all_permissions": list(ValidPermissions) + list(AppPermissions), + }, + ) + if toast_message: + response = append_toast(response, request, templates, toast_message) + return response @router.post("/", name="create_invitation") @@ -74,48 +124,37 @@ async def create_invitation( description="ID of the organization to invite the user to", ), ): - # Fetch the organization organization = session.get(Organization, organization_id) if not organization: raise OrganizationNotFoundError() - # Check if the current user has permission to invite users to this organization if not current_user.has_permission(ValidPermissions.INVITE_USER, organization): raise InsufficientPermissionsError( "You don't have permission to invite users to this organization" ) - # Verify the role exists and belongs to this organization role = session.get(Role, role_id) if not role: raise RoleNotFoundError() if role.organization_id != organization_id: raise InvalidRoleForOrganizationError() - # Check if invitee is already a member of the organization existing_account = session.exec( select(Account).where(Account.email == invitee_email) ).first() if existing_account: - # Check if any user with this account is already a member existing_user = session.exec( select(User).where(User.account_id == existing_account.id) ).first() if existing_user: - # Check if user has any role in this organization if any( role.organization_id == organization_id for role in existing_user.roles ): raise UserIsAlreadyMemberError() - # Check for active invitations with the same email - active_invitations = Invitation.get_active_for_org(session, organization_id) - if any( - invitation.invitee_email == invitee_email for invitation in active_invitations - ): - raise ActiveInvitationExistsError() + Invitation.invalidate_pending_for_email(session, organization_id, invitee_email) + session.flush() - # Create the invitation token = str(uuid4()) invitation = Invitation( organization_id=organization_id, @@ -127,30 +166,23 @@ async def create_invitation( session.add(invitation) try: - # Refresh to ensure relationships are loaded *before* sending email - session.flush() # Ensure invitation gets an ID if needed by email sender, flush changes + session.flush() session.refresh(invitation) - # Ensure organization is loaded before passing to email function - # (May already be loaded, but explicit refresh is safer) if not invitation.organization: - session.refresh(organization) # Refresh the org object fetched earlier - invitation.organization = organization # Assign if needed + session.refresh(organization) + invitation.organization = organization - # Send email synchronously BEFORE committing send_invitation_email(invitation, session) - - # Commit *only* if email sending was successful session.commit() - session.refresh(invitation) # Refresh again after commit if needed elsewhere + session.refresh(invitation) except EmailSendFailedError as e: logger.error( f"Invitation email failed for {invitee_email} in org {organization_id}: {e}" ) - session.rollback() # Rollback the invitation creation - raise InvitationEmailSendError() # Raise HTTP 500 + session.rollback() + raise InvitationEmailSendError() except Exception as e: - # Catch any other unexpected errors during flush/refresh/email/commit logger.error( f"Unexpected error during invitation creation/sending for {invitee_email} " f"in org {organization_id}: {e}", @@ -159,26 +191,87 @@ async def create_invitation( session.rollback() raise HTTPException(status_code=500, detail="An unexpected error occurred.") - # HTMX: return partial; non-HTMX: PRG redirect if is_htmx_request(request): - organization, user_permissions, active_invitations = ( - load_org_for_members_partial(session, organization_id, current_user) - ) - response = templates.TemplateResponse( + response = _members_table_response( request, - "organization/partials/members_table.html", - { - "organization": organization, - "active_invitations": active_invitations, - "user": current_user, - "user_permissions": user_permissions, - "ValidPermissions": ValidPermissions, - "all_permissions": list(ValidPermissions) + list(AppPermissions), - }, + session, + organization_id, + current_user, + "Invitation sent successfully.", ) response.headers["HX-Trigger"] = "modalDismiss" - return append_toast( - response, request, templates, "Invitation sent successfully." + return response + return RedirectResponse(url=f"/organizations/{organization_id}", status_code=303) + + +@router.post("/resend", name="resend_invitation", response_class=RedirectResponse) +async def resend_invitation( + request: Request, + current_user: User = Depends(get_authenticated_user), + session: Session = Depends(get_session), + invitation_id: int = Form( + ..., title="Invitation ID", description="ID of the invitation to resend" + ), + organization_id: int = Form( + ..., + title="Organization ID", + description="ID of the organization the invitation belongs to", + ), +) -> Response: + organization = session.get(Organization, organization_id) + if not organization: + raise OrganizationNotFoundError() + + if not current_user.has_permission(ValidPermissions.INVITE_USER, organization): + raise InsufficientPermissionsError( + "You don't have permission to resend invitations for this organization" + ) + + invitation = session.get(Invitation, invitation_id) + if ( + not invitation + or invitation.organization_id != organization_id + or invitation.used + ): + raise InvitationNotFoundError() + + invitation.token = str(uuid4()) + invitation.expires_at = utc_now() + timedelta(days=7) + + try: + session.flush() + session.refresh(invitation) + if not invitation.organization: + session.refresh(organization) + invitation.organization = organization + + send_invitation_email(invitation, session) + session.commit() + session.refresh(invitation) + + except EmailSendFailedError as e: + logger.error( + f"Invitation resend failed for {invitation.invitee_email} " + f"in org {organization_id}: {e}" + ) + session.rollback() + raise InvitationEmailSendError() + except Exception as e: + logger.error( + f"Unexpected error during invitation resend for {invitation.invitee_email} " + f"in org {organization_id}: {e}", + exc_info=True, + ) + session.rollback() + raise HTTPException(status_code=500, detail="An unexpected error occurred.") + + if is_htmx_request(request): + return _members_table_response( + request, + session, + organization_id, + current_user, + "Invitation resent.", ) return RedirectResponse(url=f"/organizations/{organization_id}", status_code=303) @@ -214,46 +307,38 @@ async def delete_invitation( session.commit() if is_htmx_request(request): - organization, user_permissions, active_invitations = ( - load_org_for_members_partial(session, organization_id, current_user) - ) - response = templates.TemplateResponse( + return _members_table_response( request, - "organization/partials/members_table.html", - { - "organization": organization, - "active_invitations": active_invitations, - "user": current_user, - "user_permissions": user_permissions, - "ValidPermissions": ValidPermissions, - "all_permissions": list(ValidPermissions) + list(AppPermissions), - }, - ) - return append_toast( - response, request, templates, "Invitation cancelled successfully." + session, + organization_id, + current_user, + "Invitation cancelled successfully.", ) return RedirectResponse(url=f"/organizations/{organization_id}", status_code=303) @router.get("/accept", name="accept_invitation") async def accept_invitation( - invitation: Invitation = Depends(get_valid_invitation), + token: str = Query(...), current_user: Optional[User] = Depends(get_optional_user), session: Session = Depends(get_session), ): """Handles the acceptance of an invitation via the link in the email.""" - # Check if an account exists for the invitee email + invitation = session.exec( + select(Invitation).where(Invitation.token == token) + ).first() + + if not invitation or not invitation.is_active(): + return _redirect_for_inactive_invitation(invitation, token, session) + account_statement = select(Account).where(Account.email == invitation.invitee_email) existing_account = session.exec(account_statement).first() if existing_account: - # Account exists - check if user is logged in and matches the invitation if current_user and current_user.account_id == existing_account.id: - # Ensure the account relationship is loaded before accessing its email if not current_user.account: session.refresh(current_user, attribute_names=["account"]) - # Check if refreshed account has an email (should always exist, but good practice) if not current_user.account or not current_user.account.email: logger.error( f"User {current_user.id} is missing account details after refresh." @@ -263,14 +348,12 @@ async def accept_invitation( detail="Internal server error retrieving user account.", ) - # Logged in as the correct user, process the invitation directly logger.info( f"User {current_user.id} ({current_user.account.email}) accepting invitation {invitation.id} directly." ) try: process_invitation(invitation, current_user, session) session.commit() - # Redirect to the organization page redirect_url = org_router.url_path_for( "read_organization", org_id=invitation.organization_id ) @@ -283,13 +366,10 @@ async def accept_invitation( exc_info=True, ) session.rollback() - # Re-raise or return a generic error response raise HTTPException( status_code=500, detail="Failed to process invitation." ) else: - # Account exists, but user is not logged in or is the wrong user - # Redirect to login, passing the token logger.info( f"Invitation {invitation.id} requires login for {invitation.invitee_email}. Redirecting." ) @@ -299,7 +379,6 @@ async def accept_invitation( url=redirect_url_with_token, status_code=status.HTTP_303_SEE_OTHER ) else: - # Account does not exist - redirect to registration logger.info( f"Invitation {invitation.id} requires registration for {invitation.invitee_email}. Redirecting." ) diff --git a/routers/core/organization.py b/routers/core/organization.py index 9c30180..4c79312 100644 --- a/routers/core/organization.py +++ b/routers/core/organization.py @@ -69,8 +69,8 @@ async def read_organization( ) ).first() - # Fetch active invitations for the organization - active_invitations = Invitation.get_active_for_org(session, org_id) + # Fetch pending invitations for the organization + pending_invitations = Invitation.get_pending_for_org(session, org_id) # Pass all required context to the template return templates.TemplateResponse( @@ -82,7 +82,7 @@ async def read_organization( "user_permissions": user_permissions, "ValidPermissions": ValidPermissions, "all_permissions": list(ValidPermissions) + list(AppPermissions), - "active_invitations": active_invitations, + "pending_invitations": pending_invitations, }, ) diff --git a/routers/core/user.py b/routers/core/user.py index 119b2a5..fffebfe 100644 --- a/routers/core/user.py +++ b/routers/core/user.py @@ -239,7 +239,7 @@ def update_user_role( session.commit() if is_htmx_request(request): - organization, user_permissions, active_invitations = ( + organization, user_permissions, pending_invitations = ( load_org_for_members_partial(session, organization_id, user) ) response = templates.TemplateResponse( @@ -247,7 +247,7 @@ def update_user_role( "organization/partials/members_table.html", { "organization": organization, - "active_invitations": active_invitations, + "pending_invitations": pending_invitations, "user": user, "user_permissions": user_permissions, "ValidPermissions": ValidPermissions, @@ -313,7 +313,7 @@ def remove_user_from_organization( session.commit() if is_htmx_request(request): - organization, user_permissions, active_invitations = ( + organization, user_permissions, pending_invitations = ( load_org_for_members_partial(session, organization_id, user) ) response = templates.TemplateResponse( @@ -321,7 +321,7 @@ def remove_user_from_organization( "organization/partials/members_table.html", { "organization": organization, - "active_invitations": active_invitations, + "pending_invitations": pending_invitations, "user": user, "user_permissions": user_permissions, "ValidPermissions": ValidPermissions, diff --git a/static/css/extras.css b/static/css/extras.css index 7822548..ca7a5db 100644 --- a/static/css/extras.css +++ b/static/css/extras.css @@ -18,6 +18,11 @@ input:invalid:not(:placeholder-shown) + .invalid-feedback { padding-right: var(--bs-card-spacer-x); } +.invitation-cancel-form .btn { + min-width: 6.75rem; +} + +.invitation-resend-form .btn-invitation-resend, .invitation-cancel-form .btn { min-width: 6.75rem; } \ No newline at end of file diff --git a/templates/account/login.html b/templates/account/login.html index 234e50e..168f8cd 100644 --- a/templates/account/login.html +++ b/templates/account/login.html @@ -6,6 +6,15 @@ {% block auth_content %}
+ {% if invitation_token_warning == 'expired' %} + + {% elif invitation_token_warning == 'invalid' %} + + {% endif %}
@@ -34,7 +43,10 @@
- +
diff --git a/templates/account/register.html b/templates/account/register.html index 46b5adf..78d6a10 100644 --- a/templates/account/register.html +++ b/templates/account/register.html @@ -6,6 +6,15 @@ {% block auth_content %}
+ {% if invitation_token_warning == 'expired' %} + + {% elif invitation_token_warning == 'invalid' %} + + {% endif %}
@@ -58,7 +67,10 @@
- +
diff --git a/templates/organization/partials/invitations_list.html b/templates/organization/partials/invitations_list.html index 52503b4..a95c984 100644 --- a/templates/organization/partials/invitations_list.html +++ b/templates/organization/partials/invitations_list.html @@ -1,20 +1,41 @@ {# Partial:
  • items for pending invitations. Swapped into
      . #} -{# Set show_invitation_cancel=true via {% with %} to render Cancel controls (members card only). #} -{% for inv in active_invitations %} +{# Set show_invitation_cancel=true via {% with %} to render Resend/Cancel controls (members card only). #} +{% for inv in pending_invitations %}
    • -
      - {{ inv.invitee_email }} (Role: {{ inv.role.name }}) - Expires: {{ inv.expires_at.strftime('%Y-%m-%d') }} +
      + + {{ inv.invitee_email }} (Role: {{ inv.role.name }}) + {% if inv.is_expired() %} + Expired + {% else %} + Active + {% endif %} + + Expires: {{ inv.expires_at.strftime('%Y-%m-%d') }} {% if show_invitation_cancel is defined and show_invitation_cancel and ValidPermissions is defined and user_permissions is defined and ValidPermissions.INVITE_USER in user_permissions %} -
      - - - -
      +
      +
      + + + +
      +
      + + + +
      +
      {% endif %}
      • diff --git a/tests/routers/core/test_invitation.py b/tests/routers/core/test_invitation.py index afc6bd3..fe3243c 100644 --- a/tests/routers/core/test_invitation.py +++ b/tests/routers/core/test_invitation.py @@ -1,7 +1,8 @@ import pytest from datetime import datetime, timedelta, UTC from unittest.mock import MagicMock -from sqlmodel import Session, select +from urllib.parse import urlparse, parse_qs +from sqlmodel import Session, select, col from tests.conftest import SetupError from utils.core.models import Role, Permission, User, Invitation, Organization, Account from utils.core.enums import ValidPermissions @@ -237,6 +238,68 @@ def test_invitation_is_active( assert not used_invitation.is_active() +def test_get_pending_for_org( + session: Session, + test_organization: Organization, + existing_invitation: Invitation, + expired_invitation: Invitation, + used_invitation: Invitation, +): + """Test get_pending_for_org returns all unused invitations, including expired.""" + assert test_organization.id is not None + member_role = session.exec( + select(Role).where( + Role.name == "Member", Role.organization_id == test_organization.id + ) + ).first() + if not member_role: + pytest.fail("Member role not found in test_get_pending_for_org") + assert member_role.id is not None + + second_active = Invitation( + organization_id=test_organization.id, + role_id=member_role.id, + invitee_email="another@example.com", + token="another-token-12345", + expires_at=datetime.now(UTC) + timedelta(days=7), + ) + session.add(second_active) + + other_org = Organization(name="Other Org Pending") + session.add(other_org) + session.commit() + assert other_org.id is not None + + other_member_role = session.exec( + select(Role).where(Role.name == "Member", Role.organization_id == other_org.id) + ).first() + if not other_member_role: + other_member_role = Role(name="Member", organization_id=other_org.id) + session.add(other_member_role) + session.commit() + session.refresh(other_member_role) + assert other_member_role.id is not None + + other_org_invitation = Invitation( + organization_id=other_org.id, + role_id=other_member_role.id, + invitee_email="other-org@example.com", + token="other-org-token-12345", + expires_at=datetime.now(UTC) + timedelta(days=7), + ) + session.add(other_org_invitation) + session.commit() + + pending_invitations = Invitation.get_pending_for_org(session, test_organization.id) + + assert len(pending_invitations) == 3 + assert existing_invitation in pending_invitations + assert second_active in pending_invitations + assert expired_invitation in pending_invitations + assert used_invitation not in pending_invitations + assert other_org_invitation not in pending_invitations + + def test_get_active_for_org( session: Session, test_organization: Organization, @@ -498,24 +561,130 @@ def test_create_invitation_for_existing_member( ) # Conflict - UserIsAlreadyMemberError -def test_create_invitation_duplicate_active( - auth_client, inviter_user: User, existing_invitation: Invitation +def test_create_invitation_resend_replaces_active_invitation( + auth_client, + inviter_user: User, + existing_invitation: Invitation, + session: Session, + mock_resend_send, ): - """Test that creating a duplicate active invitation fails.""" + """Re-inviting the same email replaces the pending invite and invalidates the old token.""" assert existing_invitation.organization_id is not None assert existing_invitation.role_id is not None + old_token = existing_invitation.token + old_id = existing_invitation.id + invitee_email = existing_invitation.invitee_email + organization_id = existing_invitation.organization_id + + response = auth_client.post( + app.url_path_for("create_invitation"), + data={ + "invitee_email": invitee_email, + "role_id": str(existing_invitation.role_id), + "organization_id": str(organization_id), + }, + follow_redirects=False, + ) + + assert response.status_code == 303, response.text + + session.expire_all() + assert session.get(Invitation, old_id) is None + + new_invitation = session.exec( + select(Invitation).where( + Invitation.invitee_email == invitee_email, + Invitation.organization_id == organization_id, + col(Invitation.used).is_(False), + ) + ).first() + assert new_invitation is not None + assert new_invitation.token != old_token + + accept_response = auth_client.get( + app.url_path_for("accept_invitation"), + params={"token": old_token}, + follow_redirects=False, + ) + assert accept_response.status_code == 303 + parsed_url = urlparse(accept_response.headers["location"]) + assert parsed_url.path == app.url_path_for("read_login") + assert parse_qs(parsed_url.query).get("invitation_token") == [old_token] + + auth_response = auth_client.get(accept_response.headers["location"]) + assert auth_response.status_code == 200 + assert "no longer valid" in auth_response.text.lower() + + +def test_create_invitation_resend_after_expired_pending_invite( + auth_client, + inviter_user: User, + expired_invitation: Invitation, + session: Session, + mock_resend_send, +): + """Re-inviting after expiry succeeds even though the old row remains used=False in DB.""" + assert expired_invitation.organization_id is not None + assert expired_invitation.role_id is not None + expired_id = expired_invitation.id + invitee_email = expired_invitation.invitee_email + organization_id = expired_invitation.organization_id + old_token = expired_invitation.token + + response = auth_client.post( + app.url_path_for("create_invitation"), + data={ + "invitee_email": invitee_email, + "role_id": str(expired_invitation.role_id), + "organization_id": str(organization_id), + }, + follow_redirects=False, + ) + + assert response.status_code == 303, response.text + session.expire_all() + assert session.get(Invitation, expired_id) is None + + replacement = session.exec( + select(Invitation).where( + Invitation.invitee_email == invitee_email, + Invitation.organization_id == organization_id, + col(Invitation.used).is_(False), + ) + ).first() + assert replacement is not None + assert replacement.token != old_token + + +def test_create_invitation_resend_email_failure_restores_old_invite( + auth_client, + inviter_user: User, + existing_invitation: Invitation, + session: Session, + mock_resend_send, +): + """Failed resend rolls back both delete and new invite creation.""" + assert existing_invitation.id is not None + old_token = existing_invitation.token + mock_resend_send.side_effect = Exception("Simulated email send failure") + response = auth_client.post( app.url_path_for("create_invitation"), data={ - "invitee_email": existing_invitation.invitee_email, # Same email + "invitee_email": existing_invitation.invitee_email, "role_id": str(existing_invitation.role_id), "organization_id": str(existing_invitation.organization_id), }, + follow_redirects=False, ) - assert response.status_code == 409, ( - f"Expected 409 Conflict, got {response.status_code}. Response: {response.text}" - ) # Conflict - ActiveInvitationExistsError + assert response.status_code == 500 + session.expire_all() + restored = session.exec( + select(Invitation).where(Invitation.token == old_token) + ).first() + assert restored is not None + assert restored.id == existing_invitation.id def test_create_invitation_role_not_found( @@ -657,7 +826,7 @@ def test_create_invitation_email_send_failure( # --- Organization Page Tests --- -def test_organization_page_shows_active_invitations( +def test_organization_page_shows_pending_invitations( auth_client_owner, test_organization: Organization, session: Session, @@ -665,7 +834,7 @@ def test_organization_page_shows_active_invitations( expired_invitation: Invitation, used_invitation: Invitation, ): - """Test that the organization page shows active invitations.""" + """Test that the organization page shows pending invitations, including expired.""" assert test_organization.id is not None response = auth_client_owner.get( app.url_path_for("read_organization", org_id=test_organization.id), @@ -674,18 +843,10 @@ def test_organization_page_shows_active_invitations( assert response.status_code == 200 response_text = response.text - # Active invitation email should be in response (Depends on Invitation model) - assert ( - existing_invitation.invitee_email in response_text - ) # Ignored Error: Invitation model not defined yet - - # Expired and used invitation emails should not be in response (Depends on Invitation model) - assert ( - expired_invitation.invitee_email not in response_text - ) # Ignored Error: Invitation model not defined yet - assert ( - used_invitation.invitee_email not in response_text - ) # Ignored Error: Invitation model not defined yet + assert existing_invitation.invitee_email in response_text + assert expired_invitation.invitee_email in response_text + assert "Expired" in response_text + assert used_invitation.invitee_email not in response_text def test_organization_page_invite_form_visibility( @@ -874,7 +1035,133 @@ def test_delete_invitation_invalidates_token( params={"token": token}, follow_redirects=False, ) - assert accept_response.status_code == 404 + assert accept_response.status_code == 303 + parsed_url = urlparse(accept_response.headers["location"]) + assert parsed_url.path == app.url_path_for("read_login") + assert parse_qs(parsed_url.query).get("invitation_token") == [token] + + +def test_resend_invitation_active_refreshes_token( + auth_client_owner, + test_organization: Organization, + existing_invitation: Invitation, + session: Session, + mock_resend_send: MagicMock, +): + assert test_organization.id is not None + assert existing_invitation.id is not None + old_token = existing_invitation.token + + response = auth_client_owner.post( + app.url_path_for("resend_invitation"), + data={ + "invitation_id": str(existing_invitation.id), + "organization_id": str(test_organization.id), + }, + follow_redirects=False, + ) + assert response.status_code == 303 + + session.refresh(existing_invitation) + assert existing_invitation.token != old_token + assert not existing_invitation.is_expired() + assert existing_invitation.used is False + mock_resend_send.assert_called_once() + + +def test_resend_invitation_expired_reactivates( + auth_client_owner, + test_organization: Organization, + expired_invitation: Invitation, + session: Session, + mock_resend_send: MagicMock, +): + assert test_organization.id is not None + assert expired_invitation.id is not None + assert expired_invitation.is_expired() + + response = auth_client_owner.post( + app.url_path_for("resend_invitation"), + data={ + "invitation_id": str(expired_invitation.id), + "organization_id": str(test_organization.id), + }, + follow_redirects=False, + ) + assert response.status_code == 303 + + session.refresh(expired_invitation) + assert not expired_invitation.is_expired() + mock_resend_send.assert_called_once() + + +def test_resend_invitation_unauthorized( + auth_client_member, + test_organization: Organization, + existing_invitation: Invitation, +): + assert test_organization.id is not None + assert existing_invitation.id is not None + + response = auth_client_member.post( + app.url_path_for("resend_invitation"), + data={ + "invitation_id": str(existing_invitation.id), + "organization_id": str(test_organization.id), + }, + follow_redirects=False, + ) + assert response.status_code == 403 + + +def test_resend_invitation_email_failure_restores_token( + auth_client_owner, + test_organization: Organization, + existing_invitation: Invitation, + session: Session, + mock_resend_send: MagicMock, +): + """Resend email failure rolls back the token and expiry changes.""" + assert test_organization.id is not None + assert existing_invitation.id is not None + old_token = existing_invitation.token + old_expires = existing_invitation.expires_at + + mock_resend_send.side_effect = Exception("Simulated email send failure") + + response = auth_client_owner.post( + app.url_path_for("resend_invitation"), + data={ + "invitation_id": str(existing_invitation.id), + "organization_id": str(test_organization.id), + }, + follow_redirects=False, + ) + assert response.status_code == 500 + + session.refresh(existing_invitation) + assert existing_invitation.token == old_token + assert existing_invitation.expires_at == old_expires + + +def test_organization_page_shows_resend_invitation_button( + auth_client_owner, + test_organization: Organization, + existing_invitation: Invitation, +): + """Test that users with INVITE_USER see a Resend button on pending invitations.""" + assert test_organization.id is not None + + response = auth_client_owner.get( + app.url_path_for("read_organization", org_id=test_organization.id), + ) + + assert response.status_code == 200 + assert ( + f'action="http://testserver{app.url_path_for("resend_invitation")}"' + in response.text.replace("&", "&") + ) + assert "Resend" in response.text def test_organization_page_shows_cancel_invitation_button( diff --git a/tests/routers/core/test_invitation_acceptance.py b/tests/routers/core/test_invitation_acceptance.py index 95e1694..2b3eaa0 100644 --- a/tests/routers/core/test_invitation_acceptance.py +++ b/tests/routers/core/test_invitation_acceptance.py @@ -188,34 +188,109 @@ def test_accept_invitation_logged_in_correct_user_get_accepts_and_redirects( assert test_invitation.accepted_at is not None -# 4. Failure: Invalid/Expired/Used Token +# 4. Inactive token on GET /accept redirects to register/login (banners shown there) @pytest.mark.parametrize( - "token_type", + "token_type,expected_path_key", [ - "invalid", - "expired", - "used", + ("invalid", "read_login"), + ("expired", "read_register"), + ("used", "read_register"), ], ) -def test_accept_invitation_get_invalid_token_fails( +def test_accept_invitation_get_inactive_token_redirects_to_auth( unauth_client: TestClient, token_type: str, - request, # Required by getfixturevalue + expected_path_key: str, + request, ): - """GET /accept with invalid, expired, or used token fails with 404.""" + """GET /accept with invalid, expired, or used token redirects to auth pages.""" token_value = "invalid-token-string" + invitee_email = None if token_type == "expired": expired_invite: Invitation = request.getfixturevalue("expired_invitation") token_value = expired_invite.token + invitee_email = expired_invite.invitee_email elif token_type == "used": used_invite: Invitation = request.getfixturevalue("used_invitation") token_value = used_invite.token + invitee_email = used_invite.invitee_email response = unauth_client.get( app.url_path_for("accept_invitation"), params={"token": token_value}, + follow_redirects=False, + ) + assert response.status_code == 303 + parsed_url = urlparse(response.headers["location"]) + query_params = parse_qs(parsed_url.query) + + assert parsed_url.path == app.url_path_for(expected_path_key) + assert query_params.get("invitation_token") == [token_value] + if expected_path_key == "read_register": + assert query_params.get("email") == [invitee_email] + + auth_response = unauth_client.get(response.headers["location"]) + assert auth_response.status_code == 200 + if token_type == "expired": + assert "invitation link has expired" in auth_response.text.lower() + else: + assert "no longer valid" in auth_response.text.lower() + + +def test_accept_invitation_get_expired_token_existing_user_redirects_to_login( + unauth_client: TestClient, + session: Session, + expired_invitation: Invitation, + existing_invitee_account: Account, +): + """Expired token for an existing account redirects to login with the token.""" + expired_invitation.invitee_email = existing_invitee_account.email + session.add(expired_invitation) + session.commit() + + response = unauth_client.get( + app.url_path_for("accept_invitation"), + params={"token": expired_invitation.token}, + follow_redirects=False, + ) + assert response.status_code == 303 + parsed_url = urlparse(response.headers["location"]) + assert parsed_url.path == app.url_path_for("read_login") + assert parse_qs(parsed_url.query).get("invitation_token") == [ + expired_invitation.token + ] + + auth_response = unauth_client.get(response.headers["location"]) + assert auth_response.status_code == 200 + assert "invitation link has expired" in auth_response.text.lower() + + +def test_accept_invitation_logged_in_user_sees_expired_warning( + auth_client_invitee: TestClient, + session: Session, + expired_invitation: Invitation, + existing_invitee_account: Account, +): + """Logged-in users clicking an expired invite link should see an expiry warning.""" + expired_invitation.invitee_email = existing_invitee_account.email + session.add(expired_invitation) + session.commit() + + response = auth_client_invitee.get( + app.url_path_for("accept_invitation"), + params={"token": expired_invitation.token}, + follow_redirects=False, ) - assert response.status_code == 404 # InvalidInvitationTokenError maps to 404 + assert response.status_code == 303 + parsed_url = urlparse(response.headers["location"]) + assert parsed_url.path == app.url_path_for("read_login") + assert parse_qs(parsed_url.query).get("invitation_token") == [ + expired_invitation.token + ] + + auth_response = auth_client_invitee.get(response.headers["location"]) + assert auth_response.status_code == 200 + assert "invitation link has expired" in auth_response.text.lower() @pytest.mark.parametrize( @@ -254,7 +329,11 @@ def test_accept_invitation_register_post_invalid_token_fails( app.url_path_for("register"), data=register_data, ) - assert response.status_code == 404 # InvalidInvitationTokenError + assert response.status_code == 404 + if token_type == "expired": + assert "expired" in response.text.lower() + else: + assert "no longer valid" in response.text.lower() @pytest.mark.parametrize( @@ -289,7 +368,11 @@ def test_accept_invitation_login_post_invalid_token_fails( app.url_path_for("login"), data=login_data, ) - assert response.status_code == 404 # InvalidInvitationTokenError + assert response.status_code == 404 + if token_type == "expired": + assert "expired" in response.text.lower() + else: + assert "no longer valid" in response.text.lower() # 5. Failure: Email Mismatch (Registration) @@ -355,3 +438,39 @@ def test_accept_invitation_logged_in_wrong_user_get_redirects_to_login( session.refresh(test_invitation) assert test_invitation.used is False assert test_invitation.accepted_by_user_id is None + + +def test_register_page_shows_expired_invitation_warning( + unauth_client: TestClient, expired_invitation: Invitation +): + response = unauth_client.get( + app.url_path_for("read_register"), + params={ + "email": expired_invitation.invitee_email, + "invitation_token": expired_invitation.token, + }, + ) + assert response.status_code == 200 + assert "invitation link has expired" in response.text.lower() + + +def test_login_page_shows_expired_invitation_warning( + unauth_client: TestClient, expired_invitation: Invitation +): + response = unauth_client.get( + app.url_path_for("read_login"), + params={"invitation_token": expired_invitation.token}, + ) + assert response.status_code == 200 + assert "invitation link has expired" in response.text.lower() + + +def test_register_page_shows_invalid_invitation_warning( + unauth_client: TestClient, +): + response = unauth_client.get( + app.url_path_for("read_register"), + params={"invitation_token": "not-a-real-token"}, + ) + assert response.status_code == 200 + assert "no longer valid" in response.text.lower() diff --git a/tests/test_templates.py b/tests/test_templates.py index 00b3186..8feb38a 100644 --- a/tests/test_templates.py +++ b/tests/test_templates.py @@ -102,6 +102,7 @@ def test_invite_member_form_has_hx_post(): def test_pending_invitations_include_cancel_confirm(): content = Path("templates/organization/partials/invitations_list.html").read_text() assert "url_for('delete_invitation')" in content + assert "url_for('resend_invitation')" in content assert "hx-confirm" in content diff --git a/tests/utils/test_dependencies.py b/tests/utils/test_dependencies.py index d6f6f17..3ed865a 100644 --- a/tests/utils/test_dependencies.py +++ b/tests/utils/test_dependencies.py @@ -20,6 +20,7 @@ get_account_from_recovery_token, get_user_with_relations, require_unauthenticated_client, + require_unauthenticated_unless_invitation_warning, get_verified_account, ) from exceptions.http_exceptions import ( @@ -573,6 +574,33 @@ def test_require_unauthenticated_client() -> None: require_unauthenticated_client(user=mock_user) +def test_require_unauthenticated_unless_invitation_warning() -> None: + """Authenticated users may view auth pages when an invite token warning applies.""" + mock_user = User(id=1, name="Test User") + mock_session = MagicMock() + + with patch( + "utils.core.dependencies.get_invitation_token_warning", + return_value=None, + ): + with pytest.raises(AlreadyAuthenticatedError): + require_unauthenticated_unless_invitation_warning( + invitation_token="some-token", + user=mock_user, + session=mock_session, + ) + + with patch( + "utils.core.dependencies.get_invitation_token_warning", + return_value="expired", + ): + require_unauthenticated_unless_invitation_warning( + invitation_token="expired-token", + user=mock_user, + session=mock_session, + ) + + def test_get_verified_account() -> None: """Tests that get_verified_account verifies email and password.""" mock_account = Account( diff --git a/utils/core/dependencies.py b/utils/core/dependencies.py index 971c12e..507a05d 100644 --- a/utils/core/dependencies.py +++ b/utils/core/dependencies.py @@ -1,5 +1,5 @@ import logging -from fastapi import Depends, Form, Request +from fastapi import Depends, Form, Query, Request from pydantic import EmailStr from sqlmodel import Session, select from sqlalchemy.orm import selectinload @@ -31,6 +31,7 @@ PasswordValidationError, ) from exceptions.exceptions import NeedsNewTokens +from utils.core.invitations import get_invitation_token_warning logger = logging.getLogger(__name__) @@ -284,6 +285,24 @@ def require_unauthenticated_client( raise AlreadyAuthenticatedError() +def require_unauthenticated_unless_invitation_warning( + invitation_token: Optional[str] = Query(None), + user: Optional[User] = Depends(get_optional_user), + session: Session = Depends(get_session), +) -> None: + """ + Allow authenticated users to view login/register when an invitation token + warning must be shown (expired or invalid invite links). + """ + warning = ( + get_invitation_token_warning(session, invitation_token) + if invitation_token + else None + ) + if user and not warning: + raise AlreadyAuthenticatedError() + + def get_verified_account( email: EmailStr = Form( ..., title="Email", description="Account email address for verification" diff --git a/utils/core/invitations.py b/utils/core/invitations.py index 1176911..2c0aaed 100644 --- a/utils/core/invitations.py +++ b/utils/core/invitations.py @@ -1,13 +1,18 @@ import os from logging import getLogger, DEBUG +from typing import Literal, Optional import resend -from sqlmodel import Session +from sqlmodel import Session, select from jinja2.environment import Template from fastapi.templating import Jinja2Templates from utils.core.models import utc_now, Invitation, Organization, User from exceptions.exceptions import EmailSendFailedError -from exceptions.http_exceptions import DataIntegrityError +from exceptions.http_exceptions import ( + DataIntegrityError, + ExpiredInvitationTokenError, + InvalidInvitationTokenError, +) # Setup logging @@ -31,6 +36,35 @@ def generate_invitation_link(token: str) -> str: return f"{os.getenv('BASE_URL')}/invitations/accept?token={token}" +InvitationTokenWarning = Literal["expired", "invalid"] + + +def get_invitation_token_warning( + session: Session, token: str +) -> Optional[InvitationTokenWarning]: + """Return a warning key for register/login UI, or None if the token is active.""" + invitation = session.exec( + select(Invitation).where(Invitation.token == token) + ).first() + if invitation is None or invitation.used: + return "invalid" + if invitation.is_expired(): + return "expired" + return None + + +def require_active_invitation_by_token(session: Session, token: str) -> Invitation: + """Load an invitation by token or raise an HTTP exception with a clear message.""" + invitation = session.exec( + select(Invitation).where(Invitation.token == token) + ).first() + if invitation is None or invitation.used: + raise InvalidInvitationTokenError() + if invitation.is_expired(): + raise ExpiredInvitationTokenError() + return invitation + + def send_invitation_email(invitation: Invitation, session: Session) -> None: """ Sends an organization invitation email using Resend. diff --git a/utils/core/models.py b/utils/core/models.py index 7181fd0..970c557 100644 --- a/utils/core/models.py +++ b/utils/core/models.py @@ -375,3 +375,36 @@ def get_active_for_org( ) results = session.exec(statement).all() return [inv for inv in results if not inv.is_expired()] + + @classmethod + def get_pending_for_org( + cls, session: Session, organization_id: int + ) -> list["Invitation"]: + """Return all unused invitations for an org, including expired rows.""" + statement = ( + select(cls) + .where( + cls.organization_id == organization_id, + col(cls.used).is_(False), + ) + .order_by(col(cls.created_at).desc()) + ) + return list(session.exec(statement).all()) + + @classmethod + def invalidate_pending_for_email( + cls, + session: Session, + organization_id: int, + invitee_email: str, + ) -> list["Invitation"]: + """Delete unused invitations for an org+email. Caller must commit or rollback.""" + statement = select(cls).where( + cls.organization_id == organization_id, + cls.invitee_email == invitee_email, + col(cls.used).is_(False), + ) + pending: list[Invitation] = list(session.exec(statement).all()) + for invitation in pending: + session.delete(invitation) + return pending diff --git a/utils/core/organizations.py b/utils/core/organizations.py index 2282d00..27e2ed6 100644 --- a/utils/core/organizations.py +++ b/utils/core/organizations.py @@ -31,8 +31,8 @@ def load_org_for_members_partial( ) ).first() user_permissions = _user_permissions_for_org(user, organization_id) - active_invitations = Invitation.get_active_for_org(session, organization_id) - return organization, user_permissions, active_invitations + pending_invitations = Invitation.get_pending_for_org(session, organization_id) + return organization, user_permissions, pending_invitations def load_org_for_roles_partial(