On Wed, Aug 8, 2012 at 4:34 PM, Thomas Broyer <t.bro...@gmail.com> wrote:
> I'm on mobile right now so I didn't review the changes, but it looks from > your comments like the home page should simply be protected. That way, auth > would be triggered by the app loading, and the RF stuff would only be used > in cased you're logged out while using the app (sign out from another > window/tab or session expiration). > Yes, this is exactly right. It's honestly been a while since I've built a webapp (curse of the tool-writer), so I apologize if some of these statements seem simplistic :). When you say protected, you mean by more than just a security-constraint, right? Because that's not enough to prevent the browser loading it from cache - there would have to be app-level logic to protect the page. > This is how I handle auth in my own apps at least (I also make the home > page dynamic, passing the user info and logout URL in the page as JS global > vars, retrieved from the app using JSNI or a Dictionary) > I think that's a good idea. I'm still inclined to commit as-is, as it's a step in the right direction, but I think it needs another iteration with your suggestions. > Le 8 août 2012 21:54, "Rajeev Dayal" <rda...@google.com> a écrit : > > Should mention that this review is based on >> http://gwt-code-reviews.appspot.com/1804803/. >> >> I've moved over the authentication functionality from Expenses to >> MobileWebApp. While I was at it, I corrected some of the inherent auth >> problems in Expenses (and therefore MobileWebApp when I copied this over). >> I've described these issues below: >> >> After logging out, if MobileWebApp.html is loaded from cache (which it >> can be, because we're not setting the no-store header), there would be a >> problem with the redirection back to the login page. The triggering event >> was an unauthorized post to /gwtRequest, which the GaeAuthFilter would >> trap. Unfortunately, /gwtRequest is not a valid URL to navigate to after >> login. I had to add code to always go back to MobileWebApp.html. I also had >> to add logic to preserve the gwt.codesvr parameter, so that the user would >> not be dropped out of development mode on the redirection. >> >> While this fix works, it's not perfect, there are still inherent problems >> with this re-authentication approach. If you look closely, when you log out >> of the app, there's a second where you're logged out, but the app is still >> visible. This is because the revalidation trigger occurs when >> RequestFactory attempts to do an RPC. We should have a re-validation >> trigger before this - that is, before the app UI even loads, we should do >> an auth check. >> >> Also, it's clunky to have to do custom hackery to preserve the >> gwt.codesvr param and know the app's home page in the revalidation case >> triggered by RequestFactory. I don't think there's any way we can get >> around baking the home page URL into the server code if we want to be able >> to trigger a re-validation due to a RequestFactory RPC. We could make >> things a bit nicer by having the DefaultRequestTransport in RequestFactory >> preserve query params, just so that things work properly in Development >> Mode. >> >> Also, I did not make any changes to the Tablet or Desktop versions; this >> is something we need to update. Any takers? >> >> On Wed, Aug 8, 2012 at 3:42 PM, <rda...@google.com> wrote: >> >>> Reviewers: drfibonacci, tbroyer, >>> >>> Description: >>> Move GAE Auth functionality from Expenses over the MobileWebApp sample. >>> >>> >>> Please review this at >>> http://gwt-code-reviews.**appspot.com/1806803/<http://gwt-code-reviews.appspot.com/1806803/> >>> >>> Affected files: >>> M samples/dynatablerf/README-**MAVEN.txt >>> M samples/dynatablerf/pom.xml >>> M samples/mobilewebapp/README-**MAVEN.txt >>> M samples/mobilewebapp/pom.xml >>> M samples/mobilewebapp/src/main/**java/com/google/gwt/sample/** >>> gaerequest/client/**GaeAuthRequestTransport.java >>> M samples/mobilewebapp/src/main/**java/com/google/gwt/sample/** >>> gaerequest/client/**GaeAuthenticationFailureEvent.**java >>> A samples/mobilewebapp/src/main/**java/com/google/gwt/sample/** >>> gaerequest/client/LoginWidget.**java >>> A samples/mobilewebapp/src/main/**java/com/google/gwt/sample/** >>> gaerequest/client/LoginWidget.**ui.xml >>> M samples/mobilewebapp/src/main/**java/com/google/gwt/sample/** >>> gaerequest/client/**ReloadOnAuthenticationFailure.**java >>> M samples/mobilewebapp/src/main/**java/com/google/gwt/sample/** >>> gaerequest/server/**GaeAuthFilter.java >>> A samples/mobilewebapp/src/main/**java/com/google/gwt/sample/** >>> gaerequest/server/**UserServiceLocator.java >>> A samples/mobilewebapp/src/main/**java/com/google/gwt/sample/** >>> gaerequest/server/**UserServiceWrapper.java >>> A samples/mobilewebapp/src/main/**java/com/google/gwt/sample/** >>> gaerequest/shared/GaeUser.java >>> A samples/mobilewebapp/src/main/**java/com/google/gwt/sample/** >>> gaerequest/shared/**GaeUserServiceRequest.java >>> A samples/mobilewebapp/src/main/**java/com/google/gwt/sample/** >>> gaerequest/shared/**MakesGaeRequests.java >>> M samples/mobilewebapp/src/main/**java/com/google/gwt/sample/** >>> mobilewebapp/client/App.java >>> M samples/mobilewebapp/src/main/**java/com/google/gwt/sample/** >>> mobilewebapp/client/**ClientFactory.java >>> M samples/mobilewebapp/src/main/**java/com/google/gwt/sample/** >>> mobilewebapp/client/**ClientFactoryImpl.java >>> M samples/mobilewebapp/src/main/**java/com/google/gwt/sample/** >>> mobilewebapp/client/desktop/**MobileWebAppShellDesktop.java >>> M samples/mobilewebapp/src/main/**java/com/google/gwt/sample/** >>> mobilewebapp/client/desktop/**MobileWebAppShellDesktop.ui.**xml >>> M samples/mobilewebapp/src/main/**java/com/google/gwt/sample/** >>> mobilewebapp/shared/**MobileWebAppRequestFactory.**java >>> M samples/mobilewebapp/src/main/**webapp/WEB-INF/appengine-web.**xml >>> M samples/mobilewebapp/src/main/**webapp/WEB-INF/web.xml >>> >>> >>> >> -- http://groups.google.com/group/Google-Web-Toolkit-Contributors