> On June 4, 2015, 4:36 p.m., Joshua Cohen wrote:
> > src/main/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityModule.java,
> >  lines 136-141
> > <https://reviews.apache.org/r/34566/diff/2/?file=977709#file977709line136>
> >
> >     Do you foresee us exposing other authorized servlets in this manner? If 
> > so, how does this scale in those cases? Might it make sense to extract 
> > something that can act on a set of servlet/auth configuration rather than 
> > explicitly listing each servlet for each switch case?
> >     
> >     A TODO acknowledging that we'd want to address if/when we add more is 
> > probably fine for now.

I am hesitant to refactor without seeing more use cases first. The registration 
order is important here and having all filters close by really helps. Punting 
for now as any attempt to refactor will add verbosity and reduce readability.


> On June 4, 2015, 4:36 p.m., Joshua Cohen wrote:
> > src/test/java/org/apache/aurora/scheduler/http/H2ConsoleModuleIT.java, line 
> > 36
> > <https://reviews.apache.org/r/34566/diff/2/?file=977713#file977713line36>
> >
> >     Use constant for this?
> >     
> >         assertEquals(ClientResponse.Status.OK.getStatusCode(), 
> > response.getStatus())

Done.


> On June 4, 2015, 4:36 p.m., Joshua Cohen wrote:
> > src/test/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityIT.java,
> >  line 303
> > <https://reviews.apache.org/r/34566/diff/2/?file=977714#file977714line303>
> >
> >     Same here and below.

Done.


- Maxim


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34566/#review86634
-----------------------------------------------------------


On June 3, 2015, 10:08 p.m., Maxim Khutornenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34566/
> -----------------------------------------------------------
> 
> (Updated June 3, 2015, 10:08 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Kevin Sweeney.
> 
> 
> Bugs: AURORA-1287
>     https://issues.apache.org/jira/browse/AURORA-1287
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Adding support for connecting to H2 DB via management console: 
> http://www.h2database.com/html/quickstart.html
> 
> The H2 console servlet is put behind Shiro. Users are expected to have 
> `h2_management_console` permission or be a member of admin role to access the 
> console.
> 
> Vagrant example JDBC URL: jdbc:h2:mem:aurora
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/aurora/scheduler/http/H2ConsoleModule.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 
> 16515f6ce4518e0d7eec27dfcde6fe036ee05d1d 
>   
> src/main/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityModule.java
>  079ff5d2c30cc94bf4aef97cd4d791d480ebeab9 
>   
> src/main/java/org/apache/aurora/scheduler/http/api/security/ShiroIniParser.java
>  0163ba1a6df647a644948a42f772bd838c2146ef 
>   
> src/main/java/org/apache/aurora/scheduler/http/api/security/ShiroKerberosAuthenticationFilter.java
>  28e6b98b39a073251bfae483575edd9635ae783a 
>   
> src/main/java/org/apache/aurora/scheduler/http/api/security/ShiroKerberosPermissiveAuthenticationFilter.java
>  PRE-CREATION 
>   src/test/java/org/apache/aurora/scheduler/http/H2ConsoleModuleIT.java 
> PRE-CREATION 
>   
> src/test/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityIT.java
>  6743d06be828247b1c3a232aea145b0a0c492eca 
>   
> src/test/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthorizingParamInterceptorTest.java
>  33783c88b7c6c1a7343f3271523a7db39c73e4a1 
>   
> src/test/java/org/apache/aurora/scheduler/http/api/security/ShiroKerberosAuthenticationFilterTest.java
>  e335a439a6ccc301cf7c93e138c90660fbfdc6d0 
>   
> src/test/java/org/apache/aurora/scheduler/http/api/security/ShiroKerberosPermissiveAuthenticationFilterTest.java
>  PRE-CREATION 
>   src/test/sh/org/apache/aurora/e2e/test_kerberos_end_to_end.sh 
> 6e9e3b2a8d731e51d0d55d0a5075b6d2d51a02ac 
> 
> Diff: https://reviews.apache.org/r/34566/diff/
> 
> 
> Testing
> -------
> 
> ./gradlew -Pq build
> ./src/test/sh/org/apache/aurora/e2e/test_kerberos_end_to_end.sh 
> Connecting to H2 in Vagrant.
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Reply via email to