> On Sept. 22, 2015, 3:41 p.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. > > Venki Korukanti wrote: > 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.
By implementing a filter level default block-everything policy, this basically means we can't take advantage of annotation based filtering. Is it possible to instead state a block all in Jersey (that only applies if another annotation doesn't override that default behavior)? This way we can continue to leverage annotation based security. > On Sept. 22, 2015, 3:41 p.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 > > Venki Korukanti wrote: > I was putting all login related resources under "/log" to avoid multiple > security constraints for each login resource (i.e "/login/*, "/logout") I think we need to solve the dual security problem then we can avoid this complexity but make the urls user friendly. - Jacques ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38359/#review99997 ----------------------------------------------------------- On Sept. 16, 2015, 7: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, 7: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 > >
