Project

General

Profile

Bug #28093

Wrong host ownership in UserMailNotifications

Added by Emil Dragu 10 months ago. Updated 3 months ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
Notifications
Target version:
-
Difficulty:
Triaged:
No
Bugzilla link:

Description

This behavior was detected as a result of incorrect hosts in host_mailer("Configuration Management Summary Report") mail notification.

Steps to reproduce.
1. Must have a usergroup and a user that is part of this group but is not administrator.
2. Set owner of a host to the above usergroup.
3. Run the following with foreman-rake console(I assume the same happens in mail notification tasks):
usr = User.find_by(login: 'testuser')
usr.hosts #correctly returns all the hosts the user owns and thus is authorized to view
Host::Managed.authorized_as(usr, :view_hosts, Host) # this should include the host owned by the groups user is part of, but is not. Scoped search seems to be executed based on current user which is foreman_console_admin in this case.

Associated revisions

Revision 2ab587b1 (diff)
Added by Emil DRAGU 5 months ago

Fixes #28093 - Wrong host ownership in UserMailNotifications

Wrong list of hosts in "Configuration Management Summary Report" when view_host permission
is used with filter "owner = current_user"

Revision 13e2ed3e (diff)
Added by Emil DRAGU 5 months ago

Fixes #28093 - Changes after review suggestion

Do it in User.as block so that the conext is restored after.

Revision 13ed812e (diff)
Added by Emil DRAGU 5 months ago

Fixes #28093 - fix identation issue

History

#1 Updated by Marek Hulán 10 months ago

If I understand that correctly, user does not get notification for host he/she doesn't have permission to see. They can still be assigned as an owner through usergroup.

I see it can be confusing but does not sound as a bug to me. Perhaps we should offer to chose usergroup if it has no member with view permission on the host. But permission should be source of the truth for whether the user can see info about the host or not.

#2 Updated by Emil Dragu 10 months ago

Marek Hulán wrote:

If I understand that correctly, user does not get notification for host he/she doesn't have permission to see. They can still be assigned as an owner through usergroup.

As member of the usergroup that own the hosts, the user has permissions to see the hosts in the web interface, he can even edit the hosts, but the report does not include these hosts. Investigating the issue, I noticed that the queries ran when report is generated are based on an user_id that corresponds to foreman_console_admin, which is the current_user when running the tasks. So somehow, the user only receives information about hosts that are visible by foreman_console_admin.

#3 Updated by Marek Hulán 10 months ago

But foreman_console_admin can always see all hosts, so that shouldn't limit the list in the notification.

#4 Updated by Emil Dragu 9 months ago

Yes, foreman_console_admin sees all hosts but my user, which is testuser, should not see all the hosts foreman_console_admin sees but only the hosts he owns, which is not the case here. To make it clearer, testuser sees more hosts than he should see.

#5 Updated by Emil Dragu 6 months ago

I found this is related to setting a filter with owner = current_user and add the role to the user or usergroup, so this must be added to the steps to reproduce this issue:
1. Create a role with view_host permission, with filter set to "owner = current_user"
2. Add role to the user or usergroup
3. When sending email report, current_user will result in getting the hosts of the current user that the mailer runs under(lib/tasks/reports.rake), which is anonymous_admin

#6 Updated by The Foreman Bot 6 months ago

  • Status changed from New to Ready For Testing
  • Pull request https://github.com/theforeman/foreman/pull/7422 added

#7 Updated by The Foreman Bot 5 months ago

  • Fixed in Releases 2.1.0 added

#8 Updated by The Foreman Bot 5 months ago

  • Assignee set to Tomer Brisker
  • Pull request https://github.com/theforeman/foreman/pull/7543 added

#9 Updated by Anonymous 5 months ago

  • Status changed from Ready For Testing to Closed

Also available in: Atom PDF