Bug 669521 - getting agent clients is now too restrictive
Summary: getting agent clients is now too restrictive
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: RHQ Project
Classification: Other
Component: Core Server
Version: 4.0.0.B02
Hardware: Unspecified
OS: Unspecified
urgent
urgent
Target Milestone: ---
: RHQ 4.3.0,JON 3.0.0
Assignee: Simeon Pinder
QA Contact: Mike Foley
URL:
Whiteboard:
Depends On:
Blocks: jon30-bugs
TreeView+ depends on / blocked
 
Reported: 2011-01-13 21:15 UTC by John Mazzitelli
Modified: 2012-02-07 19:26 UTC (History)
3 users (show)

Fixed In Version: 4.3
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2012-02-07 19:23:27 UTC
Embargoed:


Attachments (Terms of Use)
Screenshot_Agent-tab (22.12 KB, image/png)
2011-10-07 12:02 UTC, Sunil Kondkar
no flags Details
ServerLog (12.87 KB, text/plain)
2011-10-07 12:02 UTC, Sunil Kondkar
no flags Details


Links
System ID Private Priority Status Summary Last Updated
Red Hat Bugzilla 754479 0 medium NEW Revisit permission handling for AgentManagerBean methods 2022-03-31 04:28:37 UTC

Internal Links: 754479

Description John Mazzitelli 2011-01-13 21:15:44 UTC
Description of problem:

git commit 2187b1d51bfd8503236fecdadef8984e23aa9c7d has introduced a problem. Before getting agent clients didn't require a Subject. Now it does and now it only allows the agent client to be returned if the user has MANAGE_INVENTORY.

Follow all the callers to AgentManager.getAgentClient and you'll see the different places where users might not have that permission. Two that I know of by doing a quick check is deploying bundles and invoking operations.

That authz check needs to be relaxed OR perhaps have another local SLSB API that doesn't require this check (and make sure it isn't a remote API).

Comment 1 John Mazzitelli 2011-01-13 21:16:32 UTC
assigning to spinder - he added that new Subject parameter to the AgentManagerBean API.

Comment 2 Simeon Pinder 2011-01-14 14:24:43 UTC
This should be fixed with following commit to master :
commit 12e0188663cf0f6956b94f0ce2e59d24359ca112 

Fix details:
-requiring the subject to have MANAGE_INVENTORY permissions to launch operations or carry out bundle actions or a slew of other calls to AgentManagerBean.getAgentByResourceId() is unnecessarily restrictive. Previously all calls to AgentManagerBean.getAgentByResourceId() were made from within portal.war webapp which always operated server side and security was not necessary. With the creation of AgentGWTServiceImpl to directly expose the same method, permissions necessarily needed to be added to protect these now client side actions/calls. 

With this fix, only the calls to AgentMangerBean.getAgentByResourceId() from the GWT clients get the full permissions check and previous calls(formerly only in portal war) were modified to use the overlord Subject instead. The logic here is that a subject was not previously required/used(prior to GWT UI) implying overlord privileges for the previous level of functionality. Replacing such functionality here should not increase or diminish permissions for the same operation calls in any significant way.

Comment 3 Venkat 2011-09-28 12:54:05 UTC
Simeon could you please help me by providing steps to verify this BZ.

Comment 4 Simeon Pinder 2011-09-28 14:27:06 UTC
Hi Venkat. This bug was more to capture the api changes necessary for conversion to gwt. But here's the reproduction steps:

i)Startup an RHQ server, login as 'rhqadmin' and navigate to a resource(Platforms is fine).
ii)Select 'Inventory' tab and navigate to the the 'Agent' subtab where you should see details for the specific agent that's monitoring the resource.

iii)Logout and log back in as a user without MANAGE_INVENTORY.
iv) Repeat step ii. You should not be able to navigate to or see the Agent tab contents.

This will test that the 'Agent' menu option is still visible in the ui, albeit disabled, and that the contents are correctly displayed for a user with MANAGE_INVENTORY.

v) I would also test the above as well with a different user, not rhqadmin, that had MANAGE_INVENTORY perms to make sure that we aren't just doing the check correctly for 'rhqadmin'.

Comment 5 Sunil Kondkar 2011-10-07 12:01:26 UTC
Verified on build#485 (Version: 4.1.0-SNAPSHOT Build Number: e07065d).

1. Logged in as rhqadmin. Navigated to Inventory->Agent sub tab of platform resource. Verified that details of the specific agent are displayed.

2. Logged in as a user without MANAGE_INVENTORY permissions. The 'Agent' sub tab in 'Inventory' tab of the platform is disabled as expected.


3. Logged in as a user with only MANAGE_INVENTORY permissions. The 'Agent' sub tab is enabled. However, clicking on the agent sub tab displays 'You do not have permission to view details for this Agent.

Please find attached the screenshot.

The server log displays PermissionException. The server log says 'Can not get agent details - Subject[id=10011,name=test1] lacks MANAGE_SETTINGS for resource[id=10001]'.

Please refer the attached server log.

Steps to reproduce:
-Login as rhqadmin.
-Create a compatible group of platform.
-Create a user.
-Create a role with only 'MANAGE_INVENTORY' permissions.
-Assign the compatible group of platform and the user to the role created.
-Logout and login as the user.
-Navigate to the 'Inventory->Agent' sub tab of the platform.

3. User with both MANAGE_INVENTORY and MANAGE_SETTINGS permissions is able to see the agent sub tab contents.

Marking this as needinfo to confirm if user needs both MANAGE_INVENTORY and MANAGE_SETTINGS permissions to be able to see the agent sub tab contents.

Comment 6 Sunil Kondkar 2011-10-07 12:02:17 UTC
Created attachment 526877 [details]
Screenshot_Agent-tab

Comment 7 Sunil Kondkar 2011-10-07 12:02:53 UTC
Created attachment 526878 [details]
ServerLog

Comment 8 Mike Foley 2011-11-10 18:09:28 UTC
mazz ... can you respond to sunil's question at the bottom of comment #6?

Comment 9 John Mazzitelli 2011-11-13 14:06:49 UTC
(In reply to comment #8)
> mazz ... can you respond to sunil's question at the bottom of comment #6?

what question? comment #6 was just an attachment comment.

Comment 10 Mike Foley 2011-11-15 14:53:47 UTC
ah ... comment #5 ... my bad.

Marking this as needinfo to confirm if user needs both MANAGE_INVENTORY and
MANAGE_SETTINGS permissions to be able to see the agent sub tab contents.

Comment 11 Jay Shaughnessy 2011-11-15 20:05:35 UTC
To solve the immediate issue we changed the Inventory->Agent subtab (in
the gui) to require MANAGE_SETTINGS (as opposed to MANAGE_INVENTORY). This
makes the gui check and the server check look for the same perm and avoids
the issue of an inventory manager (but not a settings manager) having an
active subtab that then fails to pull the Agent data.

It's not totally clear which permission should protect
getAgentByResourceId() but MANAGE_SETTINGS seems reasonably appropriate
since the information we're trying to protect is Agent IP information,
naming, etc, basically information that a setting manager would typically
have access to.

In the future:
- We may want to also ensure the calling Subject "canView" the resource
specified.

- We may want to add explicit permission checks on
getAgentClient(subject, resourceId) and not just defer the perm check to
getAgentByResourceId(), because getAgentClient() arguable should require
MANAGE_INVENTORY in addition to MANAGE_SETTINGS (or maybe even superuser).

- We need to similarly figure out how to handle
pingAgentByResourceId(subject, resourceId), which (against
the inline docs) also defers to getAgentClient(subject, resourceId) for
permission handling.


Master commit 7bff6c7df2e5040feb4bf60028b63ffdd7adfc01

Comment 12 Jay Shaughnessy 2011-11-15 20:07:55 UTC
Oh, one more thing, when we actually make changes to the SLSB, we should
change the perm check in getAgentClient(subject, resourceId) to be
annnotation-based instead of using its own code.

Comment 13 John Mazzitelli 2011-11-15 20:11:24 UTC
agree - for now, assume SETTINGS is necessary only to view the agent info.

We should leave this issue open for further discussion for future release. Not sure SETTINGS is the perm we want - seems INVENTORY is the more appropriate one to use.

Comment 14 Jay Shaughnessy 2011-11-16 16:36:05 UTC
Release3.x commit b494ec2209ef0e6c141bacc54caf0fd7603da451

Test Notes:
Test with 2 users:
user 1: inventory manager only (not manage_settings)
  - Inventory->Agent subtab should be disabled

user 2: setting manager only (not manage_inventory)
  - Note, the user will need view access (via role+group) to the resource
  - Inventory->Agent subtab should be enabled
  - Agent data should be displayed as expected

Comment 15 Sunil Kondkar 2011-11-17 07:33:21 UTC
Verified on build#88 (Version: 4.2.0.JON300-SNAPSHOT Build Number: 292cf4c)

Verified with two users:
1. The Inventory->Agent subtab is disabled to a user with only MANAGE_INVENTORY permissions.
2. The Inventory->Agent subtab is enabled and agent data is displayed to a user with only MANAGE_SETTINGS permissions.

Comment 16 Mike Foley 2012-02-07 19:23:27 UTC
changing status of VERIFIED BZs for JON 2.4.2 and JON 3.0 to CLOSED/CURRENTRELEASE

Comment 17 Mike Foley 2012-02-07 19:26:01 UTC
marking VERIFIED JON 3 bugs to CLOSED/CURRENTRELEASE


Note You need to log in before you can comment on or make changes to this bug.