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.
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.
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
