From 211cb19fef2e7044c03199c97847e601d9790640 Mon Sep 17 00:00:00 2001 From: Marius Hein Date: Tue, 19 May 2026 11:21:37 +0200 Subject: [PATCH 1/3] fix(scim): make group member add idempotent Authentik's outgoing SCIM sync occasionally PATCHes a group with an `add members` operation listing a user that is already a member, triggering a `user_groups_pkey` IntegrityError that bubbled up as a SCIM 409 and aborted the whole sync. Switch the membership insert to `ON CONFLICT (user_id, group_id) DO NOTHING` and dedupe the input set so re-adding existing members is a no-op. --- src/struudel/services/group.py | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/src/struudel/services/group.py b/src/struudel/services/group.py index f7a170f..3102842 100644 --- a/src/struudel/services/group.py +++ b/src/struudel/services/group.py @@ -2,6 +2,7 @@ from typing import TYPE_CHECKING, Any, NamedTuple import sqlalchemy as sa +from sqlalchemy.dialects.postgresql import insert as pg_insert from sqlalchemy.orm import Session, selectinload from struudel.models.associations import user_group @@ -214,24 +215,16 @@ def _replace_members(db: Session, *, group_id: int, user_ids: list[int]) -> None def _add_members(db: Session, *, group_id: int, user_ids: list[int]) -> None: requested = set(user_ids) - existing = set( - db.scalars( - sa.select(user_group.c.user_id).where( - user_group.c.group_id == group_id, - user_group.c.user_id.in_(requested), - ) - ).all() - ) valid = set(db.scalars(sa.select(User.id).where(User.id.in_(requested))).all()) invalid = requested - valid if invalid: log.info("SCIM add_members skipped unknown ids group=%s ids=%s", group_id, sorted(invalid)) - to_add = [uid for uid in user_ids if uid in valid and uid not in existing] - if to_add: + if valid: db.execute( - sa.insert(user_group), - [{"group_id": group_id, "user_id": uid} for uid in to_add], + pg_insert(user_group) + .values([{"group_id": group_id, "user_id": uid} for uid in sorted(valid)]) + .on_conflict_do_nothing(index_elements=["user_id", "group_id"]) ) From c16f06930e4882d51d315af35a7c86559a4eb08b Mon Sep 17 00:00:00 2001 From: Marius Hein Date: Tue, 19 May 2026 11:21:37 +0200 Subject: [PATCH 2/3] ci: build container image for pull requests Adds a `pull_request` trigger that pushes the resulting image to GHCR tagged `pr-`, while `latest` stays restricted to the default branch. Lets reviewers pull and validate a PR's container before merge. --- .github/workflows/build-image.yml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/.github/workflows/build-image.yml b/.github/workflows/build-image.yml index 34ad167..99cd3f4 100644 --- a/.github/workflows/build-image.yml +++ b/.github/workflows/build-image.yml @@ -3,6 +3,8 @@ name: Build container image on: push: branches: [main] + pull_request: + branches: [main] permissions: contents: read @@ -33,7 +35,8 @@ jobs: with: images: ghcr.io/${{ github.repository }} tags: | - type=raw,value=latest + type=raw,value=latest,enable={{is_default_branch}} + type=ref,event=pr,prefix=pr- - name: Build and push uses: docker/build-push-action@v6 From 2ed63e563514dd2399d14f9e8441cb9aabe4aad6 Mon Sep 17 00:00:00 2001 From: Marius Hein Date: Tue, 19 May 2026 11:29:11 +0200 Subject: [PATCH 3/3] fix(scim): accept members remove without value-eq filter Authentik's outgoing SCIM PATCHes a group's members with `{op:"remove", path:"members", value:[{value:""}, ...]}` rather than the RFC 7644 filter syntax `members[value eq ""]`, which we rejected with 400. Accept the value-list form too, and treat a remove on `members` with no value as "clear all members". Also log the request body on any SCIM 400 so future parser gaps are diagnosable from the app log. --- src/struudel/blueprints/scim/routes.py | 11 +++++++++++ src/struudel/services/scim.py | 23 ++++++++++++++++------- 2 files changed, 27 insertions(+), 7 deletions(-) diff --git a/src/struudel/blueprints/scim/routes.py b/src/struudel/blueprints/scim/routes.py index f0f0ccd..d6d2dd8 100644 --- a/src/struudel/blueprints/scim/routes.py +++ b/src/struudel/blueprints/scim/routes.py @@ -1,3 +1,4 @@ +import logging from typing import Any from flask import request @@ -13,9 +14,19 @@ from struudel.services import user as user_service from struudel.services.scim import SCIM_CONTENT_TYPE, ScimError +log = logging.getLogger(__name__) + @bp.errorhandler(ScimError) def _handle_scim_error(e: ScimError) -> Response: + if e.status == 400: + log.warning( + "SCIM 400 %s %s detail=%r body=%r", + request.method, + request.path, + e.detail, + request.get_data(as_text=True)[:2000], + ) return scim_error(e.status, e.detail, e.scim_type) diff --git a/src/struudel/services/scim.py b/src/struudel/services/scim.py index f4b6696..9150317 100644 --- a/src/struudel/services/scim.py +++ b/src/struudel/services/scim.py @@ -253,17 +253,26 @@ def group_patch_to_actions(ops: list[dict[str, Any]]) -> dict[str, Any]: actions["add_members"].extend(_parse_member_ids(value or [])) elif op_name == "remove": - match = _REMOVE_MEMBER_PATH.match(path or "") - if not match: + path_str = path or "" + match = _REMOVE_MEMBER_PATH.match(path_str) + if match: + try: + actions["remove_members"].append(int(match.group(1))) + except ValueError as e: + raise ScimError(400, "member value must be numeric id", "invalidValue") from e + elif path_str.lower() == "members": + if value in (None, []): + actions["replace_members"] = [] + elif isinstance(value, list): + actions["remove_members"].extend(_parse_member_ids(value)) + else: + raise ScimError(400, "remove members value must be list", "invalidValue") + else: raise ScimError( 400, - 'remove only supported as members[value eq ""]', + 'remove only supported on members (with or without value-eq filter)', "invalidSyntax", ) - try: - actions["remove_members"].append(int(match.group(1))) - except ValueError as e: - raise ScimError(400, "member value must be numeric id", "invalidValue") from e return actions