Bug 798465

Summary: ldap users unable to log into ui (regression caused by recent addition of new VIEW_USERS global permission)
Product: [Other] RHQ Project Reporter: Simeon Pinder <spinder>
Component: Core UIAssignee: Simeon Pinder <spinder>
Status: CLOSED CURRENTRELEASE QA Contact: Mike Foley <mfoley>
Severity: high Docs Contact:
Priority: urgent    
Version: 4.2CC: hrupp, jsanda, skondkar
Target Milestone: ---   
Target Release: RHQ 4.3.0   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2013-08-31 05:55:33 EDT Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
Bug Depends On:    
Bug Blocks: 786159    
Attachments:
Description Flags
patch that should fix this issue none

Description Simeon Pinder 2012-02-28 18:07:43 EST
Description of problem: With LDAP integration configured, ldap users are not able to log into RHQ.


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


How reproducible:
Every time.

Steps to Reproduce:
1. Setup LDAP integration. Just Auth is enough but happens with Authz as well.
2. Attempt to log into the server with user/pass combination for an ldap user that should be authenticated.
3. 
  
Actual results:
Login goes blank.

Expected results:
Display correct login screen.

Additional info:
From the browser debug logs the following is being thrown:
Tue Feb 28 17:39:27 GMT-500 2012 
WARNING: Problem querying subjects by criteria during loginStatus check.[1330468767039] javax.ejb.EJBTransactionRolledbackException:org.hibernate.TransientObjectException: object references an unsaved transient instance - save the transient instance before flushing: org.rhq.core.domain.auth.Subject -> java.lang.IllegalStateException:org.hibernate.TransientObjectException: object references an unsaved transient instance - save the transient instance before flushing: org.rhq.core.domain.auth.Subject -> org.hibernate.TransientObjectException:object references an unsaved transient instance - save the transient instance before flushing: org.rhq.core.domain.auth.Subject

For some reason the request to query logged in users fails for LDAP but not for non-ldap users.
Comment 1 John Sanda 2012-02-29 12:03:54 EST
I think it is highly unlikely that this issue is caused by the SmartGWT upgrade. The exception listed in the description is TransientObjectException which points to JPA/EJB code.
Comment 2 John Sanda 2012-02-29 22:08:28 EST
After a lot of debugging, I have confirmed that this issue is *not* due to the SmartGWT upgrade. This bug was introduced as a result of the work done for bug 786159. Specifically, the following two commits to the master branch caused this issue:

e2bbfdf06987dd1de0dcedac98f69116f21a24ae
ffad3bbe5d11cf62073fe02f67d2cc45010ec98f

When an LDAP user attempts to log into RHQ for the first time, he has no corresponding row in the rhq_subject table. During the login process, we first authenticate against LDAP, and if it is the first login, a row is inserted into rhq_subject for that user. This happens as part of the registration process LDAP users go through during their first login. 

The problem starts in UserSessionManager.checkLoginStatus at line 261. We make call to SubjectGWTService.findSubjectsByCriteria which in turn delegates to SubjectManagerBean.findSubjectsByCriteria. The first parameter to this method is a Subject, and the newly created Subject (for the LDAP user) is passed as the argument. At this point the Subject has been created but not yet persisted. In the two commits referenced above, findSubjectsByCriteria was modified such that it now performs some authorization checks to see whether or not the subject has MANAGED_SECURITY or VIEW_USERS permissions. Those authorization checks are done with a JPQL query where the subject is one of the query parameters. If a query parameter is an entity, it cannot be transient; in other words, it must exist in the database.

Because of the combination of passing a transient subject into findSubjectsByCriteria and modifying the method to do the authorizations checks as they do, a TransientObjectException is thrown, preventing the initial login for the LDAP user.

I am going to make this a blocker for 786159 since this bug was introduced as part of that work. I am also reassigning to Ian Springer since he worked on that bug and may be in the best position to decide on the most appropriate course of action.

One other thing I want to mention is that even though the initial LDAP login worked prior to the two commits referenced above, we shouldn't be passing a transient Subject object to findSubjectsByCriteria. In virtually every method that takes a Subject argument, outside of those involved with creating one, it is assumed that the subject exists in the database.
Comment 3 Ian Springer 2012-02-29 23:50:09 EST
Created attachment 566739 [details]
patch that should fix this issue
Comment 4 Ian Springer 2012-03-01 00:00:07 EST
> One other thing I want to mention is that even though 
> the initial LDAP login worked prior to the two commits 
> referenced above, we shouldn't be passing a transient 
> Subject object to findSubjectsByCriteria. In virtually
> every method that takes a Subject argument, outside of
> those involved with creating one, it is assumed that 
> the subject exists in the database.

I agree. SubjectManagerBean.findSubjectsByCriteria() is 
a remote API method. Like all other remote API methods, 
the subject parameter is expected to be an authenticated
subject that can be used for authorization checks. If 
anything, we could have passed in overlord, rather than 
a dummy subject, as the subject in this particular call, 
but that would not be great either, since it would allow
GUI users to gain access to other subjects, including 
not only their basic info, but also their assigned roles.

In the attached patch, I simplified the logic in 
UserSessionManager.checkLoginStatus() so that it doesn't
even need to make a call to findSubjectsByCriteria().
I have not tested this, but I think it should work.
Simeon, would you please review the patch, and if it 
looks good, test it out?
Comment 5 Simeon Pinder 2012-03-01 12:26:43 EST
There does look like some consolidation that can be done here but the refactor that you proposed still creates a few different problems with registration. This will require some more effort to get right.

The crux of this issue is that LDAP users i)requiring registration and ii)coming in via case-insensitive usernames are indistinguishable until we do a Subject query.  Before the permissions change there was enough information to keep this as a client side only decision.  Now at the same decision point in the login process there is no persistent RHQ subject that can be used. If i) we need to launch ui and create one and if ii)there is a matching(case-insensitive) Subject already but we can't find out it's username because we don't have the permissions or a persistent subject to make the query with.  

As mentioned, using overlord here creates several security issues to from the client side and isn't the right solution either.  

It looks like the only thing to be done here is to rework the existing/working login process to support this new path.  This may time some time to get right.
Comment 6 Simeon Pinder 2012-03-07 18:46:18 EST
Worked with Ian to refactor the client side and server side code to move the Subject query logic back into SubjectManagerBean.

This is fixed with commit a8db366376a and available to test in master. 

Moving to ON_QA.
Comment 7 Sunil Kondkar 2012-04-25 09:09:11 EDT
Verified on build#1399 (Version: 4.4.0-SNAPSHOT Build Number: c87d039)

Verified LDAP login, authorization and authentication with case-sensitive/insensitive user-names.

For new ldap users, registration screen appears after login and for registered users, it is taken to the UI.

Verified with/without SSL support on Red Hat directory server 8.2.0 and Windows Server 2003 active directory.