Hi, As per the offline discussion with senaka, since the previous fix is causing lot of platform level issues, I'll change BAM/RSS any dependent components to use PrivilegedCarbonContext.getThreadLocalCarbonContext(), and make it work for tenant flow during the stub call.
Thanks, Sinthuka. On Sat, Dec 15, 2012 at 11:43 PM, Senaka Fernando <[email protected]> wrote: > Hi Sinthuja, > > Use PrivilegedCarbonContext.getThreadLocalCarbonContext() in BAM, which > should solve the issue AFAIU. > > Thanks, > Senaka. > > > On Sat, Dec 15, 2012 at 11:40 PM, Senaka Fernando <[email protected]> wrote: > >> 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 >> <[email protected]>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 <[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. >>>> >>>> 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 >>>> >>>> >>> >> >> >> -- >> * <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
