Bug 782022

Summary:	UI permissions review
Product:	Red Hat Satellite	Reporter:	Lukas Zapletal <lzap>
Component:	WebUI	Assignee:	Justin Sherrill <jsherril>
Status:	CLOSED CURRENTRELEASE	QA Contact:	Garik Khachikyan <gkhachik>
Severity:	medium	Docs Contact:
Priority:	unspecified
Version:	6.0.1	CC:	bkearney, gkhachik, hbrock, jsherril, mkoci, mmccune
Target Milestone:	Unspecified	Keywords:	Triaged
Target Release:	Unused
Hardware:	Unspecified
OS:	Unspecified
Whiteboard:
Fixed In Version:		Doc Type:	Bug Fix
Doc Text:		Story Points:	---
Clone Of:		Environment:
Last Closed:	2012-08-22 18:19:44 UTC	Type:	---
Regression:	---	Mount Type:	---
Documentation:	---	CRM:
Verified Versions:		Category:	---
oVirt Team:	---	RHEL 7.3 requirements from Atomic Host:
Cloudforms Team:	---	Target Upstream Version:
Bug Depends On:
Bug Blocks:	747354

Description Lukas Zapletal 2012-01-16 11:27:02 UTC

During my premissions API review and implementation this sprint I was 
also looking in the UI controller code and I have found there is the 
following construct in many places:

skip_before_filter :authorize #load the environment
before_filter :find_environment
before_filter :authorize

The skip_before_filter call is not needed in this case, so I have 
removed it from all UI controllers. I did this because now we can track
skip_before_filter lines with grep:

If you do

git grep "skip_before_filter :authorize"

you will find the following UI controllers that has (for some reason) 
disabled authorize call.

app/controllers/accounts_controller.rb
app/controllers/dashboard_controller.rb
app/controllers/errata_controller.rb
app/controllers/errors_controller.rb
app/controllers/notices_controller.rb

When I was sure it is our intention I commented it to see those lines 
with grep:

skip_before_filter :authorize # ok - is used by warden

On top of that the following controllers either do not have rules 
implemented or do not have authorize filter enabled:

packages_controller.rb
distributions_controller.rb
errata_controller.rb
templates_content_controller.rb (authorize filter call is missing)

The list is not complete. I am creating a new BZ for UI permissions 
review. It's target is to comment all skip_before_filter :authorize 
lines where appropriate, remove the rest and add rules to all 
controllers.

Comment 1 Lukas Zapletal 2012-01-16 11:29:05 UTC

Please let me know once you will be implementing rules for

packages_controller.rb
distributions_controller.rb
errata_controller.rb

Because those are not implemented in the API as well.

Comment 2 Lukas Zapletal 2012-01-16 13:28:15 UTC

Please note for distributions I already enriched the UI code with some comments. A route change is needed. Tracking BZ is:

https://bugzilla.redhat.com/show_bug.cgi?id=771411

Comment 3 Justin Sherrill 2012-03-05 13:51:49 UTC

Added permissions to packages and errata controllers.  I ended up not adding repoids for everything in the routes, to simplify the change.  Now that errata and packages are returning  repoids from pulp, checking permissions is very easy. 

05cda575fd8442aeeaa954ab5f3fad36d3622483

Comment 4 Justin Sherrill 2012-03-05 13:54:01 UTC

An easy way to test:

1.  go to promotions page with a repo synced
2.  Navigate to packages
3.  with chrome/firefox with firebug inspect a package and grab the link (i.e. /katello/packages/ca934dc5-f150-4926-990e-a8966f8a7a82)
4.  Browse to this URL (should work).
5.  Create a user that does not have a permission to access the provider for this repo
6.  Try to access the same URL (Shouldn't work).

Do the same for errata.

Comment 6 Mike McCune 2012-03-07 23:43:36 UTC

mass move ON_QA after brewing

Comment 8 Garik Khachikyan 2012-03-22 10:29:42 UTC

# COMMENT

comment#4 just checked for packages - works.

doing one for erratas now...

Comment 9 Garik Khachikyan 2012-03-22 10:36:13 UTC

# VERIFIED

same as for packages.

pushing the bug to verified now.

checked against:

---
mod_wsgi-3.3-3.pulp.el6.x86_64
katello-common-0.2.15-1.git.6.baa234a.el6.noarch
pulp-selinux-server-1.0.0-6.el6.noarch
katello-repos-testing-0.2.1-1.el6.noarch
qpid-cpp-server-ssl-0.12-6.el6.x86_64
candlepin-0.5.26-1.el6.noarch
candlepin-tomcat6-0.5.26-1.el6.noarch
katello-certs-tools-1.1.3-1.el6.noarch
qpid-cpp-server-0.12-6.el6.x86_64
katello-glue-pulp-0.2.15-1.git.6.baa234a.el6.noarch
katello-0.2.15-1.git.6.baa234a.el6.noarch
qpid-cpp-client-ssl-0.12-6.el6.x86_64
katello-qpid-client-key-pair-1.0-1.noarch
katello-cli-common-0.2.15-1.git.4.8503392.el6.noarch
katello-cli-0.2.15-1.git.4.8503392.el6.noarch
m2crypto-0.21.1.pulp-7.el6.x86_64
python-oauth2-1.5.170-2.pulp.el6.noarch
pulp-common-1.0.0-6.el6.noarch
qpid-cpp-client-0.12-6.el6.x86_64
katello-glue-candlepin-0.2.15-1.git.6.baa234a.el6.noarch
katello-selinux-0.2.3-1.git.57.6c5edb8.el6.noarch
katello-qpid-broker-key-pair-1.0-1.noarch
katello-agent-1.0.3-1.git.0.cccd0b4.el6.noarch
python-qpid-0.12-1.el6.noarch
katello-glue-foreman-0.2.15-1.git.6.baa234a.el6.noarch
pulp-1.0.0-6.el6.noarch
katello-repos-0.2.1-1.el6.noarch
katello-configure-0.2.15-1.git.4.94aa90a.el6.noarch
katello-candlepin-cert-key-pair-1.0-1.noarch
---

Comment 11 Mike McCune 2013-08-16 18:19:30 UTC

getting rid of 6.0.0 version since that doesn't exist