Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Limit displayed models in audit logging based on permissions #9181

Open
th3hamm0r opened this issue Sep 12, 2022 · 1 comment
Open

Limit displayed models in audit logging based on permissions #9181

th3hamm0r opened this issue Sep 12, 2022 · 1 comment
Assignees
Milestone

Comments

@th3hamm0r
Copy link
Contributor

th3hamm0r commented Sep 12, 2022

Is your proposal related to a problem?

Currently, a user which for example is only allowed to edit pages (like users of the default editor group), still sees every logged change of any model in the audit log, e.g. changed site settings or changed users (if a superuser has changed them for example), which he actually can't see/edit, therefore he often would not know of their existence, which is bad...
The user cannot view the underlying objects, because if the view permission is missing, there is no link, so that's good! But I think he should not see any log entries of models, for which he doesn't have at least the view permission.

Describe the solution you'd like

Basically, the querysets used for the filters and the listings should only use the content types, which at least can be viewed by the user.

On a first quick look I think it probably requires changes to the filter here (probably by adding/changing the API to contain a user to be used to filter get_content_type_ids()):

def get_content_types_for_filter():
content_type_ids = set()
for log_model in log_action_registry.get_log_entry_models():
content_type_ids.update(log_model.objects.all().get_content_type_ids())

and changes to the listing's queryset (probably by changing viewable_by_user()):

self.log_models = list(log_action_registry.get_log_entry_models())
for log_model_index, log_model in enumerate(self.log_models):
sub_queryset = (
log_model.objects.viewable_by_user(self.request.user)
.values("pk", "timestamp")
.annotate(
log_model_index=Value(log_model_index, output_field=IntegerField())
)
)

If you are ok with my proposal, I could, maybe with some guidance, try to implement a PR for that.

th3hamm0r added a commit to th3hamm0r/wagtail that referenced this issue Sep 16, 2022
Until now, a user could see the audit log for all (!) custom models,
permissions haven't been checked yet. This may disclose sensitive
information to unauthorized admin users.
Now, only log entries of those content types are displayed, where
the user has at least one permission.

This change also fixes an issue with the log entries for pages:
If the user only had access to specific parts of the sitetree, the audit
log still contained all entries of the ancestor pages which the user
actually couldn't view/edit.
For this, parts of the UserPagePermissionsProxy's explorable_pages()
have been extracted into a new viewable_pages() method.

Fixes wagtail#9181
th3hamm0r added a commit to th3hamm0r/wagtail that referenced this issue Sep 21, 2022
Until now, a user could see the audit log for all (!) custom models,
permissions haven't been checked yet. This may disclose sensitive
information to unauthorized admin users.
Now, only log entries of those content types are displayed, where
the user has at least one permission.

This change also fixes an issue with the log entries for pages:
If the user only had access to specific parts of the sitetree, the audit
log still contained all entries of the ancestor pages which the user
actually couldn't view/edit.
For this, parts of the UserPagePermissionsProxy's explorable_pages()
have been extracted into a new viewable_pages() method.

Fixes wagtail#9181
@gasman gasman added this to the 4.2 milestone Oct 18, 2022
th3hamm0r added a commit to th3hamm0r/wagtail that referenced this issue Oct 25, 2022
Until now, a user could see the audit log for all (!) custom models,
permissions haven't been checked yet. This may disclose sensitive
information to unauthorized admin users.
Now, only log entries of those content types are displayed, where
the user has at least one permission.

This change also fixes an issue with the log entries for pages:
If the user only had access to specific parts of the sitetree, the audit
log still contained all entries of the ancestor pages which the user
actually couldn't view/edit.
For this, parts of the UserPagePermissionsProxy's explorable_pages()
have been extracted into a new viewable_pages() method.

Fixes wagtail#9181
th3hamm0r added a commit to th3hamm0r/wagtail that referenced this issue Oct 31, 2022
Until now, a user could see the audit log for all (!) custom models,
permissions haven't been checked yet. This may disclose sensitive
information to unauthorized admin users.
Now, only log entries of those content types are displayed, where
the user has at least one permission.

This change also fixes an issue with the log entries for pages:
If the user only had access to specific parts of the sitetree, the audit
log still contained all entries of the ancestor pages which the user
actually couldn't view/edit.
For this, parts of the UserPagePermissionsProxy's explorable_pages()
have been extracted into a new viewable_pages() method.

Fixes wagtail#9181
@gasman gasman self-assigned this Nov 24, 2022
@gasman gasman modified the milestones: 4.2, 4.3 Jan 18, 2023
@gasman gasman modified the milestones: 5.0, 5.1 Apr 19, 2023
@gasman gasman modified the milestones: 5.1, 5.2 Jul 18, 2023
th3hamm0r added a commit to th3hamm0r/wagtail that referenced this issue Oct 9, 2023
Until now, a user could see the audit log for all custom models,
permissions haven't been checked yet.
Now, only log entries of those content types are displayed, where
the user has at least one permission.

The change also adds filtering to the action-dropdown.
This avoids showing unnecessary action types, e.g. actions which aren't
used at all.
All 3 uses of action-dropdowns (site history, page history, generic
history) only display those actions, which are used at least once.

This partially fixes wagtail#9181.
@thibaudcolas thibaudcolas modified the milestones: 5.2, 5.3 Oct 18, 2023
@gasman gasman closed this as completed in 3de6ce6 Oct 19, 2023
@gasman
Copy link
Collaborator

gasman commented Oct 19, 2023

reopening as #11020 is only a partial fix

@gasman gasman reopened this Oct 19, 2023
@gasman gasman modified the milestones: 6.0, 6.1 Jan 23, 2024
@gasman gasman modified the milestones: 6.1, 6.2 Apr 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment