Bug 798465 - ldap users unable to log into ui (regression caused by recent addition of new VIEW_USERS global permission)
Summary: ldap users unable to log into ui (regression caused by recent addition of new...
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: RHQ Project
Classification: Other
Component: Core UI
Version: 4.2
Hardware: Unspecified
OS: Unspecified
urgent
high
Target Milestone: ---
: RHQ 4.3.0
Assignee: Simeon Pinder
QA Contact: Mike Foley
URL:
Whiteboard:
Depends On:
Blocks: 786159
TreeView+ depends on / blocked
 
Reported: 2012-02-28 23:07 UTC by Simeon Pinder
Modified: 2013-08-31 09:55 UTC (History)
3 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2013-08-31 09:55:33 UTC
Embargoed:


Attachments (Terms of Use)
patch that should fix this issue (8.51 KB, patch)
2012-03-01 04:50 UTC, Ian Springer
no flags Details | Diff

Description Simeon Pinder 2012-02-28 23:07:43 UTC
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 17:03:54 UTC
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-03-01 03:08:28 UTC
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-03-01 04:50:09 UTC
Created attachment 566739 [details]
patch that should fix this issue

Comment 4 Ian Springer 2012-03-01 05:00:07 UTC
> 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 17:26:43 UTC
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 23:46:18 UTC
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 13:09:11 UTC
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.


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