DI-1113. ADDENDUM. Authentication: Enable user impersonation for Superset to HiveServer2 using hive.server2.proxy.user (a.fernandez) #3697

afernandez · 2017-10-20T02:09:04Z

Figured out how to make changes to configuration dictionary instead of having to patch the PyHive library. When connecting to Hive, will determine if impersonation is enabled, and if so, will send the currently logged on user via the hive.server2.proxy.user property in the configuration dictionary to SQLAlchemy's create_engine function.

Tested this on my local Superset instance, both Test Connection and Run SQL Query.

afernandez

cc @mistercrunch @timifasubaa

afernandez · 2017-10-20T02:09:22Z

superset/db_engines/hive.py

@@ -3,27 +3,6 @@
 from thrift import Thrift


-old_Connection = hive.Connection


No longer need to patch this.

afernandez · 2017-10-20T02:09:37Z

superset/models/core.py

+            engine_params['poolclass'] = NullPool
+
+        configuration = {}
+        configuration.update(


Update configuration dictionary instead.

afernandez · 2017-10-20T02:09:42Z

superset/views/core.py

            logging.info("Superset.testconn(). Masked URL: {0}".format(masked_url))

+            configuration = {}


Update configuration dictionary instead.

afernandez · 2017-10-20T02:10:08Z

superset/db_engine_specs.py

@@ -187,26 +187,16 @@ def select_star(cls, my_db, table_name, schema=None, limit=100,
        return sql

    @classmethod
-    def modify_url_for_impersonation(cls, url, impersonate_user, username):
+    def get_configuration_for_impersonation(cls, uri, impersonate_user, username):


Update the configuration dictionary instead of the URL's query dictionary so that we don't have to modify PyHive library.

afernandez · 2017-10-20T18:29:32Z

Fixing unit tests right now.

coveralls · 2017-10-20T23:23:01Z

Coverage increased (+0.2%) to 70.947% when pulling 2d592858a269dbdc008b8dc8aaec6d105b5f6d90 on afernandez:afernandez_impersonate into e121a85 on apache:master.

coveralls · 2017-10-20T23:23:01Z

Coverage increased (+0.2%) to 70.947% when pulling 2d592858a269dbdc008b8dc8aaec6d105b5f6d90 on afernandez:afernandez_impersonate into e121a85 on apache:master.

coveralls · 2017-10-20T23:23:01Z

Coverage increased (+0.2%) to 70.947% when pulling 2d592858a269dbdc008b8dc8aaec6d105b5f6d90 on afernandez:afernandez_impersonate into e121a85 on apache:master.

mistercrunch

A few minor nits otherwise LGTM

mistercrunch · 2017-10-25T22:28:38Z

superset/views/core.py


            connect_args = (
                request.json
                .get('extras', {})
                .get('engine_params', {})
                .get('connect_args', {}))
+
+            if len(configuration.keys()) > 0:


Same as "pythonesquier" if configurations:

mistercrunch · 2017-10-25T22:29:38Z

superset/views/core.py


-            logging.info("Superset.testconn(). Masked URL: {0}".format(masked_url))
+                configuration.update(db_engine.get_configuration_for_impersonation(uri, impersonate_user, username))


this line if very long, goes way beyond 90chars

mistercrunch · 2017-10-25T22:29:59Z

superset/views/core.py

+
+            configuration = {}
+
+            if database is not None and uri is not None:


if database and uri:

…rset to HiveServer2 using hive.server2.proxy.user (a.fernandez)

afernandez

Fixed comments

afernandez · 2017-11-03T23:39:11Z

superset/models/core.py

+            self.db_engine_spec.get_configuration_for_impersonation(str(url),
+                                                                    self.impersonate_user,
+                                                                    effective_username))
+        if configuration:


@mistercrunch fixed the long line.

afernandez · 2017-11-03T23:39:25Z

superset/models/core.py

+            self.db_engine_spec.get_configuration_for_impersonation(str(url),
+                                                                    self.impersonate_user,
+                                                                    effective_username))
+        if configuration:


@mistercrunch fixed this.

afernandez · 2017-11-03T23:39:33Z

superset/views/core.py

+
+            configuration = {}
+
+            if database and uri:


@mistercrunch fixed this.

coveralls · 2017-11-03T23:55:43Z

Coverage increased (+0.2%) to 71.461% when pulling 69d49ed on afernandez:afernandez_impersonate into 7f3edad on apache:master.

mistercrunch · 2017-11-04T07:09:25Z

LGTM, you can sync with @graceguo-supercat to merge and deploy

ottomata · 2018-01-08T20:36:14Z

We use AUTH_TYPE = AUTH_REMOTE_USER for authentication, and set REMOTE_USER to the authenticated LDAP user's shell username via Apache mod_ldap in front of superset. I'd like for folks that authenticate this way to be able to run hive queries as their own user.

2 questions:

Why check 'auth' in url.query.keys() when deciding whether or not to set this. If impersonate_user and username is not None, shouldn't that be enough? I have to configure my Hive database connection with ?auth=NONE to get this to work. (To do so I also annoyingly have to specify a database in the URI, as hostname:10000?auth=NONE seems to try and parse the full 10000?auth=NONE as the integer port for the URI.)
When I do set ?auth=NONE in the URI, testing the database connection seems fine. But, saving the database connection fails with: __init__() got an unexpected keyword argument 'hive_server2_proxy_user', and fails to save the change. Upon saving, I see INFO:root:Database.get_sqla_engine(). Masked URL: hive://hostname:10000/default?auth=NONE&hive_server2_proxy_user=otto in the superset logs, which I think looks mostly good, buuuut, where are those underscores coming from? The code explicitly sets configuration['hive.server2.proxy.user'], so something must be inserting them.

ottomata · 2018-01-08T20:38:25Z

BTW, I've also tried to save the URI as hive://hostname:10000/default?hive.server2.proxy.user=otto, just to see if that works, but it gives the similar error message of __init__() got an unexpected keyword argument 'hive.server2.proxy.user' with INFO:root:Database.get_sqla_engine(). Masked URL: hive://hostname:10000/default?hive.server2.proxy.user=otto in the superset logs

…rset to HiveServer2 using hive.server2.proxy.user (a.fernandez) (apache#3697)

afernandez commented Oct 20, 2017

View reviewed changes

afernandez force-pushed the afernandez_impersonate branch from a6183fd to 2d59285 Compare October 20, 2017 22:55

mistercrunch approved these changes Oct 25, 2017

View reviewed changes

DI-1113. ADDENDUM. Authentication: Enable user impersonation for Supe…

69d49ed

…rset to HiveServer2 using hive.server2.proxy.user (a.fernandez)

afernandez force-pushed the afernandez_impersonate branch from 2d59285 to 69d49ed Compare November 3, 2017 23:38

afernandez commented Nov 3, 2017

View reviewed changes

graceguo-supercat merged commit b059506 into apache:master Nov 6, 2017

michellethomas pushed a commit to michellethomas/panoramix that referenced this pull request May 24, 2018

DI-1113. ADDENDUM. Authentication: Enable user impersonation for Supe…

f98c3f2

…rset to HiveServer2 using hive.server2.proxy.user (a.fernandez) (apache#3697)

wenchma pushed a commit to wenchma/incubator-superset that referenced this pull request Nov 16, 2018

DI-1113. ADDENDUM. Authentication: Enable user impersonation for Supe…

261521b

…rset to HiveServer2 using hive.server2.proxy.user (a.fernandez) (apache#3697)

mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 0.21.0 labels Feb 27, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DI-1113. ADDENDUM. Authentication: Enable user impersonation for Superset to HiveServer2 using hive.server2.proxy.user (a.fernandez) #3697

DI-1113. ADDENDUM. Authentication: Enable user impersonation for Superset to HiveServer2 using hive.server2.proxy.user (a.fernandez) #3697

afernandez commented Oct 20, 2017

afernandez left a comment

afernandez Oct 20, 2017

afernandez Oct 20, 2017

afernandez Oct 20, 2017

afernandez Oct 20, 2017

afernandez commented Oct 20, 2017

coveralls commented Oct 20, 2017

coveralls commented Oct 20, 2017

coveralls commented Oct 20, 2017 •

edited

Loading

mistercrunch left a comment

mistercrunch Oct 25, 2017

mistercrunch Oct 25, 2017

mistercrunch Oct 25, 2017

afernandez left a comment

afernandez Nov 3, 2017

afernandez Nov 3, 2017

afernandez Nov 3, 2017

coveralls commented Nov 3, 2017 •

edited

Loading

mistercrunch commented Nov 4, 2017

ottomata commented Jan 8, 2018

ottomata commented Jan 8, 2018

		@@ -3,27 +3,6 @@
		from thrift import Thrift


		old_Connection = hive.Connection

		logging.info("Superset.testconn(). Masked URL: {0}".format(masked_url))

		configuration = {}


		logging.info("Superset.testconn(). Masked URL: {0}".format(masked_url))
		configuration.update(db_engine.get_configuration_for_impersonation(uri, impersonate_user, username))


		configuration = {}

		if database is not None and uri is not None:

DI-1113. ADDENDUM. Authentication: Enable user impersonation for Superset to HiveServer2 using hive.server2.proxy.user (a.fernandez) #3697

DI-1113. ADDENDUM. Authentication: Enable user impersonation for Superset to HiveServer2 using hive.server2.proxy.user (a.fernandez) #3697

Conversation

afernandez commented Oct 20, 2017

afernandez left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

afernandez commented Oct 20, 2017

coveralls commented Oct 20, 2017

coveralls commented Oct 20, 2017

coveralls commented Oct 20, 2017 • edited Loading

mistercrunch left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

afernandez left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coveralls commented Nov 3, 2017 • edited Loading

mistercrunch commented Nov 4, 2017

ottomata commented Jan 8, 2018

ottomata commented Jan 8, 2018

coveralls commented Oct 20, 2017 •

edited

Loading

coveralls commented Nov 3, 2017 •

edited

Loading