> On Sept. 22, 2015, 8:41 a.m., Jacques Nadeau wrote: > > exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/RestServerHelper.java, > > line 190 > > <https://reviews.apache.org/r/38359/diff/3-4/?file=1074935#file1074935line190> > > > > If you permissions annotations on the Login/Out resources, you > > shouldn't need this.
We still need the constraints here, because the decision whether the resource needs authentication or not is taken in SecurityHandler before the the request is even passed to the ServletContainer based on the given SecurityConstraints. SecurityHandler doesn't look at the annotations on jeresey resources. Jeresey uses the set SecurityContext to decides whether to allow the request to the resource or not based on the annotations on resource and user roles in SecurityContext in jeresey filter RolesAllowedDynamicFeature. We could move the authentication filter into jeresey filter by implementing the ContainerRequestFilter/DynamicFeature, where we could look the annotations on the resource to decide whether it needs authentication or not. The problem I had here is there seems to no way to have form authentication using this model. Basically ContainerRequestFilter.filter receives a ContainerRequestContext which doesn't have any methods to forward the request to login page in case the credentials are needed or access to HttpSession. If the request already contains userName/Password in header (basic authentication), then this method is ok to use it, but in our case we are looking for form authentication. One another approach I just thought of is we could auto generate SecurityConstraints by scanning resources in DrillRestServer. This avoids manually maintaining the SecurityConstraints. If you are ok, I can address this as an improvment in a separate JIRA. > On Sept. 22, 2015, 8:41 a.m., Jacques Nadeau wrote: > > exec/java-exec/src/main/resources/rest/log/login.ftl, line 13 > > <https://reviews.apache.org/r/38359/diff/4/?file=1075126#file1075126line13> > > > > Let's not create a separate file for the ui framework. We should share > > the existing framework we use everywhere else (via ftl include) I will update the ViewablePermissions to add one more flag "showControls" to Model. > On Sept. 22, 2015, 8:41 a.m., Jacques Nadeau wrote: > > exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/LogInLogOutResources.java, > > line 35 > > <https://reviews.apache.org/r/38359/diff/4/?file=1075108#file1075108line35> > > > > can we make this /login I was putting all login related resources under "/log" to avoid multiple security constraints for each login resource (i.e "/login/*, "/logout") - Venki ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38359/#review99997 ----------------------------------------------------------- On Sept. 16, 2015, 12:06 a.m., Venki Korukanti wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/38359/ > ----------------------------------------------------------- > > (Updated Sept. 16, 2015, 12:06 a.m.) > > > Review request for drill, Jacques Nadeau and Jason Altekruse. > > > Repository: drill-git > > > Description > ------- > > Use jetty's SecurityHandler (with FormAuthenticator and LoginService) to > enforce authentication. Use jersey's annotations to enforece authorizations. > > > Diffs > ----- > > distribution/src/resources/drill-override-example.conf 805d6e9 > exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java > 0f6a5bb > > exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/DrillRestServer.java > 8c14587 > > exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/DrillRoot.java > 3e972b4 > > exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/LogInLogOutResources.java > PRE-CREATION > > exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/LogOutServlet.java > PRE-CREATION > > exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/MetricsResources.java > 28a292b > > exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/QueryResources.java > 145a476 > > exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/QueryWrapper.java > ee31929 > > exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/RestServerHelper.java > PRE-CREATION > > exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/StatusResources.java > c99c49b > > exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/StorageResources.java > 49f387c > > exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/ThreadsResources.java > def5acb > > exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/ViewableWithPermissions.java > PRE-CREATION > > exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/auth/AbstractDrillLoginService.java > PRE-CREATION > > exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/auth/AnonymousAuthenticator.java > PRE-CREATION > > exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/auth/AnonymousLoginService.java > PRE-CREATION > > exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/auth/DrillRestLoginService.java > PRE-CREATION > > exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/auth/DrillUserPrincipal.java > PRE-CREATION > > exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/profile/ProfileResources.java > 6656bf6 > exec/java-exec/src/main/resources/drill-module.conf dbe449a > exec/java-exec/src/main/resources/rest/generic.ftl 9df2424 > exec/java-exec/src/main/resources/rest/log/login.ftl PRE-CREATION > exec/java-exec/src/main/resources/rest/static/img/apache-drill-logo.png > PRE-CREATION > > Diff: https://reviews.apache.org/r/38359/diff/ > > > Testing > ------- > > Currently testing is manual. Rest based unittests are coming in DRILL-2965. > > > Thanks, > > Venki Korukanti > >
