OpenSlides / openslides-autoupdate-service

Autoupdate service for OpenSlides 4+
MIT License
2 stars 20 forks source link

Fix autoupdate subscription for committee admins #973

Open Elblinator opened 1 month ago

Elblinator commented 1 month ago

Current behaviour: It is possible for committee admins to get information about meetings they are no part of, if their accounts are part of different meetings

Screenshot: Screenshot_20240726_122706

Reproduction:

  1. Create two committees
  2. create one meeting to both of these meetings
  3. Add a User (Ann) to both meetings
  4. Appoint a different user as the committee admin for the first committee
  5. Login as the committee admin and navigate to Ann
  6. In the table-headers you see "Meeting (2)" even though you are only allowed to see one meeting (the one where you are the committee admin

The following information is the info I got from my local instance I started it and unsed the automatically called initial data I additionally created two committees, 26 meetings, 4 users (1 OrgaAdmin and 1 committeeadmin) The information from the subscription: committee_manager_account_list:subscription

{
    "committee": {
        "1": {
            "id": 1,
            "manager_ids": [
                1,
                3
            ],
            "meeting_ids": [
                1
            ],
            "name": "Brand new committee with extra committee admin",
            "user_ids": [
                1,
                3,
                4
            ]
        }
    },
    "group": {
        "1": {
            "id": 1,
            "name": "Default"
        },
        "2": {
            "id": 2,
            "name": "Admin"
        },
        "3": {
            "id": 3,
            "name": "Delegates"
        },
        "4": {
            "id": 4,
            "name": "Staff"
        }
    },
    "meeting": {
        "1": {
            "committee_id": 1,
            "default_group_id": 1,
            "group_ids": [
                1,
                2,
                3,
                4
            ],
            "id": 1,
            "is_active_in_organization_id": 1,
            "name": "Meeting from brand new committee with extra committee admin",
            "projector_countdown_default_time": 60
        },
        "2": {
            "id": 2,
            "name": "Meeting with no extra committee admin"
        }
    },
    "meeting_user": {
        "1": {
            "group_ids": [
                2
            ],
            "id": 1,
            "meeting_id": 1,
            "user_id": 1
        },
        "2": {
            "group_ids": [],
            "id": 2,
            "user_id": 1
        },
        "3": {
            "group_ids": [
                1
            ],
            "id": 3,
            "meeting_id": 1,
            "user_id": 4
        },
        "4": {
            "group_ids": [],
            "id": 4,
            "user_id": 4
        }
    },
    "user": {
        "1": {
            "committee_ids": [
                1
            ],
            "committee_management_ids": [
                1
            ],
            "default_vote_weight": 1,
            "id": 1,
            "is_active": true,
            "is_physical_person": true,
            "last_login": 1721992653,
            "last_name": "Administrator",
            "meeting_ids": [
                1,
                2
            ],
            "meeting_user_ids": [
                1,
                2
            ],
            "organization_management_level": "superadmin",
            "username": "superadmin"
        },
        "3": {
            "committee_ids": [
                1
            ],
            "committee_management_ids": [
                1
            ],
            "default_vote_weight": 1,
            "email": "",
            "first_name": "New Committee",
            "id": 3,
            "is_active": true,
            "is_physical_person": true,
            "last_login": 1721992786,
            "last_name": "Admin",
            "title": "",
            "username": "NewCommitteeAdmin"
        },
        "4": {
            "committee_ids": [
                1
            ],
            "default_password": "[censored]",
            "default_vote_weight": 1,
            "email": "",
            "first_name": "Ann",
            "id": 4,
            "is_active": true,
            "is_physical_person": true,
            "last_name": "",
            "meeting_ids": [
                1,
                2
            ],
            "meeting_user_ids": [
                3,
                4
            ],
            "username": "Ann"
        }
    }
}
{
    "committee/1/id": 1,
    "committee/1/manager_ids": [
        1,
        3
    ],
    "committee/1/meeting_ids": [
        1
    ],
    "committee/1/name": "Brand new committee with extra committee admin",
    "committee/1/user_ids": [
        1,
        3,
        4
    ],
    "group/1/id": 1,
    "group/1/name": "Default",
    "group/2/id": 2,
    "group/2/name": "Admin",
    "group/3/id": 3,
    "group/3/name": "Delegates",
    "group/4/id": 4,
    "group/4/name": "Staff",
    "meeting/1/committee_id": 1,
    "meeting/1/default_group_id": 1,
    "meeting/1/group_ids": [
        1,
        2,
        3,
        4
    ],
    "meeting/1/id": 1,
    "meeting/1/is_active_in_organization_id": 1,
    "meeting/1/name": "Meeting from brand new committee with extra committee admin",
    "meeting/1/projector_countdown_default_time": 60,
    "meeting/2/id": 2,
    "meeting/2/name": "Meeting with no extra committee admin",
    "meeting_user/1/group_ids": [
        2
    ],
    "meeting_user/1/id": 1,
    "meeting_user/1/meeting_id": 1,
    "meeting_user/1/user_id": 1,
    "meeting_user/2/group_ids": [],
    "meeting_user/2/id": 2,
    "meeting_user/2/user_id": 1,
    "meeting_user/3/group_ids": [
        1
    ],
    "meeting_user/3/id": 3,
    "meeting_user/3/meeting_id": 1,
    "meeting_user/3/user_id": 4,
    "meeting_user/4/group_ids": [],
    "meeting_user/4/id": 4,
    "meeting_user/4/user_id": 4,
    "user/1/committee_ids": [
        1
    ],
    "user/1/committee_management_ids": [
        1
    ],
    "user/1/default_vote_weight": "1.000000",
    "user/1/id": 1,
    "user/1/is_active": true,
    "user/1/is_physical_person": true,
    "user/1/last_login": 1721992653,
    "user/1/last_name": "Administrator",
    "user/1/meeting_ids": [
        1,
        2
    ],
    "user/1/meeting_user_ids": [
        1,
        2
    ],
    "user/1/organization_management_level": "superadmin",
    "user/1/username": "superadmin",
    "user/3/committee_ids": [
        1
    ],
    "user/3/committee_management_ids": [
        1
    ],
    "user/3/default_vote_weight": "1.000000",
    "user/3/email": "",
    "user/3/first_name": "New Committee",
    "user/3/id": 3,
    "user/3/is_active": true,
    "user/3/is_physical_person": true,
    "user/3/last_login": 1721992786,
    "user/3/last_name": "Admin",
    "user/3/title": "",
    "user/3/username": "NewCommitteeAdmin",
    "user/4/committee_ids": [
        1
    ],
    "user/4/default_password": "[censored]",
    "user/4/default_vote_weight": "1.000000",
    "user/4/email": "",
    "user/4/first_name": "Ann",
    "user/4/id": 4,
    "user/4/is_active": true,
    "user/4/is_physical_person": true,
    "user/4/last_name": "",
    "user/4/meeting_ids": [
        1,
        2
    ],
    "user/4/meeting_user_ids": [
        3,
        4
    ],
    "user/4/username": "Ann"
}
ostcar commented 1 month ago

The problem is the field user/meeting_ids.

This is not a relation-list-field, but a []number-field. That means, that for the autoupdate service, it is just a list of numbers, that have no other meaning then being a number. The autoupdate-service does not know, that this numbers are actually meeting-ids and that maybe the user is not allowed to see some of the meetings.

If we want to fix this, we either have to change the concept of the autoupdate-service and make it less generic (I don't recommend that), or we have to remove this field.

This field is not really necessary. It is an redundant field, that contains the result of the calculation, in which meeting the user is.

The alternative to that field is, that every service (autoupdate and client) do not use this precalculation but do the calculation them self. Since the introduction of meeting_user, this is quite easy. Just loop user/meeting_user/meeting_id and also check, that meeting_user/group_ids is not empty (we could discussed, that this last check is not necessary).

If we do it like this, then the autoupdate-service can filter the values. Maybe we could change the restriction mode meeting_user/A that a meeting_user can only be seen, when the request user can see the user and the meeting. See also #970 for this discussion.