I'm still of the opinion that we shouldn't be reimplementing this stuff. It
requires developers to learn a custom system where there are already things
out there. It also means that we'll likely have security holes and bugs as
the requirements change. Jersey and the Servlet spec already define a clear
set of specifications around annotation based authorization. Authentication
can be driven to a login page through the built-in systems.
I'm pulling the example below from the Jersey documentation so people can
see the clarity of pattern that exists if we adopt existing solutions.
@Path("/")
@PermitAll
public class Resource {
@RolesAllowed("user")
@GET
public String get() { return "GET"; }
@RolesAllowed("admin")
@POST
public String post(String content) { return content; }
}
In general, I feel like you're approaching this as an extension of
DrillClient authentication and authorization instead of thinking of it as
securing the web ui. In that broader context, there are a number of things
that we might need to do. For example, we may want to allow all users to
have a basic experience without authentication (e.g. jdbc and odbc
connection strings, cluster stats). Then, upon login, they have more
capabilities. This type of variable levels of access fit much better with
the preexisting tools than a custom solution. You argue that these are
harder to maintain. I'm arguing the opposite: that these are easier to
maintain because they are already supported and well documented.
--
Jacques Nadeau
CTO and Co-Founder, Dremio
On Fri, Sep 4, 2015 at 12:11 PM, Venki Korukanti <[email protected]>
wrote:
> Some background on the thread:
> Currently Web UI has no authentication or authorization mechanism. Any user
> can access/update/delete storage profiles or cancel other user queries.
> DRILL-3201 <https://issues.apache.org/jira/browse/DRILL-3201> logged to
> add
> authentication and authorization controls to Web UI to make it usable in
> multi-user cluster environment. JIRA has a sheet listing access control for
> each provided Web API.
>
> Currently JDBC/ODBC has authentication, but no authorization. Currently
> users can't access or update storage plugin config or query profiles
> through JDBC/ODBC. We don't have the support yet. Users can cancel queries
> through JDBC/ODBC, but we don't check for authorization (whether the user
> who is canceling the query has authorization to do so).
>
> Issues we are trying to address apart from the authentication and
> authorization in Web UI are:
> 1) Enhance Drill User API to support access or updating storage plugin
> config and query profiles. Jacques suggested we should add two new sys
> tables (sys.storage and sys.profiles). Access to these is controlled in
> UserServer/UserWorker (or at the place where we try to create the sys
> tables).
> 2) Update UserWorker.cancelQuery to check for authorization
> 3) Currently Web UI access the Drill's internal data structures directory
> for storage config and profiles. We want to change this to use DrillClient
> to access storage or profile resources through queries on (sys.storage or
> sys.profiles). Just like JDBC/ODBC use DrillClient for accessing Drill
> cluster.
> 4) Maintain a DrillClient for each user session through Web UI. Currently
> if a user executes a 'set option' or 'use schema', it is lost and not
> available for subsequent queries.
>
>
> @Jacques:
> The reason why I am not preferring to use LoginService and Jetty/jersey's
> authorization model is:
> 1) We already have or going to add authorization in UserServer/UserWorker.
> WebServer is going to a wrapper around DrillClient (just like JDBC) which
> gets/updates data through UserServer/UserWorker.
> 2) If we add LoginService, which needs its own configuration (we could
> construct JAAS config from existing user authentication config in boot
> config or create a new LoginService implementation, but these options are
> again hard to maintain with changes in authentication support). As we are
> going through DrillClient which already has authentication support, we can
> use the same auth settings for JDBC/ODBC/WebUI.
>
> AuthFilter I created in the patch currently has two tasks:
> 1) each web request has valid user session (through the cookies). If not
> request is forwarded to the login page.
> 2) logged in user has privileges to access restricted resources. As we are
> not planning to implement sys.storage/sys.profiles in v1.2, I need to add
> this check here. This can be removed once we add sys.storage/sys.profiles.
>
> Thanks
> Venki
>
>
> On Fri, Sep 4, 2015 at 8:56 AM, Jacques Nadeau <[email protected]> wrote:
>
> > Adding Dev, which somehow got dropped off.
> >
> > On Fri, Sep 4, 2015 at 8:56 AM, Jacques Nadeau <[email protected]>
> wrote:
> >
> > > My main concern is that we're recreating an authorization management
> > > model. It looks like Jetty has a LoginService approach which would then
> > > integrate with its authorization capabilities around roles/realms/etc.
> > We
> > > also can also then use Servlet 3.x's annotation based security to
> ensure
> > > compliance.
> > >
> > > On Thu, Aug 27, 2015 at 9:18 AM, Venki Korukanti <
> > > [email protected]> wrote:
> > >
> > >> Hi Jacques,
> > >>
> > >> I am looking into details of inbuilt security capabilities in jetty
> and
> > >> jersey. If I understand correctly following are the approaches I
> found:
> > >>
> > >> 1. Add a SecurityHandler (which in turn could include
> > >> FormAuthenticator, LoginService and ConstraintMapping) to
> > >> ServletContextHolder.
> > >> 2. Add custom RolesAllowedResourceFilterFactory and ResourceFilter
> > implementations
> > >> which creates a custom SecurityContext implementation and User
> > details
> > >> (isUserInRole methods). We could add roles annotation to rest
> > methods to
> > >> control authorization.
> > >>
> > >> These approaches again seems to be reimplementing the same
> > authentication
> > >> or authorization code we use in DrillClient.
> > >>
> > >> I am wondering whether you have some time (mostly may take 15mins) to
> > >> discuss these over a hangout.
> > >>
> > >> Thanks
> > >> Venki
> > >>
> > >>
> > >> On Fri, Aug 21, 2015 at 6:05 PM, Jacques Nadeau <
> > [email protected]
> > >> > wrote:
> > >>
> > >>> It really seems like this should be three separate proposed commits.
> > >>> Here are some high level comments on each:
> > >>>
> > >>> -
> > >>>
> > >>> Add SSL to WebUI
> > >>> I'm inclined to switch Drill's default to always doing SSL. We can
> > >>> do this by using a self-signed cert rather than having to
> configure
> > a trust
> > >>> store. If someone wants to control the server cert, then they
> would
> > have to
> > >>> configure a trust store.
> > >>> -
> > >>>
> > >>> Make Rest API methods all use DrillClient
> > >>> As discussed above, I don't think we should be creating a large
> > >>> number of additional RPC wire protocol items. We should continue
> the
> > >>> existing pattern of minimizing API surface and using SQL for
> > operations.
> > >>> These operations are all things that people have also requested
> via
> > SQL.
> > >>> This allows us to have only one entry point for that code instead
> of
> > >>> multiple.
> > >>> -
> > >>>
> > >>> Add WebUI Authentication
> > >>> We need to have a design discussion on JIRA around the method to
> > >>> securing the WebUI calls. It seems like we're implementing a
> custom
> > way to
> > >>> manage security around web requests (including the use of a
> > customer filter
> > >>> and BaseModel. There are a number of standard ways to provide this
> > >>> functionality that will probably be much easier to maintain and
> more
> > >>> secure. For example Jersey has some built in capabilities. There
> is
> > also a
> > >>> bunch of capabilities built into servlets and jetty around
> security
> > >>> constraints and context. Especially for security, I'm inclined to
> > use
> > >>> pre-existing solutions rather than implementing custom solutions.
> > >>>
> > >>> —
> > >>> Reply to this email directly or view it on GitHub
> > >>> <
> >
> https://github.com/vkorukanti/drill/commit/0a01dfdbe7bfe40a4f63a9dfbfdaaeb0d7830329#commitcomment-12840456
> > >
> > >>> .
> > >>>
> > >>
> > >>
> > >
> >
>
