Bug 846451 - Users with just Resource Group access cannot access alerts on their Groups
Summary: Users with just Resource Group access cannot access alerts on their Groups
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: JBoss Operations Network
Classification: JBoss
Component: Resource Grouping
Version: JON 3.1.1
Hardware: All
OS: All
unspecified
medium
Target Milestone: ER07
: JON 3.2.0
Assignee: Jay Shaughnessy
QA Contact: Mike Foley
URL:
Whiteboard:
: 871073 (view as bug list)
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2012-08-07 20:21 UTC by Richard Hensman
Modified: 2014-01-02 20:35 UTC (History)
7 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2014-01-02 20:35:15 UTC
Type: Bug
Embargoed:


Attachments (Terms of Use)
patch to resolve 846581 plus one other issue (8.11 KB, patch)
2013-03-27 20:34 UTC, Mark Addy
no flags Details | Diff
alert def on group is visible (96.92 KB, image/jpeg)
2013-06-18 15:15 UTC, vlad crc
no flags Details
alert def on group details are empty (66.68 KB, image/jpeg)
2013-06-18 15:16 UTC, vlad crc
no flags Details
alert-definition (60.51 KB, image/jpeg)
2013-09-26 13:20 UTC, Jeeva Kandasamy
no flags Details

Description Richard Hensman 2012-08-07 20:21:43 UTC
Description of problem:

If you create an Alert Definition via the GUI on a ResourceGroup you have access to, but you do not have any Global Permissions you cannot then access the Alert Definition once created

Version-Release number of selected component (if applicable):

4.4

How reproducible:


Steps to Reproduce:
1) Create a Compatible Group (TestGroup)
2) Add a platform to TestGroup
3) Create a role (TestRole) with the Manage Alerts (read/write) Resource Permission and TestGroup as an Assigned Resource Group 
4) Create a user (testuser) with TestRole assigned
5) Login as testuser
6) Open TestGroup
7) Create a group level alert
8) Go to back to definitions and the alert definition is not displayed
9) Log back in as an admin user and the alert displays fine

  
Actual results:

Error banner displayed: Failed to fetch alert definition data

Expected results:

List Definitions

Additional info:

Comment 1 Charles Crouch 2012-11-19 19:31:36 UTC
Unsetting priority for jon312 triage

Comment 2 vlad crc 2013-03-27 08:28:46 UTC
*** Bug 871073 has been marked as a duplicate of this bug. ***

Comment 3 Mark Addy 2013-03-27 20:32:23 UTC
I've attached a patch that addresses two issues.

Firstly the issue described in 846451 is caused by the relationship between AlertDefinition and ResourceGroup.  AlertDefinition maps the relationship with a property "resourceGroup" which causes the CriteriaQueryGenerator to create an incorrect query when attempting to restrict results based on authorization.  The CriteriaQueryGenerator expects this relationship to be defined by a property named "group".  A work around has been submitted to avoid refactoring AlertDefinition and the database schema.


The second fix covers the issue described in the post https://lists.fedorahosted.org/pipermail/rhq-devel/2013-March/002676.html.

"From the main dashboard, or any view not associated to a group or 
resource the only users capable of acknowledging or deleting alerts are 
those with global permissions.  The effect is that users with specific 
role permissions to manage alerts against a resource group must drill 
into the heirarchy before they can ack or delete those alerts."

The fix affects only those users without MANAGE_INVENTORY permissions and only when they are viewing Alerts from views not associated directly with a resource or group.  In these instance selection of an Alert row in the view triggers a call to the authorization service to check if they have a MANAGE_ALERTS permission for the associated resource.

Comment 4 Mark Addy 2013-03-27 20:34:28 UTC
Created attachment 717281 [details]
patch to resolve 846581 plus one other issue

Comment 5 Jay Shaughnessy 2013-04-03 13:02:31 UTC
Mark, thanks for the detective work and submission.  

Problem 1, the AlertDefinition issue: The analysis is spot on, the entity field does not have standard "group" field name and instead has "resourceGroup". Your fix would work but I think I prefer the alternative you touched on, which is to just rename the entity field.  It's a private field and changing it will not break public API. We can deprecate the public setter/getter and add a new getter/setter for the renamed field.  Instead of adding special-case support for the misnamed entity field, let's make it conform.

Problem 2, the portlet "Ack"/"Delete" issue:  You're right about this as well.  We had originally made the conscious decision to not allow this due to the inefficient nature of the callback.  

Since you present a real need, and I'm not sure it will actually be a problem, I'll apply your change.  If we run into issues we can work on the implementation, using composites, or possibly make it optional based on maybe a portlet configuration setting.  

I made one small optimization in ResourceAuthorizedTableAction.  If constructed with a requiredPermission=null it will ignore the authz check. Using that we can make it perform like a standard table action. So, if in fact the user is an admin or otherwise has write access then it should act as before.  I took away the isEnabled() overrides in your patch in favor of this approach.  The overrides had a small issue because they did not consider "enablement". Meaning the buttons would be active (for admins) even if nothing was selected.

I'll be working this into master soon...

Comment 6 Jay Shaughnessy 2013-04-06 01:29:23 UTC
master commit dc97319555c5760bfc97662c2d2108c58e03269f
Author: Jay Shaughnessy <jshaughn>
Date:   Fri Apr 5 16:14:33 2013 -0400

Change AlertDefinition.resourceGroup to AlertDefinition.group.  This brings
the name into the convention we use everywhere else, and that the CriteriaGenerator expects for group FK relations.
    - Update all relevant queries and AlertDefinitionCriteria
    - For back compat, keep existing setter/getter and deprecate. Refactor all
      internal code to use the new setter/getter.


See Bug 949082 for the Ack/Delete issue.

Comment 7 vlad crc 2013-05-17 09:27:46 UTC
Hello,

Is this fix included in RHQ 4.7 ?

Thank you,
Vlad

Comment 8 vlad crc 2013-06-18 15:00:37 UTC
In RHQ 4.7 a user with normal privileges on a group (that means without any global privilege) is able to create an alert definition on a group, but then if he tries to view the definition the details are empty, see the screenshots.

Comment 9 vlad crc 2013-06-18 15:15:26 UTC
Created attachment 762536 [details]
alert def on group is visible

Comment 10 vlad crc 2013-06-18 15:16:02 UTC
Created attachment 762538 [details]
alert def on group details are empty

Comment 11 Larry O'Leary 2013-09-06 14:31:49 UTC
As this is MODIFIED or ON_QA, setting milestone to ER1.

Comment 12 Jeeva Kandasamy 2013-09-26 13:20:25 UTC
Created attachment 803399 [details]
alert-definition

Version: 3.2.0.ER1
Build Number: 54dd29c:464a643
GWT Version: 2.5.0
SmartGWT Version: 3.0

Able to create alert definition, however if we click that alert, it is showing empty details. screen shot is attached.

I had set "Disable When Fired :	Yes", once the alert fired it should be disabled, but it was showing as enabled. When I click alert definition link via "alert history", it's showing the alert definition properly and immediately definition shows in disabled state. From this point I can able to view/edit alert definition via "Alert definition" page.

Comment 13 Jay Shaughnessy 2013-11-11 16:12:18 UTC
The problem above is actually not the problem in the BZ, but it is certainly a severe problem.  Looking into it...

Comment 14 Jay Shaughnessy 2013-11-11 22:07:31 UTC
release/jon3.2.x commit 48116dec36c9b417d16e48cda74fa42381527b9c
Author: Jay Shaughnessy <jshaughn>
Date:   Mon Nov 11 17:06:06 2013 -0500

    A tangential but important fix to seeing the group alert def details when
    not logged in as an admin user.

    Cherry-Pick Master: e5faf67f15ead7db09ae2503082de846e72e8e61

Comment 15 Simeon Pinder 2013-11-19 15:47:28 UTC
Moving to ON_QA as available for testing with new brew build.

Comment 16 Simeon Pinder 2013-11-22 05:12:59 UTC
Mass moving all of these from ER6 to target milestone ER07 since the ER6 build was bad and QE was halted for the same reason.

Comment 17 Filip Brychta 2013-12-11 16:25:36 UTC
Verified on:
Version :	
3.2.0.GA
Build Number :	
7b00246:6d13523

Verified scenarios from description and from comment 12. Problem described in second part of comment 12 is still there. I will create a separate bz for this.


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