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



examples/vagrant/provision-dev-cluster.sh
<https://reviews.apache.org/r/32559/#comment126478>

    Is this needed to build kerberos in the test script? Do we need to build it 
from source? Can we just apt-get install kerb directly?



src/main/java/org/apache/aurora/scheduler/http/api/security/AuthorizeHeaderToken.java
<https://reviews.apache.org/r/32559/#comment126480>

    These don't appear to be used elsewhere, make them private?



src/main/java/org/apache/aurora/scheduler/http/api/security/AuthorizeHeaderToken.java
<https://reviews.apache.org/r/32559/#comment126477>

    This exception is caught in the servlet filter and its message is logged, 
so it would probably be good to include a message here when we throw it?



src/main/java/org/apache/aurora/scheduler/http/api/security/Kerberos5Realm.java
<https://reviews.apache.org/r/32559/#comment126490>

    Fits on a single line.



src/main/java/org/apache/aurora/scheduler/http/api/security/Kerberos5ShiroRealmModule.java
<https://reviews.apache.org/r/32559/#comment126492>

    Fix.



src/main/java/org/apache/aurora/scheduler/http/api/security/Kerberos5ShiroRealmModule.java
<https://reviews.apache.org/r/32559/#comment126494>

    I feel like it'd be better to just let people specify a path to their jaas 
config on the command line rather than specifying the individual pieces so that 
we can generate a config for them. That way they'd have control over the pieces 
we don't expose as command line args (e.g. debug on/off, etc.).



src/main/java/org/apache/aurora/scheduler/http/api/security/Kerberos5ShiroRealmModule.java
<https://reviews.apache.org/r/32559/#comment126495>

    If we do decide to stick with generating the jaas conf it might be useful 
to either write out the path to the generated file or write out the contents of 
the file to the log so that operators can inspect the config?



src/main/java/org/apache/aurora/scheduler/http/api/security/ShiroUtils.java
<https://reviews.apache.org/r/32559/#comment126501>

    Fill in.



src/test/java/org/apache/aurora/scheduler/http/api/security/Kerberos5ShiroRealmModuleTest.java
<https://reviews.apache.org/r/32559/#comment126503>

    It feels like we're missing a few test cases here. If we keep it, we should 
test the jaas config generation, we should test that the system property is 
restored, etc.



src/test/java/org/apache/aurora/scheduler/http/api/security/KerberosPrincipalParserTest.java
<https://reviews.apache.org/r/32559/#comment126506>

    Add a negative test case?



src/test/java/org/apache/aurora/scheduler/http/api/security/ShiroKerberosAuthenticationFilterTest.java
<https://reviews.apache.org/r/32559/#comment126513>

    It feels like most of this stuff can be tested directly with a mock 
request/response/chain rather than starting up an http server.
    
    I think it's good to have integration tests for this as well, but we can 
probably simplify those to test the happy/sad path for integration tests and 
save testing all the corners for the unit test with mocks.



src/test/java/org/apache/aurora/scheduler/http/api/security/ShiroKerberosAuthenticationFilterTest.java
<https://reviews.apache.org/r/32559/#comment126507>

    kill



src/test/sh/org/apache/aurora/e2e/test_kerberos_end_to_end.sh
<https://reviews.apache.org/r/32559/#comment126514>

    We should undo this change after running this test, otherwise subsequent 
end to end test runs will fail?
    
    Or alternately just set up a separate service for scheduler w/ kerberos and 
stop the normal scheduler, start the kerb one, run tests, then stop/start in 
reverse?


- Joshua Cohen


On March 27, 2015, 1:23 a.m., Kevin Sweeney wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32559/
> -----------------------------------------------------------
> 
> (Updated March 27, 2015, 1:23 a.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Bill Farner.
> 
> 
> Bugs: AURORA-812
>     https://issues.apache.org/jira/browse/AURORA-812
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Support authenticating to the scheduler API with Kerberos.
> 
> 
> Diffs
> -----
> 
>   config/findbugs/excludeFilter.xml 5ff5f87d6bb7d8bf3d9ecb1bc6c6b1a524d1bf15 
>   examples/vagrant/provision-dev-cluster.sh 
> ae500436e703140065e5c16fc0e38dbe3214e69f 
>   examples/vagrant/upstart/aurora-scheduler-kerberos.conf PRE-CREATION 
>   
> src/main/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityModule.java
>  ec6a02c4086ee0d5a7529083030d978ea889f677 
>   
> src/main/java/org/apache/aurora/scheduler/http/api/security/AuthorizeHeaderToken.java
>  PRE-CREATION 
>   
> src/main/java/org/apache/aurora/scheduler/http/api/security/IniShiroRealmModule.java
>  58b559e96402ef282dc0e5ac2de0930906256487 
>   
> src/main/java/org/apache/aurora/scheduler/http/api/security/Kerberos5Realm.java
>  PRE-CREATION 
>   
> src/main/java/org/apache/aurora/scheduler/http/api/security/Kerberos5ShiroRealmModule.java
>  PRE-CREATION 
>   
> src/main/java/org/apache/aurora/scheduler/http/api/security/KerberosPrincipalParser.java
>  PRE-CREATION 
>   
> src/main/java/org/apache/aurora/scheduler/http/api/security/ShiroKerberosAuthenticationFilter.java
>  PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/http/api/security/ShiroUtils.java 
> PRE-CREATION 
>   
> src/main/resources/org/apache/aurora/scheduler/http/api/security/jaas.conf.st 
> PRE-CREATION 
>   
> src/test/java/org/apache/aurora/scheduler/http/api/security/AuthorizeHeaderTokenTest.java
>  PRE-CREATION 
>   
> src/test/java/org/apache/aurora/scheduler/http/api/security/Kerberos5ShiroRealmModuleTest.java
>  PRE-CREATION 
>   
> src/test/java/org/apache/aurora/scheduler/http/api/security/KerberosPrincipalParserTest.java
>  PRE-CREATION 
>   
> src/test/java/org/apache/aurora/scheduler/http/api/security/ShiroKerberosAuthenticationFilterTest.java
>  PRE-CREATION 
>   src/test/sh/org/apache/aurora/e2e/test_kerberos_end_to_end.sh PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/32559/diff/
> 
> 
> Testing
> -------
> 
> ./gradlew -Pq build
> ./src/test/sh/org/apache/aurora/test_kerberos_end_to_end.sh
> 
> 
> Thanks,
> 
> Kevin Sweeney
> 
>

Reply via email to