search

tomaswolf / gerrit-gitblit-plugin

Integrates standard GitBlit (currently 1.7.1) as a repository viewer in Gerrit
Apache License 2.0
22 stars 5 forks source link

Gitblit plugin and Oauth plugin compatibility #7

Closed ravindranathbr closed 9 years ago

ravindranathbr commented 9 years ago

When Gitbilt plugin enabled with Gerrit Oauth provider plugin, the gitbilt plugin reads internal server error when logging with Google account. Upon checking the error log, there is a nullpointer exception reading session data. The Github Oauth login is working correctly without any issues with Gitbilt.

tomaswolf commented 9 years ago

Which version of the GitBlit plugin in which version of Gerrit?

Please provide the exception trace you mentioned.

ravindranathbr commented 9 years ago

Hi, I am using

Gerrit Code Review (2.11.1) Oauth plugin : https://github.com/davido/gerrit-oauth-provider/releases/download/v2.11/gerrit-oauth-provider.jar Gitbilt : gitblit-plugin-2.11.1.162.1.jar

Below is the complete error log

[2015-08-08 03:18:53,716] ERROR org.apache.wicket.RequestCycle : Can't instantiate page using constructor public com.gitblit.wicket.pages.RepositoriesPage() org.apache.wicket.WicketRuntimeException: Can't instantiate page using constructor public com.gitblit.wicket.pages.RepositoriesPage() at org.apache.wicket.session.DefaultPageFactory.createPage(DefaultPageFactory.java:212) at org.apache.wicket.session.DefaultPageFactory.newPage(DefaultPageFactory.java:57) at org.apache.wicket.request.target.component.BookmarkablePageRequestTarget.newPage(BookmarkablePageRequestTarget.java:298) at org.apache.wicket.request.target.component.BookmarkablePageRequestTarget.getPage(BookmarkablePageRequestTarget.java:320) at org.apache.wicket.request.target.component.BookmarkablePageRequestTarget.processEvents(BookmarkablePageRequestTarget.java:234) at org.apache.wicket.request.AbstractRequestCycleProcessor.processEvents(AbstractRequestCycleProcessor.java:92) at org.apache.wicket.RequestCycle.processEventsAndRespond(RequestCycle.java:1279) at org.apache.wicket.RequestCycle.step(RequestCycle.java:1358) at org.apache.wicket.RequestCycle.steps(RequestCycle.java:1465) at org.apache.wicket.RequestCycle.request(RequestCycle.java:545) at org.apache.wicket.protocol.http.WicketFilter.doGet(WicketFilter.java:486) at org.apache.wicket.protocol.http.WicketFilter.doFilter(WicketFilter.java:319) at com.googlesource.gerrit.plugins.gitblit.GerritWicketFilter.doFilter(GerritWicketFilter.java:135) at com.google.inject.servlet.FilterChainInvocation.doFilter(FilterChainInvocation.java:82) at com.google.inject.servlet.ManagedFilterPipeline.dispatch(ManagedFilterPipeline.java:119) at com.google.inject.servlet.GuiceFilter$1.call(GuiceFilter.java:133) at com.google.inject.servlet.GuiceFilter$1.call(GuiceFilter.java:130) at com.google.inject.servlet.GuiceFilter$Context.call(GuiceFilter.java:203) at com.google.inject.servlet.GuiceFilter.doFilter(GuiceFilter.java:130) at com.google.gerrit.httpd.plugins.HttpPluginServlet.service(HttpPluginServlet.java:233) at javax.servlet.http.HttpServlet.service(HttpServlet.java:725) at com.google.inject.servlet.ServletDefinition.doServiceImpl(ServletDefinition.java:279) at com.google.inject.servlet.ServletDefinition.doService(ServletDefinition.java:269) at com.google.inject.servlet.ServletDefinition.service(ServletDefinition.java:180) at com.google.inject.servlet.ManagedServletPipeline.service(ManagedServletPipeline.java:91) at com.google.inject.servlet.FilterChainInvocation.doFilter(FilterChainInvocation.java:85) at com.google.gerrit.httpd.GetUserFilter.doFilter(GetUserFilter.java:82) at com.google.inject.servlet.FilterChainInvocation.doFilter(FilterChainInvocation.java:82) at com.google.gwtexpui.server.CacheControlFilter.doFilter(CacheControlFilter.java:73) at com.google.inject.servlet.FilterChainInvocation.doFilter(FilterChainInvocation.java:82) at com.google.gerrit.httpd.RunAsFilter.doFilter(RunAsFilter.java:117) at com.google.inject.servlet.FilterChainInvocation.doFilter(FilterChainInvocation.java:82) at com.google.gerrit.httpd.AllRequestFilter$FilterProxy$1.doFilter(AllRequestFilter.java:64) at com.google.gerrit.httpd.AllRequestFilter$FilterProxy.doFilter(AllRequestFilter.java:57) at com.google.inject.servlet.FilterChainInvocation.doFilter(FilterChainInvocation.java:82) at com.google.gerrit.httpd.RequestContextFilter.doFilter(RequestContextFilter.java:75) at com.google.inject.servlet.FilterChainInvocation.doFilter(FilterChainInvocation.java:82) at com.google.inject.servlet.ManagedFilterPipeline.dispatch(ManagedFilterPipeline.java:119) at com.google.inject.servlet.GuiceFilter$1.call(GuiceFilter.java:133) at com.google.inject.servlet.GuiceFilter$1.call(GuiceFilter.java:130) at com.google.inject.servlet.GuiceFilter$Context.call(GuiceFilter.java:203) at com.google.inject.servlet.GuiceFilter.doFilter(GuiceFilter.java:130) at org.eclipse.jetty.servlet.ServletHandler$CachedChain.doFilter(ServletHandler.java:1652) at org.eclipse.jetty.servlet.ServletHandler.doHandle(ServletHandler.java:585) at org.eclipse.jetty.server.session.SessionHandler.doHandle(SessionHandler.java:221) at org.eclipse.jetty.server.handler.ContextHandler.doHandle(ContextHandler.java:1127) at org.eclipse.jetty.servlet.ServletHandler.doScope(ServletHandler.java:515) at org.eclipse.jetty.server.session.SessionHandler.doScope(SessionHandler.java:185) at org.eclipse.jetty.server.handler.ContextHandler.doScope(ContextHandler.java:1061) at org.eclipse.jetty.server.handler.ScopedHandler.handle(ScopedHandler.java:141) at org.eclipse.jetty.server.handler.HandlerWrapper.handle(HandlerWrapper.java:97) at org.eclipse.jetty.server.handler.RequestLogHandler.handle(RequestLogHandler.java:95) at org.eclipse.jetty.server.handler.HandlerWrapper.handle(HandlerWrapper.java:97) at org.eclipse.jetty.server.Server.handle(Server.java:497) at org.eclipse.jetty.server.HttpChannel.handle(HttpChannel.java:310) at org.eclipse.jetty.server.HttpConnection.onFillable(HttpConnection.java:257) at org.eclipse.jetty.io.AbstractConnection$2.run(AbstractConnection.java:540) at org.eclipse.jetty.util.thread.QueuedThreadPool.runJob(QueuedThreadPool.java:635) at org.eclipse.jetty.util.thread.QueuedThreadPool$3.run(QueuedThreadPool.java:555) at java.lang.Thread.run(Thread.java:745) Caused by: java.lang.reflect.InvocationTargetException at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method) at sun.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:57) at sun.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45) at java.lang.reflect.Constructor.newInstance(Constructor.java:526) at org.apache.wicket.session.DefaultPageFactory.createPage(DefaultPageFactory.java:192) ... 59 more Caused by: java.lang.NullPointerException at com.googlesource.gerrit.plugins.gitblit.auth.GerritGitBlitAuthenticationManager.authenticateFromSession(GerritGitBlitAuthenticationManager.java:145) at com.googlesource.gerrit.plugins.gitblit.auth.GerritGitBlitAuthenticationManager.authenticate(GerritGitBlitAuthenticationManager.java:112) at com.gitblit.wicket.pages.SessionPage.login(SessionPage.java:90) at com.gitblit.wicket.pages.SessionPage.(SessionPage.java:38) at com.gitblit.wicket.pages.BasePage.(BasePage.java:80) at com.gitblit.wicket.pages.RootPage.(RootPage.java:97) at com.gitblit.wicket.pages.RepositoriesPage.(RepositoriesPage.java:49)

... 64 more

Steps to reproduce:

  1. Install both plugins oauth plugin and gitbilt plugin with gerrit with the versions mentioned above
  2. Configure Google and Oauth application credentials with Gerrit
  3. Start the application
  4. Sign in with Google Oauth
  5. Click on Gitbilt > Repositories

Please let me know if you need any more details

tomaswolf commented 9 years ago

Thanks a lot. That already shows me where the problem is. Looks like some of the authentication stuff has changed.

tomaswolf commented 9 years ago

The NPE is caused by a User object stored in the session that has no username. This appears to be caused by the OAuth plugin, GoogleOAuthService.java, line 142. As a result, the Gerrit user for this will not have a username, see OAuthSession, line 142. (Yes, the two line numbers happen to be the same.) This works fine in Gerrit itself; its user management does not require a user to have a username.

However, the Gitblit user management assumes that users always have a user name. And from just looking at the sources, I would expect the official plugin to fail in a similar way when used with Google OAuth (though maybe not in an identical way; it does one null check more, which then however might lead to GitBlit sending back a HTTP basic authentication challenge).

I can adapt the plugin's authentication handling to deal with Gerrit users without usernames, but I'll have to figure out how to represent such users in Gitblit. Probably I'll use the e-mail address, if set, or the full name (display name), if set, and just "unknown" if neither is set.

One more problem will be that I won't be able to test this since I have no public URLs for any of my Gerrit installations, and Google OAuth requires one for its authorized redirect URL. As a result I cannot set up a Gerrit test instance with Google OAuth. So I'll have to fix this the best I can and then I'll have to rely on you to test it.

ravindranathbr commented 9 years ago

Thanks for the detailed explanation. Sure, I will do the testing for this fiix

tomaswolf commented 9 years ago

Can you give v2.11.1.162.2 a try, please?

ravindranathbr commented 9 years ago

hi, Thanks for the new fix., This appears to be solved with Google signing with various Google accounts, however there appears to be problem with github login oauth signing with another github account( which is not administrator account and new to gerrit installation ) NPE is occuring, I tested with both gitblit v2.11.1.162.1 and v2.11.1.162.1 may be this is same issue with another use case.. below is the exception log

[2015-08-10 07:46:24,664] ERROR org.apache.wicket.RequestCycle : Can't instantiate page using constructor public com.gitblit.wicket.pages.RepositoriesPage() org.apache.wicket.WicketRuntimeException: Can't instantiate page using constructor public com.gitblit.wicket.pages.RepositoriesPage() at org.apache.wicket.session.DefaultPageFactory.createPage(DefaultPageFactory.java:212) at org.apache.wicket.session.DefaultPageFactory.newPage(DefaultPageFactory.java:57) at org.apache.wicket.request.target.component.BookmarkablePageRequestTarget.newPage(BookmarkablePageRequestTarget.java:298) at org.apache.wicket.request.target.component.BookmarkablePageRequestTarget.getPage(BookmarkablePageRequestTarget.java:320) at org.apache.wicket.request.target.component.BookmarkablePageRequestTarget.processEvents(BookmarkablePageRequestTarget.java:234) at org.apache.wicket.request.AbstractRequestCycleProcessor.processEvents(AbstractRequestCycleProcessor.java:92) at org.apache.wicket.RequestCycle.processEventsAndRespond(RequestCycle.java:1279) at org.apache.wicket.RequestCycle.step(RequestCycle.java:1358) at org.apache.wicket.RequestCycle.steps(RequestCycle.java:1465) at org.apache.wicket.RequestCycle.request(RequestCycle.java:545) at org.apache.wicket.protocol.http.WicketFilter.doGet(WicketFilter.java:486) at org.apache.wicket.protocol.http.WicketFilter.doFilter(WicketFilter.java:319) at com.googlesource.gerrit.plugins.gitblit.GerritWicketFilter.doFilter(GerritWicketFilter.java:135) at com.google.inject.servlet.FilterChainInvocation.doFilter(FilterChainInvocation.java:82) at com.google.inject.servlet.ManagedFilterPipeline.dispatch(ManagedFilterPipeline.java:119) at com.google.inject.servlet.GuiceFilter$1.call(GuiceFilter.java:133) at com.google.inject.servlet.GuiceFilter$1.call(GuiceFilter.java:130) at com.google.inject.servlet.GuiceFilter$Context.call(GuiceFilter.java:203) at com.google.inject.servlet.GuiceFilter.doFilter(GuiceFilter.java:130) at com.google.gerrit.httpd.plugins.HttpPluginServlet.service(HttpPluginServlet.java:233) at javax.servlet.http.HttpServlet.service(HttpServlet.java:725) at com.google.inject.servlet.ServletDefinition.doServiceImpl(ServletDefinition.java:279) at com.google.inject.servlet.ServletDefinition.doService(ServletDefinition.java:269) at com.google.inject.servlet.ServletDefinition.service(ServletDefinition.java:180) at com.google.inject.servlet.ManagedServletPipeline.service(ManagedServletPipeline.java:91) at com.google.inject.servlet.FilterChainInvocation.doFilter(FilterChainInvocation.java:85) at com.google.gerrit.httpd.GetUserFilter.doFilter(GetUserFilter.java:82) at com.google.inject.servlet.FilterChainInvocation.doFilter(FilterChainInvocation.java:82) at com.google.gwtexpui.server.CacheControlFilter.doFilter(CacheControlFilter.java:73) at com.google.inject.servlet.FilterChainInvocation.doFilter(FilterChainInvocation.java:82) at com.google.gerrit.httpd.RunAsFilter.doFilter(RunAsFilter.java:117) at com.google.inject.servlet.FilterChainInvocation.doFilter(FilterChainInvocation.java:82) at com.google.gerrit.httpd.AllRequestFilter$FilterProxy$1.doFilter(AllRequestFilter.java:64) at com.google.gerrit.httpd.AllRequestFilter$FilterProxy.doFilter(AllRequestFilter.java:57) at com.google.inject.servlet.FilterChainInvocation.doFilter(FilterChainInvocation.java:82) at com.google.gerrit.httpd.RequestContextFilter.doFilter(RequestContextFilter.java:75) at com.google.inject.servlet.FilterChainInvocation.doFilter(FilterChainInvocation.java:82) at com.google.inject.servlet.ManagedFilterPipeline.dispatch(ManagedFilterPipeline.java:119) at com.google.inject.servlet.GuiceFilter$1.call(GuiceFilter.java:133) at com.google.inject.servlet.GuiceFilter$1.call(GuiceFilter.java:130) at com.google.inject.servlet.GuiceFilter$Context.call(GuiceFilter.java:203) at com.google.inject.servlet.GuiceFilter.doFilter(GuiceFilter.java:130) at org.eclipse.jetty.servlet.ServletHandler$CachedChain.doFilter(ServletHandler.java:1652) at org.eclipse.jetty.servlet.ServletHandler.doHandle(ServletHandler.java:585) at org.eclipse.jetty.server.session.SessionHandler.doHandle(SessionHandler.java:221) at org.eclipse.jetty.server.handler.ContextHandler.doHandle(ContextHandler.java:1127) at org.eclipse.jetty.servlet.ServletHandler.doScope(ServletHandler.java:515) at org.eclipse.jetty.server.session.SessionHandler.doScope(SessionHandler.java:185) at org.eclipse.jetty.server.handler.ContextHandler.doScope(ContextHandler.java:1061) at org.eclipse.jetty.server.handler.ScopedHandler.handle(ScopedHandler.java:141) at org.eclipse.jetty.server.handler.HandlerWrapper.handle(HandlerWrapper.java:97) at org.eclipse.jetty.server.handler.RequestLogHandler.handle(RequestLogHandler.java:95) at org.eclipse.jetty.server.handler.HandlerWrapper.handle(HandlerWrapper.java:97) at org.eclipse.jetty.server.Server.handle(Server.java:497) at org.eclipse.jetty.server.HttpChannel.handle(HttpChannel.java:310) at org.eclipse.jetty.server.HttpConnection.onFillable(HttpConnection.java:257) at org.eclipse.jetty.io.AbstractConnection$2.run(AbstractConnection.java:540) at org.eclipse.jetty.util.thread.QueuedThreadPool.runJob(QueuedThreadPool.java:635) at org.eclipse.jetty.util.thread.QueuedThreadPool$3.run(QueuedThreadPool.java:555) at java.lang.Thread.run(Thread.java:745) Caused by: java.lang.reflect.InvocationTargetException at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method) at sun.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:57) at sun.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45) at java.lang.reflect.Constructor.newInstance(Constructor.java:526) at org.apache.wicket.session.DefaultPageFactory.createPage(DefaultPageFactory.java:192) ... 59 more Caused by: java.lang.NullPointerException at com.gitblit.wicket.panels.GravatarImage.(GravatarImage.java:55) at com.gitblit.wicket.panels.GravatarImage.(GravatarImage.java:49) at com.gitblit.wicket.pages.RootPage$UserMenu.onInitialize(RootPage.java:621) at org.apache.wicket.Component.fireInitialize(Component.java:4105) at org.apache.wicket.MarkupContainer.initialize(MarkupContainer.java:433) at org.apache.wicket.MarkupContainer.addedComponent(MarkupContainer.java:1000) at org.apache.wicket.MarkupContainer.add(MarkupContainer.java:142) at com.gitblit.wicket.pages.RootPage.setupPage(RootPage.java:175) at com.gitblit.wicket.pages.RepositoriesPage.setup(RepositoriesPage.java:64) at com.gitblit.wicket.pages.RepositoriesPage.(RepositoriesPage.java:50)

... 64 more

tomaswolf commented 9 years ago

So the Google OAuth failure is solved?

The new exception is another error, but let's deal with this in this issue, too. Looks like GitBlit not only assumes that every user has a username, but also that it has either an e-mail address or a display name.

I'll fix that, too, and then overwrite the current v.2.11.1.162.2.

ravindranathbr commented 9 years ago

Yes, Google OAuth works good for me.. :+1:

tomaswolf commented 9 years ago

Ok; could you give v2.11.1.162.2 another try? (Download the jar again; I have overwritten the previous version.)

ravindranathbr commented 9 years ago

It is working in all the use cases that I have tested :) Thank you!

tomaswolf commented 9 years ago

Thanks for testing -- and for reporting the bugs in the first place, allowing me to fix these errors.