From 712aa96dcc9bb9fe89bdc26f00496a8a163752ee Mon Sep 17 00:00:00 2001 From: Seva D Date: Wed, 8 Jul 2026 15:15:46 +0400 Subject: [PATCH 1/2] Fix 10 issues found in high-effort review of main Backend: - SQLAlchemy: coerce relationship (m2m) filter values to int for integer pks (500 on PostgreSQL/asyncpg) and reject unsupported relation lookups with 422 instead of silently collapsing them to pk equality - PonyORM: only rewrite _ filter keys when the base attribute is a real relation, so plain columns ending in _id filter correctly - PonyORM: run list search as a single OR'd WHERE clause instead of materializing every matching row once per search field - Flask: pass through HTTPExceptions with an attached response, preserve error headers on JSON error responses, log unhandled errors with traceback - Filtering: text columns support IS NULL again via __exact=null while substring lookups keep the literal string Frontend: - only coerce "true"/"false" strings to booleans for boolean widgets, so text content that spells a boolean survives the change form - treat fields without a change widget as known non-date (null) instead of falling back to shape-based date detection - transformDataFromServer takes the model configuration directly, removing the duplicated getChangeWidgetTypes incantation at every call site --- CHANGELOG.md | 25 ++++++++ fastadmin/api/frameworks/flask/app.py | 16 +++-- fastadmin/api/helpers.py | 14 ++++- fastadmin/api/service.py | 7 ++- fastadmin/models/orms/ponyorm.py | 33 +++++----- fastadmin/models/orms/sqlalchemy.py | 10 ++- .../src/components/async-select/index.tsx | 10 +-- .../src/components/inline-widget/index.tsx | 6 +- frontend/src/containers/change/index.tsx | 6 +- frontend/src/helpers/transform.test.tsx | 56 ++++++++++++++++- frontend/src/helpers/transform.tsx | 63 ++++++++++++++----- tests/api/test_helpers.py | 21 ++++--- 12 files changed, 198 insertions(+), 69 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 51a5e2c..c26a777 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,31 @@ All notable changes to FastAdmin are documented in this file. +## Unreleased + +Bug-fix follow-up to the 0.8.1 audit. No public API changes. + +- **SQLAlchemy**: relationship (m2m) filter values are coerced to `int` for + integer primary keys (previously raised HTTP 500 on strictly typed drivers + such as PostgreSQL/asyncpg), and unsupported lookups on relation fields + (e.g. `icontains`) are rejected with HTTP 422 instead of silently matching + by primary-key equality. +- **Pony ORM**: the foreign-key filter rewrite no longer corrupts filters on + plain columns whose name merely ends with `_id`, and list-view search now + runs as a single OR'd WHERE clause instead of materializing every matching + row once per search field. +- **Flask**: `HTTPException`s with an attached response (`abort(Response(...))`) + are passed through unchanged, error headers (`Allow`, `WWW-Authenticate`, + `Retry-After`, ...) are preserved on JSON error responses, and unhandled + errors are logged with their traceback. +- **Filtering**: `IS NULL` is expressible again on text columns via + `field__exact=null` (substring lookups keep the literal string `"null"`). +- **Frontend**: stop corrupting text fields whose content is the literal word + `"true"`/`"false"` on the change form (booleans are now only coerced for + boolean widgets), stop shape-based date detection for fields known to have + no date widget, and pass the model configuration directly to + `transformDataFromServer` so every call site gets the safe behavior. + ## 0.8.1 Bug-fix release from a full-source audit of `main`. No public API changes. diff --git a/fastadmin/api/frameworks/flask/app.py b/fastadmin/api/frameworks/flask/app.py index 0a0aca3..b30ba6c 100644 --- a/fastadmin/api/frameworks/flask/app.py +++ b/fastadmin/api/frameworks/flask/app.py @@ -33,11 +33,19 @@ def default(self, o): @app.errorhandler(Exception) def exception_handler(exc): if isinstance(exc, HTTPException): + if exc.response is not None: + # abort(Response(...)) attached a fully-formed response; pass it + # through untouched instead of replacing it with a generic error. + return exc # Return API errors as JSON {"detail": ...} with the proper HTTP status # so the shared React frontend can read the message (matching the Django # and FastAPI integrations), instead of werkzeug's default HTML page. - return {"detail": exc.description}, exc.code or 500 - # Unhandled server error: log it server-side but never leak internals to the - # client, and return a real HTTP 500 (a bare dict would be sent as HTTP 200). - logger.error("Unhandled admin error: %s", exc) + # Keep the exception's headers (Allow, WWW-Authenticate, Retry-After, ...) + # apart from its text/html Content-Type, which the JSON body replaces. + headers = [(k, v) for k, v in exc.get_headers() if k.lower() != "content-type"] + return {"detail": exc.description}, exc.code or 500, headers + # Unhandled server error: log it (with traceback — this is the only place it + # is captured) but never leak internals to the client, and return a real + # HTTP 500 (a bare dict would be sent as HTTP 200). + logger.error("Unhandled admin error: %s", exc, exc_info=exc) return {"detail": "Internal server error."}, 500 diff --git a/fastadmin/api/helpers.py b/fastadmin/api/helpers.py index b63ee29..8c3c1b4 100644 --- a/fastadmin/api/helpers.py +++ b/fastadmin/api/helpers.py @@ -24,17 +24,26 @@ def sanitize_filter_value( value: str | list, field: ModelFieldWidgetSchema | None = None, + condition: str = "exact", ) -> bool | None | str | list: """Sanitize value (string or list for __in filters). :params value: a value (str or list of str for __in). :params field: the field being filtered, used to decide whether the "true"/"false"/"null" literals should be coerced (skipped for text fields). + :params condition: the filter lookup (exact/in/icontains/...), used to keep + IS NULL expressible on text fields. :return: A sanitized value. """ if isinstance(value, list): - return [sanitize_filter_value(v, field) for v in value] + return [sanitize_filter_value(v, field, condition) for v in value] if field is not None and field.filter_widget_type in TEXT_FILTER_WIDGET_TYPES: + # Text content may legitimately be the words "true"/"false"/"null", so + # they are kept literal — except "null" on an exact lookup, which stays + # the only way to express IS NULL on a text column (match the literal + # word via contains/icontains instead). + if value == "null" and condition == "exact": + return None return value match value: case "false": @@ -109,7 +118,8 @@ def build_query_filters( continue field_name = key.partition("__")[0] field = next((f for f in fields if f.name == field_name), None) - result[sanitize_filter_key(key, fields)] = sanitize_filter_value(value, field) + sanitized_key = sanitize_filter_key(key, fields) + result[sanitized_key] = sanitize_filter_value(value, field, sanitized_key[1]) return result diff --git a/fastadmin/api/service.py b/fastadmin/api/service.py index f636e7d..6a3108f 100644 --- a/fastadmin/api/service.py +++ b/fastadmin/api/service.py @@ -103,10 +103,13 @@ def _validate_filters(admin_model: Any, filters: dict, exclude_filter_fields: tu The filterable field set is ``list_filter`` when the admin defines it, otherwise the serialized field set; the lookup suffix must be one of - ALLOWED_FILTER_CONDITIONS. + ALLOWED_FILTER_CONDITIONS. Relation (m2m) fields only support pk + equality/membership, so other lookups are rejected here instead of + failing (or silently degrading) inside the ORM adapters. """ list_filter = getattr(admin_model, "list_filter", ()) allowlist = set(list_filter) if list_filter else fields + m2m_fields = {f.name for f in admin_model.get_model_fields_with_widget_types() if f.is_m2m} for k in filters: if k in exclude_filter_fields: continue @@ -115,6 +118,8 @@ def _validate_filters(admin_model: Any, filters: dict, exclude_filter_fields: tu raise AdminApiException(422, detail=f"Filter by {k} is not allowed") if field not in allowlist: raise AdminApiException(422, detail=f"Filter by {k} is not allowed") + if field in m2m_fields and condition and condition not in ("exact", "in"): + raise AdminApiException(422, detail=f"Filter by {k} is not allowed") @staticmethod def _bind_admin_context( diff --git a/fastadmin/models/orms/ponyorm.py b/fastadmin/models/orms/ponyorm.py index f035a66..20f34b5 100644 --- a/fastadmin/models/orms/ponyorm.py +++ b/fastadmin/models/orms/ponyorm.py @@ -259,8 +259,16 @@ def orm_get_list( field = field_with_condition[0] condition = field_with_condition[1] model_pk_name = self.get_model_pk_name(self.model_cls) - if field.endswith(f"_{model_pk_name}"): - field = field.replace(f"_{model_pk_name}", f".{model_pk_name}") + # A "_" key (added by sanitize_filter_key for fk + # fields) is rewritten to the "." path — but only + # when the base attribute really is a relation, so a plain + # column that merely ends with "_id" is left untouched. + pk_suffix = f"_{model_pk_name}" + if field.endswith(pk_suffix): + base_name = field[: -len(pk_suffix)] + base_attr = getattr(self.model_cls, base_name, None) if base_name else None + if base_attr is not None and getattr(base_attr, "is_relation", False): + field = f"{base_name}.{model_pk_name}" # Bind the user-supplied value as a local (`fval`) so Pony # resolves it as a query parameter. Only the field path — built # from the admin's validated/allowlisted field names — is @@ -272,21 +280,16 @@ def orm_get_list( search_fields = list(self.search_fields) if search and search_fields: - model_pk_name = self.get_model_pk_name(self.model_cls) - ids = [] # Bind the user-supplied search term as a local so Pony resolves it - # as a parameter. Only the field path (from the admin's trusted - # search_fields config) is interpolated — never the search value. + # as a parameter. Only the field paths (from the admin's trusted + # search_fields config) are interpolated — never the search value. + # All fields are OR-ed into a single filter so the search runs as + # one WHERE clause instead of materializing matching rows per field. search_term = search.lower() # noqa: F841 (referenced by Pony filter string) - for search_field in search_fields: - pony_search_field = search_field.replace("__", ".") - qs_ids = qs.filter(f"search_term in m.{pony_search_field}.lower()") - objs = list(qs_ids) - ids += [getattr(o, model_pk_name) for o in objs] - # Bind the collected pks as a local so Pony resolves it as a - # parameter; only the trusted pk field path is interpolated. - pk_ids = set(ids) # noqa: F841 (referenced by Pony filter string) - qs = qs.filter(f"m.{model_pk_name} in pk_ids") + search_expr = " or ".join( + f"search_term in m.{search_field.replace('__', '.')}.lower()" for search_field in search_fields + ) + qs = qs.filter(search_expr) ordering = [sort_by] if sort_by else self.ordering if ordering: diff --git a/fastadmin/models/orms/sqlalchemy.py b/fastadmin/models/orms/sqlalchemy.py index 2afc2e6..240f07f 100644 --- a/fastadmin/models/orms/sqlalchemy.py +++ b/fastadmin/models/orms/sqlalchemy.py @@ -340,9 +340,17 @@ def order_column(ordering_field: str): if related_mapper is not None: # Relationship field (e.g. m2m): SQLAlchemy rejects a # scalar comparison against a collection, so match on the - # related row's pk via any()/has() instead. + # related row's pk via any()/has() instead. Only pk + # equality/membership is meaningful here — anything else + # must fail loudly rather than degrade to an equality + # match (the service layer rejects these with a 422). + if condition not in ("exact", "in"): + raise ValueError(f"Filter condition {condition!r} is not supported for relation {field!r}") related_cls = related_mapper.class_ related_pk = getattr(related_cls, self.get_model_pk_name(related_cls)) + if isinstance(related_pk.expression.type, BIGINT | Integer): + with contextlib.suppress(ValueError, TypeError): + value = [int(x) for x in value] if condition == "in" else int(value) match_expr = related_pk.in_(value) if condition == "in" else related_pk == value if getattr(rel_property, "uselist", True): q.append(model_field.any(match_expr)) diff --git a/frontend/src/components/async-select/index.tsx b/frontend/src/components/async-select/index.tsx index bcf599d..fac46a4 100644 --- a/frontend/src/components/async-select/index.tsx +++ b/frontend/src/components/async-select/index.tsx @@ -28,10 +28,7 @@ import { getFetcher, patchFetcher, postFetcher } from "@/fetchers/fetchers"; import { getConfigurationModel } from "@/helpers/configuration"; import { handleError } from "@/helpers/forms"; import { getTitleFromModel } from "@/helpers/title"; -import { - getChangeWidgetTypes, - transformDataFromServer, -} from "@/helpers/transform"; +import { transformDataFromServer } from "@/helpers/transform"; import { EModelPermission } from "@/interfaces/configuration"; import { ConfigurationContext } from "@/providers/ConfigurationProvider"; @@ -90,10 +87,7 @@ export const AsyncSelect: React.FC = ({ const asyncSelectChangeInitialValues = useMemo( () => initialChangeValues != null - ? transformDataFromServer( - initialChangeValues, - getChangeWidgetTypes(modelConfiguration), - ) + ? transformDataFromServer(initialChangeValues, modelConfiguration) : undefined, [initialChangeValues, modelConfiguration], ); diff --git a/frontend/src/components/inline-widget/index.tsx b/frontend/src/components/inline-widget/index.tsx index 55a1c7f..cbd80f6 100644 --- a/frontend/src/components/inline-widget/index.tsx +++ b/frontend/src/components/inline-widget/index.tsx @@ -30,7 +30,6 @@ import { import { handleError } from "@/helpers/forms"; import { getTitleFromModel } from "@/helpers/title"; import { - getChangeWidgetTypes, transformDataFromServer, transformDataToServer, transformFiltersToServer, @@ -152,10 +151,7 @@ export const InlineWidget: React.FC = ({ const inlineChangeInitialValues = useMemo( () => initialChangeValues != null - ? transformDataFromServer( - initialChangeValues, - getChangeWidgetTypes(modelConfiguration), - ) + ? transformDataFromServer(initialChangeValues, modelConfiguration) : undefined, [initialChangeValues, modelConfiguration], ); diff --git a/frontend/src/containers/change/index.tsx b/frontend/src/containers/change/index.tsx index 86c2c8c..d1d9597 100644 --- a/frontend/src/containers/change/index.tsx +++ b/frontend/src/containers/change/index.tsx @@ -28,7 +28,6 @@ import { getConfigurationModel } from "@/helpers/configuration"; import { handleError } from "@/helpers/forms"; import { getTitleFromModel } from "@/helpers/title"; import { - getChangeWidgetTypes, transformDataFromServer, transformDataToServer, } from "@/helpers/transform"; @@ -62,10 +61,7 @@ export const Change: React.FC = () => { const initialValues = useMemo( () => initialChangeValues != null - ? transformDataFromServer( - initialChangeValues, - getChangeWidgetTypes(modelConfiguration), - ) + ? transformDataFromServer(initialChangeValues, modelConfiguration) : undefined, [initialChangeValues, modelConfiguration], ); diff --git a/frontend/src/helpers/transform.test.tsx b/frontend/src/helpers/transform.test.tsx index 2b20fdb..d0fe451 100644 --- a/frontend/src/helpers/transform.test.tsx +++ b/frontend/src/helpers/transform.test.tsx @@ -232,6 +232,30 @@ describe("transform", () => { "12:00:00", ); }); + it("keeps a boolean-looking string as-is for a non-boolean widget", () => { + expect(transformValueFromServer("false", EFieldWidgetType.Input)).toBe( + "false", + ); + expect(transformValueFromServer("True", EFieldWidgetType.TextArea)).toBe( + "True", + ); + }); + it("coerces boolean strings for boolean widgets", () => { + expect(transformValueFromServer("false", EFieldWidgetType.Switch)).toBe( + false, + ); + expect(transformValueFromServer("true", EFieldWidgetType.Checkbox)).toBe( + true, + ); + }); + it("keeps real booleans regardless of widget type", () => { + expect(transformValueFromServer(true, EFieldWidgetType.Input)).toBe(true); + expect(transformValueFromServer(false, null)).toBe(false); + }); + it("skips shape detection when the widget is known to be absent", () => { + expect(transformValueFromServer("2024-01-15", null)).toBe("2024-01-15"); + expect(transformValueFromServer("false", null)).toBe("false"); + }); }); describe("transformDataFromServer", () => { @@ -239,14 +263,40 @@ describe("transform", () => { const r = transformDataFromServer({ d: "2024-01-15" }); expect(r).toHaveProperty("d"); }); - it("respects per-field widget types", () => { + it("respects per-field widget types from the model configuration", () => { const r = transformDataFromServer( { at: "2024-01-15", code: "2024-01-15" }, - { at: EFieldWidgetType.DatePicker, code: EFieldWidgetType.Input }, + { + fields: [ + { + name: "at", + change_configuration: { + form_widget_type: EFieldWidgetType.DatePicker, + }, + }, + { + name: "code", + change_configuration: { + form_widget_type: EFieldWidgetType.Input, + }, + }, + ], + } as any, ); expect(dayjs.isDayjs(r.at)).toBe(true); expect(r.code).toBe("2024-01-15"); }); + it("does not shape-detect fields without a change widget when a configuration is given", () => { + const r = transformDataFromServer( + { code: "2024-01-15", flag: "false", unknown: "2024-01-15" }, + { + fields: [{ name: "code", change_configuration: {} }], + } as any, + ); + expect(r.code).toBe("2024-01-15"); + expect(r.flag).toBe("false"); + expect(r.unknown).toBe("2024-01-15"); + }); }); describe("getChangeWidgetTypes", () => { @@ -262,7 +312,7 @@ describe("transform", () => { { name: "code", change_configuration: {} }, ], } as any); - expect(map).toEqual({ at: EFieldWidgetType.DatePicker, code: undefined }); + expect(map).toEqual({ at: EFieldWidgetType.DatePicker, code: null }); }); it("returns an empty map when configuration is missing", () => { expect(getChangeWidgetTypes()).toEqual({}); diff --git a/frontend/src/helpers/transform.tsx b/frontend/src/helpers/transform.tsx index e43c05f..2c45e07 100644 --- a/frontend/src/helpers/transform.tsx +++ b/frontend/src/helpers/transform.tsx @@ -1,10 +1,7 @@ import { Checkbox, Tag } from "antd"; import dayjs from "dayjs"; import slugify from "slugify"; -import { - EFieldWidgetType, - type IModelField, -} from "@/interfaces/configuration"; +import { EFieldWidgetType, type IModelField } from "@/interfaces/configuration"; // Widgets that expect a dayjs value. A server string is only parsed into a // dayjs when its field uses one of these; otherwise a plain text field whose @@ -16,6 +13,14 @@ const DATE_WIDGET_TYPES: EFieldWidgetType[] = [ EFieldWidgetType.RangePicker, ]; +// Widgets that expect a boolean value. The literal strings "true"/"false" are +// only coerced to booleans for these; otherwise a text field whose content is +// the word "false" would be corrupted. +const BOOLEAN_WIDGET_TYPES: EFieldWidgetType[] = [ + EFieldWidgetType.Switch, + EFieldWidgetType.Checkbox, +]; + export const isTime = (v: string): boolean => { // Accept common backend time shapes, including: // HH:mm:ss(.ffffff), HH:mm(:ss) @@ -136,9 +141,13 @@ export const transformFiltersToServer = (data: any) => { return filtersData; }; +// widgetType semantics: an EFieldWidgetType enables the matching coercions, +// `null` means "the widget is known to not be a date/boolean one" (no shape +// coercion at all), and `undefined` means "no widget information available" +// (legacy shape-based detection for backward compatibility). export const transformValueFromServer = ( value: any, - widgetType?: EFieldWidgetType, + widgetType?: EFieldWidgetType | null, ): any => { if (value === null || value === undefined) { return value; @@ -146,13 +155,22 @@ export const transformValueFromServer = ( if (isArray(value)) { return value.map((v: any) => transformValueFromServer(v, widgetType)); } - if (isBoolean(value)) { - return value !== "false" && !!value; + if (typeof value === "boolean") { + return value; } - // Parse date/time strings into dayjs only for date widgets. When the widget - // type is unknown, fall back to shape detection for backward compatibility. + const parseByShape = widgetType === undefined; + // Coerce the literal strings "true"/"false" only for boolean widgets; a text + // field whose content is the word "false" must stay a string. + const parseBooleans = + parseByShape || + (widgetType != null && BOOLEAN_WIDGET_TYPES.includes(widgetType)); + if (parseBooleans && isString(value) && isBoolean(value)) { + return value.toLowerCase() !== "false"; + } + // Parse date/time strings into dayjs only for date widgets. const parseDates = - widgetType === undefined || DATE_WIDGET_TYPES.includes(widgetType); + parseByShape || + (widgetType != null && DATE_WIDGET_TYPES.includes(widgetType)); if (parseDates) { if (isDate(value)) { return dayjs(value); @@ -170,24 +188,35 @@ export const transformValueFromServer = ( // Map each field name to the widget type used on the change form, so // transformDataFromServer can decide, per field, whether a date-looking string // should become a dayjs (real date widget) or stay a string (e.g. a Char field). -export const getChangeWidgetTypes = ( - modelConfiguration?: { fields?: IModelField[] }, -): Record => { - const widgetTypes: Record = {}; +// A field without a change widget maps to `null` (known non-date/non-boolean), +// not `undefined`, so it never falls back to shape-based detection. +export const getChangeWidgetTypes = (modelConfiguration?: { + fields?: IModelField[]; +}): Record => { + const widgetTypes: Record = {}; for (const field of modelConfiguration?.fields || []) { - widgetTypes[field.name] = field.change_configuration?.form_widget_type; + widgetTypes[field.name] = + field.change_configuration?.form_widget_type ?? null; } return widgetTypes; }; export const transformDataFromServer = ( data: Record, - widgetTypes?: Record, + modelConfiguration?: { fields?: IModelField[] }, ) => { + const widgetTypes = modelConfiguration + ? getChangeWidgetTypes(modelConfiguration) + : undefined; return Object.fromEntries( Object.entries(data).map(([k, v]) => [ k, - transformValueFromServer(v, widgetTypes?.[k]), + // With a configuration present, an unknown field is `null` (skip shape + // detection); without one, `undefined` keeps the legacy fallback. + transformValueFromServer( + v, + widgetTypes ? (widgetTypes[k] ?? null) : undefined, + ), ]), ); }; diff --git a/tests/api/test_helpers.py b/tests/api/test_helpers.py index e1bc096..1de0024 100644 --- a/tests/api/test_helpers.py +++ b/tests/api/test_helpers.py @@ -102,12 +102,16 @@ async def test_sanitize_filter_key(): async def test_sanitize_filter_value_type_aware(): - # Text fields keep the literal "true"/"false"/"null" strings. + # Text fields keep the literal "true"/"false"/"null" strings for substring + # lookups; only "null" on an exact lookup stays coercible so IS NULL + # remains expressible on text columns. text_field = _make_field("title") # WidgetType.Input - assert sanitize_filter_value("null", text_field) == "null" - assert sanitize_filter_value("true", text_field) == "true" - assert sanitize_filter_value("false", text_field) == "false" - assert sanitize_filter_value(["null", "true"], text_field) == ["null", "true"] + assert sanitize_filter_value("null", text_field, "icontains") == "null" + assert sanitize_filter_value("null", text_field, "contains") == "null" + assert sanitize_filter_value("null", text_field, "exact") is None + assert sanitize_filter_value("true", text_field, "exact") == "true" + assert sanitize_filter_value("false", text_field, "exact") == "false" + assert sanitize_filter_value(["null", "true"], text_field, "in") == ["null", "true"] # Non-text fields still coerce. bool_field = ModelFieldWidgetSchema( @@ -130,10 +134,11 @@ async def test_build_query_filters(): _make_field("title"), _make_field("tournament", filter_widget_props={"parentModel": "Tournament"}), ] - filters = {"title__exact": "null", "tournament": "5", "search": "x"} + filters = {"title__exact": "null", "title__icontains": "null", "tournament": "5", "search": "x"} result = build_query_filters(filters, fields, exclude=("search",)) - # Text field: the literal "null" is preserved, not coerced to None. - assert result[("title", "exact")] == "null" + # Text field: exact "null" expresses IS NULL, substring lookups keep the literal. + assert result[("title", "exact")] is None + assert result[("title", "icontains")] == "null" # parentModel field gets the _id suffix. assert result[("tournament_id", "exact")] == "5" # Excluded keys are dropped. From 11a4950bb49f74ced0eb4c15016f2d85b648f152 Mon Sep 17 00:00:00 2001 From: Seva D Date: Wed, 8 Jul 2026 15:27:58 +0400 Subject: [PATCH 2/2] Add coverage for relation-filter rejection and Flask error passthrough Covers the three new defensive branches: the service-layer 422 for substring lookups on m2m filter fields, the SQLAlchemy adapter's loud failure for unsupported relation conditions, and the Flask handler's passthrough of HTTPExceptions with an attached response (plus header preservation on JSON error responses). --- tests/api/frameworks/flask/test_app.py | 21 +++++++++++++++++++-- tests/api/test_service.py | 19 +++++++++++++++++++ tests/models/test_orm.py | 6 ++++++ 3 files changed, 44 insertions(+), 2 deletions(-) diff --git a/tests/api/frameworks/flask/test_app.py b/tests/api/frameworks/flask/test_app.py index fa53601..8f9c139 100644 --- a/tests/api/frameworks/flask/test_app.py +++ b/tests/api/frameworks/flask/test_app.py @@ -2,8 +2,8 @@ from uuid import uuid4 import pytest -from flask import Flask -from werkzeug.exceptions import HTTPException +from flask import Flask, Response +from werkzeug.exceptions import HTTPException, MethodNotAllowed from fastadmin.api.frameworks.flask.api import ( change, @@ -19,6 +19,23 @@ async def test_exception_handler(): assert exception_handler(HTTPException()) is not None +async def test_exception_handler_passes_through_attached_response(): + """abort(Response(...)) must reach the client untouched, not become a generic error.""" + exc = HTTPException(response=Response("forbidden", status=403)) + assert exception_handler(exc) is exc + + +async def test_exception_handler_preserves_error_headers(): + """Headers like Allow/WWW-Authenticate survive the JSON conversion; the + exception's text/html Content-Type does not.""" + body, code, headers = exception_handler(MethodNotAllowed(valid_methods=["GET"])) + assert code == 405 + header_names = {k.lower() for k, _ in headers} + assert "allow" in header_names + assert "content-type" not in header_names + assert body == {"detail": MethodNotAllowed.description} + + async def test_json_provider(): today = datetime.now(UTC).date() now = datetime.now(UTC) diff --git a/tests/api/test_service.py b/tests/api/test_service.py index 324b146..e75dea6 100644 --- a/tests/api/test_service.py +++ b/tests/api/test_service.py @@ -638,6 +638,25 @@ async def test_list_enforces_list_filter_allowlist(monkeypatch): assert exc_info.value.status_code == 422 +async def test_list_rejects_substring_lookup_on_relation_filter(monkeypatch): + monkeypatch.setattr("fastadmin.api.service.get_user_id_from_session_id", AsyncMock(return_value=1)) + m2m_field = SimpleNamespace(name="participants", is_m2m=True, filter_widget_type=None, filter_widget_props={}) + admin_model = _list_admin_model( + get_fields_for_serialize=lambda: ["participants"], + get_model_fields_with_widget_types=lambda **_: [m2m_field], + ) + monkeypatch.setattr("fastadmin.api.service.get_admin_or_admin_inline_model", lambda _model: admin_model) + + # Relation fields only support pk equality/membership; anything else must + # be rejected instead of silently degrading to an equality match. + with pytest.raises(AdminApiException) as exc_info: + await ApiService().list("sid", "Event", filters={"participants__icontains": "ann"}) + assert exc_info.value.status_code == 422 + + await ApiService().list("sid", "Event", filters={"participants__exact": "1"}) + await ApiService().list("sid", "Event", filters={"participants__in": "1,2"}) + + async def test_add_denied_without_permission(monkeypatch): monkeypatch.setattr("fastadmin.api.service.get_user_id_from_session_id", AsyncMock(return_value=1)) admin_model = SimpleNamespace( diff --git a/tests/models/test_orm.py b/tests/models/test_orm.py index 5e97989..1ec6061 100644 --- a/tests/models/test_orm.py +++ b/tests/models/test_orm.py @@ -1,3 +1,5 @@ +import pytest + from fastadmin.models.helpers import get_admin_model from fastadmin.models.schemas import ModelFieldWidgetSchema, WidgetType @@ -461,6 +463,10 @@ async def test_sqlalchemy_orm_get_list_relation_filters(event, session_with_type assert isinstance(objs, list) assert any(getattr(obj, "id", None) == event.id for obj in objs) + # Unsupported lookups must fail loudly instead of degrading to pk equality. + with pytest.raises(ValueError, match="not supported for relation"): + await admin_model.orm_get_list(filters={("participants", "icontains"): "ann"}) + async def test_sqlalchemy_orm_get_list_ordering_non_column_is_skipped(event, session_with_type): _, session_type = session_with_type