Bug 622491

Summary: deleting a user caused user's modified groups/manually added resources to be inaccessible
Product: [Other] RHQ Project Reporter: John Mazzitelli <mazz>
Component: No ComponentAssignee: RHQ Project Maintainer <rhq-maint>
Status: CLOSED CURRENTRELEASE QA Contact: Mike Foley <mfoley>
Severity: medium Docs Contact:
Priority: high    
Version: 4.0.0CC: ccrouch, cwelton, hrupp, jshaughn, rtimaniy, sdharane
Target Milestone: ---   
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
: 655156 (view as bug list) Environment:
Last Closed: 2013-09-02 07:24:58 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On:    
Bug Blocks: 625146, 655156    

Description John Mazzitelli 2010-08-09 14:11:54 UTC
When we delete a user, the rhq_resource_group.modified_by no longer points to a valid user and anytime we try to access it, we get errors.

The modified_by is a subject ID but is not a real foreign key. We need to either:

a) add a foreign key constraint to that column so you can't delete a user unless you change or remove that row (change it to what? rhqadmin user ID???)

or

b) change the modified_by from ID to VARCHAR and replace with the user names (like we do everywhere else, unsure why we bother with subject ID here, all our other modified bys or audit columns use username for this very reason).

unsure how we easily do either one.

Comment 1 Jay Shaughnessy 2010-08-09 14:26:25 UTC
It looks like this may also be an issue for manually added resources.  RHQ_RESOURCE.MODIFIED_BY is declared similarly.  It's set to overlord for all autodiscovered resources but set to the current user for manually added resources. It is not changed if the user is removed.

Comment 2 John Mazzitelli 2010-08-09 14:29:22 UTC
Joe has a good way to update the DB to support my b) suggestion:

1) change hibernate mapping in abstract Group class from Subject to String, and update calling references accordingly in ResourceGroupManager.create/update (and any other places they occur)
2) db-upgrade tasks
2a) add new "modified_by_str" column
2b) a single insert statement to fill that new column with data correlated by the subject_id in the existing "modified_by" column (where subject_id exists in rhq_subject)
2c) delete old "modified_by" column
2d) rename "modified_by_str" to "modified_by" 

Need to do something similar for RHQ_RESOURCE.modified_by

Comment 3 Joseph Marques 2010-08-09 14:55:11 UTC
in the places where we reference subjects for audit purposes, we seem to have a rather consistent convention of:

"subject_name varchar(100) not null"

i suggest that we fix the above two issues (rhq_resource.modified_by) and (rhq_resource_group.modified_by) to use the same convention.

-----

took a look at the rest of latest rhq-4.0.0 schema, and here are some other interesting points:

"rhq_installed_packages.subject_id" has FK, is nullable, not set in the InstalledPackage entity class constructor, and there are no existing calls to setUser(Subject) after construction...does anyone know what this was supposed to represent?
"rhq_partition_event.subject_name" is size 255 but convention everywhere else is 100, rhq_subject.name is only 100 so this column should be altered to be smaller (this is a minor issue)

Comment 4 Joseph Marques 2010-08-09 14:58:03 UTC
to be clear, my inspection of the latest schema verified that the only issues relating to this bug are the two columns found by mazz/jshaughn, which are the non-FK subject references in rhq_resource and rhq_resource_group

Comment 5 John Mazzitelli 2010-08-09 17:13:10 UTC
1) start server and agent
2) log in as "rhqadmin" and import the platform into inventory (don't need any other servers imported, if all you have is the platform, that's fine)
3) create a user "doomed", and assign it to the "Super User Role" essentially giving the user all permissions
4) log out as rhqadmin and log in as the new user "doomed"
5) now that you are logged in as user "doomed", create a new group and put the platform you inventoried in step 2 into that group
6) go to the platform that you inventoried in step 2 and manually add a resource to it (I just manually created a "Script Server", the only connection property you need to give is an executable name, like "/usr/bin/ls")
7) log out as doomed and re-login as "rhqadmin"
8) confirm that you can still view the group and manually-added resource (this is just a sanity check)
9) delete the "doomed" user
10) try to navigate to the group page again... you get this:
    EntityNotFoundException
	     Unable to find org.rhq.core.domain.auth.Subject with id 10001

    /rhq/group/layout/main.xhtml @33,137 template="${(ResourceGroupUIBean.compatible) ? '/rhq/entity/layout/main.xhtml' : '/rhq/entity/layout/main-wide.xhtml'}" Cant instantiate class: org.rhq.enterprise.gui.inventory.group.ResourceGroupUIBean.

    org.rhq.enterprise.server.resource.group.ResourceGroupManagerBean.getResourceGroupComposite(ResourceGroupManagerBean.java:1350) 

11) navigating to the manually-added resource page works fine - we actually never display the modified_by field. But, go to the admin/test/sql.jsp page and do this:
 
   select name, modified_by from rhq_resource where modified_by not in (select id from rhq_subject)

and you'll see the ID and name of the resource that doomed created. You'll also see the subject ID in modified_by that was doomed (it is an ID that no longer exists in the RHQ_SUBJECT table)

Comment 6 John Mazzitelli 2010-08-09 17:19:16 UTC
these are two SQL you can perform (in admin/test/sql.jsp for example) - these will return 1 or more rows if you have the bug:

select id, name, modified_by from rhq_resource_group where modified_by not in (select id from rhq_subject);

select id, name, modified_by from rhq_resource where modified_by not in (select id from rhq_subject);

As a quick workaround, you can perform a SQL UPDATE (again, you can use admin/test/sql.jps for this, or any DB client you want) to change the modified_by to a valid subject ID. "rhqadmin" is the admin user that almost always exists, so you can do this to quickly workaround the issue (note: you can replace "rhqadmin" with a username that you want to assign the group and/or manually added resources to, if you don't want them to go to rhqadmin)

update rhq_resource_group set modified_by = (select id from rhq_subject where name='rhqadmin') where modified_by not in (select id from rhq_subject);

update rhq_resource set modified_by = (select id from rhq_subject where name='rhqadmin') where modified_by not in (select id from rhq_subject);

Comment 7 John Mazzitelli 2010-08-09 21:21:10 UTC
fix has been committed to master branch - commit: f6409557d36ab77f8baa69cb4dc4d11c5fedc39c

we should not have been storing subject ID in modifiedBy columns in RHQ_RESOURCE and RHQ_RESOURCE_GROUP. We now only store the name and our dbupgrade script will convert existing data so only subject names are stored in modifiedBy as a VARCHAR.

Comment 8 John Mazzitelli 2010-08-10 14:03:18 UTC
i'm gonna tweek the db-upgrade script to be more efficient

Comment 9 John Mazzitelli 2010-08-10 14:22:16 UTC
master commit 7a1accc180497bc215e3e416a0f19c74f6dc1647

made the db-upgrade more efficient by utilizing direct SQL to perform an actual column rename

Comment 10 Sudhir D 2010-10-18 11:17:31 UTC
Tested with rhq-server-4.0.0-SNAPSHOT build# 419 for steps mentioned in Comment #5. No exception is thrown @ step 10. I don't see subject ID either for doomed.

Marking this bug as verified.

Comment 11 Sudhir D 2010-11-12 10:43:13 UTC
Tested on the JON 241 jon-server-2.4.1-SNAPSHOT build# f188170. 

I'm able to reproduce the issue mentioned in comment# 5 @ Setp 10. I get the below exception,
EntityNotFoundException
	Unable to find org.rhq.core.domain.auth.Subject with id 10001

I'm pasting the stack trace snippet below, 
/rhq/group/layout/main.xhtml @33,137 template="${(ResourceGroupUIBean.compatible) ? '/rhq/entity/layout/main.xhtml' : '/rhq/entity/layout/main-wide.xhtml'}" Cant instantiate class: org.rhq.enterprise.gui.inventory.group.ResourceGroupUIBean.

    com.sun.facelets.tag.TagAttributeException: /rhq/group/layout/main.xhtml @33,137 template="${(ResourceGroupUIBean.compatible) ? '/rhq/entity/layout/main.xhtml' : '/rhq/entity/layout/main-wide.xhtml'}" Cant instantiate class: org.rhq.enterprise.gui.inventory.group.ResourceGroupUIBean. at com.sun.facelets.tag.TagAttribute.getObject(TagAttribute.java:235) at com.sun.facelets.tag.TagAttribute.getValue(TagAttribute.java:200) at com.sun.facelets.tag.ui.CompositionHandler.apply(CompositionHandler.java:113) at com.sun.facelets.compiler.NamespaceHandler.apply(NamespaceHandler.java:49) at com.sun.facelets.compiler.EncodingHandler.apply(EncodingHandler.java:25) at com.sun.facelets.impl.DefaultFacelet.include(DefaultFacelet.java:248) at com.sun.facelets.impl.DefaultFacelet.include(DefaultFacelet.java:294) at com.sun.facelets.impl.DefaultFacelet.include(DefaultFacelet.java:273) at com.sun.facelets.impl.DefaultFaceletContext.includeFacelet(DefaultFaceletContext.java:140) at com.sun.facelets.tag.ui.CompositionHandler.apply(CompositionHandler.java:113) at com.sun.facelets.compiler.NamespaceHandler.apply(NamespaceHandler.java:49) at com.sun.facelets.compiler.EncodingHandler.apply(EncodingHandler.java:25) at com.sun.facelets.impl.DefaultFacelet.apply(DefaultFacelet.java:95) at com.sun.facelets.FaceletViewHandler.buildView(FaceletViewHandler.java:524) at com.sun.facelets.FaceletViewHandler.renderView(FaceletViewHandler.java:567) at org.rhq.enterprise.gui.common.framework.FaceletRedirectionViewHandler.renderView(FaceletRedirectionViewHandler.java:64) at org.ajax4jsf.application.ViewHandlerWrapper.renderView(ViewHandlerWrapper.java:100) at 

Reopening the bug.

Comment 12 John Mazzitelli 2010-11-12 13:41:19 UTC
this is fixed in master branch - was not fixed in release-3.0.0 branch - hence why any testing you do on jon-2.4.1 will still fail.

Comment 14 Corey Welton 2011-01-05 14:52:51 UTC
QA Verified.  I have tested this across compatible, mixed, and dynagroups.  After creating all such groups with the 'doomed' user and subsequently deleting said user, group(s) can be accessed.

Comment 15 Heiko W. Rupp 2013-09-02 07:24:58 UTC
Bulk closing of issues that were VERIFIED, had no target release and where the status changed more than a year ago.