Right, I was asking about an example of the interim incremental approach you are proposing. Are you envisioning something like "if SHIRO_ENABLED do shiro_auth else do old_stuff" approach? The example you gave shows the end result not what we will see in 0.8.0 (unless you are suggesting moving "old_stuff" behind the new shiro facade).
On Wed, Feb 11, 2015 at 4:08 PM, Kevin Sweeney <kevi...@apache.org> wrote: > I think that would be too big a patch to land all at once. I'd like to add > in Shiro permission checks and annotations incrementally behind this flag > as there are conceptually different changes that will benefit from small > reviews. If I take the approach of ripping out the current framework first > we could be left in an "old way's broken, new way's not finished yet" state. > > On Wed, Feb 11, 2015 at 4:03 PM, Maxim Khutornenko <ma...@apache.org> wrote: > >> Any example? The original code fragment suggest our current security >> model does not map cleanly into shiro. I am +1 on the first pass to >> reduce the "if-else" ugliness if possible. >> >> On Wed, Feb 11, 2015 at 3:57 PM, Kevin Sweeney <kevi...@apache.org> wrote: >> > I'm thinking that flag will control which Guice bindings are applied, so >> > there would be 2 parallel implementations for a bit. This would >> necessitate >> > factoring out capabilityValidator calls to a decorator class (or risk >> > having #ifdef-like logic everywhere in SchedulerThriftInterface). >> > >> > On Wed, Feb 11, 2015 at 3:49 PM, Joshua Cohen <jco...@twopensource.com> >> > wrote: >> > >> >> How do you envision things looking in the intermediate phase where we >> have >> >> support for both security modes? >> >> >> >> I imagine it's easy enough on the Shiro side of if we go with the AOP >> >> annotations for authorization (the interceptor can just check if >> >> security_mode == SHIRO before doing anything), but that means we'd still >> >> have the legacy sessionValidator code in every RPC impl that would need >> to >> >> be wrapped in the inverse check (security_mode == CAPABILITY_VALIDATOR). >> >> >> >> Would it make sense to do a first pass to refactor the existing auth >> >> checking logic to a reusable method, or are we ok living with the >> temporary >> >> ugliness involved in adding that mode checking wrapper to all the >> existing >> >> auth code? >> >> >> >> On Wed, Feb 11, 2015 at 3:23 PM, Kevin Sweeney <kevi...@apache.org> >> wrote: >> >> >> >> > On Wed, Feb 11, 2015 at 3:19 PM, Zameer Manji <zma...@apache.org> >> wrote: >> >> > >> >> > > +1 to this proposal. >> >> > > >> >> > > Will we have dual implementations of API methods as we deprecate the >> >> > > SessionKey based API methods? >> >> > > >> >> > Yes for backwards-compatibility I think we'll need a flag to indicate >> >> which >> >> > system to use. It will probably be an all-or-nothing setting (think >> >> > -security_mode=SHIRO|CAPABILITY_VALIDATOR). >> >> > >> >> > > >> >> > > On Wed, Feb 11, 2015 at 3:13 PM, Kevin Sweeney <kevi...@apache.org> >> >> > wrote: >> >> > > >> >> > > > I've been thinking about revamping the authentication and >> >> authorization >> >> > > in >> >> > > > the scheduler recently. I've investigated Apache Shiro >> >> > > > <http://shiro.apache.org/> and I think it would fit into the >> >> scheduler >> >> > > > nicely as a replacement for our custom CapabilityValidator >> >> > > > < >> >> > > > >> >> > > >> >> > >> >> >> http://people.apache.org/~kevints/aurora/dist/0.5.0-incubating/javadoc/org/apache/aurora/auth/CapabilityValidator.html >> >> > > > > >> >> > > > framework (for which there currently exists no implementation). >> >> > > > >> >> > > > I'd like feedback on this proposal. >> >> > > > Status Quo >> >> > > > >> >> > > > Security is currently implemented by a hand-rolled >> SessionValidator >> >> > > > < >> >> > > > >> >> > > >> >> > >> >> >> http://people.apache.org/~kevints/aurora/dist/0.5.0-incubating/javadoc/org/apache/aurora/auth/SessionValidator.html >> >> > > > > >> >> > > > framework. No public implementations exist. >> >> > > > Proposal >> >> > > > >> >> > > > Change the scheduler to use the Apache Shiro framework for >> >> > authentication >> >> > > > and authorization. Move authentication from application to >> transport >> >> > > layer >> >> > > > and move authorization to the Shiro Permissions model. >> >> > > > Advantages >> >> > > > >> >> > > > A few things that will become possible once this work is complete: >> >> > > > >> >> > > > 1. Ability to configure secure Aurora client-to-scheduler with a >> >> simple >> >> > > > flat configuration file (shiro.ini >> >> > > > <http://shiro.apache.org/configuration.html>). >> >> > > > >> >> > > > 2. Ability to integrate Aurora with my enterprise SSO >> (Kerberos+LDAP >> >> > for >> >> > > > example) by implementing a custom Shiro Realm >> >> > > > <http://shiro.apache.org/realm.html>. >> >> > > > >> >> > > > 3. Ability to allow a CI server to continuously deploy to every >> >> role's >> >> > > > "staging" environment without being able to touch its "prod" one >> by >> >> > using >> >> > > > Shiro's WildcardPermission >> >> > > > < >> >> > > > >> >> > > >> >> > >> >> >> https://shiro.apache.org/static/1.2.3/apidocs/org/apache/shiro/authz/permission/WildcardPermission.html >> >> > > > > >> >> > > > . >> >> > > > >> >> > > > 4. Ability to authenticate to the scheduler API using Kerberos >> (via >> >> > > SPNEGO >> >> > > > <http://spnego.sourceforge.net/>) or HTTP Basic auth. >> >> > > > >> >> > > > 5. Ability to perform authenticated write operations on a job via >> the >> >> > web >> >> > > > UI >> >> > > > < >> >> > >> http://www.chromium.org/developers/design-documents/http-authentication >> >> > > >. >> >> > > > Suggested Reading >> >> > > > >> >> > > > Shiro has excellent documentation and is a fellow Apache >> Foundation >> >> > > > project. I suggest you check out at least the 10-minute tutorial >> >> > > > <http://shiro.apache.org/10-minute-tutorial.html> and the Guice >> >> > > > integration >> >> > > > documentation <http://shiro.apache.org/guice.html>. >> >> > > > Scheduler-side changes >> >> > > > >> >> > > > The best way to show the proposed changes is by example. In >> addition >> >> to >> >> > > > Guice wiring changes to place the Shiro authentication filter into >> >> the >> >> > > > request chain, code that previously looked like >> >> > > > >> >> > > > @Override >> >> > > > >> >> > > > public Response createJob( >> >> > > > >> >> > > > JobConfiguration mutableJob, >> >> > > > >> >> > > > @Nullable final Lock mutableLock, >> >> > > > >> >> > > > SessionKey session) { >> >> > > > >> >> > > > requireNonNull(session); >> >> > > > >> >> > > > try { >> >> > > > >> >> > > > sessionValidator.checkAuthenticated( >> >> > > > >> >> > > > session, >> >> > > > >> >> > > > ImmutableSet.of(mutableJob.getKey().getRole())); >> >> > > > >> >> > > > } catch (AuthFailedException e) { >> >> > > > >> >> > > > return errorResponse(AUTH_FAILED, e); >> >> > > > >> >> > > > } >> >> > > > >> >> > > > // Request is authenticated and authorized, continue. >> >> > > > >> >> > > > } >> >> > > > >> >> > > > becomes >> >> > > > >> >> > > > @Override >> >> > > > >> >> > > > public Response createJob( >> >> > > > >> >> > > > JobConfiguration mutableJob, >> >> > > > >> >> > > > @Nullable final Lock mutableLock) { >> >> > > > >> >> > > > // subject is injected in the constructor by Guice each >> request. >> >> > > > >> >> > > > // checkPermission will throw an unchecked >> >> > > > >> >> > > > // AuthorizationException that bubbles up as a 401. >> >> > > > >> >> > > > // This line could also be inserted by inspection of the method >> >> > > > >> >> > > > // call in a security AOP layer. >> >> > > > >> >> > > > subject.checkPermission( >> >> > > > >> >> > > > // A Shiro WildcardPermission job:create:mesos:prod:labrat >> >> > > > >> >> > > > new JobScopedPermission("job:create", mutableJob.getKey())); >> >> > > > >> >> > > > // Request is authenticated and authorized, continue. >> >> > > > >> >> > > > } >> >> > > > >> >> > > > Some admin methods are protected by annotations like >> >> > > > >> >> > > > @Requires(Capability.PROVISIONER) >> >> > > > >> >> > > > public Response startMaintenance(Set<String> hosts, SessionKey >> >> session) >> >> > > { … >> >> > > > } >> >> > > > >> >> > > > They'd become >> >> > > > >> >> > > > @RequiresPermission("maintenance:create") >> >> > > > >> >> > > > public Response startMaintenance(Set<String> hosts) { … } >> >> > > > Client-side changes >> >> > > > >> >> > > > No changes are necessary to use HTTP Basic Auth - requests will >> >> > > > automatically use a .netrc file today. >> >> > > > >> >> > > > An optional dependency on kerberos and requests-kerberos can be >> added >> >> > to >> >> > > > support SPNEGO authentication. >> >> > > > Timeline >> >> > > > >> >> > > > I would like to land support for HTTP Basic Auth and SPNEGO in >> 0.8.0 >> >> > and >> >> > > > deprecate the SessionKey-based API for authentication in favor of >> >> fully >> >> > > > transport-based authentication. >> >> > > > >> >> > > > In 0.9.0 I propose removing SessionKey from the API entirely along >> >> with >> >> > > > SessionValidator from the scheduler. >> >> > > > >> >> > > >> >> > > >> >> > > >> >> > > -- >> >> > > Zameer Manji >> >> > > >> >> > >> >> >>
