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 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/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"]) ) 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