Bug #10689
closed
Unattended controller permission check does not work
Description
Create a non-admin user (e.g. viewer role only), create new host, and as the viewer user log on and try to view a template.
2015-06-03 14:35:41 [I] Started GET "/unattended/iPXE?hostname=bdsktest.local.lan" for 127.0.0.1 at 2015-06-03 14:35:41 +0200
2015-06-03 14:35:41 [I] Processing by UnattendedController#iPXE as HTML
...
app/controllers/application_controller.rb:224:in `block (2 levels) in render_403'
app/controllers/application_controller.rb:223:in `render_403'
app/controllers/application_controller.rb:63:in `deny_access'
app/controllers/application_controller.rb:55:in `authorize'
app/controllers/unattended_controller.rb:20:in `block (2 levels) in <class:UnattendedController>'
app/models/concerns/foreman/thread_session.rb:32:in `clear_thread'
lib/middleware/catch_json_parse_errors.rb:9:in `call'
We have two issues basically:
A) Permissions are not checked properly (we obviously use template kinds as actions and we don't have such permissions)
B) Method render_403 does not work from UnattendedController context (layout error).
Updated by Dominic Cleal over 9 years ago
- Related to Bug #2892: unattended spoof mode only work for an administrator added
Updated by Marek Hulán over 9 years ago
Ideally I'd prefer getting rid of https://github.com/theforeman/foreman/blob/develop/app/controllers/unattended_controller.rb#L62 and add one action review that would accept parameter kind. To keep backward compatibility routes should be defined accordingly.
Other way is to add add kinds to https://github.com/theforeman/foreman/blob/develop/app/services/foreman/access_permissions.rb#L111 but note that through view_template permission one can read much more data (with combination of edit_template, it's quite powerful) so adding extra permission just for reviewing should be considered.
Updated by Lukas Zapletal over 9 years ago
I will add review_templates permission. Thanks!
Updated by The Foreman Bot over 9 years ago
- Status changed from Assigned to Ready For Testing
- Pull request https://github.com/theforeman/foreman/pull/2428 added
- Pull request deleted (
)
Updated by Dominic Cleal over 8 years ago
- Related to Bug #15490: CVE-2016-4995 - view_hosts permissions/filters not checked for provisioning template previews added
Updated by Dominic Cleal over 8 years ago
- Related to Refactor #13039: Remove DB queries from class of UnattendedController added
Updated by Dominic Cleal over 8 years ago
The change first suggested by Marek was actually made in #13039, causing this issue to be fixed in Foreman 1.11.0.
It's however introduced a security vulnerability recorded at #15490 (first noted at https://github.com/theforeman/foreman/pull/2428#issuecomment-114842874 for this patch), but hopefully this patch can be repurposed to fix that under the new security ticket. Leaving this open for now, but will probably change.
Updated by Dominic Cleal over 8 years ago
- Status changed from Ready For Testing to Duplicate
Fixed in #13039 for 1.11.0.