Re: [gatein-dev] WCI/PC branch WIP

it is now merged in gatein-portal/master kuddos to you and Trong for the responsiveness and the good work On Sep 18, 2012, at 1:59 PM, Julien Viet <julien(a)julienviet.com&gt; wrote: > > On Sep 18, 2012, at 1:42 PM, Marek Posolda <mposolda(a)redhat.com&gt; wrote: > >> Hi Julien, >> >> I am checking the branch https://github.com/vietj/gatein-portal/tree/pcwci . Thanks for this authentication simplification. I have some comments: >> >> 1) I tested on AS7 and saw some compilation failures in wsrp-extension component (namely in class WSRPServiceIntegration) because it still used old WCI api for obtaining ServletContainer. >> I fixed it simply by using: ServletContainer servletContainer = ServletContainerFactory.getServletContainer(); >> instead of DefaultServletContainerFactory as you suggested on irc. >> >> 2) Second thing is more major and it's related to the WSS issue with credentials. Unfortunately it doesn't work correctly in your branch because LoginServlet.bindCredentialsToConversationState is called directly after authentication, which is too early as it still don't have correct conversationState at this moment. What happens here is: >> - Request http://localhost:8080/portal/login?username=john&password=gtnis send >> -- SetCurrentIdentityFilter is called at the beginning of the request, but at this moment user is not authenticated. So it uses ConversationState for anonymous identity (IdentityConstants.ANONIM) >> -- request is processed by LoginServlet, which invokes wci authentication. But at this moment there is still ConversationState for anonymous user bind to threadLocal, so Credentials are registered to this conversationState. Then we have redirection to initialURI like "/portal/classic" >> - Request http://localhost:8080/portal/classic is send >> -- SetCurrentIdentityFilter is called and it recognize that user is authenticated so it creates new ConversationState for this user and register it to ConversationRegistry. But note that previous ConversationState is lost. So credentials are not registered in current ConversationState --> WSS can't work >> >> I have my branch here https://github.com/mposolda/gatein-portal/tree/pcwci which is based on top of your branch and it addresses (1) and (2). I fixed (2) by similar approach like was before your changes, which means: >> - After successful authentication are credentials bind to temporary authenticationRegistry, which exists only during authentication >> - I added new filter ConversationStateUpdateFilter, which removes credentials from temporary registry and adds them to ConversationState. ConversationStateUpdateFilter is added after SetCurrentIdentityFilter, which ensures that correct ConversationState is bound to threadLocal when it's called. >> >> I did not test with WSS enabled, but I verified that after my change is ConversationState.getCurrent().getAttribute(Credentials.CREDENTIALS) returning correct credentials value, which should fix GTNPORTAL-2567 . >> >> Let me know if my branch with those changes https://github.com/mposolda/gatein-portal/tree/pcwci is ok or you have some other ideas to improve this. >> >> Personally I would prefer to use WCI authenticationListener for register credentials but unfortunately listener doesn't have information about password of user, so I created WCI jira https://issues.jboss.org/browse/GTNWCI-37 to address this. I think you told about this limitation on IRC yesterday as well. WDYT? > > we can fix WCI and make password available again. > > if yes we can make a release today. > >> >> 3) I saw some exceptions due to gatein-sample-extension and gatein-sample-portal. It seems that we need to remove those obsolete servlets from web.xml and login.jsp. Also it seems that other products on top of GateIn will need to adapt those changes as well. I did not fix those in my branch for now… > > this is correct, it needs more correction before final merge, this is still a WIP :-) > >> >> I will try to test with SSO later in the afternoon but I did not expect any major problems as you keep "/dologin" secure URL, so I think that it should work without any changes. Will update later if found any problems. > >> >> Marek >> >> On 18/09/12 08:51, Julien Viet wrote: >>> they actually need more work from Trong before being merged and careful checks. >>> >>> I believe we can finish that by the end of this week. >>> >>> On Sep 17, 2012, at 10:45 PM, Julien Viet <julien(a)julienviet.com&gt; wrote: >>> >>>> Hi, >>>> >>>> I pushed the initial rewrite of the authentication in GateIn that is based on WCI and Servlet 3.0 authentication. >>>> >>>> The good news is it improves things by simplifying the code and remove all the previous http redirects that we needed. >>>> >>>> I wrote a doc about it there : https://community.jboss.org/wiki/GateIn35Authentication >>>> >>>> - Marek and Matt should look at the authentication with SSO and WS-Security >>>> - Trong should look at the login form redirection for groups and users as now we don't rely anymore on servlet container security constraint (like it is already for portal pages) >>>> >>>> Note that this integrates with the new Alpha common/wci/pc stack, Bolek and I have been working for a couple of weeks. I guess it could also affect WSRP if it is using WCI in some way (WCI has had many simplifications last week with the removal of unnecessary stuff). >>>> >>>> cheers >>>> >>>> Julien >>> >>> _______________________________________________ >>> gatein-dev mailing list >>> gatein-dev(a)lists.jboss.org >>> https://lists.jboss.org/mailman/listinfo/gatein-dev >> > _______________________________________________ gatein-dev mailing list gatein-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/gatein-dev