From e4749b4e7cdd78ab95627736d3fcd84fecc55e53 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 18 Mar 2026 23:43:16 +0000 Subject: [PATCH] feat(api): allow disabled tokens/devices to be deleted & reactivated; add token lifetime Co-authored-by: christianlouis <361235+christianlouis@users.noreply.github.com> --- app/api/api_tokens.py | 79 +++++- app/api/mobile.py | 29 ++- app/auth.py | 10 + app/models.py | 3 + docs/API.md | 39 ++- frontend/templates/api_tokens.html | 125 +++++++++- frontend/templates/devices.html | 131 +++++++++- frontend/translations/en.json | 22 ++ .../versions/038_add_api_token_expires_at.py | 43 ++++ tests/test_api_mobile.py | 32 ++- tests/test_api_tokens.py | 227 +++++++++++++++++- 11 files changed, 687 insertions(+), 53 deletions(-) create mode 100644 migrations/versions/038_add_api_token_expires_at.py diff --git a/app/api/api_tokens.py b/app/api/api_tokens.py index 1beef61b..62074b8a 100644 --- a/app/api/api_tokens.py +++ b/app/api/api_tokens.py @@ -13,7 +13,7 @@ plaintext is returned exactly once at creation time. import hashlib import logging import secrets -from datetime import datetime, timezone +from datetime import datetime, timedelta, timezone from typing import Annotated, Any from fastapi import APIRouter, Depends, HTTPException, Request, status @@ -105,6 +105,7 @@ def _token_to_dict(t: ApiToken) -> dict[str, Any]: "last_used_ip": t.last_used_ip, "created_at": t.created_at, "revoked_at": t.revoked_at, + "expires_at": t.expires_at, } @@ -117,6 +118,12 @@ class TokenCreate(BaseModel): """Schema for creating a new API token.""" name: str = Field(..., min_length=1, max_length=255, description="Human-readable label for the token") + expires_in_days: int | None = Field( + default=None, + ge=1, + le=3650, # Maximum 10 years; keeps tokens from being effectively permanent while allowing long-lived CI/CD tokens. + description="Optional lifetime in days. If omitted the token never expires.", + ) class TokenResponse(BaseModel): @@ -130,6 +137,7 @@ class TokenResponse(BaseModel): last_used_ip: str | None created_at: datetime | None revoked_at: datetime | None + expires_at: datetime | None model_config = {"from_attributes": True} @@ -160,11 +168,16 @@ async def create_token( token_hash_value = hash_token(plaintext) prefix = plaintext[:12] # "de_" prefix + 9 random chars = 12 chars total + expires_at = None + if body.expires_in_days is not None: + expires_at = datetime.now(timezone.utc) + timedelta(days=body.expires_in_days) + db_token = ApiToken( owner_id=owner_id, name=body.name, token_hash=token_hash_value, token_prefix=prefix, + expires_at=expires_at, ) try: db.add(db_token) @@ -185,6 +198,7 @@ async def create_token( "last_used_ip": db_token.last_used_ip, "created_at": db_token.created_at, "revoked_at": db_token.revoked_at, + "expires_at": db_token.expires_at, "token": plaintext, } @@ -235,30 +249,73 @@ async def list_mobile_tokens( @router.delete("/{token_id}", status_code=status.HTTP_200_OK) -async def revoke_token( +async def revoke_or_delete_token( token_id: int, owner_id: CurrentOwner, db: DbSession, ) -> dict[str, str]: - """Revoke (soft-delete) an API token. + """Revoke or permanently delete an API token. - The token row is kept for audit purposes but marked inactive with a - ``revoked_at`` timestamp. + * **Active token** – soft-revoked: the row is kept for audit purposes + but marked inactive with a ``revoked_at`` timestamp. + * **Already-revoked token** – hard-deleted: the row is permanently + removed from the database. """ db_token = db.query(ApiToken).filter(ApiToken.id == token_id, ApiToken.owner_id == owner_id).first() if not db_token: raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail="Token not found") - if not db_token.is_active: - raise HTTPException(status_code=status.HTTP_400_BAD_REQUEST, detail="Token is already revoked") + if db_token.is_active: + # Soft-revoke the active token. + try: + db_token.is_active = False + db_token.revoked_at = datetime.now(timezone.utc) + db.commit() + except Exception: + db.rollback() + raise + logger.info("API token revoked: id=%s owner=%s", token_id, owner_id) + return {"detail": "Token revoked"} + # Hard-delete an already-revoked token. try: - db_token.is_active = False - db_token.revoked_at = datetime.now(timezone.utc) + db.delete(db_token) db.commit() except Exception: db.rollback() raise + logger.info("API token permanently deleted: id=%s owner=%s", token_id, owner_id) + return {"detail": "Token deleted"} - logger.info("API token revoked: id=%s owner=%s", token_id, owner_id) - return {"detail": "Token revoked"} + +@router.post("/{token_id}/reactivate", status_code=status.HTTP_200_OK, response_model=TokenResponse) +async def reactivate_token( + token_id: int, + owner_id: CurrentOwner, + db: DbSession, +) -> dict[str, Any]: + """Reactivate a previously revoked API token. + + Clears the ``revoked_at`` timestamp and sets ``is_active`` back to + ``True``. The token can be used for authentication again immediately. + If the token had an ``expires_at`` in the past the caller should + consider re-creating a new token instead. + """ + db_token = db.query(ApiToken).filter(ApiToken.id == token_id, ApiToken.owner_id == owner_id).first() + if not db_token: + raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail="Token not found") + + if db_token.is_active: + raise HTTPException(status_code=status.HTTP_400_BAD_REQUEST, detail="Token is already active") + + try: + db_token.is_active = True + db_token.revoked_at = None + db.commit() + db.refresh(db_token) + except Exception: + db.rollback() + raise + + logger.info("API token reactivated: id=%s owner=%s", token_id, owner_id) + return _token_to_dict(db_token) diff --git a/app/api/mobile.py b/app/api/mobile.py index 465872ab..c9163f00 100644 --- a/app/api/mobile.py +++ b/app/api/mobile.py @@ -273,31 +273,44 @@ async def list_devices( return [_device_to_response(d) for d in devices] -@router.delete("/devices/{device_id}", status_code=status.HTTP_204_NO_CONTENT) +@router.delete("/devices/{device_id}", status_code=status.HTTP_200_OK) @require_login async def deactivate_device( request: Request, device_id: int, owner_id: CurrentOwner, db: DbSession, -) -> None: - """Deactivate a push-notification device registration. +) -> dict[str, str]: + """Deactivate or permanently delete a push-notification device registration. - The device record is kept for audit purposes but will no longer receive - push notifications. + * **Active device** – soft-deactivated: the record is kept for audit + purposes but will no longer receive push notifications. + * **Already-inactive device** – hard-deleted: the record is permanently + removed from the database. """ device = db.get(MobileDevice, device_id) if not device or device.owner_id != owner_id: raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail="Device not found") - device.is_active = False + if device.is_active: + device.is_active = False + try: + db.commit() + except Exception: + db.rollback() + raise + logger.info("Mobile device deactivated: id=%s owner=%s", device_id, owner_id) + return {"detail": "Device deactivated"} + + # Hard-delete an already-inactive device. try: + db.delete(device) db.commit() except Exception: db.rollback() raise - - logger.info("Mobile device deactivated: id=%s owner=%s", device_id, owner_id) + logger.info("Mobile device permanently deleted: id=%s owner=%s", device_id, owner_id) + return {"detail": "Device deleted"} @router.get("/whoami", response_model=WhoAmIResponse) diff --git a/app/auth.py b/app/auth.py index c5883dac..072e173b 100644 --- a/app/auth.py +++ b/app/auth.py @@ -186,6 +186,16 @@ def _resolve_bearer_user(request: Request, db: Session) -> dict | None: logger.debug("[AUTH] _resolve_bearer_user: no active API token matched the provided hash") return None + # Reject tokens that have passed their optional expiry. + if db_token.expires_at is not None: + now_utc = datetime.now(timezone.utc) + expires_aware = db_token.expires_at + if expires_aware.tzinfo is None: + expires_aware = expires_aware.replace(tzinfo=timezone.utc) + if now_utc > expires_aware: + logger.debug("[AUTH] _resolve_bearer_user: API token id=%s has expired", db_token.id) + return None + logger.debug( "[AUTH] _resolve_bearer_user: matched API token id=%s owner=%s", db_token.id, diff --git a/app/models.py b/app/models.py index 5f406953..2bba0446 100644 --- a/app/models.py +++ b/app/models.py @@ -786,6 +786,9 @@ class ApiToken(Base): created_at = Column(DateTime(timezone=True), server_default=func.now()) revoked_at = Column(DateTime(timezone=True), nullable=True) + # Optional expiry: if set, the token is rejected after this timestamp. + expires_at = Column(DateTime(timezone=True), nullable=True) + class SharedLink(Base): """Shareable, time-limited or view-limited document link. diff --git a/docs/API.md b/docs/API.md index 29a3dc7a..84bffa75 100644 --- a/docs/API.md +++ b/docs/API.md @@ -115,7 +115,8 @@ curl -X GET "http:///api/files" \ |--------|----------|-------------| | `POST` | `/api/api-tokens/` | Create a new token | | `GET` | `/api/api-tokens/` | List all your tokens | -| `DELETE` | `/api/api-tokens/{id}` | Revoke a token | +| `DELETE` | `/api/api-tokens/{id}` | Revoke (active) or permanently delete (revoked) a token | +| `POST` | `/api/api-tokens/{id}/reactivate` | Reactivate a revoked token | ### Session Authentication @@ -2127,12 +2128,14 @@ Usage tracking records when each token was last used and from which IP address. ### POST /api/api-tokens/ -Create a new API token. +Create a new API token. Optionally specify a lifetime in days via +`expires_in_days` (1–3650). If omitted the token never expires. **Request:** ```json { - "name": "CI Pipeline" + "name": "CI Pipeline", + "expires_in_days": 90 } ``` @@ -2147,7 +2150,8 @@ Create a new API token. "last_used_at": null, "last_used_ip": null, "created_at": "2026-03-08T12:00:00Z", - "revoked_at": null + "revoked_at": null, + "expires_at": "2026-06-06T12:00:00Z" } ``` @@ -2169,15 +2173,20 @@ List all tokens for the authenticated user. The full token value is never includ "last_used_at": "2026-03-08T15:30:00Z", "last_used_ip": "203.0.113.42", "created_at": "2026-03-08T12:00:00Z", - "revoked_at": null + "revoked_at": null, + "expires_at": "2026-06-06T12:00:00Z" } ] ``` ### DELETE /api/api-tokens/{token_id} -Revoke a token. The token is soft-deleted (kept for audit purposes) and can no -longer be used for authentication. +Revoke or permanently delete a token: + +* **Active token** – soft-revoked (kept for audit purposes, marked inactive). + Response: `{"detail": "Token revoked"}` +* **Already-revoked token** – permanently deleted from the database. + Response: `{"detail": "Token deleted"}` **Response (200):** ```json @@ -2186,6 +2195,13 @@ longer be used for authentication. } ``` +### POST /api/api-tokens/{token_id}/reactivate + +Reactivate a previously revoked token. Clears `revoked_at` and sets +`is_active` back to `true`. + +**Response (200):** The updated `TokenResponse` object. + ### Using API Tokens Include the token in the `Authorization` header of any API request: @@ -2268,9 +2284,14 @@ List all registered push-notification devices for the current user. ### DELETE /api/mobile/devices/{device_id} -Deactivate a push-notification device. The device will no longer receive push notifications. +Deactivate or permanently delete a push-notification device: -**Response (204 No Content)** +* **Active device** – soft-deactivated (record kept, will no longer receive push notifications). + Response: `{"detail": "Device deactivated"}` +* **Already-inactive device** – permanently deleted from the database. + Response: `{"detail": "Device deleted"}` + +**Response (200)** ### GET /api/mobile/whoami diff --git a/frontend/templates/api_tokens.html b/frontend/templates/api_tokens.html index 5e4633cf..b4facee8 100644 --- a/frontend/templates/api_tokens.html +++ b/frontend/templates/api_tokens.html @@ -33,6 +33,20 @@ aria-required="true" /> +
+ + +
+ + + + @@ -209,9 +258,10 @@ function apiTokens() { tokens: [], loading: true, creating: false, - revoking: null, + acting: null, error: null, newTokenName: '', + newTokenExpiresDays: null, newlyCreatedToken: null, copied: false, baseUrl: window.location.origin, @@ -238,13 +288,15 @@ function apiTokens() { this.error = null; this.newlyCreatedToken = null; try { + const body = { name: this.newTokenName.trim() }; + if (this.newTokenExpiresDays) body.expires_in_days = parseInt(this.newTokenExpiresDays); const res = await fetch('/api/api-tokens/', { method: 'POST', headers: { 'Content-Type': 'application/json', 'X-CSRF-Token': csrfToken, }, - body: JSON.stringify({ name: this.newTokenName.trim() }), + body: JSON.stringify(body), }); if (!res.ok) { const data = await res.json().catch(() => ({})); @@ -253,6 +305,7 @@ function apiTokens() { const data = await res.json(); this.newlyCreatedToken = data.token; this.newTokenName = ''; + this.newTokenExpiresDays = null; await this.loadTokens(); } catch (e) { this.error = e.message; @@ -263,7 +316,7 @@ function apiTokens() { async revokeToken(token) { if (!confirm(`Revoke token "${token.name}"? This cannot be undone.`)) return; - this.revoking = token.id; + this.acting = token.id; this.error = null; try { const res = await fetch(`/api/api-tokens/${token.id}`, { @@ -278,10 +331,66 @@ function apiTokens() { } catch (e) { this.error = e.message; } finally { - this.revoking = null; + this.acting = null; } }, + async reactivateToken(token) { + if (!confirm({{ _("api_tokens.reactivate_confirm") | tojson }})) return; + this.acting = token.id; + this.error = null; + try { + const res = await fetch(`/api/api-tokens/${token.id}/reactivate`, { + method: 'POST', + headers: { 'X-CSRF-Token': csrfToken }, + }); + if (!res.ok) { + const data = await res.json().catch(() => ({})); + throw new Error(data.detail || 'Failed to reactivate token'); + } + await this.loadTokens(); + } catch (e) { + this.error = e.message; + } finally { + this.acting = null; + } + }, + + async deleteToken(token) { + if (!confirm({{ _("api_tokens.delete_confirm") | tojson }})) return; + this.acting = token.id; + this.error = null; + try { + const res = await fetch(`/api/api-tokens/${token.id}`, { + method: 'DELETE', + headers: { 'X-CSRF-Token': csrfToken }, + }); + if (!res.ok) { + const data = await res.json().catch(() => ({})); + throw new Error(data.detail || 'Failed to delete token'); + } + await this.loadTokens(); + } catch (e) { + this.error = e.message; + } finally { + this.acting = null; + } + }, + + tokenStatusClass(token) { + if (!token.is_active) return 'bg-red-100 text-red-800 dark:bg-red-900/30 dark:text-red-400'; + if (token.expires_at && new Date(token.expires_at) < new Date()) + return 'bg-yellow-100 text-yellow-800 dark:bg-yellow-900/30 dark:text-yellow-400'; + return 'bg-green-100 text-green-800 dark:bg-green-900/30 dark:text-green-400'; + }, + + tokenStatusLabel(token) { + if (!token.is_active) return '{{ _("api_tokens.status_revoked") }}'; + if (token.expires_at && new Date(token.expires_at) < new Date()) + return '{{ _("api_tokens.status_expired") }}'; + return '{{ _("api_tokens.status_active") }}'; + }, + copyToken() { if (this.newlyCreatedToken) { navigator.clipboard.writeText(this.newlyCreatedToken); diff --git a/frontend/templates/devices.html b/frontend/templates/devices.html index 40e0a255..498fd486 100644 --- a/frontend/templates/devices.html +++ b/frontend/templates/devices.html @@ -83,20 +83,51 @@ > + + + + + @@ -182,16 +213,31 @@ x-show="device.is_active" type="button" @click="deactivateDevice(device)" - :disabled="deactivatingDevice === device.id" + :disabled="actingDevice === device.id" class="flex-shrink-0 inline-flex items-center px-3 py-1.5 text-sm font-medium text-red-600 hover:text-red-800 dark:text-red-400 dark:hover:text-red-300 hover:bg-red-50 dark:hover:bg-red-900/20 rounded-md focus:outline-none focus:ring-2 focus:ring-red-500 disabled:opacity-50 transition-colors" style="min-height:36px; min-width:44px;" :aria-label="'{{ _('devices.deactivate_device') }} ' + (device.device_name || 'device')" > - + {{ _("devices.deactivate_device") }} + + @@ -242,8 +288,8 @@ function devicesPage() { devices: [], loadingTokens: true, loadingDevices: true, - revokingToken: null, - deactivatingDevice: null, + actingToken: null, + actingDevice: null, tokenError: null, deviceError: null, banner: { visible: false, error: false, message: '' }, @@ -286,7 +332,7 @@ function devicesPage() { async revokeToken(token) { if (!confirm({{ _("devices.confirm_revoke_token") | tojson }})) return; - this.revokingToken = token.id; + this.actingToken = token.id; try { const res = await fetch(`/api/api-tokens/${token.id}`, { method: 'DELETE', @@ -301,19 +347,61 @@ function devicesPage() { } catch (e) { this._showBanner(e.message, true); } finally { - this.revokingToken = null; + this.actingToken = null; + } + }, + + async reactivateMobileToken(token) { + if (!confirm({{ _("devices.reactivate_token_confirm") | tojson }})) return; + this.actingToken = token.id; + try { + const res = await fetch(`/api/api-tokens/${token.id}/reactivate`, { + method: 'POST', + headers: { 'X-CSRF-Token': csrfToken }, + }); + if (!res.ok) { + const data = await res.json().catch(() => ({})); + throw new Error(data.detail || 'Failed to reactivate token'); + } + await this.loadMobileTokens(); + this._showBanner({{ _("devices.token_reactivated_success") | tojson }}, false); + } catch (e) { + this._showBanner(e.message, true); + } finally { + this.actingToken = null; + } + }, + + async deleteMobileToken(token) { + if (!confirm({{ _("devices.delete_token_confirm") | tojson }})) return; + this.actingToken = token.id; + try { + const res = await fetch(`/api/api-tokens/${token.id}`, { + method: 'DELETE', + headers: { 'X-CSRF-Token': csrfToken }, + }); + if (!res.ok) { + const data = await res.json().catch(() => ({})); + throw new Error(data.detail || 'Failed to delete token'); + } + await this.loadMobileTokens(); + this._showBanner({{ _("devices.token_deleted_success") | tojson }}, false); + } catch (e) { + this._showBanner(e.message, true); + } finally { + this.actingToken = null; } }, async deactivateDevice(device) { if (!confirm({{ _("devices.confirm_deactivate_device") | tojson }})) return; - this.deactivatingDevice = device.id; + this.actingDevice = device.id; try { const res = await fetch(`/api/mobile/devices/${device.id}`, { method: 'DELETE', headers: { 'X-CSRF-Token': csrfToken }, }); - if (!res.ok && res.status !== 204) { + if (!res.ok) { const data = await res.json().catch(() => ({})); throw new Error(data.detail || 'Failed to remove device'); } @@ -322,7 +410,28 @@ function devicesPage() { } catch (e) { this._showBanner(e.message, true); } finally { - this.deactivatingDevice = null; + this.actingDevice = null; + } + }, + + async deleteDevice(device) { + if (!confirm({{ _("devices.delete_device_confirm") | tojson }})) return; + this.actingDevice = device.id; + try { + const res = await fetch(`/api/mobile/devices/${device.id}`, { + method: 'DELETE', + headers: { 'X-CSRF-Token': csrfToken }, + }); + if (!res.ok) { + const data = await res.json().catch(() => ({})); + throw new Error(data.detail || 'Failed to delete device'); + } + await this.loadDevices(); + this._showBanner({{ _("devices.device_deleted_success") | tojson }}, false); + } catch (e) { + this._showBanner(e.message, true); + } finally { + this.actingDevice = null; } }, diff --git a/frontend/translations/en.json b/frontend/translations/en.json index ebb67b2e..dbb9f8fb 100644 --- a/frontend/translations/en.json +++ b/frontend/translations/en.json @@ -316,6 +316,7 @@ "admin_users.total_no_users": "No users", "admin_users.total_one_user": "1 user", "api_tokens.col_created": "Created", + "api_tokens.col_expires": "Expires", "api_tokens.col_last_ip": "Last IP", "api_tokens.col_last_used": "Last Used", "api_tokens.col_name": "Name", @@ -327,6 +328,14 @@ "api_tokens.create_heading": "Create New Token", "api_tokens.create_token": "Create Token", "api_tokens.creating": "Creating…", + "api_tokens.delete": "Delete", + "api_tokens.delete_confirm": "Permanently delete this revoked token? This cannot be undone.", + "api_tokens.delete_prefix": "Permanently delete token", + "api_tokens.expires_at_label": "Expires (optional)", + "api_tokens.expires_at_placeholder": "e.g. 30, 90, 365 days", + "api_tokens.expires_in_days_label": "Token lifetime (days)", + "api_tokens.expires_never": "Never", + "api_tokens.expires_on": "Expires", "api_tokens.heading": "API Tokens", "api_tokens.intro": "Create personal API tokens to interact with the DocuElevate API programmatically. Use tokens for webhook uploads, CI/CD pipelines, or any script that needs to upload or retrieve documents.", "api_tokens.loading_tokens": "Loading tokens…", @@ -334,9 +343,13 @@ "api_tokens.no_tokens_heading": "No API tokens yet", "api_tokens.no_tokens_help": "Create your first token above to get started.", "api_tokens.page_title": "API Tokens – DocuElevate", + "api_tokens.reactivate": "Reactivate", + "api_tokens.reactivate_confirm": "Reactivate this token? It will be usable again immediately.", + "api_tokens.reactivate_prefix": "Reactivate token", "api_tokens.revoke": "Revoke", "api_tokens.revoke_prefix": "Revoke token", "api_tokens.status_active": "Active", + "api_tokens.status_expired": "Expired", "api_tokens.status_revoked": "Revoked", "api_tokens.table_aria": "API Tokens", "api_tokens.token_created": "Token created successfully!", @@ -620,6 +633,11 @@ "devices.confirm_deactivate_device": "Remove this device? It will stop receiving push notifications.", "devices.confirm_revoke_token": "Revoke access for this device? It will need to log in again.", "devices.deactivate_device": "Remove", + "devices.delete_device": "Delete", + "devices.delete_device_confirm": "Permanently delete this inactive device? This cannot be undone.", + "devices.delete_token": "Delete", + "devices.delete_token_confirm": "Permanently delete this revoked token? This cannot be undone.", + "devices.device_deleted_success": "Device permanently deleted.", "devices.device_removed_success": "Device removed successfully.", "devices.heading": "Mobile Devices", "devices.intro": "Manage your mobile app connections and registered devices. You can revoke access for individual devices here.", @@ -632,12 +650,16 @@ "devices.no_mobile_tokens_help": "Log in via the mobile app or scan a QR code to create a mobile token.", "devices.page_title": "Devices – DocuElevate", "devices.qr_login_cta": "Connect a new device via QR code", + "devices.reactivate_token": "Reactivate", + "devices.reactivate_token_confirm": "Reactivate this token? The device will be able to use it again immediately.", "devices.registered_devices_description": "Devices registered for push notifications from the DocuElevate mobile app.", "devices.registered_devices_heading": "Registered Devices", "devices.revoke_token": "Revoke", "devices.status_active": "Active", "devices.status_inactive": "Inactive", "devices.status_revoked": "Revoked", + "devices.token_deleted_success": "Token permanently deleted.", + "devices.token_reactivated_success": "Token reactivated successfully.", "devices.token_revoked_success": "Device token revoked successfully.", "duplicates.file_id_label": "File ID", "duplicates.file_id_placeholder": "e.g. 42", diff --git a/migrations/versions/038_add_api_token_expires_at.py b/migrations/versions/038_add_api_token_expires_at.py new file mode 100644 index 00000000..20d932aa --- /dev/null +++ b/migrations/versions/038_add_api_token_expires_at.py @@ -0,0 +1,43 @@ +"""Add expires_at column to api_tokens table. + +Allows API tokens to be issued with an optional lifetime. If ``expires_at`` +is set, the token is automatically rejected after that timestamp. + +Revision ID: 038_add_api_token_expires_at +Revises: 037_add_user_sessions_and_qr_challenges +Create Date: 2026-03-18 +""" + +from typing import Union + +import sqlalchemy as sa +from alembic import op + +revision: str = "038_add_api_token_expires_at" +down_revision: Union[str, None] = "037_add_user_sessions_and_qr_challenges" +depends_on: Union[str, None] = None + + +def upgrade() -> None: + """Add expires_at column to api_tokens (idempotent).""" + conn = op.get_bind() + inspector = sa.inspect(conn) + if "api_tokens" not in inspector.get_table_names(): + return + existing_columns = {col["name"] for col in inspector.get_columns("api_tokens")} + if "expires_at" not in existing_columns: + op.add_column( + "api_tokens", + sa.Column("expires_at", sa.DateTime(timezone=True), nullable=True), + ) + + +def downgrade() -> None: + """Remove expires_at column from api_tokens.""" + conn = op.get_bind() + inspector = sa.inspect(conn) + if "api_tokens" not in inspector.get_table_names(): + return + existing_columns = {col["name"] for col in inspector.get_columns("api_tokens")} + if "expires_at" in existing_columns: + op.drop_column("api_tokens", "expires_at") diff --git a/tests/test_api_mobile.py b/tests/test_api_mobile.py index 54b08967..6069694e 100644 --- a/tests/test_api_mobile.py +++ b/tests/test_api_mobile.py @@ -329,7 +329,7 @@ class TestDeactivateDevice: """Tests for DELETE /api/mobile/devices/{device_id}.""" def test_deactivate_own_device(self, mob_engine, mob_session): - """Deactivating a device sets is_active to False.""" + """Deactivating an active device sets is_active to False (soft-delete, returns 200).""" from app.main import app device = MobileDevice( @@ -346,7 +346,8 @@ class TestDeactivateDevice: client = _make_client(mob_engine) try: resp = client.delete(f"/api/mobile/devices/{device_id}") - assert resp.status_code == 204 + assert resp.status_code == 200 + assert resp.json()["detail"] == "Device deactivated" mob_session.expire_all() updated = mob_session.get(MobileDevice, device_id) @@ -355,6 +356,33 @@ class TestDeactivateDevice: finally: _cleanup(app) + def test_delete_inactive_device(self, mob_engine, mob_session): + """Deleting an already-inactive device permanently removes it (hard-delete, returns 200).""" + from app.main import app + + device = MobileDevice( + owner_id=_OWNER, + push_token=_EXPO_TOKEN, + platform="ios", + is_active=False, + ) + mob_session.add(device) + mob_session.commit() + mob_session.refresh(device) + device_id = device.id + + client = _make_client(mob_engine) + try: + resp = client.delete(f"/api/mobile/devices/{device_id}") + assert resp.status_code == 200 + assert resp.json()["detail"] == "Device deleted" + + mob_session.expire_all() + deleted = mob_session.get(MobileDevice, device_id) + assert deleted is None + finally: + _cleanup(app) + def test_deactivate_other_users_device_returns_404(self, mob_engine, mob_session): """Attempting to deactivate another user's device returns 404.""" from app.main import app diff --git a/tests/test_api_tokens.py b/tests/test_api_tokens.py index dabf2dd8..2a2b124a 100644 --- a/tests/test_api_tokens.py +++ b/tests/test_api_tokens.py @@ -314,8 +314,8 @@ class TestTokenRevoke: _cleanup(app) @pytest.mark.unit - def test_revoke_already_revoked_token(self, tok_engine): - """Revoking an already-revoked token should return 400.""" + def test_delete_already_revoked_token(self, tok_engine): + """Deleting an already-revoked token should permanently remove it (hard-delete, 200).""" from app.main import app client = _make_client(tok_engine) @@ -324,9 +324,15 @@ class TestTokenRevoke: token_id = create_resp.json()["id"] client.delete(f"/api/api-tokens/{token_id}") + # Second DELETE should hard-delete the revoked token. resp = client.delete(f"/api/api-tokens/{token_id}") - assert resp.status_code == 400 - assert resp.json()["detail"] == "Token is already revoked" + assert resp.status_code == 200 + assert resp.json()["detail"] == "Token deleted" + + # Token must no longer appear in the list. + list_resp = client.get("/api/api-tokens/") + ids = [t["id"] for t in list_resp.json()] + assert token_id not in ids finally: _cleanup(app) @@ -677,3 +683,216 @@ class TestTokenUtils: token = "de_test_token_value" expected_hash = "9b89d9adf2f390c75bf2fd0ff2bb5622ef5a9dce438354cce6e39f2f5401129e" assert hash_token(token) == expected_hash + + +# --------------------------------------------------------------------------- +# Tests – Token reactivation +# --------------------------------------------------------------------------- + + +class TestTokenReactivate: + """Tests for POST /api/api-tokens/{id}/reactivate.""" + + @pytest.mark.unit + def test_reactivate_revoked_token(self, tok_engine): + """Reactivating a revoked token should set is_active=True and clear revoked_at.""" + from app.main import app + + client = _make_client(tok_engine) + try: + create_resp = client.post("/api/api-tokens/", json={"name": "Reactivate Me"}) + token_id = create_resp.json()["id"] + client.delete(f"/api/api-tokens/{token_id}") + + resp = client.post(f"/api/api-tokens/{token_id}/reactivate") + assert resp.status_code == 200 + data = resp.json() + assert data["is_active"] is True + assert data["revoked_at"] is None + finally: + _cleanup(app) + + @pytest.mark.unit + def test_reactivate_active_token_returns_400(self, tok_engine): + """Reactivating an already-active token should return 400.""" + from app.main import app + + client = _make_client(tok_engine) + try: + create_resp = client.post("/api/api-tokens/", json={"name": "Already Active"}) + token_id = create_resp.json()["id"] + + resp = client.post(f"/api/api-tokens/{token_id}/reactivate") + assert resp.status_code == 400 + assert resp.json()["detail"] == "Token is already active" + finally: + _cleanup(app) + + @pytest.mark.unit + def test_reactivate_nonexistent_token(self, tok_engine): + """Reactivating a non-existent token should return 404.""" + from app.main import app + + client = _make_client(tok_engine) + try: + resp = client.post("/api/api-tokens/99999/reactivate") + assert resp.status_code == 404 + finally: + _cleanup(app) + + @pytest.mark.unit + def test_reactivate_other_users_token(self, tok_engine): + """A user cannot reactivate another user's token.""" + from app.main import app + + client_a = _make_client(tok_engine, _OWNER) + try: + create_resp = client_a.post("/api/api-tokens/", json={"name": "A Token"}) + token_id = create_resp.json()["id"] + client_a.delete(f"/api/api-tokens/{token_id}") + finally: + _cleanup(app) + + client_b = _make_client(tok_engine, _OTHER_OWNER) + try: + resp = client_b.post(f"/api/api-tokens/{token_id}/reactivate") + assert resp.status_code == 404 + finally: + _cleanup(app) + + +# --------------------------------------------------------------------------- +# Tests – Token lifetime (expires_at) +# --------------------------------------------------------------------------- + + +class TestTokenExpiry: + """Tests for token creation with optional lifetime and expiry enforcement.""" + + @pytest.mark.unit + def test_create_token_without_expiry(self, tok_engine): + """Creating a token without expires_in_days should leave expires_at as None.""" + from app.main import app + + client = _make_client(tok_engine) + try: + resp = client.post("/api/api-tokens/", json={"name": "No Expiry"}) + assert resp.status_code == 201 + data = resp.json() + assert data["expires_at"] is None + finally: + _cleanup(app) + + @pytest.mark.unit + def test_create_token_with_expiry(self, tok_engine, tok_session): + """Creating a token with expires_in_days should set expires_at in the future.""" + from datetime import datetime, timezone + + from app.main import app + + client = _make_client(tok_engine) + try: + resp = client.post("/api/api-tokens/", json={"name": "With Expiry", "expires_in_days": 30}) + assert resp.status_code == 201 + data = resp.json() + assert data["expires_at"] is not None + # Parse the returned datetime; handle both tz-aware and tz-naive serialisations + expires_str = data["expires_at"].replace("Z", "+00:00") + expires_at = datetime.fromisoformat(expires_str) + if expires_at.tzinfo is None: + expires_at = expires_at.replace(tzinfo=timezone.utc) + now = datetime.now(timezone.utc) + delta_days = (expires_at - now).days + assert 28 <= delta_days <= 30 + finally: + _cleanup(app) + + @pytest.mark.unit + def test_expired_token_not_resolved(self, tok_engine, tok_session): + """A token past its expires_at should not authenticate.""" + from datetime import datetime, timedelta, timezone + from unittest.mock import MagicMock + + from app.api.api_tokens import generate_api_token, hash_token + from app.auth import _resolve_bearer_user + + plaintext = generate_api_token() + token_hash = hash_token(plaintext) + + db_token = ApiToken( + owner_id=_OWNER, + name="Expired Token", + token_hash=token_hash, + token_prefix=plaintext[:12], + is_active=True, + expires_at=datetime.now(timezone.utc) - timedelta(days=1), # expired yesterday + ) + tok_session.add(db_token) + tok_session.commit() + + mock_request = MagicMock() + mock_request.headers = {"authorization": f"Bearer {plaintext}"} + mock_request.client.host = "127.0.0.1" + + user = _resolve_bearer_user(mock_request, tok_session) + assert user is None + + @pytest.mark.unit + def test_non_expired_token_resolves(self, tok_engine, tok_session): + """A token before its expires_at should authenticate normally.""" + from datetime import datetime, timedelta, timezone + from unittest.mock import MagicMock + + from app.api.api_tokens import generate_api_token, hash_token + from app.auth import _resolve_bearer_user + + plaintext = generate_api_token() + token_hash = hash_token(plaintext) + + db_token = ApiToken( + owner_id=_OWNER, + name="Valid Token", + token_hash=token_hash, + token_prefix=plaintext[:12], + is_active=True, + expires_at=datetime.now(timezone.utc) + timedelta(days=30), # expires in 30 days + ) + tok_session.add(db_token) + tok_session.commit() + + mock_request = MagicMock() + mock_request.headers = {"authorization": f"Bearer {plaintext}"} + mock_request.client.host = "127.0.0.1" + + user = _resolve_bearer_user(mock_request, tok_session) + assert user is not None + assert user["preferred_username"] == _OWNER + + @pytest.mark.unit + def test_create_token_expires_in_days_zero_rejected(self, tok_engine): + """expires_in_days=0 should be rejected with 422 (ge=1).""" + from app.main import app + + client = _make_client(tok_engine) + try: + resp = client.post("/api/api-tokens/", json={"name": "Bad Expiry", "expires_in_days": 0}) + assert resp.status_code == 422 + finally: + _cleanup(app) + + @pytest.mark.unit + def test_expires_at_included_in_list_response(self, tok_engine): + """List endpoint should include expires_at field.""" + from app.main import app + + client = _make_client(tok_engine) + try: + client.post("/api/api-tokens/", json={"name": "Listed", "expires_in_days": 7}) + resp = client.get("/api/api-tokens/") + assert resp.status_code == 200 + tokens = resp.json() + assert len(tokens) == 1 + assert "expires_at" in tokens[0] + assert tokens[0]["expires_at"] is not None + finally: + _cleanup(app)