Hi Senaka,
On Sat, Dec 15, 2012 at 10:40 PM, Senaka Fernando <[email protected]> wrote:
> Hi all,
>
> So, I debugged this and here's the scenario. The e-mail is a bit lengthy
> but it explains the root cause of this, :).
>
> *Normal Login Sequence, without Basic Auth Headers*
>
> 1. Login Request Comes in.
> 2. Hits the CarbonContextCreator valve, which sets the details to
> Thread Local CarbonContext (now the authenticated session has not been
> created so it sets username null).
> 3. Authentication happens, which updates session.
> 4. Subsequent requests are made using the session, which user separate
> threads.
> 5. Hits the CarbonContextCreator valve, which sets the details to
> Thread Local CarbonContext (now the authenticated session has already been
> created so it reads proper username).
> 6. Hits AbstractAdmin which uses CarbonContext.getCurrentContext()
> 7. Returns Context with proper values.
>
> *Login with Basic Auth Sequence.*
>
> 1. Login Request Comes in.
> 2. Hits the CarbonContextCreator valve, which sets the details to
> Thread Local CarbonContext (now the authenticated session has not been
> created so it sets username null).
> 3. Authentication happens, which updates session.
> 4. Hits AbstractAdmin which uses CarbonContext.getCurrentContext()
> 5. Returns Context with *wrong* values.
>
> Now, as you can see, the wrong values are returned because
> the CarbonContextCreator valve was not hit after a valid session was
> available. If you take a look @ how the CarbonContext.getCurrentContext()
> works,
>
> public static CarbonContextDataHolder getCurrentCarbonContextHolder() {
> //1. Check ThreadLocalCarbonContextHolder
> CarbonContextDataHolder currentCarbonContext =
> getThreadLocalCarbonContextHolder();
> if (null == currentCarbonContext){
> try {
> //2. Check MessageContext
> MessageContext messageContext =
> MessageContext.getCurrentMessageContext();
> if (messageContext != null) {
> currentCarbonContext =
> getCurrentCarbonContextHolder(messageContext);
> } else {
> //3. Check ConfigurationContext
> currentCarbonContext =
> getCurrentCarbonContextHolder((ConfigurationContext) null);
> }
> } catch (NullPointerException ignore) {
> currentCarbonContext = null;
> } catch (NoClassDefFoundError ignore) {
> currentCarbonContext = null;
> }
> }
> return currentCarbonContext;
> }
>
> You'll understand that it,
>
> 1. First checks the thread
> 2. Then checks the message context
> 3. And finally the configuration context
>
> If you expand on step #2, you'll find that it will utilize the context on
> the session when it checks the message context. Therefore, in the second
> sequence, if you did not have a valve setting anything to the thread, it
> will actually pick the one on session, which will in return mean,
> AbstractAdmin would actually see the context with proper values.
>
> Now, this explains that this interesting situation is a regression of the
> fix introduced by the CarbonContextCreator valve modification. Now, looking
> back into the reasons of why the CarbonContextCreator valve modification
> was needed, you will understand that by doing so, it addressed issues of
> having null values on the context returned
> by CarbonContext.getCurrentContext(). Digging deeper into why this
> happened, I was able to find the older code of this to be:
>
> public static CarbonContextDataHolder getCurrentCarbonContextHolder() {
> try {
> MessageContext messageContext =
> MessageContext.getCurrentMessageContext();
> if (messageContext != null) {
> return getCurrentCarbonContextHolder(messageContext);
> } else {
> return
> getCurrentCarbonContextHolder((ConfigurationContext) null);
> }
> } catch (NullPointerException ignore) {
> // This is thrown when the message context is not initialized
> // So return the Threadlocal
> return getThreadLocalCarbonContextHolder();
> } catch (NoClassDefFoundError ignore) {
> // There can be situations where the CarbonContext is
> accessed, when there is no Axis2
> // library on the classpath.
> return getThreadLocalCarbonContextHolder();
> }
> }
>
> You'll understand that it,
>
> 1. First checks the message context
> 2. Then checks the configuration context
> 3. And finally the thread
>
> Therefore, the actual cause of what CarbonContextCreator valve
> modification tried to fix was the change of this sequence. The current
> sequence assumes that "*null == currentCarbonContext*" will happen at
> times, when the CarbonContext should not exist on the thread, which will
> then return the one on the MessageContext or the ConfigurationContext. But,
> as it turns out to be, that condition is *always false*. This is because
> the initialValue of the thread local variable ensures that this will never
> be null.
>
>
I observed the behavior. When the older code is used it works fine.
> So, in summary, as a solution to this, I believe that it is best to revert
> the change to the sequencing of getCurrentCarbonContextHolder and revert
> back the changes done in the valve, which should solve most issues.
> However, the change of sequencing AFAIU, must have been done for a
> performance improvement. But, for that, we will have to come up with a
> proper implementation that does not lead into these issues.
>
>
+1. Can we revert the change please? Is there any objections for reverting?
thanks,
dimuthu
> Thanks,
> Senaka.
>
> On Sat, Dec 15, 2012 at 9:36 PM, Dimuthu Leelarathne <[email protected]>wrote:
>
>> Hi Senaka,
>>
>> You can reproduce the issue using SOAP UI, without any code. I try
>> calling add method of GenericArtifact Admin method.
>>
>> thanks,
>> dimuthu
>>
>> On Sat, Dec 15, 2012 at 7:44 PM, Senaka Fernando <[email protected]> wrote:
>>
>>> Hi all,
>>>
>>> I reproduced this issue. When we log in with Basic Auth Credentials, the
>>> CarbonContext does not have the Username as it seems. Shariq can you look
>>> into this?
>>>
>>> Here is a basic code snippet to try this out if you have G-Reg.
>>>
>>> import
>>> org.wso2.carbon.automation.api.clients.registry.SearchAdminServiceClient;
>>> import
>>> org.wso2.carbon.registry.search.metadata.test.bean.SearchParameterBean;
>>> import org.wso2.carbon.registry.search.stub.beans.xsd.ArrayOfString;
>>> import
>>> org.wso2.carbon.registry.search.stub.beans.xsd.CustomSearchParameterBean;
>>> import java.io.File;
>>>
>>> public static void main(String[] args) throws Exception {
>>> System.setProperty("javax.net.ssl.trustStore",
>>> "D:\\Work\\Downloads\\wso2greg-4.5.3" + File.separator + "repository" +
>>> File.separator + "resources" + File.separator +
>>> "security" + File.separator +
>>> "wso2carbon.jks");
>>> System.setProperty("javax.net.ssl.trustStorePassword",
>>> "wso2carbon");
>>> System.setProperty("javax.net.ssl.trustStoreType", "JKS");
>>> SearchAdminServiceClient searchAdminServiceClient =
>>> new SearchAdminServiceClient("
>>> https://localhost:9443/services/";,
>>> "admin", "admin");
>>> CustomSearchParameterBean searchQuery = new
>>> CustomSearchParameterBean();
>>> SearchParameterBean paramBean = new SearchParameterBean();
>>> paramBean.setResourceName("testService");
>>>
>>> ArrayOfString[] paramList = paramBean.getParameterList();
>>>
>>> searchQuery.setParameterValues(paramList);
>>> searchAdminServiceClient.saveAdvancedSearchFilter(searchQuery,
>>> "boo");
>>> }
>>>
>>> On Sat, Dec 15, 2012 at 1:16 PM, Senaka Fernando <[email protected]>wrote:
>>>
>>>> Hi Shariq,
>>>>
>>>> On Sat, Dec 15, 2012 at 12:07 PM, Muhammed Shariq <[email protected]>wrote:
>>>>
>>>>> On Fri, Dec 14, 2012 at 4:16 PM, Dimuthu Leelarathne <
>>>>> [email protected]> wrote:
>>>>>
>>>>>> Hi Senaka,
>>>>>>
>>>>>> Username in Carbon Context is null. I am debugging more to find out
>>>>>> how the username in CC got set to null.
>>>>>>
>>>>>
>>>>> We fixed a similar issue recently. Recently Sinthuja did a fix to give
>>>>> priority to the ThreadLocal CC instead of MessageContext. The CC was not
>>>>> getting initialized properly for the login request cz the TomcatValve
>>>>> wasn't initializing the tenant's CC, reason for that is the login request
>>>>> (/carbon/admin/login_action.jsp) doesn't have tenant info ..
>>>>>
>>>>> We fixed this by using current MessageContext to populate the CC for
>>>>> post login operations (fix in LoggedUserInfoAdmin) .. diff is as
>>>>> follows;
>>>>>
>>>>> - UserRealm userRealm = getUserRealm();
>>>>> + UserRealm userRealm = (UserRealm) PrivilegedCarbonContext.
>>>>> getCurrentContext(messageContext).getUserRealm();
>>>>>
>>>>
>>>> That's a hack.
>>>>
>>>>>
>>>>> You could try this fix just to verify if the issue ur facing has
>>>>> anything to do with CC hierarchy ...
>>>>>
>>>>> Also I changed the tomcat valve to set the username when it gets hit
>>>>> .. but in the CC class, it doesn't try to fetch the username if its null,
>>>>> it merely does a return expecting upstream code set it .. which is not
>>>>> happening ..
>>>>>
>>>>
>>>> That has to happen. We need to locate this and fix it. It should be
>>>> some combination of calls which ends up leading into this.
>>>>
>>>> Thanks,
>>>> Senaka.
>>>>
>>>>>
>>>>>
>>>>>> thanks,
>>>>>> dimuthu
>>>>>>
>>>>>>
>>>>>> On Fri, Dec 14, 2012 at 3:49 PM, Senaka Fernando <[email protected]>wrote:
>>>>>>
>>>>>>> Hi Dimuthu,
>>>>>>>
>>>>>>> What's null? Based on that, please check back the stacktrace to see
>>>>>>> how that value is obtained and passed into UM - because IIRC we don't
>>>>>>> construct anything about users and/or permissions within the registry
>>>>>>> kernel. And, from what I understand it seems that the CCtx does not
>>>>>>> seem to
>>>>>>> have the proper value of something.
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Senaka.
>>>>>>>
>>>>>>> On Fri, Dec 14, 2012 at 2:57 PM, Dimuthu Leelarathne <
>>>>>>> [email protected]> wrote:
>>>>>>>
>>>>>>>> ManageGenericArtifactService
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> --
>>>>>>> * <http://wso2con.com/>
>>>>>>> *
>>>>>>> *
>>>>>>>
>>>>>>> Senaka Fernando*
>>>>>>> Member - Integration Technologies Management Committee;
>>>>>>> Technical Lead; WSO2 Inc.; http://wso2.com*
>>>>>>> Member; Apache Software Foundation; http://apache.org
>>>>>>>
>>>>>>> E-mail: senaka AT wso2.com
>>>>>>> **P: +1 408 754 7388; ext: 51736*; *M: +94 77 322 1818
>>>>>>> Linked-In: http://linkedin.com/in/senakafernando
>>>>>>>
>>>>>>> *Lean . Enterprise . Middleware
>>>>>>>
>>>>>>>
>>>>>>
>>>>>> _______________________________________________
>>>>>> Dev mailing list
>>>>>> [email protected]
>>>>>> http://wso2.org/cgi-bin/mailman/listinfo/dev
>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> Thanks,
>>>>> Shariq.
>>>>> Phone: +94 777 202 225
>>>>>
>>>>>
>>>>
>>>>
>>>> --
>>>> * <http://wso2con.com/>
>>>> *
>>>> *
>>>>
>>>> Senaka Fernando*
>>>> Member - Integration Technologies Management Committee;
>>>> Technical Lead; WSO2 Inc.; http://wso2.com*
>>>> Member; Apache Software Foundation; http://apache.org
>>>>
>>>> E-mail: senaka AT wso2.com
>>>> **P: +1 408 754 7388; ext: 51736*; *M: +94 77 322 1818
>>>> Linked-In: http://linkedin.com/in/senakafernando
>>>>
>>>> *Lean . Enterprise . Middleware
>>>>
>>>>
>>>
>>>
>>> --
>>> * <http://wso2con.com/>
>>> *
>>> *
>>>
>>> Senaka Fernando*
>>> Member - Integration Technologies Management Committee;
>>> Technical Lead; WSO2 Inc.; http://wso2.com*
>>> Member; Apache Software Foundation; http://apache.org
>>>
>>> E-mail: senaka AT wso2.com
>>> **P: +1 408 754 7388; ext: 51736*; *M: +94 77 322 1818
>>> Linked-In: http://linkedin.com/in/senakafernando
>>>
>>> *Lean . Enterprise . Middleware
>>>
>>>
>>
>
>
> --
> * <http://wso2con.com/>
> *
> *
>
> Senaka Fernando*
> Member - Integration Technologies Management Committee;
> Technical Lead; WSO2 Inc.; http://wso2.com*
> Member; Apache Software Foundation; http://apache.org
>
> E-mail: senaka AT wso2.com
> **P: +1 408 754 7388; ext: 51736*; *M: +94 77 322 1818
> Linked-In: http://linkedin.com/in/senakafernando
>
> *Lean . Enterprise . Middleware
>
>
_______________________________________________
Dev mailing list
[email protected]
http://wso2.org/cgi-bin/mailman/listinfo/dev
