[
https://issues.apache.org/jira/browse/CALCITE-2294?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16483069#comment-16483069
]
ASF GitHub Bot commented on CALCITE-2294:
-----------------------------------------
Github user joshelser commented on a diff in the pull request:
https://github.com/apache/calcite-avatica/pull/48#discussion_r189716276
--- Diff:
server/src/main/java/org/apache/calcite/avatica/server/HttpServer.java ---
@@ -239,18 +239,8 @@ protected void internalStart() {
server.setConnectors(new Connector[] { connector });
// Default to using the handler that was passed in
- final HandlerList handlerList = new HandlerList();
- Handler avaticaHandler = handler;
-
- // Wrap the provided handler for security if we made one
- if (null != securityHandler) {
- securityHandler.setHandler(handler);
- avaticaHandler = securityHandler;
- }
+ configureHandlers(securityHandler);
--- End diff --
> one can directly remove all the connectors , security handlers and other
relevant properties from the server, which can potentially lead to unexpected
bugs.
Yeah, this is the angle I'm coming from. I can appreciate the flexibility
of `CUSTOM`, but am worried if that's "safe" for the common-folk.
> I believe that AvaticaServerConfiguration interface should provide
methods to authenticate the user.
I think that makes sense. My biggest worry about
`AvaticaServerConfiguration` is that it's started to become a "dumping ground".
Every time I've added a new authn option, it results in an implementation
specific option that is only valid "sometimes". Not saying that what you're
doing here requires an immediate re-write, just trying to give you my
perspective :)
> Ideally this method should be wrapped around every handler that is
implemented, since I can potentially add a new Handler and forget to call this
method at the start
+1
> If you can point me to the code that helps in such wrapping, I am happy
to do the refactoring.
`o.a.c.a.server.HandlerFactory` is what instantiates the `Avatica*Handler`
classes now. I think you'd just need to pass the `ASConfiguration` (or some
interface which just has the authz/impersonation related methods -- the Avatica
handlers shouldn't need to know all HTTP server configuration details) into the
construction of the Handler.
This would be some great follow-on.
> Allow customization for AvaticaServerConfiguration for plugging new
> authentication mechanisms
> ---------------------------------------------------------------------------------------------
>
> Key: CALCITE-2294
> URL: https://issues.apache.org/jira/browse/CALCITE-2294
> Project: Calcite
> Issue Type: Improvement
> Components: avatica
> Reporter: Karan Mehta
> Priority: Major
>
> {{AvaticaServerConfiguration}} is currently only created if authentication
> mechanism such as {{BASIC, DIGEST or SPNEGO}} is provided. We can change it
> to a builder pattern to create this object and provide a way for users to
> plugin their own security configuration.
> An example here can be using it for custom config that supports MTLS.
> Thanks [~alexaraujo] for suggesting this approach.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
