> On Jan. 21, 2016, 6:44 p.m., Zameer Manji wrote:
> > src/main/java/org/apache/aurora/scheduler/http/api/security/HttpSecurityModule.java,
> >  line 80
> > <https://reviews.apache.org/r/42565/diff/3/?file=1203749#file1203749line80>
> >
> >     Just to be clear, this command line argument will only accept two 
> > modules, the ini realm module and the kerberos realm module?
> >     
> >     This means users who need to setup a custom realm (probably any medium 
> > to large cluster operator), must create a custom main.
> >     
> >     I'm unsure if this is the ideal approach to take w.r.t security. I 
> > think this requires some discussion on the mailing list. For example, an 
> > alternative to what you have done here would be to remove this argument 
> > entirely, and request users specify realms in the ini file (see 
> > http://shiro.apache.org/configuration.html section "INI Sections").

In case the concern about a custom main is that it sounds dautning, here's an 
illustration:

```
public class CustomMain {
  public static void main(String[] args) {
    SchedulerMain.applyStaticArgumentValues(args);
    SchedulerMain.publicMain(new CustomRealmModule());
  }
}
```

(It may be worth building in the `applyStaticArgumentValues` step, so we can 
reduce even further.)

I see 2 upsides with this:
- The API for customization (whether security or other arbitrary feature 
addition) is uniform.
- The `CustomMain` can use the same commane line arguments system as the rest 
of the scheduler (this will work with the forthcoming replacement as well).

I believe your suggestion to use an INI section falls apart in 2 ways:
- the `[main]` section only applies when using pure INI configuration for 
shiro. I believe we fall under the programmatic configuration section, with 
shiro-guice doing the programmatic bits.  The ini file aurora accepts is read 
by `org.apache.shiro.realm.text.IniRealm` and it only uses the `users` and 
`roles` sections (see 
https://shiro.apache.org/static/1.2.3/apidocs/org/apache/shiro/realm/text/IniRealm.html)
- i don't believe classes configured with pure shiro INI configuration would 
participate in guice injection

I'm open to ideas here.  Ultimately i'm trying to preserve integration of 
custom secruity controls with features that exist today:
1. support for participation in guice injection
2. support for participation in the same configuration system as the rest of 
the scheduler

If (1) is not necessary, that simplifies things.  An alternative to (2) is to 
force custom code to handle its own configuration (e.g. with environment 
variables).


- Bill


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


On Jan. 20, 2016, 12:21 p.m., Bill Farner wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42565/
> -----------------------------------------------------------
> 
> (Updated Jan. 20, 2016, 12:21 p.m.)
> 
> 
> Review request for Aurora, John Sirois and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> This patch proposes that users installing custom modules do so via a custom 
> main.
> 
> Specifying custom modules on the command line has proven troublesome for 
> replacing the command line args system with one where all arguments are 
> injected and explicitly-defined.  It also adds complexity to the scheduler 
> application by unnecessarily punching holes at specific places.
> 
> If this proposal is agreeable, i will add a NEWS entry and document how one 
> might implement a custom main to add modules.  The tl;dr, however, is to 
> invoke `SchedulerMain.publicMain(customModule)`
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/aurora/scheduler/app/MoreModules.java 
> d5f96543d1068bf30b9d173a247c2806faf35578 
>   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
> 0659c358479283756179c2cabebc8416730cc3e3 
>   
> src/main/java/org/apache/aurora/scheduler/http/api/security/HttpSecurityModule.java
>  e32862034a1ad47dae8fff89cb04deb34ccd90e2 
>   
> src/main/java/org/apache/aurora/scheduler/http/api/security/ModuleParser.java 
> ccd9a20e8b18831458cba2d53e6b8b84fef06162 
>   src/test/java/org/apache/aurora/scheduler/app/MoreModulesTest.java 
> b2fb3c9dcba64f69a05894f506ba43766f74ddaa 
>   
> src/test/java/org/apache/aurora/scheduler/http/api/security/HttpSecurityIT.java
>  3e811a4f4d2c82892217ca1f950ac792303fbcf3 
>   
> src/test/java/org/apache/aurora/scheduler/http/api/security/ModuleParserTest.java
>  baaeb2390a909de1a92e4328d35a49f7b74c36cb 
> 
> Diff: https://reviews.apache.org/r/42565/diff/
> 
> 
> Testing
> -------
> 
> end-to-end tests, `./gradlew run`
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Reply via email to