Fixes 5521: Adding permissions for managing subscriptions and manifests. #4042
Conversation
ehelms
changed the title
|
|
||
| # Authorize must be before find_subscription since find_subscription reaches out to Candlepin | ||
| # and needs a current user set | ||
| before_filter :authorize |
There was a problem hiding this comment.
Why remove the before_filter :authorize here?
There was a problem hiding this comment.
Handled by the base controller. We only ever did this so we could override it.
There was a problem hiding this comment.
What do you mean by "we did this so that we could override it"?. It seems the reason was the ordering, as the comment describes. It's possible the Katello as an engine made that code redundant though
There was a problem hiding this comment.
It was a mix of the old comment, and the issues brought with enginification effort. But at this juncture the call doesn't need to be there based on how things work.
|
ACK |
| { | ||
| 'katello/api/v2/subscriptions' => [:index, :show, :available] | ||
| }, | ||
| :resource_type => 'Organization' |
There was a problem hiding this comment.
Why not just?
permission :view_subscriptions,
{ 'katello/api/v2/subscriptions' => [:index, :show, :available] },
:resource_type => 'Organization'There was a problem hiding this comment.
Makes it easier to add more routes/actions this way and we've done both the single item and multi-item hashes the same way this way.
|
ack - permission granularity looks good to me. thanks! |
Fixes 5521: Adding permissions for managing subscriptions and manifests.
No description provided.