> On Sept. 14, 2015, 7 p.m., Jacques Nadeau wrote: > > exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/WebUserSession.java, > > line 31 > > <https://reviews.apache.org/r/38359/diff/2/?file=1072648#file1072648line31> > > > > Why do we have a WebUserSession and a principal? Shouldn't they be > > interconnected?
Initially I thought it would be better to keep the resources separate from the identity info which the Principal contains, but looking at again it seems to better if the DrillClient (additional property in WebUserSession) is part of the Principal. > On Sept. 14, 2015, 7 p.m., Jacques Nadeau wrote: > > exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/auth/DrillUserPrincipal.java, > > line 47 > > <https://reviews.apache.org/r/38359/diff/2/?file=1072652#file1072652line47> > > > > It seems wrong that we are holding onto this. Do we need to? I kept this for two reasons: 1) We need the password to create the DrillClient later when are inserting the WebUserSession. Removed the WebUserSession and made the DrillClient part of the Principal itself. So now we don't need the password for this case. 2) LoginService has a method called validate which can be called with the UserIdentity returned from login() to see if the user is still valid. Thinking about now, it looks like we don't have to worry that a user credentials are valid or not after the login is successful. From JDBC/ODBC we don't recheck the credentials after the login. Also in Linux bases systems, users can still continue to function in logged in session even if the password has changed. Let me know your opinion on this. > On Sept. 14, 2015, 7 p.m., Jacques Nadeau wrote: > > exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/WebUserSession.java, > > line 42 > > <https://reviews.apache.org/r/38359/diff/2/?file=1072648#file1072648line42> > > > > What happens if a user happens to have the account named anonymous? Fixed this by getting the auth method from SecurityContext.getAuthenticationScheme(). It is part of ViewableWithPermissions class now. > On Sept. 14, 2015, 7 p.m., Jacques Nadeau wrote: > > exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/StorageResources.java, > > line 94 > > <https://reviews.apache.org/r/38359/diff/2/?file=1072646#file1072646line94> > > > > Can you update all of these. Let's just create a > > ViewableWithPermissions or similar. > > > > Then we can create a Map<String, Object> as the object with "model" as > > the value we pass in. And the appropriate other values as booleans. This > > should work and allow everything else to be the same. See here for why I > > think it will work: > > > > > > https://github.com/jersey/jersey/blob/master/ext/mvc-freemarker/src/main/java/org/glassfish/jersey/server/mvc/freemarker/FreemarkerViewProcessor.java#L120 updated - Venki ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38359/#review98978 ----------------------------------------------------------- On Sept. 14, 2015, 10:43 a.m., Venki Korukanti wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/38359/ > ----------------------------------------------------------- > > (Updated Sept. 14, 2015, 10:43 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/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/ModelWrapper.java > PRE-CREATION > > 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/WebUserSession.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/index.ftl 99e9d8c > exec/java-exec/src/main/resources/rest/options.ftl 7ba1250 > exec/java-exec/src/main/resources/rest/profile/list.ftl cf92ede > exec/java-exec/src/main/resources/rest/profile/profile.ftl 47c7e06 > exec/java-exec/src/main/resources/rest/query/errorMessage.ftl dbdcc9e > exec/java-exec/src/main/resources/rest/query/result.ftl 7fe52a4 > exec/java-exec/src/main/resources/rest/static/img/apache-drill-logo.png > PRE-CREATION > exec/java-exec/src/main/resources/rest/static/login.html PRE-CREATION > exec/java-exec/src/main/resources/rest/status.ftl cafa523 > exec/java-exec/src/main/resources/rest/storage/list.ftl ef97561 > exec/java-exec/src/main/resources/rest/storage/update.ftl 2a276e1 > pom.xml c17e612 > > Diff: https://reviews.apache.org/r/38359/diff/ > > > Testing > ------- > > Currently testing is manual. Rest based unittests are coming in DRILL-2965. > > > Thanks, > > Venki Korukanti > >
