Hi Sinthuja,
No I don't think this will break BAM analytics functionality. Also, the
proposed fix won't work since the incoming tenant ID is valid, but the
username is wrong, which is the issue that we ran into. I have made the
change in r151877 (please note the changes are in two places). Can you
validate BAM and if it breaks, can you paste the trace in here?
Thanks,
Senaka.
On Sat, Dec 15, 2012 at 11:34 PM, Sinthuja Ragendran <sinth...@wso2.com>wrote:
> Hi,
>
> this change was done because there was an issue when starting tenant flow
> during stub call. Please refer mail thread in 'problem in starting tenant
> flow with stub call'. Therefore If you revert this change it will break BAM
> analytics functionality . :(
>
> So rather reverting the change in getCurrentCarbonContextHolder(), shall
> we check whether returned tenant id from the ThreadLocalCarbonContext is
> valid and if it's invalid shall we pick it from MessageContext and
> ConfigurationContext?
>
>
> public static CarbonContextDataHolder getCurrentCarbonContextHolder() {
> //1. Check ThreadLocalCarbonContextHolder
> CarbonContextDataHolder currentCarbonContext =
> getThreadLocalCarbonContextHolder();
> *if (MultitenantConstants.INVALID_TENANT_ID ==
> currentCarbonContext.getTenantId()){*
>
> 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;
> }
>
>
> Thanks,
> Sinthuja.
>
>
> On Sat, Dec 15, 2012 at 10:40 PM, Senaka Fernando <sen...@wso2.com> 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.
>>
>> 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
>> <dimut...@wso2.com>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 <sen...@wso2.com>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 <sen...@wso2.com>wrote:
>>>>
>>>>> Hi Shariq,
>>>>>
>>>>> On Sat, Dec 15, 2012 at 12:07 PM, Muhammed Shariq <sha...@wso2.com>wrote:
>>>>>
>>>>>> On Fri, Dec 14, 2012 at 4:16 PM, Dimuthu Leelarathne <
>>>>>> dimut...@wso2.com> 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 <sen...@wso2.com>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 <
>>>>>>>> dimut...@wso2.com> 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
>>>>>>> Dev@wso2.org
>>>>>>> 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
>> Dev@wso2.org
>> http://wso2.org/cgi-bin/mailman/listinfo/dev
>>
>>
>
--
* <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
Dev@wso2.org
http://wso2.org/cgi-bin/mailman/listinfo/dev
