| 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 UI | Assignee: | Simeon Pinder <spinder> | ||||
| Status: | CLOSED CURRENTRELEASE | QA Contact: | Mike Foley <mfoley> | ||||
| Severity: | high | Docs Contact: | |||||
| Priority: | urgent | ||||||
| Version: | 4.2 | CC: | 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 09:55:33 UTC | Type: | --- | ||||
| Regression: | --- | Mount Type: | --- | ||||
| Documentation: | --- | CRM: | |||||
| Verified Versions: | Category: | --- | |||||
| oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |||||
| Cloudforms Team: | --- | Target Upstream Version: | |||||
| Bug Depends On: | |||||||
| Bug Blocks: | 786159 | ||||||
| Attachments: |
|
||||||
|
Description
Simeon Pinder
2012-02-28 23:07:43 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. 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. Created attachment 566739 [details]
patch that should fix this issue
> 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?
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. 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. 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. |