I'm proposing more-or-less the "if SHIRO_ENABLED do shiro_auth else do old_stuff" approach so that the SHIRO_ENABLED branch can throw UnsupportedOperationException for a period of time.
On Wed, Feb 11, 2015 at 4:14 PM, Maxim Khutornenko <ma...@apache.org> wrote: > 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 > >> >> > > > >> >> > > >> >> > >> >
