OpenSlides / openslides-autoupdate-service

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

Users without the permission to see the `content_object` of a projection does not see the `content_object_id`, too #342

Closed GabrielInTheWorld closed 2 years ago

GabrielInTheWorld commented 2 years ago

I sent this request:

{
    "collection": "meeting",
    "ids": [
        253
    ],
    "fields": {
        "description": null,
        "name": null,
        "start_time": null,
        "end_time": null,
        "is_active_in_organization_id": null,
        "id": null,
        "reference_projector_id": {
            "type": "relation",
            "collection": "projector",
            "fields": {
                "name": null,
                "scale": null,
                "scroll": null,
                "width": null,
                "aspect_ratio_numerator": null,
                "aspect_ratio_denominator": null,
                "color": null,
                "background_color": null,
                "header_background_color": null,
                "header_font_color": null,
                "header_h1_color": null,
                "chyron_background_color": null,
                "chyron_font_color": null,
                "show_header_footer": null,
                "show_title": null,
                "show_logo": null,
                "show_clock": null,
                "used_as_reference_projector_meeting_id": null,
                "id": null,
                "current_projection_ids": {
                    "type": "relation-list",
                    "collection": "projection",
                    "fields": {
                        "stable": null,
                        "type": null,
                        "options": null,
                        "weight": null,
                        "content": null,
                        "current_projector_id": null,
                        "id": null,
                        "content_object_id": {
                            "type": "generic-relation",
                            "fields": {
                                "title": null,
                                "number": null,
                                "created": null,
                                "sequential_number": null,
                                "text": null,
                                "reason": null,
                                "recommendation_id": null,
                                "tag_ids": null,
                                "personal_note_ids": null,
                                "block_id": null,
                                "category_id": null,
                                "lead_motion_id": null,
                                "comment_ids": null,
                                "modified_final_version": null,
                                "state_extension": null,
                                "recommendation_extension": null,
                                "agenda_item_id": null,
                                "amendment_paragraph_$": {
                                    "type": "template"
                                },
                                "poll_ids": null,
                                "origin_id": null,
                                "id": null,
                                "is_directory": null,
                                "parent_id": null,
                                "child_ids": null,
                                "mimetype": null,
                                "filesize": null,
                                "create_timestamp": null,
                                "has_inherited_access_groups": null,
                                "pdf_information": null,
                                "closed": null,
                                "content_object_id": null,
                                "speaker_ids": null,
                                "internal": null,
                                "open_posts": null,
                                "phase": null,
                                "candidate_ids": null,
                                "description": null,
                                "default_poll_description": null,
                                "number_poll_candidates": null,
                                "item_number": null,
                                "comment": null,
                                "type": null,
                                "is_hidden": null,
                                "is_internal": null,
                                "duration": null,
                                "weight": null,
                                "level": null,
                                "meeting_id": null,
                                "first_name": null,
                                "last_name": null,
                                "username": null,
                                "vote_weight_$": {
                                    "type": "template"
                                },
                                "structure_level_$": {
                                    "type": "template"
                                },
                                "number_$": {
                                    "type": "template"
                                },
                                "email": null,
                                "gender": null,
                                "is_active": null,
                                "is_physical_person": null,
                                "is_present_in_meeting_ids": null,
                                "last_email_send": null,
                                "default_number": null,
                                "default_structure_level": null,
                                "default_vote_weight": null,
                                "comment_$": {
                                    "type": "template"
                                },
                                "about_me_$": {
                                    "type": "template"
                                },
                                "default_password": null,
                                "entitled_group_ids": null,
                                "state": null,
                                "pollmethod": null,
                                "voted_ids": null,
                                "votescast": null,
                                "votesinvalid": null,
                                "votesvalid": null,
                                "option_ids": null,
                                "onehundred_percent_base": null,
                                "global_option_id": null,
                                "global_yes": null,
                                "global_no": null,
                                "global_abstain": null,
                                "min_votes_amount": null,
                                "max_votes_amount": null,
                                "entitled_users_at_stop": null,
                                "message": null,
                                "default_time": null,
                                "countdown_time": null,
                                "running": null,
                                "list_of_speakers_id": {
                                    "type": "relation",
                                    "collection": "list_of_speakers",
                                    "fields": {
                                        "closed": null,
                                        "content_object_id": null,
                                        "speaker_ids": {
                                            "type": "relation-list",
                                            "collection": "speaker",
                                            "fields": {
                                                "begin_time": null,
                                                "end_time": null,
                                                "point_of_order": null,
                                                "speech_state": null,
                                                "weight": null,
                                                "note": null,
                                                "id": null,
                                                "user_id": {
                                                    "type": "relation",
                                                    "collection": "user",
                                                    "fields": {
                                                        "title": null,
                                                        "first_name": null,
                                                        "last_name": null,
                                                        "username": null,
                                                        "id": null
                                                    }
                                                }
                                            }
                                        },
                                        "id": null
                                    }
                                }
                            }
                        }
                    }
                }
            }
        }
    }
}

Hint: The underlying motion is in a state, in which only users can see the motion, who have the permissionmotion.can_manageor if they are the submitters of the motion.

As a delegate, I get this update:

{
    "meeting/253/description": "",
    "meeting/253/end_time": 1634248800,
    "meeting/253/id": 253,
    "meeting/253/is_active_in_organization_id": 1,
    "meeting/253/name": "Das Alphabet (1)",
    "meeting/253/reference_projector_id": 64,
    "meeting/253/start_time": 1634248800,
    "projection/7/content": {
        "id": 10,
        "title": "Kombüse",
        "number": "1",
        "submitters": [
            "Administrator"
        ],
        "show_sidebox": false,
        "line_length": 85,
        "preamble": "The assembly may decide:",
        "line_numbering": "outside",
        "change_recommendations": null,
        "text": "<p>yeah</p>"
    },
    "projection/7/current_projector_id": 64,
    "projection/7/id": 7,
    "projection/7/options": {},
    "projection/7/stable": false,
    "projector/64/aspect_ratio_denominator": 9,
    "projector/64/aspect_ratio_numerator": 16,
    "projector/64/background_color": "#ffffff",
    "projector/64/chyron_background_color": "#317796",
    "projector/64/chyron_font_color": "#ffffff",
    "projector/64/color": "#000000",
    "projector/64/current_projection_ids": [
        7
    ],
    "projector/64/header_background_color": "#317796",
    "projector/64/header_font_color": "#f5f5f5",
    "projector/64/header_h1_color": "#317796",
    "projector/64/id": 64,
    "projector/64/name": "Default projector",
    "projector/64/scale": 0,
    "projector/64/scroll": 0,
    "projector/64/show_clock": true,
    "projector/64/show_header_footer": true,
    "projector/64/show_logo": true,
    "projector/64/show_title": true,
    "projector/64/used_as_reference_projector_meeting_id": 253,
    "projector/64/width": 1200
}

As a superadmin, I get this update:

{
    "list_of_speakers/47/closed": false,
    "list_of_speakers/47/content_object_id": "motion/10",
    "list_of_speakers/47/id": 47,
    "list_of_speakers/47/speaker_ids": [
        14
    ],
    "meeting/253/description": "",
    "meeting/253/end_time": 1634248800,
    "meeting/253/id": 253,
    "meeting/253/is_active_in_organization_id": 1,
    "meeting/253/name": "Das Alphabet (1)",
    "meeting/253/reference_projector_id": 64,
    "meeting/253/start_time": 1634248800,
    "motion/10/agenda_item_id": 40,
    "motion/10/created": 1634279827,
    "motion/10/id": 10,
    "motion/10/list_of_speakers_id": 47,
    "motion/10/meeting_id": 253,
    "motion/10/number": "1",
    "motion/10/sequential_number": 1,
    "motion/10/text": "<p>yeah</p>",
    "motion/10/title": "Kombüse",
    "projection/7/content": {
        "id": 10,
        "title": "Kombüse",
        "number": "1",
        "submitters": [
            "Administrator"
        ],
        "show_sidebox": false,
        "line_length": 85,
        "preamble": "The assembly may decide:",
        "line_numbering": "outside",
        "change_recommendations": null,
        "text": "<p>yeah</p>"
    },
    "projection/7/content_object_id": "motion/10",
    "projection/7/current_projector_id": 64,
    "projection/7/id": 7,
    "projection/7/options": {},
    "projection/7/stable": false,
    "projector/64/aspect_ratio_denominator": 9,
    "projector/64/aspect_ratio_numerator": 16,
    "projector/64/background_color": "#ffffff",
    "projector/64/chyron_background_color": "#317796",
    "projector/64/chyron_font_color": "#ffffff",
    "projector/64/color": "#000000",
    "projector/64/current_projection_ids": [
        7
    ],
    "projector/64/header_background_color": "#317796",
    "projector/64/header_font_color": "#f5f5f5",
    "projector/64/header_h1_color": "#317796",
    "projector/64/id": 64,
    "projector/64/name": "Default projector",
    "projector/64/scale": 0,
    "projector/64/scroll": 0,
    "projector/64/show_clock": true,
    "projector/64/show_header_footer": true,
    "projector/64/show_logo": true,
    "projector/64/show_title": true,
    "projector/64/used_as_reference_projector_meeting_id": 253,
    "projector/64/width": 1200,
    "speaker/14/id": 14,
    "speaker/14/user_id": 1,
    "speaker/14/weight": 1,
    "user/1/first_name": "",
    "user/1/id": 1,
    "user/1/last_name": "Administrator",
    "user/1/title": "",
    "user/1/username": "admin"
}

As you can see, there are some fields missing, e.g. the content_object_id. This field - for example - is necessary to show the current projection correctly.

ostcar commented 2 years ago

I have not checked this. But it sounds, that it is the same problem then in #343

@GabrielInTheWorld could you confirm this and then close this as duplicate?

ostcar commented 2 years ago

This is not a duplicate of #343. I found the problem but I don't know how to solve it. I think the current behavior is correct.

The issue is, that a User can not see a motion but the motion is on the projector (which the user can see). The question is, can the user see projection/X/content_object_id.

To see a [generic-]relation-field, the user needs not only the permission for that field but also the permission for the related field. So in this case, the user has to have the permission for motion/X/projection_ids. Since the user can not see the motion, also projection/X/content_object_id is restricted.

I think this behavior is important. If the user could see projection/X/content_object_id but not the motion, then it would seem as if the relation / database was broken.

If it is important that every user that can see the projection can see projection/content_object_id then we have to change the restriction of motion and all other objects, that can be on the projector, so the back-reference can be seen when the object is on the projection. It is a bit more complicated, because the id-field has also to be seen, if one field of a object can be seen (see the description in the overview).

For example, for motion, it would be look like this:

// Group A
  id
// Group B
  number
  title
// Group C
  projection_ids

The question is, if it is really necessary to see projection/content_object_id to show the projection. All requied fields should be contained in projection/content. If not, @r-peschke could add them.

r-peschke commented 2 years ago

Seems okay to me. Especially the projection/7/content can be shown at all without restrictions, including the motion data. What do you wanna show more?

GabrielInTheWorld commented 2 years ago

Okay, the only thing I need to fix this, is the information about projection/X/content_object_id. I need this, because the content_object_id is a fqid and thus I get the collection from the content_object.

GabrielInTheWorld commented 2 years ago

As of projection restrictions every user, who can see a projector, can also see the content_object_id and this is missing.

r-peschke commented 2 years ago

Wiki page projection restrictions allows the user with the right projector.can_see to see the field projection.content_object_id. But, see example above, this field was removed by the restrictor.

ostcar commented 2 years ago

I documented in the wiki how the restrictor handles relation fields. If hope this is clearer know: https://github.com/OpenSlides/OpenSlides/wiki/Restrictions-Overview

@GabrielInTheWorld I don't understand why you need field content_object_id in a situation, where the client can not see the related content_object.

In your first post, you have the case, that there in a motion on the projector, that the user is not allowed to see. But if the user can not see the motion, then content_object has to be null. Why do you need the id of a motion, if the client should not see the motion?

GabrielInTheWorld commented 2 years ago

@GabrielInTheWorld I don't understand why you need field content_object_id in a situation, where the client can not see the related content_object.

In your first post, you have the case, that there in a motion on the projector, that the user is not allowed to see. But if the user can not see the motion, then content_object has to be null. Why do you need the id of a motion, if the client should not see the motion?

I do not need only the id of an object, I even need the fqid of the object. The reason is, that an fqid contains the information about the collection of an object. The content_object_id is the fqid of an object, which is e.g. projected. In your projection overview is documented, that everyone, who can see a projector, is allowed to see the content_object_id from a projection. So, please implement it.

ostcar commented 2 years ago

Could you explain the concrete situation what you want to do with the fqid? In which situation is it needed. What bug/problem should be solved with it?

GabrielInTheWorld commented 2 years ago

As I already wrote, the client get the collection of a content_object from a projection by its content_object_id. To determine which slide the client should show, it needs the collection.

ostcar commented 2 years ago

Would it help to change the content of projection/content to include the type of the content object?

For example:

{
  "type": "assignment",
  "title": "...",
  ...
}
GabrielInTheWorld commented 2 years ago

Yeah, this would be helpful. But why do you not send just the content_object_id as described in the wiki?

ostcar commented 2 years ago

After relation fields have passed the first filter (descried in the wiki), that the user can see them, they have to pass another filter, that the user can also see the other end of the relation. This is a generic check that I can not change only for this case. See this wiki page.

The reason is to prevent invalid relations.

I add the change to the projection/content field as soon as possible. @r-peschke I hope it is ok for you that I do that. If you don't like this change or you want to do it, please ping me. I will add you for a review.

r-peschke commented 2 years ago

I add the change to the projection/content field as soon as possible. @r-peschke I hope it is ok for you that I do that. If you don't like this change or you want to do it, please ping me. I will add you for a review.

Surely this is okay for me.:-). You should realize it for all projections, it will be a problem not only for the motion. But I think it is meant this way. Another thing to mention is the name: I would prefer collection instead of type, because this part of an fqid is the collection.